Re: CompletableFuture.cancel vs Future.cancel bug?

Anecdotally, we do have at least one place in Guava in which we rely on *our
own Future implementations* to return true from cancel(...) only if that
particular call was responsible for the cancellation. Here's the code,
which could otherwise NPE
<https://github.com/google/guava/blob/9b972a21f793fcbb1ac2d41d6c6d6c97d54adb25/guava/src/com/google/common/util/concurrent/Futures.java#L918>
(by
dereferencing state/localState, which would be nulled out by the first
cancellation). Glancing at the first few pages of hits in the wider Google
codebase, I can see at least one more bit of code that would throw an
exception, at least one that would lose track of the number of outstanding
tasks, and a few that would track metrics incorrectly.

That said:

   - As I suggested above, we are very likely to encounter only our own
   Future implementations. (Sometimes this is outright guaranteed, as in the
   example I linked.)
   - If we encounter other Future implementations, the problem already
   exists. Changing the doc to better explain the state of the world sounds
   like a good thing.
   - We might be able to work around the different return value (though
   perhaps sometimes at a noticeable cost in complexity or performance).

So practically speaking, we are not very likely to be bitten by the current
behavior. And I agree that a change to that behavior is riskier than doing
nothing. So I like the idea of just better documenting how things stand
today.

If anyone out there is implementing Future or a similar API from scratch,
though, I wanted to at least encourage you to implement cancel(...)'s
return value according to the current docs: The extra information can be
useful (and it sometimes turns out to be easy enough to provide, as in
CompleteableFuture, which actually has to do extra work to return true in
the "already cancelled" case).

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