Re: CompletableFuture.cancel vs Future.cancel bug?

Got it, thanks!

Regarding the javadoc, I would avoid the words "success" and "failure" or
we need to clarify what exactly they mean in this context.

Maybe something like this would be more clear:

     /**
      * Attempts to cancel execution of this task. This method has no
      * effect if the task is already completed or cancelled or
      * if the task could not be cancelled for any other reason. If
      * this task has been cancelled before it has started, it should never
run.

Also, the description of the return value makes me think that it is allowed
to return true more than once only if multiple threads have executed
cancel() at the same time,
but a single thread can not see this effect if it calls cancel() multiple
times sequentially. In other words the following test must pass:

var future = new CompletableFuture<>();
assertTrue(future.cancel(true)); // Causes cancellation.
assertFalse(future.cancel(true)); // Does not cause cancellation because
the future has already been cancelled by the previous call.

But obviously this does not hold for CompletableFuture. I would avoid
mentioning that there must be more than one thread at play.

I would rather rewrite this return value description completely, because
the part "return {@code false} if the task could not be cancelled" sounds
very misleading.
To me it still means that if a future has already been cancelled then it
can not be cancelled twice and thus the second cancel() call must return
false.

From what I see there are two main types of cancel() implementation: one is
CAS-like, another returns true if already cancelled.
The most clear and comprehensive description I could come up with:

      * @return {@code true} if the cancellation is successful.
      * The exact semantics of "successful cancellation" is implementation
specific:
      * it may mean that this exact call caused the future to become {@link
#isCancelled cancelled}
      * or that the future is {@link #isCancelled cancelled} now regardless
of what caused it,
      * other implementations are permitted as well.
      */




On Sun, Aug 23, 2020 at 11:15 PM Doug Lea <[email protected]> wrote:

> On 8/23/20 9:30 AM, Sergi Vladykin wrote:
> >
> >
> > Yes, my interpretation was exactly "successful attempt" == "return
> > true", "failed attempt" == "return false". Would be nice to have this
> > spec improved.
> See draft update below, that also tries to clarify mayInterruptIfRunning.
> >
> > Also, could you please clarify why the decision was made not to have
> > CAS-like semantics for CompletableFuture.cancel() while it was clearly
> > possible? Maybe there are some hidden benefits I don't see?
> >
> I don't have a clear recollection (it was during JDK5), but it may have
> been a reflection of issues with (concurrent) Collection remove()
> methods requiring that the return value indicate that the current call
> was responsible for removal, which is sometimes arbitrary and causes
> extra expense and complexity. (We considered trying to weaken this, but
> didn't.)
>
> ... possible javadoc update:
>
>      /**
>       * Attempts to cancel execution of this task.  This method has no
>       * effect if the task is already completed or cancelled, and may
>       * fail if the task could not be cancelled for any reason. If
>       * successful, and this task has not started when {@code cancel}
>       * is called, this task should never run.  If the task has already
>       * started, then the {@code mayInterruptIfRunning} parameter
>       * determines whether the thread executing this task (if it is
>       * known by the implementation) should be interrupted in an
>       * attempt to stop the task.
>       *
>       * <p>After this method returns, subsequent calls to {@link
> #isDone} will
>       * always return {@code true}.  Subsequent calls to {@link
> #isCancelled}
>       * will always return {@code true} if this method returned {@code
> true}.
>       *
>       * @param mayInterruptIfRunning {@code true} if the thread
>       * executing this task should be interrupted (if the thread is
>       * known to the implementation); otherwise, in-progress tasks are
>       * allowed to complete
>       * @return {@code false} if the task could not be cancelled,
>       * typically because it has already completed; {@code true}
>       * otherwise. If two or more threads cause a task to be cancelled,
>       * then at least one of them returns {@code true}.
>       */
>      boolean cancel(boolean mayInterruptIfRunning);
>
>

_______________________________________________
Concurrency-interest mailing list
[email protected]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest