[Home]

Summary:ASTERISK-22252: res_musiconhold cleanup - REF_DEBUG reload warnings and ref leaks
Reporter:Walter Doekes (wdoekes)Labels:
Date Opened:2013-08-05 04:41:08Date Closed:2013-09-12 11:45:03
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Resources/res_musiconhold
Versions:1.8.23.0 Frequency of
Occurrence
Related
Issues:
is related toASTERISK-21775 FPE during MOH playback
Environment:Attachments:( 0) 18_moh_debug_ref_patch.diff
Description:Actually tested on trunk and 10. But the problem is the same:

Add REF_DEBUG on top of {{res/res_musiconhold.c}} and we get this, alternating:
{noformat}
*CLI> moh reload
[2013-08-05 11:34:03] WARNING[31076]: res_musiconhold.c:257 _mohclass_unref: Attempt to unref mohclass 0x7f50902826a8 (default) when only 1 ref remained, and class is still in a container! (at res_musiconhold.c:1324 (_moh_register))

*CLI> moh reload
(nothing)
{noformat}

I don't see what that custom REF_DEBUG code is doing there. Removing the custom {{_mohclass_unref}} makes the warning go away. refcounter(1) is now happy.

For the record, the offending code:
{noformat}
#define mohclass_unref(class,string) _mohclass_unref(class, string, __FILE__,__LINE__,__PRETTY_FUNCTION__)
static struct mohclass *_mohclass_unref(struct mohclass *class, const char *tag, const char *file, int line, const char *funcname)
{
       struct mohclass *dup;
       if ((dup = ao2_find(mohclasses, class, OBJ_POINTER))) {
               if (__ao2_ref_debug(dup, -1, (char *) tag, (char *) file, line, funcname) == 2) {
                       FILE *ref = fopen("/tmp/refs", "a");
                       if (ref) {
                               fprintf(ref, "%p =1   %s:%d:%s (%s) BAD ATTEMPT!\n", class, file, line, funcname, tag);
                               fclose(ref);
                       }
                       ast_log(LOG_WARNING, "Attempt to unref mohclass %p (%s) when only 1 ref remained, and class is still in a container! (at %s:%d (%s))\n",
                               class, class->name, file, line, funcname);
               } else {
                       ao2_ref(class, -1);
               }
       } else {
               ao2_t_ref(class, -1, (char *) tag);
       }
       return NULL;
}
{noformat}
Comments:By: Michael L. Young (elguero) 2013-08-05 09:56:58.319-0500

Just to help add some history around that custom _mohclass_unref function.  That code was added in r233718 as part of a fix for ASTERISK-15271.  The code review is here: https://reviewboard.asterisk.org/r/442/.
By: Jonathan Rose (jrose) 2013-09-04 15:19:46.063-0500

I did some debugging to our debugging code... and came across this little bit of inception.

The reason this was causing problems was because dup was pointing to the wrong object (it was pointing to a separate object in the container with the same name -- it appears that we have both the old one and the new one in the container simultaneously for a while during the reload, which is somewhat scary in itself). ao2_find can't be used there since it can match the entries from the previous load. Fixing it is pretty trivial though and I don't think it's necessary to remove the debugging code entirely. I'm supplying a new patch to solve this particular issue, and I would request that we not remove the debug code in https://reviewboard.asterisk.org/r/2742/