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

1 files changed, 42 insertions, 34 deletions

diff --git a/main/channel.c b/main/channel.c
index 126e44cf6..9998225f0 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -1669,11 +1669,13 @@ int ast_is_deferrable_frame(const struct ast_frame *frame)
 }
 
 /*! \brief Wait, look for hangups and condition arg */
-int ast_safe_sleep_conditional(struct ast_channel *chan, int ms, int (*cond)(void*), void *data)
+int ast_safe_sleep_conditional(struct ast_channel *chan, int timeout_ms, int (*cond)(void*), void *data)
 {
 	struct ast_frame *f;
 	struct ast_silence_generator *silgen = NULL;
 	int res = 0;
+	struct timeval start;
+	int ms;
 	AST_LIST_HEAD_NOLOCK(, ast_frame) deferred_frames;
 
 	AST_LIST_HEAD_INIT_NOLOCK(&deferred_frames);
@@ -1683,8 +1685,10 @@ int ast_safe_sleep_conditional(struct ast_channel *chan, int ms, int (*cond)(voi
 		silgen = ast_channel_start_silence_generator(chan);
 	}
 
-	while (ms > 0) {
+	start = ast_tvnow();
+	while ((ms = ast_remaining_ms(start, timeout_ms))) {
 		struct ast_frame *dup_f = NULL;
+
 		if (cond && ((*cond)(data) == 0)) {
 			break;
 		}
@@ -3005,12 +3009,15 @@ int __ast_answer(struct ast_channel *chan, unsigned int delay, int cdr_answer)
 		do {
 			AST_LIST_HEAD_NOLOCK(, ast_frame) frames;
 			struct ast_frame *cur, *new;
-			int ms = MAX(delay, 500);
+			int timeout_ms = MAX(delay, 500);
 			unsigned int done = 0;
+			struct timeval start;
 
 			AST_LIST_HEAD_INIT_NOLOCK(&frames);
 
+			start = ast_tvnow();
 			for (;;) {
+				int ms = ast_remaining_ms(start, timeout_ms);
 				ms = ast_waitfor(chan, ms);
 				if (ms < 0) {
 					ast_log(LOG_WARNING, "Error condition occurred when polling channel %s for a voice frame: %s\n", ast_channel_name(chan), strerror(errno));
@@ -3568,11 +3575,13 @@ struct ast_channel *ast_waitfor_n(struct ast_channel **c, int n, int *ms)
 
 int ast_waitfor(struct ast_channel *c, int ms)
 {
-	int oldms = ms;	/* -1 if no timeout */
-
-	ast_waitfor_nandfds(&c, 1, NULL, 0, NULL, NULL, &ms);
-	if ((ms < 0) && (oldms < 0)) {
-		ms = 0;
+	if (ms < 0) {
+		do {
+			ms = 100000;
+			ast_waitfor_nandfds(&c, 1, NULL, 0, NULL, NULL, &ms);
+		} while (!ms);
+	} else {
+		ast_waitfor_nandfds(&c, 1, NULL, 0, NULL, NULL, &ms);
 	}
 	return ms;
 }
@@ -3628,6 +3637,7 @@ int ast_settimeout(struct ast_channel *c, unsigned int rate, int (*func)(const v
 int ast_waitfordigit_full(struct ast_channel *c, int timeout_ms, int audiofd, int cmdfd)
 {
 	struct timeval start = ast_tvnow();
+	int ms;
 
 	/* Stop if we're a zombie or need a soft hangup */
 	if (ast_test_flag(ast_channel_flags(c), AST_FLAG_ZOMBIE) || ast_check_hangup(c))
@@ -3639,19 +3649,9 @@ int ast_waitfordigit_full(struct ast_channel *c, int timeout_ms, int audiofd, in
 	/* Wait for a digit, no more than timeout_ms milliseconds total.
 	 * Or, wait indefinitely if timeout_ms is <0.
 	 */
-	while (ast_tvdiff_ms(ast_tvnow(), start) < timeout_ms || timeout_ms < 0) {
+	while ((ms = ast_remaining_ms(start, timeout_ms))) {
 		struct ast_channel *rchan;
-		int outfd=-1;
-		int ms;
-
-		if (timeout_ms < 0) {
-			ms = timeout_ms;
-		} else {
-			ms = timeout_ms - ast_tvdiff_ms(ast_tvnow(), start);
-			if (ms < 0) {
-				ms = 0;
-			}
-		}
+		int outfd = -1;
 
 		errno = 0;
 		/* While ast_waitfor_nandfds tries to help by reducing the timeout by how much was waited,
@@ -4690,25 +4690,32 @@ int ast_recvchar(struct ast_channel *chan, int timeout)
 
 char *ast_recvtext(struct ast_channel *chan, int timeout)
 {
-	int res, done = 0;
+	int res;
 	char *buf = NULL;
+	struct timeval start = ast_tvnow();
+	int ms;
 
-	while (!done) {
+	while ((ms = ast_remaining_ms(start, timeout))) {
 		struct ast_frame *f;
-		if (ast_check_hangup(chan))
+
+		if (ast_check_hangup(chan)) {
 			break;
-		res = ast_waitfor(chan, timeout);
-		if (res <= 0) /* timeout or error */
+		}
+		res = ast_waitfor(chan, ms);
+		if (res <= 0)  {/* timeout or error */
 			break;
-		timeout = res;	/* update timeout */
+		}
 		f = ast_read(chan);
-		if (f == NULL)
+		if (f == NULL) {
 			break; /* no frame */
-		if (f->frametype == AST_FRAME_CONTROL && f->subclass.integer == AST_CONTROL_HANGUP)
-			done = 1;	/* force a break */
-		else if (f->frametype == AST_FRAME_TEXT) {		/* what we want */
+		}
+		if (f->frametype == AST_FRAME_CONTROL && f->subclass.integer == AST_CONTROL_HANGUP) {
+			ast_frfree(f);
+			break;
+		} else if (f->frametype == AST_FRAME_TEXT) {		/* what we want */
 			buf = ast_strndup((char *) f->data.ptr, f->datalen);	/* dup and break */
-			done = 1;
+			ast_frfree(f);
+			break;
 		}
 		ast_frfree(f);
 	}
@@ -5724,18 +5731,19 @@ struct ast_channel *__ast_request_and_dial(const char *type, struct ast_format_c
 	if (ast_call(chan, addr, 0)) {	/* ast_call failed... */
 		ast_log(LOG_NOTICE, "Unable to call channel %s/%s\n", type, addr);
 	} else {
+		struct timeval start = ast_tvnow();
 		res = 1;	/* mark success in case chan->_state is already AST_STATE_UP */
 		while (timeout && ast_channel_state(chan) != AST_STATE_UP) {
 			struct ast_frame *f;
-			res = ast_waitfor(chan, timeout);
+			int ms = ast_remaining_ms(start, timeout);
+
+			res = ast_waitfor(chan, ms);
 			if (res == 0) { /* timeout, treat it like ringing */
 				*outstate = AST_CONTROL_RINGING;
 				break;
 			}
 			if (res < 0) /* error or done */
 				break;
-			if (timeout > -1)
-				timeout = res;
 			if (!ast_strlen_zero(ast_channel_call_forward(chan))) {
 				if (!(chan = ast_call_forward(NULL, chan, NULL, cap, oh, outstate))) {
 					return NULL;