[asterisk-dev] [Code Review] [16388] MOH refcount fix

[Russell Bryant]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/442/#review1285
-----------------------------------------------------------



/trunk/res/res_musiconhold.c
<https://reviewboard.asterisk.org/r/442/#comment2949>

    If the only reference was the reference owned by the container, wouldn't you want to check for a return value of 2 here?  ao2_ref() returns the _previous_ value of the reference count.  In this error case, I would expect 2: 1 for the container, and 1 that was returned by the ao2_find().



/trunk/res/res_musiconhold.c
<https://reviewboard.asterisk.org/r/442/#comment2950>

    I don't think this is the best place to unref the module.  I think it should only be where the state object is destroyed (perhaps just in ast_moh_cleanup()?).
    
    With the way the code is now, it is possible to leak memory if this module gets unloaded.  Take the case where MOH has been started and stopped.  The state is still on the ast_channel, but the module ref does not exist.  If the module gets unloaded, we will leak the state when the channel goes away.


- Russell


On 2009-12-07 16:55:33, Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/442/
> -----------------------------------------------------------
> 
> (Updated 2009-12-07 16:55:33)
> 
> 
> Review request for Asterisk Developers and Russell Bryant.
> 
> 
> Summary
> -------
> 
> See issue #16388
> 
> 
> This addresses bug 16388.
>     https://issues.asterisk.org/view.php?id=16388
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_musiconhold.c 232854 
> 
> Diff: https://reviewboard.asterisk.org/r/442/diff
> 
> 
> Testing
> -------
> 
> Works for the reporter
> 
> 
> Thanks,
> 
> Tilghman
> 
>