[asterisk-commits] mjordan: branch 11 r405215 - in /branches/11/apps: ./ confbridge/

[SVN commits to the Asterisk project]

Author: mjordan
Date: Thu Jan  9 09:41:31 2014
New Revision: 405215

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=405215
Log:
app_confbridge: Fix crash caused when waitmarked/marked users leave together

When waitmarked users join a ConfBridge, the conference state is transitioned
from EMPTY -> INACTIVE. In this state, the users are maintined in a waiting
users list. When a marked user joins, the ConfBridge conference transitions
from INACTIVE -> MULTI_MARKED, and all users are put onto the active list of
users. This process works correctly.

When the marked user leaves, if they are the last marked user, the MULTI_MARKED
state does the following:
(1) It plays back a message to the bridge stating that the leader has left the
    conference. This requires an unlocking of the bridge.
(2) It moves waitmarked users back to the waiting list
(3) It transitions to the appropriate state: in this case, INACTIVE

However, because it plays the prompt back to the bridge before moving the users
and before finishing the state transition, this creates a race condition: with
the bridge unlocked, waitmarked users who leave the conference (or are kicked
from it) can cause a state transition of the bridge to another state before
the conference is transitioned to the INACTIVE state. This causes the state
machine to get a bit wonky, often leading to a crash when the MULTI_MARKED state
attempts to conclude its processing.

This patch fixes this problem:
(1) It prevents kicked users from being kicked again. That's just a nicety.
(2) More importantly, it fixes the race condition by only playing the prompt
    once the state has transitioned correctly to INACTIVE. If waitmarked users
    sneak out during the prompt being played, no harm no foul.

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

(closes issue AST-1258)
Reported by: Steve Pitts


Modified:
    branches/11/apps/app_confbridge.c
    branches/11/apps/confbridge/conf_state_multi_marked.c

Modified: branches/11/apps/app_confbridge.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/apps/app_confbridge.c?view=diff&rev=405215&r1=405214&r2=405215
==============================================================================
--- branches/11/apps/app_confbridge.c (original)
+++ branches/11/apps/app_confbridge.c Thu Jan  9 09:41:31 2014
@@ -1961,7 +1961,7 @@
 		ast_stream_and_wait(bridge_channel->chan,
 			conf_get_sound(CONF_SOUND_ERROR_MENU, conference_bridge_user->b_profile.sounds),
 			"");
-	} else if (last_participant) {
+	} else if (last_participant && !last_participant->kicked) {
 		last_participant->kicked = 1;
 		ast_bridge_remove(conference_bridge->bridge, last_participant->chan);
 		ao2_unlock(conference_bridge);
@@ -2139,7 +2139,7 @@
 
 	ao2_lock(bridge);
 	AST_LIST_TRAVERSE(&bridge->active_list, participant, list) {
-		if (!strcasecmp(ast_channel_name(participant->chan), channel)) {
+		if (!strcasecmp(ast_channel_name(participant->chan), channel) && !participant->kicked) {
 			participant->kicked = 1;
 			ast_bridge_remove(bridge->bridge, participant->chan);
 			ao2_unlock(bridge);
@@ -2147,7 +2147,7 @@
 		}
 	}
 	AST_LIST_TRAVERSE(&bridge->waiting_list, participant, list) {
-		if (!strcasecmp(ast_channel_name(participant->chan), channel)) {
+		if (!strcasecmp(ast_channel_name(participant->chan), channel) && !participant->kicked) {
 			participant->kicked = 1;
 			ast_bridge_remove(bridge->bridge, participant->chan);
 			ao2_unlock(bridge);

Modified: branches/11/apps/confbridge/conf_state_multi_marked.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/apps/confbridge/conf_state_multi_marked.c?view=diff&rev=405215&r1=405214&r2=405215
==============================================================================
--- branches/11/apps/confbridge/conf_state_multi_marked.c (original)
+++ branches/11/apps/confbridge/conf_state_multi_marked.c Thu Jan  9 09:41:31 2014
@@ -80,23 +80,16 @@
 static void leave_marked(struct conference_bridge_user *cbu)
 {
 	struct conference_bridge_user *cbu_iter;
+	int need_prompt = 0;
 
 	conf_remove_user_marked(cbu->conference_bridge, cbu);
 
 	if (cbu->conference_bridge->markedusers == 0) {
-		/* Play back the audio prompt saying the leader has left the conference */
-		if (!ast_test_flag(&cbu->u_profile, USER_OPT_QUIET)) {
-			ao2_unlock(cbu->conference_bridge);
-			ast_autoservice_start(cbu->chan);
-			play_sound_file(cbu->conference_bridge,
-				conf_get_sound(CONF_SOUND_LEADER_HAS_LEFT, cbu->b_profile.sounds));
-			ast_autoservice_stop(cbu->chan);
-			ao2_lock(cbu->conference_bridge);
-		}
+		need_prompt = 1;
 
 		AST_LIST_TRAVERSE_SAFE_BEGIN(&cbu->conference_bridge->active_list, cbu_iter, list) {
 			/* Kick ENDMARKED cbu_iters */
-			if (ast_test_flag(&cbu_iter->u_profile, USER_OPT_ENDMARKED)) {
+			if (ast_test_flag(&cbu_iter->u_profile, USER_OPT_ENDMARKED) && !cbu_iter->kicked) {
 				if (ast_test_flag(&cbu_iter->u_profile, USER_OPT_WAITMARKED)
 					&& !ast_test_flag(&cbu_iter->u_profile, USER_OPT_MARKEDUSER)) {
 					AST_LIST_REMOVE_CURRENT(list);
@@ -162,6 +155,18 @@
 			break; /* Stay in marked */
 		}
 	}
+
+	if (need_prompt) {
+		/* Play back the audio prompt saying the leader has left the conference */
+		if (!ast_test_flag(&cbu->u_profile, USER_OPT_QUIET)) {
+			ao2_unlock(cbu->conference_bridge);
+			ast_autoservice_start(cbu->chan);
+			play_sound_file(cbu->conference_bridge,
+				conf_get_sound(CONF_SOUND_LEADER_HAS_LEFT, cbu->b_profile.sounds));
+			ast_autoservice_stop(cbu->chan);
+			ao2_lock(cbu->conference_bridge);
+		}
+	}
 }
 
 static void transition_to_marked(struct conference_bridge_user *cbu)