RFR: jsr166 jdk9 integration wave 2

[Peter Levart]

Hi,


On 11/23/2015 03:12 PM, Doug Lea wrote:
> On 11/23/2015 04:54 AM, Peter Levart wrote:
>
>>
>> In CompletableFuture.uniWhenComplete method, the possible exception 
>> thrown from
>> BiConsumer action is added as suppressed exception to the exception 
>> of the
>> previous stage. This updated exception is then passed as completion 
>> result to
>> next stage. When previous stage is appended with more than one 
>> asynchronous
>> continuation:
>>
>> ...then both secondary exceptions are added as suppressed to the same 
>> primary
>> exception:
>>
>> This is not nice for two reasons:
>>
>> - Throwable::addSuppressed is not thread-safe
>> - The consumer of the result of one CompletableFuture can see the 
>> exceptional
>> result being modified as it observes it.
>>
>
> Thanks. The minimal solution is to lock, at least avoiding conflict
> among multiple whenCompletes:
>
> !                 else if (x != ex)
> !                     x.addSuppressed(ex);
>
> --- 771,781 ----
>
> !                 else if (x != ex) {
> !                     synchronized (x) {
> !                         x.addSuppressed(ex);
> !                     }
> !                 }
>
> This is not as good a solution as your proposal to add Throwable.clone(),
> but we should do this until something like clone is in place.

Sorry for confusion. I now noticed that Throwable.addSuppressed() and 
getSuppressed() methods are actually synchronized! I don't know how I 
missed that...

So above patch is not needed. Even printing uses getSuppressed() that 
returns a snapshot. So the only thing remaining is the behavior that 
exceptions can change while they are being observed. If this is 
acceptable then this whole report of mine is just a bunch of misinformation.

Regards, Peter

>
>> When this stage is complete, the given action is invoked with the 
>> result (or
>> null if none) and the exception (or null if none) of this stage as 
>> arguments.
>> The returned stage is completed when the action returns. If the 
>> supplied action
>> itself encounters an exception, then the returned stage exceptionally 
>> completes
>> with this exception unless this stage also completed exceptionally."
>>
>> Could specification be tweaked a bit? The last statement leaves it 
>> open to what
>> actually happens when "this stage also completes exceptionally".
>
> The looseness was intentional. We'd like to improve debuggability of
> implementations without strictly promising a particular form in 
> interfaces.
> This is similar to what's done in ForkJoinTask, where we try to
> relay exception causes from other threads, but can't promise anything
> beyond a plain exception.
>
> -Doug
>