RFR: jsr166 jdk9 integration wave 2

[Martin Buchholz]

I'm in agreement with Peter.

I don't see "handle", but rather "exceptionally", as the analog of java "catch".

I agree with both Doug and Peter that it would be nice to have
whenComplete as the analog of finally, but as Peter points out, the
exception from the finally block wins, and that strengthens my
argument that we should not throw the source exception when the action
throws.

The existing specs are unclear.  I see both
--
In the case of method whenComplete, when the supplied action itself
encounters an exception, then the stage exceptionally completes with
this exception if not already completed exceptionally.

If the supplied action itself encounters an exception, then the
returned stage exceptionally completes with this exception unless this
stage also completed exceptionally (in which case, the returned stage
exceptionally completes with the original exception).
--
and I am reading those differently (but "if not already completed
exceptionally" is ambiguous!)

It's time for wave 3, and our failure to agree here is in the way a little.

On Wed, Dec 16, 2015 at 8:46 AM, Peter Levart <peter.levart at gmail.com> wrote:
> 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)
>>
>