[asterisk-commits] rmudgett: branch 1.8 r378356 - in /branches/1.8: include/asterisk/ main/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed Jan 2 15:08:20 CST 2013


Author: rmudgett
Date: Wed Jan  2 15:08:15 2013
New Revision: 378356

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=378356
Log:
Fix AMI redirect action with two channels failing to redirect both channels.

The AMI redirect action can fail to redirect two channels that are bridged
together.  There is a race between the AMI thread redirecting the two
channels and the bridge thread noticing that a channel is hungup from the
redirects.

* Made the bridge wait for both channels to be redirected before exiting.

* Made the AMI redirect check that all required headers are present before
proceeding with the redirection.

* Made the AMI redirect require that any supplied ExtraChannel exist
before proceeding.  Previously the code fell back to a single channel
redirect operation.

(closes issue ASTERISK-18975)
Reported by: Ben Klang

(closes issue ASTERISK-19948)
Reported by: Brent Dalgleish
Patches:
      jira_asterisk_19948_v11.patch (license #5621) patch uploaded by rmudgett
Tested by: rmudgett, Thomas Sevestre, Deepak Lohani, Kayode

Review: https://reviewboard.asterisk.org/r/2243/

Modified:
    branches/1.8/include/asterisk/channel.h
    branches/1.8/main/features.c
    branches/1.8/main/manager.c

Modified: branches/1.8/include/asterisk/channel.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/include/asterisk/channel.h?view=diff&rev=378356&r1=378355&r2=378356
==============================================================================
--- branches/1.8/include/asterisk/channel.h (original)
+++ branches/1.8/include/asterisk/channel.h Wed Jan  2 15:08:15 2013
@@ -938,12 +938,19 @@
 	 *  some non-traditional dialplans (like AGI) to continue to function.
 	 */
 	AST_FLAG_DISABLE_WORKAROUNDS = (1 << 20),
-	/*! Disable device state event caching.  This allows allows channel
-	 * drivers to selectively prevent device state events from being cached
-	 * by certain channels such as anonymous calls which have no persistent
-	 * represenatation that can be tracked.
+	/*!
+	 * Disable device state event caching.  This allows channel
+	 * drivers to selectively prevent device state events from being
+	 * cached by certain channels such as anonymous calls which have
+	 * no persistent represenatation that can be tracked.
 	 */
 	AST_FLAG_DISABLE_DEVSTATE_CACHE = (1 << 21),
+	/*!
+	 * This flag indicates that a dual channel redirect is in
+	 * progress.  The bridge needs to wait until the flag is cleared
+	 * to continue.
+	 */
+	AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT = (1 << 22),
 };
 
 /*! \brief ast_bridge_config flags */

Modified: branches/1.8/main/features.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/features.c?view=diff&rev=378356&r1=378355&r2=378356
==============================================================================
--- branches/1.8/main/features.c (original)
+++ branches/1.8/main/features.c Wed Jan  2 15:08:15 2013
@@ -4375,6 +4375,11 @@
 		silgen = NULL;
 	}
 
+	/* Wait for any dual redirect to complete. */
+	while (ast_test_flag(chan, AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT)) {
+		sched_yield();
+	}
+
 	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) {

Modified: branches/1.8/main/manager.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/manager.c?view=diff&rev=378356&r1=378355&r2=378356
==============================================================================
--- branches/1.8/main/manager.c (original)
+++ branches/1.8/main/manager.c Wed Jan  2 15:08:15 2013
@@ -3448,6 +3448,7 @@
 /*! \brief  action_redirect: The redirect manager command */
 static int action_redirect(struct mansession *s, const struct message *m)
 {
+	char buf[256];
 	const char *name = astman_get_header(m, "Channel");
 	const char *name2 = astman_get_header(m, "ExtraChannel");
 	const char *exten = astman_get_header(m, "Exten");
@@ -3456,8 +3457,10 @@
 	const char *context2 = astman_get_header(m, "ExtraContext");
 	const char *priority = astman_get_header(m, "Priority");
 	const char *priority2 = astman_get_header(m, "ExtraPriority");
-	struct ast_channel *chan, *chan2 = NULL;
-	int pi, pi2 = 0;
+	struct ast_channel *chan;
+	struct ast_channel *chan2;
+	int pi = 0;
+	int pi2 = 0;
 	int res;
 
 	if (ast_strlen_zero(name)) {
@@ -3465,84 +3468,134 @@
 		return 0;
 	}
 
-	if (!ast_strlen_zero(priority) && (sscanf(priority, "%30d", &pi) != 1)) {
-		if ((pi = ast_findlabel_extension(NULL, context, exten, priority, NULL)) < 1) {
-			astman_send_error(s, m, "Invalid priority");
+	if (ast_strlen_zero(context)) {
+		astman_send_error(s, m, "Context not specified");
+		return 0;
+	}
+	if (ast_strlen_zero(exten)) {
+		astman_send_error(s, m, "Exten not specified");
+		return 0;
+	}
+	if (ast_strlen_zero(priority)) {
+		astman_send_error(s, m, "Priority not specified");
+		return 0;
+	}
+	if (sscanf(priority, "%30d", &pi) != 1) {
+		pi = ast_findlabel_extension(NULL, context, exten, priority, NULL);
+	}
+	if (pi < 1) {
+		astman_send_error(s, m, "Priority is invalid");
+		return 0;
+	}
+
+	if (!ast_strlen_zero(name2) && !ast_strlen_zero(context2)) {
+		/* We have an ExtraChannel and an ExtraContext */
+		if (ast_strlen_zero(exten2)) {
+			astman_send_error(s, m, "ExtraExten not specified");
 			return 0;
 		}
-	}
-
-	if (!ast_strlen_zero(priority2) && (sscanf(priority2, "%30d", &pi2) != 1)) {
-		if ((pi2 = ast_findlabel_extension(NULL, context2, exten2, priority2, NULL)) < 1) {
-			astman_send_error(s, m, "Invalid ExtraPriority");
+		if (ast_strlen_zero(priority2)) {
+			astman_send_error(s, m, "ExtraPriority not specified");
 			return 0;
 		}
-	}
-
-	if (!(chan = ast_channel_get_by_name(name))) {
-		char buf[256];
+		if (sscanf(priority2, "%30d", &pi2) != 1) {
+			pi2 = ast_findlabel_extension(NULL, context2, exten2, priority2, NULL);
+		}
+		if (pi2 < 1) {
+			astman_send_error(s, m, "ExtraPriority is invalid");
+			return 0;
+		}
+	}
+
+	chan = ast_channel_get_by_name(name);
+	if (!chan) {
 		snprintf(buf, sizeof(buf), "Channel does not exist: %s", name);
 		astman_send_error(s, m, buf);
 		return 0;
 	}
-
 	if (ast_check_hangup_locked(chan)) {
 		astman_send_error(s, m, "Redirect failed, channel not up.");
 		chan = ast_channel_unref(chan);
 		return 0;
 	}
 
-	if (!ast_strlen_zero(name2)) {
-		chan2 = ast_channel_get_by_name(name2);
-	}
-
-	if (chan2 && ast_check_hangup_locked(chan2)) {
+	if (ast_strlen_zero(name2)) {
+		/* Single channel redirect in progress. */
+		if (chan->pbx) {
+			ast_channel_lock(chan);
+			/* don't let the after-bridge code run the h-exten */
+			ast_set_flag(chan, AST_FLAG_BRIDGE_HANGUP_DONT);
+			ast_channel_unlock(chan);
+		}
+		res = ast_async_goto(chan, context, exten, pi);
+		if (!res) {
+			astman_send_ack(s, m, "Redirect successful");
+		} else {
+			astman_send_error(s, m, "Redirect failed");
+		}
+		chan = ast_channel_unref(chan);
+		return 0;
+	}
+
+	chan2 = ast_channel_get_by_name(name2);
+	if (!chan2) {
+		snprintf(buf, sizeof(buf), "ExtraChannel does not exist: %s", name2);
+		astman_send_error(s, m, buf);
+		chan = ast_channel_unref(chan);
+		return 0;
+	}
+	if (ast_check_hangup_locked(chan2)) {
 		astman_send_error(s, m, "Redirect failed, extra channel not up.");
+		chan2 = ast_channel_unref(chan2);
 		chan = ast_channel_unref(chan);
-		chan2 = ast_channel_unref(chan2);
 		return 0;
 	}
 
+	/* Dual channel redirect in progress. */
 	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 */
+		/* don't let the after-bridge code run the h-exten */
+		ast_set_flag(chan, AST_FLAG_BRIDGE_HANGUP_DONT
+			| AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT);
 		ast_channel_unlock(chan);
 	}
-
+	if (chan2->pbx) {
+		ast_channel_lock(chan2);
+		/* don't let the after-bridge code run the h-exten */
+		ast_set_flag(chan2, AST_FLAG_BRIDGE_HANGUP_DONT
+			| AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT);
+		ast_channel_unlock(chan2);
+	}
 	res = ast_async_goto(chan, context, exten, pi);
 	if (!res) {
-		if (!ast_strlen_zero(name2)) {
-			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);
-				}
-				if (!ast_strlen_zero(context2)) {
-					res = ast_async_goto(chan2, context2, exten2, pi2);
-				} else {
-					res = ast_async_goto(chan2, context, exten, pi);
-				}
-			} else {
-				res = -1;
-			}
-			if (!res) {
-				astman_send_ack(s, m, "Dual Redirect successful");
-			} else {
-				astman_send_error(s, m, "Secondary redirect failed");
-			}
+		if (!ast_strlen_zero(context2)) {
+			res = ast_async_goto(chan2, context2, exten2, pi2);
 		} else {
-			astman_send_ack(s, m, "Redirect successful");
+			res = ast_async_goto(chan2, context, exten, pi);
+		}
+		if (!res) {
+			astman_send_ack(s, m, "Dual Redirect successful");
+		} else {
+			astman_send_error(s, m, "Secondary redirect failed");
 		}
 	} else {
 		astman_send_error(s, m, "Redirect failed");
 	}
 
+	/* Release the bridge wait. */
+	if (chan->pbx) {
+		ast_channel_lock(chan);
+		ast_clear_flag(chan, AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT);
+		ast_channel_unlock(chan);
+	}
+	if (chan2->pbx) {
+		ast_channel_lock(chan2);
+		ast_clear_flag(chan2, AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT);
+		ast_channel_unlock(chan2);
+	}
+
+	chan2 = ast_channel_unref(chan2);
 	chan = ast_channel_unref(chan);
-	if (chan2) {
-		chan2 = ast_channel_unref(chan2);
-	}
-
 	return 0;
 }
 




More information about the asterisk-commits mailing list