Mailing List Archive: feature-class compiletime memory leak and SAVEDESTRUCTOR

This is one of those "I want to do A so I can do B so I can solve bug
C" situations; the story is a little long-winded so I'll give a brief
overview and then dive into detail. Maybe someone can point out a
better approach?

## The Problem

5.37.9 (and likely upcoming 5.37.10) has a memory leak related to the
feature 'class' if a compiletime syntax error occurs at a certain point
in the code. Namely, the point at which it checks for disallowed
control flow in a field initialiser expression:

$ perl5.37.9 -Mexperimental=class
class XXX {
field $yyy = do { say "Hello world"; last };
}

Can't "last" out of field initialiser expression at - line ...

The leak is that the already-compiled OP_SAY and its arguments (the
OP_CONST of "Hello world") don't get freed again. They belong to the
"suspended compilation CV" that was stored in the XXX class, that would
be the container for all of the field initialiser expressions and would
get compiled up into a single initfields CV to be stored in the class.
Because compilation was aborted, that never happened.

Or, actually the situation is more complicated than that. We can't
compile the CV yet out of any single `field` expression, because the
entire point is that we're collecting them all up together in a single
CV at the end. That has to be deferred until the end of the class
declaration. This is a job for the Perl_class_seal_stash() function.

In the current implementation we arrange for that function to be
invoked by calling SAVEDESTRUCTOR_X() with it, so that at the end of
the current block that gets invoked.

https://github.com/Perl/perl5/blob/75ea41ae51200ab26d84c418f08859a784a71b85/class.c#L374

Because of this, it means we ensure the class gets sealed in any of
these interesting cases:

class XX1 { ... }

class XX2; ...; ((EOF))

class XX3; ...; class XX4; ... ((EOF))

{ class XX5; ... } (more code here...)

*However*, the use of SAVEDESTRUCTOR_X is nonideal here because it also
gets invoked on exceptional unwind due to, in this case, syntax error.
The function doesn't have enough information to know that there's no
point compiling the CV, and instead it should abort the entire thing
and throw it all away.

What is needed here, is a way to call Perl_class_seal_stash() only on
successful return from the containing block, and call some other code
to abort the compilation and throw things away on a failure.

I don't know of an existing mechanism to do this.

## Possible Solutions

A few ideas come to mind:

* A new kind of SAVEDESTRUCTOR_... whose function is invoked being
told *why* the stack is being unwound - success or failure. It can
decide on that

* Finish implementing the currently-stalled `PL_throwing` idea, so
that the destructor function can just ask that to find out
https://github.com/Perl/perl5/pull/20407

* Don't use SAVEDESTRUCTOR for this, but instead add a new storage
somewhere (maybe a new AV* in intrpvars) that stores classes which
need sealing. Push to it instead, and flush it out in
Perl_block_end().

This would need some careful safety around keeping a "floor" and
ensuring that multiple nested blocks with classes in all happened at
the right times, but should be doable.

I kinda don't like any of those solutions. They're all kinda icky. Plus
none of them are the kind of thing we can get in in time for 5.38.0.

But it strikes me that surely this can't be the first situation for
needing something like this? Surely all over the place there must be
code that in some way creates a kindof "run this only on success" /
"only on fail" setup. I just don't know of any so far. Can anyone point
me at anything similar?

I suspect this "syntax errors can cause compiletime memory leaks" bug
will have to remain for now. It's a bit annoying but at least it
doesn't affect well-formed programs, only ones that are erroneous
anyway. If nothing else happens, I'll at least put a note in the
perldelta under "known bugs".

--
Paul "LeoNerd" Evans

leonerd@leonerd.org.uk | https://metacpan.org/author/PEVANS
http://www.leonerd.org.uk/ | https://www.tindie.com/stores/leonerd/

On Mon, Mar 20, 2023 at 06:05:15PM +0000, Paul "LeoNerd" Evans wrote:
> This is one of those "I want to do A so I can do B so I can solve bug
> C" situations; the story is a little long-winded so I'll give a brief
> overview and then dive into detail. Maybe someone can point out a
> better approach?

[. disclaimer: I still haven't read up properly on the class syntax nor how
it's implemented, so the following my be totally missing the point, but
here goes...]

The whole 'deferred compilation' thing sets off lots of alarm bells in my
head. It sounds complex and open to confusing edge cases, as this thread
shows.

Why can't a class declaration be treated just as a block- or file-scoped
sub declaration with some funny special behaviours. So for example,

class Foo {
field $x = do { stuff x };
field $y = do { stuff y };
method foo { ...; }
...
}

is treated by the lexer and parser as something akin to

sub Foo {
do { stuff x };
do { stuff y };
sub foo { .... }
...
}

so the do's accumulate in a special CV associated with the class,
while every time a method is seen, a start_subparse() is done, the
method is compiled in its own CV, then PL_compcv reverts to the 'special'
CV and processing the class declaration continues.

On end of block / EOF / new class declaration as appropriate, the
special CV is finished off.

--
Little fly, thy summer's play my thoughtless hand
has terminated with extreme prejudice.
(with apologies to William Blake)