[asterisk-dev] [Code Review]: Read on idle timerfd will result in deadlocks

[kobaz]

> On July 6, 2011, 10:20 a.m., David Vossel wrote:
> > branches/1.8/res/res_timing_timerfd.c, lines 174-178
> > <https://reviewboard.asterisk.org/r/1255/diff/2/?file=16826#file16826line174>
> >
> >     I had to revert the patch from 1.8 and trunk.  This statement right hear is causing some serious performance problems.  See jrose's comment.
> >     
> >     If we comment out this line, everything works fine for him.
> 
> David Vossel wrote:
>     This same issue was reported here. https://issues.asterisk.org/jira/browse/ASTERISK-17867
> 
> kobaz wrote:
>     So, the whole patch was reverted.  Is your vote to have the same code fix but minus the debugging?  I've also heard from several people that the fix works with the debugging removed.
> 
> David Vossel wrote:
>     We tried taking out the debug statement in this if block, but it still caused performance issues.  Things did not work well until we completely took out that if statement.
> 
> jrose wrote:
>     Actually, the if statement was fine as long as you didn't include the return.  It's running through the whole loop until the call is answered that causes the problem.
> 
> jrose wrote:
>     Scratch that, I want to investigate this a little more.  It seems like it's actually not getting into the while loop that causes the problem.  Should have looked over it more carefully.

It seems to me that the higher level functions that use this need to be investigated.  The upper level calls to ack the timer should themselves be making sure they won't read an idle timer.  The upper level stuff should be aware of timers it needs to ack and only access ones that have events waiting.


- kobaz


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


On June 3, 2011, 4:11 p.m., kobaz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1255/
> -----------------------------------------------------------
> 
> (Updated June 3, 2011, 4:11 p.m.)
> 
> 
> Review request for Asterisk Developers and russelb.
> 
> 
> Summary
> -------
> 
> Fix for issue https://issues.asterisk.org/view.php?id=18028
> And several others.
> 
> 
> Diffs
> -----
> 
>   branches/1.8/res/res_timing_timerfd.c 320171 
> 
> Diff: https://reviewboard.asterisk.org/r/1255/diff
> 
> 
> Testing
> -------
> 
> Make enough calls to trigger a read on an idle timer
> 
> Wait for message:
> Reading attempt on idle timerfd. This would have caused a deadlock.
> 
> Deadlock avoided.
> 
> 
> Thanks,
> 
> kobaz
> 
>

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