diff options
author | Mark Michelson <mmichelson@digium.com> | 2012-11-07 19:15:26 +0000 |
---|---|---|
committer | Mark Michelson <mmichelson@digium.com> | 2012-11-07 19:15:26 +0000 |
commit | f2bb9afe17ee3e321a2d853dbfed4a93a67c25ca (patch) | |
tree | e86066d0a7b10defce1f17b3e2038527555f92e9 /apps | |
parent | 6ad0126425993bd9c9c73b0c05a52984cbcadc5d (diff) |
Multiple revisions 375993-375994
........
r375993 | mmichelson | 2012-11-07 11:01:13 -0600 (Wed, 07 Nov 2012) | 30 lines
Fix misuses of timeouts throughout the code.
Prior to this change, a common method for determining if a timeout
was reached was to call a function such as ast_waitfor_n() and inspect
the out parameter that told how many milliseconds were left, then use
that as the input to ast_waitfor_n() on the next go-around.
The problem with this is that in some cases, submillisecond timeouts
can occur, resulting in the out parameter not decreasing any. When this
happens thousands of times, the result is that the timeout takes much
longer than intended to be reached. As an example, I had a situation where
a 3 second timeout took multiple days to finally end since most wakeups
from ast_waitfor_n() were under a millisecond.
This patch seeks to fix this pattern throughout the code. Now we log the
time when an operation began and find the difference in wall clock time
between now and when the event started. This means that sub-millisecond timeouts
now cannot play havoc when trying to determine if something has timed out.
Part of this fix also includes changing the function ast_waitfor() so that it
is possible for it to return less than zero when a negative timeout is given
to it. This makes it actually possible to detect errors in ast_waitfor() when
there is no timeout.
(closes issue ASTERISK-20414)
reported by David M. Lee
Review: https://reviewboard.asterisk.org/r/2135/
........
r375994 | mmichelson | 2012-11-07 11:08:44 -0600 (Wed, 07 Nov 2012) | 3 lines
Remove some debugging that accidentally made it in the last commit.
........
Merged revisions 375993-375994 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........
Merged revisions 375995 from http://svn.asterisk.org/svn/asterisk/branches/10
........
Merged revisions 376014 from http://svn.asterisk.org/svn/asterisk/branches/11
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@376015 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Diffstat (limited to 'apps')
-rw-r--r-- | apps/app_dial.c | 14 | ||||
-rw-r--r-- | apps/app_jack.c | 4 | ||||
-rw-r--r-- | apps/app_meetme.c | 2 | ||||
-rw-r--r-- | apps/app_queue.c | 24 | ||||
-rw-r--r-- | apps/app_record.c | 27 | ||||
-rw-r--r-- | apps/app_waitforring.c | 25 |
6 files changed, 59 insertions, 37 deletions
diff --git a/apps/app_dial.c b/apps/app_dial.c index 310326dc1..b162a4127 100644 --- a/apps/app_dial.c +++ b/apps/app_dial.c @@ -1116,6 +1116,7 @@ static struct ast_channel *wait_for_answer(struct ast_channel *in, int is_cc_recall; int cc_frame_received = 0; int num_ringing = 0; + struct timeval start = ast_tvnow(); ast_party_connected_line_init(&connected_caller); if (single) { @@ -1158,7 +1159,7 @@ static struct ast_channel *wait_for_answer(struct ast_channel *in, } #endif - while (*to && !peer) { + while ((*to = ast_remaining_ms(start, orig)) && !peer) { struct chanlist *o; int pos = 0; /* how many channels do we handle */ int numlines = prestart; @@ -1702,10 +1703,13 @@ static struct ast_channel *wait_for_answer(struct ast_channel *in, skip_frame:; ast_frfree(f); } - if (!*to) - ast_verb(3, "Nobody picked up in %d ms\n", orig); - if (!*to || ast_check_hangup(in)) - ast_cdr_noanswer(ast_channel_cdr(in)); + } + + if (!*to) { + ast_verb(3, "Nobody picked up in %d ms\n", orig); + } + if (!*to || ast_check_hangup(in)) { + ast_cdr_noanswer(ast_channel_cdr(in)); } #ifdef HAVE_EPOLL diff --git a/apps/app_jack.c b/apps/app_jack.c index 10964f462..d1bdfa38f 100644 --- a/apps/app_jack.c +++ b/apps/app_jack.c @@ -768,7 +768,9 @@ static int jack_exec(struct ast_channel *chan, const char *data) while (!jack_data->stop) { struct ast_frame *f; - ast_waitfor(chan, -1); + if (ast_waitfor(chan, -1) < 0) { + break; + } f = ast_read(chan); if (!f) { diff --git a/apps/app_meetme.c b/apps/app_meetme.c index c2bfccd03..da66c5941 100644 --- a/apps/app_meetme.c +++ b/apps/app_meetme.c @@ -1917,7 +1917,7 @@ static void conf_flush(int fd, struct ast_channel *chan) /* when no frames are available, this will wait for 1 millisecond maximum */ - while (ast_waitfor(chan, 1)) { + while (ast_waitfor(chan, 1) > 0) { f = ast_read(chan); if (f) ast_frfree(f); diff --git a/apps/app_queue.c b/apps/app_queue.c index a6ec76616..775f34348 100644 --- a/apps/app_queue.c +++ b/apps/app_queue.c @@ -4052,6 +4052,7 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte #endif struct ast_party_connected_line connected_caller; char *inchan_name; + struct timeval start_time_tv = ast_tvnow(); ast_party_connected_line_init(&connected_caller); @@ -4068,7 +4069,7 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte } #endif - while (*to && !peer) { + while ((*to = ast_remaining_ms(start_time_tv, orig)) && !peer) { int numlines, retry, pos = 1; struct ast_channel *watchers[AST_MAX_WATCHERS]; watchers[0] = in; @@ -4341,10 +4342,10 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte rna(endtime * 1000, qe, on, membername, qe->parent->autopausebusy); if (qe->parent->strategy != QUEUE_STRATEGY_RINGALL) { if (qe->parent->timeoutrestart) { - *to = orig; + start_time_tv = ast_tvnow(); } /* Have enough time for a queue member to answer? */ - if (*to > 500) { + if (ast_remaining_ms(start_time_tv, orig) > 500) { ring_one(qe, outgoing, &numbusies); starttime = (long) time(NULL); } @@ -4362,9 +4363,9 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte do_hang(o); if (qe->parent->strategy != QUEUE_STRATEGY_RINGALL) { if (qe->parent->timeoutrestart) { - *to = orig; + start_time_tv = ast_tvnow(); } - if (*to > 500) { + if (ast_remaining_ms(start_time_tv, orig) > 500) { ring_one(qe, outgoing, &numbusies); starttime = (long) time(NULL); } @@ -4457,9 +4458,9 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte do_hang(o); if (qe->parent->strategy != QUEUE_STRATEGY_RINGALL) { if (qe->parent->timeoutrestart) { - *to = orig; + start_time_tv = ast_tvnow(); } - if (*to > 500) { + if (ast_remaining_ms(start_time_tv, orig) > 500) { ring_one(qe, outgoing, &numbusies); starttime = (long) time(NULL); } @@ -4538,10 +4539,11 @@ skip_frame:; ast_frfree(f); } - if (!*to) { - for (o = start; o; o = o->call_next) { - rna(orig, qe, o->interface, o->member->membername, 1); - } + } + + if (!*to) { + for (o = start; o; o = o->call_next) { + rna(orig, qe, o->interface, o->member->membername, 1); } if (ast_channel_cdr(in) diff --git a/apps/app_record.c b/apps/app_record.c index ff233d6a9..051f97bb8 100644 --- a/apps/app_record.c +++ b/apps/app_record.c @@ -160,7 +160,6 @@ static int record_exec(struct ast_channel *chan, const char *data) int terminator = '#'; struct ast_format rfmt; int ioflags; - int waitres; struct ast_silence_generator *silgen = NULL; struct ast_flags flags = { 0, }; AST_DECLARE_APP_ARGS(args, @@ -169,8 +168,11 @@ static int record_exec(struct ast_channel *chan, const char *data) AST_APP_ARG(maxduration); AST_APP_ARG(options); ); + int ms; + struct timeval start; ast_format_clear(&rfmt); + /* The next few lines of code parse out the filename and header from the input string */ if (ast_strlen_zero(data)) { /* no data implies no filename or anything is present */ ast_log(LOG_WARNING, "Record requires an argument (filename)\n"); @@ -331,14 +333,15 @@ static int record_exec(struct ast_channel *chan, const char *data) if (maxduration <= 0) maxduration = -1; - while ((waitres = ast_waitfor(chan, maxduration)) > -1) { - if (maxduration > 0) { - if (waitres == 0) { - gottimeout = 1; - pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "TIMEOUT"); - break; - } - maxduration = waitres; + start = ast_tvnow(); + while ((ms = ast_remaining_ms(start, maxduration))) { + ms = ast_waitfor(chan, ms); + if (ms < 0) { + break; + } + + if (maxduration > 0 && ms == 0) { + break; } f = ast_read(chan); @@ -390,6 +393,12 @@ static int record_exec(struct ast_channel *chan, const char *data) } ast_frfree(f); } + + if (maxduration > 0 && !ms) { + gottimeout = 1; + pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "TIMEOUT"); + } + if (!f) { ast_debug(1, "Got hangup\n"); res = -1; diff --git a/apps/app_waitforring.c b/apps/app_waitforring.c index bd0353b07..fc02de303 100644 --- a/apps/app_waitforring.c +++ b/apps/app_waitforring.c @@ -63,22 +63,29 @@ static int waitforring_exec(struct ast_channel *chan, const char *data) struct ast_silence_generator *silgen = NULL; int res = 0; double s; + int timeout_ms; int ms; + struct timeval start = ast_tvnow(); if (!data || (sscanf(data, "%30lg", &s) != 1)) { ast_log(LOG_WARNING, "WaitForRing requires an argument (minimum seconds)\n"); return 0; } + if (s < 0.0) { + ast_log(LOG_WARNING, "Invalid timeout provided for WaitForRing (%lg)\n", s); + return 0; + } + if (ast_opt_transmit_silence) { silgen = ast_channel_start_silence_generator(chan); } - ms = s * 1000.0; - while (ms > 0) { + timeout_ms = s * 1000.0; + while ((ms = ast_remaining_ms(start, timeout_ms))) { ms = ast_waitfor(chan, ms); if (ms < 0) { - res = ms; + res = -1; break; } if (ms > 0) { @@ -95,14 +102,12 @@ static int waitforring_exec(struct ast_channel *chan, const char *data) } /* Now we're really ready for the ring */ if (!res) { - ms = 99999999; - while(ms > 0) { - ms = ast_waitfor(chan, ms); - if (ms < 0) { - res = ms; + for (;;) { + int wait_res = ast_waitfor(chan, -1); + if (wait_res < 0) { + res = -1; break; - } - if (ms > 0) { + } else { f = ast_read(chan); if (!f) { res = -1; |