[Home]

Summary:ASTERISK-15740: [patch] Rogue Newchannel events for failed Originate calls
Reporter:Atis Lezdins (atis)Labels:
Date Opened:2010-03-03 16:36:07.000-0600Date Closed:2010-09-14 13:49:17
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/PBX
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) fake_newchannel_event_2.patch
( 1) fake_newchannel_event.patch
Description:When using AMI Originate to unavailable device, there's NewChannel event without following Hangup event.

I traced this to ast_pbx_outgoing_cdr_failed() which creates new channel for CDR posting purposes, however destroys without ast_hangup() sending Hangup event. It could probably be fixed by replacing ast_channell_free() with ast_hangup(), however in SVN trunk there's function for allocating dummy channel instead.

Attached patch that won't generate events for channels with empty names (this is probably why there was check for empty name_fmt, however it's impossible to call ast_channel_alloc with empty name_fmt.

Affected 1.6.1 and 1.6.2 and probably below. SVN trunk uses dummy channel, so probably unaffected.
Comments:By: David Woolley (davidw) 2010-03-04 05:19:49.000-0600

Duplicates ASTERISK-1511291 I believe.
By: Atis Lezdins (atis) 2010-03-04 09:15:35.000-0600

Somehow I got to compile another patch, so that name_fmt is empty when passed to ast_channel_alloc, so it won't generate Newchannel AMI event and also removes error "Received invalid event that had no device IE" from issue ASTERISK-14284
By: Atis Lezdins (atis) 2010-03-04 09:26:13.000-0600

There are many places in asterisk where ast_channel_alloc() is called with non-empty name and then destroyed with ast_channel_free(). It seems that any such use would generate Newchannel AMI event as well as device state error.

Most weird cases are:

apps/app_minivm.c:      ast = ast_channel_alloc(0, AST_STATE_DOWN, 0, 0, "", "", "", 0, "%s", "");

apps/app_voicemail.c:           if ((ast = ast_channel_alloc(0, AST_STATE_DOWN, 0, 0, "", "", "", 0, "Substitution/voicemail"))) {

main/logger.c:          struct ast_channel *c = ast_channel_alloc(0, 0, "", "", "", "", "", 0, "Logger/rotate");

main/manager.c:                 c = ast_channel_alloc(0, 0, "", "", "", "", "", 0, "Bogus/manager");

main/pbx.c:                                     struct ast_channel *bogus = ast_channel_alloc(0, 0, "", "", "", "", "", 0, "Bogus/%p", vars);

main/pbx.c:             struct ast_channel *tmpchan = ast_channel_alloc(0, chan->_state, 0, 0, chan->accountcode, chan->exten, chan->context, chan->amaflags, "AsyncGoto/%s", chan->name);

main/pbx.c:     struct ast_channel *chan = ast_channel_alloc(0, AST_STATE_DOWN, 0, 0, "", "", "", 0, "");

main/pbx.c:                             chan = ast_channel_alloc(0, AST_STATE_DOWN, 0, 0, "", "", "", 0, "OutgoingSpoolFailed");

I wonder is the channel name for them really necessary? If so, it could be united to format "Bogus/" so that both channel event and device IE is not used.
By: Atis Lezdins (atis) 2010-03-31 12:58:02

Last patch works great on our production system since I posted.

However other occurrences probably need some attention
By: Mark Michelson (mmichelson) 2010-03-31 13:04:40

I have an idea that should cover the other cases that atis has pointed out. The determination of whether a NewChannel event is sent depends on the name_fmt parameter to ast_channel_alloc_ap(). I think what makes more sense is to perform a search for a specific channel technology. If one is found, then send the NewChannel event. If not, then do not send the NewChannel event.

As far as I know, most administrators don't actually care to know that a bogus channel was created just for facilitating variable substitution, so not sending a NewChannel event in these situations should be okay. I'll have a patch up as soon as I have verified that what I have written will work.
By: Mark Michelson (mmichelson) 2010-04-05 11:46:05

I posted a code review request several days ago here: https://reviewboard.asterisk.org/r/601/

There has been discussion, and it appears the patch I have provided there will do the trick for you. Feel free to give it a shot. To download the patch, just click the "Download Diff" button in the top right corner of the review request.
By: Digium Subversion (svnbot) 2010-04-26 16:03:09

Repository: asterisk
Revision: 259018

U   branches/1.4/main/channel.c

------------------------------------------------------------------------
r259018 | mmichelson | 2010-04-26 16:03:08 -0500 (Mon, 26 Apr 2010) | 13 lines

Prevent Newchannel manager events for dummy channels.

No Newchannel manager event will be fired for channels that are
allocated to not match a registered technology type. Thus bogus
channels allocated solely for variable substitution or CDR
operations do not result in a Newchannel event.

(closes issue ASTERISK-15740)
Reported by: atis

Review: https://reviewboard.asterisk.org/r/601


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=259018
By: Digium Subversion (svnbot) 2010-04-26 16:13:35

Repository: asterisk
Revision: 259023

_U  trunk/
U   trunk/main/channel.c

------------------------------------------------------------------------
r259023 | mmichelson | 2010-04-26 16:13:35 -0500 (Mon, 26 Apr 2010) | 19 lines

Merged revisions 259018 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r259018 | mmichelson | 2010-04-26 16:03:08 -0500 (Mon, 26 Apr 2010) | 13 lines
 
 Prevent Newchannel manager events for dummy channels.
 
 No Newchannel manager event will be fired for channels that are
 allocated to not match a registered technology type. Thus bogus
 channels allocated solely for variable substitution or CDR
 operations do not result in a Newchannel event.
 
 (closes issue ASTERISK-15740)
 Reported by: atis
 
 Review: https://reviewboard.asterisk.org/r/601
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=259023
By: Digium Subversion (svnbot) 2010-04-26 16:19:58

Repository: asterisk
Revision: 259036

_U  branches/1.6.0/
U   branches/1.6.0/main/channel.c

------------------------------------------------------------------------
r259036 | mmichelson | 2010-04-26 16:19:57 -0500 (Mon, 26 Apr 2010) | 26 lines

Merged revisions 259023 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r259023 | mmichelson | 2010-04-26 16:13:35 -0500 (Mon, 26 Apr 2010) | 19 lines
 
 Merged revisions 259018 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r259018 | mmichelson | 2010-04-26 16:03:08 -0500 (Mon, 26 Apr 2010) | 13 lines
   
   Prevent Newchannel manager events for dummy channels.
   
   No Newchannel manager event will be fired for channels that are
   allocated to not match a registered technology type. Thus bogus
   channels allocated solely for variable substitution or CDR
   operations do not result in a Newchannel event.
   
   (closes issue ASTERISK-15740)
   Reported by: atis
   
   Review: https://reviewboard.asterisk.org/r/601
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=259036
By: Digium Subversion (svnbot) 2010-04-26 16:28:30

Repository: asterisk
Revision: 259078

_U  branches/1.6.1/
U   branches/1.6.1/main/channel.c

------------------------------------------------------------------------
r259078 | mmichelson | 2010-04-26 16:28:30 -0500 (Mon, 26 Apr 2010) | 26 lines

Merged revisions 259023 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r259023 | mmichelson | 2010-04-26 16:13:35 -0500 (Mon, 26 Apr 2010) | 19 lines
 
 Merged revisions 259018 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r259018 | mmichelson | 2010-04-26 16:03:08 -0500 (Mon, 26 Apr 2010) | 13 lines
   
   Prevent Newchannel manager events for dummy channels.
   
   No Newchannel manager event will be fired for channels that are
   allocated to not match a registered technology type. Thus bogus
   channels allocated solely for variable substitution or CDR
   operations do not result in a Newchannel event.
   
   (closes issue ASTERISK-15740)
   Reported by: atis
   
   Review: https://reviewboard.asterisk.org/r/601
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=259078
By: Digium Subversion (svnbot) 2010-04-26 16:35:13

Repository: asterisk
Revision: 259103

_U  branches/1.6.2/
U   branches/1.6.2/main/channel.c

------------------------------------------------------------------------
r259103 | mmichelson | 2010-04-26 16:35:13 -0500 (Mon, 26 Apr 2010) | 26 lines

Merged revisions 259023 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r259023 | mmichelson | 2010-04-26 16:13:35 -0500 (Mon, 26 Apr 2010) | 19 lines
 
 Merged revisions 259018 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r259018 | mmichelson | 2010-04-26 16:03:08 -0500 (Mon, 26 Apr 2010) | 13 lines
   
   Prevent Newchannel manager events for dummy channels.
   
   No Newchannel manager event will be fired for channels that are
   allocated to not match a registered technology type. Thus bogus
   channels allocated solely for variable substitution or CDR
   operations do not result in a Newchannel event.
   
   (closes issue ASTERISK-15740)
   Reported by: atis
   
   Review: https://reviewboard.asterisk.org/r/601
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=259103