[asterisk-commits] rmudgett: branch group/bridge_construction r385255 - in /team/group/bridge_co...

[SVN commits to the Asterisk project]

Author: rmudgett
Date: Wed Apr 10 12:27:59 2013
New Revision: 385255

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=385255
Log:
confbridge: Fix announcer channel bridge depart getting stuck.

When the announcer channel needs to depart from the bridge, the thread
needs to be poked to get it out of an indefinite wait for media.
Unfortunately, this fix does not guarantee that ast_bridge_depart() will
not get stuck waiting for the bridge channel thread to die.  Thres is a
small window of opportunity in bridge_channel_wait() where the thread is
about to enter the ast_waitfor_nandfds() when the thread poke will be
ignored.

Modified:
    team/group/bridge_construction/bridges/bridge_softmix.c
    team/group/bridge_construction/main/bridging.c

Modified: team/group/bridge_construction/bridges/bridge_softmix.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/bridges/bridge_softmix.c?view=diff&rev=385255&r1=385254&r2=385255
==============================================================================
--- team/group/bridge_construction/bridges/bridge_softmix.c (original)
+++ team/group/bridge_construction/bridges/bridge_softmix.c Wed Apr 10 12:27:59 2013
@@ -1080,6 +1080,7 @@
 	softmix_data->thread = AST_PTHREADT_NULL;
 	ast_mutex_unlock(&softmix_data->lock);
 	if (thread != AST_PTHREADT_NULL) {
+		ast_debug(1, "Waiting for mixing thread to die.\n");
 		pthread_join(thread, NULL);
 	}
 

Modified: team/group/bridge_construction/main/bridging.c
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/main/bridging.c?view=diff&rev=385255&r1=385254&r2=385255
==============================================================================
--- team/group/bridge_construction/main/bridging.c (original)
+++ team/group/bridge_construction/main/bridging.c Wed Apr 10 12:27:59 2013
@@ -215,6 +215,14 @@
 		}
 		ast_bridge_unlock(bridge);
 		ao2_ref(bridge, -1);
+	}
+}
+
+static void bridge_channel_poke(struct ast_bridge_channel *bridge_channel)
+{
+	if (!pthread_equal(pthread_self(), bridge_channel->thread)) {
+		pthread_kill(bridge_channel->thread, SIGURG);
+		ast_cond_signal(&bridge_channel->cond);
 	}
 }
 
@@ -2933,9 +2941,21 @@
 
 	/* We are claiming the reference held by the depart thread. */
 
-	ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_HANGUP);
+	ast_bridge_channel_lock(bridge_channel);
+	ast_bridge_change_state_nolock(bridge_channel, AST_BRIDGE_CHANNEL_STATE_HANGUP);
+
+/* BUGBUG hack alert ast_bridge_depart() may fail to return because of a race condition. */
+	/*
+	 * XXX The poke is a bit of a hack since there is a race
+	 * condition in bridge_channel_wait() when it is just about to
+	 * enter ast_waitfor_nandfds() and we poke the thread.
+	 */
+	bridge_channel_poke(bridge_channel);
+	ast_bridge_channel_unlock(bridge_channel);
 
 	/* Wait for the depart thread to die */
+	ast_debug(1, "Waiting for %p(%s) bridge thread to die.\n",
+		bridge_channel, ast_channel_name(bridge_channel->chan));
 	pthread_join(bridge_channel->thread, NULL);
 
 	ast_channel_lock(chan);
@@ -3955,6 +3975,7 @@
 		manager->stop = 1;
 		ast_cond_signal(&manager->cond);
 		ao2_unlock(manager);
+		ast_debug(1, "Waiting for bridge manager thread to die.\n");
 		pthread_join(manager->thread, NULL);
 	}