[asterisk-commits] rmudgett: trunk r402595 - in /trunk: ./ res/res_stasis.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Nov 8 14:37:11 CST 2013


Author: rmudgett
Date: Fri Nov  8 14:37:08 2013
New Revision: 402595

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=402595
Log:
res_stasis.c: Fix locking issues with the app_bridge_moh container.

* Fix unlinking from the app_bridges_moh container in remove_bridge_moh()
without a lock under normal circumstances.

* Made check ast_bridge_set_after_callback() return value in
bridge_moh_create() to handle failure.

* Fixed SCOPED_AO2LOCK() locking over too much scope in
stasis_app_bridge_moh_channel() and stasis_app_bridge_moh_stop().

* Fixed unusual usage of ao2_unlink_flag() in control_unlink().

* Fixed orphaned bridge from off nominal path in
stasis_app_bridge_create().

* Fixed strange construct in stasis_app_unsubscribe().  From a bad merge?

* Made load_module() cleanup on failure.

Review: https://reviewboard.asterisk.org/r/2962/
........

Merged revisions 402593 from http://svn.asterisk.org/svn/asterisk/branches/12

Modified:
    trunk/   (props changed)
    trunk/res/res_stasis.c

Propchange: trunk/
------------------------------------------------------------------------------
--- branch-12-merged (original)
+++ branch-12-merged Fri Nov  8 14:37:08 2013
@@ -1,1 +1,1 @@
-/branches/12:1-398558,398560-398577,398579-399305,399307-401390,401392-402570,402582,402584
+/branches/12:1-398558,398560-398577,398579-399305,399307-401390,401392-402570,402582,402584,402593

Modified: trunk/res/res_stasis.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_stasis.c?view=diff&rev=402595&r1=402594&r2=402595
==============================================================================
--- trunk/res/res_stasis.c (original)
+++ trunk/res/res_stasis.c Fri Nov  8 14:37:08 2013
@@ -403,11 +403,7 @@
 /*! Removes the bridge to music on hold channel link */
 static void remove_bridge_moh(char *bridge_id)
 {
-	RAII_VAR(struct stasis_app_bridge_moh_wrapper *, moh_wrapper, ao2_find(app_bridges_moh, bridge_id, OBJ_SEARCH_KEY), ao2_cleanup);
-
-	if (moh_wrapper) {
-		ao2_unlink_flags(app_bridges_moh, moh_wrapper, OBJ_NOLOCK);
-	}
+	ao2_find(app_bridges_moh, bridge_id, OBJ_SEARCH_KEY | OBJ_UNLINK | OBJ_NODATA);
 	ast_free(bridge_id);
 }
 
@@ -448,7 +444,8 @@
 {
 	struct ast_channel *moh_channel = data;
 
-	while (!ast_safe_sleep(moh_channel, 1000));
+	while (!ast_safe_sleep(moh_channel, 1000)) {
+	}
 
 	ast_moh_stop(moh_channel);
 	ast_hangup(moh_channel);
@@ -477,14 +474,16 @@
 	}
 
 	chan = prepare_bridge_moh_channel();
-
 	if (!chan) {
 		return NULL;
 	}
 
 	/* The after bridge callback assumes responsibility of the bridge_id. */
-	ast_bridge_set_after_callback(chan, moh_after_bridge_cb, moh_after_bridge_cb_failed, bridge_id);
-
+	if (ast_bridge_set_after_callback(chan,
+		moh_after_bridge_cb, moh_after_bridge_cb_failed, bridge_id)) {
+		ast_hangup(chan);
+		return NULL;
+	}
 	bridge_id = NULL;
 
 	if (ast_unreal_channel_push_to_bridge(chan, bridge,
@@ -493,7 +492,8 @@
 		return NULL;
 	}
 
-	new_wrapper = ao2_alloc_options(sizeof(*new_wrapper), stasis_app_bridge_moh_wrapper_destructor, AO2_ALLOC_OPT_LOCK_NOLOCK);
+	new_wrapper = ao2_alloc_options(sizeof(*new_wrapper),
+		stasis_app_bridge_moh_wrapper_destructor, AO2_ALLOC_OPT_LOCK_NOLOCK);
 	if (!new_wrapper) {
 		ast_hangup(chan);
 		return NULL;
@@ -503,11 +503,10 @@
 		ast_hangup(chan);
 		return NULL;
 	}
-
 	ast_string_field_set(new_wrapper, bridge_id, bridge->uniqueid);
 	ast_string_field_set(new_wrapper, channel_id, ast_channel_uniqueid(chan));
 
-	if (!ao2_link(app_bridges_moh, new_wrapper)) {
+	if (!ao2_link_flags(app_bridges_moh, new_wrapper, OBJ_NOLOCK)) {
 		ast_hangup(chan);
 		return NULL;
 	}
@@ -526,13 +525,13 @@
 {
 	RAII_VAR(struct stasis_app_bridge_moh_wrapper *, moh_wrapper, NULL, ao2_cleanup);
 
-	SCOPED_AO2LOCK(lock, app_bridges_moh);
-
-	moh_wrapper = ao2_find(app_bridges_moh, bridge->uniqueid, OBJ_SEARCH_KEY | OBJ_NOLOCK);
-
-	if (!moh_wrapper) {
-		struct ast_channel *bridge_moh_channel = bridge_moh_create(bridge);
-		return bridge_moh_channel;
+	{
+		SCOPED_AO2LOCK(lock, app_bridges_moh);
+
+		moh_wrapper = ao2_find(app_bridges_moh, bridge->uniqueid, OBJ_SEARCH_KEY | OBJ_NOLOCK);
+		if (!moh_wrapper) {
+			return bridge_moh_create(bridge);
+		}
 	}
 
 	return ast_channel_get_by_name(moh_wrapper->channel_id);
@@ -543,10 +542,7 @@
 	RAII_VAR(struct stasis_app_bridge_moh_wrapper *, moh_wrapper, NULL, ao2_cleanup);
 	struct ast_channel *chan;
 
-	SCOPED_AO2LOCK(lock, app_bridges_moh);
-
-	moh_wrapper = ao2_find(app_bridges_moh, bridge->uniqueid, OBJ_SEARCH_KEY | OBJ_NOLOCK);
-
+	moh_wrapper = ao2_find(app_bridges_moh, bridge->uniqueid, OBJ_SEARCH_KEY | OBJ_UNLINK);
 	if (!moh_wrapper) {
 		return -1;
 	}
@@ -559,8 +555,6 @@
 	ast_moh_stop(chan);
 	ast_softhangup(chan, AST_CAUSE_NORMAL_CLEARING);
 	ao2_cleanup(chan);
-
-	ao2_unlink_flags(app_bridges_moh, moh_wrapper, OBJ_NOLOCK);
 
 	return 0;
 }
@@ -582,15 +576,15 @@
 		return;
 	}
 
-	ao2_unlink_flags(app_controls, control,
-		OBJ_SEARCH_OBJECT | OBJ_UNLINK | OBJ_NODATA);
+	ao2_unlink(app_controls, control);
 	ao2_cleanup(control);
 }
 
 struct ast_bridge *stasis_app_bridge_create(const char *type)
 {
 	struct ast_bridge *bridge;
-	int capabilities, flags = AST_BRIDGE_FLAG_MERGE_INHIBIT_FROM | AST_BRIDGE_FLAG_MERGE_INHIBIT_TO
+	int capabilities;
+	int flags = AST_BRIDGE_FLAG_MERGE_INHIBIT_FROM | AST_BRIDGE_FLAG_MERGE_INHIBIT_TO
 		| AST_BRIDGE_FLAG_SWAP_INHIBIT_FROM | AST_BRIDGE_FLAG_SWAP_INHIBIT_TO
 		| AST_BRIDGE_FLAG_TRANSFER_PROHIBITED;
 
@@ -607,7 +601,10 @@
 
 	bridge = ast_bridge_base_new(capabilities, flags);
 	if (bridge) {
-		ao2_link(app_bridges, bridge);
+		if (!ao2_link(app_bridges, bridge)) {
+			ast_bridge_destroy(bridge, 0);
+			bridge = NULL;
+		}
 	}
 	return bridge;
 }
@@ -1072,12 +1069,12 @@
 	int i;
 
 	if (app_name) {
+		app = ao2_find(apps_registry, app_name, OBJ_SEARCH_KEY);
+	}
+
+	if (!app) {
 		ast_log(LOG_WARNING, "Could not find app '%s'\n",
 			app_name ? : "(null)");
-		app = ao2_find(apps_registry, app_name, OBJ_SEARCH_KEY);
-	}
-
-	if (!app) {
 		return STASIS_ASR_APP_NOT_FOUND;
 	}
 
@@ -1139,49 +1136,37 @@
 	ast_module_unref(ast_module_info->self);
 }
 
+static int unload_module(void)
+{
+	ao2_cleanup(apps_registry);
+	apps_registry = NULL;
+
+	ao2_cleanup(app_controls);
+	app_controls = NULL;
+
+	ao2_cleanup(app_bridges);
+	app_bridges = NULL;
+
+	ao2_cleanup(app_bridges_moh);
+	app_bridges_moh = NULL;
+
+	return 0;
+}
+
 static int load_module(void)
 {
-	apps_registry =	ao2_container_alloc(APPS_NUM_BUCKETS, app_hash,
-		app_compare);
-	if (apps_registry == NULL) {
-		return AST_MODULE_LOAD_FAILURE;
-	}
-
-	app_controls = ao2_container_alloc(CONTROLS_NUM_BUCKETS, control_hash,
-		control_compare);
-	if (app_controls == NULL) {
-		return AST_MODULE_LOAD_FAILURE;
-	}
-
-        app_bridges = ao2_container_alloc(BRIDGES_NUM_BUCKETS, bridges_hash,
-		bridges_compare);
-
+	apps_registry = ao2_container_alloc(APPS_NUM_BUCKETS, app_hash, app_compare);
+	app_controls = ao2_container_alloc(CONTROLS_NUM_BUCKETS, control_hash, control_compare);
+	app_bridges = ao2_container_alloc(BRIDGES_NUM_BUCKETS, bridges_hash, bridges_compare);
 	app_bridges_moh = ao2_container_alloc_hash(
 		AO2_ALLOC_OPT_LOCK_MUTEX, AO2_CONTAINER_ALLOC_OPT_DUPS_REJECT,
 		37, bridges_moh_hash_fn, bridges_moh_sort_fn, NULL);
-
-	if (!app_bridges_moh) {
+	if (!apps_registry || !app_controls || !app_bridges || !app_bridges_moh) {
+		unload_module();
 		return AST_MODULE_LOAD_FAILURE;
 	}
 
 	return AST_MODULE_LOAD_SUCCESS;
-}
-
-static int unload_module(void)
-{
-	ao2_cleanup(apps_registry);
-	apps_registry = NULL;
-
-	ao2_cleanup(app_controls);
-	app_controls = NULL;
-
-	ao2_cleanup(app_bridges);
-	app_bridges = NULL;
-
-	ao2_cleanup(app_bridges_moh);
-	app_bridges_moh = NULL;
-
-	return 0;
 }
 
 AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_GLOBAL_SYMBOLS, "Stasis application support",




More information about the asterisk-commits mailing list