Asterisk
/
asterisk
mirror of https://github.com/asterisk/asterisk
1
0
Fork 0
Code Issues Releases Wiki Activity

Merged revisions 334009 via svnmerge from

Browse Source 
https://origsvn.digium.com/svn/asterisk/branches/1.8

........
  r334009 | rmudgett | 2011-08-31 10:20:31 -0500 (Wed, 31 Aug 2011) | 43 lines
  
  Call pickup race leaves orphaned channels or crashes.
  
  Multiple users attempting to pickup a call that has been forked to
  multiple extensions either crashes or fails a masquerade with a "bad
  things may happen" message.
  
  This is the scenario that is causing all the grief:
  1) Pickup target is selected
  2) target is marked as being picked up in ast_do_pickup()
  3) target is unlocked by ast_do_pickup()
  4) app dial or queue gets a chance to hang up losing calls and calls
  ast_hangup() on target
  5) SINCE A MASQUERADE HAS NOT BEEN SETUP YET BY ast_do_pickup() with
  ast_channel_masquerade(), ast_hangup() completes successfully and the
  channel is no longer in the channels container.
  6) ast_do_pickup() then calls ast_channel_masquerade() to schedule the
  masquerade on the dead channel.
  7) ast_do_pickup() then calls ast_do_masquerade() on the dead channel
  8) bad things happen while doing the masquerade and in the process
  ast_do_masquerade() puts the dead channel back into the channels container
  9) The "orphaned" channel is visible in the channels list if a crash does
  not happen.
  
  This patch does the following:
  
  * Made ast_hangup() set AST_FLAG_ZOMBIE on a successfully hung-up channel
  and not release the channel lock until that has happened.
  
  * Made __ast_channel_masquerade() not setup a masquerade if either channel
  has AST_FLAG_ZOMBIE set.
  
  * Fix chan_agent misuse of AST_FLAG_ZOMBIE since it would no longer work.
  
  (closes issue ASTERISK-18222)
  Reported by: Alec Davis
  Tested by: rmudgett, Alec Davis, irroot, Karsten Wemheuer
  
  (closes issue ASTERISK-18273)
  Reported by: Karsten Wemheuer
  Tested by: rmudgett, Alec Davis, irroot, Karsten Wemheuer
  
  Review: https://reviewboard.asterisk.org/r/1400/
........


git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/10@334010 65c4cc65-6c06-0410-ace0-fbb531ad65f3
10-digiumphones
Richard Mudgett 14 years ago
parent 1180e8d803
commit e6de0ecf48
2 changed files with 115 additions and 65 deletions
Show Stats Download Patch File Download Diff File

4
channels/chan_agent.c
Unescape View File

@ -1301,10 +1301,8 @@ static int check_availability(struct agent_pvt *newlyavailable, int needlock)
ast_setstate(parent, AST_STATE_UP); ast_setstate(parent, AST_STATE_UP);
ast_setstate(chan, AST_STATE_UP); ast_setstate(chan, AST_STATE_UP);
ast_copy_string(parent->context, chan->context, sizeof(parent->context)); ast_copy_string(parent->context, chan->context, sizeof(parent->context));
/* Go ahead and mark the channel as a zombie so that masquerade will
destroy it for us, and we need not call ast_hangup */
ast_set_flag(chan, AST_FLAG_ZOMBIE);
ast_channel_masquerade(parent, chan); ast_channel_masquerade(parent, chan);
ast_hangup(chan);
p->abouttograb = 0; p->abouttograb = 0;
} else { } else {
ast_debug(1, "Sneaky, parent disappeared in the mean time...\n"); ast_debug(1, "Sneaky, parent disappeared in the mean time...\n");

176
main/channel.c
Unescape View File

@ -2773,47 +2773,57 @@ void ast_set_hangupsource(struct ast_channel *chan, const char *source, int forc
/*! \brief Hangup a channel */ /*! \brief Hangup a channel */
int ast_hangup(struct ast_channel *chan) int ast_hangup(struct ast_channel *chan)
{ {
int res = 0;
char extra_str[64]; /* used for cel logging below */ char extra_str[64]; /* used for cel logging below */
/* Don't actually hang up a channel that will masquerade as someone else, or ast_autoservice_stop(chan);
if someone is going to masquerade as us */
ao2_lock(channels);
ast_channel_lock(chan); ast_channel_lock(chan);
if (chan->audiohooks) { if (chan->audiohooks) {
ast_audiohook_detach_list(chan->audiohooks); ast_audiohook_detach_list(chan->audiohooks);
chan->audiohooks = NULL; chan->audiohooks = NULL;
} }
ast_framehook_list_destroy(chan); ast_framehook_list_destroy(chan);
ast_autoservice_stop(chan); /*
* Do the masquerade if someone is setup to masquerade into us.
if (chan->masq) { *
* NOTE: We must hold the channel lock after testing for a
* pending masquerade and setting the channel as a zombie to
* prevent __ast_channel_masquerade() from setting up a
* masquerade with a dead channel.
*/
while (chan->masq) {
ast_channel_unlock(chan); ast_channel_unlock(chan);
ao2_unlock(channels);
if (ast_do_masquerade(chan)) { if (ast_do_masquerade(chan)) {
ast_log(LOG_WARNING, "Failed to perform masquerade\n"); ast_log(LOG_WARNING, "Failed to perform masquerade\n");
/* Abort the loop or we might never leave. */
ao2_lock(channels);
ast_channel_lock(chan);
break;
} }
ao2_lock(channels);
ast_channel_lock(chan); ast_channel_lock(chan);
} }
if (chan->masq) {
ast_log(LOG_WARNING, "%s getting hung up, but someone is trying to masq into us?!?\n", chan->name);
ast_channel_unlock(chan);
return 0;
}
/* If this channel is one which will be masqueraded into something,
mark it as a zombie already, so we know to free it later */
if (chan->masqr) { if (chan->masqr) {
/*
* This channel is one which will be masqueraded into something.
* Mark it as a zombie already so ast_do_masquerade() will know
* to free it later.
*/
ast_set_flag(chan, AST_FLAG_ZOMBIE); ast_set_flag(chan, AST_FLAG_ZOMBIE);
ast_channel_unlock(chan); ast_channel_unlock(chan);
ao2_unlock(channels);
return 0; return 0;
} }
ast_channel_unlock(chan);
ao2_unlink(channels, chan); ao2_unlink(channels, chan);
ao2_unlock(channels);
ast_channel_lock(chan);
free_translation(chan); free_translation(chan);
/* Close audio stream */ /* Close audio stream */
if (chan->stream) { if (chan->stream) {
@ -2830,9 +2840,11 @@ int ast_hangup(struct ast_channel *chan)
chan->sched = NULL; chan->sched = NULL;
} }
if (chan->generatordata) /* Clear any tone stuff remaining */ if (chan->generatordata) { /* Clear any tone stuff remaining */
if (chan->generator && chan->generator->release) if (chan->generator && chan->generator->release) {
chan->generator->release(chan, chan->generatordata); chan->generator->release(chan, chan->generatordata);
}
}
chan->generatordata = NULL; chan->generatordata = NULL;
chan->generator = NULL; chan->generator = NULL;
@ -2841,19 +2853,27 @@ int ast_hangup(struct ast_channel *chan)
if (ast_test_flag(chan, AST_FLAG_BLOCKING)) { if (ast_test_flag(chan, AST_FLAG_BLOCKING)) {
ast_log(LOG_WARNING, "Hard hangup called by thread %ld on %s, while fd " ast_log(LOG_WARNING, "Hard hangup called by thread %ld on %s, while fd "
"is blocked by thread %ld in procedure %s! Expect a failure\n", "is blocked by thread %ld in procedure %s! Expect a failure\n",
(long)pthread_self(), chan->name, (long)chan->blocker, chan->blockproc); (long) pthread_self(), chan->name, (long)chan->blocker, chan->blockproc);
ast_assert(ast_test_flag(chan, AST_FLAG_BLOCKING) == 0); ast_assert(ast_test_flag(chan, AST_FLAG_BLOCKING) == 0);
} }
if (!ast_test_flag(chan, AST_FLAG_ZOMBIE)) { if (!ast_test_flag(chan, AST_FLAG_ZOMBIE)) {
ast_debug(1, "Hanging up channel '%s'\n", chan->name); ast_debug(1, "Hanging up channel '%s'\n", chan->name);
if (chan->tech->hangup)
res = chan->tech->hangup(chan); /*
* This channel is now dead so mark it as a zombie so anyone
* left holding a reference to this channel will not use it.
*/
ast_set_flag(chan, AST_FLAG_ZOMBIE);
if (chan->tech->hangup) {
chan->tech->hangup(chan);
}
} else { } else {
ast_debug(1, "Hanging up zombie '%s'\n", chan->name); ast_debug(1, "Hanging up zombie '%s'\n", chan->name);
} }
ast_channel_unlock(chan); ast_channel_unlock(chan);
ast_cc_offer(chan); ast_cc_offer(chan);
ast_manager_event(chan, EVENT_FLAG_CALL, "Hangup", ast_manager_event(chan, EVENT_FLAG_CALL, "Hangup",
"Channel: %s\r\n" "Channel: %s\r\n"
@ -2874,20 +2894,19 @@ int ast_hangup(struct ast_channel *chan)
ast_cause2str(chan->hangupcause) ast_cause2str(chan->hangupcause)
); );
if (chan->cdr && !ast_test_flag(chan->cdr, AST_CDR_FLAG_BRIDGED) && if (chan->cdr && !ast_test_flag(chan->cdr, AST_CDR_FLAG_BRIDGED) &&
!ast_test_flag(chan->cdr, AST_CDR_FLAG_POST_DISABLED) && !ast_test_flag(chan->cdr, AST_CDR_FLAG_POST_DISABLED) &&
(chan->cdr->disposition != AST_CDR_NULL || ast_test_flag(chan->cdr, AST_CDR_FLAG_DIALED))) { (chan->cdr->disposition != AST_CDR_NULL || ast_test_flag(chan->cdr, AST_CDR_FLAG_DIALED))) {
ast_channel_lock(chan); ast_channel_lock(chan);
ast_cdr_end(chan->cdr); ast_cdr_end(chan->cdr);
ast_cdr_detach(chan->cdr); ast_cdr_detach(chan->cdr);
chan->cdr = NULL; chan->cdr = NULL;
ast_channel_unlock(chan); ast_channel_unlock(chan);
} }
chan = ast_channel_release(chan); ast_channel_unref(chan);
return res; return 0;
} }
int ast_raw_answer(struct ast_channel *chan, int cdr_answer) int ast_raw_answer(struct ast_channel *chan, int cdr_answer)
@ -5953,48 +5972,81 @@ static int __ast_channel_masquerade(struct ast_channel *original, struct ast_cha
int res = -1; int res = -1;
struct ast_channel *final_orig, *final_clone, *base; struct ast_channel *final_orig, *final_clone, *base;
retrymasq: for (;;) {
final_orig = original; final_orig = original;
final_clone = clonechan; final_clone = clonechan;
ast_channel_lock(original);
while (ast_channel_trylock(clonechan)) {
ast_channel_unlock(original);
usleep(1);
ast_channel_lock(original);
}
/* each of these channels may be sitting behind a channel proxy (i.e. chan_agent) ast_channel_lock_both(original, clonechan);
and if so, we don't really want to masquerade it, but its proxy */
if (original->_bridge && (original->_bridge != ast_bridged_channel(original)) && (original->_bridge->_bridge != original))
final_orig = original->_bridge;
if (clonechan->_bridge && (clonechan->_bridge != ast_bridged_channel(clonechan)) && (clonechan->_bridge->_bridge != clonechan)) if (ast_test_flag(original, AST_FLAG_ZOMBIE)
final_clone = clonechan->_bridge; || ast_test_flag(clonechan, AST_FLAG_ZOMBIE)) {
/* Zombies! Run! */
if (final_clone->tech->get_base_channel && (base = final_clone->tech->get_base_channel(final_clone))) { ast_log(LOG_WARNING,
final_clone = base; "Can't setup masquerade. One or both channels is dead. (%s <-- %s)\n",
} original->name, clonechan->name);
if ((final_orig != original) || (final_clone != clonechan)) {
/* Lots and lots of deadlock avoidance. The main one we're competing with
* is ast_write(), which locks channels recursively, when working with a
* proxy channel. */
if (ast_channel_trylock(final_orig)) {
ast_channel_unlock(clonechan); ast_channel_unlock(clonechan);
ast_channel_unlock(original); ast_channel_unlock(original);
goto retrymasq; return -1;
}
/*
* Each of these channels may be sitting behind a channel proxy
* (i.e. chan_agent) and if so, we don't really want to
* masquerade it, but its proxy
*/
if (original->_bridge
&& (original->_bridge != ast_bridged_channel(original))
&& (original->_bridge->_bridge != original)) {
final_orig = original->_bridge;
}
if (clonechan->_bridge
&& (clonechan->_bridge != ast_bridged_channel(clonechan))
&& (clonechan->_bridge->_bridge != clonechan)) {
final_clone = clonechan->_bridge;
} }
if (ast_channel_trylock(final_clone)) { if (final_clone->tech->get_base_channel
ast_channel_unlock(final_orig); && (base = final_clone->tech->get_base_channel(final_clone))) {
final_clone = base;
}
if ((final_orig != original) || (final_clone != clonechan)) {
/*
* Lots and lots of deadlock avoidance. The main one we're
* competing with is ast_write(), which locks channels
* recursively, when working with a proxy channel.
*/
if (ast_channel_trylock(final_orig)) {
ast_channel_unlock(clonechan);
ast_channel_unlock(original);
/* Try again */
continue;
}
if (ast_channel_trylock(final_clone)) {
ast_channel_unlock(final_orig);
ast_channel_unlock(clonechan);
ast_channel_unlock(original);
/* Try again */
continue;
}
ast_channel_unlock(clonechan); ast_channel_unlock(clonechan);
ast_channel_unlock(original); ast_channel_unlock(original);
goto retrymasq; original = final_orig;
clonechan = final_clone;
if (ast_test_flag(original, AST_FLAG_ZOMBIE)
|| ast_test_flag(clonechan, AST_FLAG_ZOMBIE)) {
/* Zombies! Run! */
ast_log(LOG_WARNING,
"Can't setup masquerade. One or both channels is dead. (%s <-- %s)\n",
original->name, clonechan->name);
ast_channel_unlock(clonechan);
ast_channel_unlock(original);
return -1;
}
} }
ast_channel_unlock(clonechan); break;
ast_channel_unlock(original);
original = final_orig;
clonechan = final_clone;
} }
if (original == clonechan) { if (original == clonechan) {

Write Preview
Loading…
Cancel
Save