[asterisk-commits] dvossel: branch dvossel/masq_locking_order_trunk r222350 - in /team/dvossel/m...

[SVN commits to the Asterisk project]

Author: dvossel
Date: Tue Oct  6 15:33:15 2009
New Revision: 222350

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=222350
Log:
adds channel ref during sip transfers, adds a function note to channel.h


Modified:
    team/dvossel/masq_locking_order_trunk/channels/chan_sip.c
    team/dvossel/masq_locking_order_trunk/include/asterisk/channel.h
    team/dvossel/masq_locking_order_trunk/main/channel.c

Modified: team/dvossel/masq_locking_order_trunk/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/team/dvossel/masq_locking_order_trunk/channels/chan_sip.c?view=diff&rev=222350&r1=222349&r2=222350
==============================================================================
--- team/dvossel/masq_locking_order_trunk/channels/chan_sip.c (original)
+++ team/dvossel/masq_locking_order_trunk/channels/chan_sip.c Tue Oct  6 15:33:15 2009
@@ -19783,7 +19783,11 @@
 	else
 		ast_debug(4, "Invite/Replaces: Going to masquerade %s into %s\n", c->name, replacecall->name);
 
-	/* C should now be in place of replacecall. all channel locks and pvt locks should be removed before issuing the masq */
+	/* C should now be in place of replacecall. all channel locks and pvt locks should be removed
+	 * before issuing the masq.  Since we are unlocking both the pvt (p) and its owner channel (c)
+	 * it is possible for channel c to be destroyed on us.  To prevent this, we must give c a reference
+	 * before any unlocking takes place and remove it only once we are completely done with it */
+	ast_channel_ref(c);
 	ast_channel_unlock(replacecall);
 	ast_channel_unlock(c);
 	sip_pvt_unlock(p->refer->refer_call);
@@ -19791,7 +19795,6 @@
 	if (ast_do_masquerade(replacecall)) {
 		ast_log(LOG_WARNING, "Failed to perform masquerade with INVITE replaces\n");
 	}
-
 	ast_channel_lock(c);
 	if (earlyreplace || oneleggedreplace ) {
 		c->hangupcause = AST_CAUSE_SWITCH_CONGESTION;
@@ -19804,11 +19807,11 @@
 
 	/* c and c's tech pvt must be unlocked at this point for ast_hangup */
 	ast_hangup(c);
-
 	/* this indicates to handle_request_do that the owner channel has already been unlocked */
 	*nounlock = 1;
 	/* lock PVT structure again after hangup */
 	sip_pvt_lock(p);
+	ast_channel_unref(c);
 	return 0;
 }
 
@@ -21038,10 +21041,15 @@
 		 * can queue connected line updates where they need to go.
 		 *
 		 * before a masquerade, all channel and pvt locks must be unlocked.  Any recursive
-		 * channel locks held before this function invalidates channel container locking order
+		 * channel locks held before this function invalidates channel container locking order.
+		 * Since we are unlocking both the pvt (transferer) and its owner channel (current.chan1)
+		 * it is possible for current.chan1 to be destroyed in the pbx thread.  To prevent this
+		 * we must give c a reference before any unlocking takes place.
 		 */
+
+		ast_channel_ref(current->chan1);
+		ast_channel_unlock(current->chan1); /* current.chan1 is p->owner before the masq, it was locked by socket_read()*/
 		ast_channel_unlock(target.chan1);
-		ast_channel_unlock(current->chan1); /* current.chan1 is p->owner before the masq, it was locked by socket_read()*/
 		*nounlock = 1;  /* we just unlocked the dialog's channel and have no plans of locking it again. */
 		sip_pvt_unlock(targetcall_pvt);
 		sip_pvt_unlock(transferer);
@@ -21061,6 +21069,7 @@
 			 */
 			ast_channel_update_connected_line(target.chan1, &connected_to_target);
 		}
+		ast_channel_unref(current->chan1);
 	}
 
 	/* at this point if the transfer is successful only the transferer pvt should be locked. */

Modified: team/dvossel/masq_locking_order_trunk/include/asterisk/channel.h
URL: http://svnview.digium.com/svn/asterisk/team/dvossel/masq_locking_order_trunk/include/asterisk/channel.h?view=diff&rev=222350&r1=222349&r2=222350
==============================================================================
--- team/dvossel/masq_locking_order_trunk/include/asterisk/channel.h (original)
+++ team/dvossel/masq_locking_order_trunk/include/asterisk/channel.h Tue Oct  6 15:33:15 2009
@@ -1060,6 +1060,11 @@
  * \param newname the name to change to
  *
  * \return nothing
+ *
+ * \note this function must _NEVER_ be used when any channels are locked
+ * regardless if it is the channel who's name is being changed or not because
+ * it invalidates our channel container locking order... lock container first,
+ * then the individual channels, never the other way around.
  */
 void ast_change_name(struct ast_channel *chan, const char *newname);
 

Modified: team/dvossel/masq_locking_order_trunk/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/team/dvossel/masq_locking_order_trunk/main/channel.c?view=diff&rev=222350&r1=222349&r2=222350
==============================================================================
--- team/dvossel/masq_locking_order_trunk/main/channel.c (original)
+++ team/dvossel/masq_locking_order_trunk/main/channel.c Tue Oct  6 15:33:15 2009
@@ -4819,10 +4819,6 @@
 	ast_string_field_set(chan, name, newname);
 }
 
-/*! \note this function must _NEVER_ ever ever be used when any channels
- * are locked regardless if it is the channel who's name is being changed
- * or not because it invalidates our channel container locking order...
- * lock container first, then the channels, never the other way around. */
 void ast_change_name(struct ast_channel *chan, const char *newname)
 {
 	/* We must re-link, as the hash value will change here. */