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; } #endifThread Previous