diff options
author | Jeff Peeler <jpeeler@digium.com> | 2011-01-14 17:34:28 +0000 |
---|---|---|
committer | Jeff Peeler <jpeeler@digium.com> | 2011-01-14 17:34:28 +0000 |
commit | a0e4c4ee5ba8a65137c05c0239d04b0de0f6e4d3 (patch) | |
tree | ad863b1dc956a45b14a0bd46d969f46a0ba380be | |
parent | 89fe21382abf0c3e6a878ad667089e11d9df454e (diff) |
Merged revisions 301790 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8
........
r301790 | jpeeler | 2011-01-14 11:32:52 -0600 (Fri, 14 Jan 2011) | 42 lines
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/trunk@301791 65c4cc65-6c06-0410-ace0-fbb531ad65f3
-rw-r--r-- | channels/chan_sip.c | 28 |
1 files changed, 16 insertions, 12 deletions
diff --git a/channels/chan_sip.c b/channels/chan_sip.c index f57479912..84dd41535 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -22353,11 +22353,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(¤t.req, req); ast_clear_flag(&p->flags[0], SIP_GOTREFER); p->refer->status = REFER_200OK; @@ -22383,6 +22392,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; } @@ -22391,6 +22401,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); @@ -22412,15 +22425,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 */ |