This patch fixes h-exten running misbehavior in manager-redirected

situations.

What it does:
1. A new Flag value is defined in include/asterisk/channel.h,
 AST_FLAG_BRIDGE_HANGUP_DONT, which used as a messenge to the
 bridge hangup exten code not to run the h-exten there (nor
 publish the bridge cdr there). It will done at the pbx-loop
 level instead.
2. In the manager Redirect code, I set this flag on the channel
 if the channel has a non-null pbx pointer. I did the same for the
 second (chan2) channel, which gets run if name2 is set...
 and the first succeeds.
3. I restored the ending of the cdr for the pbx loop h-exten
 running code. Don't know why it was removed in the first place.
4. The first attempt at the fix for this bug was to place code
   directly in the async_goto routine, which was called from a
   large number of places, and could affect a large number of
   cases, so I tested that fix against a fair number of transfer
   scenarios, both with and without the patch. In the process,
   I saw that putting the fix in async_goto seemed not to affect
   any of the blind or attended scenarios, but still, I was
   was highly concerned that some other scenarios I had not tested
   might be negatively impacted, so I refined the patch to
   its current scope, and jmls tested both. In the process, tho,
   I saw that blind xfers in one situation, when the one-touch
   blind-xfer feature is used by the peer, we got strange
   h-exten behavior.  So, I  inserted code to swap CDRs and
   to set the HANGUP_DONT field, to get uniform behavior.
5. I added code to the bridge to obey the HANGUP_DONT flag,
   skipping both publishing the bridge CDR, and running
   the h-exten; they will be done at the pbx-loop (higher)
   level instead.
6. I removed all the debug logs from the patch before committing.
7. I moved the AUTOLOOP set/reset in the h-exten code in res_features
   so it's only done if the h-exten is going to be run. A very
   minor performance improvement, but technically correct.

(closes issue #14241)
Reported by: jmls
Patches:
      14241_redirect_no_bridgeCDR_or_h_exten_via_transfer uploaded by murf (license 17)
Tested by: murf, jmls

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

diff --git a/apps/app_channelredirect.c b/apps/app_channelredirect.c
index a8eedf9b4e03bc9447eb142d61f54ba92e9915c0..7d1812b1d81605a92b022905dde6a18342bb19e7 100644 (file)
--- a/apps/app_channelredirect.c
+++ b/apps/app_channelredirect.c
@@ -108,6 +108,11 @@ static int asyncgoto_exec(struct ast_channel *chan, void *data)
        if (option_debug > 1)
                ast_log(LOG_DEBUG, "Attempting async goto (%s) to %s|%s|%d\n", args.channel, S_OR(context, chan2->context), S_OR(exten, chan2->exten), prio);
 
+       if (chan2->pbx) {
+               ast_channel_lock(chan2);
+               ast_set_flag(chan2, AST_FLAG_BRIDGE_HANGUP_DONT); /* don't let the after-bridge code run the h-exten */
+               ast_channel_unlock(chan2);
+       }
        if (ast_async_goto_if_exists(chan2, S_OR(context, chan2->context), S_OR(exten, chan2->exten), prio))
                ast_log(LOG_WARNING, "%s failed for %s\n", app, args.channel);
        else

diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 7ae0dbeb598cd6628ba43a2e8680aabd3dc95145..957d95d5c8d137c7f40847ca41c2bb519206cbe1 100644 (file)
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -514,6 +514,10 @@ enum {
         *  a message aimed at preventing a subsequent hangup exten being run at the pbx_run
         *  level */
        AST_FLAG_BRIDGE_HANGUP_RUN = (1 << 16),
+       /*! This flag indicates that the hangup exten should NOT be run when the 
+        *  bridge terminates, this will allow the hangup in the pbx loop to be run instead.
+        *  */
+       AST_FLAG_BRIDGE_HANGUP_DONT = (1 << 17),
 };
 
 /*! \brief ast_bridge_config flags */

diff --git a/main/manager.c b/main/manager.c
index 7f436ad9487abb21cf21b6d75d51e3b55c577317..8b795a86f4cabfdc7cece0994abc25b5c89e8521 100644 (file)
--- a/main/manager.c
+++ b/main/manager.c
@@ -1672,13 +1672,24 @@ static int action_redirect(struct mansession *s, const struct message *m)
                ast_channel_unlock(chan2);
                return 0;
        }
+       if (chan->pbx) {
+               ast_channel_lock(chan);
+               ast_set_flag(chan, AST_FLAG_BRIDGE_HANGUP_DONT); /* don't let the after-bridge code run the h-exten */
+               ast_channel_unlock(chan);
+       }
        res = ast_async_goto(chan, context, exten, pi);
        if (!res) {
                if (!ast_strlen_zero(name2)) {
-                       if (chan2)
+                       if (chan2) {
+                               if (chan2->pbx) {
+                                       ast_channel_lock(chan2);
+                                       ast_set_flag(chan2, AST_FLAG_BRIDGE_HANGUP_DONT); /* don't let the after-bridge code run the h-exten */
+                                       ast_channel_unlock(chan2);
+                               }
                                res = ast_async_goto(chan2, context, exten, pi);
-                       else
+                       } else {
                                res = -1;
+                       }
                        if (!res)
                                astman_send_ack(s, m, "Dual Redirect successful");
                        else

diff --git a/main/pbx.c b/main/pbx.c
index 655181f7f23e63e8edc09769131966b3ee01072e..44a769a6a4fc3c8263c443cfc761a0a50c3df2e3 100644 (file)
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -2544,6 +2544,9 @@ static int __ast_pbx_run(struct ast_channel *c)
                ast_softhangup(c, c->hangupcause ? c->hangupcause : AST_CAUSE_NORMAL_CLEARING);
        if ((res != AST_PBX_KEEPALIVE) && !ast_test_flag(c, AST_FLAG_BRIDGE_HANGUP_RUN) && ast_exists_extension(c, c->context, "h", 1, c->cid.cid_num)) {
                set_ext_pri(c, "h", 1);
+               if (c->cdr && ast_opt_end_cdr_before_h_exten) {
+                       ast_cdr_end(c->cdr);
+               }
                while(ast_exists_extension(c, c->context, c->exten, c->priority, c->cid.cid_num)) {
                        if ((res = ast_spawn_extension(c, c->context, c->exten, c->priority, c->cid.cid_num))) {
                                /* Something bad happened, or a hangup has been requested. */

diff --git a/res/res_features.c b/res/res_features.c
index 5017188f2f1b2da4297c62e8b41f554870b46228..6db08aeda15c709e1d68c98356a17650b05c4dac 100644 (file)
--- a/res/res_features.c
+++ b/res/res_features.c
@@ -758,16 +758,18 @@ static int builtin_blindtransfer(struct ast_channel *chan, struct ast_channel *p
                pbx_builtin_setvar_helper(transferer, "BLINDTRANSFER", transferee->name);
                pbx_builtin_setvar_helper(transferee, "BLINDTRANSFER", transferer->name);
                res=finishup(transferee);
-               if (!transferer->cdr) {
+               if (!transferer->cdr) { /* this code should never get called (in a perfect world) */
                        transferer->cdr=ast_cdr_alloc();
-                       if (transferer) {
+                       if (transferer->cdr) {
                                ast_cdr_init(transferer->cdr, transferer); /* initilize our channel's cdr */
                                ast_cdr_start(transferer->cdr);
                        }
                }
                if (transferer->cdr) {
-                       ast_cdr_setdestchan(transferer->cdr, transferee->name);
-                       ast_cdr_setapp(transferer->cdr, "BLINDTRANSFER","");
+                       struct ast_cdr *swap = transferer->cdr;
+                       /* swap cdrs-- it will save us some time & work */
+                       transferer->cdr = transferee->cdr;
+                       transferee->cdr = swap;
                }
                if (!transferee->pbx) {
                        /* Doh!  Use our handy async_goto functions */
@@ -779,6 +781,7 @@ static int builtin_blindtransfer(struct ast_channel *chan, struct ast_channel *p
                        res = -1;
                } else {
                        /* Set the channel's new extension, since it exists, using transferer context */
+                       ast_set_flag(transferee, AST_FLAG_BRIDGE_HANGUP_DONT); /* don't let the after-bridge code run the h-exten */
                        set_c_e_p(transferee, transferer_real_context, xferto, 0);
                }
                check_goto_on_transfer(transferer);
@@ -1541,9 +1544,11 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast
                        ast_cdr_answer(bridge_cdr);
                        ast_cdr_answer(chan_cdr); /* for the sake of cli status checks */
                }
-               ast_set_flag(chan_cdr, AST_CDR_FLAG_BRIDGED);
-               if (peer_cdr) {
-                       ast_set_flag(peer_cdr, AST_CDR_FLAG_BRIDGED);
+               if (ast_test_flag(chan,AST_FLAG_BRIDGE_HANGUP_DONT)) {
+                       ast_set_flag(chan_cdr, AST_CDR_FLAG_BRIDGED);
+                       if (peer_cdr) {
+                               ast_set_flag(peer_cdr, AST_CDR_FLAG_BRIDGED);
+                       }
                }
        }
        
@@ -1712,19 +1717,30 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast
 
        }
   before_you_go:
+
+       if (ast_test_flag(chan,AST_FLAG_BRIDGE_HANGUP_DONT)) {
+               ast_clear_flag(chan,AST_FLAG_BRIDGE_HANGUP_DONT); /* its job is done */
+               if (bridge_cdr) {
+                       ast_cdr_discard(bridge_cdr);
+                       /* QUESTION: should we copy bridge_cdr fields to the peer before we throw it away? */
+               }
+               return res; /* if we shouldn't do the h-exten, we shouldn't do the bridge cdr, either! */
+       }
+
        if (config->end_bridge_callback) {
                config->end_bridge_callback(config->end_bridge_callback_data);
        }
 
-       autoloopflag = ast_test_flag(chan, AST_FLAG_IN_AUTOLOOP);
-       ast_set_flag(chan, AST_FLAG_IN_AUTOLOOP);
-       if (!ast_test_flag(&(config->features_caller),AST_FEATURE_NO_H_EXTEN) && ast_exists_extension(chan, chan->context, "h", 1, chan->cid.cid_num)) {
+       if (!ast_test_flag(&(config->features_caller),AST_FEATURE_NO_H_EXTEN) && 
+           ast_exists_extension(chan, chan->context, "h", 1, chan->cid.cid_num)) {
                struct ast_cdr *swapper = NULL;
                char savelastapp[AST_MAX_EXTENSION];
                char savelastdata[AST_MAX_EXTENSION];
                char save_exten[AST_MAX_EXTENSION];
                int  save_prio;
                
+               autoloopflag = ast_test_flag(chan, AST_FLAG_IN_AUTOLOOP);
+               ast_set_flag(chan, AST_FLAG_IN_AUTOLOOP);
                if (bridge_cdr && ast_opt_end_cdr_before_h_exten) {
                        ast_cdr_end(bridge_cdr);
                }
@@ -1766,9 +1782,9 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast
                        ast_copy_string(bridge_cdr->lastapp, savelastapp, sizeof(bridge_cdr->lastapp));
                        ast_copy_string(bridge_cdr->lastdata, savelastdata, sizeof(bridge_cdr->lastdata));
                }
+               ast_set2_flag(chan, autoloopflag, AST_FLAG_IN_AUTOLOOP);
        }
-       ast_set2_flag(chan, autoloopflag, AST_FLAG_IN_AUTOLOOP);
-
+       
        /* obey the NoCDR() wishes. -- move the DISABLED flag to the bridge CDR if it was set on the channel during the bridge... */
        new_chan_cdr = pick_unlocked_cdr(chan->cdr); /* the proper chan cdr, if there are forked cdrs */
        if (bridge_cdr && new_chan_cdr && ast_test_flag(new_chan_cdr, AST_CDR_FLAG_POST_DISABLED)) {
@@ -1858,6 +1874,7 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast
                        ast_cdr_specialized_reset(peer_cdr,0); /* nothing changed, reset the peer_cdr  */
                }
        }
+       
        return res;
 }