[asterisk-dev] [Code Review] Avoid deadlocks in chan_sip - 1.6.0

[Olle E Johansson]

> On 2009-11-19 15:22:38, Russell Bryant wrote:
> > I would be very interested in seeing the "core show locks" output for the deadlocks that you encountered.  Then, I would be more than happy to help with determining the safest changes to make.
> > 
> > There is a "Locking in Asterisk" section of the CODING-GUIDELINES document that may help, as well.

I'll see what we can do about the "core show locks".  There are some clear paths that will deadlock, but there might be other locks that we can change to let these code pieces do their work. I just could not find them. 

Thanks for the quick feedback, Russell!


- Olle E


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


On 2009-11-19 11:02:38, Olle E Johansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/433/
> -----------------------------------------------------------
> 
> (Updated 2009-11-19 11:02:38)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> While running 1.6.0 in production, we have stumbled over a few deadlocks that we have proven to avoid with these changes.
> 
> - CLI "sip show channels" can cause serious damage before this change
> - Retrans_pkt had some issues with locking the Ast_channel
> - Both sip_write and sip_read was hanging waiting for a better time.
> 
> We debugged with GDB to see where each thread was hanging and found all these issues. Tested with and without patches and the issues disappeared on the patched systems.
> 
> Locking is a part of Asterisk coding that I'm far away from being a master in - so please give feedback. I'm sure there's a lot of room for improvement.
> 
> (And yes, I noticed the red box in the patch. Will remove.)
> 
> 
> Diffs
> -----
> 
>   /team/oej/moretrylock-1.6.0/channels/chan_sip.c 230467 
> 
> Diff: https://reviewboard.asterisk.org/r/433/diff
> 
> 
> Testing
> -------
> 
> The patches has been running in production on a couple of PBXs for a couple of weeks without causing any harm that we're aware of.
> 
> 
> Thanks,
> 
> Olle E
> 
>