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

[schmidts]

> On Nov. 3, 2011, 4:03 a.m., schmidts wrote:
> > i dont get now why are you changing the callid but except this it looks good so far.
> 
> rmudgett wrote:
>     The callid is the hash key for the dialogs and dialogs_rtpcheck container.  If you change the hash key while the object is in the container then you *cannot* use ao2_unlink to remove it.  It will not be found because it is likely to hash to a different bucket.  I changed the dialogs_needdestroy container to have only one bucket so the hash container degenerates to a simple linked list.
>

i know why you unlink it before chaning the callid and link it in again with the new callid. Thanks for your long description.

what i dont understand is why the callid is even changed at all?


> On Nov. 3, 2011, 4:03 a.m., schmidts wrote:
> > /branches/1.8/channels/chan_sip.c, line 16690
> > <https://reviewboard.asterisk.org/r/1557/diff/2/?file=21580#file21580line16690>
> >
> >     i had this in my original patch but i am not sure if we shouldnt let this checks for an active audio or video bridge stay in here.
> >     
> >
> 
> rmudgett wrote:
>     You can highlight more than one line to comment on.  Just drag the mouse over the line numbers you are commenting on.  For chan_sip, this may be slow because of the file size.

sorry looks like i missed the other rows but you figured out what i meant ;)


- schmidts


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


On Nov. 2, 2011, 8:25 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1557/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2011, 8:25 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 343275 
> 
> 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/20111103/37db355c/attachment.htm>