Don't dump autodie from core (was Re: Coring Variable::Magic / autodie fights with string eval in Perl 5.10.x)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

G'day Dave,

Dave Mitchell wrote:

> Okay I think you've convinced me. So basically you'll release a 2.06 which
> is 2.05 + bug documentation; 2.06 goes in 5.10.1; then the bug fix goes in
> 2.07?

Patch attached, including documentation fixes, and TODO tests.

I did slip a small code change in, which fixes a regression that was
introduced (but not spotted) around autodie 1.997ish.  Many thanks to rafl
and particularly to Vincent for their help in debugging and documenting.

All the best,

	Paul

- --
Paul Fenwick <pjf@perltraining.com.au> | http://perltraining.com.au/
Director of Training                   | Ph:  +61 3 9354 6001
Perl Training Australia                | Fax: +61 3 9354 2681
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (MingW32)

iD8DBQFKUgdnx5N6j7FHnlURAhcbAJ4zQ5WujSXK5uNpEfdOp0RMi8FxVQCghU02
Z3ZBYlXddRMOmm9SVC7UPTE=
=qf29
-----END PGP SIGNATURE-----

>From 689bc09292ba5173542d208dca85b6f514c91aba Mon Sep 17 00:00:00 2001
From: Paul Fenwick <pjf@perltraining.com.au>
Date: Tue, 7 Jul 2009 00:11:42 +1000
Subject: [PATCH] Merge autodie 2.06 with core.

NOTE: This commit is different from that on the CPAN by three lines
of POD.  A documented bug was fixed, but documentation about this bug
was accidentally not removed from the CPAN release.

Significant changes in this release are:

        * BUG: Explicitly documented that autodie does NOT play
          nicely with string evals, especially under Perl 5.10.x.
          Please avoid using string evals while autodie is in scope.

        * TEST: Check for autodie leaking out of scope in the
          presence of string evals.  (string-eval-leak.t)
          Thanks to Florian Ragwitz and Vincent Pit for identifying
          this.

        * BUGFIX: autodie once again correctly works when used
          inside a string eval.  (This was accidently broken
          somewhere around 1.997-1.998).

7 files changed, 101 insertions, 11 deletions

diff --git a/lib/Fatal.pm b/lib/Fatal.pm
index 50ad335a60..e756957e3b 100644
--- a/lib/Fatal.pm
+++ b/lib/Fatal.pm
@@ -39,7 +39,7 @@ use constant ERROR_58_HINTS => q{Non-subroutine %s hints for %s are not supporte
 use constant MIN_IPC_SYS_SIMPLE_VER => 0.12;
 
 # All the Fatal/autodie modules share the same version number.
-our $VERSION = '2.05';
+our $VERSION = '2.06';
 
 our $Debug ||= 0;
 
@@ -100,6 +100,7 @@ my %TAGS = (
     ':2.03'  => [qw(:default)],
     ':2.04'  => [qw(:default)],
     ':2.05'  => [qw(:default)],
+    ':2.06'  => [qw(:default)],
 );
 
 $TAGS{':all'}  = [ keys %TAGS ];
@@ -1086,10 +1087,19 @@ sub _make_fatal {
 
                 my \$caller_level = 0;
 
-                while ( (caller \$caller_level)[1] =~ m{^\\(eval \\d+\\)\$} ) {
+                my \$caller;
+
+                while ( (\$caller = (caller \$caller_level)[1]) =~ m{^\\(eval \\d+\\)\$} ) {
+
+                    # If our filename is actually an eval, and we
+                    # reach it, then go to our autodying code immediatately.
+
+                    goto &\$code if (\$caller eq \$filename);
                     \$caller_level++;
                 }
 
+                # We're now out of the eval stack.
+
                 # If we're called from the correct file, then use the
                 # autodying code.
                 goto &\$code if ((caller \$caller_level)[1] eq \$filename);
diff --git a/lib/autodie.pm b/lib/autodie.pm
index 5cfb643411..efb93d32bc 100644
--- a/lib/autodie.pm
+++ b/lib/autodie.pm
@@ -8,7 +8,7 @@ our @ISA = qw(Fatal);
 our $VERSION;
 
 BEGIN {
-    $VERSION = '2.05';
+    $VERSION = '2.06';
 }
 
 use constant ERROR_WRONG_FATAL => q{
@@ -351,11 +351,6 @@ See also L<Fatal/DIAGNOSTICS>.
 is used with package filehandles (eg, C<FILE>).  Scalar filehandles are
 strongly recommended instead.
 
-Under Perl 5.8 only, C<autodie> I<does not> propagate into string C<eval>
-statements, although it can be explicitly enabled inside a string
-C<eval>.  This bug does not affect block C<eval> statements in
-any version of Perl.
-
 When using C<autodie> or C<Fatal> with user subroutines, the
 declaration of those subroutines must appear before the first use of
 C<Fatal> or C<autodie>, or have been exported from a module.
@@ -365,6 +360,28 @@ result in a compile-time error.
 Due to a bug in Perl, C<autodie> may "lose" any format which has the
 same name as an autodying built-in or function.
 
+C<autodie> may not work correctly if used inside a file with a
+name that looks like a string eval, such as F<eval (3)>.
+
+=head2 autodie and string eval
+
+Due to the current implementation of C<autodie>, unexpected results
+may be seen when used near or with the string version of eval.
+I<None of these bugs exist when using block eval>.
+
+Under Perl 5.8 only, C<autodie> I<does not> propagate into string C<eval>
+statements, although it can be explicitly enabled inside a string
+C<eval>.
+
+Under Perl 5.10 only, using a string eval when C<autodie> is in
+effect can cause the autodie behaviour to leak into the surrounding
+scope.  This can be worked around by using a C<no autodie> at the
+end of the scope to explicitly remove autodie's effects, or by
+avoiding the use of string eval.
+
+I<None of these bugs exist when using block eval>.  The use of
+C<autodie> with block eval is considered good practice.
+
 =head2 REPORTING BUGS
 
 Please report bugs via the CPAN Request Tracker at
diff --git a/lib/autodie/exception.pm b/lib/autodie/exception.pm
index b04c82ad03..ac578a1722 100644
--- a/lib/autodie/exception.pm
+++ b/lib/autodie/exception.pm
@@ -14,7 +14,7 @@ use overload
 
 use if ($] >= 5.010), overload => '~~'  => "matches";
 
-our $VERSION = '2.05';
+our $VERSION = '2.06';
 
 my $PACKAGE = __PACKAGE__;  # Useful to have a scalar for hash keys.
 
diff --git a/lib/autodie/exception/system.pm b/lib/autodie/exception/system.pm
index d5b5bad0a3..ebfa6bacca 100644
--- a/lib/autodie/exception/system.pm
+++ b/lib/autodie/exception/system.pm
@@ -5,7 +5,7 @@ use warnings;
 use base 'autodie::exception';
 use Carp qw(croak);
 
-our $VERSION = '2.05';
+our $VERSION = '2.06';
 
 my $PACKAGE = __PACKAGE__;
 
diff --git a/lib/autodie/hints.pm b/lib/autodie/hints.pm
index 80952eff99..f5ba5d832b 100644
--- a/lib/autodie/hints.pm
+++ b/lib/autodie/hints.pm
@@ -5,7 +5,7 @@ use warnings;
 
 use constant PERL58 => ( $] < 5.009 );
 
-our $VERSION = '2.05';
+our $VERSION = '2.06';
 
 =head1 NAME
 
diff --git a/lib/autodie/t/string-eval-basic.t b/lib/autodie/t/string-eval-basic.t
new file mode 100755
index 0000000000..62e55006ea
--- /dev/null
+++ b/lib/autodie/t/string-eval-basic.t
@@ -0,0 +1,24 @@
+#!/usr/bin/perl -w
+use strict;
+use warnings;
+use Test::More tests => 3;
+
+use constant NO_SUCH_FILE => 'this_file_had_better_not_exist';
+
+# Keep this test alone in its file as it can be hidden by using autodie outside
+# the eval.
+
+# Just to make sure we're absolutely not encountering any weird $@ clobbering
+# events, we'll capture a result from our string eval.
+
+my $result = eval q{
+    use autodie "open";
+
+    open(my $fh, '<', NO_SUCH_FILE);
+
+    1;
+};
+
+ok( ! $result, "Eval should fail with autodie/no such file");
+ok($@, "enabling autodie in string eval should throw an exception");
+isa_ok($@, 'autodie::exception');
diff --git a/lib/autodie/t/string-eval-leak.t b/lib/autodie/t/string-eval-leak.t
new file mode 100755
index 0000000000..329bcfa40e
--- /dev/null
+++ b/lib/autodie/t/string-eval-leak.t
@@ -0,0 +1,39 @@
+#!/usr/bin/perl -w
+use strict;
+use warnings;
+use Test::More tests => 2;
+
+# Under Perl 5.10.x, a string eval can cause a copy to be taken of
+# %^H, which delays stringification of our scope guard objects,
+# which in turn causes autodie to leak.  These tests check to see
+# if we've successfully worked around this issue.
+
+eval {
+
+    {
+        use autodie;
+        eval "1";
+    }
+
+    open(my $fh, '<', 'this_file_had_better_not_exist');
+};
+
+TODO: {
+    local $TODO;
+
+    if ( $] >= 5.010 ) {
+        $TODO = "Autodie can leak near string evals in 5.10.x";
+    }
+
+    is("$@","","Autodie should not leak out of scope");
+}
+
+# However, we can plug the leak with 'no autodie'.
+
+no autodie;
+
+eval {
+    open(my $fh, '<', 'this_file_had_better_not_exist');
+};
+
+is("$@","",'no autodie should be able to workaround this bug');