Improve `finally` block exception handling

[some-java-user-99206970363698485155 at vodafonemail.de]

Thanks a lot for your feedback! My original message was a bit vague in parts, sorry for that.

The proposal consists of the following:
1. Forbidding usage of `break`, `continue`, `yield` and `return` in `finally` blocks
2. Adding exceptions as suppressed exceptions:
   If exception E1 led to execution of the `finally` block and the block is left due to
   another exception E2, then either E1 or E2 should be thrown with the other one added
   as suppressed exception. For consistency with try-with-resources and because E1 is
   most likely more relevant ideally E1 would be thrown and E2 should be suppressed. But
   if you think the impact on backward compatibility would be too large, then E2 should
   be thrown (the current behavior), but E1 should at least be added as suppressed exception.

The following replies to your comments hopefully make the rationale for these proposed
changes clearer.


> The behaviour of try/catch/finally is not "obvious at all", you have to read what it means
> and when you do that you clearly see what happens regarding exceptions.

For try-catch you see that the code catches some specific exception and then handles it in a
certain way, whether that is by rethrowing, logging or itentionally ignoring it.
The issue with `finally` is that in its current form, exceptions which occurred in the `try`
block might just silently disappear. Consider this simple example:
```
try {
    throw new RuntimeException();
} finally {
    return true;
}
```
Here it is not clear at all, unless you have read the JLS in detail, that the thrown exception
just vanishes. There is no explicit indication that the `finally` has any effect on the
thrown exception. Of course this is a contrived example, but consider the same situation where
the `try` block calls a method which might throw an exception (or the general case that a
VirtualMachineError occurs), and that the `return` (or any of the other affected statements)
is not the only statement in the `finally` block.

Similar confusing code can be written where the `try` or `catch` block returns a value
(or continues or breaks loop iteration), but the `finally` block overwrites that action:
```
try {
    return 1;
} finally {
    return 2;
}
```
Again, contrived, but consider the same code with additional statements in the `try` and
`finally` blocks. This breaks fundamental assumptions one has about the behavior of the
statements, such as `return`.

>> Are there any plans to forbid usage of `break`, `continue`, `yield` and `return` in
>> `finally` blocks?
>
> Why would we do that? What would that gain?

It would prevent code such as the one shown above, where code, most likely unintentionally,
discards exceptions. Now also consider that inside the `try` block a VirtualMachineError
(or a subclass of it) may occur which you really should not catch. Yet the code in the
`finally` block silently discards it without you ever noticing the error, before it occurs
later in another part of the application again and has possibly already corrupted the state
of the application.


>> Similarly for `throw` and any implicitly thrown exceptions, is there a plan to at least
>> add the original exception as suppressed one to the newly thrown?
>
> That suggestion is not completely without merit, but nor is it a "slam-dunk" obvious thing to do. The semantic implications of the exceptions matter, and semantics come from the programmers intent. There's no reasonable way to automagically determine that when an exception is created that another exception (that led to the finally block) should be inserted as a "suppressed exception". That would actually be completely wrong to do in many circumstances you would instead need to act when the exception would terminate the finally block and then add the original exception as the "suppressed" exception.

To clarify my suggestion: If a `finally` block is entered due to an exception E1, and is
exited due to an exception E2 (regardless of whether explicitly thrown by a `throw` statement),
and E1 != E2, then both exceptions should be preserved, with one being added as suppressed
exception to the other one.


> But really the programmer is in the best position to decide how exceptions need to be handled.

Except that in a `finally` block they don't have access to the exception which led to execution
of the `finally` block, unless they write verbose hacky code to first have a `catch (Throwable)`
which stores the caught exception in a local variable. To me it appears for many `finally`
blocks in existing code it was not actively decided how exception handling should be done,
but rather it was forgotten that the original exception might get discarded when the `finally`
throws a new exception; or that behavior was considered acceptable because currently Java does
not allow you to handle it in a better way (which is one of the main points of this proposal).

>> Of course one could argue that the same applies to `catch` clauses whose body accidentally
>> causes an exception which discards the caught one. However the main difference is that
>
> Yes exactly the same.

This point was mainly a safeguard in my proposal to avoid someone extending the scope of
this proposal to `catch` clauses as well, which then could cause this proposal as a whole
to be turned down. Therefore I am explicitly not arguing for any changes to `catch` clauses
exception handling / suppression here, because there it is at least somewhat more obvious
what is going on with the exceptions.

>> there, only a specific exception type is caught (and discarded), whereas for `finally`
>> exceptions of _any_ type are discarded. It could also be argued that adding suppressed
>
> We have multi-catch now so that argument is somewhat weaker.

The point here was that you explicitly write the exception type, whether that is a single type
or a union type in case of a multi-catch, in constrast to how `finally` discards any exception
type without you explicitly naming the type.

>> exceptions decreases performance or causes memory leaks, but one the other hand
>> this behavior was explicitly included try-with-resources statements, most likely because the
>> value this adds was considered more important than any performance issues this
>> might cause.
>
> try-with-resources added support for suppressed exceptions because the automatic closing of the resource could throw an exception, and that had to be factored in to the whole mechanism.

Yes, but I think the scenarios are exactly identical:
try-with-resources:
  1. Original exception E1 occurs
  2. Resource cleanup is done, here through implicit `close()`
  3. `close()` throws an exception E2
  4. E2 is added as suppressed to E1

Current `finally` block behavior:
  1. Original exception E1 occurs
  2. Resource cleanup is done, here manually in `finally` block
  3. `finally` block throws an exception E2
  4. !! E1 is silently lost !!

Why is it here acceptable to lose the original exception E1, even though the use
cases are exactly the same?

I assume the reason why they behave differently is historic, because `finally` blocks
exist way longer than suppressed exceptions. However, that does in my opinion not
preclude updating it retroactively, especially when it is unlikely that the difference
will have a visible negative effect at runtime, but instead makes understanding
application failures easier.

Kind regards