kenmirrors/asterisk
1
0
Fork 0
mirror of https://github.com/asterisk/asterisk.git synced 2025-09-05 04:11:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

The magic trick to avoid this crash is not to

Browse Source 
try to find the channel by name in the list,
which is slow and resource consuming, but rather
to pay attention to the result codes from the
ast_bridge_call, to which I added the 
AST_PBX_NO_HANGUP_PEER_PARKED value, which
now are returned when a channel is parked.

If you get AST_PBX_KEEPALIVE,
then don't touch the channel pointer.

If you get AST_PBX_NO_HANGUP_PEER, or
AST_PBX_NO_HANGUP_PEER_PARKED, then don't
touch the peer pointer.

Updated the several places where the results
from a bridge were not being properly obeyed,
and fixed some code I had introduced so that
the results of the bridge were not overridden 
(in trunk).

All the places that previously tested for 
AST_PBX_NO_HANGUP_PEER now have to check for
both AST_PBX_NO_HANGUP_PEER and AST_PBX_NO_HANGUP_PEER_PARKED.

I tested this against the 4 common parking
scenarios:


1. A calls B; B answers; A parks B; B hangs up while A is getting the parking
slot announcement, immediately after being put on hold.

2. A calls B; B answers; A parks B; B hangs up after A has been hung up, but
before the park times out.

3. A calls B; B answers; B parks A; A hangs up while B is getting the parking slot announcement, immediately after being put on hold.

4. A calls B; B answers; B parks A; A hangs up after B has been hung up, but before the park times out.


No crash.

I also ran the scenarios above against valgrind, and accesses looked good.




git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.4@152535 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
Steve Murphy
2008-10-29 04:36:32 +00:00
parent fd9de775c5
commit 961bc7e758
5 changed files with 116 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
apps/app_dial.c
View File

@@ -1788,7 +1788,7 @@ static int dial_exec_full(struct ast_channel *chan, void *data, struct ast_flags
}
res = ast_bridge_call(chan,peer,&config);
time(&end_time);
{
if (res != AST_PBX_KEEPALIVE) { /* if keepalive is set, don't even think about accessing chan! */
char toast[80];
snprintf(toast, sizeof(toast), "%ld", (long)(end_time - answer_time));
pbx_builtin_setvar_helper(chan, "ANSWEREDTIME", toast);
@@ -1797,36 +1797,40 @@ static int dial_exec_full(struct ast_channel *chan, void *data, struct ast_flags
time(&end_time);
res = -1;
}
{
if (res != AST_PBX_KEEPALIVE) { /* if keepalive is set, don't even think about accessing chan! */
char toast[80];
snprintf(toast, sizeof(toast), "%ld", (long)(end_time - start_time));
pbx_builtin_setvar_helper(chan, "DIALEDTIME", toast);
}
if (res != AST_PBX_NO_HANGUP_PEER) {
if (!chan->_softhangup)
if (res != AST_PBX_NO_HANGUP_PEER && res != AST_PBX_NO_HANGUP_PEER_PARKED) {
if (res != AST_PBX_KEEPALIVE && !chan->_softhangup)
chan->hangupcause = peer->hangupcause;
ast_hangup(peer);
}
}
out:
if (moh) {
moh = 0;
ast_moh_stop(chan);
} else if (sentringing) {
sentringing = 0;
ast_indicate(chan, -1);
}
ast_rtp_early_bridge(chan, NULL);
hanguptree(outgoing, NULL);
pbx_builtin_setvar_helper(chan, "DIALSTATUS", status);
if (option_debug)
ast_log(LOG_DEBUG, "Exiting with DIALSTATUS=%s.\n", status);
if ((ast_test_flag(peerflags, OPT_GO_ON)) && (!chan->_softhangup) && (res != AST_PBX_KEEPALIVE)) {
if (calldurationlimit)
chan->whentohangup = 0;
res = 0;
/* cleaning up chan is not a good idea here if AST_PBX_KEEPALIVE
is returned; chan will get the love it needs from another
thread */
if (res != AST_PBX_KEEPALIVE) {
if (moh) {
moh = 0;
ast_moh_stop(chan);
} else if (sentringing) {
sentringing = 0;
ast_indicate(chan, -1);
}
ast_rtp_early_bridge(chan, NULL);
hanguptree(outgoing, NULL);
pbx_builtin_setvar_helper(chan, "DIALSTATUS", status);
if (option_debug)
ast_log(LOG_DEBUG, "Exiting with DIALSTATUS=%s.\n", status);
if ((ast_test_flag(peerflags, OPT_GO_ON)) && (!chan->_softhangup) && (res != AST_PBX_KEEPALIVE)) {
if (calldurationlimit)
chan->whentohangup = 0;
res = 0;
}
}
done:

8
apps/app_queue.c
View File

@@ -3143,7 +3143,7 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
setup_transfer_datastore(qe, member, callstart, callcompletedinsl);
bridge = ast_bridge_call(qe->chan,peer, &bridge_config);
if (!attended_transfer_occurred(qe->chan)) {
if (bridge != AST_PBX_KEEPALIVE && !attended_transfer_occurred(qe->chan)) {
struct ast_datastore *transfer_ds;
if (strcasecmp(oldcontext, qe->chan->context) || strcasecmp(oldexten, qe->chan->exten)) {
ast_queue_log(queuename, qe->chan->uniqueid, member->membername, "TRANSFER", "%s|%s|%ld|%ld",
@@ -3152,7 +3152,7 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
} else if (qe->chan->_softhangup) {
ast_queue_log(queuename, qe->chan->uniqueid, member->membername, "COMPLETECALLER", "%ld|%ld|%d",
(long) (callstart - qe->start), (long) (time(NULL) - callstart), qe->opos);
if (qe->parent->eventwhencalled)
if (bridge != AST_PBX_NO_HANGUP_PEER && bridge != AST_PBX_NO_HANGUP_PEER_PARKED && qe->parent->eventwhencalled)
manager_event(EVENT_FLAG_AGENT, "AgentComplete",
"Queue: %s\r\n"
"Uniqueid: %s\r\n"
@@ -3169,7 +3169,7 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
} else {
ast_queue_log(queuename, qe->chan->uniqueid, member->membername, "COMPLETEAGENT", "%ld|%ld|%d",
(long) (callstart - qe->start), (long) (time(NULL) - callstart), qe->opos);
if (qe->parent->eventwhencalled)
if (bridge != AST_PBX_NO_HANGUP_PEER && bridge != AST_PBX_NO_HANGUP_PEER_PARKED && qe->parent->eventwhencalled)
manager_event(EVENT_FLAG_AGENT, "AgentComplete",
"Queue: %s\r\n"
"Uniqueid: %s\r\n"
@@ -3193,7 +3193,7 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
update_queue(qe->parent, member, callcompletedinsl);
}
if (bridge != AST_PBX_NO_HANGUP_PEER)
if (bridge != AST_PBX_NO_HANGUP_PEER && bridge != AST_PBX_NO_HANGUP_PEER_PARKED)
ast_hangup(peer);
res = bridge ? bridge : 1;
ao2_ref(member, -1);

6
funcs/func_channel.c
View File

@@ -26,15 +26,11 @@
ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <regex.h>
#include "asterisk/module.h"
#include "asterisk/channel.h"
#include "asterisk/pbx.h"
#include "asterisk/logger.h"
#include "asterisk/utils.h"
#include "asterisk/app.h"
#include "asterisk/indications.h"

5
include/asterisk/pbx.h
View File

@@ -37,8 +37,9 @@ extern "C" {
#define AST_PBX_REPLACE 1
/*! \brief Special return values from applications to the PBX { */
#define AST_PBX_KEEPALIVE 10 /*!< Destroy the thread, but don't hang up the channel */
#define AST_PBX_NO_HANGUP_PEER 11
#define AST_PBX_KEEPALIVE 10 /*!< Destroy the thread, but don't hang up the channel */
#define AST_PBX_NO_HANGUP_PEER 11 /*!< The peer has been involved in a transfer */
#define AST_PBX_NO_HANGUP_PEER_PARKED 12 /*!< Don't touch the peer channel - it was sent to the parking lot and might be gone by now */
/*! } */
#define PRIORITY_HINT -1 /*!< Special Priority for a hint */

143
res/res_features.c
View File

@@ -536,14 +536,15 @@ static int masq_park_call_announce(struct ast_channel *rchan, struct ast_channel
return masq_park_call(rchan, peer, timeout, extout, 1);
}
#define FEATURE_RETURN_HANGUP -1
#define FEATURE_RETURN_SUCCESSBREAK 0
#define FEATURE_RETURN_PBX_KEEPALIVE AST_PBX_KEEPALIVE
#define FEATURE_RETURN_NO_HANGUP_PEER AST_PBX_NO_HANGUP_PEER
#define FEATURE_RETURN_PASSDIGITS 21
#define FEATURE_RETURN_STOREDIGITS 22
#define FEATURE_RETURN_SUCCESS 23
#define FEATURE_RETURN_KEEPTRYING 24
#define FEATURE_RETURN_HANGUP -1
#define FEATURE_RETURN_SUCCESSBREAK 0
#define FEATURE_RETURN_PBX_KEEPALIVE AST_PBX_KEEPALIVE
#define FEATURE_RETURN_NO_HANGUP_PEER AST_PBX_NO_HANGUP_PEER
#define FEATURE_RETURN_NO_HANGUP_PEER_PARKED AST_PBX_NO_HANGUP_PEER_PARKED
#define FEATURE_RETURN_PASSDIGITS 21
#define FEATURE_RETURN_STOREDIGITS 22
#define FEATURE_RETURN_SUCCESS 23
#define FEATURE_RETURN_KEEPTRYING 24
#define FEATURE_SENSE_CHAN (1 << 0)
#define FEATURE_SENSE_PEER (1 << 1)
@@ -588,7 +589,7 @@ static int builtin_parkcall(struct ast_channel *chan, struct ast_channel *peer,
res = ast_park_call(parkee, parker, 0, NULL);
if (!res) {
if (sense == FEATURE_SENSE_CHAN) {
res = AST_PBX_NO_HANGUP_PEER;
res = AST_PBX_NO_HANGUP_PEER_PARKED;
} else {
res = AST_PBX_KEEPALIVE;
}
@@ -755,8 +756,7 @@ static int builtin_blindtransfer(struct ast_channel *chan, struct ast_channel *p
/* 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;
return (transferer == peer) ? AST_PBX_KEEPALIVE : AST_PBX_NO_HANGUP_PEER_PARKED;
} else {
ast_log(LOG_WARNING, "Unable to park call %s\n", transferee->name);
}
@@ -1112,14 +1112,15 @@ static int feature_exec_app(struct ast_channel *chan, struct ast_channel *peer,
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))
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 FEATURE_RETURN_NO_HANGUP_PEER;
else
} else
return FEATURE_RETURN_PBX_KEEPALIVE;
}
else if (res == AST_PBX_NO_HANGUP_PEER)
} else if (res == AST_PBX_NO_HANGUP_PEER) {
return FEATURE_RETURN_NO_HANGUP_PEER;
else if (res)
} else if (res == AST_PBX_NO_HANGUP_PEER_PARKED) {
return FEATURE_RETURN_NO_HANGUP_PEER_PARKED;
} else if (res)
return FEATURE_RETURN_SUCCESSBREAK;
return FEATURE_RETURN_SUCCESS; /*! \todo XXX should probably return res */
@@ -1710,11 +1711,8 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast
ast_frfree(f);
}
before_you_go:
new_chan_cdr = pick_unlocked_cdr(chan->cdr); /* the proper chan cdr, if there are forked cdrs */
new_peer_cdr = pick_unlocked_cdr(peer->cdr); /* the proper chan cdr, if there are forked cdrs */
if (!ast_test_flag(&(config->features_caller),AST_FEATURE_NO_H_EXTEN) && ast_exists_extension(chan, chan->context, "h", 1, chan->cid.cid_num)) {
before_you_go:
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)) {
struct ast_cdr *swapper;
char savelastapp[AST_MAX_EXTENSION];
char savelastdata[AST_MAX_EXTENSION];
@@ -1758,41 +1756,59 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast
ast_copy_string(bridge_cdr->lastapp, savelastapp, sizeof(bridge_cdr->lastapp));
ast_copy_string(bridge_cdr->lastdata, savelastdata, sizeof(bridge_cdr->lastdata));
}
/* 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 (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 */
ast_cdr_end(bridge_cdr);
ast_cdr_detach(bridge_cdr);
/* obey the NoCDR() wishes. */
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))
ast_set_flag(new_peer_cdr, AST_CDR_FLAG_POST_DISABLED); /* DISABLED is viral-- it will propagate across a bridge */
if (!new_chan_cdr || (new_chan_cdr && !ast_test_flag(new_chan_cdr, AST_CDR_FLAG_POST_DISABLED))) {
/* do a specialized reset on the beginning channel
CDR's, if they still exist, so as not to mess up
issues in future bridges;
Here are the rules of the game:
1. The chan and peer channel pointers will not change
during the life of the bridge.
2. But, in transfers, the channel names will change.
between the time the bridge is started, and the
time the channel ends.
Usually, when a channel changes names, it will
also change CDR pointers.
3. Usually, only one of the two channels (chan or peer)
will change names.
4. Usually, if a channel changes names during a bridge,
it is because of a transfer. Usually, in these situations,
it is normal to see 2 bridges running simultaneously, and
it is not unusual to see the two channels that change
swapped between bridges.
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) {
struct ast_channel *chan_ptr = NULL;
ast_cdr_end(bridge_cdr);
ast_cdr_detach(bridge_cdr);
/* do a specialized reset on the beginning channel
CDR's, if they still exist, so as not to mess up
issues in future bridges;
Here are the rules of the game:
1. The chan and peer channel pointers will not change
during the life of the bridge.
2. But, in transfers, the channel names will change.
between the time the bridge is started, and the
time the channel ends.
Usually, when a channel changes names, it will
also change CDR pointers.
3. Usually, only one of the two channels (chan or peer)
will change names.
4. Usually, if a channel changes names during a bridge,
it is because of a transfer. Usually, in these situations,
it is normal to see 2 bridges running simultaneously, and
it is not unusual to see the two channels that change
swapped between bridges.
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.
*/
if (strcasecmp(orig_channame, chan->name) != 0) {
/* old channel */
chan_ptr = ast_get_channel_by_name_locked(orig_channame);
@@ -1814,9 +1830,16 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast
} else {
ast_cdr_specialized_reset(chan_cdr,0); /* nothing changed, reset the chan_cdr */
}
chan_ptr = ast_get_channel_by_name_locked(orig_peername);
if (chan_ptr && strcasecmp(orig_peername, peer->name) != 0) {
}
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))
ast_set_flag(new_peer_cdr, AST_CDR_FLAG_POST_DISABLED); /* DISABLED is viral-- it will propagate across a bridge */
if (strcasecmp(orig_peername, peer->name) != 0) {
/* old channel */
chan_ptr = ast_get_channel_by_name_locked(orig_peername);
if (chan_ptr) {
if (!ast_bridged_channel(chan_ptr)) {
struct ast_cdr *cur;
@@ -1828,19 +1851,17 @@ int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast
if (cur)
ast_cdr_specialized_reset(peer_cdr,0);
}
ast_channel_unlock(chan_ptr);
}
/* new channel */
ast_cdr_specialized_reset(new_peer_cdr,0);
} else {
if (chan_ptr)
ast_cdr_specialized_reset(peer_cdr,0); /* nothing changed, reset the peer_cdr */
ast_cdr_specialized_reset(peer_cdr,0); /* nothing changed, reset the peer_cdr */
}
if (chan_ptr)
ast_channel_unlock(chan_ptr);
}
return res;
}
static void post_manager_event(const char *s, char *parkingexten, struct ast_channel *chan)
{
manager_event(EVENT_FLAG_CALL, s,
@@ -2209,7 +2230,7 @@ static int park_exec(struct ast_channel *chan, void *data)
ast_cdr_setdestchan(chan->cdr, peer->name);
/* Simulate the PBX hanging up */
if (res != AST_PBX_NO_HANGUP_PEER)
if (res != AST_PBX_NO_HANGUP_PEER && res != AST_PBX_NO_HANGUP_PEER_PARKED)
ast_hangup(peer);
ast_module_user_remove(u);
return res;