[asterisk-dev] [Code Review] chan_sip, kill deadlock avoidance in do_monitor.

[Russell Bryant]

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

Ship it!


After more testing, i'm good with it.


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

    You could use the precondition and postcondition doxygen commands to help describe things here:
    
    \pre pvt is not locked.
    
    \post pvt is locked
    \post pvt->owner is locked and its reference count has been increased (if pvt->owner is non-NULL)



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

    and if it is more than once, it's going to be just 2 times 99.999% of the time (I think it may be 100%, but I'm not positive there isn't some crazy scenario that I'm not thinking of)


- Russell


On 2011-04-14 11:58:51, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1182/
> -----------------------------------------------------------
> 
> (Updated 2011-04-14 11:58:51)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Deadlock avoidance between the sip pvt and the pvt->owner is very difficult.  Now that channel's are ao2 objects, this complication is no longer necessary.  It turns out the pvt's msg queue only exists because of deadlock avoidance (when deadlock avoidance fails msgs were added to a queue to be processed later), so this goes away as well.
> 
> The technique used in the new sip_lock_pvt_full() function should be used as a template for replacing all locations where deadlock avoidance occurs between a channel tech_pvt and the pvt's owner.  My hope is that this will begin a reversal of the invalid channel driver locking architecture we have been using for so long. 
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 313741 
> 
> Diff: https://reviewboard.asterisk.org/r/1182/diff
> 
> 
> Testing
> -------
> 
> I have tested this briefly.  It will be proven under load before being introduced into any branch.
> 
> 
> Thanks,
> 
> David
> 
>

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