[asterisk-dev] [Code Review] Fix deadlock between subscription event RWLOCK and dialogs container lock in chan_sip.

[David Vossel]

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


To avoid the alloc and free during each iteration if the do_monitor loop, we could create a container just used to store the items marked for destruction, then do a callback on that other container to unlink the objects from both containers.  You could pass this container and the &t pointer in to the dialog_need_destroy callback as the arg in some new structure you create.

This is essentially the exact same thing you've done here, except we don't have to allocate the new container every iteration, which is what is happens behind the scenes when the iterator is created.


/branches/1.8/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1557/#comment8837>

    Before, when OBJ_NOTDATA was used in conjunction with OBJ_MULITPLE, no internal interator was alloced.  With this change we now have an internal ao2_container alloced every do_monitor loop iteration whether items are being destroyed or not.


- David


On Nov. 1, 2011, 5:13 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1557/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2011, 5:13 p.m.)
> 
> 
> Review request for Asterisk Developers and David Vossel.
> 
> 
> Summary
> -------
> 
> Timing between dialog destruction and a MWI event sending a message could result in a deadlock.
> 
> Order of events causing deadlock:
> 
> 1a) The event subscription system calls the registered callbacks with its list RWLOCK held.
> 1b) The SIP monitor checks for dialogs needing destruction.  It does an ao2_callback that holds the dialogs container lock while searching for dialogs to destroy.
> 2a) The event subscription SIP callback needs to create a temporary dialog to send out the MWI notification.  That temporary dialog needs to be inserted in the dialogs container so it must wait.
> 2b) The dialog search finds a dialog to destroy and as a result releases the last reference for a peer.  The peer destructor attempts to get the subscription RWLOCK but must wait.
> 3) deadlock
> 
> 
> This addresses bug ASTERISK-18747.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18747
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 342973 
> 
> Diff: https://reviewboard.asterisk.org/r/1557/diff
> 
> 
> Testing
> -------
> 
> It compiles. :)
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111101/bba1d8e9/attachment-0001.htm>