This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
avoid local *f = \&foo resetting the method cache
authorsyber <syber@crazypanda.ru>
Wed, 27 Aug 2014 21:05:41 +0000 (01:05 +0400)
committerDavid Mitchell <davem@iabyn.com>
Tue, 2 Sep 2014 11:20:32 +0000 (12:20 +0100)
{
    local *MyClass::func = sub {...}; # LINE A
    ...
} # LINE B

This example caused global method cache reset at both lines A and B
because glob_assign_ref and leave_scope thought that GV's GP refcnt was 2
(because of saving to Save Stack).

Issue has been fixed.
SAVEt_GVSLOT (on leave_scope) now requires refcnt > 2 to reset cache
globally).  Additionally, glob_assign_ref when GvINTRO is set temporarily
decrements gp's refcnt by 1.  This handles all common cases, however there
are still uncommon use cases when perl still resets cache globally, for
example:

{
    local *MyClass::func = sub {...}; # OK
    *MyClass::func = sub {...}; # OOPS :(
} # OK
or
{
    local *MyClass::func = sub {...}; # OK
    {
        local *MyClass::func = sub {...}; # OOPS :(
    } # OOPS :(
} # OK

* OOPS is a line where global cache reset occurs
* OK - one package cache reset

scope.c
sv.c

diff --git a/scope.c b/scope.c
index 66589ab..50036d0 100644 (file)
--- a/scope.c
+++ b/scope.c
@@ -846,7 +846,7 @@ Perl_leave_scope(pTHX_ I32 base)
            {
                if ((char *)svp < (char *)GvGP(ARG2_GV)
                 || (char *)svp > (char *)GvGP(ARG2_GV) + sizeof(struct gp)
-                || GvREFCNT(ARG2_GV) > 1)
+                || GvREFCNT(ARG2_GV) > 2) /* "> 2" to ignore savestack's ref */
                    PL_sub_generation++;
                else mro_method_changed_in(hv);
            }
diff --git a/sv.c b/sv.c
index da21d25..291406e 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -3968,7 +3968,15 @@ S_glob_assign_ref(pTHX_ SV *const dstr, SV *const sstr)
            }
            GvCVGEN(dstr) = 0; /* Switch off cacheness. */
            GvASSUMECV_on(dstr);
-           if(GvSTASH(dstr)) gv_method_changed(dstr); /* sub foo { 1 } sub bar { 2 } *bar = \&foo */
+           if(GvSTASH(dstr)) { /* sub foo { 1 } sub bar { 2 } *bar = \&foo */
+               if (intro && GvREFCNT(dstr) > 1) {
+                   /* temporary remove extra savestack's ref */
+                   --GvREFCNT(dstr);
+                   gv_method_changed(dstr);
+                   ++GvREFCNT(dstr);
+               }
+               else gv_method_changed(dstr);
+           }
        }
        *location = SvREFCNT_inc_simple_NN(sref);
        if (import_flag && !(GvFLAGS(dstr) & import_flag)