Summary: | ASTERISK-20708: Deadlock in chan_sip on transfer when trying to update redirecting information | ||
Reporter: | Mark Michelson (mmichelson) | Labels: | |
Date Opened: | 2012-11-20 17:24:50.000-0600 | Date Closed: | 2012-12-11 18:04:45.000-0600 |
Priority: | Major | Regression? | No |
Status: | Closed/Complete | Components: | Channels/chan_sip/Transfers |
Versions: | 11.0.1 | Frequency of Occurrence | Occasional |
Related Issues: | |||
Environment: | Attachments: | ( 0) ASTERISK-20708.patch ( 1) ASTERISK-20708-2.patch ( 2) ASTERISK-20708-3.patch | |
Description: | Issue reported by marv (Tim Ringenbach) in #asterisk-dev
There is a code path that results in an incorrect locking order between a sip_pvt and its owner channel. The problem is that the owner and pvt get unlocked, then the pvt gets relocked and calls change_redirecting_information(), which calls get_rdnis(), which then calls pbx_builtin_setvar_helper() on the pvt's owner. A patch is attached that moves where the redirecting updates occur to a place where both the pvt and owner are unlocked so that we can safely lock them in the correct order. | ||
Comments: | By: Tim Ringenbach at Asteria Solutions Group (tim_ringenbach) 2012-11-21 11:36:07.790-0600 I found a problem with the patch. The last sip_pvt_unlock(p) on the original line 25881 needs to be there. Otherwise, we first lock current.chan1, violating the locking order, but then we go on to lock p again, and return p double locked instead of just locked. By: Tim Ringenbach at Asteria Solutions Group (tim_ringenbach) 2012-11-21 11:57:58.499-0600 I think this is correct, but I'm still in the process of testing it. By: Mark Michelson (mmichelson) 2012-11-26 09:05:26.782-0600 Good catch. The sip_pvt_unlock() location you added is not quite correct though. If current.chan2 is NULL, then we'll end up returning with p unlocked instead of locked. I believe the proper place to put the sip_pvt_unlock() is just below where the SIP_DEFER_BYE_ON_TRANSFER flag is set. By: Mark Michelson (mmichelson) 2012-11-26 09:06:45.195-0600 I've uploaded ASTERISK-20708-3.patch. That should do the trick. By: Tim Ringenbach at Asteria Solutions Group (tim_ringenbach) 2012-11-29 15:16:02.414-0600 That patch seems good. I put it on a customer's box who was running into the deadlock, and asterisk has been stable ever since. |