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

diff --git a/channels/chan_agent.c b/channels/chan_agent.c
index 0c79a66..0193839 100644 (file)
--- 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 292640b..8e438bb 100644 (file)
--- 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 35155bb..f97855f 100644 (file)
--- 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;