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

Re: Fixed: [perl #24255] shared hash ref memory leak in 5.8.1

From:
Nicholas Clark
Date:
October 23, 2003 14:36
Subject:
Re: Fixed: [perl #24255] shared hash ref memory leak in 5.8.1
Message ID:
20031023213020.GT22077@plum.flirble.org
On Thu, Oct 23, 2003 at 09:47:21PM +0100, Dave Mitchell wrote:
> On Wed, Oct 22, 2003 at 10:15:52PM +0200, Elizabeth Mattijsen wrote:
> > Well, maybe someone can figure out what is going on here and maybe 
> > we'll get a fix for 5.8.2?

I meant to reply to this "not likely unless someone does it real soon"


> Whether this will be integrated in 5.8.2 is up to Nick.

I confess I don't understand the sharing system well enough to be a good
judge of this.

> The gory details:
> 
> While in shared interpeter context, the call to sv_setsv() that updates
> the (shared interpreter) RV at $a[0] to refer to the new (shared
> interpreter) [] also causes the old thing the RV was pointing to to be
> freed.  But a cunning bit of code in sv_unref_flags() doesn't free the old
> referent immediately; instead it is converted into a mortal, and so freed
> later. This is apparently to handle stuff like $a = $a->[0]. However, this
> means that the mortal was added to the tmps stack of the *shared
> interpreter*, which never gets freed. Adding a SAVETMPS/FREETMPS within
> the shared context block of code solved the problem. 

Is it safe to add to maint?
[I know that it's desirable to stem the leak, but is it unlikely to add
new bugs?]

> +	if (SvTYPE(ssv) == SVt_PVAV)	/* #24061 */
> +	    AvREAL_on(ssv);

Is AvREAL documented anywhere?

>  	CALLER_CONTEXT;
>      }
>  
> @@ -436,6 +438,12 @@
>  	if (target) {
>  	    SV *tmp;
>  	    SHARED_CONTEXT;
> +	    /* #24255: sv_setsv() (via sv_unref_flags()) may cause a
> +	     * deferred free with sv_2mortal(). Ensure that the free_tmps
> +	     * is done within this inpterpreter. DAPM.
> +	     */
> +	    ENTER;
> +	    SAVETMPS;
>  	    tmp = newRV(SHAREDSvPTR(target));
>  	    sv_setsv_nomg(SHAREDSvPTR(shared), tmp);
>  	    SvREFCNT_dec(tmp);
> @@ -444,6 +452,8 @@
>  	      SvOBJECT_on(SHAREDSvPTR(target));
>  	      SvSTASH(SHAREDSvPTR(target)) = (HV*)fake_stash;
>  	    }
> +	    FREETMPS;
> +	    LEAVE;
>  	    CALLER_CONTEXT;
>  	}
>  	else {

This bit seems safe to me. (But who am I to judge?)

Nicholas Clark



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