[asterisk-commits] rmudgett: branch 1.8 r359451 - in /branches/1.8: channels/ include/asterisk/ ...

[SVN commits to the Asterisk project]

Author: rmudgett
Date: Wed Mar 14 17:20:24 2012
New Revision: 359451

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=359451
Log:
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.

Modified:
    branches/1.8/channels/chan_agent.c
    branches/1.8/include/asterisk/channel.h
    branches/1.8/main/channel.c

Modified: branches/1.8/channels/chan_agent.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_agent.c?view=diff&rev=359451&r1=359450&r2=359451
==============================================================================
--- branches/1.8/channels/chan_agent.c (original)
+++ branches/1.8/channels/chan_agent.c Wed Mar 14 17:20:24 2012
@@ -966,9 +966,7 @@
 		p->chan->_bridge = 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);
@@ -977,11 +975,9 @@
 	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);
 	}
 

Modified: branches/1.8/include/asterisk/channel.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/include/asterisk/channel.h?view=diff&rev=359451&r1=359450&r2=359451
==============================================================================
--- branches/1.8/include/asterisk/channel.h (original)
+++ branches/1.8/include/asterisk/channel.h Wed Mar 14 17:20:24 2012
@@ -1631,6 +1631,7 @@
 
 /*!
  * \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
@@ -1640,6 +1641,7 @@
 
 /*!
  * \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

Modified: branches/1.8/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/channel.c?view=diff&rev=359451&r1=359450&r2=359451
==============================================================================
--- branches/1.8/main/channel.c (original)
+++ branches/1.8/main/channel.c Wed Mar 14 17:20:24 2012
@@ -3976,12 +3976,14 @@
 						ast_party_connected_line_free(&connected);
 						break;
 					}
+					ast_channel_unlock(chan);
 					if (ast_channel_connected_line_macro(NULL, chan, &connected, 1, 0)) {
 						ast_indicate_data(chan, AST_CONTROL_CONNECTED_LINE,
 							read_action_payload->payload,
 							read_action_payload->payload_size);
 					}
 					ast_party_connected_line_free(&connected);
+					ast_channel_lock(chan);
 					break;
 				}
 				ast_frfree(f);
@@ -9368,10 +9370,16 @@
 	}
 	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, &macro_chan->connected, NULL);
+		ast_party_connected_line_copy(&saved_connected, &macro_chan->connected);
 		ast_channel_unlock(macro_chan);
+		ast_channel_update_connected_line(macro_chan, &saved_connected, NULL);
+		ast_party_connected_line_free(&saved_connected);
 	}
 
 	return retval;
@@ -9409,9 +9417,14 @@
 
 	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, &macro_chan->redirecting, NULL);
+		ast_party_redirecting_copy(&saved_redirecting, &macro_chan->redirecting);
 		ast_channel_unlock(macro_chan);
+		ast_channel_update_redirecting(macro_chan, &saved_redirecting, NULL);
+		ast_party_redirecting_free(&saved_redirecting);
 	}
 
 	return retval;