Summary:	ASTERISK-09371: Asterisk CLI Busy Loop
Reporter:	Matthew Nicholson (mnicholson)	Labels:
Date Opened:	2007-05-02 17:02:21	Date Closed:	2007-07-06 19:18:19
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	We have found a bug that causes Asterisk to enter an infinite loop and peg the CPU.  When this occurs strace reports the following output. write(1, "asterisk*CLI> ", 15)         = -1 EIO (Input/output error) ioctl(0, SNDCTL_TMR_TIMEBASE or TCGETS, 0xbfd9c548) = -1 EIO (Input/output error) Notice Asterisk (via editline) is attempting to write a CLI prompt to STDOUT but failing.  This just loops as fast as the machine will allow. You can reproduce this bug within the comfort of your own home as follows: 1. Use xterm (or whatever) to connect to your server. 2. Start asterisk. 3. Connect to the CLI with asterisk -r. 4. Start a shell using '!'. 5. Connect to the CLI again with asterisk -r. 6. Start another shell using '!'. 7. Close xterm. Top should now report asterisk using 100% of the CPU or however much is available to it. ****** ADDITIONAL INFORMATION ****** This bug occurs on asterisk 1.4 and asterisk 1.2. In the asterisk 1.2.17 source the loop starts on line 1818 in asterisk.c.  The el_gets() function is what attempts to write the CLI prompt. The remoteconsolehandler() function on line 1024 in asterisk.c is what calls ast_safe_system() to shell out to bash/sh (line 1036). This bug is really two bugs.  When the xterm window is closed it sends a SIGHUP to the child process (a bash shell) which causes it to exit.  The first bug is here. The ast_safe_system() function should return -1 if a child process exited abnormally, the remoteconsolehandler() function does not check the return value from ast_safe_system(), thus does not detect some wrong may have happened.  Even if we did check for -1 as a return value it may be better to specifically check for SIGHUP, as SIGTERM, SIGINT, and SIGKILL could all cause ast_safe_system() to return -1. The first step to fixing this bug should be to detect when a child process exits with SIGHUP and then send ourselves a SIGHUP (IMO). The second part of this bug is on line 1818 is asterisk.c where we call el_gets().  According to the man page for el_gets(), it returns NULL if no characters were read or if there was an error, we do not handle NULL different for other return values (we just loop and call el_gets() again). We should detect the NULL return value and exit. There may be other ways to reproduce this bug, so I think we should handle both of the issues reported here.
Comments:	By: Brett Bryant (bbryant) 2007-06-22 09:57:55 Fixed in 1.2, 1.4 and trunk thanks to mnicholson's idea for the patch. Revisions 71067, 71066, and 71064. By: Russell Bryant (russell) 2007-06-24 14:57:38 It looks like this patch causes some problems when running Asterisk in console mode (with the -c argument).  There are cases where el_gets() returns NULL when everything is normal.  I noticed this when Asterisk was outputting verbose output when I was making a call using "console dial ...".  When this occurs, the code will break out of the loop and then run monitor_sig_flags, making you no longer to run any Asterisk CLI commands. We'll need a smarter way to figure out if the controlling terminal has been closed when in console mode. By: Matthew Nicholson (mnicholson) 2007-06-25 10:16:30 Another way we can handle this is by checking the exit status of the child process.  See the "Additional Information" on this bug for more details. I wonder what specific cases cause el_gets() to return NULL (other than error cases). By: Brett Bryant (bbryant) 2007-06-25 19:13:46 Looking at the man pages, it seems that el_gets returns NULL when no characters are read from the buffer, instead of an empty string... When I get back to the office i'll work on a way of handling SIGHUP's to work around it... By: Brett Bryant (bbryant) 2007-06-27 15:48:23 Fixed in 1.2, 1.4, trunk. Revisions 72259, 72260, 72261. By: Digium Subversion (svnbot) 2007-07-04 18:38:36 Repository: asterisk Revision: 73297 ------------------------------------------------------------------------ r73297 | file | 2007-07-04 18:38:34 -0500 (Wed, 04 Jul 2007) | 817 lines Merged revisions 72207,72232-72233,72241,72258,72261,72274,72304,72325-72326,72329-72330,72332,72337,72354,72358,72382,72384,72437,72452,72454-72457,72466,72490-72492,72494,72524,72539,72555,72557,72598,72600,72666,72670,72700-72701,72706,72727,72738,72741,72767,72807,72867-72869,72889,72920-72923,72927-72932,72935-72940,72982,72986-72987,73003,73006,73054,73127,73144,73174-73175,73191,73209,73254 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r72207 | kpfleming | 2007-06-27 16:13:54 -0300 (Wed, 27 Jun 2007) | 10 lines Merged revisions 72205 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r72205 | kpfleming | 2007-06-27 14:13:21 -0500 (Wed, 27 Jun 2007) | 2 lines use the proper type for storing group number bits so that if someone specifies 'group=42' it will actually work instead of being silently ignored ........ ................ r72232 | mmichelson | 2007-06-27 16:50:21 -0300 (Wed, 27 Jun 2007) | 10 lines Adding feature to support the storage and retrieval of voicemail greetings using IMAP storage. This feature may be turned on by adding imapgreetings=yes to the general section of voicemail.conf voicemail.conf.sample has details on the options added. As a result, IMAP storage now has RETRIEVE and DISPOSE macros defined. In addition to the IMAP greeting changes, I also have added an enum for the voicemail folders and so now the code should be easier to understand and maintain when it comes to this area. ................ r72233 | file | 2007-06-27 16:57:36 -0300 (Wed, 27 Jun 2007) | 2 lines Fix -T option. (issue ASTERISK-9767 reported by xylome) ................ r72241 | file | 2007-06-27 17:07:46 -0300 (Wed, 27 Jun 2007) | 2 lines Fix up properties. ................ r72258 | file | 2007-06-27 17:26:53 -0300 (Wed, 27 Jun 2007) | 18 lines Merged revisions 72257 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ................ r72257 | file | 2007-06-27 16:25:24 -0400 (Wed, 27 Jun 2007) | 10 lines Merged revisions 72256 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.2 ........ r72256 | file | 2007-06-27 16:23:24 -0400 (Wed, 27 Jun 2007) | 2 lines I may possibly get shot for doing this... but... defer CDR processing until after the channel has been dealt with. This should eliminate all of the issues with channels going funky (SIP/PRI) when you are posting CDRs to a database that is either slow or unavailable and do not want to enable batching. ........ ................ ................ r72261 | bbryant | 2007-06-27 17:47:45 -0300 (Wed, 27 Jun 2007) | 20 lines Merged revisions 72260 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ................ r72260 | bbryant | 2007-06-27 15:46:12 -0500 (Wed, 27 Jun 2007) | 12 lines Merged revisions 72259 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.2 ........ r72259 | bbryant | 2007-06-27 15:43:53 -0500 (Wed, 27 Jun 2007) | 4 lines Fixes 1000ad when controlling terminal disappears. Issue ASTERISK-9371, ASTERISK-9710 ........ ................ ................ r72274 | russell | 2007-06-27 18:09:24 -0300 (Wed, 27 Jun 2007) | 21 lines Merged revisions 72272 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ................ r72272 | russell | 2007-06-27 16:08:34 -0500 (Wed, 27 Jun 2007) | 13 lines Merged revisions 72267 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.2 ........ r72267 | russell | 2007-06-27 16:06:45 -0500 (Wed, 27 Jun 2007) | 5 lines Fix a minor issue with parsing the priority number.  You could have as much whitespace as you want around a numeric priority, but you couldn't have any whitespace around a special priority like "n" or "hint". (issue ASTERISK-9736, reported by mitheloc, fixed by me) ........ ................ ................ r72304 | mattf | 2007-06-27 18:44:13 -0300 (Wed, 27 Jun 2007) | 1 line Let's NOT create a deadlock scenario here ................ r72325 | qwell | 2007-06-27 19:17:59 -0300 (Wed, 27 Jun 2007) | 4 lines Add support for Thai language in say.c Issue 9417, patch by dome, with some cleanup done by me. ................ r72326 | qwell | 2007-06-27 19:27:09 -0300 (Wed, 27 Jun 2007) | 4 lines Fix a segfault when trying to tab complete the "core show uptime" command. Reported in #asterisk-dev on IRC by jcmoore, fixed by me. ................ r72329 | mmichelson | 2007-06-27 19:47:08 -0300 (Wed, 27 Jun 2007) | 4 lines Added ability to customize which buttons control forward, reverse, pause, and stop during message playback. (closes issue 9474, reported and patched by jaroth with modifications by me) ................ r72330 | file | 2007-06-27 19:48:15 -0300 (Wed, 27 Jun 2007) | 18 lines Merged revisions 72328 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ................ r72328 | file | 2007-06-27 18:45:49 -0400 (Wed, 27 Jun 2007) | 10 lines Merged revisions 72327 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.2 ........ r72327 | file | 2007-06-27 18:43:11 -0400 (Wed, 27 Jun 2007) | 2 lines Fix issue where queue log events might be missing. (issue ASTERISK-7561 reported by mtryfoss) ........ ................ ................ r72332 | file | 2007-06-27 19:58:53 -0300 (Wed, 27 Jun 2007) | 10 lines Merged revisions 72331 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r72331 | file | 2007-06-27 18:58:02 -0400 (Wed, 27 Jun 2007) | 2 lines Make payload IDs for iLBC/Speex match to our list. Since these are dynamic payloads the other side shouldn't care. (issue ASTERISK-9150 reported by irroot) ........ ................ r72337 | bbryant | 2007-06-27 20:04:06 -0300 (Wed, 27 Jun 2007) | 18 lines Merged revisions 72335 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ................ r72335 | bbryant | 2007-06-27 18:03:01 -0500 (Wed, 27 Jun 2007) | 10 lines Merged revisions 72333 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.2 ........ r72333 | bbryant | 2007-06-27 17:58:53 -0500 (Wed, 27 Jun 2007) | 2 lines Reverted changes for earlier revisions 72259 to 72261. Issue ASTERISK-9371, ASTERISK-9710 ........ ................ ................ r72354 | file | 2007-06-27 20:13:09 -0300 (Wed, 27 Jun 2007) | 2 lines Add SIPREFERRINGCONTEXT and SIPREFERREDBYHDR variables when a transfer takes place. (issue ASTERISK-8148 reported by jcovert) ................ r72358 | file | 2007-06-27 20:14:39 -0300 (Wed, 27 Jun 2007) | 2 lines Silly jingle... ................ r72382 | file | 2007-06-27 20:26:46 -0300 (Wed, 27 Jun 2007) | 18 lines Merged revisions 72381 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ................ r72381 | file | 2007-06-27 19:25:12 -0400 (Wed, 27 Jun 2007) | 10 lines Merged revisions 72378 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.2 ........ r72378 | file | 2007-06-27 19:24:01 -0400 (Wed, 27 Jun 2007) | 2 lines Update documentation to clarify variable usage with MixMonitor. (issue ASTERISK-9218 reported by netoguy) ........ ................ ................ r72384 | bbryant | 2007-06-27 20:30:31 -0300 (Wed, 27 Jun 2007) | 19 lines Merged revisions 72383 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ................ r72383 | bbryant | 2007-06-27 18:29:14 -0500 (Wed, 27 Jun 2007) | 11 lines Merged revisions 72373 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.2 ........ r72373 | bbryant | 2007-06-27 18:22:13 -0500 (Wed, 27 Jun 2007) | 3 lines Reinstating patch. This actually fixes the problem, however I was running a development branch without it and mistakenly thought it wasn't fixed. Fixes issue ASTERISK-9710, and ASTERISK-9371: 100