From: Lukas Mai <l.mai@web.de>
Date: Fri, 19 Feb 2016 17:24:53 +0000 (+0100)
Subject: always run finally blocks no matter how we leave the try block (RT #112099)
X-Git-Tag: v0.25~4
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=80352025d169218f726e3a0e3ad1ecd226a37596;p=p5sagit%2FTry-Tiny.git

always run finally blocks no matter how we leave the try block (RT #112099)
---

diff --git a/lib/Try/Tiny.pm b/lib/Try/Tiny.pm
index 7c97b62..90963b0 100644
--- a/lib/Try/Tiny.pm
+++ b/lib/Try/Tiny.pm
@@ -29,6 +29,8 @@ BEGIN {
   *_HAS_SUBNAME = ($su || $sn) ? sub(){1} : sub(){0};
 }
 
+our @_finally_guards;
+
 # Need to prototype as @ not $$ because of the way Perl evaluates the prototype.
 # Keeping it at $$ means you only ever get 1 sub because we need to eval in a list
 # context & not a scalar one
@@ -72,6 +74,13 @@ sub try (&;@) {
   _subname("${caller}::try {...} " => $try)
     if _HAS_SUBNAME;
 
+  # set up scope guards to invoke the finally blocks at the end.
+  # this should really be a lexical variable instead of our/local but that
+  # causes issues with perls < 5.20 due to perl rt#119311
+  local @_finally_guards =
+    map { Try::Tiny::ScopeGuard->_new($_) }
+    @finally;
+
   # save the value of $@ so we can set $@ back to it in the beginning of the eval
   # and restore $@ after the eval finishes
   my $prev_error = $@;
@@ -99,14 +108,12 @@ sub try (&;@) {
   $error = $@;
   $@ = $prev_error;
 
-  # set up a scope guard to invoke the finally block at the end
-  my @guards =
-    map { Try::Tiny::ScopeGuard->_new($_, $failed ? $error : ()) }
-    @finally;
-
   # at this point $failed contains a true value if the eval died, even if some
   # destructor overwrote $@ as the eval was unwinding.
   if ( $failed ) {
+    # pass $error to the finally blocks
+    push @$_, $error for @_finally_guards;
+
     # if we got an error, invoke the catch block.
     if ( $catch ) {
       # This works like given($error), but is backwards compatible and
diff --git a/t/finally.t b/t/finally.t
index 2624cc9..a13c9a8 100644
--- a/t/finally.t
+++ b/t/finally.t
@@ -3,7 +3,7 @@
 use strict;
 use warnings;
 
-use Test::More tests => 27;
+use Test::More tests => 30;
 
 use Try::Tiny;
 
@@ -128,4 +128,16 @@ is($_, "foo", "same afterwards");
   like $originals[1], qr/fin 3 at/, 'Second warning contains original exception';
 }
 
-1;
+{
+  my $finally;
+  SKIP: {
+    try {
+      pass('before skip in try');
+      skip 'whee', 1;
+      fail('not reached');
+    } finally {
+      $finally = 1;
+    };
+  }
+  ok $finally, 'finally ran';
+}
diff --git a/t/global_destruction_forked.t b/t/global_destruction_forked.t
index a533000..05c35e9 100644
--- a/t/global_destruction_forked.t
+++ b/t/global_destruction_forked.t
@@ -18,15 +18,15 @@ use Try::Tiny;
   package WithFinally;
   use Try::Tiny;
 
+  our $_in_destroy;
   sub DESTROY {
+    local $_in_destroy = 1;
     try {}
     finally {};
     return;
   }
 }
 
-my $parent = $$;
-
 try {
   my $pid = fork;
   unless ($pid) {
@@ -52,6 +52,6 @@ try {
   waitpid $pid, 0;
   is $?, 0, 'nested try in cleanup after fork does not maintain outer finally block';
 }
-finally { exit 1 if $parent != $$ };
+finally { exit 1 if $WithFinally::_in_destroy };
 
 pass("Didn't just exit");