[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

[Joshua Colp]

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



/branches/12/res/ari/resource_channels.c
<https://reviewboard.asterisk.org/r/3710/#comment22749>

    Holding the channel lock while subscribing is hazardous. The stasis_app_subscribe will internally call ast_channel_get_by_name which locks the the individual channels during comparison. If two (or more) originates happen at the same time it is possible for one to hold a channel lock and the other to block waiting to acquire it, while the first one blocks waiting to find the channel... and yeah, no good.


- Joshua Colp


On July 3, 2014, 8:50 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, 8:50 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/ari/resource_channels.c 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/20140703/0f6a9f62/attachment.html>