| Summary: | ASTERISK-08990: [patch] Eliminate the comment related global vars from main/config.c | ||
| Reporter: | Steve Murphy (murf) | Labels: | |
| Date Opened: | 2007-03-12 13:36:48 | Date Closed: | 2007-03-26 15:29:19 |
| Priority: | Minor | Regression? | No |
| Status: | Closed/Complete | Components: | Core/Configuration |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) diff.CB | |
| Description: | A problem was encountered in overlapping config loads; It would be better to eliminate the global vars for comments from config.c (from an architectural standpoint). | ||
| Comments: | By: Anthony LaMantia-2 (anthonyl) 2007-03-12 16:29:20 can you elaborate on the issue/post a patch? By: Steve Murphy (murf) 2007-03-13 00:15:06 I shall do both. 1. I've attached the patch. 2. Elaboration: This morning, Jason was able to crash asterisk consistently by hitting a button as many times as he could per second, in the GUI. He noted that it died in the CB_RESET func in main/config.c, the heart of the config file reader, in the comment capture code I added some months ago. It was reading in a fairly hefty extensions.conf file. I checked, there is no no locking for reading in the config files. You can have more than one thread reading in files simultaneously. I guessed that two overlapping reads were going on, and the first to finish will zero out the lline_buffer and comment_buffer global vars, and then the other thread will crash when it tried to RESET those buffers. So, we threw in a lock, for the duration of the read. That fixed the problem, but won't help with thruput, as it might be nice to load, say, a voicemail.conf while loading a sip.conf, etc. Kevin felt that the buffer pointers that I put in the global variable space might be better made local to the functions, which made sense to me, so I spent a few hours and store these pointers in the stack. Now, multiple threads can again run without fear of clashing comment code. We could remove the locking now, or use R/W locks instead, as there still is a need to make sure that the engine lists aren't changed while a file is being read in. Not a huge, dire need, but to be academical about it, you still need something. I sent Jason the code to test with the GUI, and if all goes well, we'll commit this stuff to trunk, mayhaps 1.4. By: Steve Murphy (murf) 2007-03-26 15:29:19 patch tested by Qwell; 1.4 fixed via r. 59225 trunk via r. 59226 | ||