Merged revisions 166665 via svnmerge from

https://origsvn.digium.com/svn/asterisk/trunk

This merged from trunk with no conflicts. I tested
mostly the 'tired' cases, and for the most part
ignored the tests for reconnecting and dialing in
to fetch a parked call, after the first case.

................
  r166665 | murf | 2008-12-23 11:13:49 -0700 (Tue, 23 Dec 2008) | 153 lines
  
  Merged revisions 166093 via svnmerge from 
  https://origsvn.digium.com/svn/asterisk/branches/1.4
  
  In order to merge this 1.4 patch into trunk,
  I had to resolve some conflicts and wait for
  Russell to make some changes to res_agi.
  I re-ran all the tests; 39 calls in all, and
  made fairly careful notes and comparisons: I
  don't want this to blow up some aspect of 
  asterisk; I completely removed the KEEPALIVE
  from the pbx.h decls. The first 3 scenarios
  involving feature park; feature xfer to 700;
  hookflash park to Park() app call all behave
  the same, don't appear to leave hung channels,
  and no crashes.
  
  ........
    r166093 | murf | 2008-12-19 15:30:32 -0700 (Fri, 19 Dec 2008) | 131 lines
    
    This merges the masqpark branch into 1.4
    
    These changes eliminate the need for (and use of)
    the KEEPALIVE return code in res_features.c;
    There are other places that use this result code
    for similar purposes at a higher level, these appear
    to be left alone in 1.4, but attacked in trunk.
    
    The reason these changes are being made in 1.4, is
    that parking ends a channel's life, in some situations,
    and the code in the bridge (and some other places),
    was not checking the result code properly, and dereferencing
    the channel pointer, which could lead to memory corruption
    and crashes.
    
    Calling the masq_park function eliminates this danger 
    in higher levels.
    
    A series of previous commits have replaced some parking calls
    with masq_park, but this patch puts them ALL to rest,
    (except one, purposely left alone because a masquerade
    is done anyway), and gets rid of the code that tests
    the KEEPALIVE result, and the NOHANGUP_PEER result codes.
    
    While bug 13820 inspired this work, this patch does
    not solve all the problems mentioned there.
    
    I have tested this patch (again) to make sure I have
    not introduced regressions. 
    
    Crashes that occurred when a parked party hung up
    while the parking party was listening to the numbers
    of the parking stall being assigned, is eliminated.
    
    These are the cases where parking code may be activated:
    
    1. Feature one touch (eg. *3)
    2. Feature blind xfer to parking lot (eg ##700)
    3. Run Park() app from dialplan (eg sip xfer to 700)
       (eg. dahdi hookflash xfer to 700)
    4. Run Park via manager.
    
    The interesting testing cases for parking are:
    I. A calls B, A parks B
        a. B hangs up while A is getting the numbers announced.
        b. B hangs up after A gets the announcement, but 
           before the parking time expires
        c. B waits, time expires, A is redialed,
           A answers, B and A are connected, after
           which, B hangs up.
        d. C picks up B while still in parking lot.
    
    II. A calls B, B parks A
        a. A hangs up while B is getting the numbers announced.
        b. A hangs up after B gets the announcement, but 
           before the parking time expires
        c. A waits, time expires, B is redialed,
           B answers, A and B are connected, after
           which, A hangs up.
        d. C picks up A while still in parking lot.
    
    Testing this throroughly involves acting all the permutations
    of I and II, in situations 1,2,3, and 4.
    
    Since I added a few more changes (ALL references to KEEPALIVE in the bridge
    code eliimated (I missed one earlier), I retested
    most of the above cases, and no crashes.
    
    H-extension weirdness.
    
    Current h-extension execution is not completely
    correct for several of the cases.
    
    For the case where A calls B, and A parks B, the
    'h' exten is run on A's channel as soon as the park
    is accomplished. This is expected behavior.
    
    But when A calls B, and B parks A, this will be
    current behavior:
    
    After B parks A, B is hung up by the system, and
    the 'h' (hangup) exten gets run, but the channel
    mentioned will be a derivative of A's...
    
    Thus, if A is DAHDI/1, and B is DAHDI/2,
    the h-extension will be run on channel
    Parked/DAHDI/1-1<ZOMBIE>, and the 
    start/answer/end info will be those 
    relating to Channel A.
    
    And, in the case where A is reconnected to
    B after the park time expires, when both parties
    hang up after the joyful reunion, no h-exten
    will be run at all.
    
    In the case where C picks up A from the 
    parking lot, when either A or C hang up,
    the h-exten will be run for the C channel.
    
    CDR's are a separate issue, and not addressed
    here.
    
    As to WHY this strange behavior occurs, 
    the answer lies in the procedure followed
    to accomplish handing over the channel
    to the parking manager thread. This procedure
    is called masquerading. In the process,
    a duplicate copy of the channel is created,
    and most of the active data is given to the
    new copy. The original channel gets its name
    changed to XXX<ZOMBIE> and keeps the PBX
    information for the sake of the original
    thread (preserving its role as a call 
    originator, if it had this role to begin
    with), while the new channel is without
    this info and becomes a call target (a
    "peer").
    
    In this case, the parking lot manager
    thread is handed the new (masqueraded)
    channel. It will not run an h-exten
    on the channel if it hangs up while
    in the parking lot. The h exten will
    be run on the original channel instead,
    in the original thread, after the bridge
    completes.
    
    See bug 13820 for our intentions as
    to how to clean up the h exten behavior.
  
  Review: http://reviewboard.digium.com/r/29/
  
  ........
................


git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.1@166730 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
Steve Murphy
2008-12-24 01:15:43 +00:00
parent 827f655dac
commit a7aeaf341b
7 changed files with 65 additions and 127 deletions

View File

@@ -765,29 +765,22 @@ static int builtin_parkcall(struct ast_channel *chan, struct ast_channel *peer,
int res = 0;
set_peers(&parker, &parkee, peer, chan, sense);
/* Setup the exten/priority to be s/1 since we don't know
where this call should return */
strcpy(chan->exten, "s");
chan->priority = 1;
/* we used to set chan's exten and priority to "s" and 1
here, but this generates (in some cases) an invalid
extension, and if "s" exists, could errantly
cause execution of extensions you don't expect. It
makes more sense to let nature take its course
when chan finishes, and let the pbx do its thing
and hang up when the park is over.
*/
if (chan->_state != AST_STATE_UP)
res = ast_answer(chan);
if (!res)
res = ast_safe_sleep(chan, 1000);
if (!res) {
if (sense == FEATURE_SENSE_CHAN) {
res = ast_park_call(parkee, parker, 0, NULL);
if (!res) {
if (sense == FEATURE_SENSE_CHAN) {
res = AST_PBX_NO_HANGUP_PEER_PARKED;
} else {
res = AST_PBX_KEEPALIVE;
}
}
} else if (sense == FEATURE_SENSE_PEER) {
masq_park_call_announce(parkee, parker, 0, NULL);
res = 0; /* PBX should hangup zombie channel */
}
if (!res) { /* one direction used to call park_call.... */
masq_park_call_announce(parkee, parker, 0, NULL);
res = 0; /* PBX should hangup zombie channel */
}
return res;
@@ -1124,12 +1117,12 @@ static int builtin_blindtransfer(struct ast_channel *chan, struct ast_channel *p
res = finishup(transferee);
if (res)
res = -1;
else if (!ast_park_call(transferee, transferer, 0, NULL)) { /* success */
else if (!masq_park_call_announce(transferee, transferer, 0, NULL)) { /* success */
/* We return non-zero, but tell the PBX not to hang the channel when
the thread dies -- We have to be careful now though. We are responsible for
hanging up the channel, else it will never be hung up! */
return (transferer == peer) ? AST_PBX_KEEPALIVE : AST_PBX_NO_HANGUP_PEER_PARKED;
return 0;
} else {
ast_log(LOG_WARNING, "Unable to park call %s\n", transferee->name);
}
@@ -1680,7 +1673,7 @@ struct ast_call_feature *ast_find_call_feature(const char *name)
* \param chan,peer,config,code,sense,data
*
* Find a feature, determine which channel activated
* \retval AST_FEATURE_RETURN_PBX_KEEPALIVE,AST_FEATURE_RETURN_NO_HANGUP_PEER
* \retval AST_FEATURE_RETURN_NO_HANGUP_PEER
* \retval -1 error.
* \retval -2 when an application cannot be found.
*/
@@ -1735,20 +1728,9 @@ static int feature_exec_app(struct ast_channel *chan, struct ast_channel *peer,
ast_autoservice_stop(idle);
if (res == AST_PBX_KEEPALIVE) {
/* do not hangup peer if feature is to be activated on it */
if ((ast_test_flag(feature, AST_FEATURE_FLAG_ONPEER) && sense == FEATURE_SENSE_CHAN) || (ast_test_flag(feature, AST_FEATURE_FLAG_ONSELF) && sense == FEATURE_SENSE_PEER))
return AST_FEATURE_RETURN_NO_HANGUP_PEER;
else
return AST_FEATURE_RETURN_PBX_KEEPALIVE;
}
else if (res == AST_PBX_NO_HANGUP_PEER)
return AST_FEATURE_RETURN_NO_HANGUP_PEER;
else if (res == AST_PBX_NO_HANGUP_PEER_PARKED)
return AST_FEATURE_RETURN_NO_HANGUP_PEER_PARKED;
else if (res)
if (res) {
return AST_FEATURE_RETURN_SUCCESSBREAK;
}
return AST_FEATURE_RETURN_SUCCESS; /*! \todo XXX should probably return res */
}
@@ -2403,7 +2385,7 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast
}
before_you_go:
if (res != AST_PBX_KEEPALIVE && config->end_bridge_callback) {
if (config->end_bridge_callback) {
config->end_bridge_callback(config->end_bridge_callback_data);
}
@@ -2413,7 +2395,7 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast
*/
autoloopflag = ast_test_flag(chan, AST_FLAG_IN_AUTOLOOP);
ast_set_flag(chan, AST_FLAG_IN_AUTOLOOP);
if (res != AST_PBX_KEEPALIVE && !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];
@@ -2464,11 +2446,9 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast
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... */
if (res != AST_PBX_KEEPALIVE) {
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))
ast_set_flag(bridge_cdr, AST_CDR_FLAG_POST_DISABLED);
}
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))
ast_set_flag(bridge_cdr, AST_CDR_FLAG_POST_DISABLED);
/* we can post the bridge CDR at this point */
if (bridge_cdr) {
@@ -2498,23 +2478,9 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast
5. After a bridge occurs, we have 2 or 3 channels' CDRs
to attend to; if the chan or peer changed names,
we have the before and after attached CDR's.
6. Parking has to be accounted for in the code:
a. Parking will cause ast_bridge_call to return
either AST_PBX_NO_HANGUP_PEER or AST_PBX_NO_HANGUP_PEER_PARKED;
in the latter case, peer is (most likely) a bad
pointer, you can no longer deref it. If it does still
exist, it is under another's thread control, and
could be destroyed at any time.
b. The same applies to AST_PBX_KEEPALIVE, in which
case, the chan ptr cannot be used, as another thread
owns it and may have destroyed the channel.
c. In the former case, you need to check peer to see if it
still exists before you deref it, and obtain a lock.
d. In neither case should you do an ast_hangup(peer).
e. Do not overwrite the result code from ast_bridge_call.
*/
if (res != AST_PBX_KEEPALIVE && new_chan_cdr) {
if (new_chan_cdr) {
struct ast_channel *chan_ptr = NULL;
if (strcasecmp(orig_channame, chan->name) != 0) {
@@ -2540,7 +2506,7 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast
}
}
if (res != AST_PBX_NO_HANGUP_PEER_PARKED) { /* if the peer was involved in a park, don't even touch it; it's probably gone */
{
struct ast_channel *chan_ptr = NULL;
new_peer_cdr = pick_unlocked_cdr(peer->cdr); /* the proper chan cdr, if there are forked cdrs */
if (new_chan_cdr && ast_test_flag(new_chan_cdr, AST_CDR_FLAG_POST_DISABLED) && new_peer_cdr && !ast_test_flag(new_peer_cdr, AST_CDR_FLAG_POST_DISABLED))
@@ -2887,14 +2853,19 @@ static int park_call_exec(struct ast_channel *chan, void *data)
ast_app_parse_options(park_call_options, &flags, NULL, app_args.options);
args.flags = flags.flags;
res = ast_park_call_full(chan, chan, &args);
res = ast_park_call_full(chan, chan, &args); /* In experiments, using the masq_park_call
func here yielded no difference with
current implementation. I saw no advantage
in calling it instead.
*/
/* Continue on in the dialplan */
if (res == 1) {
ast_copy_string(chan->exten, orig_exten, sizeof(chan->exten));
chan->priority = orig_priority;
res = 0;
} else if (!res)
res = AST_PBX_KEEPALIVE;
} else if (!res) {
res = 1;
}
}
return res;
@@ -3019,8 +2990,7 @@ static int park_exec_full(struct ast_channel *chan, void *data, struct ast_parki
ast_cdr_setdestchan(chan->cdr, peer->name);
/* Simulate the PBX hanging up */
if (res != AST_PBX_NO_HANGUP_PEER && res != AST_PBX_NO_HANGUP_PEER_PARKED)
ast_hangup(peer);
ast_hangup(peer);
return res;
} else {
/*! \todo XXX Play a message XXX */