[Asterisk-code-review] res agi: Prevent an AGI from eating frames it should not. (R... (asterisk[master])

Joshua Colp asteriskteam at digium.com
Fri Feb 3 05:32:12 CST 2017


Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/4879 )

Change subject: res_agi: Prevent an AGI from eating frames it should not. (Re-do)
......................................................................


res_agi: Prevent an AGI from eating frames it should not. (Re-do)

A dialplan intercept routine is equivalent to an interrupt routine.  As
such, the routine must be done quickly and you do not have access to the
media stream.  These restrictions are necessary because the media stream
is the responsibility of some other code and interfering with or delaying
that processing is bad.  A possible future dialplan processing
architecture change may allow the interception routine to run in a
different thread from the main thread handling the media and remove the
execution time restriction.

* Made res_agi.c:run_agi() running an AGI in an interception routine run
in DeadAGI mode.  No touchy channel frames.

ASTERISK-25951

ASTERISK-26343

ASTERISK-26716

Change-Id: I638f147ca7a7f2590d7194a8ef4090eb191e4e43
---
M include/asterisk/channel.h
M main/channel.c
M res/res_agi.c
3 files changed, 61 insertions(+), 4 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Anonymous Coward #1000019: Verified



diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 9cf313f..e5f792f 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -4717,4 +4717,21 @@
  */
 enum ast_channel_error ast_channel_errno(void);
 
+/*!
+ * \brief Am I currently running an intercept dialplan routine.
+ * \since 13.14.0
+ *
+ * \details
+ * A dialplan intercept routine is equivalent to an interrupt
+ * routine.  As such, the routine must be done quickly and you
+ * do not have access to the media stream.  These restrictions
+ * are necessary because the media stream is the responsibility
+ * of some other code and interfering with or delaying that
+ * processing is bad.
+ *
+ * \retval 0 Not in an intercept routine.
+ * \retval 1 In an intercept routine.
+ */
+int ast_channel_get_intercept_mode(void);
+
 #endif /* _ASTERISK_CHANNEL_H */
diff --git a/main/channel.c b/main/channel.c
index 7c8d3a9..1c7743a 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -10300,6 +10300,36 @@
 	ast_queue_control_data(chan, AST_CONTROL_REDIRECTING, data, datalen);
 }
 
+/*!
+ * Storage to determine if the current thread is running an intercept dialplan routine.
+ */
+AST_THREADSTORAGE_RAW(in_intercept_routine);
+
+/*!
+ * \internal
+ * \brief Set the current intercept dialplan routine status mode.
+ * \since 13.14.0
+ *
+ * \param in_intercept_mode New intercept mode.  (Non-zero if in intercept mode)
+ *
+ * \return Nothing
+ */
+static void channel_set_intercept_mode(int in_intercept_mode)
+{
+	int status;
+
+	status = ast_threadstorage_set_ptr(&in_intercept_routine,
+		in_intercept_mode ? (void *) 1 : (void *) 0);
+	if (status) {
+		ast_log(LOG_ERROR, "Failed to set dialplan intercept mode\n");
+	}
+}
+
+int ast_channel_get_intercept_mode(void)
+{
+	return ast_threadstorage_get_ptr(&in_intercept_routine) ? 1 : 0;
+}
+
 int ast_channel_connected_line_macro(struct ast_channel *autoservice_chan, struct ast_channel *macro_chan, const void *connected_info, int is_caller, int is_frame)
 {
 	static int deprecation_warning = 0;
@@ -10335,7 +10365,9 @@
 	}
 	ast_channel_unlock(macro_chan);
 
+	channel_set_intercept_mode(1);
 	retval = ast_app_run_macro(autoservice_chan, macro_chan, macro, macro_args);
+	channel_set_intercept_mode(0);
 	if (!retval) {
 		struct ast_party_connected_line saved_connected;
 
@@ -10385,7 +10417,9 @@
 	}
 	ast_channel_unlock(macro_chan);
 
+	channel_set_intercept_mode(1);
 	retval = ast_app_run_macro(autoservice_chan, macro_chan, macro, macro_args);
+	channel_set_intercept_mode(0);
 	if (!retval) {
 		struct ast_party_redirecting saved_redirecting;
 
@@ -10428,7 +10462,9 @@
 	}
 	ast_channel_unlock(sub_chan);
 
+	channel_set_intercept_mode(1);
 	retval = ast_app_run_sub(autoservice_chan, sub_chan, sub, sub_args, 0);
+	channel_set_intercept_mode(0);
 	if (!retval) {
 		struct ast_party_connected_line saved_connected;
 
@@ -10471,7 +10507,9 @@
 	}
 	ast_channel_unlock(sub_chan);
 
+	channel_set_intercept_mode(1);
 	retval = ast_app_run_sub(autoservice_chan, sub_chan, sub, sub_args, 0);
+	channel_set_intercept_mode(0);
 	if (!retval) {
 		struct ast_party_redirecting saved_redirecting;
 
diff --git a/res/res_agi.c b/res/res_agi.c
index 5e047d4..557f349 100644
--- a/res/res_agi.c
+++ b/res/res_agi.c
@@ -4073,7 +4073,7 @@
 			break;
 		}
 	} else if (c) {
-		ami_res = "Command Not Permitted on a dead channel";
+		ami_res = "Command Not Permitted on a dead channel or intercept routine";
 		resultcode = 511;
 
 		ast_agi_send(agi->fd, chan, "%d %s\n", resultcode, ami_res);
@@ -4109,6 +4109,8 @@
 	const char *sighup_str;
 	const char *exit_on_hangup_str;
 	int exit_on_hangup;
+	/*! Running in an interception routine is like DeadAGI mode.  No touchy the channel frames. */
+	int in_intercept = ast_channel_get_intercept_mode();
 
 	ast_channel_lock(chan);
 	sighup_str = pbx_builtin_getvar_helper(chan, "AGISIGHUP");
@@ -4143,7 +4145,7 @@
 			}
 		}
 		ms = -1;
-		if (dead) {
+		if (dead || in_intercept) {
 			c = ast_waitfor_nandfds(&chan, 0, &agi->ctrl, 1, NULL, &outfd, &ms);
 		} else if (!ast_check_hangup(chan)) {
 			c = ast_waitfor_nandfds(&chan, 1, &agi->ctrl, 1, NULL, &outfd, &ms);
@@ -4221,10 +4223,10 @@
 
 			if (agidebug)
 				ast_verbose("<%s>AGI Rx << %s\n", ast_channel_name(chan), buf);
-			cmd_status = agi_handle_command(chan, agi, buf, dead);
+			cmd_status = agi_handle_command(chan, agi, buf, dead || in_intercept);
 			switch (cmd_status) {
 			case AGI_RESULT_FAILURE:
-				if (dead || !ast_check_hangup(chan)) {
+				if (dead || in_intercept || !ast_check_hangup(chan)) {
 					/* The failure was not because of a hangup. */
 					returnstatus = AGI_RESULT_FAILURE;
 				}

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I638f147ca7a7f2590d7194a8ef4090eb191e4e43
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>



More information about the asterisk-code-review mailing list