[asterisk-commits] alecdavis: trunk r308946 - in /trunk: ./ channels/chan_sip.c

[SVN commits to the Asterisk project]

Author: alecdavis
Date: Fri Feb 25 12:58:10 2011
New Revision: 308946

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=308946
Log:
Merged revisions 308945 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/branches/1.8

........
  r308945 | alecdavis | 2011-02-26 07:52:53 +1300 (Sat, 26 Feb 2011) | 21 lines
  
  Fix Deadlock with attended transfer of SIP call
  
  Call path 
    sip_set_rtp_peer (locks chan then pvt)
     transmit_reinvite_with_sdp
      try_suggested_sip_codec
       pbx_builtin_getvar_helper (locks p->owner)
  
  But by the time p->owner lock was attempted, seems as though chan and p->owner were different.
  
  So in sip_set_rtp_peer, lock pvt first then lock p->owner using deadlocking methods.
  
  (closes issue #18837)
  Reported by: alecdavis
  Patches: 
        bug18837-trunk.diff3.txt uploaded by alecdavis (license 585)
  Tested by: alecdavis, Irontec, ZX81, cmaj
  
  Review: [https://reviewboard.asterisk.org/r/1126/]
........

Modified:
    trunk/   (props changed)
    trunk/channels/chan_sip.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.

Modified: trunk/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_sip.c?view=diff&rev=308946&r1=308945&r2=308946
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Fri Feb 25 12:58:10 2011
@@ -27966,7 +27966,20 @@
 	if (!p) {
 		return -1;
 	}
+	/*
+	 * Lock both the pvt and it's owner safely.
+	 */
 	sip_pvt_lock(p);
+	while (p->owner && ast_channel_trylock(p->owner)) {
+		sip_pvt_unlock(p);
+		usleep(1);
+		sip_pvt_lock(p);
+	}
+
+	if (!p->owner) {
+		sip_pvt_unlock(p);
+		return 0;
+	}
 	if (udptl) {
 		ast_udptl_get_peer(udptl, &p->udptlredirip);
 	} else {
@@ -27985,6 +27998,7 @@
 	}
 	/* Reset lastrtprx timer */
 	p->lastrtprx = p->lastrtptx = time(NULL);
+	ast_channel_unlock(p->owner);
 	sip_pvt_unlock(p);
 	return 0;
 }
@@ -28102,12 +28116,25 @@
 		return 0;
 	}
 
-	ast_channel_lock(chan);
+	/*
+	 * Lock both the pvt and it's owner safely.
+	 */
 	sip_pvt_lock(p);
+	while (p->owner && ast_channel_trylock(p->owner)) {
+		sip_pvt_unlock(p);
+		usleep(1);
+		sip_pvt_lock(p);
+	}
+
+	if (!p->owner) {
+		sip_pvt_unlock(p);
+		return 0;
+	}
+
 	if (p->alreadygone) {
 		/* If we're destroyed, don't bother */
+		ast_channel_unlock(p->owner);
 		sip_pvt_unlock(p);
-		ast_channel_unlock(chan);
 		return 0;
 	}
 
@@ -28115,8 +28142,8 @@
 	   that are known to be behind a NAT, then stop the process now
 	*/
 	if (nat_active && !ast_test_flag(&p->flags[0], SIP_DIRECT_MEDIA_NAT)) {
+		ast_channel_unlock(p->owner);
 		sip_pvt_unlock(p);
-		ast_channel_unlock(chan);
 		return 0;
 	}
 
@@ -28158,8 +28185,8 @@
 	}
 	/* Reset lastrtprx timer */
 	p->lastrtprx = p->lastrtptx = time(NULL);
+	ast_channel_unlock(p->owner);
 	sip_pvt_unlock(p);
-	ast_channel_unlock(chan);
 	return 0;
 }