[perl #85026] Deleting the current iterator in void context
authorTon Hospel <me-02@ton.iguana.be>
Fri, 20 May 2011 00:05:16 +0000 (17:05 -0700)
committerFather Chrysostomos <sprout@cpan.org>
Fri, 20 May 2011 01:01:46 +0000 (18:01 -0700)
Looking at the delete code I see another strange thing. If the delete
of the current iterator is done with the G_DISCARD flag, the corres-
ponding value is not freed and survives until the lazy deleted entry
gets removed on the next hash iteration. This is easily demonstrated
like this:

perl -wle '
sub DESTROY { print "DESTROY" }
%a=(a=>bless[]);
each %a;
delete $a{a};
print "END"
'

This prints:

END
DESTROY

notice the difference with:

perl -wle '
sub DESTROY { print "DESTROY" }
%hash = (a => bless[]);
each %hash;
$dummy = delete $hash{a}; $dummy = 0;
print "END"
'

This prints:

DESTROY
END

This is easily solved by always replacing the deleted entry value with
&PL_sv_placeholder. It actually simplifies the code a bit except for the
fact that the mro_method_changed from free_hash_ent now has to be done
in hv_delete

hv.c

diff --git a/hv.c b/hv.c
index a445a2f..2bb8ab0 100644 (file)
--- a/hv.c
+++ b/hv.c
@@ -1040,12 +1040,18 @@ S_hv_delete_common(pTHX_ HV *hv, SV *keysv, const char *key, STRLEN klen,
                    mro_changes = 1;
        }
 
-       if (d_flags & G_DISCARD)
-           sv = NULL;
-       else {
-           sv = sv_2mortal(HeVAL(entry));
-           HeVAL(entry) = &PL_sv_placeholder;
-       }
+       if (d_flags & G_DISCARD) {
+           sv = HeVAL(entry);
+           if (sv) {
+               /* deletion of method from stash */
+               if (isGV(sv) && isGV_with_GP(sv) && GvCVu(sv)
+                && HvENAME_get(hv))
+                   mro_method_changed_in(hv);
+               SvREFCNT_dec(sv);
+               sv = NULL;
+           }
+       } else sv = sv_2mortal(HeVAL(entry));
+       HeVAL(entry) = &PL_sv_placeholder;
 
        /*
         * If a restricted hash, rather than really deleting the entry, put
@@ -1053,13 +1059,11 @@ S_hv_delete_common(pTHX_ HV *hv, SV *keysv, const char *key, STRLEN klen,
         * we can still access via not-really-existing key without raising
         * an error.
         */
-       if (SvREADONLY(hv)) {
-           SvREFCNT_dec(HeVAL(entry));
-           HeVAL(entry) = &PL_sv_placeholder;
+       if (SvREADONLY(hv))
            /* We'll be saving this slot, so the number of allocated keys
             * doesn't go down, but the number placeholders goes up */
            HvPLACEHOLDERS(hv)++;
-       else {
+       else {
            *oentry = HeNEXT(entry);
            if (SvOOK(hv) && entry == HvAUX(hv)->xhv_eiter /* HvEITER(hv) */)
                HvLAZYDEL_on(hv);