process xhv_backreferences early in S_hfreeentries

When deleting a stash, make the algorithm
    GvSTASH($_) = NULL for (@xhv_backreferences);
    delete xhv_backreferences;
    free each stash entry;

Previously the algorithm was

    hide xhv_backreferences as ordinary backref magic;
    free each stash entry:
	this may trigger a sv_del_backref() for each GV being freed
    delete @xhv_backreferences

The new method is:

* more efficient: one scan through @xhv_backreferences rather than lots of
calls to sv_del_backref(), removing elements one by one;

* makes the code simpler; the 'hide xhv_backreferences as backref magic'
hack no longer needs to be done

* removes a bug whereby GVs that had a refcnt > 1 (the usual case) were
left with a GvSTASH pointing to the freed stash; it's now NULL instead. I
couldn't think of a test for this.

There are two drawbacks:

* If the GV gets freed at the same time as the stash, the freeing code
sees the GV with a GVSTASH of NULL rather than still pointing to the
stash.

* As far as I can see, the only difference this currently makes is that
mro_method_changed_in() is no longer called by sv_clear(), but since we're
blowing away the whole stash anyway, method resolution doesn't really
bother us any more.

At some point in the future I might set GvSTASH to %__ANON__ rather than
NULL.

1 files changed, 7 insertions, 40 deletions

diff --git a/hv.c b/hv.c
index 880a46d4b6..f94d6d45dd 100644
--- a/hv.c
+++ b/hv.c
@@ -1708,28 +1708,13 @@ S_hfreeentries(pTHX_ HV *hv)
 	if (SvOOK(hv)) {
 	    HE *entry;
             struct mro_meta *meta;
-	    struct xpvhv_aux *iter = HvAUX(hv);
-	    /* If there are weak references to this HV, we need to avoid
-	       freeing them up here.  In particular we need to keep the AV
-	       visible as what we're deleting might well have weak references
-	       back to this HV, so the for loop below may well trigger
-	       the removal of backreferences from this array.  */
-
-	    if (iter->xhv_backreferences) {
-		/* So donate them to regular backref magic to keep them safe.
-		   The sv_magic will increase the reference count of the AV,
-		   so we need to drop it first. */
-		SvREFCNT_dec(iter->xhv_backreferences);
-		if (AvFILLp(iter->xhv_backreferences) == -1) {
-		    /* Turns out that the array is empty. Just free it.  */
-		    SvREFCNT_dec(iter->xhv_backreferences);
+	    struct xpvhv_aux * const iter = HvAUX(hv);
+	    SV *const av = iter->xhv_backreferences;
 
-		} else {
-		    sv_magic(MUTABLE_SV(hv),
-			     MUTABLE_SV(iter->xhv_backreferences),
-			     PERL_MAGIC_backref, NULL, 0);
-		}
-		iter->xhv_backreferences = NULL;
+	    if (av) {
+		Perl_sv_kill_backrefs(aTHX_ MUTABLE_SV(hv), av);
+		SvREFCNT_dec(av);
+		iter->xhv_backreferences = 0;
 	    }
 
 	    entry = iter->xhv_eiter; /* HvEITER(hv) */
@@ -1765,7 +1750,7 @@ S_hfreeentries(pTHX_ HV *hv)
 	}
 
 	/* make everyone else think the array is empty, so that the destructors
-	 * called for freed entries can't recusively mess with us */
+	 * called for freed entries can't recursively mess with us */
 	HvARRAY(hv) = NULL;
 	((XPVHV*) SvANY(hv))->xhv_keys = 0;
 
@@ -2068,24 +2053,6 @@ Perl_hv_backreferences_p(pTHX_ HV *hv) {
     return &(iter->xhv_backreferences);
 }
 
-void
-Perl_hv_kill_backrefs(pTHX_ HV *hv) {
-    AV *av;
-
-    PERL_ARGS_ASSERT_HV_KILL_BACKREFS;
-
-    if (!SvOOK(hv))
-	return;
-
-    av = HvAUX(hv)->xhv_backreferences;
-
-    if (av) {
-	HvAUX(hv)->xhv_backreferences = 0;
-	Perl_sv_kill_backrefs(aTHX_ MUTABLE_SV(hv), av);
-	SvREFCNT_dec(av);
-    }
-}
-
 /*
 hv_iternext is implemented as a macro in hv.h