[asterisk-dev] [Code Review] 2965: stasis: Ensure channels are departed from bridges

[rmudgett]

> On Oct. 28, 2013, 6:42 p.m., rmudgett wrote:
> > /branches/12/res/stasis/control.c, lines 492-493
> > <https://reviewboard.asterisk.org/r/2965/diff/1/?file=47645#file47645line492>
> >
> >     I expect you are going to have conflicts with app_control_continue() and app_control_remove_channel_from_bridge() because they depart the channel as well.
> >     
> >     I think using departable channels for stasis is just wrong.  I suggest looking at app_agent_pool.c for how to reclaim control of a channel when it leaves a bridge.
> 
> Matt Jordan wrote:
>     Richard: how would using a non-departable channel work for Stasis? In Stasis, the thread that puts the channel into the bridge cannot block while the channel is in the bridge, as it is most likely (and almost certainly not) the PBX thread that the channel used when it entered into the application.

Ok.  Forget the last two sentences in my original comment.

However, calling app_control_continue() to remove the channel from a bridge is going to have problems with ast_bridge_depart() being called twice.  This is not allowed.  When app_control_continue() calls ast_bridge_depert() which kicks the channel out of a bridge, the after bridge callback calls the failure callback.  With this patch, the normal and failure callbacks do the same thing.  At this time, they call ast_bridge_depart() on a channel already being departed.


- rmudgett


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


On Oct. 27, 2013, 5:19 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2965/
> -----------------------------------------------------------
> 
> (Updated Oct. 27, 2013, 5:19 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22703
>     https://issues.asterisk.org/jira/browse/ASTERISK-22703
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Currently within Stasis there is no guarantee that a channel that leaves a bridge will have ast_bridge_depart called on it. The attached change changes this by having a command queued on the channel that explicitly departs it when it leaves a bridge. This works for outside manipulation, bridge deletion, hangup, etc.
> 
> This fixes an issue that the referenced bug exposed. When hanging up while in a bridge no synchronization occurred between the bridge and stasis - causing both to act on a channel at the same time. Depending on how fast the stasis side executed it was possible for the channel itself to be freed and the bridge reference this freed memory.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/stasis/control.c 401922 
> 
> Diff: https://reviewboard.asterisk.org/r/2965/diff/
> 
> 
> Testing
> -------
> 
> Deleted a bridge with a channel in it, confirmed with the patch that departure occurs and the bridge is destroyed.
> 
> Hungup while in a bridge, confirmed with the patch that departure occurs.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131029/350a0cda/attachment.html>