develooper Front page | perl.perl5.porters | Postings from October 2014

about FC commit "CV-based slab allocation for ops"

Thread Next
From:
bulk88
Date:
October 28, 2014 10:33
Subject:
about FC commit "CV-based slab allocation for ops"
Message ID:
BLU436-SMTP173078C0FE361089C80AAC2DF9F0@phx.gbl
In commit "CV-based slab allocation for ops" 
http://perl5.git.perl.org/perl.git/commitdiff/8be227ab5eaa23f2d21fd15f70190e494496dcbe 
  I see

------------------------------------------------------
diff --git a/pad.c b/pad.c

index 5473b64..9f6ccb8 100644 (file)

--- a/pad.c
+++ b/pad.c
@@ -333,6 +333,7 @@ Perl_cv_undef(pTHX_ CV *cv)
  {
      dVAR;
      const PADLIST *padlist = CvPADLIST(cv);
+    bool const slabbed = !!CvSLABBED(cv);

      PERL_ARGS_ASSERT_CV_UNDEF;

@@ -346,6 +347,7 @@ Perl_cv_undef(pTHX_ CV *cv)
      }
      CvFILE(cv) = NULL;

+    CvSLABBED_off(cv);
      if (!CvISXSUB(cv) && CvROOT(cv)) {
         if (SvTYPE(cv) == SVt_PVCV && CvDEPTH(cv))
             Perl_croak(aTHX_ "Can't undef active subroutine");
@@ -353,11 +355,29 @@ Perl_cv_undef(pTHX_ CV *cv)

         PAD_SAVE_SETNULLPAD();

+#ifndef PL_OP_SLAB_ALLOC
+       if (slabbed) OpslabREFCNT_dec_padok(OpSLAB(CvROOT(cv)));
+#endif
         op_free(CvROOT(cv));
         CvROOT(cv) = NULL;
         CvSTART(cv) = NULL;
         LEAVE;
      }
+#ifndef PL_OP_SLAB_ALLOC
+    else if (slabbed && CvSTART(cv)) {
+       ENTER;
+       PAD_SAVE_SETNULLPAD();
+
+       /* discard any leaked ops */
+       opslab_force_free((OPSLAB *)CvSTART(cv));
+       CvSTART(cv) = NULL;
+
+       LEAVE;
+    }
+# ifdef DEBUGGING
+    else if (slabbed) Perl_warn(aTHX_ "Slab leaked from cv %p", cv);
+# endif
+#endif
      SvPOK_off(MUTABLE_SV(cv));         /* forget prototype */
      CvGV_set(cv, NULL);
------------------------------------------------------

So why does CvSLABBED have to be turned off before "if (slabbed) 
OpslabREFCNT_dec_padok(OpSLAB(CvROOT(cv)));" and/or 
"PAD_SAVE_SETNULLPAD();" executes, or in the CvSTART branch, before 
"opslab_force_free((OPSLAB *)CvSTART(&cvbody));" executes?

Can we leave the turning off of CvSLABBED flag until the very end of 
Perl_cv_undef_flags to be done in the existing
------------------------------------------------------
     /* delete all flags except WEAKOUTSIDE and CVGV_RC, which indicate the
      * ref status of CvOUTSIDE and CvGV, and ANON, NAMED and
      * LEXICAL, which are used to determine the sub's name.  */
     CvFLAGS(&cvbody) &= (CVf_WEAKOUTSIDE|CVf_CVGV_RC|CVf_ANON|CVf_LEXICAL
		   |CVf_NAMED);
}
-------------------------------------------------------
statement or there is a croak/exception risk before reaching that 
statement at the very end of Perl_cv_undef_flags which means CvSLABBED 
must be turned off earlier so if Perl_cv_undef_flags is recursed on the 
same CV* CvSLABBED is off and a double free doesn't occur on the PP op 
tree (or if OpslabREFCNT_dec_padok croaks, it needs to execute again on 
the 2nd try or something like that, which means turning off CvSLABBED 
early is a bug in and of itself?), or it is impossible for 
Perl_cv_undef_flags to croak in any normal usage?

Why this post? Working on a refactoring patch. See attached if anyone is 
curious.

Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About