From: Jeff Peeler <jpeeler@digium.com>
Date: Fri, 14 Jan 2011 17:32:52 +0000 (+0000)
Subject: Resolve deadlock involving REFER.
X-Git-Tag: 1.8.3-rc1~8
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=91908544174d6843e8ee63859a4f4e172176a26a;p=thirdparty%2Fasterisk.git

Resolve deadlock involving REFER.

Two fixes:
1) One must always have the private unlocked before calling
pbx_builtin_setvar_helper to not invalidate locking order since it locks the
channel.
2) Unlock the channel before calling pbx_find_extension, which starts and stops
autoservice during the lookup. The problem scenario as illustrated by the
reporter:

Thread: do_monitor
-----------------------
handle_request_do
 handle_incoming
  handle_request_refer
   ast_parking_ext_valid
    pbx_find_extension
     ast_autoservice_stop
      while (chan_list_state == as_chan_list_state) { usleep(1000); }

Thread: autoservice_run
-----------------------
autoservice_run
 chan = ast_waitfor_n
  ast_waitfor_nandfds
   ast_waitfor_nandfds_classic / simple / complex (depending on your system)
    ast_channel_lock(c[x]);

handle_request_do and schedule_process_request_queue locks the owner
if it exists. The autoservice thread is waiting for the channel lock, which
wasn't ever released since the do_monitor thread was waiting for autoservice
operations to complete. Solved by unlocking the channel but keeping a reference
to guarantee safety.

(closes issue #18403)
Reported by: jthurman
Patches:
      20110103-blind_deadlock.diff uploaded by jthurman (license 614)
      issue18403.patch uploaded by jpeeler (license 325)
Tested by: jthurman



git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@301790 65c4cc65-6c06-0410-ace0-fbb531ad65f3
---

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 6b047493e8..058e602b90 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -22159,11 +22159,20 @@ static int handle_request_refer(struct sip_pvt *p, struct sip_request *req, int
 		/* Fallthrough if we can't find the call leg internally */
 	}
 
+	/* Must release lock now, because it will not longer
+	   be accessible after the transfer! */
+	*nounlock = 1;
+	/*
+	 * Increase ref count so that we can delay channel destruction until after
+	 * we get a chance to fire off some events.
+	 */
+	ast_channel_ref(current.chan1);
+	sip_pvt_unlock(p);
+	ast_channel_unlock(current.chan1);
+
 	/* Parking a call */
 	if (p->refer->localtransfer && ast_parking_ext_valid(p->refer->refer_to, p->owner, p->owner->context)) {
-		/* Must release c's lock now, because it will not longer be accessible after the transfer! */
-		*nounlock = 1;
-		ast_channel_unlock(current.chan1);
+		sip_pvt_lock(p);
 		copy_request(&current.req, req);
 		ast_clear_flag(&p->flags[0], SIP_GOTREFER);
 		p->refer->status = REFER_200OK;
@@ -22189,6 +22198,7 @@ static int handle_request_refer(struct sip_pvt *p, struct sip_request *req, int
 		if (sip_park(current.chan2, current.chan1, req, seqno, p->refer->refer_to)) {
 			transmit_notify_with_sipfrag(p, seqno, "500 Internal Server Error", TRUE);
 		}
+		ast_channel_unref(current.chan1);
 		return res;
 	}
 
@@ -22197,6 +22207,9 @@ static int handle_request_refer(struct sip_pvt *p, struct sip_request *req, int
 		ast_debug(3, "chan1->name: %s\n", current.chan1->name);
 		pbx_builtin_setvar_helper(current.chan1, "BLINDTRANSFER", current.chan2->name);
 	}
+
+	sip_pvt_lock(p);
+
 	if (current.chan2) {
 		pbx_builtin_setvar_helper(current.chan2, "BLINDTRANSFER", current.chan1->name);
 		pbx_builtin_setvar_helper(current.chan2, "SIPDOMAIN", p->refer->refer_to_domain);
@@ -22218,15 +22231,6 @@ static int handle_request_refer(struct sip_pvt *p, struct sip_request *req, int
 		if (current.chan2)
 			pbx_builtin_setvar_helper(current.chan2, "_SIPTRANSFER_REPLACES", tempheader);
 	}
-	/* Must release lock now, because it will not longer
-	   be accessible after the transfer! */
-	*nounlock = 1;
-	/*
-	 * Increase ref count so that we can delay channel destruction until after
-	 * we get a chance to fire off some events.
-	 */
-	ast_channel_ref(current.chan1);
-	ast_channel_unlock(current.chan1);
 
 	/* Connect the call */