[asterisk-dev] [Code Review] Remove some unnecessary locking from ast_hangup()

[rmudgett]

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

Ship it!


Yes holding the channels container lock is not necessary.

However, your example about the audiohooks could still cause problems because the channel lock is held and many ao2_callbacks over the channels container also lock each channel in turn.

Would moving the audiohook and framehook destruction after the channel is unlinked be needed to completely avoid your example holding up the system?

- rmudgett


On Feb. 3, 2012, 9:58 a.m., Russell Bryant wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1712/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2012, 9:58 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch removes some unnecessary locking of the channels container in ast_hangup().  The reason this came up is that this lock can very quickly block the entire system.  If any of the channel cleanup code decides to block, it causes a problem for the whole system.  For example, when audiohooks get destroyed, if that blocks for a while waiting on the mixmonitor thread to exit because it's busy blocking on some I/O, it causes a problem for many other threads in the meantime.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/main/channel.c 353961 
> 
> Diff: https://reviewboard.asterisk.org/r/1712/diff
> 
> 
> Testing
> -------
> 
> Ran under load in a test environment, about 1/2 million calls
> 
> 
> Thanks,
> 
> Russell
> 
>

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