Future.await doesn't propagate cancelation signal to future

[svasilinets]

invokeOnCancellation only clears a reference to future in that coroutine

kotlinx.coroutines/integration/kotlinx-coroutines-guava/src/ListenableFuture.kt

Line 129 in 07c5ce4

callback.cont = null // clear the reference to continuation from the future's callback

[elizarov]

It is done so by design. The same approach is used in CompletableFuture.await() extension. The pattern when future is cancelled on await() is quite brittle. Future is generally a multi-use object and the fact that one waiter is cancelled does not generally mean than the future itself needs cancelled. However, indeed, there are some case where it would be convenient. Consider a case when you convert Java code that returns a future into a suspending function in Kotlin:

fun myFunctionAsync(): Future<T>

suspend fun myFunction(): T = myFunctionAsync().await()

If myFunctionAsync starts a new asynchronous operation and returns it as a future, it is clear that myFunction it the only user of that future and it shall cancel this operation. I don't have a good design for this case. I can propose that we have a dedicated version of await() that propagates cancellation, but I don't know how to call it.

[svasilinets]

Ah, I see. thanks for explanation.

As for naming, I don't have any good suggestions only if awaitWithCancelation(), which is superLongJavaName =\ What do you think about parameter with default in await() function?

PS Currently I miss one thing in the whole picture. Do you have the same issue with Deferred? its await function propagates cancelation, though nothing stops you from sharing this Deferred and await from two different coroutine contexts.

[qwwdfsad]

@svasilinets did you notice this behaviour accidentally or did it arise from a real problem which is hard to resolve without cancellation?

Do you have the same issue with Deferred?

Yes, though I'm not sure it is an issue.
Both Future and Deferred represent a value (or a promise that there will be one soon), not a computation. One can cancel, pause or interrupt computation, but cannot cache or reuse it, while a value can be cached and reused, but cannot be cancelled or paused.
We have cancel method on both Deferred and Future, but mostly to make API simple and avoid a lot of vague classes and bridges related to computation. I can speculate it's the reason JDK doesn't separate Future and Promise as well.

The simplest solution is to provide some boolean default parameter in await, e.g. await(cancelFuture = true), but then it will be inconsistent with Deferred and have it on Deferred as well IMO is not the best idea for the sake of (value-based) mental model simplicity.

[svasilinets]

I notice it accidentally, just exploring this topic, because we're going to expose ListenableFuture in our java api and have corresponding suspend functions in Kotlin apis.

I thought and browse code a bit and now I think that default behavior for ListenableFuture.await() should be propagate cancelation (instead of current one), for following reasons:

1. This is default behavior for guava. Implementation examples:
   TimeoutFuture. It is implementation of Futures.withTimeout().
   AggregateFuture that backs Futures.allAsList and others.
2. it is default behavior for coroutines (= Deferred)

So it will be weird that this behavior breaks on the edge of two frameworks that have the same defaults.

PS. In guava there is special nonCancellationPropagating if anyone wants to stop propagation.

[elizarov]

It is not a default for Deferred in coroutines at the moment, but it is a "default" for suspending functions. If you treat suspending function invocation as asyncXXX().await(), where asyncXXX() returns some kind of a future (Deferred, CompletableFuture, Guava's ListenableFuture, etc), then this difference in behavior breaks isomorphism between Kotlin coroutines API (based on suspending functions ) and asynchronous APIs that are based on futures.

[svasilinets]

Yeah, I was confused by that differences between cancelation+deferred and cancelation+suspend functions. So yeah, it makes sense to align ListenableFuture.await() with Deferred.await().

However, I find this behavior for Deferred surprising as well, for the exact "isomorphism" reasons you described.

But I'm not sure what is your position on it: is it WAI or is it something that you want to tweak?

[svasilinets]

Ok I found this issue and it answers my question.

In my humble opinion, once you've done those mentioned changes and it'd make sense to reiterate on decision about futures, because right now in similar fashion all future are always "global". However, I acknowledge that it is no straightforward way to apply same pattern to futures as you do for child coroutines.