[asterisk-dev] [Code Review] 4341: stasis transfers: fix race condition on set replace channel

[rmudgett]

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


I think the changes to res/stasis/app.c need to be reverted.  That information seems to be used for the AMI transfer events, the stasis transfer message JSON blobs, and app_queue's queue_log.


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

    space after comma 3,"Copying...



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

    May need to add this call here to ensure that the swap channel leaves the bridge:
    ast_bridge_channel_leave_bridge(swap, BRIDGE_CHANNEL_STATE_END_NO_DISSOLVE, 0);
    
    Have to test to see if it is needed.


- rmudgett


On Jan. 16, 2015, 3:58 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4341/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2015, 3:58 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24649
>     https://issues.asterisk.org/jira/browse/ASTERISK-24649
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> After a transfer completes that uses a local replacement channel, stasis receives the stasis transfer message with the details of the transfer and makes changes on the replacement channel.  However, since a separate thread was already started for the purpose of starting stasis on the new replacement channel, this allowed for a race condition.  Occasionally later then normal arrival of the stasis transfer message would result in the stasis app name not being set on the replacement channel before it was needed by the other thread, causing it to fail to start stasis and hang up.
> 
> This change moves the operations (assignment of the stasis app name and setting the replacement snapshot on the new channel) into the bridge_stasis_push() callback from the bridge transfer logic.  This allows these steps to be completed earlier and more deterministically, eliminating the race condition.
> 
> 
> Diffs
> -----
> 
>   /branches/13/res/stasis/stasis_bridge.c 430394 
>   /branches/13/res/stasis/app.c 430394 
> 
> Diff: https://reviewboard.asterisk.org/r/4341/diff/
> 
> 
> Testing
> -------
> 
> The stasis start/end tests that discovered the issue are now passing, and I've not found any other test failures.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

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