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
