Bridges: Fix feature interruption/unintended kick caused by external actions

If a manager or CLI user attached a mixmonitor to a call running a dynamic
bridge feature while in a bridge, the feature would be interrupted and the
channel would be forcibly kicked out of the bridge (usually ending the call
during a simple 1 to 1 call). This would also occur during any similar action
that could set the unbridge soft hangup flag, so the fix for this was to
remove unbridge from the soft hangup flags and make it a separate thing all
together.

ASTERISK-24027 #close
Reported by: mjordan
Review: https://reviewboard.asterisk.org/r/3900/
........

Merged revisions 420934 from http://svn.asterisk.org/svn/asterisk/branches/12
........

Merged revisions 420940 from http://svn.asterisk.org/svn/asterisk/branches/13

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

diff --git a/apps/app_chanspy.c b/apps/app_chanspy.c
index 5806b997d7e68fc13131a7cb4bf2f47508bcd27f..9f530c50dab8cb5e4a7063481ac4a519ab34dcb0 100644 (file)
--- a/apps/app_chanspy.c
+++ b/apps/app_chanspy.c
@@ -508,7 +508,7 @@ static int start_spying(struct ast_autochan *autochan, const char *spychan_name,
        if (!res) {
                ast_channel_lock(autochan->chan);
                if (ast_channel_is_bridged(autochan->chan)) {
-                       ast_softhangup_nolock(autochan->chan, AST_SOFTHANGUP_UNBRIDGE);
+                       ast_channel_set_unbridged_nolock(autochan->chan, 1);
                }
                ast_channel_unlock(autochan->chan);
        }

diff --git a/apps/app_mixmonitor.c b/apps/app_mixmonitor.c
index 52128942ab4a0bbefe91f3d93dd5accc2283fd74..cf7b935185320bef8fff7e50ae01a6eaf72bf47f 100644 (file)
--- a/apps/app_mixmonitor.c
+++ b/apps/app_mixmonitor.c
@@ -461,7 +461,7 @@ static int startmon(struct ast_channel *chan, struct ast_audiohook *audiohook)
        if (!res) {
                ast_channel_lock(chan);
                if (ast_channel_is_bridged(chan)) {
-                       ast_softhangup_nolock(chan, AST_SOFTHANGUP_UNBRIDGE);
+                       ast_channel_set_unbridged_nolock(chan, 1);
                }
                ast_channel_unlock(chan);
        }

diff --git a/apps/app_stack.c b/apps/app_stack.c
index b05afb0e5c7c69730d0d039004bf110d5d5f15ea..c9d37cd3445b3047da6dec576ecf4c19ae70b08f 100644 (file)
--- a/apps/app_stack.c
+++ b/apps/app_stack.c
@@ -976,10 +976,9 @@ static int gosub_run(struct ast_channel *chan, const char *sub_args, int ignore_
 
        /* Save non-hangup softhangup flags. */
        saved_hangup_flags = ast_channel_softhangup_internal_flag(chan)
-               & (AST_SOFTHANGUP_ASYNCGOTO | AST_SOFTHANGUP_UNBRIDGE);
+               & AST_SOFTHANGUP_ASYNCGOTO;
        if (saved_hangup_flags) {
-               ast_channel_clear_softhangup(chan,
-                       AST_SOFTHANGUP_ASYNCGOTO | AST_SOFTHANGUP_UNBRIDGE);
+               ast_channel_clear_softhangup(chan, AST_SOFTHANGUP_ASYNCGOTO);
        }
 
        /* Save autoloop flag */
@@ -1028,10 +1027,6 @@ static int gosub_run(struct ast_channel *chan, const char *sub_args, int ignore_
                 */
                do {
                        /* Check for hangup. */
-                       if (ast_channel_softhangup_internal_flag(chan) & AST_SOFTHANGUP_UNBRIDGE) {
-                               saved_hangup_flags |= AST_SOFTHANGUP_UNBRIDGE;
-                               ast_channel_clear_softhangup(chan, AST_SOFTHANGUP_UNBRIDGE);
-                       }
                        if (ast_check_hangup(chan)) {
                                if (ast_channel_softhangup_internal_flag(chan) & AST_SOFTHANGUP_ASYNCGOTO) {
                                        ast_log(LOG_ERROR, "%s An async goto just messed up our execution location.\n",

diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 08f4effe6084a5c3bb9daefe6a9cf71707946ce7..cee07f2cb7d00715460e1a07e320bd6c8a5bb376 100644 (file)
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -1048,11 +1048,6 @@ enum {
         * needed.
         */
        AST_SOFTHANGUP_EXPLICIT =  (1 << 5),
-       /*!
-        * Used to request that the bridge core re-evaluate the current
-        * bridging technology in use by the bridge this channel is in.
-        */
-       AST_SOFTHANGUP_UNBRIDGE =  (1 << 6),
        /*!
         * Used to indicate that the channel is currently executing hangup
         * logic in the dialplan. The channel has been hungup when this is
@@ -1574,6 +1569,40 @@ int ast_check_hangup(struct ast_channel *chan);
 
 int ast_check_hangup_locked(struct ast_channel *chan);
 
+/*! \brief This function will check if the bridge needs to be re-evaluated due to
+ *         external changes.
+ *
+ *  \param chan Channel on which to check the unbridge_eval flag
+ *
+ *  \return Returns 0 if the flag is down or 1 if the flag is up.
+ */
+int ast_channel_unbridged(struct ast_channel *chan);
+
+/*! \brief ast_channel_unbridged variant. Use this if the channel
+ *         is already locked prior to calling.
+ *
+ *  \param chan Channel on which to check the unbridge flag
+ *
+ *  \return Returns 0 if the flag is down or 1 if the flag is up.
+ */
+int ast_channel_unbridged_nolock(struct ast_channel *chan);
+
+/*! \brief Sets the unbridged flag and queues a NULL frame on the channel to trigger
+ *         a check by bridge_channel_wait
+ *
+ *  \param chan Which channel is having its unbridged value set
+ *  \param value What the unbridge value is being set to
+ */
+void ast_channel_set_unbridged(struct ast_channel *chan, int value);
+
+/*! \brief Variant of ast_channel_set_unbridged. Use this if the channel
+ *         is already locked prior to calling.
+ *
+ *  \param chan Which channel is having its unbridged value set
+ *  \param value What the unbridge value is being set to
+ */
+void ast_channel_set_unbridged_nolock(struct ast_channel *chan, int value);
+
 /*!
  * \brief Lock the given channel, then request softhangup on the channel with the given causecode
  * \param chan channel on which to hang up

diff --git a/main/bridge_after.c b/main/bridge_after.c
index fbe4e605a53873e27833a94cc862a9edaafaf206..a21cbf58ea60093e93ddb1d6c72e3edc25018500 100644 (file)
--- a/main/bridge_after.c
+++ b/main/bridge_after.c
@@ -452,9 +452,9 @@ int ast_bridge_setup_after_goto(struct ast_channel *chan)
        int goto_failed = -1;
 
        /* We are going to be leaving the bridging system now;
-        * clear any pending UNBRIDGE flags
+        * clear any pending unbridge flags
         */
-       ast_channel_clear_softhangup(chan, AST_SOFTHANGUP_UNBRIDGE);
+       ast_channel_set_unbridged(chan, 0);
 
        /* Determine if we are going to setup a dialplan location and where. */
        if (ast_channel_softhangup_internal_flag(chan) & AST_SOFTHANGUP_ASYNCGOTO) {

diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index 5c2e56241d28296333e57900528c8d89384a1ca8..f6b46654550bd5cc977f859b895cd134d1d1a3f5 100644 (file)
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -2281,8 +2281,8 @@ static void bridge_channel_wait(struct ast_bridge_channel *bridge_channel)
                ms = bridge_channel_next_interval(bridge_channel);
                chan = ast_waitfor_nandfds(&bridge_channel->chan, 1,
                        &bridge_channel->alert_pipe[0], 1, NULL, &outfd, &ms);
-               if (ast_channel_softhangup_internal_flag(bridge_channel->chan) & AST_SOFTHANGUP_UNBRIDGE) {
-                       ast_channel_clear_softhangup(bridge_channel->chan, AST_SOFTHANGUP_UNBRIDGE);
+               if (ast_channel_unbridged(bridge_channel->chan)) {
+                       ast_channel_set_unbridged(bridge_channel->chan, 0);
                        ast_bridge_channel_lock_bridge(bridge_channel);
                        bridge_channel->bridge->reconfigured = 1;
                        bridge_reconfigured(bridge_channel->bridge, 0);

diff --git a/main/channel.c b/main/channel.c
index 50b9e872619eabb4a0e8e33a774402910b07f97e..5681ec1a52958d559e3d91538770e6d05a5fad11 100644 (file)
--- a/main/channel.c
+++ b/main/channel.c
@@ -10217,11 +10217,11 @@ int ast_channel_is_bridged(const struct ast_channel *chan)
 int ast_channel_is_leaving_bridge(struct ast_channel *chan)
 {
        int hangup_flags = ast_channel_softhangup_internal_flag(chan);
-       int hangup_test = hangup_flags & (AST_SOFTHANGUP_ASYNCGOTO | AST_SOFTHANGUP_UNBRIDGE);
+       int hangup_test = hangup_flags & AST_SOFTHANGUP_ASYNCGOTO;
 
-       /* This function should only return true if either ASYNCGOTO
-        * or UNBRIDGE is set, or both flags are set. It should return
-        * false if any other flag is set.
+       /* This function should only return true if only the ASYNCGOTO
+        * is set. It should false if any other flag is set or if the
+        * ASYNCGOTO flag is not set.
         */
        return (hangup_test && (hangup_test == hangup_flags));
 }
@@ -10518,7 +10518,7 @@ void ast_channel_end_dtmf(struct ast_channel *chan, char digit, struct timeval s
        ast_channel_lock(chan);
        dead = ast_test_flag(ast_channel_flags(chan), AST_FLAG_ZOMBIE)
                || (ast_channel_softhangup_internal_flag(chan)
-                       & ~(AST_SOFTHANGUP_ASYNCGOTO | AST_SOFTHANGUP_UNBRIDGE));
+                       & ~AST_SOFTHANGUP_ASYNCGOTO);
        ast_channel_unlock(chan);
        if (dead) {
                /* Channel is a zombie or a real hangup. */

diff --git a/main/channel_internal_api.c b/main/channel_internal_api.c
index 8a9e18eb0061e149815efd192dfd56361a021468..e32a52791a20e14f58f75f8417b378a9d6c26fa6 100644 (file)
--- a/main/channel_internal_api.c
+++ b/main/channel_internal_api.c
@@ -173,6 +173,8 @@ struct ast_channel {
                                                         *   See \arg \ref AstFileDesc */
        int softhangup;                         /*!< Whether or not we have been hung up...  Do not set this value
                                                         *   directly, use ast_softhangup() */
+       int unbridged;              /*!< If non-zero, the bridge core needs to re-evaluate the current
+                                        bridging technology which is in use by this channel's bridge. */
        int fdno;                                       /*!< Which fd had an event detected on */
        int streamid;                                   /*!< For streaming playback, the schedule ID */
        int vstreamid;                                  /*!< For streaming video playback, the schedule ID */
@@ -377,7 +379,6 @@ int ast_channel_data_add_structure(struct ast_data *tree,
        ast_data_add_bool(data_softhangup, "timeout", ast_channel_softhangup_internal_flag(chan) & AST_SOFTHANGUP_TIMEOUT);
        ast_data_add_bool(data_softhangup, "appunload", ast_channel_softhangup_internal_flag(chan) & AST_SOFTHANGUP_APPUNLOAD);
        ast_data_add_bool(data_softhangup, "explicit", ast_channel_softhangup_internal_flag(chan) & AST_SOFTHANGUP_EXPLICIT);
-       ast_data_add_bool(data_softhangup, "unbridge", ast_channel_softhangup_internal_flag(chan) & AST_SOFTHANGUP_UNBRIDGE);
 
        /* channel flags */
        data_flags = ast_data_add_node(tree, "flags");
@@ -1141,6 +1142,33 @@ void ast_channel_softhangup_internal_flag_clear(struct ast_channel *chan, int va
        chan ->softhangup &= ~value;
 }
 
+int ast_channel_unbridged_nolock(struct ast_channel *chan)
+{
+       return chan->unbridged;
+}
+
+int ast_channel_unbridged(struct ast_channel *chan)
+{
+       int res;
+       ast_channel_lock(chan);
+       res = ast_channel_unbridged_nolock(chan);
+       ast_channel_unlock(chan);
+       return res;
+}
+
+void ast_channel_set_unbridged_nolock(struct ast_channel *chan, int value)
+{
+       chan->unbridged = value;
+       ast_queue_frame(chan, &ast_null_frame);
+}
+
+void ast_channel_set_unbridged(struct ast_channel *chan, int value)
+{
+       ast_channel_lock(chan);
+       ast_channel_set_unbridged_nolock(chan, value);
+       ast_channel_unlock(chan);
+}
+
 void ast_channel_callid_cleanup(struct ast_channel *chan)
 {
        if (chan->callid) {

diff --git a/main/framehook.c b/main/framehook.c
index 0d42b49906b2a65722c8627834bb847e4e69c5c4..ec4e1695f9b69a360f2ca04731e4c6f5897822ad 100644 (file)
--- a/main/framehook.c
+++ b/main/framehook.c
@@ -170,7 +170,7 @@ int ast_framehook_attach(struct ast_channel *chan, struct ast_framehook_interfac
        }
 
        if (ast_channel_is_bridged(chan)) {
-               ast_softhangup_nolock(chan, AST_SOFTHANGUP_UNBRIDGE);
+               ast_channel_set_unbridged_nolock(chan, 1);
        }
 
        return framehook->id;
@@ -199,7 +199,7 @@ int ast_framehook_detach(struct ast_channel *chan, int id)
        AST_LIST_TRAVERSE_SAFE_END;
 
        if (ast_channel_is_bridged(chan)) {
-               ast_softhangup_nolock(chan, AST_SOFTHANGUP_UNBRIDGE);
+               ast_channel_set_unbridged_nolock(chan, 1);
        }
 
        return res;

diff --git a/main/pbx.c b/main/pbx.c
index 785175fc4b472a8f3260ca9be97093bf6c05ae40..d814c5ac614c8fe4b0e1c61772afcedb680374d9 100644 (file)
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -6363,11 +6363,6 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c,
                        S_COR(ast_channel_caller(c)->id.number.valid, ast_channel_caller(c)->id.number.str, NULL),
                        &found, 1))) {
 
-                       /* Defensively clear the UNBRIDGE flag in case it leaked
-                        * out of the bridging framework. UNBRIDE never implies
-                        * that a channel is hung up.
-                        */
-                       ast_channel_clear_softhangup(c, AST_SOFTHANGUP_UNBRIDGE);
                        if (!ast_check_hangup(c)) {
                                ast_channel_priority_set(c, ast_channel_priority(c) + 1);
                                continue;