[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