[asterisk-dev] tilghman: trunk r232660 - in /trunk: include/asterisk/ res/

[Russell Bryant]

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.

Why isn't just removing them from the container when they are removed
from configuration sufficient?  The references on the channels actively
use them should be sufficient to extend the object's lifetime until it
can be destroyed.

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.

-- 
Russell Bryant
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