[asterisk-dev] [Code Review] Read on disabled timerfd results in hangs, Take 2

[rmudgett]

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

Ship it!


Looks ok.

- rmudgett


On Aug. 12, 2011, 9:41 a.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1361/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2011, 9:41 a.m.)
> 
> 
> Review request for Asterisk Developers, David Vossel, kobaz, irroot, and jrose.
> 
> 
> Summary
> -------
> 
> This was originally reviewed as https://reviewboard.asterisk.org/r/1255 and then committed and reverted when a performance issue was found.
> 
> There were three problems with this patch. 1) There was no locking done around the timer, 2) We never released the reference to our_timer, and 3) The saved_timer values do not necessarily equal the actual timer value--it exists solely for setting things back the way they were after enabling/disabling continuous mode. We should not be using it to decide whether or not to do a read().
> 
> This patch adds locking, releases the reference from the ao2_find(), and removes the troublesome if() block, and uses ast_debug instead of ast_log(LOG_DEBUG, ...). In all other ways it is identical to the patch that was originally committed.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/res/res_timing_timerfd.c 331573 
> 
> Diff: https://reviewboard.asterisk.org/r/1361/diff
> 
> 
> Testing
> -------
> 
> None, yet. The other patch worked, but caused a regression with analog phones. jrose, if you still have a setup available to test this patch to see if the regression is gone, the patch is very much like the original so it should work. If anyone has a good example of how to reliably reproduce the original issue I would be happy to test that as well.
> 
> 
> Thanks,
> 
> Terry
> 
>

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