develooper Front page | perl.perl5.porters | Postings from April 2000

Re: [ID 20000409.003] Disappearing constant values [PATCH]

Thread Previous
From:
Eric Blood
Date:
April 10, 2000 21:30
Subject:
Re: [ID 20000409.003] Disappearing constant values [PATCH]
Message ID:
000a01bfa36e$83657e20$0101000a@tlizard.xmission.com
On Monday, April 10, 2000 9:58 PM, "Gurusamy Sarathy" 
<gsar@ActiveState.com> wrote:

> On Mon, 10 Apr 2000 21:27:58 MDT, "Eric Blood" wrote:
> 
> I haven't looked closely, but I'd think we need a couple of lines
> here to avoid a scalar leak:
> 
>       SvREFCNT_dec(cSVOPo->op_sv);
>       cSVOPo->op_sv = Nullsv;
> 

Eeek!! You're right we do need those lines but I don't think its
because we would have scalar leak (the reference count should be
decremented when the op is cleared which is admittedly later than
it needs to be).  However, the real problem is that pp_const 
will use op_sv instead of the pad value if op_sv is set.  So we
could have multiple threads all accessing the same constant SV.
Not good. 

Here's a (hopefully) corrected patch.

--- op.c~ Fri Mar 24 22:24:40 2000
+++ op.c Mon Apr 10 22:21:50 2000
@@ -6417,10 +6417,19 @@
       * for reference counts, sv_upgrade() etc. */
      if (cSVOP->op_sv) {
   PADOFFSET ix = pad_alloc(OP_CONST, SVs_PADTMP);
-  SvREFCNT_dec(PL_curpad[ix]);
-  SvPADTMP_on(cSVOPo->op_sv);
-  PL_curpad[ix] = cSVOPo->op_sv;
-  cSVOPo->op_sv = Nullsv;
+  if (SvPADTMP(cSVOPo->op_sv)) {
+    /* If op_sv is already a PADTMP then it is being used by */
+    /* another pad so make a copy.                           */
+    sv_setsv(PL_curpad[ix],cSVOPo->op_sv);
+    SvREADONLY_on(PL_curpad[ix]);
+    SvREFCNT_dec(cSVOPo->op_sv);
+    cSVOPo->op_sv = Nullsv;
+  } else {
+    SvREFCNT_dec(PL_curpad[ix]);
+    SvPADTMP_on(cSVOPo->op_sv);
+    PL_curpad[ix] = cSVOPo->op_sv;
+    cSVOPo->op_sv = Nullsv;
+  }
   o->op_targ = ix;
      }
 #endif



Thread Previous


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