diff options
author | jpeeler <jpeeler@f38db490-d61c-443f-a65b-d21fe96a405b> | 2011-01-14 17:32:52 +0000 |
---|---|---|
committer | jpeeler <jpeeler@f38db490-d61c-443f-a65b-d21fe96a405b> | 2011-01-14 17:32:52 +0000 |
commit | 9968bd1681491d840d1a2831c63a511168a4858f (patch) | |
tree | fd271293edf6ac3f50bd1adae9e59573aa8e3451 /channels | |
parent | f7fb23b4e27acd709a05ec1bde8746c3c7e0f1bc (diff) |
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: http://svn.digium.com/svn/asterisk/branches/1.8@301790 f38db490-d61c-443f-a65b-d21fe96a405b
Diffstat (limited to 'channels')
-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 6b047493e..058e602b9 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(¤t.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 */ |