Re: CompletableFuture#postComplete() can be invoked concurrently?

Martin Buchholz via Concurrency-interest <[email protected]> Sat, 22 Aug 2020 23:25:07 -0700
Newsgroups gmane.comp.java.jsr.166-concurrency
Message-ID <CA+kOe0_ELO2oLGvOnietFWHGqaHf6tWZhar-hYPojdhhf+8_dQ@mail.gmail.com>
You may be right, but ... I recall when working on this code myself I
ended up asking the same sorts of questions, and later realizing the
code was correct, but as usual I forget the details.

Current CompletableFuture has no known correctness bugs, so we'd be
reluctant to fix anything here without a repro.
You only need the repro to fail once in a million tries.

You might try changing the code in jsr166 CVS and running the tests,
perhaps repeatedly, perhaps with expensiveTests turned on.

     *   forms.)  The claim() callback suppresses function invocation
     *   if already claimed by another thread.

Looks like claim() is only intended to suppress duplicate runs of a
completion function, which normally only happens on successful
completion of a source future.

On Sat, Aug 22, 2020 at 8:42 PM Liu <[email protected]> wrote:
>
> It is kind of difficult to test it, because it all depend on special Thread execution order.
> I can't think of the test case temporarily.
>
> Let my opinion simpler.
>
> http://hg.openjdk.java.net/jdk/jdk15/file/d2c6eb3b2c8d/src/java.base/share/classes/java/util/concurrent/CompletableFuture.java#l615
>
> In JDK15, CompletableFuture.UniApply#tryFire(), I think
> it should be like this:
>
>     @SuppressWarnings("serial")
>     static final class UniApply<T,V> extends UniCompletion<T,V> {
>         Function<? super T,? extends V> fn;
>         UniApply(Executor executor, CompletableFuture<V> dep,
>                  CompletableFuture<T> src,
>                  Function<? super T,? extends V> fn) {
>             super(executor, dep, src); this.fn = fn;
>         }
>         final CompletableFuture<V> tryFire(int mode) {
>             CompletableFuture<V> d; CompletableFuture<T> a;
>             Object r; Throwable x; Function<? super T,? extends V> f;
>             if ((a = src) == null || (r = a.result) == null
>                 || (d = dep) == null || (f = fn) == null)
>                 return null;
>             tryComplete: if (d.result == null) {
>                 if (r instanceof AltResult) {
>                     if ((x = ((AltResult)r).ex) != null) {
>                         if (!claim())         //before completeThrowable, it should be claimed (this two lines is added by me)
>                             return null;
>                         d.completeThrowable(x, r);
>                         break tryComplete;
>                     }
>                     r = null;
>                 }
>                 try {
>                     if (mode <= 0 && !claim())
>                         return null;
>                     else {
>                         @SuppressWarnings("unchecked") T t = (T) r;
>                         d.completeValue(f.apply(t));
>                     }
>                 } catch (Throwable ex) {
>                     d.completeThrowable(ex);
>                 }
>             }
>             src = null; dep = null; fn = null;
>             return d.postFire(a, mode);
>         }
>     }
>
> before completeThrowable, it should be claimed just like following code.
> This is mean to avoid two diffrent threads can invoke tryFire on same Object,
> and to avoid it will both do the d.completeThrowable(x, r) successfully and do
> d.postFire(a, mode) both.
>
> Or you can tell me, Why not need to claim() before d.completeThrowable(x, r).
> I suppose it is necessary.
>
_______________________________________________
Concurrency-interest mailing list
[email protected]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest