[Asterisk-code-review] res agi: Prevent run agi from eating frame is shouldn't (asterisk[master])

[George Joseph]

George Joseph has uploaded a new change for review.

  https://gerrit.asterisk.org/2688

Change subject: res_agi:  Prevent run_agi from eating frame is shouldn't
......................................................................

res_agi:  Prevent run_agi from eating frame is shouldn't

The run_agi function is eating control frames when it shouldn't be. This is
causing issues when an AGI is run from CONNECTED_LINE_SEND_SUB in a blond
transfer.

Alice calls Bob. Bob attended transfers to Charlie but hangs up before Charlie
answers.

Alice gets the COLP UPDATE indicating Charlie but Charlie never gets an UPDATE
and is left thinking he's connected to Bob.

In this case, when CONNECTED_LINE_SEND_SUB runs on Alice's channel and it calls
an AGI, the extra eaten frames prevent CONNECTED_LINE_SEND_SUB from running on
Charlie's channel.

The fix was to accumulate deferrable frames in the "forever" loop instead of
dropping them, and re-queue them just before running the actual agi command
or exiting.

ASTERISK-25951 #close

Change-Id: I0f4bbfd72fc1126c2aaba41da3233a33d0433645
---
M res/res_agi.c
1 file changed, 33 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/88/2688/1

diff --git a/res/res_agi.c b/res/res_agi.c
index f6ce749..f3e588b 100644
--- a/res/res_agi.c
+++ b/res/res_agi.c
@@ -3754,6 +3754,7 @@
 	const char *sighup_str;
 	const char *exit_on_hangup_str;
 	int exit_on_hangup;
+	AST_LIST_HEAD_NOLOCK(, ast_frame) deferred_frames = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
 
 	ast_channel_lock(chan);
 	sighup_str = pbx_builtin_getvar_helper(chan, "AGISIGHUP");
@@ -3815,8 +3816,20 @@
 					/* Write, ignoring errors */
 					if (write(agi->audio, f->data.ptr, f->datalen) < 0) {
 					}
+					ast_frfree(f);
+				} else if (ast_is_deferrable_frame(f)) {
+					struct ast_frame *dup_f;
+
+					if ((dup_f = ast_frisolate(f))) {
+						AST_LIST_INSERT_HEAD(&deferred_frames, dup_f, frame_list);
+					}
+
+					if (dup_f != f) {
+						ast_frfree(f);
+					}
+				} else {
+					ast_frfree(f);
 				}
-				ast_frfree(f);
 			}
 		} else if (outfd > -1) {
 			size_t len = sizeof(buf);
@@ -3864,6 +3877,15 @@
 				buf[buflen - 1] = '\0';
 			}
 
+			if (!AST_LIST_EMPTY(&deferred_frames)) {
+				ast_channel_lock(chan);
+				while ((f = AST_LIST_REMOVE_HEAD(&deferred_frames, frame_list))) {
+					ast_queue_frame_head(chan, f);
+					ast_frfree(f);
+				}
+				ast_channel_unlock(chan);
+			}
+
 			if (agidebug)
 				ast_verbose("<%s>AGI Rx << %s\n", ast_channel_name(chan), buf);
 			cmd_status = agi_handle_command(chan, agi, buf, dead);
@@ -3885,6 +3907,16 @@
 			}
 		}
 	}
+
+	if (!AST_LIST_EMPTY(&deferred_frames)) {
+		ast_channel_lock(chan);
+		while ((f = AST_LIST_REMOVE_HEAD(&deferred_frames, frame_list))) {
+			ast_queue_frame_head(chan, f);
+			ast_frfree(f);
+		}
+		ast_channel_unlock(chan);
+	}
+
 	if (agi->speech) {
 		ast_speech_destroy(agi->speech);
 	}

-- 
To view, visit https://gerrit.asterisk.org/2688
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0f4bbfd72fc1126c2aaba41da3233a33d0433645
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <gjoseph at digium.com>