[Asterisk-code-review] autoservice: Don't start channel autoservice if the thread i... (asterisk[master])

[George Joseph]

George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/9221 )

Change subject: autoservice: Don't start channel autoservice if the thread is a user interface.
......................................................................

autoservice: Don't start channel autoservice if the thread is a user interface.

Executing dialplan functions from either AMI or ARI by getting a variable
could place the channel into autoservice.  However, these user interface
threads do not handle the channel's media so we wind up with two threads
attempting to handle the media.

There can be one and only one thread handling a channel's media at a time.
Otherwise, we don't know which thread is going to handle the media frames.

ASTERISK-27625

Change-Id: If2dc94ce15ddabf923ed1e2a65ea0ef56e013e49
---
M include/asterisk/utils.h
M main/autoservice.c
M main/tcptls.c
M main/utils.c
4 files changed, 80 insertions(+), 0 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
  Sean Bright: Looks good to me, approved



diff --git a/include/asterisk/utils.h b/include/asterisk/utils.h
index b892cda..6f9a11e 100644
--- a/include/asterisk/utils.h
+++ b/include/asterisk/utils.h
@@ -913,4 +913,22 @@
 int __ast_fd_set_flags(int fd, int flags, enum ast_fd_flag_operation op,
 	const char *file, int lineno, const char *function);
 
+/*!
+ * \brief Set the current thread's user interface status.
+ *
+ * \param is_user_interface Non-zero to mark the thread as a user interface.
+ *
+ * \return 0 if successfuly marked current thread.
+ * \return Non-zero if marking current thread failed.
+ */
+int ast_thread_user_interface_set(int is_user_interface);
+
+/*!
+ * \brief Indicates whether the current thread is a user interface
+ *
+ * \return True (non-zero) if thread is a user interface.
+ * \return False (zero) if thread is not a user interface.
+ */
+int ast_thread_is_user_interface(void);
+
 #endif /* _ASTERISK_UTILS_H */
diff --git a/main/autoservice.c b/main/autoservice.c
index cd7388b..8ff2cb5 100644
--- a/main/autoservice.c
+++ b/main/autoservice.c
@@ -202,6 +202,13 @@
 	int res = 0;
 	struct asent *as;
 
+	if (ast_thread_is_user_interface()) {
+		/* User interface threads do not handle channel media. */
+		ast_debug(1, "Thread is a user interface, not putting channel %s into autoservice\n",
+			ast_channel_name(chan));
+		return 0;
+	}
+
 	AST_LIST_LOCK(&aslist);
 	AST_LIST_TRAVERSE(&aslist, as, list) {
 		if (as->chan == chan) {
@@ -263,6 +270,13 @@
 	struct ast_frame *f;
 	int chan_list_state;
 
+	if (ast_thread_is_user_interface()) {
+		/* User interface threads do not handle channel media. */
+		ast_debug(1, "Thread is a user interface, not removing channel %s from autoservice\n",
+			ast_channel_name(chan));
+		return 0;
+	}
+
 	AST_LIST_LOCK(&aslist);
 
 	/* Save the autoservice channel list state.  We _must_ verify that the channel
diff --git a/main/tcptls.c b/main/tcptls.c
index f555730..3ba52ff 100644
--- a/main/tcptls.c
+++ b/main/tcptls.c
@@ -134,6 +134,19 @@
 		return NULL;
 	}
 
+	/*
+	 * TCP/TLS connections are associated with external protocols which can
+	 * be considered to be user interfaces (even for SIP messages), and
+	 * will not handle channel media.  This may need to be pushed down into
+	 * the individual protocol handlers, but this seems like a good start.
+	 */
+	if (ast_thread_user_interface_set(1)) {
+		ast_log(LOG_ERROR, "Failed to set user interface status; killing connection\n");
+		ast_tcptls_close_session_file(tcptls_session);
+		ao2_ref(tcptls_session, -1);
+		return NULL;
+	}
+
 	if (tcptls_session->parent->tls_cfg) {
 #ifdef DO_SSL
 		if (ast_iostream_start_tls(&tcptls_session->stream, tcptls_session->parent->tls_cfg->ssl_ctx, tcptls_session->client) < 0) {
diff --git a/main/utils.c b/main/utils.c
index 6ecd2b8..2eaf1f9 100644
--- a/main/utils.c
+++ b/main/utils.c
@@ -2750,3 +2750,38 @@
 
 	return 0;
 }
+
+/*!
+ * \brief A thread local indicating whether the current thread is a user interface.
+ */
+AST_THREADSTORAGE(thread_user_interface_tl);
+
+int ast_thread_user_interface_set(int is_user_interface)
+{
+	int *thread_user_interface;
+
+	thread_user_interface = ast_threadstorage_get(
+		&thread_user_interface_tl, sizeof(*thread_user_interface));
+	if (thread_user_interface == NULL) {
+		ast_log(LOG_ERROR, "Error setting user interface status for current thread\n");
+		return -1;
+	}
+
+	*thread_user_interface = !!is_user_interface;
+	return 0;
+}
+
+int ast_thread_is_user_interface(void)
+{
+	int *thread_user_interface;
+
+	thread_user_interface = ast_threadstorage_get(
+		&thread_user_interface_tl, sizeof(*thread_user_interface));
+	if (thread_user_interface == NULL) {
+		ast_log(LOG_ERROR, "Error checking thread's user interface status\n");
+		/* On error, assume that we are not a user interface thread */
+		return 0;
+	}
+
+	return *thread_user_interface;
+}

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If2dc94ce15ddabf923ed1e2a65ea0ef56e013e49
Gerrit-Change-Number: 9221
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180621/45277da2/attachment-0001.html>