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
........

Merged revisions 405215 from http://svn.asterisk.org/svn/asterisk/branches/11

git-svn-id: https://origsvn.digium.com/svn/asterisk/certified/branches/11.6@405233 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/apps/app_confbridge.c b/apps/app_confbridge.c
index 57de6d46cf4e036961452d20817d6498f45d5976..20f2332bc74bdcfff5a264a94280f1b81dc77e7f 100644 (file)
--- a/apps/app_confbridge.c
+++ b/apps/app_confbridge.c
@@ -1917,7 +1917,7 @@ static int action_kick_last(struct conference_bridge *conference_bridge,
                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);
@@ -2095,7 +2095,7 @@ static int kick_conference_participant(struct conference_bridge *bridge, const c
 
        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);
@@ -2103,7 +2103,7 @@ static int kick_conference_participant(struct conference_bridge *bridge, const c
                }
        }
        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);

diff --git a/apps/confbridge/conf_state_multi_marked.c b/apps/confbridge/conf_state_multi_marked.c
index d25f65d0d44d1eaac8244fcb623befcfa918a194..ff4b38d4ae9fc8d406926205522ad7d6eee97bac 100644 (file)
--- a/apps/confbridge/conf_state_multi_marked.c
+++ b/apps/confbridge/conf_state_multi_marked.c
@@ -78,23 +78,16 @@ static void leave_active(struct conference_bridge_user *cbu)
 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);
@@ -159,6 +152,18 @@ static void leave_marked(struct conference_bridge_user *cbu)
                        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)