Kotlinx.coroutines: Canceling a job while suspended in CompletableFuture.await should cancel the Future as well

[mvysny]

I believe that when a Job is canceled while suspended in the CompletableFuture, the CompletableFuture.cancel() should be called as well, so that the action ongoing in the future can terminate soon. Instead, cancel() is not called and the following code prints false:

fun main(args: Array<String>) {
    val f = CompletableFuture<Unit>()
    val job = launch {
        f.await()
        println("SHOULD NEVER BE PRINTED")
    }
    Thread.sleep(1000)
    job.cancel()
    Thread.sleep(1000)
    println(f.isCancelled)
}

[elizarov]

That's a tough question. Let me explain why. Consider this asynchronous function (the function that returns a future):

fun loadImageAsync(name: String): SomeKindOfAFuture<Image> 

Now, one way to use this function is like this:

val image = loadImageAsync(name).await()
doSomethingWithImage(image)

When use it like this, you naturally think that a cancellation of await should cancel the underlying operation by cancelling its future. However, consider the other way to use this async function:

val future1 = loadImageAsync(name1) // start loading one
val future2 = loadImageAsync(name2) // start loading another
doSomethingWithImages(future1.await(), future2.await()) // wait for both & process

This is the actual use-case for async function (and futures in particular). Should cancelled await cancel the operation here? It does not make a lot of sense, since we first wait for future1 and when its await gets cancelled only future1 gets cancelled. The other one (future2) will be still working. It is quite a misleading behavior.

If you want to cancel the future when the operation is cancelled you have to be explicit about it somehow:

val future1 = loadImageAsync(name1) // start loading one
val future2 = loadImageAsync(name2) // start loading another
try {
    doSomethingWithImages(future1.await(), future2.await()) // wait for both & process
} finally {
    future1.cancel()
    future2.cancel()
}

Bottom line here is that you should not use futures for where you meant to be doing sequential processing in the first place (like the first example). Futures are for concurrency. If your use-case is sequential like the first one, then you should try to express your code without futures, but using suspending functions.

[mvysny]

Thank you, I haven't thought of that. Yeah, canceling just one future would lead to a false sense of safety that everything has been canceled.

I was thinking of somehow using child jobs, but I guess it's impossible to force job API into the loadImageAsync() function. And this is true not only for Futures, but for all background processing launched from coroutines. Also, adding extension function Future.register() to register to current job as a child job would probably not work since people would tend to forget to call it.

What if await() was not present on the Future itself, but only on Deferred? And the only way to produce a Deferred from Future would be through a method, say, Future.bind()? Yes it could be misused:

val future1 = loadImageAsync(name1) // start loading one
val future2 = loadImageAsync(name2) // start loading another
doSomethingWithImages(future1.bind().await(), future2.bind().await()) // wait for both & process

But there could be a BIG warning to not to do this in the bind() method, but to call the bind() method asap? So that the code would look like this:

val d1 = loadImageAsync(name1).bind() // start loading one
val d2 = loadImageAsync(name2).bind() // start loading another
doSomethingWithImages(d1.await(), d2.await()) // wait for both & process

[elizarov]

The answer is simple. Just don't design your API around async functions (functions that return futures). It is bad and error-prone design practice. Design your code with suspending functions. Define the following function to load an image:

suspend fun Image loadImage(name: String)

Now, if you want to load two images concurrently you can request it explicitly and use parent-child relations for automatic cancellation:

val future1 = async(coroutineContext) { loadImage(name1) }
val future2 = async(coroutineContext) { loadImageAsync(name2) }
doSomethingWithImages(future1.await(), future2.await())