[asterisk-dev] [Code Review] 3710: ARI: Subscribe to both halves of a Local channel pair when originating a Local channel to a Stasis application; clean up subscription leak

[Matt Jordan]

> On July 4, 2014, 6:43 a.m., Joshua Colp wrote:
> > /branches/12/res/ari/resource_channels.c, line 869
> > <https://reviewboard.asterisk.org/r/3710/diff/2/?file=62271#file62271line869>
> >
> >     You need to check if local_peer exists here as it won't for non-Local and following stuff assumes it will (ie: the debug statement in stasis_app_subscribe_channel calls ast_channel_uniqueid which assumes it will be non-NULL).

Heh. I woke up this morning and thought, "oh snap, I didn't check if that was NULL".

And then I browsed twitter.


- Matt


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


On July 3, 2014, 10:01 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3710/
> -----------------------------------------------------------
> 
> (Updated July 3, 2014, 10:01 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23939
>     https://issues.asterisk.org/jira/browse/ASTERISK-23939
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch fixes two bugs:
> 
> (1) When originating a channel into a Stasis application, we already create a subscription for the channel that is going into our Stasis app. Unfortunately, when you create a Local channel and pass it off to a Stasis app, you really aren't creating just one channel: you're creating two. This patch snags the second half of the Local channel pair (assuming it is a Local channel pair, but luckily core_local is kind about such assumptions) and subscribes to it as well.
> 
> (2) Subscriptions are a bit sticky right now. If a subscription is made, the 'interest' count gets bumped on the Stasis subscription - but unless something explicitly unsubscribes the channel, said subscription sticks around. This is not much of a problem is a user is creating the subscription - if they made it, they must want it. However, when we are creating implicit subscriptions, we need to make sure something clears them out. This patch takes a pessimistic approach: it watches the cache updates coming from Stasis and, if we notice that the cache just cleared out an object, we delete our subscription object. This keeps our ao2 container of Stasis forwards in an application from growing out of hand; it also is a bit more forgiving for end users who may not realize they were supposed to unsubscribe from that channel that just hung up.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/stasis/app.c 417955 
>   /branches/12/res/res_stasis.c 417955 
>   /branches/12/res/ari/resource_channels.c 417955 
>   /branches/12/include/asterisk/stasis_app.h 417955 
> 
> Diff: https://reviewboard.asterisk.org/r/3710/diff/
> 
> 
> Testing
> -------
> 
> The channels originate test (which makes 25 Local channels) now gets 25 channels in its Stasis application, but gets 50 destruction messages. Inspection of the log file shows that it also gets the dialplan execution messages for the Local channel halves off executing dialplan.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140704/2f362e3d/attachment.html>