[asterisk-dev] [Code Review] fix Deadlock with attended transfers of SIP calls

[Matthew Nicholson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1126/#review3216
-----------------------------------------------------------



trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1126/#comment6696>

    The original patch is mostly the right fix.  The problem with the original patch is that pbx_builtin_getvar_helper() gets called on p->owner and p->owner is not always 'chan' or is changed from 'chan' some time before we lock p.  The proper fix would be locking p then locking p->owner with deadlock protection.



trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1126/#comment6695>

    transmit_reinvite_with_sdp() needs to be called with the sip pvt locked.


- Matthew


On 2011-02-24 03:42:16, Alec Davis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1126/
> -----------------------------------------------------------
> 
> (Updated 2011-02-24 03:42:16)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> in 1.8.2.3 and prior to trunk r306216, when the REFER transfer is complete then a deadlock occurs.
> 
> Reason being sip_set_rtp_peer locks pvt, then pbx_builtin_getvar_helper tries to lock chan
> Violating locking order.
>  
> === Thread ID: -1292625008 (do_monitor started at [24470] chan_sip.c restart_monitor())
> === ---> Lock #0 (chan_sip.c): MUTEX 23964 handle_request_do &netlock 0xb6796e80 (1)
> === ---> Lock #1 (channel.c): MUTEX 6211 ast_do_masquerade channels 0x8d4e0c8 (1)
> === ---> Lock #2 (channel.c): MUTEX 6214 ast_do_masquerade original 0xbd98f48 (1)
> === ---> Lock #3 (channel.c): MUTEX 6234 ast_do_masquerade clonechan 0xb24bf7d0 (1)
> === ---> Waiting for Lock #4 (chan_sip.c): MUTEX 6164 sip_fixup p 0xb24bab10 (1)
> === --- ---> Locked Here: chan_sip.c line 27632 (sip_set_rtp_peer)
> 
> === -------------------------------------------------------------------
> ===
> === Thread ID: -1315861616 (pbx_thread started at [ 5035] pbx.c ast_pbx_start())
> === ---> Lock #0 (chan_sip.c): MUTEX 27632 sip_set_rtp_peer p 0xb24bab10 (1)
> === ---> Waiting for Lock #1 (pbx.c): MUTEX 9467 pbx_builtin_getvar_helper chan 0xb24bf7d0 (1)
> === --- ---> Locked Here: channel.c line 6234 (ast_do_masquerade)
> 
> multiple other bug reports reveal the sip_set_rtp_peer pvt locking issue.
> 
> Solution:
>   inc pvt refcount to prevent the possibility of it disappearing while unlocked.
>   unlock the pvt
>     call transmit_reinvite_with_sdp (which finishes up invoking pbx_builtin_getvar_helper)
>   lock the pvt
>   dec pvt refcount
> 
> Applies to 1.8.2.3 and trunk. Both verified deadlocks.
> Presumably 1.8 branch
> 
> r306216 tries to solve it, but causes more channel locking issues 
> see https://issues.asterisk.org/view.php?id=18837#132337
> 
> 
> This addresses bugs 18468, 18491, 18734, and 18837.
>     https://issues.asterisk.org/view.php?id=18468
>     https://issues.asterisk.org/view.php?id=18491
>     https://issues.asterisk.org/view.php?id=18734
>     https://issues.asterisk.org/view.php?id=18837
> 
> 
> Diffs
> -----
> 
>   trunk/channels/chan_sip.c 308720 
> 
> Diff: https://reviewboard.asterisk.org/r/1126/diff
> 
> 
> Testing
> -------
> 
> Testing: trunk
> 
> 3 SIP phones.
> 
> A calls B, and B answers on line 1.
> B puts A on hold by selecting line2.
> B calls C, and C answers.
> B initiates transfer of line1 to line2, phone uses REFER.
> 
> 
> Thanks,
> 
> Alec
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110224/8141a2ef/attachment.htm>