[asterisk-dev] [Code Review] 4213: Stasis: Fix StasisStart and StasisEnd ordering and missing events

[opticron]

> On Dec. 2, 2014, 7:23 a.m., Matt Jordan wrote:
> > branches/12/res/res_stasis.c, lines 1069-1075
> > <https://reviewboard.asterisk.org/r/4213/diff/1/?file=69250#file69250line1069>
> >
> >     I think we have an error here, in that we don't clean up the implicit application subscription to the channel here.

The subscription is dropped inside of app_send_end_msg().


> On Dec. 2, 2014, 7:23 a.m., Matt Jordan wrote:
> > branches/12/res/res_stasis.c, lines 1112-1114
> > <https://reviewboard.asterisk.org/r/4213/diff/1/?file=69250#file69250line1112>
> >
> >     I'm confused about where the channel being masqueraded out has its StasisEnd raised. Are you guaranteed that the stolen callback will be raised for the channel being replaced? If not, where does the replaced channel get its StasisEnd and its subscription modified?

It was being generated based on the StasisStart with replace_channel, but this has been reverted to a simpler mechanism in the new patch.


> On Dec. 2, 2014, 7:23 a.m., Matt Jordan wrote:
> > branches/12/res/stasis/app.c, lines 1165-1190
> > <https://reviewboard.asterisk.org/r/4213/diff/1/?file=69252#file69252line1165>
> >
> >     I'm not sure why this entire block was removed. It still feels like we would need to clean up the implicit application subscriptions on replacement. If not, the review summary does not explain why this code was no longer necessary.

The subscription swap is still happening, it's just partially integrated with the start and end message sending routines. I'll finish that integration in the next patch.


- opticron


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


On Nov. 25, 2014, 2:29 p.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4213/
> -----------------------------------------------------------
> 
> (Updated Nov. 25, 2014, 2:29 p.m.)
> 
> 
> Review request for Asterisk Developers and Mark Michelson.
> 
> 
> Bugs: ASTERISK-24537
>     https://issues.asterisk.org/jira/browse/ASTERISK-24537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This corrects several bugs that currently exist in the 12/13/trunk stasis application code.
> 
> * Masquerades now swap channel topics appropriately
> * StasisStart and StasisEnd publishing is now properly restricted to controlling apps via app topics
> * StasisEnd published because of replacement via masquerade-in is now published as a byproduct of the associated StasisStart to avoid race conditions
> * StasisEnds now appear in some cases in which they were missing involving masquerades and bridge swaps into and out of Stasis()
> 
> 
> Diffs
> -----
> 
>   branches/12/res/stasis/app.c 428504 
>   branches/12/res/stasis/app.h 428504 
>   branches/12/res/res_stasis.c 428504 
>   branches/12/main/channel_internal_api.c 428504 
>   branches/12/main/channel.c 428504 
>   branches/12/include/asterisk/channel.h 428504 
> 
> Diff: https://reviewboard.asterisk.org/r/4213/diff/
> 
> 
> Testing
> -------
> 
> Ran existing ARI tests and a few new ones that are in-progress along with the scenarios that originally found these issues.
> 
> 
> Thanks,
> 
> opticron
> 
>

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