Front page | perl.perl5.porters |
Postings from March 2017
Re: [perl #32714] Objects destroyed in the wrong order during globaldestruction
Thread Previous
|
Thread Next
From:
Ævar Arnfjörð Bjarmason
Date:
March 11, 2017 12:33
Subject:
Re: [perl #32714] Objects destroyed in the wrong order during globaldestruction
Message ID:
CACBZZX6O4UYY51LLGkfR0nW416C+cCP49r53QcXwQOHfY0h+aw@mail.gmail.com
On Sat, Mar 11, 2017 at 2:41 AM, Father Chrysostomos via RT
<perlbug-followup@perl.org> wrote:
> On Fri, 10 Mar 2017 07:38:02 -0800, wellnhofer@aevum.de wrote:
>> On Fri, 10 Mar 2017 02:16:13 -0800, LeonT wrote:
>> > Destructors can and do depend on package variables, this would not
>> > solve
>> > all non-circular cases.
>>
>> Right, but it would solve a large class of cases. Also note that Perl
>> seems to do the right thing for "my" variables with package scope.
>> Only for "our" variables, it seems impossible to get a sane
>> destruction order. If you run the program below, $my1 if always
>> destroyed before $my2, regardless of initialization order. But "our"
>> variables are always destroyed in the DESTRUCT phase without taking
>> references between objects into account.
>>
>> > The proper solution would be to topologically sort the objects and
>> > then
>> > destroy them in that order (ideally even taking into account that the
>> > destruction can add new nodes to the graph), but this is non-trivial
>> > to
>> > implement and to execute; and obviously would still not solve
>> > circular
>> > references.
>>
>> Just because a solution isn't perfect doesn't mean that it shouldn't
>> be implemented. Anyway, I'm only asking to acknowledge that this bug
>> isn't fixed and to reopen it.
>
> Seems fair enough. I am doing exactly that.
>
> But let me note that simply destroying ‘our’ variables before the general reference sweep is not so straightforward. If you start by deleting everything stored by globs, then you end up deleting the destructors themselves. Again, deleting everything from globs except subs would only work for some code, and not other code. What Leon Timmermans said about a topological order would probably be the only solution.
On a related note. At Booking.com in our web applications we don't run
destruction beyond just destroying the Plack $env, then we
POSIX::_exit(). See https://github.com/unbit/uwsgi/pull/1392/files &
https://github.com/unbit/uwsgi/issues/1384 for why, but tl;dr: With
forked processes it creates a CPU/Memory DoS attack on the kernel as
it frantically tries to unshare all the pages whose refcounts change.
So uWSGI has an option now to simply skip the perl_destruct() &
perl_free() phases, which from my looking at the source & testing it
seemed like the sane solution:
https://github.com/avar/uwsgi/blob/bafc14c80771a2c55063c3c9cc9c9dd0377a0294/plugins/psgi/psgi_plugin.c#L860
I can't think of a use-case for this, but is it a good idea to provide
some alternative to POSIX::_exit() in this case, i.e. is there some
subset of perl_{destruct,free}() that we could run that wouldn't free
everything, but e.g. just enough to run any DESTROY handlers?
Currently if we have some DESTROY handler hanging off anything but
$env it simply won't execute when we teardown the process.
I don't know if that's possible or even a mad idea, but since you're
poking this code I thought I'd bring this use-case to your attention.
>> #!/usr/bin/perl
>> use strict;
>> use warnings;
>>
>> package TestObj;
>>
>> sub new {
>> my ( $class, $name ) = @_;
>> return bless { name => $name }, $class;
>> }
>>
>> sub set_ref {
>> my ( $self, $ref ) = @_;
>> $self->{ref} = $ref;
>> }
>>
>> sub DESTROY {
>> my $self = shift;
>> print "Destroying $self->{name}, phase is ${^GLOBAL_PHASE}\n";
>> }
>>
>> my $my1 = TestObj->new('my1');
>> my $my2 = TestObj->new('my2');
>> $my1->set_ref($my2);
>>
>> our $our1 = TestObj->new('our1');
>> our $our2 = TestObj->new('our2');
>> $our1->set_ref($our2);
>
>
> --
>
> Father Chrysostomos
>
>
> ---
> via perlbug: queue: perl5 status: resolved
> https://rt.perl.org/Ticket/Display.html?id=32714
Thread Previous
|
Thread Next