<p>Richard Mudgett has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/9212">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">channel.c: Fix usage of CHECK_BLOCKING()<br><br>The CHECK_BLOCKING() macro is used to indicate if a channel's handling<br>thread is about to do a blocking operation (poll, read, or write) of<br>media. A few operations such as ast_queue_frame(), soft hangup, and<br>masquerades use the indication to wake up the blocked thread to reevaluate<br>what is going on.<br><br>ASTERISK-27625<br><br>Change-Id: I4dfc33e01e60627d962efa29d0a4244cf151a84d<br>---<br>M include/asterisk/channel.h<br>M main/channel.c<br>2 files changed, 44 insertions(+), 17 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/12/9212/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h<br>index 0fc236a..8c171b6 100644<br>--- a/include/asterisk/channel.h<br>+++ b/include/asterisk/channel.h<br>@@ -2649,15 +2649,30 @@<br> return state;<br> }<br> <br>-#define CHECK_BLOCKING(c) do { \<br>- if (ast_test_flag(ast_channel_flags(c), AST_FLAG_BLOCKING)) {\<br>- ast_debug(1, "Thread %p is blocking '%s', already blocked by thread %p in procedure %s\n", \<br>- (void *) pthread_self(), ast_channel_name(c), (void *) ast_channel_blocker(c), ast_channel_blockproc(c)); \<br>- } else { \<br>+/*!<br>+ * \brief Set the blocking indication on the channel.<br>+ *<br>+ * \details<br>+ * Indicate that the thread handling the channel is about to do a blocking<br>+ * operation to wait for media on the channel. (poll, read, or write)<br>+ *<br>+ * Masquerading and ast_queue_frame() use this indication to wake up the thread.<br>+ *<br>+ * \pre The channel needs to be locked<br>+ */<br>+#define CHECK_BLOCKING(c) \<br>+ do { \<br>+ if (ast_test_flag(ast_channel_flags(c), AST_FLAG_BLOCKING)) { \<br>+ /* This should not happen as there should only be one thread handling a channel's media at a time. */ \<br>+ ast_log(LOG_DEBUG, "Thread %p is blocking '%s', already blocked by thread %p in procedure %s\n", \<br>+ (void *) pthread_self(), ast_channel_name(c), \<br>+ (void *) ast_channel_blocker(c), ast_channel_blockproc(c)); \<br>+ ast_assert(0); \<br>+ } \<br> ast_channel_blocker_set((c), pthread_self()); \<br> ast_channel_blockproc_set((c), __PRETTY_FUNCTION__); \<br> ast_set_flag(ast_channel_flags(c), AST_FLAG_BLOCKING); \<br>- } } while (0)<br>+ } while (0)<br> <br> ast_group_t ast_get_group(const char *s);<br> <br>diff --git a/main/channel.c b/main/channel.c<br>index 27a959a..2a72aee 100644<br>--- a/main/channel.c<br>+++ b/main/channel.c<br>@@ -3266,10 +3266,10 @@<br> }<br> }<br> <br>- ast_channel_unlock(chan);<br>-<br> /* Time to make this channel block... */<br> CHECK_BLOCKING(chan);<br>+<br>+ ast_channel_unlock(chan);<br> <br> if (*ms > 0) {<br> start = ast_tvnow();<br>@@ -3279,7 +3279,9 @@<br> res = epoll_wait(ast_channel_epfd(chan), ev, 1, rms);<br> <br> /* Stop blocking */<br>+ ast_channel_lock(chan);<br> ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);<br>+ ast_channel_unlock(chan);<br> <br> /* Simulate a timeout if we were interrupted */<br> if (res < 0) {<br>@@ -3305,12 +3307,14 @@<br> <br> /* See what events are pending */<br> aed = ev[0].data.ptr;<br>+ ast_channel_lock(chan);<br> ast_channel_fdno_set(chan, aed->which);<br> if (ev[0].events & EPOLLPRI) {<br> ast_set_flag(ast_channel_flags(chan), AST_FLAG_EXCEPTION);<br> } else {<br> ast_clear_flag(ast_channel_flags(chan), AST_FLAG_EXCEPTION);<br> }<br>+ ast_channel_unlock(chan);<br> <br> if (*ms > 0) {<br> *ms -= ast_tvdiff_ms(ast_tvnow(), start);<br>@@ -3346,8 +3350,8 @@<br> whentohangup = diff;<br> }<br> }<br>- ast_channel_unlock(c[i]);<br> CHECK_BLOCKING(c[i]);<br>+ ast_channel_unlock(c[i]);<br> }<br> <br> rms = *ms;<br>@@ -3365,7 +3369,9 @@<br> res = epoll_wait(ast_channel_epfd(c[0]), ev, 25, rms);<br> <br> for (i = 0; i < n; i++) {<br>+ ast_channel_lock(c[i]);<br> ast_clear_flag(ast_channel_flags(c[i]), AST_FLAG_BLOCKING);<br>+ ast_channel_unlock(c[i]);<br> }<br> <br> if (res < 0) {<br>@@ -3400,12 +3406,14 @@<br> }<br> <br> winner = aed->chan;<br>+ ast_channel_lock(winner);<br> if (ev[i].events & EPOLLPRI) {<br> ast_set_flag(ast_channel_flags(winner), AST_FLAG_EXCEPTION);<br> } else {<br> ast_clear_flag(ast_channel_flags(winner), AST_FLAG_EXCEPTION);<br> }<br> ast_channel_fdno_set(winner, aed->which);<br>+ ast_channel_unlock(winner);<br> }<br> <br> if (*ms > 0) {<br>@@ -5220,11 +5228,9 @@<br> /* There is a generator running while we're in the middle of a digit.<br> * It's probably inband DTMF, so go ahead and pass it so it can<br> * stop the generator */<br>- ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);<br> ast_channel_unlock(chan);<br> res = ast_senddigit_end(chan, fr->subclass.integer, fr->len);<br> ast_channel_lock(chan);<br>- CHECK_BLOCKING(chan);<br> } else if (fr->frametype == AST_FRAME_CONTROL<br> && fr->subclass.integer == AST_CONTROL_UNHOLD) {<br> /*<br>@@ -5242,7 +5248,6 @@<br> /* High bit prints debugging */<br> if (ast_channel_fout(chan) & DEBUGCHAN_FLAG)<br> ast_frame_dump(ast_channel_name(chan), fr, ">>");<br>- CHECK_BLOCKING(chan);<br> switch (fr->frametype) {<br> case AST_FRAME_CONTROL:<br> indicate_data_internal(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);<br>@@ -5256,11 +5261,9 @@<br> f = fr;<br> }<br> send_dtmf_begin_event(chan, DTMF_SENT, fr->subclass.integer);<br>- ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);<br> ast_channel_unlock(chan);<br> res = ast_senddigit_begin(chan, fr->subclass.integer);<br> ast_channel_lock(chan);<br>- CHECK_BLOCKING(chan);<br> break;<br> case AST_FRAME_DTMF_END:<br> if (ast_channel_audiohooks(chan)) {<br>@@ -5272,13 +5275,12 @@<br> }<br> }<br> send_dtmf_end_event(chan, DTMF_SENT, fr->subclass.integer, fr->len);<br>- ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);<br> ast_channel_unlock(chan);<br> res = ast_senddigit_end(chan, fr->subclass.integer, fr->len);<br> ast_channel_lock(chan);<br>- CHECK_BLOCKING(chan);<br> break;<br> case AST_FRAME_TEXT:<br>+ CHECK_BLOCKING(chan);<br> if (ast_format_cmp(fr->subclass.format, ast_format_t140) == AST_FORMAT_CMP_EQUAL) {<br> res = (ast_channel_tech(chan)->write_text == NULL) ? 0 :<br> ast_channel_tech(chan)->write_text(chan, fr);<br>@@ -5286,19 +5288,26 @@<br> res = (ast_channel_tech(chan)->send_text == NULL) ? 0 :<br> ast_channel_tech(chan)->send_text(chan, (char *) fr->data.ptr);<br> }<br>+ ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);<br> break;<br> case AST_FRAME_HTML:<br>+ CHECK_BLOCKING(chan);<br> res = (ast_channel_tech(chan)->send_html == NULL) ? 0 :<br> ast_channel_tech(chan)->send_html(chan, fr->subclass.integer, (char *) fr->data.ptr, fr->datalen);<br>+ ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);<br> break;<br> case AST_FRAME_VIDEO:<br> /* XXX Handle translation of video codecs one day XXX */<br>+ CHECK_BLOCKING(chan);<br> res = (ast_channel_tech(chan)->write_video == NULL) ? 0 :<br> ast_channel_tech(chan)->write_video(chan, fr);<br>+ ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);<br> break;<br> case AST_FRAME_MODEM:<br>+ CHECK_BLOCKING(chan);<br> res = (ast_channel_tech(chan)->write == NULL) ? 0 :<br> ast_channel_tech(chan)->write(chan, fr);<br>+ ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);<br> break;<br> case AST_FRAME_VOICE:<br> if (ast_channel_tech(chan)->write == NULL)<br>@@ -5459,6 +5468,7 @@<br> /* the translator on chan->writetrans may have returned multiple frames<br> from the single frame we passed in; if so, feed each one of them to the<br> channel, freeing each one after it has been written */<br>+ CHECK_BLOCKING(chan);<br> if ((f != fr) && AST_LIST_NEXT(f, frame_list)) {<br> struct ast_frame *cur, *next = NULL;<br> unsigned int skip = 0;<br>@@ -5487,6 +5497,7 @@<br> } else {<br> res = ast_channel_tech(chan)->write(chan, f);<br> }<br>+ ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);<br> break;<br> case AST_FRAME_NULL:<br> case AST_FRAME_IAX:<br>@@ -5497,13 +5508,14 @@<br> /* At this point, fr is the incoming frame and f is NULL. Channels do<br> * not expect to get NULL as a frame pointer and will segfault. Hence,<br> * we output the original frame passed in. */<br>+ CHECK_BLOCKING(chan);<br> res = ast_channel_tech(chan)->write(chan, fr);<br>+ ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);<br> break;<br> }<br> <br> if (f && f != fr)<br> ast_frfree(f);<br>- ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);<br> <br> /* Consider a write failure to force a soft hangup */<br> if (res < 0) {<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/9212">change 9212</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/9212"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I4dfc33e01e60627d962efa29d0a4244cf151a84d </div>
<div style="display:none"> Gerrit-Change-Number: 9212 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>