From fc558d28f249148b97b6df4d9650924cb7966413 Mon Sep 17 00:00:00 2001 From: Mark Michelson Date: Fri, 2 Mar 2012 01:33:06 +0000 Subject: [PATCH] Fix race condition that can cause important control frames (such as a hangup) to be missed. This takes two actions. 1. Move the reading of the alertpipe in __ast_read() to immediately before the removal of frames from the readq. This means we won't do something silly like read from the alertpipe, then ignore the fact that there's a frame to get from the readq since channel's fdno is the AST_TIMING_FD. 2. When ast_settimeout() sets the rate to 0 and the timingfunc to NULL, if the channel's fdno is the AST_TIMING_FD, then set the fdno to -1. This is because if the rate is 0 and the timingfunc is NULL, it means that the channel's timing fd is being invalidated, so any pending reads should not occur. This may actually solve more issues than the referenced one below, but it's not known at this time for sure. (closes issue ASTERISK-19223) reported by Frank-Michael Wittig Review: https://reviewboard.asterisk.org/r/1779 ........ Merged revisions 357761 from http://svn.asterisk.org/svn/asterisk/branches/1.8 ........ Merged revisions 357762 from http://svn.asterisk.org/svn/asterisk/branches/10 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@357775 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- main/channel.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/main/channel.c b/main/channel.c index 9222a33c66..39edcd1436 100644 --- a/main/channel.c +++ b/main/channel.c @@ -3342,6 +3342,16 @@ int ast_settimeout(struct ast_channel *c, unsigned int rate, int (*func)(const v c->timingfunc = func; ast_channel_timingdata_set(c, data); + if (func == NULL && rate == 0 && ast_channel_fdno(c) == AST_TIMING_FD) { + /* Clearing the timing func and setting the rate to 0 + * means that we don't want to be reading from the timingfd + * any more. Setting c->fdno to -1 means we won't have any + * errant reads from the timingfd, meaning we won't potentially + * miss any important frames. + */ + ast_channel_fdno_set(c, -1); + } + ast_channel_unlock(c); return res; @@ -3614,13 +3624,6 @@ static struct ast_frame *__ast_read(struct ast_channel *chan, int dropaudio) prestate = ast_channel_state(chan); - /* Read and ignore anything on the alertpipe, but read only - one sizeof(blah) per frame that we send from it */ - if (ast_channel_internal_alert_read(chan) == AST_ALERT_READ_FATAL) { - f = &ast_null_frame; - goto done; - } - if (ast_channel_timingfd(chan) > -1 && ast_channel_fdno(chan) == AST_TIMING_FD) { enum ast_timer_event res; @@ -3671,6 +3674,13 @@ static struct ast_frame *__ast_read(struct ast_channel *chan, int dropaudio) ast_clear_flag(chan, AST_FLAG_EXCEPTION); } + /* Read and ignore anything on the alertpipe, but read only + one sizeof(blah) per frame that we send from it */ + if (ast_channel_internal_alert_read(chan) == AST_ALERT_READ_FATAL) { + f = &ast_null_frame; + goto done; + } + /* Check for pending read queue */ if (!AST_LIST_EMPTY(ast_channel_readq(chan))) { int skip_dtmf = should_skip_dtmf(chan);