This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
Stop local $_[0] from leaking
authorFather Chrysostomos <sprout@cpan.org>
Sun, 18 Nov 2012 06:49:57 +0000 (22:49 -0800)
committerFather Chrysostomos <sprout@cpan.org>
Sun, 18 Nov 2012 22:02:35 +0000 (14:02 -0800)
local $_[0] puts the current $_[0] on to the savestack and gives the
array a brand new SV.  If the array is not marked REAL, it holds no
reference counts on its elements.  @_ is surreal by default.

The localisation code was making @_ hold a reference count on its new
element.  The restore code was assuming it had a reference count, so
everything worked out if $_[0] was not modified after localisation.

But if the array is surreal, then modifications to it will assume that
it does *not* hold a reference count on $_[0].  So doing shift, or
@_=undef would cause the new element to leak.

Also, taking a reference to the array (\@_) will trigger, making
the reference count of all elements increeas, likwies leaking the
new element.

Since there is only one REAL flag, which indicates that all elements
of the array are reference-counted, we cannot have some elements ref-
erence-counted and some not (which local $_[0] does), and have every-
thing behave correctly.

So the only solution is to reify arrays before localising
their elements.

scope.c
t/op/svleak.t

diff --git a/scope.c b/scope.c
index 8b82381..75b2835 100644 (file)
--- a/scope.c
+++ b/scope.c
@@ -627,9 +627,10 @@ Perl_save_aelem_flags(pTHX_ AV *av, I32 idx, SV **sptr, const U32 flags)
     SvGETMAGIC(*sptr);
     save_pushptri32ptr(SvREFCNT_inc_simple(av), idx, SvREFCNT_inc(*sptr),
                       SAVEt_AELEM);
-    /* if it gets reified later, the restore will have the wrong refcnt */
+    /* The array needs to hold a reference count on its new element, so it
+       must be AvREAL. */
     if (!AvREAL(av) && AvREIFY(av))
-       SvREFCNT_inc_void(*sptr);
+       av_reify(av);
     save_scalar_at(sptr, flags); /* XXX - FIXME - see #60360 */
     if (flags & SAVEf_KEEPOLDELEM)
        return;
index d38c92d..7b53092 100644 (file)
@@ -15,7 +15,7 @@ BEGIN {
 
 use Config;
 
-plan tests => 69;
+plan tests => 71;
 
 # run some code N times. If the number of SVs at the end of loop N is
 # greater than (N-1)*delta at the end of loop 1, we've got a leak
@@ -197,6 +197,12 @@ eleak(2, 0, 'no warnings; sub {1 1}', 'anon sub with syntax error');
 eleak(2, 0, 'no warnings; use feature ":all"; my sub a{1 1}',
      'my sub with syntax error');
 
+# Reification (or lack thereof)
+leak(2, 0, sub { sub { local $_[0]; shift }->(1) },
+    'local $_[0] on surreal @_, followed by shift');
+leak(2, 0, sub { sub { local $_[0]; \@_ }->(1) },
+    'local $_[0] on surreal @_, followed by reification');
+
 # Syntax errors
 eleak(2, 0, '"${<<END}"
                  ', 'unterminated here-doc in quotes in multiline eval');