[asterisk-commits] file: branch 11 r428299 - /branches/11/main/bridging.c

[SVN commits to the Asterisk project]

Author: file
Date: Thu Nov 20 08:20:08 2014
New Revision: 428299

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=428299
Log:
AST-2014-014: Fix race condition where channels may get stuck in ConfBridge under load.

Under load it was possible for the bridging API, and thus ConfBridge, to get
channels that may have hung up stuck in it. This is because handling of state
transitions for a bridged channel within a bridge was not protected and simply
set the new state without regard to the existing state. If the existing state
had been hung up this would get overwritten.

This change adds locking to protect changing of the state and also
takes into consideration the existing state.

ASTERISK-24440 #close
Reported by: Ben Klang

Review: https://reviewboard.asterisk.org/r/4173/

Modified:
    branches/11/main/bridging.c

Modified: branches/11/main/bridging.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/main/bridging.c?view=diff&rev=428299&r1=428298&r2=428299
==============================================================================
--- branches/11/main/bridging.c (original)
+++ branches/11/main/bridging.c Thu Nov 20 08:20:08 2014
@@ -120,8 +120,22 @@
 
 void ast_bridge_change_state(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;
+	/* Change the state on the bridge channel with some manner of intelligence. */
+	ao2_lock(bridge_channel);
+	switch (bridge_channel->state) {
+	case AST_BRIDGE_CHANNEL_STATE_DEPART:
+		break;
+	case AST_BRIDGE_CHANNEL_STATE_END:
+	case AST_BRIDGE_CHANNEL_STATE_HANGUP:
+		if (new_state != AST_BRIDGE_CHANNEL_STATE_DEPART) {
+			break;
+		}
+		/* Fall through */
+	default:
+		bridge_channel->state = new_state;
+		break;
+	}
+	ao2_unlock(bridge_channel);
 
 	/* Only poke the channel's thread if it is not us */
 	if (!pthread_equal(pthread_self(), bridge_channel->thread)) {
@@ -130,8 +144,6 @@
 		ast_cond_signal(&bridge_channel->cond);
 		ao2_unlock(bridge_channel);
 	}
-
-	return;
 }
 
 /*! \brief Helper function to poke the bridge thread */
@@ -1147,8 +1159,12 @@
 	state = bridge_channel_join(bridge_channel);
 
 	/* If no other thread is going to take the channel then hang it up, or else we would have to service it until something else came along */
-	if (bridge_channel->allow_impart_hangup && (state == AST_BRIDGE_CHANNEL_STATE_END || state == AST_BRIDGE_CHANNEL_STATE_HANGUP)) {
+	if (bridge_channel->allow_impart_hangup
+		&& state != AST_BRIDGE_CHANNEL_STATE_DEPART) {
 		ast_hangup(bridge_channel->chan);
+
+		/* nobody is waiting to join me. */
+		pthread_detach(pthread_self());
 	}
 
 	/* cleanup */