Better protect bridge_channel state from other threads.

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@379789 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/bridges/bridge_builtin_features.c b/bridges/bridge_builtin_features.c
index 7b61334..428d6de 100644 (file)
--- a/bridges/bridge_builtin_features.c
+++ b/bridges/bridge_builtin_features.c
@@ -111,14 +111,12 @@ static int feature_blind_transfer(struct ast_bridge *bridge, struct ast_bridge_c
        /* 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 @@ static int feature_attended_transfer(struct ast_bridge *bridge, struct ast_bridg
        /* 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 @@ static int feature_attended_transfer(struct ast_bridge *bridge, struct ast_bridg
        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 @@ static int feature_attended_transfer(struct ast_bridge *bridge, struct ast_bridg
                }
        } 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 */

diff --git a/main/bridging.c b/main/bridging.c
index 886bf83..dbd3531 100644 (file)
--- a/main/bridging.c
+++ b/main/bridging.c
@@ -118,7 +118,8 @@ int ast_bridge_technology_unregister(struct ast_bridge_technology *technology)
        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,12 +127,17 @@ void ast_bridge_change_state(struct ast_bridge_channel *bridge_channel, enum ast
        /* 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 */
 static void bridge_poke(struct ast_bridge *bridge)
 {
@@ -245,15 +251,17 @@ static void bridge_force_out_all(struct ast_bridge *bridge)
        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 @@ static void bridge_channel_feature(struct ast_bridge *bridge, struct ast_bridge_
        } 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 @@ static void bridge_channel_talking(struct ast_bridge *bridge, struct ast_bridge_
        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 @@ static void bridge_channel_dtmf_stream(struct ast_bridge *bridge, struct ast_bri
 
        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 @@ static enum ast_bridge_channel_state bridge_channel_join(struct ast_bridge_chann
                        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 @@ static enum ast_bridge_channel_state bridge_channel_join(struct ast_bridge_chann
                        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 @@ enum ast_bridge_channel_state ast_bridge_join(struct ast_bridge *bridge,
        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;