[svn-commits] rmudgett: trunk r319998 - in /trunk: ./ apps/ main/
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Fri May 20 10:52:25 CDT 2011
Author: rmudgett
Date: Fri May 20 10:52:20 2011
New Revision: 319998
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=319998
Log:
Merged revisions 319997 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8
........
r319997 | rmudgett | 2011-05-20 10:48:25 -0500 (Fri, 20 May 2011) | 25 lines
Crash when using directed pickup applications.
The directed pickup applications can cause a crash if the pickup was
successful because the dialplan keeps executing.
This patch does the following:
* Completes the channel masquerade on a successful pickup before the
application returns. The channel is now guaranteed a zombie and must not
continue executing the dialplan.
* Changes the return value of the directed pickup applications to return
zero if the pickup failed and nonzero(-1) if the pickup succeeded.
* Made some code optimizations that no longer require re-checking the
pickup channel to see if it is still available to pickup.
(closes issue #19310)
Reported by: remiq
Patches:
issue19310_v1.8_v2.patch uploaded by rmudgett (license 664)
Tested by: alecdavis, remiq, rmudgett
Review: https://reviewboard.asterisk.org/r/1221/
........
Modified:
trunk/ (props changed)
trunk/apps/app_directed_pickup.c
trunk/main/features.c
Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.
Modified: trunk/apps/app_directed_pickup.c
URL: http://svnview.digium.com/svn/asterisk/trunk/apps/app_directed_pickup.c?view=diff&rev=319998&r1=319997&r2=319998
==============================================================================
--- trunk/apps/app_directed_pickup.c (original)
+++ trunk/apps/app_directed_pickup.c Fri May 20 10:52:20 2011
@@ -158,22 +158,21 @@
return ast_channel_callback(pickup_by_name_cb, NULL, &pickup_args, 0);
}
-/*! \brief Attempt to pick up specified channel named , does not use context */
+/*! \brief Attempt to pick up named channel, does not use context */
static int pickup_by_channel(struct ast_channel *chan, char *pickup)
{
- int res = 0;
+ int res = -1;
struct ast_channel *target;
- if (!(target = my_ast_get_channel_by_name_locked(pickup))) {
- return -1;
- }
-
- /* Just check that we are not picking up the SAME as target */
- if (chan != target) {
- res = ast_do_pickup(chan, target);
- }
- ast_channel_unlock(target);
- target = ast_channel_unref(target);
+ target = my_ast_get_channel_by_name_locked(pickup);
+ if (target) {
+ /* Just check that we are not picking up the SAME as target. (i.e. ourself) */
+ if (chan != target) {
+ res = ast_do_pickup(chan, target);
+ }
+ ast_channel_unlock(target);
+ target = ast_channel_unref(target);
+ }
return res;
}
@@ -215,17 +214,16 @@
struct ast_channel *c = obj;
const char *mark = data;
const char *tmp;
- int res;
ast_channel_lock(c);
-
- res = (tmp = pbx_builtin_getvar_helper(c, PICKUPMARK)) &&
- !strcasecmp(tmp, mark) &&
- can_pickup(c);
-
+ tmp = pbx_builtin_getvar_helper(c, PICKUPMARK);
+ if (tmp && !strcasecmp(tmp, mark) && can_pickup(c)) {
+ /* Return with the channel still locked on purpose */
+ return CMP_MATCH | CMP_STOP;
+ }
ast_channel_unlock(c);
- return res ? CMP_MATCH | CMP_STOP : 0;
+ return 0;
}
/* Attempt to pick up specified mark */
@@ -234,18 +232,13 @@
struct ast_channel *target;
int res = -1;
- if (!(target = ast_channel_callback(find_by_mark, NULL, (char *) mark, 0))) {
- return res;
- }
-
- ast_channel_lock(target);
- if (can_pickup(target)) {
+ /* The found channel is already locked. */
+ target = ast_channel_callback(find_by_mark, NULL, (char *) mark, 0);
+ if (target) {
res = ast_do_pickup(chan, target);
- } else {
- ast_log(LOG_WARNING, "target has gone, or not ringing anymore for %s\n", chan->name);
- }
- ast_channel_unlock(target);
- target = ast_channel_unref(target);
+ ast_channel_unlock(target);
+ target = ast_channel_unref(target);
+ }
return res;
}
@@ -254,14 +247,15 @@
{
struct ast_channel *chan = obj;
struct ast_channel *c = data;
- int i;
ast_channel_lock(chan);
- i = (c != chan) && (c->pickupgroup & chan->callgroup) &&
- can_pickup(chan);
-
+ if (c != chan && (c->pickupgroup & chan->callgroup) && can_pickup(chan)) {
+ /* Return with the channel still locked on purpose */
+ return CMP_MATCH | CMP_STOP;
+ }
ast_channel_unlock(chan);
- return i ? CMP_MATCH | CMP_STOP : 0;
+
+ return 0;
}
static int pickup_by_group(struct ast_channel *chan)
@@ -269,19 +263,14 @@
struct ast_channel *target;
int res = -1;
- if (!(target = ast_channel_callback(find_channel_by_group, NULL, chan, 0))) {
- return res;
- }
-
- ast_log(LOG_NOTICE, "%s, pickup attempt by %s\n", target->name, chan->name);
- ast_channel_lock(target);
- if (can_pickup(target)) {
+ /* The found channel is already locked. */
+ target = ast_channel_callback(find_channel_by_group, NULL, chan, 0);
+ if (target) {
+ ast_log(LOG_NOTICE, "pickup %s attempt by %s\n", target->name, chan->name);
res = ast_do_pickup(chan, target);
- } else {
- ast_log(LOG_WARNING, "target has gone, or not ringing anymore for %s\n", chan->name);
- }
- ast_channel_unlock(target);
- target = ast_channel_unref(target);
+ ast_channel_unlock(target);
+ target = ast_channel_unref(target);
+ }
return res;
}
@@ -289,13 +278,11 @@
/* application entry point for Pickup() */
static int pickup_exec(struct ast_channel *chan, const char *data)
{
- int res = 0;
char *tmp = ast_strdupa(data);
char *exten = NULL, *context = NULL;
if (ast_strlen_zero(data)) {
- res = pickup_by_group(chan);
- return res;
+ return pickup_by_group(chan) ? 0 : -1;
}
/* Parse extension (and context if there) */
@@ -303,16 +290,21 @@
if ((context = strchr(exten, '@')))
*context++ = '\0';
if (!ast_strlen_zero(context) && !strcasecmp(context, PICKUPMARK)) {
- if (!pickup_by_mark(chan, exten))
- break;
+ if (!pickup_by_mark(chan, exten)) {
+ /* Pickup successful. Stop the dialplan this channel is a zombie. */
+ return -1;
+ }
} else {
- if (!pickup_by_exten(chan, exten, !ast_strlen_zero(context) ? context : chan->context))
- break;
+ if (!pickup_by_exten(chan, exten, !ast_strlen_zero(context) ? context : chan->context)) {
+ /* Pickup successful. Stop the dialplan this channel is a zombie. */
+ return -1;
+ }
}
ast_log(LOG_NOTICE, "No target channel found for %s.\n", exten);
}
- return res;
+ /* Pickup failed. Keep going in the dialplan. */
+ return 0;
}
/* Find channel for pick up specified by partial channel name */
@@ -320,16 +312,16 @@
{
struct ast_channel *c = obj;
const char *part = data;
- int res = 0;
int len = strlen(part);
ast_channel_lock(c);
- if (len <= strlen(c->name)) {
- res = !(strncmp(c->name, part, len)) && (can_pickup(c));
+ if (len <= strlen(c->name) && !strncmp(c->name, part, len) && can_pickup(c)) {
+ /* Return with the channel still locked on purpose */
+ return CMP_MATCH | CMP_STOP;
}
ast_channel_unlock(c);
- return res ? CMP_MATCH | CMP_STOP : 0;
+ return 0;
}
/* Attempt to pick up specified by partial channel name */
@@ -338,13 +330,10 @@
struct ast_channel *target;
int res = -1;
- if ((target = ast_channel_callback(find_by_part, NULL, (char *) part, 0))) {
- ast_channel_lock(target);
- if (can_pickup(target)) {
- res = ast_do_pickup(chan, target);
- } else {
- ast_log(LOG_WARNING, "target has gone, or not ringing anymore for %s\n", chan->name);
- }
+ /* The found channel is already locked. */
+ target = ast_channel_callback(find_by_part, NULL, (char *) part, 0);
+ if (target) {
+ res = ast_do_pickup(chan, target);
ast_channel_unlock(target);
target = ast_channel_unref(target);
}
@@ -355,7 +344,6 @@
/* application entry point for PickupChan() */
static int pickupchan_exec(struct ast_channel *chan, const char *data)
{
- int res = 0;
int partial_pickup = 0;
char *pickup = NULL;
char *parse = ast_strdupa(data);
@@ -367,7 +355,8 @@
if (ast_strlen_zero(args.channel)) {
ast_log(LOG_WARNING, "PickupChan requires an argument (channel)!\n");
- return -1;
+ /* Pickup failed. Keep going in the dialplan. */
+ return 0;
}
if (!ast_strlen_zero(args.options) && strchr(args.options, 'p')) {
@@ -381,16 +370,19 @@
} else {
if (partial_pickup) {
if (!pickup_by_part(chan, pickup)) {
- break;
+ /* Pickup successful. Stop the dialplan this channel is a zombie. */
+ return -1;
}
} else if (!pickup_by_channel(chan, pickup)) {
- break;
+ /* Pickup successful. Stop the dialplan this channel is a zombie. */
+ return -1;
}
ast_log(LOG_NOTICE, "No target channel found for %s.\n", pickup);
}
}
- return res;
+ /* Pickup failed. Keep going in the dialplan. */
+ return 0;
}
static int unload_module(void)
Modified: trunk/main/features.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/features.c?view=diff&rev=319998&r1=319997&r2=319998
==============================================================================
--- trunk/main/features.c (original)
+++ trunk/main/features.c Fri May 20 10:52:20 2011
@@ -5759,21 +5759,21 @@
static int find_channel_by_group(void *obj, void *arg, void *data, int flags)
{
+ struct ast_channel *chan = obj;
struct ast_channel *c = data;
- struct ast_channel *chan = obj;
- int i;
-
- i = !chan->pbx &&
- /* Accessing 'chan' here is safe without locking, because there is no way for
- the channel to disappear from under us at this point. pickupgroup *could*
- change while we're here, but that isn't a problem. */
- (c != chan) &&
- (c->pickupgroup & chan->callgroup) &&
+
+ ast_channel_lock(chan);
+ if (c != chan && (c->pickupgroup & chan->callgroup) &&
+ !chan->pbx &&
((chan->_state == AST_STATE_RINGING) || (chan->_state == AST_STATE_RING)) &&
!chan->masq &&
- !ast_test_flag(chan, AST_FLAG_ZOMBIE);
-
- return i ? CMP_MATCH | CMP_STOP : 0;
+ !ast_test_flag(chan, AST_FLAG_ZOMBIE)) {
+ /* Return with the channel still locked on purpose */
+ return CMP_MATCH | CMP_STOP;
+ }
+ ast_channel_unlock(chan);
+
+ return 0;
}
/*!
@@ -5790,35 +5790,26 @@
int res = -1;
ast_debug(1, "pickup attempt by %s\n", chan->name);
- /* If 2 callers try to pickup the same call, allow 1 to win, and other to fail early.*/
- if ((target = ast_channel_callback(find_channel_by_group, NULL, chan, 0))) {
- ast_channel_lock(target);
-
- /* We now have the target locked, but is the target still available to pickup */
- if (!target->masq && !ast_test_flag(target, AST_FLAG_ZOMBIE) &&
- ((target->_state == AST_STATE_RINGING) || (target->_state == AST_STATE_RING))) {
-
- ast_log(LOG_NOTICE, "pickup %s attempt by %s\n", target->name, chan->name);
-
- /* A 2nd pickup attempt will bail out when we unlock the target */
- if (!(res = ast_do_pickup(chan, target))) {
- ast_channel_unlock(target);
- if (!(res = ast_do_masquerade(target))) {
- if (!ast_strlen_zero(pickupsound)) {
- ast_channel_lock(target);
- ast_stream_and_wait(target, pickupsound, "");
- ast_channel_unlock(target);
- }
- } else {
- ast_log(LOG_WARNING, "Failed to perform masquerade\n");
- }
- } else {
- ast_log(LOG_WARNING, "pickup %s failed by %s\n", target->name, chan->name);
- ast_channel_unlock(target);
+ /* The found channel is already locked. */
+ target = ast_channel_callback(find_channel_by_group, NULL, chan, 0);
+ if (target) {
+ ast_log(LOG_NOTICE, "pickup %s attempt by %s\n", target->name, chan->name);
+
+ res = ast_do_pickup(chan, target);
+ if (!res) {
+ if (!ast_strlen_zero(pickupsound)) {
+ /*!
+ * \todo We are not the bridge thread when we inject this sound
+ * so we need to hold the target channel lock while the sound is
+ * played. A better way needs to be found as this pauses the
+ * system.
+ */
+ ast_stream_and_wait(target, pickupsound, "");
}
} else {
- ast_channel_unlock(target);
- }
+ ast_log(LOG_WARNING, "pickup %s failed by %s\n", target->name, chan->name);
+ }
+ ast_channel_unlock(target);
target = ast_channel_unref(target);
}
@@ -5881,6 +5872,11 @@
/* If you want UniqueIDs, set channelvars in manager.conf to CHANNEL(uniqueid) */
ast_manager_event_multichan(EVENT_FLAG_CALL, "Pickup", 2, chans,
"Channel: %s\r\nTargetChannel: %s\r\n", chan->name, target->name);
+
+ /* Do the masquerade manually to make sure that is is completed. */
+ ast_channel_unlock(target);
+ ast_do_masquerade(target);
+ ast_channel_lock(target);
return 0;
}
More information about the svn-commits
mailing list