Merged revisions 295843 via svnmerge from

https://origsvn.digium.com/svn/asterisk/branches/1.6.2

................
  r295843 | rmudgett | 2010-11-22 13:28:23 -0600 (Mon, 22 Nov 2010) | 53 lines

  Merged revisions 295790 via svnmerge from
  https://origsvn.digium.com/svn/asterisk/branches/1.4

  ........
    r295790 | rmudgett | 2010-11-22 12:46:26 -0600 (Mon, 22 Nov 2010) | 46 lines

    The channel redirect function (CLI or AMI) hangs up the call instead of redirecting the call.

    To recreate the problem:
    1) Party A calls Party B
    2) Invoke CLI "channel redirect" command to redirect channel call leg
    associated with A.
    3) All associated channels are hung up.

    Note that if the CLI command were done on the channel call leg associated
    with B it works.

    This regression was a result of the fix for issue #16946
    (https://reviewboard.asterisk.org/r/740/).

    The regression affects all features that use an async goto to execute the
    dialplan because of an external event: Channel redirect, AMI redirect, SIP
    REFER, and FAX detection.

    The struct ast_channel._softhangup code is a mess.  The variable is used
    for several purposes that do not necessarily result in the call being hung
    up.  I have added doxygen comments to describe how the various _softhangup
    bits are used.  I have corrected all the places where the variable was
    tested in a non-bit oriented manner.

    The primary fix is the new AST_CONTROL_END_OF_Q frame.  It acts as a weak
    hangup request so the soft hangup requests that do not normally result in
    a hangup do not hangup.

    JIRA SWP-2470
    JIRA SWP-2489

    (closes issue #18171)
    Reported by: SantaFox
    (closes issue #18185)
    Reported by: kwemheuer
    (closes issue #18211)
    Reported by: zahir_koradia
    (closes issue #18230)
    Reported by: vmarrone
    (closes issue #18299)
    Reported by: mbrevda
    (closes issue #18322)
    Reported by: nerbos

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

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

diff --git a/apps/app_macro.c b/apps/app_macro.c
index e5ab2bc21a5e35cd78aabcc83dc654e4c7bcc670..08780ed4de954e2ff8c18587da0d36c16478d699 100644 (file)
--- a/apps/app_macro.c
+++ b/apps/app_macro.c
@@ -539,22 +539,20 @@ static int _macro_exec(struct ast_channel *chan, const char *data, int exclusive
        }
 
        if (!strcasecmp(chan->context, fullmacro)) {
+               const char *offsets;
+
                /* If we're leaving the macro normally, restore original information */
                chan->priority = oldpriority;
                ast_copy_string(chan->context, oldcontext, sizeof(chan->context));
-               if (!(ast_check_hangup(chan) & AST_SOFTHANGUP_ASYNCGOTO)) {
-                       /* Copy the extension, so long as we're not in softhangup, where we could be given an asyncgoto */
-                       const char *offsets;
-                       ast_copy_string(chan->exten, oldexten, sizeof(chan->exten));
-                       if ((offsets = pbx_builtin_getvar_helper(chan, "MACRO_OFFSET"))) {
-                               /* Handle macro offset if it's set by checking the availability of step n + offset + 1, otherwise continue
-                               normally if there is any problem */
-                               if (sscanf(offsets, "%30d", &offset) == 1) {
-                                       if (ast_exists_extension(chan, chan->context, chan->exten,
-                                               chan->priority + offset + 1,
-                                               S_COR(chan->caller.id.number.valid, chan->caller.id.number.str, NULL))) {
-                                               chan->priority += offset;
-                                       }
+               ast_copy_string(chan->exten, oldexten, sizeof(chan->exten));
+               if ((offsets = pbx_builtin_getvar_helper(chan, "MACRO_OFFSET"))) {
+                       /* Handle macro offset if it's set by checking the availability of step n + offset + 1, otherwise continue
+                       normally if there is any problem */
+                       if (sscanf(offsets, "%30d", &offset) == 1) {
+                               if (ast_exists_extension(chan, chan->context, chan->exten,
+                                       chan->priority + offset + 1,
+                                       S_COR(chan->caller.id.number.valid, chan->caller.id.number.str, NULL))) {
+                                       chan->priority += offset;
                                }
                        }
                }

diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 172f69b035dbc8b18f47980142899f613e939ab4..f23fab2863cdae29ad2a13c36021df506026202f 100644 (file)
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -979,14 +979,42 @@ struct outgoing_helper {
 };
 
 enum {
-       /*! Soft hangup by device */
+       /*!
+        * Soft hangup requested by device or other internal reason.
+        * Actual hangup needed.
+        */
        AST_SOFTHANGUP_DEV =       (1 << 0),
-       /*! Soft hangup for async goto */
+       /*!
+        * Used to break the normal frame flow so an async goto can be
+        * done instead of actually hanging up.
+        */
        AST_SOFTHANGUP_ASYNCGOTO = (1 << 1),
+       /*!
+        * Soft hangup requested by system shutdown.  Actual hangup
+        * needed.
+        */
        AST_SOFTHANGUP_SHUTDOWN =  (1 << 2),
+       /*!
+        * Used to break the normal frame flow after a timeout so an
+        * implicit async goto can be done to the 'T' exten if it exists
+        * instead of actually hanging up.  If the exten does not exist
+        * then actually hangup.
+        */
        AST_SOFTHANGUP_TIMEOUT =   (1 << 3),
+       /*!
+        * Soft hangup requested by application/channel-driver being
+        * unloaded.  Actual hangup needed.
+        */
        AST_SOFTHANGUP_APPUNLOAD = (1 << 4),
+       /*!
+        * Soft hangup requested by non-associated party.  Actual hangup
+        * needed.
+        */
        AST_SOFTHANGUP_EXPLICIT =  (1 << 5),
+       /*!
+        * Used to break a bridge so the channel can be spied upon
+        * instead of actually hanging up.
+        */
        AST_SOFTHANGUP_UNBRIDGE =  (1 << 6),
 };
 

diff --git a/include/asterisk/frame.h b/include/asterisk/frame.h
index 3755c5b50221cde6489e2f5be9c88ec8f0fa9db1..c16a3c15085720be0122d2b7bffcb88230af080b 100644 (file)
--- a/include/asterisk/frame.h
+++ b/include/asterisk/frame.h
@@ -332,6 +332,7 @@ enum ast_control_frame_type {
        AST_CONTROL_SRCCHANGE = 26,  /*!< Media source has changed and requires a new RTP SSRC */
        AST_CONTROL_READ_ACTION = 27, /*!< Tell ast_read to take a specific action */
        AST_CONTROL_AOC = 28,           /*!< Advice of Charge with encoded generic AOC payload */
+       AST_CONTROL_END_OF_Q = 29,              /*!< Indicate that this position was the end of the channel queue for a softhangup. */
 };
 
 enum ast_frame_read_action {

diff --git a/main/channel.c b/main/channel.c
index 6fcf7d76744db45eae3ae218a1c9c650f81c7b4f..a47aa61c6c61d19670c9f3a4e30d4f2402abd5c8 100644 (file)
--- a/main/channel.c
+++ b/main/channel.c
@@ -1360,15 +1360,42 @@ static int __ast_queue_frame(struct ast_channel *chan, struct ast_frame *fin, in
 
        ast_channel_lock(chan);
 
-       /* See if the last frame on the queue is a hangup, if so don't queue anything */
-       if ((cur = AST_LIST_LAST(&chan->readq)) &&
-           (cur->frametype == AST_FRAME_CONTROL) &&
-           (cur->subclass.integer == AST_CONTROL_HANGUP)) {
-               ast_channel_unlock(chan);
-               return 0;
+       /*
+        * Check the last frame on the queue if we are queuing the new
+        * frames after it.
+        */
+       cur = AST_LIST_LAST(&chan->readq);
+       if (cur && cur->frametype == AST_FRAME_CONTROL && !head && (!after || after == cur)) {
+               switch (cur->subclass.integer) {
+               case AST_CONTROL_END_OF_Q:
+                       if (fin->frametype == AST_FRAME_CONTROL
+                               && fin->subclass.integer == AST_CONTROL_HANGUP) {
+                               /*
+                                * Destroy the end-of-Q marker frame so we can queue the hangup
+                                * frame in its place.
+                                */
+                               AST_LIST_REMOVE(&chan->readq, cur, frame_list);
+                               ast_frfree(cur);
+
+                               /*
+                                * This has degenerated to a normal queue append anyway.  Since
+                                * we just destroyed the last frame in the queue we must make
+                                * sure that "after" is NULL or bad things will happen.
+                                */
+                               after = NULL;
+                               break;
+                       }
+                       /* Fall through */
+               case AST_CONTROL_HANGUP:
+                       /* Don't queue anything. */
+                       ast_channel_unlock(chan);
+                       return 0;
+               default:
+                       break;
+               }
        }
 
-       /* Build copies of all the frames and count them */
+       /* Build copies of all the new frames and count them */
        AST_LIST_HEAD_INIT_NOLOCK(&frames);
        for (cur = fin; cur; cur = AST_LIST_NEXT(cur, frame_list)) {
                if (!(f = ast_frdup(cur))) {
@@ -3613,14 +3640,17 @@ static struct ast_frame *__ast_read(struct ast_channel *chan, int dropaudio)
                if (chan->generator)
                        ast_deactivate_generator(chan);
 
-               /* It is possible for chan->_softhangup to be set, yet there still be control
-                * frames that still need to be read. Instead of just going to 'done' in the
-                * case of ast_check_hangup(), we instead need to send the HANGUP frame so that
-                * it can mark the end of the read queue. If there are frames to be read, 
-                * ast_queue_control will be called repeatedly, but will only queue one hangup
-                * frame. */
-               if (ast_check_hangup(chan)) {
-                       ast_queue_control(chan, AST_CONTROL_HANGUP);
+               /*
+                * It is possible for chan->_softhangup to be set and there
+                * still be control frames that need to be read.  Instead of
+                * just going to 'done' in the case of ast_check_hangup(), we
+                * need to queue the end-of-Q frame so that it can mark the end
+                * of the read queue.  If there are frames to be read,
+                * ast_queue_control() will be called repeatedly, but will only
+                * queue the first end-of-Q frame.
+                */
+               if (chan->_softhangup) {
+                       ast_queue_control(chan, AST_CONTROL_END_OF_Q);
                } else {
                        goto done;
                }
@@ -3743,12 +3773,21 @@ static struct ast_frame *__ast_read(struct ast_channel *chan, int dropaudio)
                        }
                }
 
-               /* Interpret hangup and return NULL */
+               /* Interpret hangup and end-of-Q frames to return NULL */
                /* XXX why not the same for frames from the channel ? */
-               if (f->frametype == AST_FRAME_CONTROL && f->subclass.integer == AST_CONTROL_HANGUP) {
-                       cause = f->data.uint32;
-                       ast_frfree(f);
-                       f = NULL;
+               if (f->frametype == AST_FRAME_CONTROL) {
+                       switch (f->subclass.integer) {
+                       case AST_CONTROL_HANGUP:
+                               chan->_softhangup |= AST_SOFTHANGUP_DEV;
+                               cause = f->data.uint32;
+                               /* Fall through */
+                       case AST_CONTROL_END_OF_Q:
+                               ast_frfree(f);
+                               f = NULL;
+                               break;
+                       default:
+                               break;
+                       }
                }
        } else {
                chan->blocker = pthread_self();
@@ -4074,7 +4113,9 @@ static struct ast_frame *__ast_read(struct ast_channel *chan, int dropaudio)
                }
        } else {
                /* Make sure we always return NULL in the future */
-               chan->_softhangup |= AST_SOFTHANGUP_DEV;
+               if (!chan->_softhangup) {
+                       chan->_softhangup |= AST_SOFTHANGUP_DEV;
+               }
                if (cause)
                        chan->hangupcause = cause;
                if (chan->generator)
@@ -4148,6 +4189,7 @@ static int attribute_const is_visible_indication(enum ast_control_frame_type con
        case AST_CONTROL_CC:
        case AST_CONTROL_READ_ACTION:
        case AST_CONTROL_AOC:
+       case AST_CONTROL_END_OF_Q:
                break;
 
        case AST_CONTROL_CONGESTION:
@@ -4330,6 +4372,7 @@ int ast_indicate_data(struct ast_channel *chan, int _condition,
        case AST_CONTROL_CC:
        case AST_CONTROL_READ_ACTION:
        case AST_CONTROL_AOC:
+       case AST_CONTROL_END_OF_Q:
                /* Nothing left to do for these. */
                res = 0;
                break;
@@ -6749,11 +6792,13 @@ static enum ast_bridge_result ast_generic_bridge(struct ast_channel *c0, struct
                        /* No frame received within the specified timeout - check if we have to deliver now */
                        if (jb_in_use)
                                ast_jb_get_and_deliver(c0, c1);
-                       if (c0->_softhangup == AST_SOFTHANGUP_UNBRIDGE || c1->_softhangup == AST_SOFTHANGUP_UNBRIDGE) {
-                               if (c0->_softhangup == AST_SOFTHANGUP_UNBRIDGE)
-                                       c0->_softhangup = 0;
-                               if (c1->_softhangup == AST_SOFTHANGUP_UNBRIDGE)
-                                       c1->_softhangup = 0;
+                       if ((c0->_softhangup | c1->_softhangup) & AST_SOFTHANGUP_UNBRIDGE) {/* Bit operators are intentional. */
+                               if (c0->_softhangup & AST_SOFTHANGUP_UNBRIDGE) {
+                                       c0->_softhangup &= ~AST_SOFTHANGUP_UNBRIDGE;
+                               }
+                               if (c1->_softhangup & AST_SOFTHANGUP_UNBRIDGE) {
+                                       c1->_softhangup &= ~AST_SOFTHANGUP_UNBRIDGE;
+                               }
                                c0->_bridge = c1;
                                c1->_bridge = c0;
                        }
@@ -7091,11 +7136,13 @@ enum ast_bridge_result ast_channel_bridge(struct ast_channel *c0, struct ast_cha
                        ast_clear_flag(config, AST_FEATURE_WARNING_ACTIVE);
                }
 
-               if (c0->_softhangup == AST_SOFTHANGUP_UNBRIDGE || c1->_softhangup == AST_SOFTHANGUP_UNBRIDGE) {
-                       if (c0->_softhangup == AST_SOFTHANGUP_UNBRIDGE)
-                               c0->_softhangup = 0;
-                       if (c1->_softhangup == AST_SOFTHANGUP_UNBRIDGE)
-                               c1->_softhangup = 0;
+               if ((c0->_softhangup | c1->_softhangup) & AST_SOFTHANGUP_UNBRIDGE) {/* Bit operators are intentional. */
+                       if (c0->_softhangup & AST_SOFTHANGUP_UNBRIDGE) {
+                               c0->_softhangup &= ~AST_SOFTHANGUP_UNBRIDGE;
+                       }
+                       if (c1->_softhangup & AST_SOFTHANGUP_UNBRIDGE) {
+                               c1->_softhangup &= ~AST_SOFTHANGUP_UNBRIDGE;
+                       }
                        c0->_bridge = c1;
                        c1->_bridge = c0;
                        ast_debug(1, "Unbridge signal received. Ending native bridge.\n");
@@ -7149,8 +7196,9 @@ enum ast_bridge_result ast_channel_bridge(struct ast_channel *c0, struct ast_cha
                                ast_clear_flag(c0, AST_FLAG_NBRIDGE);
                                ast_clear_flag(c1, AST_FLAG_NBRIDGE);
 
-                               if (c0->_softhangup == AST_SOFTHANGUP_UNBRIDGE || c1->_softhangup == AST_SOFTHANGUP_UNBRIDGE)
+                               if ((c0->_softhangup | c1->_softhangup) & AST_SOFTHANGUP_UNBRIDGE) {/* Bit operators are intentional. */
                                        continue;
+                               }
 
                                c0->_bridge = NULL;
                                c1->_bridge = NULL;

diff --git a/main/pbx.c b/main/pbx.c
index 956450bc1bc3f8e76f7d1e40b4ad039d55149485..3ac257badaf0d78ee2ad21f09a0f776cca518477 100644 (file)
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -4631,8 +4631,8 @@ static int collect_digits(struct ast_channel *c, int waittime, char *buf, int bu
                /* As long as we're willing to wait, and as long as it's not defined,
                   keep reading digits until we can't possibly get a right answer anymore.  */
                digit = ast_waitfordigit(c, waittime);
-               if (c->_softhangup == AST_SOFTHANGUP_ASYNCGOTO) {
-                       c->_softhangup = 0;
+               if (c->_softhangup & AST_SOFTHANGUP_ASYNCGOTO) {
+                       c->_softhangup &= ~AST_SOFTHANGUP_ASYNCGOTO;
                } else {
                        if (!digit)     /* No entry */
                                break;
@@ -4703,22 +4703,22 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c,
                while (!(res = ast_spawn_extension(c, c->context, c->exten, c->priority,
                        S_COR(c->caller.id.number.valid, c->caller.id.number.str, NULL),
                        &found, 1))) {
-                       if (c->_softhangup == AST_SOFTHANGUP_TIMEOUT
+                       if ((c->_softhangup & AST_SOFTHANGUP_TIMEOUT)
                                && ast_exists_extension(c, c->context, "T", 1,
                                        S_COR(c->caller.id.number.valid, c->caller.id.number.str, NULL))) {
                                set_ext_pri(c, "T", 0); /* 0 will become 1 with the c->priority++; at the end */
                                /* If the AbsoluteTimeout is not reset to 0, we'll get an infinite loop */
                                memset(&c->whentohangup, 0, sizeof(c->whentohangup));
                                c->_softhangup &= ~AST_SOFTHANGUP_TIMEOUT;
-                       } else if (c->_softhangup == AST_SOFTHANGUP_TIMEOUT
+                       } else if ((c->_softhangup & AST_SOFTHANGUP_TIMEOUT)
                                && ast_exists_extension(c, c->context, "e", 1,
                                        S_COR(c->caller.id.number.valid, c->caller.id.number.str, NULL))) {
                                pbx_builtin_raise_exception(c, "ABSOLUTETIMEOUT");
                                /* If the AbsoluteTimeout is not reset to 0, we'll get an infinite loop */
                                memset(&c->whentohangup, 0, sizeof(c->whentohangup));
                                c->_softhangup &= ~AST_SOFTHANGUP_TIMEOUT;
-                       } else if (c->_softhangup == AST_SOFTHANGUP_ASYNCGOTO) {
-                               c->_softhangup = 0;
+                       } else if (c->_softhangup & AST_SOFTHANGUP_ASYNCGOTO) {
+                               c->_softhangup &= ~AST_SOFTHANGUP_ASYNCGOTO;
                                continue;
                        } else if (ast_check_hangup(c)) {
                                ast_debug(1, "Extension %s, priority %d returned normally even though call was hung up\n",
@@ -4765,10 +4765,10 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c,
                                        }
                                }
 
-                               if (c->_softhangup == AST_SOFTHANGUP_ASYNCGOTO) {
-                                       c->_softhangup = 0;
+                               if (c->_softhangup & AST_SOFTHANGUP_ASYNCGOTO) {
+                                       c->_softhangup &= ~AST_SOFTHANGUP_ASYNCGOTO;
                                        continue;
-                               } else if (c->_softhangup == AST_SOFTHANGUP_TIMEOUT
+                               } else if ((c->_softhangup & AST_SOFTHANGUP_TIMEOUT)
                                        && ast_exists_extension(c, c->context, "T", 1,
                                                S_COR(c->caller.id.number.valid, c->caller.id.number.str, NULL))) {
                                        set_ext_pri(c, "T", 1);
@@ -4814,9 +4814,9 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c,
                                error = 1; /* we know what to do with it */
                                break;
                        }
-               } else if (c->_softhangup == AST_SOFTHANGUP_TIMEOUT) {
+               } else if (c->_softhangup & AST_SOFTHANGUP_TIMEOUT) {
                        /* If we get this far with AST_SOFTHANGUP_TIMEOUT, then we know that the "T" extension is next. */
-                       c->_softhangup = 0;
+                       c->_softhangup &= ~AST_SOFTHANGUP_TIMEOUT;
                } else {        /* keypress received, get more digits for a full extension */
                        int waittime = 0;
                        if (digit)
@@ -9274,7 +9274,7 @@ static int pbx_builtin_waitexten(struct ast_channel *chan, const char *data)
                if (ast_exists_extension(chan, chan->context, chan->exten, chan->priority + 1,
                        S_COR(chan->caller.id.number.valid, chan->caller.id.number.str, NULL))) {
                        ast_verb(3, "Timeout on %s, continuing...\n", chan->name);
-               } else if (chan->_softhangup == AST_SOFTHANGUP_TIMEOUT) {
+               } else if (chan->_softhangup & AST_SOFTHANGUP_TIMEOUT) {
                        ast_verb(3, "Call timeout on %s, checking for 'T'\n", chan->name);
                        res = -1;
                } else if (ast_exists_extension(chan, chan->context, "t", 1,