From 9b31bd3cd83b50e0843f42b70d5e111287fa8107 Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Wed, 14 Mar 2012 22:38:29 +0000 Subject: Fix deadlock potential with some ast_indicate/ast_indicate_data calls. Calling ast_indicate()/ast_indicate_data() with the channel lock held can result in a deadlock with a local channel because of how local channels need to avoid deadlock. ........ Merged revisions 359451 from http://svn.asterisk.org/svn/asterisk/branches/1.8 ........ Merged revisions 359453 from http://svn.asterisk.org/svn/asterisk/branches/10 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@359455 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- channels/chan_agent.c | 4 ---- include/asterisk/channel.h | 2 ++ main/channel.c | 36 ++++++++++++++++++++++++++++++------ 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/channels/chan_agent.c b/channels/chan_agent.c index 0c79a66de..01938396d 100644 --- a/channels/chan_agent.c +++ b/channels/chan_agent.c @@ -967,9 +967,7 @@ static int agent_hangup(struct ast_channel *ast) ast_channel_internal_bridged_channel_set(p->chan, NULL); /* If they're dead, go ahead and hang up on the agent now */ if (p->dead) { - ast_channel_lock(p->chan); ast_softhangup(p->chan, AST_SOFTHANGUP_EXPLICIT); - ast_channel_unlock(p->chan); } else if (p->loginstart) { indicate_chan = ast_channel_ref(p->chan); tmp_moh = ast_strdupa(p->moh); @@ -978,11 +976,9 @@ static int agent_hangup(struct ast_channel *ast) ast_mutex_unlock(&p->lock); if (indicate_chan) { - ast_channel_lock(indicate_chan); ast_indicate_data(indicate_chan, AST_CONTROL_HOLD, S_OR(tmp_moh, NULL), !ast_strlen_zero(tmp_moh) ? strlen(tmp_moh) + 1 : 0); - ast_channel_unlock(indicate_chan); indicate_chan = ast_channel_unref(indicate_chan); } diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h index 292640bb4..8e438bbd5 100644 --- a/include/asterisk/channel.h +++ b/include/asterisk/channel.h @@ -1525,6 +1525,7 @@ int ast_call(struct ast_channel *chan, const char *addr, int timeout); /*! * \brief Indicates condition of channel + * \note Absolutely _NO_ channel locks should be held before calling this function. * \note Indicate a condition such as AST_CONTROL_BUSY, AST_CONTROL_RINGING, or AST_CONTROL_CONGESTION on a channel * \param chan channel to change the indication * \param condition which condition to indicate on the channel @@ -1534,6 +1535,7 @@ int ast_indicate(struct ast_channel *chan, int condition); /*! * \brief Indicates condition of channel, with payload + * \note Absolutely _NO_ channel locks should be held before calling this function. * \note Indicate a condition such as AST_CONTROL_HOLD with payload being music on hold class * \param chan channel to change the indication * \param condition which condition to indicate on the channel diff --git a/main/channel.c b/main/channel.c index 35155bb6b..f97855fb6 100644 --- a/main/channel.c +++ b/main/channel.c @@ -3765,6 +3765,7 @@ static struct ast_frame *__ast_read(struct ast_channel *chan, int dropaudio) ast_party_connected_line_free(&connected); break; } + ast_channel_unlock(chan); if (ast_channel_connected_line_sub(NULL, chan, &connected, 0) && ast_channel_connected_line_macro(NULL, chan, &connected, 1, 0)) { ast_indicate_data(chan, AST_CONTROL_CONNECTED_LINE, @@ -3772,6 +3773,7 @@ static struct ast_frame *__ast_read(struct ast_channel *chan, int dropaudio) read_action_payload->payload_size); } ast_party_connected_line_free(&connected); + ast_channel_lock(chan); break; } ast_frfree(f); @@ -9361,10 +9363,16 @@ int ast_channel_connected_line_macro(struct ast_channel *autoservice_chan, struc } ast_channel_unlock(macro_chan); - if (!(retval = ast_app_run_macro(autoservice_chan, macro_chan, macro, macro_args))) { + retval = ast_app_run_macro(autoservice_chan, macro_chan, macro, macro_args); + if (!retval) { + struct ast_party_connected_line saved_connected; + + ast_party_connected_line_init(&saved_connected); ast_channel_lock(macro_chan); - ast_channel_update_connected_line(macro_chan, ast_channel_connected(macro_chan), NULL); + ast_party_connected_line_copy(&saved_connected, ast_channel_connected(macro_chan)); ast_channel_unlock(macro_chan); + ast_channel_update_connected_line(macro_chan, &saved_connected, NULL); + ast_party_connected_line_free(&saved_connected); } return retval; @@ -9407,9 +9415,14 @@ int ast_channel_redirecting_macro(struct ast_channel *autoservice_chan, struct a retval = ast_app_run_macro(autoservice_chan, macro_chan, macro, macro_args); if (!retval) { + struct ast_party_redirecting saved_redirecting; + + ast_party_redirecting_init(&saved_redirecting); ast_channel_lock(macro_chan); - ast_channel_update_redirecting(macro_chan, ast_channel_redirecting(macro_chan), NULL); + ast_party_redirecting_copy(&saved_redirecting, ast_channel_redirecting(macro_chan)); ast_channel_unlock(macro_chan); + ast_channel_update_redirecting(macro_chan, &saved_redirecting, NULL); + ast_party_redirecting_free(&saved_redirecting); } return retval; @@ -9443,10 +9456,16 @@ int ast_channel_connected_line_sub(struct ast_channel *autoservice_chan, struct } ast_channel_unlock(sub_chan); - if (!(retval = ast_app_run_sub(autoservice_chan, sub_chan, sub, sub_args))) { + retval = ast_app_run_sub(autoservice_chan, sub_chan, sub, sub_args); + if (!retval) { + struct ast_party_connected_line saved_connected; + + ast_party_connected_line_init(&saved_connected); ast_channel_lock(sub_chan); - ast_channel_update_connected_line(sub_chan, ast_channel_connected(sub_chan), NULL); + ast_party_connected_line_copy(&saved_connected, ast_channel_connected(sub_chan)); ast_channel_unlock(sub_chan); + ast_channel_update_connected_line(sub_chan, &saved_connected, NULL); + ast_party_connected_line_free(&saved_connected); } return retval; @@ -9482,9 +9501,14 @@ int ast_channel_redirecting_sub(struct ast_channel *autoservice_chan, struct ast retval = ast_app_run_sub(autoservice_chan, sub_chan, sub, sub_args); if (!retval) { + struct ast_party_redirecting saved_redirecting; + + ast_party_redirecting_init(&saved_redirecting); ast_channel_lock(sub_chan); - ast_channel_update_redirecting(sub_chan, ast_channel_redirecting(sub_chan), NULL); + ast_party_redirecting_copy(&saved_redirecting, ast_channel_redirecting(sub_chan)); ast_channel_unlock(sub_chan); + ast_channel_update_redirecting(sub_chan, &saved_redirecting, NULL); + ast_party_redirecting_free(&saved_redirecting); } return retval; -- cgit v1.2.3