[asterisk-commits] rmudgett: trunk r390975 - in /trunk: bridges/ include/asterisk/ main/

[SVN commits to the Asterisk project]

Author: rmudgett
Date: Sat Jun  8 00:18:22 2013
New Revision: 390975

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=390975
Log:
Fix a crash when a bridge switches from the softmix bridge technology to another.

A three party bridge uses the softmix bridging technology.  This
technology has a dedicated thread used to perform the analog mixing.  When
one of these parties leaves the bridge, the bridge technology is changed
from the softmix technology to a two-party mixing technology.  Changing
technologies is done by removing channels from the old technology and
adding them to the new technology.  Since the remaining channels do not
leave the bridge, the softmix mixing thread could continue to process all
channels in the bridge.  If the bridge code is not able to start
destruction of the softmix technology before the softmix mixing thread
wakes up, a crash happens.

* Added a stop technology callback that technologies can use to request
any helper threads to stop in preparation for being destroyed.

(closes issue AST-1156)
Reported by: John Bigelow

Modified:
    trunk/bridges/bridge_softmix.c
    trunk/include/asterisk/bridging_technology.h
    trunk/main/bridging.c

Modified: trunk/bridges/bridge_softmix.c
URL: http://svnview.digium.com/svn/asterisk/trunk/bridges/bridge_softmix.c?view=diff&rev=390975&r1=390974&r2=390975
==============================================================================
--- trunk/bridges/bridge_softmix.c (original)
+++ trunk/bridges/bridge_softmix.c Sat Jun  8 00:18:22 2013
@@ -1064,6 +1064,29 @@
 	return 0;
 }
 
+/*!
+ * \internal
+ * \brief Request the softmix mixing thread stop.
+ * \since 12.0.0
+ *
+ * \param bridge Which bridge is being stopped.
+ *
+ * \return Nothing
+ */
+static void softmix_bridge_stop(struct ast_bridge *bridge)
+{
+	struct softmix_bridge_data *softmix_data;
+
+	softmix_data = bridge->tech_pvt;
+	if (!softmix_data) {
+		return;
+	}
+
+	ast_mutex_lock(&softmix_data->lock);
+	softmix_data->stop = 1;
+	ast_mutex_unlock(&softmix_data->lock);
+}
+
 /*! \brief Function called when a bridge is destroyed */
 static void softmix_bridge_destroy(struct ast_bridge *bridge)
 {
@@ -1096,6 +1119,7 @@
 	.capabilities = AST_BRIDGE_CAPABILITY_MULTIMIX,
 	.preference = AST_BRIDGE_PREFERENCE_BASE_MULTIMIX,
 	.create = softmix_bridge_create,
+	.stop = softmix_bridge_stop,
 	.destroy = softmix_bridge_destroy,
 	.join = softmix_bridge_join,
 	.leave = softmix_bridge_leave,

Modified: trunk/include/asterisk/bridging_technology.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/bridging_technology.h?view=diff&rev=390975&r1=390974&r2=390975
==============================================================================
--- trunk/include/asterisk/bridging_technology.h (original)
+++ trunk/include/asterisk/bridging_technology.h Sat Jun  8 00:18:22 2013
@@ -62,6 +62,12 @@
 	 * However, it can be accessed as if it were locked.
 	 */
 	int (*create)(struct ast_bridge *bridge);
+	/*!
+	 * \brief Request a bridge technology instance stop in preparation for being destroyed.
+	 *
+	 * \note On entry, bridge is already locked.
+	 */
+	void (*stop)(struct ast_bridge *bridge);
 	/*!
 	 * \brief Destroy a bridging technology instance for a bridge.
 	 *

Modified: trunk/main/bridging.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/bridging.c?view=diff&rev=390975&r1=390974&r2=390975
==============================================================================
--- trunk/main/bridging.c (original)
+++ trunk/main/bridging.c Sat Jun  8 00:18:22 2013
@@ -1325,6 +1325,13 @@
 
 	/* Pass off the bridge to the technology to destroy if needed */
 	if (bridge->technology) {
+		ast_debug(1, "Bridge %s: calling %s technology stop\n",
+			bridge->uniqueid, bridge->technology->name);
+		if (bridge->technology->stop) {
+			ast_bridge_lock(bridge);
+			bridge->technology->stop(bridge);
+			ast_bridge_unlock(bridge);
+		}
 		ast_debug(1, "Bridge %s: calling %s technology destructor\n",
 			bridge->uniqueid, bridge->technology->name);
 		if (bridge->technology->destroy) {
@@ -1743,6 +1750,12 @@
 		return -1;
 	}
 
+	ast_debug(1, "Bridge %s: calling %s technology stop\n",
+		dummy_bridge.uniqueid, old_technology->name);
+	if (old_technology->stop) {
+		old_technology->stop(&dummy_bridge);
+	}
+
 	/* Move existing channels over to the new technology. */
 	AST_LIST_TRAVERSE(&bridge->channels, bridge_channel, entry) {
 		if (bridge_channel->just_joined) {
@@ -1782,11 +1795,11 @@
 	 */
 	if (old_technology->destroy) {
 		ast_debug(1, "Bridge %s: deferring %s technology destructor\n",
-			bridge->uniqueid, old_technology->name);
+			dummy_bridge.uniqueid, old_technology->name);
 		bridge_queue_action_nodup(bridge, deferred_action);
 	} else {
 		ast_debug(1, "Bridge %s: calling %s technology destructor\n",
-			bridge->uniqueid, old_technology->name);
+			dummy_bridge.uniqueid, old_technology->name);
 		ast_module_unref(old_technology->mod);
 	}