[asterisk-dev] [Code Review] Convert device state callbacks to ao2 objects to fix a deadlock.

[schmidts]

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


i have tried to write this type of patch but i didnt know i could use container for the linked callbacks for a hint. nice idea ;)

if i have some time this week i will try this patch against trunk and look if this could hold the performance or if we would loose something out of this but i think it shouldnt change anything.




/branches/1.6.2/main/pbx.c
<https://reviewboard.asterisk.org/r/1072/#comment6340>

    sorry if this is a stupid question but why do you set the refcounter for this container one down?
    
    you dont use the same container to iterate through so IMHO you dont need to reset the refcounter.
    
    the iterator is reinitialised below this row for the callback container of this hint so i dont think you need this.
    
    be honest if i am wrong ;)



/branches/1.6.2/main/pbx.c
<https://reviewboard.asterisk.org/r/1072/#comment6339>

    IMHO you dont need to allocate this container with 563 buckets, cause you will in fact only need a link list. 
    The normal callback container should have buckets, even if they are useless cause all general callbacks have id=0. 
    maybe you could change this cause with this patch general and linked callbacks are split up. 
    the id=0 was only used to find these general callbacks. 
    but again the general callbacks and also the linked only need a link list ;)


nice patch!

best regards.



- schmidts


On 2011-01-07 15:06:45, Jeff Peeler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1072/
> -----------------------------------------------------------
> 
> (Updated 2011-01-07 15:06:45)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Lock scenario:
> Thread 1
>  holds ast_rdlock_contexts &conlock
>  holds handle_statechange hints
>  holds handle_statechange hint
>   waiting for cb_extensionstate
>    Locked Here: chan_sip.c line 7428 (find_call)
> Thread 2
>  holds handle_request_do &netlock
>  holds find_call sip_pvt_ptr
>   waiting for ast_rdlock_contexts &conlock
>    Locked Here: pbx.c line 9911 (ast_rdlock_contexts)
> 
> Chan_sip has an established locking order of locking the sip_pvt and then getting the context lock. So the as stated by the summary, the operations in thread 2 have been modified to no longer require the context lock.
> 
> 
> This addresses bug 18310.
>     https://issues.asterisk.org/view.php?id=18310
> 
> 
> Diffs
> -----
> 
>   /branches/1.6.2/main/pbx.c 301088 
> 
> Diff: https://reviewboard.asterisk.org/r/1072/diff
> 
> 
> Testing
> -------
> 
> one47 has reported on the issue that it has been going well so far (for 18 hours):
> https://issues.asterisk.org/view.php?id=18310#130297
> 
> 
> Thanks,
> 
> Jeff
> 
>

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