[asterisk-dev] [Code Review] 4382: stasis bridge: handle early hangup of swap channel

[Matt Jordan]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4382/#review14340
-----------------------------------------------------------



/branches/13/res/stasis/stasis_bridge.c
<https://reviewboard.asterisk.org/r/4382/#comment24845>

    You may want to consider the usage of a goto here (gasp!) to reduce indentation:
    
    if (!bridge_channel->swap) {
        goto return_base_impl;
    }
    
    ...
    
    return_base_impl:
    
    return ast_bridge_base_v_table.push_peek(self, bridge_channel, swap);



/branches/13/res/stasis/stasis_bridge.c
<https://reviewboard.asterisk.org/r/4382/#comment24846>

    Same idea here as well:
    
    struct stasis_app_control *swap_control = stasis_app_control_find_by_channel(bridge_channel->swap);
    
    if (!swap_control) {
        goto return_base_impl;
    }
    
    ...
    
    return_base_impl:
    
    return ast_bridge_base_v_table.push_peek(self, bridge_channel, swap);



/branches/13/res/stasis/stasis_bridge.c
<https://reviewboard.asterisk.org/r/4382/#comment24847>

    If you bounce away early here when swap_control is NULL, use ao2_ref instead of ao2_cleanup.


- Matt Jordan


On Jan. 27, 2015, 8:55 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4382/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2015, 8:55 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24649
>     https://issues.asterisk.org/jira/browse/ASTERISK-24649
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> During an attended transfer of a channel in stasis, where a local channel is used to replace (swap in for) a PJSIP channel, there is a race condition.  The PJSIP REFER transfer has been initiated (ast_bridge_impart) but will be completed later on another thread (bridge_channel_ind_thread), however, prior to the stasis bridging callback (bridge_stasis_push) being called the PJSIP channel is hungup, which allows the stasis app loop to exit, deleting the stasis control.  That prevents the stasis bridge callback from getting the app name from the channel being replaced (swap).
> 
> This patch adds a new stasis bridge callback push_peek, which is called from the ast_bridge_impart() thread.  This allows the new bridge_stasis_push_peek() function to copy the stasis app name from the originating channel before the PJSIP channel can be hungup.
> 
> 
> Diffs
> -----
> 
>   /branches/13/res/stasis/stasis_bridge.c 431242 
>   /branches/13/main/bridge_channel.c 431242 
>   /branches/13/main/bridge.c 431242 
>   /branches/13/include/asterisk/bridge.h 431242 
> 
> Diff: https://reviewboard.asterisk.org/r/4382/diff/
> 
> 
> Testing
> -------
> 
> Running tests/rest_api/external_interaction/attended_transfer/stasis_bridge_to_stasis_app now does not result in an occasional failure.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150128/3650f5b6/attachment-0001.html>