Test-More / Test2-Harness

Alternative to Test::Harness
Other
23 stars 26 forks source link

fix infinite hang when preload fails #240

Closed aeruder closed 1 year ago

aeruder commented 2 years ago

This is a subtle one - but we do our preload here in the scripts/yath BEGIN {} block with a localized $?.

When exit() is called, END { } blocks, etc. can change $?. Also, any surrounding local scopes are also unwound. Because of this localized $? on the outer block, essentially every exit() triggered within this BEGIN {} was always going to exit 0. This is particularly problematic because our preloads run in this BEGIN {} context, so all the stage exit codes are getting swallowed up. This leads to some poor behavior where stage runners exit "successfully" when preloading fails and the entire test runner just deadlocks.

The perl behavior can be easily seen with this example program:

test.pl

#!/usr/bin/env perl

BEGIN {
  local $?;
  print STDERR "bad exit\n";
  exit(1);
}

and then running:

% perl -w test.pl && echo "exited successfully"
bad exit
exited successfully
%

This can be worked around by using POSIX::_exit everywhere, but we really don't control what is running in the BEGIN {} block here (particularly with preloading where anything can exit()).

I, however, don't think the local block here in our top-level is really necessary (surely its ok if sourcing this script adjusts $?/$@,$.) so have gone with just removing all of it, but the only part that is important is really not doing the $? part. In general, $? needs to be scoped at as small scopes as possible where you fully control all the code, and even then you have to ensure you're always exiting with POSIX::_exit(). Even die can get its error code swallowed:

#!/usr/bin/env perl

sub func {
  local $?;
  die "dying";
}
func();

and running:

% perl test.pl ; echo $?
dying at /home/aeruder/test.pl line 5.
0

Fun!

exodist commented 1 year ago

Merged and altered slightly, thanks for the test and the pointer!