[asterisk-commits] murf: trunk r172063 - in /trunk: ./ apps/ include/asterisk/ main/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed Jan 28 14:31:06 CST 2009


Author: murf
Date: Wed Jan 28 14:31:06 2009
New Revision: 172063

URL: http://svn.digium.com/svn-view/asterisk?view=rev&rev=172063
Log:
Merged revisions 172030 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
  r172030 | murf | 2009-01-28 11:51:16 -0700 (Wed, 28 Jan 2009) | 46 lines
  
  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
........

Modified:
    trunk/   (props changed)
    trunk/apps/app_channelredirect.c
    trunk/include/asterisk/channel.h
    trunk/main/features.c
    trunk/main/manager.c
    trunk/main/pbx.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-1.4-merged' - no diff available.

Modified: trunk/apps/app_channelredirect.c
URL: http://svn.digium.com/svn-view/asterisk/trunk/apps/app_channelredirect.c?view=diff&rev=172063&r1=172062&r2=172063
==============================================================================
--- trunk/apps/app_channelredirect.c (original)
+++ trunk/apps/app_channelredirect.c Wed Jan 28 14:31:06 2009
@@ -93,6 +93,9 @@
 		return 0;
 	}
 
+	if (chan2->pbx) {
+		ast_set_flag(chan2, AST_FLAG_BRIDGE_HANGUP_DONT); /* don't let the after-bridge code run the h-exten */
+	}
 	res = ast_async_parseable_goto(chan2, args.label);
 	pbx_builtin_setvar_helper(chan, "CHANNELREDIRECT_STATUS", "SUCCESS");
 	ast_channel_unlock(chan2);

Modified: trunk/include/asterisk/channel.h
URL: http://svn.digium.com/svn-view/asterisk/trunk/include/asterisk/channel.h?view=diff&rev=172063&r1=172062&r2=172063
==============================================================================
--- trunk/include/asterisk/channel.h (original)
+++ trunk/include/asterisk/channel.h Wed Jan 28 14:31:06 2009
@@ -556,6 +556,10 @@
 	 *  a message aimed at preventing a subsequent hangup exten being run at the pbx_run
 	 *  level */
 	AST_FLAG_BRIDGE_HANGUP_RUN = (1 << 17),
+	/*! 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 */

Modified: trunk/main/features.c
URL: http://svn.digium.com/svn-view/asterisk/trunk/main/features.c?view=diff&rev=172063&r1=172062&r2=172063
==============================================================================
--- trunk/main/features.c (original)
+++ trunk/main/features.c Wed Jan 28 14:31:06 2009
@@ -1227,16 +1227,24 @@
 		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;
+			ast_log(LOG_DEBUG,"transferer=%s; transferee=%s; lastapp=%s; lastdata=%s; chan=%s; dstchan=%s\n",
+					transferer->name, transferee->name, transferer->cdr->lastapp, transferer->cdr->lastdata, 
+					transferer->cdr->channel, transferer->cdr->dstchannel);
+			ast_log(LOG_DEBUG,"TRANSFEREE; lastapp=%s; lastdata=%s, chan=%s; dstchan=%s\n",
+					transferee->cdr->lastapp, transferee->cdr->lastdata, transferee->cdr->channel, transferee->cdr->dstchannel);
+			ast_log(LOG_DEBUG,"transferer_real_context=%s; xferto=%s\n", transferer_real_context, xferto);
+			/* 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 */
@@ -1246,6 +1254,8 @@
 				ast_log(LOG_WARNING, "Async goto failed :-(\n");
 		} 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 */
+			ast_log(LOG_DEBUG,"ABOUT TO AST_ASYNC_GOTO, have a pbx... set HANGUP_DONT on chan=%s\n", transferee->name);
 			set_c_e_p(transferee, transferer_real_context, xferto, 0);
 		}
 		check_goto_on_transfer(transferer);
@@ -2316,9 +2326,11 @@
 			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);
+			}
 		}
 	}
 	for (;;) {
@@ -2483,6 +2495,16 @@
 
 	}
    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);
 	}
@@ -2491,9 +2513,8 @@
 	 * if it were, then chan belongs to a different thread now, and might have been hung up long
      * ago.
 	 */
-	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];
@@ -2502,6 +2523,8 @@
 		int  found = 0;	/* set if we find at least one match */
 		int  spawn_error = 0;
 		
+		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);
 		}
@@ -2540,9 +2563,9 @@
 			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))
@@ -2631,6 +2654,7 @@
 			ast_cdr_specialized_reset(peer_cdr,0); /* nothing changed, reset the peer_cdr  */
 		}
 	}
+	
 	return res;
 }
 

Modified: trunk/main/manager.c
URL: http://svn.digium.com/svn-view/asterisk/trunk/main/manager.c?view=diff&rev=172063&r1=172062&r2=172063
==============================================================================
--- trunk/main/manager.c (original)
+++ trunk/main/manager.c Wed Jan 28 14:31:06 2009
@@ -2054,13 +2054,24 @@
 		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

Modified: trunk/main/pbx.c
URL: http://svn.digium.com/svn-view/asterisk/trunk/main/pbx.c?view=diff&rev=172063&r1=172062&r2=172063
==============================================================================
--- trunk/main/pbx.c (original)
+++ trunk/main/pbx.c Wed Jan 28 14:31:06 2009
@@ -4353,6 +4353,9 @@
 			!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 ((res = ast_spawn_extension(c, c->context, c->exten, c->priority, c->cid.cid_num, &found, 1)) == 0) {
 			c->priority++;
 		}




More information about the asterisk-commits mailing list