Stop delete local $ENV{foo} from leaking

It was only leaking when the env var did not already exist.

The code in question in pp.c:S_do_delete_local was calling hv_delete
to delete the element, which autovivifies, deletes, and returns a mag-
ical mortal for magical hashes.  It was assuming that if a value was
returned the element must have existed, so it was calling SvREFCNT_inc
on the returned mortal to de-mortalize it (since it has to be
restored).  Whether the element had existed previously was already
recorded in a bool named ‘preeminent’ (strange name).  This variable
should be checked before the SvREFCNT_inc.

I found the same bug in the array code path, potentially affecting
@- and @+, so I fixed it.  But I didn’t write a test, as that would
involve a custom regexp engine.

diff --git a/pp.c b/pp.c
index ea7b4c2..3eb3cea 100644 (file)
--- a/pp.c
+++ b/pp.c
@@ -4511,7 +4511,8 @@ S_do_delete_local(pTHX)
                }
                else {
                    sv = hv_delete_ent(hv, keysv, 0, 0);
-                   SvREFCNT_inc_simple_void(sv); /* De-mortalize */
+                   if (preeminent)
+                       SvREFCNT_inc_simple_void(sv); /* De-mortalize */
                }
                if (preeminent) {
                    if (!sv) DIE(aTHX_ PL_no_helem_sv, SVfARG(keysv));
@@ -4546,7 +4547,8 @@ S_do_delete_local(pTHX)
                    }
                    else {
                        sv = av_delete(av, idx, 0);
-                       SvREFCNT_inc_simple_void(sv); /* De-mortalize */
+                       if (preeminent)
+                          SvREFCNT_inc_simple_void(sv); /* De-mortalize */
                    }
                    if (preeminent) {
                        save_aelem_flags(av, idx, &sv, SAVEf_KEEPOLDELEM);

diff --git a/t/op/svleak.t b/t/op/svleak.t
index 71bfbb7..b1a5b32 100644 (file)
--- a/t/op/svleak.t
+++ b/t/op/svleak.t
@@ -15,7 +15,7 @@ BEGIN {
 
 use Config;
 
-plan tests => 124;
+plan tests => 125;
 
 # 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
@@ -69,6 +69,14 @@ leak(5, 0, sub {},                 "basic check 1 of leak test infrastructure");
 leak(5, 0, sub {push @a,1;pop @a}, "basic check 2 of leak test infrastructure");
 leak(5, 1, sub {push @a,1;},       "basic check 3 of leak test infrastructure");
 
+# delete
+{
+    my $key = "foo";
+    $key++ while exists $ENV{$key};
+    leak(2, 0, sub { delete local $ENV{$key} },
+       'delete local on nonexistent env var');
+}
+
 # Fatal warnings
 my $f = "use warnings FATAL =>";
 my $all = "$f 'all';";