[asterisk-dev] tilghman: trunk r232660 - in /trunk: include/asterisk/ res/
tlesher at digium.com
Fri Dec 4 15:22:00 CST 2009
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
> > Log:
> > 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
> > possible.
> > * 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
> > Patches:
> > 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
> 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.
Digium, Inc. | Senior Software Developer
twitter: Corydon76 | IRC: Corydon76-dig (Freenode)
Check us out at: www.digium.com & www.asterisk.org
More information about the asterisk-dev