[Asterisk-code-review] res stasis device state: Protect the adding/removing of subs... (asterisk[14])

[Joshua Colp]

Joshua Colp has uploaded a new change for review. ( https://gerrit.asterisk.org/4891 )

Change subject: res_stasis_device_state: Protect the adding/removing of subscriptions.
......................................................................

res_stasis_device_state: Protect the adding/removing of subscriptions.

The adding and removing of device state subscriptions did not protect
fully against simultaneous manipulation. In particular the subscribe
case allowed a small window where two subscriptions could be added for
the same device state instead of just one.

This change makes the code hold the subscriptions lock for the entirety
of each operation to ensure that two are not occurring at the same time.

ASTERISK-26770

Change-Id: I3e7f8eb9d09de440c9024d2dd52029f6f20e725b
---
M res/res_stasis_device_state.c
1 file changed, 17 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/91/4891/1

diff --git a/res/res_stasis_device_state.c b/res/res_stasis_device_state.c
index 453bfae..71c4161 100644
--- a/res/res_stasis_device_state.c
+++ b/res/res_stasis_device_state.c
@@ -148,13 +148,13 @@
 		.device_name = name
 	};
 
-	return ao2_find(device_state_subscriptions, &dummy_sub, OBJ_SEARCH_OBJECT);
+	return ao2_find(device_state_subscriptions, &dummy_sub, OBJ_SEARCH_OBJECT | OBJ_NOLOCK);
 }
 
 static void remove_device_state_subscription(
 	struct device_state_subscription *sub)
 {
-	ao2_unlink(device_state_subscriptions, sub);
+	ao2_unlink_flags(device_state_subscriptions, sub, OBJ_NOLOCK);
 }
 
 struct ast_json *stasis_app_device_state_to_json(
@@ -364,7 +364,10 @@
 		topic = ast_device_state_topic_all();
 	}
 
+	ao2_lock(device_state_subscriptions);
+
 	if (is_subscribed_device_state(app, sub->device_name)) {
+		ao2_unlock(device_state_subscriptions);
 		ast_debug(3, "App %s is already subscribed to %s\n", stasis_app_name(app), sub->device_name);
 		return 0;
 	}
@@ -373,6 +376,7 @@
 
 	sub->sub = stasis_subscribe_pool(topic, device_state_cb, ao2_bump(sub));
 	if (!sub->sub) {
+		ao2_unlock(device_state_subscriptions);
 		ast_log(LOG_ERROR, "Unable to subscribe to device %s\n",
 			sub->device_name);
 		/* Reference we added when attempting to stasis_subscribe_pool */
@@ -380,15 +384,23 @@
 		return -1;
 	}
 
-	ao2_link(device_state_subscriptions, sub);
+	ao2_link_flags(device_state_subscriptions, sub, OBJ_NOLOCK);
+	ao2_unlock(device_state_subscriptions);
+
 	return 0;
 }
 
 static int unsubscribe_device_state(struct stasis_app *app, const char *name)
 {
-	RAII_VAR(struct device_state_subscription *, sub,
-		 find_device_state_subscription(app, name), ao2_cleanup);
+	struct device_state_subscription *sub;
+
+	ao2_lock(device_state_subscriptions);
+	sub = find_device_state_subscription(app, name);
 	remove_device_state_subscription(sub);
+	ao2_unlock(device_state_subscriptions);
+
+	ao2_cleanup(sub);
+
 	return 0;
 }
 

-- 
To view, visit https://gerrit.asterisk.org/4891
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3e7f8eb9d09de440c9024d2dd52029f6f20e725b
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Joshua Colp <jcolp at digium.com>