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

Anonymous Coward asteriskteam at digium.com
Thu Feb 2 23:54:13 CST 2017


Anonymous Coward #1000019 has submitted this change and it was merged. ( https://gerrit.asterisk.org/4876 )

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 268c993..a57a0b4 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -4707,4 +4707,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 64ce4da..6923c3a 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -10268,6 +10268,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;
@@ -10303,7 +10333,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;
 
@@ -10353,7 +10385,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;
 
@@ -10396,7 +10430,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;
 
@@ -10439,7 +10475,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 cb23a07..d46a019 100644
--- a/res/res_agi.c
+++ b/res/res_agi.c
@@ -4075,7 +4075,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);
@@ -4111,6 +4111,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");
@@ -4145,7 +4147,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);
@@ -4223,10 +4225,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/4876
To unsubscribe, visit https://gerrit.asterisk.org/settings

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



More information about the asterisk-code-review mailing list