This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
expand the xhv_backreferences code notes
authorDavid Mitchell <davem@iabyn.com>
Thu, 29 Jul 2010 16:56:24 +0000 (17:56 +0100)
committerDavid Mitchell <davem@iabyn.com>
Thu, 29 Jul 2010 16:56:24 +0000 (17:56 +0100)
(so that I don't get so confused when I revisit this code in 5 years time)

hv.c
sv.c

diff --git a/hv.c b/hv.c
index d7d6db1..f110d15 100644 (file)
--- a/hv.c
+++ b/hv.c
@@ -1666,15 +1666,23 @@ S_hfreeentries(pTHX_ HV *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.  */
+           /* weak references: if called from sv_clear(), the backrefs
+            * should already have been killed; if there are any left, its
+            * because we're doing hv_clear() or hv_undef(), and the HV
+            * will continue to live.
+            * Because while freeing the entries we fake up a NULL HvARRAY
+            * (and hence HvAUX), we need to store the backref array
+            * somewhere else; but it still needs to be visible in case
+            * any the things we free happen to call sv_del_backref().
+            * We do this by storing it in magic instead.
+            * If, during the entry freeing, a destructor happens to add
+            * a new weak backref, then sv_add_backref will look in both
+            * places (magic in HvAUX) for the AV, but will create a new
+            * AV in HvAUX if it can't find one. So at the end of the
+            * iteration we have to allow for this. */
 
            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,
+               /* 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) {
diff --git a/sv.c b/sv.c
index 0f91734..c95f9ed 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -5338,13 +5338,13 @@ Perl_sv_add_backref(pTHX_ SV *const tsv, SV *const sv)
 
        av = *avp;
        if (!av) {
-           /* There is no AV in the offical place - try a fixup.  */
+           /* There is no AV in the official place - try a fixup.  */
            MAGIC *const mg = mg_find(tsv, PERL_MAGIC_backref);
 
            if (mg) {
                /* Aha. They've got it stowed in magic.  Bring it back.  */
                av = MUTABLE_AV(mg->mg_obj);
-               /* Stop mg_free decreasing the refernce count.  */
+               /* Stop mg_free decreasing the reference count.  */
                mg->mg_obj = NULL;
                /* Stop mg_free even calling the destructor, given that
                   there's no AV to free up.  */