<p>George Joseph has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/6369">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">stasis/control:  Fix possible deadlock with swap channel<br><br>If an error occurs during a bridge impart it's possible that<br>the "bridge_after" callback might try to run before<br>control_swap_channel_in_bridge has been signalled to continue.<br>Since control_swap_channel_in_bridge is holding the control lock<br>and the callback needs it, a deadlock will occur.<br><br>* control_swap_channel_in_bridge now only holds the control<br>  lock while it's actually modifying the control structure and<br>  releases it while the bridge impart is running.<br>* bridge_after_cb is now tolerant of impart failures.<br><br>Change-Id: Ifd239aa93955b3eb475521f61e284fcb0da2c3b3<br>---<br>M res/stasis/control.c<br>1 file changed, 21 insertions(+), 8 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/69/6369/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/res/stasis/control.c b/res/stasis/control.c<br>index 503f111..92b8776 100644<br>--- a/res/stasis/control.c<br>+++ b/res/stasis/control.c<br>@@ -989,8 +989,12 @@<br>  SCOPED_AO2LOCK(lock, control);<br>        struct ast_bridge_channel *bridge_channel;<br> <br>+        /* control->bridge may be NULL because of a race with<br>+      * control_swap_channel_in_bridge if an error occurrs<br>+         * during the bridge impart.  We need to be tolerant of that.<br>+         */<br>   ast_debug(3, "%s, %s: Channel leaving bridge\n",<br>-           ast_channel_uniqueid(chan), control->bridge->uniqueid);<br>+                ast_channel_uniqueid(chan), control->bridge ? control->bridge->uniqueid : "FAILED");<br> <br>    ast_assert(chan == control->channel);<br> <br>@@ -1000,16 +1004,19 @@<br> <br>       app_unsubscribe_bridge(control->app, control->bridge);<br> <br>-      /* No longer in the bridge */<br>-        control->bridge = NULL;<br>-<br>         /* Get the bridge channel so we don't depart from the wrong bridge */<br>     ast_channel_lock(chan);<br>       bridge_channel = ast_channel_get_bridge_channel(chan);<br>        ast_channel_unlock(chan);<br> <br>- /* Depart this channel from the bridge using the command queue if possible */<br>-        stasis_app_send_command_async(control, bridge_channel_depart, bridge_channel, __ao2_cleanup);<br>+        if (control->bridge) {<br>+            /* Depart this channel from the bridge using the command queue if possible */<br>+                stasis_app_send_command_async(control, bridge_channel_depart, bridge_channel, __ao2_cleanup);<br>+        }<br>+<br>+ /* No longer in the bridge */<br>+        control->bridge = NULL;<br>+<br>         if (stasis_app_channel_is_stasis_end_published(chan)) {<br>               /* The channel has had a StasisEnd published on it, but until now had remained in<br>              * the bridging system. This means that the channel moved from a Stasis bridge to a<br>@@ -1191,25 +1198,31 @@<br>                  ast_channel_pbx_set(chan, NULL);<br>              }<br> <br>+         control->bridge = NULL;<br>+           /* We can't be holding the control lock while impart is running<br>+           * or we could create a deadlock with bridge_after_cb which also<br>+              * tries to lock control.<br>+             */<br>+          ao2_unlock(control);<br>          res = ast_bridge_impart(bridge,<br>                       chan,<br>                         swap,<br>                         NULL, /* features */<br>                  AST_BRIDGE_IMPART_CHAN_DEPARTABLE);<br>+          ao2_lock(control);<br>            if (res != 0) {<br>                       ast_log(LOG_ERROR, "Error adding channel to bridge\n");<br>                     ast_channel_pbx_set(chan, control->pbx);<br>                   control->pbx = NULL;<br>                       return -1;<br>            }<br>-<br>-         ast_assert(stasis_app_get_bridge(control) == NULL);<br>           control->bridge = bridge;<br> <br>               ast_channel_lock(chan);<br>               set_interval_hook(chan);<br>              ast_channel_unlock(chan);<br>     }<br>+<br>  return 0;<br> }<br> <br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6369">change 6369</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/6369"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Ifd239aa93955b3eb475521f61e284fcb0da2c3b3 </div>
<div style="display:none"> Gerrit-Change-Number: 6369 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>