Summary:	ASTERISK-06316: [patch] new agi function, NiceAGI
Reporter:	Donny Kavanagh (donnyk)	Labels:
Date Opened:	2006-02-15 00:15:23.000-0600	Date Closed:	2006-05-25 14:02:57
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Resources/res_agi
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 20060215__aginohup.diff.txt ( 1) niceagi.diff
Description:	http://bugs.digium.com/view.php?id=4854 http://bugs.digium.com/view.php?id=6474 This patch was a bug fix which properly implemented SIGHUP functionality in asterisk AGI.  This patch however changed the behavior people had grown used to (in 1.0.x). I myself prefer to handle hangups (in some cases) in my agi scripts, and recompiling php (my language of choice) to support the functionality to ignore SIGHUP (http://ca3.php.net/pcntl) is somewhat of a nuisance.  Doing so would also remove the ability for me to keep php up to date via the os package manager.  So with these complications in mind, i present NiceAGI(). When you call your AGI with NiceAGI() rather then AGI() when a hangup occurs in asterisk, NiceAGI() will terminate, however your script will be left to clean itself up gracefully, the following situation is an example of where it is useful: I personally have a system which consists of a asterisk server running as a conference bridge with a php web inteface that manages the conference bridge, including seeing who is in what rooms, being able to invite or kick people via the interface and whatnot.  If a user is in a conference room and they hangup, in 1.0.x i am able to detect the hangup via the return values of the agi functions, and then gracefully terminate the php with some proper cleanup (eg execute the database functionality to remove someone from the web based room listing) with 1.2.x this functionality is removed if the language of choice is unable to ignore SIGHUP due to design, or in its default configuration. This has undergone only slight testing, however, due to the simplicity of the patch, i am quite sure it will be fine.  I wanted to get it done/post it before i lost intreast in doing so :) I will test further at work tomorow, and of course update if there are any issues. ****** ADDITIONAL INFORMATION ****** Disclaimer is on file. This patch will apply cleanly to trunk as well as asterisk-1.2. Please link this with 4854 & 6474.
Comments:	By: Tilghman Lesher (tilghman) 2006-02-15 11:21:37.000-0600 Rather than creating yet another application, why not just check a channel variable which describes whether or not we want this behavior (defaulting to send a SIGHUP)?  For example, Set(NOHUP=yes) to disable sending the SIGHUP.  You could even set this as a global variable in extensions.conf if you always wanted this behavior. By: Donny Kavanagh (donnyk) 2006-02-15 11:24:44.000-0600 Alright i'll give it a shot. Perhaps, AGI_NOHUP is better? suggestions for the best suited chan var name? By: Donny Kavanagh (donnyk) 2006-02-15 11:26:00.000-0600 Do you have an example on a simplistic dialplan app which uses dialplan variables?  Just looking for an example to go by, my asterisk expirence is limited. By: Donny Kavanagh (donnyk) 2006-02-15 12:05:07.000-0600 Corydon, i thought about this, and talked about it with drumkilla, and we seem to both agree having a dial plan function depend on a variable you just set is kinda messy.  It seems odd to have it depend on options and not pass them in, just reading some random variable doesnt really apeal to me. Using the NiceAGI() method is definitally cleaner. Thoughts? By: Tilghman Lesher (tilghman) 2006-02-15 12:06:33.000-0600 juggie:  this isn't a dialplan function.  It's an application, and we depend upon other variables in applications already. By: Donny Kavanagh (donnyk) 2006-02-15 12:13:17.000-0600 Regardless of the type, i agree, you do, but that doesnt mean its a great design to do so.  I was just going with the flow of the current, i agree, theres no point in adding more and more functions, but Having [Dead/E/]AGI and then having just AGI reading a channel var is kinda dumb too, because AGI is doing two conflicting things, one way for some options, another way for another.   If i was going to implement it this way, i would want to implement it across the board, or it doesnt really make sense. By: Tilghman Lesher (tilghman) 2006-02-15 12:14:48.000-0600 This isn't an option; it's a behavior change to old, deprecated behavior.  Here's a suggested patch to accomplish this. By: Tilghman Lesher (tilghman) 2006-02-15 12:23:44.000-0600 You're right; this whole problem is a huge kludge.  We aren't going to commit hacks for problems which are in your package management system. By: Donny Kavanagh (donnyk) 2006-02-15 12:37:59.000-0600 just reopening. By: Russell Bryant (russell) 2006-02-15 12:57:57.000-0600 What if instead of calling it AGINOHUP, we call it something like AGIOPTIONS.  Then, if we want to add any more options in the future, they could all be put in this single variable, instead of creating a different variable for each option.  The format for these options would be the same as they would be as if we were passing a string of options to an application. Options:  h - Do not send SIGHUP to the script when the caller hangs up Set(AGIOPTIONS=h) AGI(......) By: Russell Bryant (russell) 2006-02-15 12:59:54.000-0600 How is this a problem in terms of package management systems?  Isn't the issue using a programming language for AGI where SIGHUP can not be caught and handled? By: Donny Kavanagh (donnyk) 2006-02-15 13:03:20.000-0600 I was just explaining that php does not have PCNTL compiled in by default on most linux distro's, (using php as an example of a lang that doesnt support catching SIGHUP, at least by default) My example was that to do a custom install of php would break package manangement/updates for php hence it may be simpler/more effective/better for users to allow SIGHUP to be optional since all agi functions return -1 on a hangup you can detect a hangup in your script. By: Donny Kavanagh (donnyk) 2006-02-15 13:37:44.000-0600 I'm going to work on this in a way that would allow options for AGI() rather then multiple application names (not to totally remove them, but move in that direction, and definitally not add any new ones), and include DeadAGI/EAGI into the mix.  I'll update this when i have a chance to do so. However what would be better, createing an AGIOPTIONS dialplan var which AGI() will then use, and you could do say Set(AGIOPTIONS=d) and AGI() would run in deadagi mode. OR something like this which could be implemented in AGI without breaking backword compat, AGI(flags!script^arg1^arg2) thoughts? By: Matthew Nicholson (mnicholson) 2006-02-15 14:42:59.000-0600 Isn't DeadAGI supposed to do this?  And if it is supposed to do this, and it is not, then we should fix DeadAGI rather than adding another command. By: Donny Kavanagh (donnyk) 2006-02-15 20:50:09.000-0600 Dead AGI is intended to run on a channel (that has no channel) :)  Running DeadAGI on a active call does not work well, as far as my expirence goes, last time i tried it caused major auto delays (eg, 30seconds + in meetme) Consequentially, it would probally be wise to refuse to run deadagi if the channel is active. By: Matthew Nicholson (mnicholson) 2006-02-16 10:55:05.000-0600 That does not make much sense.  Using DeadAGI should not cause audio issues.  Maybe this should be investigated futher. By: Donny Kavanagh (donnyk) 2006-02-16 11:32:44.000-0600 the only actual difference between AGI/DeadAGI call is this line here, c = ast_waitfor_nandfds(&chan, dead ? 0 : 1, &agi->ctrl, 1, NULL, &outfd, &ms); without knowing too much about the code, presumabally since its telling this function not to wait on a channel, but then you would issue AGI commands that affect the channel, things go out of sync?? not sure. I'm not sure dead agi is the appropriate solution here, the intent of this application was to run an agi command on channel cleanup when there really is no active channel at all.  You could almost make a case for the fact that DeadAGI should not run at all when a channel exists, much like AGI wont run when a channel does not. By: Donny Kavanagh (donnyk) 2006-02-16 12:02:28.000-0600 A better way to look at it may be this, DeadAGI could potentially be entirely removed. calls to AGI on an active channel would result in calls to ast_waitfor_nandfds, asking it to wait on a channel, while if AGI was started on a dead channel it would instead say not to wait for a channel in this agi instance. Since we know if the channel state is up or not, now that i think about it, i dont really see why this application has a new name. DeadAGI actually doesnt have any special code stopping it from sending a SIGHUP when theres a hangup, its just a side effect of the ast_waitfor_nandfds loop.  Since with DeadAGI we arnt asking ast_waitfor_nandfds to wait on a channel, then it doesnt care when the channel infact does hangup.  And as i mentioned, i'm not sure this is appropriate, deadagi should probally *NOT* run when a channel is live. By: Olle Johansson (oej) 2006-04-05 10:40:38 Where are we with this discussion? By: Yunlong Liu (lyl) 2006-04-05 23:07:51 Why not use signal handler in your AGI By: Donny Kavanagh (donnyk) 2006-04-06 00:08:28 I've been really busy with work and i havnt had time to look at this. My intent is still to add some more inteligence to AGI() to operate properly in live channel/dead channel mode, rather then how you call it, eg it should work in AGI or DeadAGI mode automatically. By: Serge Vecher (serge-v) 2006-05-04 10:21:36 juggie: any chance of an update here? By: Russell Bryant (russell) 2006-05-25 14:02:55 I added the option to set the channel variable, AGISIGHUP, to the trunk in revision 30337, thanks!