[svn-commits] rmudgett: trunk r400850 - in /trunk: ./ bridges/bridge_softmix.c

[SVN commits to the Digium repositories]

Author: rmudgett
Date: Fri Oct 11 11:28:50 2013
New Revision: 400850

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=400850
Log:
Softmix: Fix crash when switching from softmix to another bridge technology.

The crash is caused by a race condition when switching between native RTP
and softmix bridging technologies.  In this situation, the bridging
technology is switched from native RTP to softmix, and then back to native
RTP fast enough that the softmix private data gets destroyed before the
softmix mixing thread gets started.

Thanks to Kinsey Moore for the crash analysis.

* Fix race condition when starting the softmix mixing thread and switching
to another bridge technology.

(closes issue ASTERISK-22678)
Reported by: John Bigelow
Patches:
      jira_asterisk_22678_v12.patch (license #5621) patch uploaded by rmudgett
Tested by: John Bigelow
........

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

Modified:
    trunk/   (props changed)
    trunk/bridges/bridge_softmix.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-12-merged' - no diff available.

Modified: trunk/bridges/bridge_softmix.c
URL: http://svnview.digium.com/svn/asterisk/trunk/bridges/bridge_softmix.c?view=diff&rev=400850&r1=400849&r2=400850
==============================================================================
--- trunk/bridges/bridge_softmix.c (original)
+++ trunk/bridges/bridge_softmix.c Fri Oct 11 11:28:50 2013
@@ -119,6 +119,14 @@
 
 struct softmix_bridge_data {
 	struct ast_timer *timer;
+	/*!
+	 * \brief Bridge pointer passed to the softmix mixing thread.
+	 *
+	 * \note Does not need a reference because the bridge will
+	 * always exist while the mixing thread exists even if the
+	 * bridge is no longer actively using the softmix technology.
+	 */
+	struct ast_bridge *bridge;
 	/*! Lock for signaling the mixing thread. */
 	ast_mutex_t lock;
 	/*! Condition, used if we need to wake up the mixing thread. */
@@ -980,8 +988,8 @@
  */
 static void *softmix_mixing_thread(void *data)
 {
-	struct ast_bridge *bridge = data;
-	struct softmix_bridge_data *softmix_data;
+	struct softmix_bridge_data *softmix_data = data;
+	struct ast_bridge *bridge = softmix_data->bridge;
 
 	ast_bridge_lock(bridge);
 	if (bridge->callid) {
@@ -990,7 +998,6 @@
 
 	ast_debug(1, "Bridge %s: starting mixing thread\n", bridge->uniqueid);
 
-	softmix_data = bridge->tech_pvt;
 	while (!softmix_data->stop) {
 		if (!bridge->num_active) {
 			/* Wait for something to happen to the bridge. */
@@ -1029,6 +1036,7 @@
 		softmix_data->timer = NULL;
 	}
 	ast_mutex_destroy(&softmix_data->lock);
+	ast_cond_destroy(&softmix_data->cond);
 	ast_free(softmix_data);
 }
 
@@ -1041,7 +1049,9 @@
 	if (!softmix_data) {
 		return -1;
 	}
+	softmix_data->bridge = bridge;
 	ast_mutex_init(&softmix_data->lock);
+	ast_cond_init(&softmix_data->cond, NULL);
 	softmix_data->timer = ast_timer_open();
 	if (!softmix_data->timer) {
 		ast_log(AST_LOG_WARNING, "Failed to open timer for softmix bridge\n");
@@ -1055,7 +1065,8 @@
 	bridge->tech_pvt = softmix_data;
 
 	/* Start the mixing thread. */
-	if (ast_pthread_create(&softmix_data->thread, NULL, softmix_mixing_thread, bridge)) {
+	if (ast_pthread_create(&softmix_data->thread, NULL, softmix_mixing_thread,
+		softmix_data)) {
 		softmix_data->thread = AST_PTHREADT_NULL;
 		softmix_bridge_data_destroy(softmix_data);
 		bridge->tech_pvt = NULL;