diff options
author | Russell Bryant <russell@russellbryant.com> | 2012-02-09 02:28:18 +0000 |
---|---|---|
committer | Russell Bryant <russell@russellbryant.com> | 2012-02-09 02:28:18 +0000 |
commit | 747cd61edf54868689dac7207e09cf9c3011715b (patch) | |
tree | a45fa413dc954d9f1a82f7543002f4451bd81642 /main | |
parent | 2ce189c5b87c82c1f985f8b62d01621ac477ea1d (diff) |
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
........
Merged revisions 354493 from http://svn.asterisk.org/svn/asterisk/branches/10
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@354494 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Diffstat (limited to 'main')
-rw-r--r-- | main/channel.c | 39 |
1 files changed, 21 insertions, 18 deletions
diff --git a/main/channel.c b/main/channel.c index d00dc84c9..a37f33ac2 100644 --- a/main/channel.c +++ b/main/channel.c @@ -2570,22 +2570,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 */ int ast_hangup(struct ast_channel *chan) { char extra_str[64]; /* used for cel logging below */ + int was_zombie; ast_autoservice_stop(chan); - ao2_lock(channels); 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. * @@ -2596,16 +2600,13 @@ int ast_hangup(struct ast_channel *chan) */ while (chan->masq) { ast_channel_unlock(chan); - ao2_unlock(channels); if (ast_do_masquerade(chan)) { 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); } @@ -2616,13 +2617,20 @@ int ast_hangup(struct ast_channel *chan) * to free it later. */ ast_set_flag(chan, AST_FLAG_ZOMBIE); + destroy_hooks(chan); ast_channel_unlock(chan); - ao2_unlock(channels); 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_unlock(channels); + ast_channel_lock(chan); + + destroy_hooks(chan); free_translation(chan); /* Close audio stream */ @@ -2657,14 +2665,9 @@ int ast_hangup(struct ast_channel *chan) (long) pthread_self(), ast_channel_name(chan), (long)chan->blocker, chan->blockproc); 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", ast_channel_name(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); } |