context in END can segfault perl

[haarg]

Using context() in an END block can segfault perl. The root cause is perl rt#127774. Here is an example of this being triggered by Test2:

use strict;
use warnings;
use Test2::API qw/context/;

eval(' sub { die } ')->();
END { my $ctx = context() }

Given how pervasive context calls are with Test2, this can be triggered in a lot of other ways. For example, Test::Builder->new->is_passing (with the Test::Builder dev version).

[exodist]

Thanks for the report.

First off, the way you are using context() there is incorrect, context() is always supposed to be called inside a tool sub (such as a tool sub) directly calling it at the package level (or in a BEGIN/END/etc. block) is not supported, so a better test is this:

eval(' sub { die } ')->();
END { sub { my $ctx = context(); $ctx->release }->() }

Which still demonstrates the problem, but limits it to a supported set of scenarios. This is important because caller() is used 3 times in context(), but only the third one triggers the bug in this proper use case. In the incorrect test case all 3 trigger it.

Now that I have limited it, I am working on a workaround.

[exodist]

Please see the commit above, I am not releasing it yet. I would like feedback on it. In my testing it appears to work around the issue, and does not break older perls (including ones from before ${^GLOBAL_PHASE} was added.

The test shows that the segv from caller is fixed, however there are occasionally (random?) segv's in some 5.12 and 5.16 versions, very rare, but I have made the test an AUTHOR_TESTING test just to keep it from being an issue on cpantesters.

[exodist]

oops, added a commit after that one that removes some crud from the test.

[exodist]

not ready to close this just yet..

[haarg]

1. The test still reliably segfaults on 5.8.9.
2. Having the AUTHOR_TESTING guard seems unnecessary for a test in xt/
3. Having the test output a skip all note is confusing. And 'foo' is obviously not a proper message for it.

[exodist]

27a83f7

Fixes 5.8.9, and makes both the logic and the test a lot more sane.

[haarg]

If you change require Test2::API; to use Test2::API (); it will still segfault.

[exodist]

to be specific, it looks like the change makes it segv on 5.8.9, and a couple 5.12.* versions (with some in the middle working fine)

Just adding this as a note.

[exodist]

Correction, only 2 of them were segv's (5.12.3, and 5.8.9) The others appear to be $? getting the wrong value (but not a segv)

[exodist]

Is there any way to detect that conditions are set for the segv to occur, without actually triggering the segv? I cannot simply stop using caller in context(), so my efforts are in avoiding caller() when it looks unsafe. Unfortunately unless there is a safe-consistent way to detect the condition my efforts will always be "best efforts" and not "fixes all ways to trigger it".

[exodist]

Adding notes from IRC conversation:

the depth checking is valuable protection. However it is not safe to use in END/Global Destruction.

The depth check will be enabled by default on perls 5.14+ which have the ${^GLOBAL_PHASE} variable. The variable will be used to turn off depth checking in END/DESTRUCTION.

The depth check will be disabled by default on older perls.

Depth checking can be forcefully enabled if desired.

Also leaving in the END { $ENDING = 1 } bits for extra safety (only matters for forcefully enabled older perls)

[exodist]

I believe this is fixed now. Will leave it open until a stable release has the fix.

[ribasushi]

Test::More does not declare a correct minimum Test2 version, leading to a valid combination of T::M 1.302013_014 and Test2 0.000032, which in turn breaks the idiom END { $ok && done_testing }

Have not investigated which is the correct version to ask for.

[exodist]

The new version of Test2 was released yesterday, and is 0.000036. I have a new Test-Simple dev release ready to upload which depends on the new Test2. The upload will happen as soon as my downstream testing is done running (which will be another hour or so). I don't upload Test-Simple until I have run it through a gauntlet of verification.

I technically already ran all these tests before releasing Test2 using the current cpan dev version of Test-Simple, but even though my change was only a version bump I feel compelled to run all the tests again before releasing Test-Simple.

[exodist]

This has been fixed for a couple days now, closing.