RFR: jsr166 jdk9 integration wave 2

[Peter Levart]

Hi Doug,

On 12/16/2015 03:09 PM, Doug Lea wrote:
> On 12/02/2015 07:51 PM, Martin Buchholz wrote:
>> We're stuck.  Peter wants to make Throwables cloneable, I want to
>> reverse the order of throwables and add the past Throwable as a
>> suppressed exception to the exception of a dependent action, and Doug
>> wants the current webrev behavior of adding the dependent action as a
>> suppressed exception of the source.
>>
>
> After letting this sit for a week or so, I think the main issue
> here is that we do not properly explain how and when to use
> whenComplete() vs handle(). We should explain that handle()
> should be used in the same way as "catch", and whenComplete as
> "finally"/try-with-resources. See below.
>
> Given this, the most appropriate response in whenComplete upon
> a secondary exception encountered when handling the primary exception
> is the same as the finally-based scheme used in try-with-resources:
> primary.addSuppressed(secondary).  Which we should have done
> originally (vs dropping secondary), and is the current proposed
> webrev.
>
> (In contrast, if someone would like to translate one exception
> into another by throwing the new one, they could use handle().)
>
> Peter raised the issue that addSuppressed could visibly (but safely)
> alter the primary while it is visible to some concurrent action.
> Which is true, but also no different than is possible with
> try-with-resources (consider that the primary exception there could
> be already somehow accessible). 

It could be, but in try-with-resources there is usually just a sequence 
of actions in a single thread that is always deterministic, while in 
whenCompleteAsync for example, a thread requesting the outcome of 
previous stage can observe it's outcome with suppressed exception added 
or not, depending on timing.

> If exceptions were cloneable,
> it might be less surprising to users in both cases to create a new
> cloned exception with the suppressed secondary. But I think we now
> agree that pseudo-cloning exceptions has even more opportunity for
> surprising users, so should be done only when there aren't any
> alternatives.
>
> Agreed?

Agreed. Pseudo cloning (using reflection to invoke constructor) is not 
reliable.

>
> Here's a clarification for CompletionStage (un-reformatted).
> Improvements welcome. Someday we might consider adding some examples.
>
>    *
>    * <li>Two method forms support processing whether the triggering
>    * stage completed normally or exceptionally: Method {@link
> !  * #whenComplete whenComplete} allows injection of an action
>    * regardless of outcome, otherwise preserving the outcome in its
> !  * completion. Method {@link #handle handle} additionally allows the
>    * stage to compute a replacement result that may enable further
>    * processing by other dependent stages.  In all other cases, if a
>    * stage's computation terminates abruptly with an (unchecked)
> --- 58,66 ----
>    *
>    * <li>Two method forms support processing whether the triggering
>    * stage completed normally or exceptionally: Method {@link
> !  * #whenComplete whenComplete} is similar to a {@code finally} 
> clause, allowing injection of an action
>    * regardless of outcome, otherwise preserving the outcome in its

The finally block (in classical try/finally or in 
try-with-resources/finally) preserves the outcome of try block only if 
the finally block completes normally. Otherwise the outcome is replaced 
with the exception thrown or value returned in finally block. For example:

// IllegalStateException propagates out of:

         try {
             throw new IllegalArgumentException();
         } finally {
             throw new IllegalStateException();
         }

// as well as out of:

         try (AutoCloseable r = () -> {}){
             throw new IllegalArgumentException();
         } finally {
             throw new IllegalStateException();
         }

// this method returns 2:

     public static int x() {
         try {
             return 1;
         } finally {
             return 2;
         }
     }

If we are talking about the automatic clean-up action 
(AutoCloseable::close()) and not of the finally block, then the 
exceptional outcome of try block is always preserved and the exception 
thrown from AutoCloseable::close() is just added as suppressed to the 
exception from try block:

// IllegalArgumentException propagates out of:

         try (AutoCloseable r = () -> { throw new 
IllegalStateException(); }){
             throw new IllegalArgumentException();
         }

...but non-exceptional outcome of try block is replaced with the 
exception from AutoCloseable::close():

// this method throws IllegalStateException:

     public static int x() throws Exception {
         try (AutoCloseable r = () -> { throw new 
IllegalStateException(); }){
             return 12;
         }
     }

If we are talking about auto-closing, then the text should not speak 
about "finally", as whenComplete is not equivalent to finally block then.

Since whenComplete takes a BiConsumer, it can not "replace" the 
non-exceptional outcome of previous  stage. In this respect it is more 
similar to void method AutoCloseable::close() than it is to finally 
block which can return a value from the method. So perhaps we should 
model it to the AutoCloseable::close() in try-with-resources then. Which 
is what is proposed. It's just that non-determinism when multiple 
threads are involved that might cause surprises.

Regards, Peter

> ! * completion. Method {@link #handle handle} is similar to a {@code 
> catch} clause, allowing the
>    * stage to compute a replacement result that may enable further
>    * processing by other dependent stages.  In all other cases, if a
>    * stage's computation terminates abruptly with an (unchecked)
>