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

Terry Wilson reviewboard at asterisk.org
Fri Aug 12 09:41:02 CDT 2011


-----------------------------------------------------------
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.


Changes
-------

Add irroot and kobaz since they looked at the last try at fixing this.


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/20110812/a82f39e4/attachment-0001.htm>


More information about the asterisk-dev mailing list