Test::Builder's new guts don't play nice with Test::Most

[barefootcoder]

Now, I certainly don't expect Test2 and Test::Most to get along, as Test2 can pretty much just replace Test::Most altogether. But, in this case, no one asked Test2 to step in; it comes in because Test::Builder is now apparently made of Test2. The problem is trivial to replicate:

[caemlyn:~] perl -MTest::Most -le 'bail_on_fail; fail(); done_testing();'
not ok 1
#   Failed test at -e line 1.
1..1
# Looks like you failed 1 test of 1.
Use of uninitialized value $hid in hash element at /home/buddy/perl5/perlbrew/perls/perl-5.14.4/lib/site_perl/5.14.4/Test2/API.pm line 284 during global destruction.
Use of uninitialized value $hid in hash element at /home/buddy/perl5/perlbrew/perls/perl-5.14.4/lib/site_perl/5.14.4/Test2/API.pm line 373 during global destruction.
Use of uninitialized value $hid in hash element at /home/buddy/perl5/perlbrew/perls/perl-5.14.4/lib/site_perl/5.14.4/Test2/API.pm line 374 during global destruction.
Use of uninitialized value $hid in hash element at /home/buddy/perl5/perlbrew/perls/perl-5.14.4/lib/site_perl/5.14.4/Test2/API/Context.pm line 128 during global destruction.
Use of uninitialized value $hid in hash element at /home/buddy/perl5/perlbrew/perls/perl-5.14.4/lib/site_perl/5.14.4/Test2/API/Context.pm line 128 during global destruction.
Use of uninitialized value $hid in delete at /home/buddy/perl5/perlbrew/perls/perl-5.14.4/lib/site_perl/5.14.4/Test2/API/Context.pm line 133 during global destruction.
Use of uninitialized value $hid in hash element at /home/buddy/perl5/perlbrew/perls/perl-5.14.4/lib/site_perl/5.14.4/Test2/API.pm line 284 during global destruction.
Use of uninitialized value $hid in hash element at /home/buddy/perl5/perlbrew/perls/perl-5.14.4/lib/site_perl/5.14.4/Test2/API.pm line 373 during global destruction.
Use of uninitialized value $hid in hash element at /home/buddy/perl5/perlbrew/perls/perl-5.14.4/lib/site_perl/5.14.4/Test2/API.pm line 374 during global destruction.

Now, they're just warnings, so I could ignore them (which is what I have been doing), but I wonder if there's a better way. If this is not patchable on your side, is there something I could submit to Ovid to solve the problem from the other direction?

[barefootcoder]

It occurred to me that you might need to know what version(s) I'm using. I believe everything is moderately recent, as I just had to blow away my perlbrewed version of 5.14.4 and rebuild it from scratch.

[caemlyn:~] perl -MTest::Builder -le 'print $Test::Builder::VERSION'
1.302112
[caemlyn:~] perl -MTest::Most -le 'print $Test::Most::VERSION'
0.35

[exodist]

Sorry for the inconvenience. Test::Most compatibility was something I worked hard to insure, and my test cases all still passed, sprry this use case got overlooked. This is an issue to fix on my end as soon as I can.

[barefootcoder]

No worries. I thought I'd remembered you saying that it ought to work with Test::Most, so I thought I'd start here. :-)

Hopefully it's not too much of a pain to find and fix ...

[barefootcoder]

Any word on this issue?

[exodist]

hmm, this is a garbage collection race condition issue. Something in the global destruction/cleanup is trying to call Test::Builder->new->something. This happens after both the Test::Builder singleton and the Test2::API::Instance have been purged from memory. This then (in cleanup) tries to re-create the Test::Builder singleton, which then tries to re-init the Test2::API instance. Problem is there is no sane way to re-init the Test2::API instance once it is destroyed.

[exodist]

Figured out the trigger. Test::Most injects a DESTROY sub into Test::Builder. This is a bad thing to do, first off it would mask any DESTROY block Test::Builder might add in the future. Second Test::Builder has no DESTROY block normally, and internal logic was written with that in mind.

This causes GC issues because the object being DESTROYED is normally a singleton in a variable, but the GC has made that variable undef. This also occurs after Test2 GC has occurred. Big chain reaction.

[exodist]

to clarify, the new DESTROY block calls an action handler, the action handler in this case calls Test::More::BAIL_OUT, which then tries to re-create the singleton instead of using the object being DESTROYED.

I think the fix needs to be made in Test::Most, but is really simple. The DESTROY it injects should localize $Test::Builder::Test to the object being destroyed:

local $Test::Builder::Test = $_[0]; a simple 1-line fix that will probably solve this. But it needs to be fixed in Test::Most. This is actually a Test::Most bug, the new internals of Test::Builder simply exposed it. It was a bug to rely on Test::Builder creating a new singleton after the first is DESTROY'd

[exodist]

Closing in favor of https://rt.cpan.org/Ticket/Display.html?id=127909

[barefootcoder]

Test::Most injects a DESTROY sub into Test::Builder. This is a bad thing to do, ...

Oh, no doubt! :-D I'm not sure if Ovid had any other choices, though, given the old infrastructure.

Closing in favor of https://rt.cpan.org/Ticket/Display.html?id=127909

Fair enough. I'll jump in over there.

[Ovid]

Buddy, Per your comment on RT, I've added you as co-maintainer on Test::Most. Thank you for the offer! Best, Ovid

…

On Sat, Dec 8, 2018 at 12:48 AM Buddy Burden ***@***.***> wrote: Test::Most injects a DESTROY sub into Test::Builder. This is a bad thing to do, ... Oh, no doubt! :-D I'm not sure if Ovid had any other choices, though, given the old infrastructure. Closing in favor of https://rt.cpan.org/Ticket/Display.html?id=127909 Fair enough. I'll jump in over there. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#816 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABgOpTSb3eqkE20Hmf6oumc6_ahfi6bks5u2v5MgaJpZM4WKi9i> .

-- Curtis "Ovid" Poe -- CTO, All Around the World World-class software development and consulting https://allaroundtheworld.fr/