| Summary: | ASTERISK-13479: NULL file descriptors causing GUI to eventually stop functioning | ||
| Reporter: | Ryan Brindley (rbrindley) | Labels: | |
| Date Opened: | 2009-01-29 09:17:29.000-0600 | Date Closed: | 2009-02-10 15:54:47.000-0600 |
| Priority: | Blocker | Regression? | No |
| Status: | Closed/Complete | Components: | Core/ManagerInterface |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) 14364_better.patch ( 1) 14364.patch | |
| Description: | I'm having an issue where file descriptors for GUI manager sessions are NULL, and after a period of time (24 hrs possibly), the GUI stops functioning as it can no longer open config files (see below). fd == -1 in astman_append, should not happen fd == -1 in astman_append, should not happen fd == -1 in astman_append, should not happen == HTTP Manager 'admin' timed out from 127.0.0.1 == HTTP Manager 'admin' logged on from 127.0.0.1 == Parsing '/etc/asterisk/http.conf': == Not found (Too many open files) == Parsing '/etc/asterisk/http.conf': == Not found (Too many open files) == Parsing '/etc/asterisk/http.conf': == Not found (Too many open files) I'm not exactly sure what other info will be useful as Asterisk doesn't crash or deadlock. I haven't checked yet to see if this happens in 1.6.1. It doesn't appear to happen in my 1.4 install. ****** ADDITIONAL INFORMATION ****** OS: ubuntu 8.04 | ||
| Comments: | By: Ryan Brindley (rbrindley) 2009-02-03 09:01:45.000-0600 confirming that these messages are also found in latest svn of 1.6.1. By: Mark Michelson (mmichelson) 2009-02-03 14:41:06.000-0600 First of all, a big thanks to awk for letting me make use of his setup to see how this issue was occurring. All right, so here's the deal, as far as I can tell. There is an originate command that is issued. While the originate command is being executed, the same manager session is used for handling more http requests. During that time, the mansession object has its f and fd fields altered. The result of this is a large fd leak. One way to handle this is to keep the mansession locked for the entirety of any manager action. This way, actions are guaranteed to execute in the order we receive them, and we know that what was set at the beginning of the action will still be set at the end. While this would have the benefit of fixing this particular issue, I don't like the performance penalty that would be incurred as a result. I'll try to come up with something better. By: Mark Michelson (mmichelson) 2009-02-04 10:57:47.000-0600 I've uploaded a patch that fixes the issue of leaked fds. The solution I have come up with is to use a boolean and a thread condition variable to ensure that no two commands can overlap. While this fixes the reported problem, this solution does not work well in reality. The reason for this is that WaitEvent manager actions used by the GUI block other actions from being performed. On an idle system, this can mean that the system status page, for example, takes several minutes to completely load. On a loaded system (even just a call or two), however, things actually move quickly and the page will load promptly. awk and I are working together to try to come up with something better. He's looking at the GUI side of things to see if there are improvements that may be made on that end. By: Mark Michelson (mmichelson) 2009-02-10 13:58:56.000-0600 I've created a new method of fixing this issue, and I have posted the diff on reviewboard. It may be found here: http://reviewboard.digium.com/r/148/ I have also uploaded the diff here as 14364_better.patch. Feel free to test it out and let me know what you think. By: Digium Subversion (svnbot) 2009-02-10 15:45:15.000-0600 Repository: asterisk Revision: 174764 U trunk/main/manager.c ------------------------------------------------------------------------ r174764 | mmichelson | 2009-02-10 15:45:14 -0600 (Tue, 10 Feb 2009) | 21 lines Fix an fd leak that would occur in HTTP AMI sessions The explanation behind this fix is a bit complicated, and I've already typed it up in the code as a huge comment inside of manager.c, so I'll give the abridged version here. We needed a way to separate action-specific data from session-specific data. Unfortunately, the only way to maintain API compatibility and to not have to change every single manager action was to rename the current mansession structure and wrap it inside a new mansession structure which actually contains action- specific data. (closes issue ASTERISK-13479) Reported by: awk Patches: 14364_better.patch uploaded by putnopvut (license 60) Tested by: putnopvut Review: http://reviewboard.digium.com/r/148/ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=174764 By: Digium Subversion (svnbot) 2009-02-10 15:49:14.000-0600 Repository: asterisk Revision: 174765 _U branches/1.6.0/ U branches/1.6.0/main/manager.c ------------------------------------------------------------------------ r174765 | mmichelson | 2009-02-10 15:49:14 -0600 (Tue, 10 Feb 2009) | 29 lines Merged revisions 174764 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r174764 | mmichelson | 2009-02-10 15:45:14 -0600 (Tue, 10 Feb 2009) | 21 lines Fix an fd leak that would occur in HTTP AMI sessions The explanation behind this fix is a bit complicated, and I've already typed it up in the code as a huge comment inside of manager.c, so I'll give the abridged version here. We needed a way to separate action-specific data from session-specific data. Unfortunately, the only way to maintain API compatibility and to not have to change every single manager action was to rename the current mansession structure and wrap it inside a new mansession structure which actually contains action- specific data. (closes issue ASTERISK-13479) Reported by: awk Patches: 14364_better.patch uploaded by putnopvut (license 60) Tested by: putnopvut Review: http://reviewboard.digium.com/r/148/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=174765 By: Digium Subversion (svnbot) 2009-02-10 15:54:46.000-0600 Repository: asterisk Revision: 174769 _U branches/1.6.1/ U branches/1.6.1/main/manager.c ------------------------------------------------------------------------ r174769 | mmichelson | 2009-02-10 15:54:46 -0600 (Tue, 10 Feb 2009) | 29 lines Merged revisions 174764 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r174764 | mmichelson | 2009-02-10 15:45:14 -0600 (Tue, 10 Feb 2009) | 21 lines Fix an fd leak that would occur in HTTP AMI sessions The explanation behind this fix is a bit complicated, and I've already typed it up in the code as a huge comment inside of manager.c, so I'll give the abridged version here. We needed a way to separate action-specific data from session-specific data. Unfortunately, the only way to maintain API compatibility and to not have to change every single manager action was to rename the current mansession structure and wrap it inside a new mansession structure which actually contains action- specific data. (closes issue ASTERISK-13479) Reported by: awk Patches: 14364_better.patch uploaded by putnopvut (license 60) Tested by: putnopvut Review: http://reviewboard.digium.com/r/148/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=174769 | ||