Bug #78619 for Devel-GlobalDestruction: Document (or maybe fix by means yet unknown) END interaction with perl -c

Fri Jul 27 17:28:18 2012 ribasushi [...] leporine.io - Ticket created

Subject:

Document (or maybe fix by means yet unknown) END interaction with perl -c

Placeholder for the problem identified by RT#78617

Fri Jul 27 17:28:38 2012 ribasushi [...] leporine.io - Reference to ticket #78617 added

Sat Aug 11 20:38:48 2012 ribasushi [...] leporine.io - Correspondence added

Apparently there is an elegant way to fix it ;)

Sat Aug 11 20:38:49 2012 ribasushi [...] leporine.io - Status changed from 'new' to 'patched'

Fri Feb 01 16:43:33 2013 haarg [...] haarg.org - Correspondence added

I've come up with an alternate fix for this, which also simplifies the code greatly. PL_main_start is nulled immediately before setting PL_dirty, and B gives access to it via B::main_start(). So you can check for global destruction using B::main_start()->isa('B::NULL'), eliminating the need for any END games. I've pushed a branch main_start to p5sagit that replaces the pure-perl fallback with this, and it passes tests on the perl's I have available, including 5.8.1 (with and without threads).

Fri Feb 01 16:43:35 2013 The RT System itself - Status changed from 'patched' to 'open'

Fri Feb 01 17:58:52 2013 ribasushi [...] leporine.io - Correspondence added

Subject:	Re: [rt.cpan.org #78619] Document (or maybe fix by means yet unknown) END interaction with perl -c
Date:	Sat, 2 Feb 2013 09:58:37 +1100
To:	Graham Knop via RT <bug-Devel-GlobalDestruction [...] rt.cpan.org>
From:	Peter Rabbitson <ribasushi [...] cpan.org>

On Fri, Feb 01, 2013 at 04:43:35PM -0500, Graham Knop via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=78619 > > > I've come up with an alternate fix for this, which also simplifies the > code greatly. > > PL_main_start is nulled immediately before setting PL_dirty, and B gives > access to it via B::main_start(). So you can check for global > destruction using B::main_start()->isa('B::NULL'), eliminating the need > for any END games.

This is fucking awesome. I am +1 on getting this shipped, if someone doesn't beat me to it will do so after FOSDEM. One question though - is there *any* state between PL_main_start and PL_dirty where a DESTROY could be called? I am not versed in the p5 core at all, hence making sure there isn't something we are overlooking. Cheers

Fri Feb 01 18:32:18 2013 haarg [...] haarg.org - Correspondence added

On Fri Feb 01 17:58:52 2013, RIBASUSHI wrote: Show quoted text

> On Fri, Feb 01, 2013 at 04:43:35PM -0500, Graham Knop via RT wrote:

> > <URL: https://rt.cpan.org/Ticket/Display.html?id=78619 > > > > > I've come up with an alternate fix for this, which also simplifies the > > code greatly. > > > > PL_main_start is nulled immediately before setting PL_dirty, and B gives > > access to it via B::main_start(). So you can check for global > > destruction using B::main_start()->isa('B::NULL'), eliminating the need > > for any END games.

> > > This is fucking awesome. I am +1 on getting this shipped, if someone > doesn't beat me to it will do so after FOSDEM. One question though - is > there *any* state between PL_main_start and PL_dirty where a DESTROY > could be called? I am not versed in the p5 core at all, hence making > sure there isn't something we are overlooking. > > Cheers

Pasting from perl.c (5.8.1): PL_main_start = Nullop; SvREFCNT_dec(PL_main_cv); PL_main_cv = Nullcv; PL_dirty = TRUE; Other versions look roughly the same. The SvREFCNT_dec call is the only part that could possibly cause any complications, and I don't know the core well enough to say for certain. However, this code is shortly after the code that runs END blocks, so it couldn't be any worse than that. A larger concern would be if main_start would be null in other cases and show a false positive. I hadn't checked that thoroughly before, but I'm looking at it now.

Fri Feb 01 19:14:15 2013 haarg [...] haarg.org - Correspondence added

Turns out that the main_start is null until compilation finishes, so there is some extra work needed. I've updated the branch to detect starting the runloop using a CHECK, and only reporting global destruction after that has happened. Also added some tests to make sure global destruction isn't detected in BEGIN.

Mon Feb 04 16:28:07 2013 ribasushi [...] leporine.io - Correspondence added

Subject:	Re: [rt.cpan.org #78619] Document (or maybe fix by means yet unknown) END interaction with perl -c
Date:	Tue, 5 Feb 2013 08:27:53 +1100
To:	Graham Knop via RT <bug-Devel-GlobalDestruction [...] rt.cpan.org>
From:	Peter Rabbitson <ribasushi [...] cpan.org>

On Fri, Feb 01, 2013 at 07:14:16PM -0500, Graham Knop via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=78619 > > > Turns out that the main_start is null until compilation finishes, so > there is some extra work needed. > > I've updated the branch to detect starting the runloop using a CHECK, > and only reporting global destruction after that has happened. Also > added some tests to make sure global destruction isn't detected in BEGIN.

I now wonder if this doesn't extend further. E.g. what about this state in a CLONE block when a fresh thread has just spawned? Or what about the transition between run and compile time (any do/eval)? I *think* there is even more stuff we are missing... Perhas asking p5p is a logical next step? After all we do not have a moving target anymore - anything after 5.14 is irrelevant.

Tue Feb 05 16:15:46 2013 haarg [...] haarg.org - Correspondence added

On Mon Feb 04 16:28:07 2013, RIBASUSHI wrote: Show quoted text

> On Fri, Feb 01, 2013 at 07:14:16PM -0500, Graham Knop via RT wrote: > > I now wonder if this doesn't extend further. E.g. what about this state > in a CLONE block when a fresh thread has just spawned? Or what about the > transition between run and compile time (any do/eval)? I *think* there > is even more stuff we are missing... > > Perhas asking p5p is a logical next step? After all we do not have a > moving target anymore - anything after 5.14 is irrelevant.

I've played around a bit with it and made sure that the only time B::main_start()->isa('B::NULL') will be true is during the initial compile phase and global destruction, not any other state. And CHECK happens after the initial compile, making it a safe time to start checking main_start. Threading is a somewhat different matter. My changes pass the tests (with threading), but they focus mainly on end/destruction time, which is why my initial try missed the CHECK portion. I'll work on extending those tests as needed. Running perl embedded is another thing I haven't tested. So yes, probably worth checking with p5p in case there's something missing. As an aside, I released the module Devel::GlobalPhase that emulates the rest of ${^GLOBAL_PHASE}.

Wed Feb 06 17:42:51 2013 haarg [...] haarg.org - Correspondence added

I've checked that my code works under threads during CLONE time, and added a test to verify that.

Tue Mar 26 23:59:00 2013 haarg [...] haarg.org - Correspondence added

0.10 fixes this issue

Wed Mar 27 02:40:22 2013 ribasushi [...] leporine.io - Fixed in 0.10 added

Wed Mar 27 02:40:22 2013 ribasushi [...] leporine.io - Status changed from 'open' to 'resolved'