Merge changes from topic 'ASTERISK-27625'
authorGeorge Joseph <gjoseph@digium.com>
Thu, 21 Jun 2018 15:26:31 +0000 (10:26 -0500)
committerGerrit Code Review <gerrit2@gerrit.digium.api>
Thu, 21 Jun 2018 15:26:31 +0000 (10:26 -0500)
* 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
include/asterisk/utils.h
main/autoservice.c
main/channel.c
main/channel_internal_api.c
main/tcptls.c
main/utils.c

index 91aed34..3f22cdd 100644 (file)
@@ -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);
 
index f1607ff..f459b31 100644 (file)
@@ -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 */
index cd7388b..8ff2cb5 100644 (file)
@@ -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
index 9d5d55e..094658d 100644 (file)
@@ -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) {
index d31ce94..b926514 100644 (file)
@@ -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;
index f555730..3ba52ff 100644 (file)
@@ -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) {
index 6ecd2b8..2eaf1f9 100644 (file)
@@ -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;
+}