[asterisk-dev] [Code Review] 4271: ARI: Allow interoperation of ASYNCGOTO and channels originated to Stasis()

[Matt Jordan]

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


I'm generally confused by the solution here (and to a lesser extent, the problem). There is already code in the AMI Redirect action that looks to see if a channel has a PBX and - if it doesn't - provides a PBX for the channel. I would expect that a channel that is created via an Originate to Stasis that is then later redirected out via AMI would fall into this category.

Why is the existing code not sufficient?


branches/13/main/pbx.c
<https://reviewboard.asterisk.org/r/4271/#comment24497>

    A few comments, in addition to Mark's.
    
    First, you can always check to see if a channel has a PBX by using ast_channel_pbx. I'm not sure why that function wouldn't be sufficient for what we are trying to solve here.
    
    Second, I don't think we should be modifying anything in pbx.c. Generally, when a channel is Originated to an application, we don't let it continue along once that application finishes. If I Originate a channel to a Bridge dialplan application, I would expect it to be hung up when it leaves that bridge.
    
    Now, the same is not entirely true for Stasis. Take a channel that was originated into Stasis - at that point in time, I've handed the channel over to an external application. The PBX doesn't own it. Thus:
    * If I /continue a channel into the dialplan (leaving Stasis), I would expect it to go execute dialplan.
    * If I Redirect the channel using AMI, I would expect it to go execute dialplan.
    
    But that's really up to the controlling mechanism to ensure happens properly - either by releasing it with a PBX, or by stealing it and giving it a PBX. Either way, it is not up to the code in pbx.c to "know" whether or not the channel that it just got handed back came out of anything. If it has a valid channel at that point, it should just hang it up.
    
    Changing this behaviour would have major implications outside of just Stasis, and I don't think we've thought through what all those would be.



branches/13/res/ari/resource_channels.c
<https://reviewboard.asterisk.org/r/4271/#comment24498>

    Along the same lines as my previous comment, I don't think solving this problem here solves it in the fashion that we want. It may fix it for a channel created via ARI, but that's not generally applicable - as a channel that is created via an Originate into Stasis should also survive this operation.


- Matt Jordan


On Dec. 16, 2014, 2:41 p.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4271/
> -----------------------------------------------------------
> 
> (Updated Dec. 16, 2014, 2:41 p.m.)
> 
> 
> Review request for Asterisk Developers and Joshua Colp.
> 
> 
> Bugs: ASTERISK-24591
>     https://issues.asterisk.org/jira/browse/ASTERISK-24591
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> When the AMI Redirect action is used with a channel bridged inside Stasis() and not running a pbx, the channel is hung up instead of proceeding to the desired location in dialplan. This change allows such channels to be Redirected properly by detecting the operation used by Redirect (ASYNCGOTO) and starting a pbx if necessary to allow the channel to execute dialplan at the desired location.
> 
> 
> Diffs
> -----
> 
>   branches/13/res/ari/resource_channels.c 429611 
>   branches/13/main/pbx.c 429611 
> 
> Diff: https://reviewboard.asterisk.org/r/4271/diff/
> 
> 
> Testing
> -------
> 
> Ran the test that found this bug and verified that it passed with the expected events. See review 4272 for the test in question.
> 
> 
> Thanks,
> 
> opticron
> 
>

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