Re: [perl #59268] PerlIO::via leaks memory at a high rate

On 2008-10-26, at 17:41:26 +0000, Nicholas Clark wrote:

> On Wed, Sep 24, 2008 at 01:40:20AM -0700, Marc Lehmann wrote:
> 
> Sorry about the delay in anyone replying. I don't think that anyone (alive)
> is familiar with this code.
> 
> Having been forced to have a look at it (5.8.9 show stopper) I can't say that
> it fills me with joy.
> 
> > If I had to make a very quick guess, could it be that the
> > pseudo-filehandle creates via newGVgen creates a new _GEN_xxx symbol in
> > the ...::bla package that never ever gets freed?
> > 
> > Indeed, adding this line into the while loop:
> > 
> >    warn keys %PerlIO::via::bla::;
> > 
> > creates successively longer output lines. Could that mean that
> > newGVgen leaks?
> 
> I am suspicious of this part of the code of via.xs
> 
> 	    if (!s->fh) {
> 		GV *gv = newGVgen(HvNAME_get(s->stash));
> 		GvIOp(gv) = newIO();
> 		s->fh = newRV((SV *) gv);
> 		s->io = GvIOp(gv);
> 	    }

The trick is to get rid of the GV reference created in the
stash immediately, as it will otherwise survive the death
of the layer. This is exactly what IO::File does in its
implementation of new_tmpfile().

> I suspect that yes, it practical terms it is leaking at the Perl level, in
> that all the memory is still reachable, and will be correctly accounted for
> at interpreter exit, but the code never thinks to clean up its temporaries
> immediately after it has finished with them.
> 
> I'm afraid that I'm not going to have time to make a fix that I'm comfortable
> with in time for 5.8.9. However, I don't see any particular reason why a
> fixed version can't be put on CPAN subsequently, which would allow you (or
> anyone else) to have a dependency on it, and hence "fix" existing perl
> installations.

Agreed. I've made the change below in blead so we can see
if it causes any problems. If it turns out to be fine, a
PerlIO::via release should fix it for older perls.

What remains is the question whether we can avoid adding
the GV to the stash in the first place. Maybe by passing
yet another flag to Perl_gv_fetchpvn_flags()?

Marcus

PS: Yes, the "if (gv)" is dumb. It'll never get there if
    gv == NULL. Just noticed after committing...


Change 34779 by mhx@mhx-r2d2 on 2008/11/08 18:31:09

	Make sure PerlIO::via doesn't leave its old GV's hanging
	in the stash they were created in. Steal the idea from
	IO::File to just delete the stash entry after creating
	the GV.
	This fixes perl #59268.

Affected files ...

... //depot/perl/ext/PerlIO/via/via.pm#11 edit
... //depot/perl/ext/PerlIO/via/via.xs#18 edit

Differences ...

==== //depot/perl/ext/PerlIO/via/via.pm#11 (text) ====

@@ -1,5 +1,5 @@
 package PerlIO::via;
-our $VERSION = '0.06';
+our $VERSION = '0.07';
 use XSLoader ();
 XSLoader::load 'PerlIO::via';
 1;

==== //depot/perl/ext/PerlIO/via/via.xs#18 (text) ====

@@ -91,6 +91,10 @@
 		GvIOp(gv) = newIO();
 		s->fh = newRV((SV *) gv);
 		s->io = GvIOp(gv);
+		if (gv) {
+		    /* shamelessly stolen from IO::File's new_tmpfile() */
+		    hv_delete(GvSTASH(gv), GvNAME(gv), GvNAMELEN(gv), G_DISCARD);
+		}
 	    }
 	    IoIFP(s->io) = PerlIONext(f);
 	    IoOFP(s->io) = PerlIONext(f);

• [perl #59268] PerlIO::via leaks memory at a high rate by Marc Lehmann
• Re: [perl #59268] PerlIO::via leaks memory at a high rate by Nicholas Clark
• Re: [perl #59268] PerlIO::via leaks memory at a high rate by Marcus Holland-Moritz