finally { } block is not always called

[pali]

When doing jump from try { } or catch { } block outside of block then finally is not called.

Example 1:

use Try::Catch;
for (1) {
  try {
    last;
  } catch {
  } finally {
    print "finally was called\n";
  }
}

Example 2:

use Try::Catch;
for (1) {
  try {
    die;
  } catch {
    goto skip;
  } finally {
    print "finally was called\n";
  }
} 
skip:

finally was called is never printed even it was specified in finally { } block which should be always called.

When both examples are used with Try::Tiny instead Try::Catch then examples correctly print finally was called.

This makes finally { } block with Try::Catch totally unusable.

[pali]

The only way how to fix finally { } block (without XS code) is to use scope guards, similarly to Try::Tiny...

[mamod]

I've never thought of such a use case
by the way Try::Tiny v0.22 had the same behavior, so it must be fixed with later versions, I'll take a look

[pali]

Look at the last test in Try-Tiny/t/finally.t. This is the most used use-case.

{
  my $finally;
  SKIP: {
    try {
      pass('before skip in try');
      skip 'whee', 1;
      fail('not reached');
    } finally {
      $finally = 1;
    };
  }
  ok $finally, 'finally ran';
}

skip in Test::More is implemented as call to last SKIP; which is same as in my examples.

[mamod]

Yes this was introduced in v0.25 with this commit p5sagit/Try-Tiny@8035202

I'll try to fix and add tests, this will add some overhead to the execution time.

[pali]

This is not so easy as Try::Catch supports throwing errors from finally block. Try::Tiny does not support it...

But I have some "hack" in my mind which should supports throwing errors when jump/goto is not used. Wait some time and I will try to write it :-)

[mamod]

Same thought here so LOL that would be much appreciated :)

[pali]

I tried to fix this problem, but do not see any solution in pure perl code which ensure that finally { } block will be always called and also finally { } block can die. This is really limitation of perl.

Next I tried hack to create scope guard for finally { } block and also call finally { } block before exiting whole Try::Catch::try implementation. In this case finally block signal scope guard if it was called or not. It would allowed me to propagate die from finally { } block if some jump/goto was not used. But my implementation again not worked correctly (problems with fork() and exit()) and I guess this is not easy to write correctly...

Moreover such implementation slow down Try::Catch module... and we already have Try::Tiny which implement this support (but without die propagation in finally { } blocks).

So I would suggest to document this bug in module POD section with information that using jump/goto or fork/exit is not supported in this module. I understood that primary goal of this module is speed and those who needs different support for finally { } blocks then should use Try::Tiny.

[mamod]

I'm sorry for the delay and I really appreciate your working on this.
I guess you're right, the best solution is to document this behavior, I'll do that and release to CPAN
Thank you 👍

[pali]

Hm... I was thinking about it and I do not feel that it a good idea to have non-working finally { } block implementation even it will be somehow documented...

Maybe it would be better to completely remove finally { } support and not provide false experience and view that finally { } block is provides, so it should work correctly....

But it is up to you how you decide... In this bug report I reported that current implementation is non-working and broken... If aim of this module is speed, then maybe it really make sense no support for finally { } block.

Anyway, see this commit in DBIx::Class module:
Perl5/DBIx-Class@e2741c7#diff-c13797cc2e5864c4a1d6a92ba65871b6L611

Try::Tiny is no so tiny and slow... So looks like some modules already needs some lightweight try { } catch { } implementation even without finally { } support...

[mamod]

@pali oops missed this one :(

I think you're right again, keeping the finally with this odd behavior will trick users, as the whole point of finally block is to be executed on all situations except in up-normal situations like system exit, skipping the finally block is the same as putting code outside the try/catch!!

I just uploaded try::catch v1.1.0 to cpan with the new fixes you submitted and added a known bugs section describing this behavior, I'll be deprecating the finally block in v2.0.0, what do you think?

[pali]

Yes, as I wrote it would be really better to remove not-fully-working finally { } block support...

[pali]

Are you going to to remove that not-fully-working finally { } block support?