[asterisk-dev] [Code Review] 2441: res_timing_pthread: Reduce probability of deadlocking on timer.

[Matt Jordan]

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


I'm not sure I can fully articulate what is happening here yet, so bear with me.

Assume we are using res_timing_pthread and this patch. When a silence generator is on a channel and the channel timer is put into continuous mode (which occurs during play_and_record), there appears to be a race condition between the timer being serviced - where write_byte is called and pending_ticks is incremented - and the thread servicing the timer acknowledging the timer properly. At some point, timer->pending_ticks is incremented past 1, at which point all hell breaks loose.

By "all hell breaking loose", the channel in question which has the audio streamed on it is no longer serviced; the dreaded "exceptionally long queue" message is displayed; the channel cannot be hung up; and the CPU pegs out. This so far can only be reproduced if the silence generator on record option is enabled in asterisk.conf and a Local channel is put into VoiceMail.

A simple fix thus far has been to put the following at the beginning of write_byte:

if (timer->pending_ticks) {
    return;
}
timer->pending_ticks++;

This means we can only be in overflow (which would be a very strange situation to get into, and would only happen if someone in a future patch incremented pending_ticks elsewhere) or enabled.

Although this patch did prevent the situation I was seeing, I did notice that the silence generator will still sometimes prevent Asterisk from detecting silence on a channel and killing the recording. That's potentially a separate bug (and does not happen every time).

- Matt Jordan


On April 11, 2013, 10:16 p.m., Shaun Ruffell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2441/
> -----------------------------------------------------------
> 
> (Updated April 11, 2013, 10:16 p.m.)
> 
> 
> Review request for Asterisk Developers and Matt Jordan.
> 
> 
> Bugs: ASTERISK-21389
>     https://issues.asterisk.org/jira/browse/ASTERISK-21389
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> From the patch attached on ASTERISK-21389:
> 
> res_timing_pthread: Reduce probability of deadlocking on timer.
> 
> There were several reports of deadlock when using
> res_timing_pthread. Backtraces indicated that one thread was blocked
> waiting for the write to the pipe to complete and this thread held
> the container lock for the timers.  Therefore any thread that wanted
> to create a new timer or read an existing timer would block waiting
> for either the timer lock or the container lock and deadlock ensued.
> 
> This patch changes the way the pipe is used to eliminate this source
> of deadlocks:
> 
> 1) The pipe is placed in non-blocking mode so that it would never
> block even if the following changes someone fail...
> 
> 2) Instead of writing bytes into the pipe for each "tick" that's
> fired the pipe now has two states--signaled and unsignaled. If
> signaled, the pipe is hot and any pollers of the read side
> filedescriptor will be woken up. If unsigned the pipe is idle. This
> eliminates even the chance of filling up the pipe and reduces the
> potential overhead of calling unnecessary writes.
> 
> 3) Since we're tracking the signaled / unsignaled state, we can
> eliminate the exta poll system call for every firing because we know
> that there is data to be read.
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_timing_pthread.c 385382 
> 
> Diff: https://reviewboard.asterisk.org/r/2441/diff/
> 
> 
> Testing
> -------
> 
> ran timing test on the console and also put several channels in a confbridge.
> 
> 
> Thanks,
> 
> Shaun Ruffell
> 
>

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