Under ithreads, convert SVOPs stored in MADPROPs to PADOPs.
authorNicholas Clark <nick@ccl4.org>
Mon, 11 Apr 2011 19:18:49 +0000 (20:18 +0100)
committerNicholas Clark <nick@ccl4.org>
Thu, 19 May 2011 07:55:56 +0000 (08:55 +0100)
Else if a child thread attempts to free an optree with MADPROPs containing OPs
pointing directly to SVs, it will by trying to free SVs to the wrong
interpreter, at which point bad things(tm) happen.

(There still seems to be some fixing needed for the MADPROPs direct pointers,
but as no tests are failing because of them, I'm postponing them until the
failures are addressed)

op.c

diff --git a/op.c b/op.c
index 5740aa6..8591c4b 100644 (file)
--- a/op.c
+++ b/op.c
@@ -9303,6 +9303,47 @@ Perl_rpeep(pTHX_ register OP *o)
     SAVEOP();
     SAVEVPTR(PL_curcop);
     for (; o; o = o->op_next) {
+#if defined(PERL_MAD) && defined(USE_ITHREADS)
+       MADPROP *mp = o->op_madprop;
+       while (mp) {
+           if (mp->mad_type == MAD_OP && mp->mad_vlen) {
+               OP *prop_op = (OP *) mp->mad_val;
+               /* I *think* that this is roughly the right thing to do. It
+                  seems that sometimes the optree hooked into the madprops
+                  doesn't have its next pointers set, so it's not possible to
+                  use them to locate all the OPs needing a fixup. Possibly
+                  it's a bit overkill calling LINKLIST to do this, when we
+                  could instead iterate over the OPs (without changing them)
+                  the way op_linklist does internally. However, I'm not sure
+                  if there are corner cases where we have a chain of partially
+                  linked OPs. Or even if we do, does that matter? Or should
+                  we always iterate on op_first,op_next? */
+               LINKLIST(prop_op);
+               do {
+                   if (prop_op->op_opt)
+                       break;
+                   prop_op->op_opt = 1;
+                   switch (prop_op->op_type) {
+                   case OP_CONST:
+                   case OP_HINTSEVAL:
+                   case OP_METHOD_NAMED:
+                       /* Duplicate the "relocate sv to the pad for thread
+                          safety" code, as otherwise an opfree of this madprop
+                          in the wrong thread will free the SV to the wrong
+                          interpreter.  */
+                       if (((SVOP *)prop_op)->op_sv) {
+                           const PADOFFSET ix = pad_alloc(OP_CONST, SVs_PADTMP);
+                           sv_setsv(PAD_SVl(ix),((SVOP *)prop_op)->op_sv);
+                           SvREFCNT_dec(((SVOP *)prop_op)->op_sv);
+                           ((SVOP *)prop_op)->op_sv = NULL;
+                       }
+                       break;
+                   }
+               } while ((prop_op = prop_op->op_next));
+           }
+           mp = mp->mad_next;
+       }
+#endif
        if (o->op_opt)
            break;
        /* By default, this op has now been optimised. A couple of cases below