Merge changes from topic 'ASTERISK-27625'

* changes:
  channel.c: Make CHECK_BLOCKING() save thread LWP id for messages.
  channel.c: Fix usage of CHECK_BLOCKING()
  autoservice: Don't start channel autoservice if the thread is a user interface.

include/asterisk/channel.h

@@ -2710,15 +2710,31 @@ static inline enum ast_t38_state ast_channel_get_t38_state(struct ast_channel *c
 	return state;
 }
 
-#define CHECK_BLOCKING(c) do { 	 \
-	if (ast_test_flag(ast_channel_flags(c), AST_FLAG_BLOCKING)) {\
-		ast_debug(1, "Thread %p is blocking '%s', already blocked by thread %p in procedure %s\n", \
-			(void *) pthread_self(), ast_channel_name(c), (void *) ast_channel_blocker(c), ast_channel_blockproc(c)); \
-	} else { \
+/*!
+ * \brief Set the blocking indication on the channel.
+ *
+ * \details
+ * Indicate that the thread handling the channel is about to do a blocking
+ * operation to wait for media on the channel.  (poll, read, or write)
+ *
+ * Masquerading and ast_queue_frame() use this indication to wake up the thread.
+ *
+ * \pre The channel needs to be locked
+ */
+#define CHECK_BLOCKING(c) \
+	do { \
+		if (ast_test_flag(ast_channel_flags(c), AST_FLAG_BLOCKING)) { \
+			/* This should not happen as there should only be one thread handling a channel's media at a time. */ \
+			ast_log(LOG_DEBUG, "Thread LWP %d is blocking '%s', already blocked by thread LWP %d in procedure %s\n", \
+				ast_get_tid(), ast_channel_name(c), \
+				ast_channel_blocker_tid(c), ast_channel_blockproc(c)); \
+			ast_assert(0); \
+		} \
+		ast_channel_blocker_tid_set((c), ast_get_tid()); \
 		ast_channel_blocker_set((c), pthread_self()); \
 		ast_channel_blockproc_set((c), __PRETTY_FUNCTION__); \
 		ast_set_flag(ast_channel_flags(c), AST_FLAG_BLOCKING); \
-	} } while (0)
+	} while (0)
 
 ast_group_t ast_get_group(const char *s);
 
@@ -4354,6 +4370,9 @@ int ast_channel_fd_add(struct ast_channel *chan, int value);
 pthread_t ast_channel_blocker(const struct ast_channel *chan);
 void ast_channel_blocker_set(struct ast_channel *chan, pthread_t value);
 
+int ast_channel_blocker_tid(const struct ast_channel *chan);
+void ast_channel_blocker_tid_set(struct ast_channel *chan, int tid);
+
 ast_timing_func_t ast_channel_timingfunc(const struct ast_channel *chan);
 void ast_channel_timingfunc_set(struct ast_channel *chan, ast_timing_func_t value);
 

include/asterisk/utils.h

@@ -913,4 +913,22 @@ enum ast_fd_flag_operation {
 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 */

main/autoservice.c

@@ -202,6 +202,13 @@ int ast_autoservice_start(struct ast_channel *chan)
 	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 @@ int ast_autoservice_stop(struct ast_channel *chan)
 	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

main/channel.c

@@ -2618,10 +2618,10 @@ void ast_hangup(struct ast_channel *chan)
 	ast_channel_generator_set(chan, NULL);
 
 	if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING)) {
-		ast_log(LOG_WARNING, "Hard hangup called by thread %ld on %s, while fd "
-			"is blocked by thread %ld in procedure %s!  Expect a failure\n",
-			(long) pthread_self(), ast_channel_name(chan), (long)ast_channel_blocker(chan), ast_channel_blockproc(chan));
-		ast_assert(ast_test_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING) == 0);
+		ast_log(LOG_WARNING, "Hard hangup called by thread LWP %d on %s, while blocked by thread LWP %d in procedure %s!  Expect a failure\n",
+			ast_get_tid(), ast_channel_name(chan), ast_channel_blocker_tid(chan),
+			ast_channel_blockproc(chan));
+		ast_assert(0);
 	}
 
 	if (ast_channel_tech(chan)->hangup) {
@@ -3661,7 +3661,6 @@ static struct ast_frame *__ast_read(struct ast_channel *chan, int dropaudio, int
 			}
 		}
 	} else {
-		ast_channel_blocker_set(chan, pthread_self());
 		if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_EXCEPTION)) {
 			if (ast_channel_tech(chan)->exception)
 				f = ast_channel_tech(chan)->exception(chan);
@@ -5033,11 +5032,9 @@ int ast_write_stream(struct ast_channel *chan, int stream_num, struct ast_frame
 				/* There is a generator running while we're in the middle of a digit.
 				 * It's probably inband DTMF, so go ahead and pass it so it can
 				 * stop the generator */
-				ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
 				ast_channel_unlock(chan);
 				res = ast_senddigit_end(chan, fr->subclass.integer, fr->len);
 				ast_channel_lock(chan);
-				CHECK_BLOCKING(chan);
 			} else if (fr->frametype == AST_FRAME_CONTROL
 				&& fr->subclass.integer == AST_CONTROL_UNHOLD) {
 				/*
@@ -5055,7 +5052,6 @@ int ast_write_stream(struct ast_channel *chan, int stream_num, struct ast_frame
 	/* High bit prints debugging */
 	if (ast_channel_fout(chan) & DEBUGCHAN_FLAG)
 		ast_frame_dump(ast_channel_name(chan), fr, ">>");
-	CHECK_BLOCKING(chan);
 	switch (fr->frametype) {
 	case AST_FRAME_CONTROL:
 		indicate_data_internal(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);
@@ -5069,11 +5065,9 @@ int ast_write_stream(struct ast_channel *chan, int stream_num, struct ast_frame
 				f = fr;
 		}
 		send_dtmf_begin_event(chan, DTMF_SENT, fr->subclass.integer);
-		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
 		ast_channel_unlock(chan);
 		res = ast_senddigit_begin(chan, fr->subclass.integer);
 		ast_channel_lock(chan);
-		CHECK_BLOCKING(chan);
 		break;
 	case AST_FRAME_DTMF_END:
 		if (ast_channel_audiohooks(chan)) {
@@ -5085,13 +5079,12 @@ int ast_write_stream(struct ast_channel *chan, int stream_num, struct ast_frame
 			}
 		}
 		send_dtmf_end_event(chan, DTMF_SENT, fr->subclass.integer, fr->len);
-		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
 		ast_channel_unlock(chan);
 		res = ast_senddigit_end(chan, fr->subclass.integer, fr->len);
 		ast_channel_lock(chan);
-		CHECK_BLOCKING(chan);
 		break;
 	case AST_FRAME_TEXT:
+		CHECK_BLOCKING(chan);
 		if (ast_format_cmp(fr->subclass.format, ast_format_t140) == AST_FORMAT_CMP_EQUAL) {
 			res = (ast_channel_tech(chan)->write_text == NULL) ? 0 :
 				ast_channel_tech(chan)->write_text(chan, fr);
@@ -5099,13 +5092,17 @@ int ast_write_stream(struct ast_channel *chan, int stream_num, struct ast_frame
 			res = (ast_channel_tech(chan)->send_text == NULL) ? 0 :
 				ast_channel_tech(chan)->send_text(chan, (char *) fr->data.ptr);
 		}
+		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
 		break;
 	case AST_FRAME_HTML:
+		CHECK_BLOCKING(chan);
 		res = (ast_channel_tech(chan)->send_html == NULL) ? 0 :
 			ast_channel_tech(chan)->send_html(chan, fr->subclass.integer, (char *) fr->data.ptr, fr->datalen);
+		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
 		break;
 	case AST_FRAME_VIDEO:
 		/* XXX Handle translation of video codecs one day XXX */
+		CHECK_BLOCKING(chan);
 		if (ast_channel_tech(chan)->write_stream) {
 			if (stream) {
 				res = ast_channel_tech(chan)->write_stream(chan, ast_stream_get_position(stream), fr);
@@ -5117,8 +5114,10 @@ int ast_write_stream(struct ast_channel *chan, int stream_num, struct ast_frame
 		} else {
 			res = 0;
 		}
+		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
 		break;
 	case AST_FRAME_MODEM:
+		CHECK_BLOCKING(chan);
 		if (ast_channel_tech(chan)->write_stream) {
 			if (stream) {
 				res = ast_channel_tech(chan)->write_stream(chan, ast_stream_get_position(stream), fr);
@@ -5130,6 +5129,7 @@ int ast_write_stream(struct ast_channel *chan, int stream_num, struct ast_frame
 		} else {
 			res = 0;
 		}
+		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
 		break;
 	case AST_FRAME_VOICE:
 		if (ast_opt_generic_plc && ast_format_cmp(fr->subclass.format, ast_format_slin) == AST_FORMAT_CMP_EQUAL) {
@@ -5179,15 +5179,15 @@ int ast_write_stream(struct ast_channel *chan, int stream_num, struct ast_frame
 
 			if (ast_channel_writetrans(chan)) {
 				struct ast_frame *trans_frame = ast_translate(ast_channel_writetrans(chan), f, 0);
-                               if (trans_frame != f && f != fr) {
-                                       /*
-                                        * If translate gives us a new frame and so did the audio
-                                        * hook then we need to free the one from the audio hook.
-                                        */
-                                       ast_frfree(f);
-                               }
-                               f = trans_frame;
-                       }
+				if (trans_frame != f && f != fr) {
+					/*
+					 * If translate gives us a new frame and so did the audio
+					 * hook then we need to free the one from the audio hook.
+					 */
+					ast_frfree(f);
+				}
+				f = trans_frame;
+			}
 		}
 
 		if (!f) {
@@ -5285,6 +5285,7 @@ int ast_write_stream(struct ast_channel *chan, int stream_num, struct ast_frame
 		/* the translator on chan->writetrans may have returned multiple frames
 		   from the single frame we passed in; if so, feed each one of them to the
 		   channel, freeing each one after it has been written */
+		CHECK_BLOCKING(chan);
 		if ((f != fr) && AST_LIST_NEXT(f, frame_list)) {
 			struct ast_frame *cur, *next = NULL;
 			unsigned int skip = 0;
@@ -5323,6 +5324,7 @@ int ast_write_stream(struct ast_channel *chan, int stream_num, struct ast_frame
 				res = 0;
 			}
 		}
+		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
 		break;
 	case AST_FRAME_NULL:
 	case AST_FRAME_IAX:
@@ -5331,23 +5333,26 @@ int ast_write_stream(struct ast_channel *chan, int stream_num, struct ast_frame
 		break;
 	case AST_FRAME_RTCP:
 		/* RTCP information is on a per-stream basis and only available on multistream capable channels */
+		CHECK_BLOCKING(chan);
 		if (ast_channel_tech(chan)->write_stream && stream) {
 			res = ast_channel_tech(chan)->write_stream(chan, ast_stream_get_position(stream), fr);
 		} else {
 			res = 0;
 		}
+		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
 		break;
 	default:
 		/* At this point, fr is the incoming frame and f is NULL.  Channels do
 		 * not expect to get NULL as a frame pointer and will segfault.  Hence,
 		 * we output the original frame passed in. */
+		CHECK_BLOCKING(chan);
 		res = ast_channel_tech(chan)->write(chan, fr);
+		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
 		break;
 	}
 
 	if (f && f != fr)
 		ast_frfree(f);
-	ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
 
 	/* Consider a write failure to force a soft hangup */
 	if (res < 0) {

main/channel_internal_api.c

@@ -165,6 +165,7 @@ struct ast_channel {
 	unsigned long insmpl;				/*!< Track the read/written samples for monitor use */
 	unsigned long outsmpl;				/*!< Track the read/written samples for monitor use */
 
+	int blocker_tid;					/*!< If anyone is blocking, this is their thread id */
 	AST_VECTOR(, int) fds;				/*!< File descriptors for channel -- Drivers will poll on
 							 *   these file descriptors, so at least one must be non -1.
 							 *   See \arg \ref AstFileDesc */
@@ -1178,6 +1179,15 @@ void ast_channel_blocker_set(struct ast_channel *chan, pthread_t value)
 	chan->blocker = value;
 }
 
+int ast_channel_blocker_tid(const struct ast_channel *chan)
+{
+	return chan->blocker_tid;
+}
+void ast_channel_blocker_tid_set(struct ast_channel *chan, int value)
+{
+	chan->blocker_tid = value;
+}
+
 ast_timing_func_t ast_channel_timingfunc(const struct ast_channel *chan)
 {
 	return chan->timingfunc;

main/tcptls.c

@@ -134,6 +134,19 @@ static void *handle_tcptls_connection(void *data)
 		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) {

main/utils.c

@@ -2750,3 +2750,38 @@ int __ast_fd_set_flags(int fd, int flags, enum ast_fd_flag_operation op,
 
 	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;
+}