[asterisk-commits] russell: branch 1.2 r43778 - in /branches/1.2: channel.c res/res_features.c

[asterisk-commits at lists.digium.com]

Author: russell
Date: Wed Sep 27 11:54:30 2006
New Revision: 43778

URL: http://svn.digium.com/view/asterisk?rev=43778&view=rev
Log:
Fix a problem that occurred if a user entered a digit that matched a bridge
feature that was configured using multiple digits, and the digit that was
pressed timed out in the feature digit timeout period.  For example, if blind
transfer is configured as '##', and a user presses just '#'.  In this situation,
the call would lock up and no longer pass any frames.
(issue #7977 reported by festr, and issue #7982 reported by michaels and
 valuable input provided by mneuhauser and kuj.  Fixed by me, with testing help
 and peer review from Joshua Colp).

There are a couple of issues involved in this fix:

1) When ast_generic_bridge determines that there has been a timeout, it returned
   AST_BRIDGE_RETRY.  Then, when ast_channel_bridge gets this result, it calls
   ast_generic_bridge over again with the same timestamp for the next event.
   This results in an endless loop of nothing until the call is terminated.
   This is resolved by simply changing ast_generic_bridge to return 
   AST_BRIDGE_COMPLETE when it sees a timeout.

2) I also changed ast_channel_bridge such that if in the process of calculating
   the time until the next event, it knows a timeout has already occured, to
   immediately return AST_BRIDGE_COMPLETE instead of attempting to bridge the
   channels anyway.

3) In the process of testing the previous two changes, I ran into a problem in
   res_features where ast_channel_bridge would return because it determined
   that there was a timeout.  However, ast_bridge_call in res_features would
   then determine by its own calculation that there was still 1 ms before the
   timeout really occurs.  It would then proceed, and since the bridge broke
   out and did *not* return a frame, it interpreted this as the call was over
   and hung up the channels.

   The reason for this was because ast_bridge_call in res_features and
   ast_channel_bridge in channel.c were using different times for their
   calculations.  channel.c uses the start_time on the bridge config, which
   is the time that the feature digit was recieved.  However, res_features
   had another time, 'start', which was set right before calling 
   ast_channel_bridge.  'start' will always be slightly after start_time in the
   bridge config, and sometimes enough to round up to one ms.

   This is fixed by making ast_bridge_call use the same time as 
   ast_channel_bridge for the timeout calculation.

Modified:
    branches/1.2/channel.c
    branches/1.2/res/res_features.c

Modified: branches/1.2/channel.c
URL: http://svn.digium.com/view/asterisk/branches/1.2/channel.c?rev=43778&r1=43777&r2=43778&view=diff
==============================================================================
--- branches/1.2/channel.c (original)
+++ branches/1.2/channel.c Wed Sep 27 11:54:30 2006
@@ -3351,7 +3351,7 @@
 		if (bridge_end.tv_sec) {
 			to = ast_tvdiff_ms(bridge_end, ast_tvnow());
 			if (to <= 0) {
-				res = AST_BRIDGE_RETRY;
+				res = AST_BRIDGE_COMPLETE;
 				break;
 			}
 		} else
@@ -3510,8 +3510,10 @@
 		if (!ast_tvzero(nexteventts)) {
 			now = ast_tvnow();
 			to = ast_tvdiff_ms(nexteventts, now);
-			if (to < 0)
-				to = 0;
+			if (to <= 0) {
+				res = AST_BRIDGE_COMPLETE;
+				break;
+			}
 		}
 
 		if (config->timelimit) {

Modified: branches/1.2/res/res_features.c
URL: http://svn.digium.com/view/asterisk/branches/1.2/res/res_features.c?rev=43778&r1=43777&r2=43778&view=diff
==============================================================================
--- branches/1.2/res/res_features.c (original)
+++ branches/1.2/res/res_features.c Wed Sep 27 11:54:30 2006
@@ -1274,7 +1274,6 @@
 	int hasfeatures=0;
 	int hadfeatures=0;
 	struct ast_option_header *aoh;
-	struct timeval start = { 0 , 0 };
 	struct ast_bridge_config backup_config;
 	char *monitor_exec;
 
@@ -1323,14 +1322,11 @@
 		peer->cdr = NULL;
 	}
 	for (;;) {
-		if (config->feature_timer)
-			start = ast_tvnow();
-
 		res = ast_channel_bridge(chan, peer, config, &f, &who);
 
 		if (config->feature_timer) {
 			/* Update time limit for next pass */
-			diff = ast_tvdiff_ms(ast_tvnow(), start);
+			diff = ast_tvdiff_ms(ast_tvnow(), config->start_time);
 			config->feature_timer -= diff;
 			if (hasfeatures) {
 				/* Running on backup config, meaning a feature might be being
@@ -1367,7 +1363,12 @@
 					hadfeatures = hasfeatures;
 					/* Continue as we were */
 					continue;
-				}
+				} else if (!f) {
+					/* The bridge returned without a frame and there is a feature in progress.
+					 * However, we don't think the feature has quite yet timed out, so just
+					 * go back into the bridge. */
+					continue;
+ 				}
 			} else {
 				if (config->feature_timer <=0) {
 					/* We ran out of time */