Re: mod_perl 2.0.5 -> Attempt to free unreferenced scalar on startup

Niko Tyni <[email protected]> Sat, 7 Jan 2012 08:53:32 +0200

Newsgroups	gmane.comp.apache.mod-perl
Message-ID	<[email protected]>

[I sent earlier versions of these to [email protected] on Dec 14th and 17th,
 Message-ID: <[email protected]> and
 Message-ID: <[email protected]>,
 but they never made it through to the list for some reason. Retrying on the
 modperl list, apologies for any duplicates.]
 
On Sat, Aug 06, 2011 at 08:02:20PM +0200, [email protected] wrote:
 
> while trying to get Embperl ready with Perl 5.14.1, I found an the
> following message in the error log:

> Attempt to free unreferenced scalar: SV 0x7fc218, Perl interpreter:
> 0x7cfdb0 during global destruction.

We're also seeing this in Debian now that we've switched to Perl 5.14,
(see http://bugs.debian.org/650675 [cc'd.])

I managed to cut the problem down to the small attached program,
which embeds perl and mimics modperl_perl_core_global_init() for the
essential part of

        GV *gv = gv_fetchpv("myglob", TRUE, SVt_PVCV);
        GvCV_set(gv, get_cv("myfunc", TRUE));

The warnings only seem to happen with -Dusethreads, and I've bisected that
they started with 5.13.6 - specifically

 http://perl5.git.perl.org/perl.git/commit/ca556bcdca736b2f85c11650c70b2371169c0225

commit ca556bcdca736b2f85c11650c70b2371169c0225
Author: David Mitchell <[email protected]>
Date:   Sun Sep 19 12:33:04 2010 +0100

    [perl #40389] perl_destruct() leaks PL_defstash
    
    With PERL_DESTRUCT_LEVEL >= 1, PL_defstash is explicitly freed,
    but doesn't actually get freed at that point due to a reference loop
    between %:: and *::. Break that loop to ensure that PL_defstash gets freed
    at that point. Actually, its not as serious as it sounds, as it would get
    freed a bit later anyway by sv_clean_all(), but this new way has these
    benefits:
    
    * it gets freed where you expect it to be
    * it gets freed cleanly, rather than by the more brutal sv_clean_all()
      (which can leave dangling pointers to freed SVs)
    * since its freed while *not* under the influence of
      PL_in_clean_all = TRUE, it's more likely to flag up bugs related to
      double-freeing etc. Indeed, the two previous commits to this are a
      result of that.

My limited understanding is that the CV pointer needs its refcount
incremented along with the GvCV_set() call. See the attached proposed
patches which fix all the dozens of such warnings in t/logs/error_log
after running the whole test suite.

Feel free to tell me if I got it all wrong, though :)

Cheers,
-- 
Niko Tyni   [email protected]

e.c (text/x-csrc, 687 B)

#include <EXTERN.h>               /* from the Perl distribution     */
#include <perl.h>                 /* from the Perl distribution     */

/* compatibility for perl <= 5.12 */
#ifndef GvCV_set
#    define GvCV_set(gv, cv) (GvCV(gv)=(cv))
#endif

static PerlInterpreter *my_perl;  /***    The Perl interpreter    ***/

int main(int argc, char **argv, char **env)
{
	PERL_SYS_INIT3(&argc,&argv,&env);
	my_perl = perl_alloc();
	perl_construct(my_perl);
	
	perl_parse(my_perl, NULL, argc, argv, (char **)NULL);
	GV *gv = gv_fetchpv("myglob", TRUE, SVt_PVCV);
	GvCV_set(gv, get_cv("myfunc", TRUE));
	
	perl_run(my_perl);
	perl_destruct(my_perl);
	perl_free(my_perl);
	PERL_SYS_TERM();
}

0001-Fix-a-reference-counting-bug-uncovered-by-Perl-5.13..patch (text/x-diff, 1.3 KB)

>From 2099956795016f3bd08768dda195aeec7dc2267b Mon Sep 17 00:00:00 2001
From: Niko Tyni <[email protected]>
Date: Wed, 14 Dec 2011 22:51:27 +0200
Subject: [PATCH] Fix a reference counting bug uncovered by Perl 5.13.6

As seen in
 http://bugs.debian.org/650675
 http://article.gmane.org/gmane.comp.apache.mod-perl.devel/9928

perl since 5.13.6 with -Dusethreads makes mod_perl2 issue warnings like

 Attempt to free unreferenced scalar: SV 0x7f9c0c347490, Perl interpreter: 0x7f9c0c1a2dd0 during global destruction.

on startup regardless of the Perl code executed.

The double-freed scalar is the CV pointer for ModPerl::Util::exit()
whose reference count wasn't increased properly.
---
 src/modules/perl/modperl_perl.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/modules/perl/modperl_perl.c b/src/modules/perl/modperl_perl.c
index e992df4..1a2b3ce 100644
--- a/src/modules/perl/modperl_perl.c
+++ b/src/modules/perl/modperl_perl.c
@@ -55,7 +55,9 @@ void modperl_perl_core_global_init(pTHX)
 
     while (cglobals->name) {
         GV *gv = gv_fetchpv(cglobals->core_name, TRUE, SVt_PVCV);
-        GvCV_set(gv, get_cv(cglobals->sub_name, TRUE));
+        CV *cv = get_cv(cglobals->sub_name, TRUE);
+        GvCV_set(gv, cv);
+        SvREFCNT_inc(cv);
         GvIMPORTED_CV_on(gv);
         cglobals++;
     }
-- 
1.7.7.3

0002-Fix-another-reference-counting-bug-uncovered-by-Perl.patch (text/x-diff, 1.2 KB)

From cbf08a2de6f967863d764eb0bc620178c4ed43fe Mon Sep 17 00:00:00 2001
From: Niko Tyni <[email protected]>
Date: Sat, 17 Dec 2011 09:16:22 +0200
Subject: [PATCH 2/2] Fix another reference counting bug uncovered by Perl
 5.13.6

As seen in
 http://bugs.debian.org/650675
 http://article.gmane.org/gmane.comp.apache.mod-perl.devel/9928

perl since 5.13.6 with -Dusethreads makes mod_perl2 issue warnings like

 Attempt to free unreferenced scalar: SV 0x7f9c0c347490, Perl interpreter: 0x7f9c0c1a2dd0 during global de
struction.

An earlier commit fixed some of these in modperl_perl_core_global_init(),
follow the suit with new_constsub().
---
 src/modules/perl/modperl_const.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/modules/perl/modperl_const.c b/src/modules/perl/modperl_const.c
index 8732d4f..39a3ec9 100644
--- a/src/modules/perl/modperl_const.c
+++ b/src/modules/perl/modperl_const.c
@@ -51,7 +51,9 @@ static void new_constsub(pTHX_ constants_lookup lookup,
             gv_init(alias, caller_stash, name, name_len, TRUE);
         }
 
-        GvCV_set(alias, GvCV(*gvp));
+        CV *cv = GvCV(*gvp);
+        GvCV_set(alias, cv);
+        SvREFCNT_inc(cv);
     }
 }
 
-- 
1.7.7.3