Remove some unnecessary locking from ast_hangup().

This patch removes some unnecessary locking of the channels container in
ast_hangup().  The reason this came up is that this lock can very quickly block
the entire system.  If any of the channel cleanup code decides to block, it
causes a problem for the whole system.  For example, when audiohooks get
destroyed, if that blocks for a while waiting on the mixmonitor thread to exit
because it's busy blocking on some I/O, it causes a problem for many other
threads in the meantime.

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

Merged revisions 354492 from http://svn.asterisk.org/svn/asterisk/branches/1.8


git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/10@354493 65c4cc65-6c06-0410-ace0-fbb531ad65f3
10-digiumphones
Russell Bryant 13 years ago
parent 6fd4756e8a
commit 10e41e37b5

@ -2771,22 +2771,26 @@ void ast_set_hangupsource(struct ast_channel *chan, const char *source, int forc
} }
} }
static void destroy_hooks(struct ast_channel *chan)
{
if (chan->audiohooks) {
ast_audiohook_detach_list(chan->audiohooks);
chan->audiohooks = NULL;
}
ast_framehook_list_destroy(chan);
}
/*! \brief Hangup a channel */ /*! \brief Hangup a channel */
int ast_hangup(struct ast_channel *chan) int ast_hangup(struct ast_channel *chan)
{ {
char extra_str[64]; /* used for cel logging below */ char extra_str[64]; /* used for cel logging below */
int was_zombie;
ast_autoservice_stop(chan); ast_autoservice_stop(chan);
ao2_lock(channels);
ast_channel_lock(chan); ast_channel_lock(chan);
if (chan->audiohooks) {
ast_audiohook_detach_list(chan->audiohooks);
chan->audiohooks = NULL;
}
ast_framehook_list_destroy(chan);
/* /*
* Do the masquerade if someone is setup to masquerade into us. * Do the masquerade if someone is setup to masquerade into us.
* *
@ -2797,16 +2801,13 @@ int ast_hangup(struct ast_channel *chan)
*/ */
while (chan->masq) { 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. */ /* Abort the loop or we might never leave. */
ao2_lock(channels);
ast_channel_lock(chan); ast_channel_lock(chan);
break; break;
} }
ao2_lock(channels);
ast_channel_lock(chan); ast_channel_lock(chan);
} }
@ -2817,13 +2818,20 @@ int ast_hangup(struct ast_channel *chan)
* to free it later. * to free it later.
*/ */
ast_set_flag(chan, AST_FLAG_ZOMBIE); ast_set_flag(chan, AST_FLAG_ZOMBIE);
destroy_hooks(chan);
ast_channel_unlock(chan); ast_channel_unlock(chan);
ao2_unlock(channels);
return 0; return 0;
} }
if (!(was_zombie = ast_test_flag(chan, AST_FLAG_ZOMBIE))) {
ast_set_flag(chan, AST_FLAG_ZOMBIE);
}
ast_channel_unlock(chan);
ao2_unlink(channels, chan); ao2_unlink(channels, chan);
ao2_unlock(channels); ast_channel_lock(chan);
destroy_hooks(chan);
free_translation(chan); free_translation(chan);
/* Close audio stream */ /* Close audio stream */
@ -2858,14 +2866,9 @@ int ast_hangup(struct ast_channel *chan)
(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 (!was_zombie) {
ast_debug(1, "Hanging up channel '%s'\n", chan->name); ast_debug(1, "Hanging up channel '%s'\n", chan->name);
/*
* 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) { if (chan->tech->hangup) {
chan->tech->hangup(chan); chan->tech->hangup(chan);
} }

Loading…
Cancel
Save