From f82d0a9380b4ae707b0992f5a6b32b0ce131d52c Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Tue, 12 Jun 2018 14:09:54 -0500 Subject: [PATCH] channel.c: Fix usage of CHECK_BLOCKING() The CHECK_BLOCKING() macro is used to indicate if a channel's handling thread is about to do a blocking operation (poll, read, or write) of media. A few operations such as ast_queue_frame(), soft hangup, and masquerades use the indication to wake up the blocked thread to reevaluate what is going on. ASTERISK-27625 Change-Id: I4dfc33e01e60627d962efa29d0a4244cf151a84d --- include/asterisk/channel.h | 27 +++++++++++++++++++------ main/channel.c | 40 ++++++++++++++++++++++---------------- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h index 16b8a7be5c..d0c0dbf270 100644 --- a/include/asterisk/channel.h +++ b/include/asterisk/channel.h @@ -2758,15 +2758,30 @@ 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 %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)); \ + ast_assert(0); \ + } \ 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); diff --git a/main/channel.c b/main/channel.c index a29c9683e6..6386de2dc9 100644 --- a/main/channel.c +++ b/main/channel.c @@ -5055,11 +5055,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) { /* @@ -5077,7 +5075,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); @@ -5091,11 +5088,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)) { @@ -5107,13 +5102,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); @@ -5121,13 +5115,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); @@ -5139,8 +5137,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); @@ -5152,6 +5152,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) { @@ -5201,15 +5202,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) { @@ -5307,6 +5308,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; @@ -5345,6 +5347,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: @@ -5353,23 +5356,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) { -- 2.47.2