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

[SVN commits to the Asterisk project]

Author: rmudgett
Date: Wed Apr 10 17:14:14 2013
New Revision: 385272

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=385272
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.  More
generally, the bridge channel thread needs to be poked whenever it is in
the normal bridge_channel_wait()'s ast_waitfor_nandfds() call and the
state changes.

* Fixes the small window of opportunity in bridge_channel_wait() when
entering ast_waitfor_nandfds().

Modified:
    team/group/bridge_construction/include/asterisk/bridging.h
    team/group/bridge_construction/main/bridging.c

Modified: team/group/bridge_construction/include/asterisk/bridging.h
URL: http://svnview.digium.com/svn/asterisk/team/group/bridge_construction/include/asterisk/bridging.h?view=diff&rev=385272&r1=385271&r2=385272
==============================================================================
--- team/group/bridge_construction/include/asterisk/bridging.h (original)
+++ team/group/bridge_construction/include/asterisk/bridging.h Wed Apr 10 17:14:14 2013
@@ -177,6 +177,8 @@
 	AST_LIST_HEAD_NOLOCK(, ast_frame) wr_queue;
 	/*! Pipe to alert thread when frames are put into the wr_queue. */
 	int alert_pipe[2];
+	/*! TRUE if the bridge channel thread is waiting on channels (needs to be atomically settable) */
+	int waiting;
 };
 
 enum ast_bridge_action_type {

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=385272&r1=385271&r2=385272
==============================================================================
--- team/group/bridge_construction/main/bridging.c (original)
+++ team/group/bridge_construction/main/bridging.c Wed Apr 10 17:14:14 2013
@@ -221,8 +221,10 @@
 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);
+		while (bridge_channel->waiting) {
+			pthread_kill(bridge_channel->thread, SIGURG);
+			sched_yield();
+		}
 	}
 }
 
@@ -239,6 +241,8 @@
 
 	/* Change the state on the bridge channel */
 	bridge_channel->state = new_state;
+
+	bridge_channel_poke(bridge_channel);
 }
 
 void ast_bridge_change_state(struct ast_bridge_channel *bridge_channel, enum ast_bridge_channel_state new_state)
@@ -2145,11 +2149,13 @@
 		ast_debug(10, "Bridge %s: %p(%s) is going into a waitfor\n",
 			bridge_channel->bridge->uniqueid, bridge_channel,
 			ast_channel_name(bridge_channel->chan));
+		bridge_channel->waiting = 1;
 		ast_bridge_channel_unlock(bridge_channel);
 		outfd = -1;
 /* BUGBUG need to make the next expiring active interval setup ms timeout rather than holding up the chan reads. */
 		chan = ast_waitfor_nandfds(&bridge_channel->chan, 1,
 			&bridge_channel->alert_pipe[0], 1, NULL, &outfd, &ms);
+		bridge_channel->waiting = 0;
 		if (!bridge_channel->suspended
 			&& bridge_channel->state == AST_BRIDGE_CHANNEL_STATE_WAIT) {
 			if (chan) {
@@ -2941,17 +2947,7 @@
 
 	/* We are claiming the reference held by the depart thread. */
 
-	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);
+	ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_HANGUP);
 
 	/* Wait for the depart thread to die */
 	ast_debug(1, "Waiting for %p(%s) bridge thread to die.\n",