[asterisk-dev] [Code Review] Prevent NewChannel manager event when channel is created solely for variable substitution purposes

[Mark Michelson]

> On 2010-04-01 09:06:14, Russell Bryant wrote:
> > It feels like a hack, but I don't have a better idea and it will fix the problem.
> 
> Mark Michelson wrote:
>     Would the idea of backporting ast_dummy_channel_alloc feel like less of a hack?
> 
> Russell Bryant wrote:
>     It would, yes.

I started redoing this by backporting ast_dummy_channel_alloc. What I found was that not all temporary channels are necessarily being used for variable substition. In some cases, they're being allocated as a shell for another channel to masquerade into. In another case, it's being allocated strictly so that CDR operations can be done on it.

While it would certainly be possible to backport ast_dummy_channel_alloc and use it only in the variable substitution situations, it does not correct the original problem reported, which was to fix the fact that NewChannel manager events would show up unexpectedly. As such, I think I'm going to just stick with the original approach even if it does feel like a hack.


- Mark


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


On 2010-03-31 13:59:29, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/601/
> -----------------------------------------------------------
> 
> (Updated 2010-03-31 13:59:29)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Issue 16957 was opened because there were NewChannel events being fired which did not have a matching Hangup event. In this particular example, the NewChannel events were being sent due to an unavailable device when attempting an originate action. ast_pbx_outgoing_cdr_failed() created a new channel solely for variable substitution, leading to a NewChannel event. Rather than calling ast_hangup on the channel, ast_channel_free was called instead.
> 
> My solution for this is to make a slight alteration to ast_channel_alloc_ap() such that we determine whether to send a NewChannel manager event based on whether the channel technology is one that is registered. Technologies such as "Bogus" as used by the manager and "Substitution" as used by voicemail will not result in NewChannel events. My understanding is that an Asterisk administrator would not care about these channels since they are not "proper" channels but rather a mechanism of convenience for the Asterisk developer to facilitate variable substitution.
> 
> An alternate solution, proposed by Atis Lezdins, is instead to backport ast_dummy_channel_alloc() from trunk to all affected branches. I am certainly open to this idea; however, I don't feel it is right for me to make such a decision on my own and would like feedback from other developers on the matter. The main advantage to ast_dummy_channel_alloc() is that it is a lightweight channel allocation mechanism designed specifically for facilitating variable substitution. The only drawback I can think of is that this would require touching more code. Anyway, tell me what you think.
> 
> 
> This addresses bug 16957.
>     https://issues.asterisk.org/view.php?id=16957
> 
> 
> Diffs
> -----
> 
>   /branches/1.6.1/main/channel.c 255505 
> 
> Diff: https://reviewboard.asterisk.org/r/601/diff
> 
> 
> Testing
> -------
> 
> I used the AMI GetVar action like so:
> 
> Action: GetVar
> Channel:
> Variable: CALLERID(name)
> 
> Doing so without the patch caused a NewChannel event to be generated. Doing so with the patch caused no such NewChannel event to be generated. I also made sure that NewChannel events were emitted when SIP channels placed calls.
> 
> 
> Thanks,
> 
> Mark
> 
>