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:48Date Closed:2007-03-26 15:29:19
Versions:Frequency of
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