[asterisk-dev] [Code Review]: Hangup handlers - Dialplan subroutines that run when the channel hangs up.
rmudgett
reviewboard at asterisk.org
Tue Jun 26 18:24:46 CDT 2012
> On June 26, 2012, 5:08 p.m., Matt Jordan wrote:
> > /trunk/apps/app_dial.c, lines 3027-3048
> > <https://reviewboard.asterisk.org/r/2002/diff/1/?file=29134#file29134line3027>
> >
> > This previously would not end the CDR. ast_pbx_h_exten_run explicitly does.
> >
> > Will that cause a problem?
I don't think so since this is done on the *peer* channel and likely was an overlooked h-exten execution location when the endbeforehexten option was added. The peer is going to be hung up right after anyway.
> On June 26, 2012, 5:08 p.m., Matt Jordan wrote:
> > /trunk/funcs/func_channel.c, lines 116-123
> > <https://reviewboard.asterisk.org/r/2002/diff/1/?file=29139#file29139line116>
> >
> > We talked a bit about this, and replace - to me - still sounds like a 'swap' operation, where a hangup handler would be replaced (see?) with another hangup handler.
> >
> > Clear, wipe, remove_all, or something else that implies total destruction of all existing hangup handlers seems clearer to me. The fact that you can *also* (optionally) put a new hangup handler on the stack seems secondary to the effect that you nuke the entire stack in the first place.
It is a swap operation. The entire hangup handler stack is swapped with a new hangup handler.
I think wipe is the better choice here so I will change it to that.
> On June 26, 2012, 5:08 p.m., Matt Jordan wrote:
> > /trunk/main/pbx.c, lines 5381-5384
> > <https://reviewboard.asterisk.org/r/2002/diff/1/?file=29146#file29146line5381>
> >
> > This is a little tricky, in that it looks like you're forgetting to unlock the channel when you come out of the loop.
> >
> > A comment here might be nice.
ok
> On June 26, 2012, 5:08 p.m., Matt Jordan wrote:
> > /trunk/main/pbx.c, lines 5428-5449
> > <https://reviewboard.asterisk.org/r/2002/diff/1/?file=29146#file29146line5428>
> >
> > I'm curious, - why is this in a for (;;)?
> >
> > I would think you would be able to get the handlers, then do a:
> >
> > while ((h_handler = AST_LIST_REMOVE_HEAD(handlers, node)) {
> > ...
> > }
> >
> > Is there an expectation that the list might be consumed and a new handler added by an executing hangup handler?
Once the channel lock is released, you really should not depend on a pointer returned by the ast_channel accessor function to remain the same. For Asterisk 11 this will not be the case but going forward it might not be.
> On June 26, 2012, 5:08 p.m., Matt Jordan wrote:
> > /trunk/main/channel.c, line 6924
> > <https://reviewboard.asterisk.org/r/2002/diff/1/?file=29143#file29143line6924>
> >
> > If you're getting rid of this here, then is the ast_kill_tech still needed in feature_request_and_dial?
> >
> > If not, then I don't think its needed at all anywhere anymore.
That tech was created initially for feature_request_and_dial(). The reason for it there is still valid. It was just promoted to a public name, location, and made unconditional code when ast_do_masquerade() had a need for it. It is now only used by feature_request_and_dial() so technically it could be moved back.
> On June 26, 2012, 5:08 p.m., Matt Jordan wrote:
> > /trunk/.cleancount, line 1
> > <https://reviewboard.asterisk.org/r/2002/diff/1/?file=29133#file29133line1>
> >
> > Why is this needed?
Because of this comment for struct ast_channel:
* \brief Main Channel structure associated with a channel.
*
* \note XXX It is important to remember to increment .cleancount each time
* this structure is changed. XXX
Though with channel opaquification the note is not really needed anymore.
- rmudgett
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2002/#review6580
-----------------------------------------------------------
On June 25, 2012, 11:30 a.m., rmudgett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2002/
> -----------------------------------------------------------
>
> (Updated June 25, 2012, 11:30 a.m.)
>
>
> Review request for Asterisk Developers and kobaz.
>
>
> Summary
> -------
>
> This feature originated with https://reviewboard.asterisk.org/r/1230/ by kobaz.
>
> See
> https://wiki.asterisk.org/wiki/display/AST/Asterisk+11+Projects
> https://wiki.asterisk.org/wiki/display/AST/Hangup+handlers
>
> Hangup handlers are an alternative to the h extension. They can be used
> in addition to the h extension. The idea is to attach a Gosub routine to
> a channel that will execute when the call hangs up. Whereas which h
> extension gets executed depends on the location of dialplan execution when
> the call hangs up, hangup handlers are attached to the call channel. You
> can attach multiple handlers that will execute in the order of most
> recently added first.
>
> Call transfers, call pickup, and call parking can result in channels on
> both sides of a bridge containing hangup handlers.
>
> Hangup handlers can also be attached to any call leg because of pre-dial
> routines.
>
>
> When hangup handlers are executed
>
> Any hangup handlers associated with a channel are always executed when the
> channel is hung up.
>
>
> Manipulating hangup handlers on a channel
>
> Hangup handlers pass the saved handler string to the Gosub application to
> execute. The syntax is intentionally the same as the Gosub application.
> If context or exten are not supplied then the current values from the
> channel pushing the hangup handler are inserted before storing on the
> hangup handler stack.
>
> Push a hangup handler onto a channel:
> same => n,Set(CHANNEL(hangup_handler_push)=[[context,]exten,]priority[(arg1[,...][,argN])]);
>
> Pop a hangup handler off a channel and optionally push a replacement:
> same => n,Set(CHANNEL(hangup_handler_pop)=[[[context,]exten,]priority[(arg1[,...][,argN])]]);
>
> Pop all hangup handlers off a channel and optionally push a replacement:
> same => n,Set(CHANNEL(hangup_handler_replace)=[[[context,]exten,]priority[(arg1[,...][,argN])]]);
>
> Cascading hangup handlers
> same => n,Set(CHANNEL(hangup_handler_push)=hdlr3,s,1(args));
> same => n,Set(CHANNEL(hangup_handler_push)=hdlr2,s,1(args));
> same => n,Set(CHANNEL(hangup_handler_push)=hdlr1,s,1(args));
>
>
> AMI events
>
> The hangup handler AMI events are output as part of the AMI dialplan
> permission class.
>
> * The AMI event HangupHandlerPush is generated whenever a hangup handler
> is pushed on the stack by the CHANNEL() function.
>
> * The AMI event HangupHandlerPop is generated whenever a hangup handler is
> popped off the stack by the CHANNEL() function.
>
> * The AMI event HangupHandlerRun is generated as a hangup handler is about
> to be executed.
>
>
> CLI commands
>
> Single channel:
> core show hanguphandlers <chan>
>
> Output:
> Channel Handler
> <chan-name> <first handler to execute>
> <second handler to execute>
> <third handler to execute>
>
> All channels:
> core show hanguphandlers all
>
> Output:
> Channel Handler
> <chan1-name> <first handler to execute>
> <second handler to execute>
> <third handler to execute>
> <chan2-name> <first handler to execute>
> <chan3-name> <first handler to execute>
> <second handler to execute>
>
>
> This addresses bug ASTERISK-19549.
> https://issues.asterisk.org/jira/browse/ASTERISK-19549
>
>
> Diffs
> -----
>
> /trunk/.cleancount 369297
> /trunk/apps/app_dial.c 369297
> /trunk/apps/app_followme.c 369297
> /trunk/apps/app_queue.c 369297
> /trunk/channels/chan_local.c 369297
> /trunk/configs/cdr.conf.sample 369297
> /trunk/funcs/func_channel.c 369297
> /trunk/include/asterisk/channel.h 369297
> /trunk/include/asterisk/pbx.h 369297
> /trunk/main/autoservice.c 369297
> /trunk/main/channel.c 369297
> /trunk/main/channel_internal_api.c 369297
> /trunk/main/features.c 369297
> /trunk/main/pbx.c 369297
>
> Diff: https://reviewboard.asterisk.org/r/2002/diff
>
>
> Testing
> -------
>
> Attached hangup handlers to channels and done the following kinds of calls:
> Simple A to B call.
> DTMF attended transfer
> DTMF blind transfer
>
> Hangup handlers were executed when expected.
>
>
> Thanks,
>
> rmudgett
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120626/ef4c88f1/attachment-0001.htm>
More information about the asterisk-dev
mailing list