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

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Mon Jan 21 14:35:16 CST 2013


Author: rmudgett
Date: Mon Jan 21 14:35:12 2013
New Revision: 379789

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=379789
Log:
Better protect bridge_channel state from other threads.

Modified:
    trunk/bridges/bridge_builtin_features.c
    trunk/main/bridging.c

Modified: trunk/bridges/bridge_builtin_features.c
URL: http://svnview.digium.com/svn/asterisk/trunk/bridges/bridge_builtin_features.c?view=diff&rev=379789&r1=379788&r2=379789
==============================================================================
--- trunk/bridges/bridge_builtin_features.c (original)
+++ trunk/bridges/bridge_builtin_features.c Mon Jan 21 14:35:12 2013
@@ -111,14 +111,12 @@
 	/* Grab the extension to transfer to */
 	if (!grab_transfer(bridge_channel->chan, exten, sizeof(exten), context)) {
 		ast_stream_and_wait(bridge_channel->chan, "pbx-invalid", AST_DIGIT_ANY);
-		ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
 		return 0;
 	}
 
 	/* Get a channel that is the destination we wish to call */
 	if (!(chan = dial_transfer(bridge_channel->chan, exten, context))) {
 		ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_ANY);
-		ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
 		return 0;
 	}
 
@@ -180,14 +178,12 @@
 	/* Grab the extension to transfer to */
 	if (!grab_transfer(bridge_channel->chan, exten, sizeof(exten), context)) {
 		ast_stream_and_wait(bridge_channel->chan, "pbx-invalid", AST_DIGIT_ANY);
-		ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
 		return 0;
 	}
 
 	/* Get a channel that is the destination we wish to call */
 	if (!(chan = dial_transfer(bridge_channel->chan, exten, context))) {
 		ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_ANY);
-		ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
 		return 0;
 	}
 
@@ -195,7 +191,6 @@
 	if (!(attended_bridge = ast_bridge_new(AST_BRIDGE_CAPABILITY_1TO1MIX, 0))) {
 		ast_hangup(chan);
 		ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_ANY);
-		ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
 		return 0;
 	}
 
@@ -232,7 +227,6 @@
 		}
 	} else {
 		ast_stream_and_wait(bridge_channel->chan, "beeperr", AST_DIGIT_ANY);
-		ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
 	}
 
 	/* Now that all channels are out of it we can destroy the bridge and the called features structure */

Modified: trunk/main/bridging.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/bridging.c?view=diff&rev=379789&r1=379788&r2=379789
==============================================================================
--- trunk/main/bridging.c (original)
+++ trunk/main/bridging.c Mon Jan 21 14:35:12 2013
@@ -118,7 +118,8 @@
 	return current ? 0 : -1;
 }
 
-void ast_bridge_change_state(struct ast_bridge_channel *bridge_channel, enum ast_bridge_channel_state new_state)
+/*! \note This function assumes the bridge_channel is locked. */
+static void ast_bridge_change_state_nolock(struct ast_bridge_channel *bridge_channel, enum ast_bridge_channel_state new_state)
 {
 	/* Change the state on the bridge channel */
 	bridge_channel->state = new_state;
@@ -126,10 +127,15 @@
 	/* Only poke the channel's thread if it is not us */
 	if (!pthread_equal(pthread_self(), bridge_channel->thread)) {
 		pthread_kill(bridge_channel->thread, SIGURG);
-		ao2_lock(bridge_channel);
 		ast_cond_signal(&bridge_channel->cond);
-		ao2_unlock(bridge_channel);
-	}
+	}
+}
+
+void ast_bridge_change_state(struct ast_bridge_channel *bridge_channel, enum ast_bridge_channel_state new_state)
+{
+	ao2_lock(bridge_channel);
+	ast_bridge_change_state_nolock(bridge_channel, new_state);
+	ao2_unlock(bridge_channel);
 }
 
 /*! \brief Helper function to poke the bridge thread */
@@ -245,15 +251,17 @@
 	struct ast_bridge_channel *bridge_channel;
 
 	AST_LIST_TRAVERSE(&bridge->channels, bridge_channel, entry) {
+		ao2_lock(bridge_channel);
 		switch (bridge_channel->state) {
 		case AST_BRIDGE_CHANNEL_STATE_END:
 		case AST_BRIDGE_CHANNEL_STATE_HANGUP:
 		case AST_BRIDGE_CHANNEL_STATE_DEPART:
 			break;
 		default:
-			ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_HANGUP);
+			ast_bridge_change_state_nolock(bridge_channel, AST_BRIDGE_CHANNEL_STATE_HANGUP);
 			break;
 		}
+		ao2_unlock(bridge_channel);
 	}
 }
 
@@ -908,11 +916,6 @@
 	} else {
 		ast_bridge_dtmf_stream(bridge, dtmf, bridge_channel->chan);
 	}
-
-	/* if the channel is still in feature state, revert it back to wait state */
-	if (bridge_channel->state == AST_BRIDGE_CHANNEL_STATE_FEATURE) {
-		ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
-	}
 }
 
 static void bridge_channel_talking(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
@@ -922,11 +925,10 @@
 	if (features && features->talker_cb) {
 		features->talker_cb(bridge, bridge_channel, features->talker_pvt_data);
 	}
-	ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
 }
 
 /*! \brief Internal function that plays back DTMF on a bridge channel */
-static void bridge_channel_dtmf_stream(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
+static void bridge_channel_dtmf_stream(struct ast_bridge_channel *bridge_channel)
 {
 	char dtmf_q[8] = "";
 
@@ -935,8 +937,6 @@
 
 	ast_debug(1, "Playing DTMF stream '%s' out to bridge channel %p\n", dtmf_q, bridge_channel);
 	ast_dtmf_stream(bridge_channel->chan, NULL, dtmf_q, 250, 0);
-
-	ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
 }
 
 /*! \brief Join a channel to a bridge and handle anything the bridge may want us to do */
@@ -1021,11 +1021,23 @@
 			ao2_unlock(bridge_channel->bridge);
 			bridge_channel_feature(bridge_channel->bridge, bridge_channel);
 			ao2_lock(bridge_channel->bridge);
+			ao2_lock(bridge_channel);
+			if (bridge_channel->state == AST_BRIDGE_CHANNEL_STATE_FEATURE) {
+				ast_bridge_change_state_nolock(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
+			}
+			ao2_unlock(bridge_channel);
 			bridge_channel_unsuspend(bridge_channel->bridge, bridge_channel);
 			break;
 		case AST_BRIDGE_CHANNEL_STATE_DTMF:
 			bridge_channel_suspend(bridge_channel->bridge, bridge_channel);
-			bridge_channel_dtmf_stream(bridge_channel->bridge, bridge_channel);
+			ao2_unlock(bridge_channel->bridge);
+			bridge_channel_dtmf_stream(bridge_channel);
+			ao2_lock(bridge_channel->bridge);
+			ao2_lock(bridge_channel);
+			if (bridge_channel->state == AST_BRIDGE_CHANNEL_STATE_DTMF) {
+				ast_bridge_change_state_nolock(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
+			}
+			ao2_unlock(bridge_channel);
 			bridge_channel_unsuspend(bridge_channel->bridge, bridge_channel);
 			break;
 		case AST_BRIDGE_CHANNEL_STATE_START_TALKING:
@@ -1033,6 +1045,16 @@
 			ao2_unlock(bridge_channel->bridge);
 			bridge_channel_talking(bridge_channel->bridge, bridge_channel);
 			ao2_lock(bridge_channel->bridge);
+			ao2_lock(bridge_channel);
+			switch (bridge_channel->state) {
+			case AST_BRIDGE_CHANNEL_STATE_START_TALKING:
+			case AST_BRIDGE_CHANNEL_STATE_STOP_TALKING:
+				ast_bridge_change_state_nolock(bridge_channel, AST_BRIDGE_CHANNEL_STATE_WAIT);
+				break;
+			default:
+				break;
+			}
+			ao2_unlock(bridge_channel);
 			break;
 		default:
 			break;
@@ -1141,11 +1163,9 @@
 	state = bridge_channel_join(bridge_channel);
 
 	/* Cleanup all the data in the bridge channel after it leaves the bridge. */
-	ao2_lock(bridge_channel);
 	bridge_channel->chan = NULL;
 	bridge_channel->swap = NULL;
 	bridge_channel->features = NULL;
-	ao2_unlock(bridge_channel);
 
 	ao2_ref(bridge_channel, -1);
 	return state;




More information about the asterisk-commits mailing list