[asterisk-commits] mmichelson: trunk r199588 - /trunk/channels/chan_sip.c

[SVN commits to the Asterisk project]

Author: mmichelson
Date: Mon Jun  8 12:32:04 2009
New Revision: 199588

URL: http://svn.asterisk.org/svn-view/asterisk?view=rev&rev=199588
Log:
Fix a deadlock that could occur when setting rtp stats on SIP calls.

(closes issue #15143)
Reported by: cristiandimache
Patches:
      15143.patch uploaded by mmichelson (license 60)
Tested by: cristiandimache


Modified:
    trunk/channels/chan_sip.c

Modified: trunk/channels/chan_sip.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/channels/chan_sip.c?view=diff&rev=199588&r1=199587&r2=199588
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Mon Jun  8 12:32:04 2009
@@ -5933,6 +5933,17 @@
 				struct ast_channel *bridge = ast_bridged_channel(oldowner);
 				char quality_buf[AST_MAX_USER_FIELD], *quality;
 
+				/* We need to get the lock on bridge because ast_rtp_instance_set_stats_vars will attempt
+				 * to lock the bridge. This may get hairy...
+				 */
+				while (bridge && ast_channel_trylock(bridge)) {
+					sip_pvt_unlock(p);
+					do {
+						CHANNEL_DEADLOCK_AVOIDANCE(oldowner);
+					} while (sip_pvt_trylock(p));
+					bridge = ast_bridged_channel(oldowner);
+				}
+
 				if (p->rtp) {
 					ast_rtp_instance_set_stats_vars(oldowner, p->rtp);
 				}
@@ -5943,6 +5954,7 @@
 					if (IS_SIP_TECH(bridge->tech) && q && q->rtp) {
 						ast_rtp_instance_set_stats_vars(bridge, q->rtp);
 					}
+					ast_channel_unlock(bridge);
 				}
 
 				if (p->do_history || oldowner) {
@@ -21061,7 +21073,22 @@
 	/* Get RTCP quality before end of call */
 	if (p->do_history || p->owner) {
 		char quality_buf[AST_MAX_USER_FIELD], *quality;
-		struct ast_channel *bridge = p->owner ? ast_bridged_channel(p->owner) : NULL;
+		struct ast_channel *bridge = p->owner ? ast_bridged_channel(p->owner) : NULL;  
+
+		/* We need to get the lock on bridge because ast_rtp_instance_set_stats_vars will attempt
+		 * to lock the bridge. This may get hairy...
+		 */
+		while (bridge && ast_channel_trylock(bridge)) {
+			ast_channel_unlock(p->owner);
+			do {
+				/* Can't use DEADLOCK_AVOIDANCE since p is an ao2 object */
+				sip_pvt_unlock(p);
+				usleep(1);
+				sip_pvt_lock(p);
+			} while (p->owner && ast_channel_trylock(p->owner));
+			bridge = p->owner ? ast_bridged_channel(p->owner) : NULL;
+		}
+
 
 		if (p->rtp && (quality = ast_rtp_instance_get_quality(p->rtp, AST_RTP_INSTANCE_STAT_FIELD_QUALITY, quality_buf, sizeof(quality_buf)))) {
 			if (p->do_history) {
@@ -21090,6 +21117,7 @@
 			if (IS_SIP_TECH(bridge->tech) && q && q->rtp) {
 				ast_rtp_instance_set_stats_vars(bridge, q->rtp);
 			}
+			ast_channel_unlock(bridge);
 		}
 
 		if (p->vrtp && (quality = ast_rtp_instance_get_quality(p->vrtp, AST_RTP_INSTANCE_STAT_FIELD_QUALITY, quality_buf, sizeof(quality_buf)))) {