[asterisk-dev] tilghman: trunk r232660 - in /trunk: include/asterisk/ res/
russell at digium.com
Fri Dec 4 16:52:54 CST 2009
Tilghman Lesher wrote:
> On Friday 04 December 2009 14:37:41 Russell Bryant wrote:
>> SVN commits to the Digium repositories wrote:
>>> Author: tilghman
>>> Date: Wed Dec 2 18:08:55 2009
>>> New Revision: 232660
>>> URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=232660
>>> Fix multiple issues with musiconhold, which led to classes not getting
>>> destroyed properly. * Classes are now tracked past removal from the core
>>> container, and module removal is actively prevented until all references
>>> are freed. * A hanging reference stored in the channel has been removed.
>>> This could have caused a mismatch and the music state not properly
>>> cleared, if two or more reloads occurred between MOH being stopped and
>>> MOH being restarted. * In certain circumstances, duplicate classes were
>>> * A race existed at reload time between a process being killed and the
>>> thread responsible for reading from the related pipe respawning that
>>> process. * Several reference counts have also been corrected. At least
>>> one could have caused deleted classes to stick around forever, consuming
>>> resources. This originally manifested as MOH external processes that
>>> were not killed at reload time.
>>> (closes issue #16279, closes issue #16207)
>>> Reported by: parisioa, dcabot
>>> 20091202__issue16279__2.diff.txt uploaded by tilghman (license 14)
>>> Tested by: parisioa, tilghman
>> <snip code>
>> Could you please expand on the reasoning behind some of these changes?
>> Specifically, I don't understand why a new deleted classes container is
>> needed, and why a new thread is needed to delete MOH classes. We use
>> reference counted objects from configuration in many places, and nothing
>> like this is used anywhere else.
> The reporter tried unloading the res_musiconhold module after the classes had
> been removed from the container, but before the channels had stopped using
> them. That caused a crash. So I think it's clear that we need to track those
> references and ensure the module cannot be unloaded prior to all references
> being deleted.
The need to track module references makes sense. I would expect module
references to be incremented and decremented while references exist on
channels. I still don't understand the need for the new container and a
>> Also, based on the latest notes in the bug, it looks like there are
>> still problems in this code. The fact that the astobj2 errors occur
>> when the "moh show classes" command runs seems very indicative of a
>> problem in the MOH code.
> It actually looks like memory corruption, where chan_sip used memory after
> it was freed. However, I'll need Valgrind output to confirm.
The valgrind is there now. It does appear to be a MOH bug. The "moh
show classes" command is trying to read from classes that have already
Digium, Inc. | Engineering Manager, Open Source Software
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: www.digium.com & www.asterisk.org
More information about the asterisk-dev