[asterisk-dev] [Code Review] timerfd: Remove special workaround in channel.c ast_channel_alloc_ap function to not unset needqueue value because it breaks certain hangup conditions in meetme

[jrose]

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

Review request for Asterisk Developers, Mark Michelson and Matt Jordan.


Summary
-------

http://lists.digium.com/pipermail/asterisk-commits/2010-November/046240.html

Back slightly before I joined up with Digium, Jeff Peeler created this patch to address the following bug:
ASTERISK-16783 - https://issues.asterisk.org/jira/browse/ASTERISK-16783

It looks like he was only able to reproduce it on the reporter's own machine, but going by the commit message the patch seemed to work around the internal problems (IE obscuring them) rather than address them.

As far as the meetme problem is concerned, this patch had an immediate effect of producing a warning in in the playback function regarding an inability to write a frame. There are actually numerous playback conditions where this warning will be produced and it isn't exclusive to meetme. The problems start coming a little later.

In January of the following year (right around when I was being interviewed), Russell committed a patch to address some problems with MeetMe. This patch removed a hangup check which was holding back the floodgates. After this patch was committed, hanging up during the sound prompt in meetme would start causing a long string of write error messages because ast_waitfor_nandfds could no longer get the channel after it hung up. Without being able to get the channel, meetme's main loop in confrun couldn't exit under normal conditions. Eventually, the channel gets destroyed by some other mechanism which ends the log messages and allows meetme to exit, but obviously the behavior isn't ideal.

Jeff's patch contained a workaround which has actually served to obscure certain problems in timerfd (many of which may have been resolved already since timerfd has received numerous patches over the past couple years).  As such, a small part of it is reverted by this patch, specifically the workaround introduced in the channel allocation function.


This addresses bugs ASTERISK-19594, ASTERISK-19949 and ASTERISK-20486.
    https://issues.asterisk.org/jira/browse/ASTERISK-19594
    https://issues.asterisk.org/jira/browse/ASTERISK-19949
    https://issues.asterisk.org/jira/browse/ASTERISK-20486


Diffs
-----

  /branches/1.8/main/channel.c 375992 

Diff: https://reviewboard.asterisk.org/r/2186/diff


Testing
-------

Reproduction of the bug was simple and merely involved hanging up during playback of the conf-onlyperson prompt.  I'd seen similar bugs in numerous places in Asterisk for a while now. Confirming the patch was fairly simple and I just did the same thing and checked that the nasty string of errors didn't occur. It didn't and the channels closed out properly.


Thanks,

jrose

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