Cancellation deadlocks

[heiberg]

I'm seeing a pattern of deadlocks when attempting to cancel futures via a cancellation token that I can't make sense of.

I managed to boil it down to a pretty simple reproduction. Maybe I'm doing something wrong here, but I'm not sure what it is.

Let's start out with a utility function creating a simple, cancellable, long-running future:

func createFuture() -> Future<String> {
    let p = Promise<String>()
    p.automaticallyCancelOnRequestCancel()
    Executor.Default.executeAfterDelay(10) {
        p.completeWithSuccess("Done")
    }
    return p.future
}

This future will attempt to complete after 10 seconds.

In the following we'll try to cancel this future after 1 second.

Works fine

let future = createFuture()
let token = future.getCancelToken()
Executor.Default.executeAfterDelay(1) {
    token.cancel()
}

This works as expected. The future is cancelled after 1 second, and completeWithSuccess has no effect when that block runs after 10 seconds.

Deadlock 1

If we change the executor to .Main then the call to token.cancel() never completes. The main thread is deadlocked.

let future = createFuture()
let token = future.getCancelToken()
Executor.Main.executeAfterDelay(1) {
    token.cancel() // NEVER RETURNS
}

My attempts at troubleshooting:

It seems that token.cancel() goes through the onCancel handler set when the token was issued. This handler calls Future._cancelRequested with a synchronization object. This synchronization object is equal to future.synchObject and was given to the token when it was issued. We lock the object and proceed with the cancellation.

Because we used automaticallyCancelOnRequestCancel the handler attempts to complete the future immediately as part of the cancellation.

It ends up invoking future.completeAndNotify() which then tries to lock future.synchObject again. But we're already inside a lockAndModify on that object from the token.cancel() call, and so we have a deadlock.

Deadlock 2

Now let's go back to the cancellation which worked fine, invoking cancel on the .Default executor.

But now we add an onSuccess handler to the future.

let future = createFuture().onSuccess { _ in print("onSuccess") }
let token = future.getCancelToken()
Executor.Default.executeAfterDelay(1) {
    token.cancel() // NEVER COMPLETES
}

This also triggers a deadlock and token.cancel() never completes.

Workaround

Since it is the immediate completion of the future upon cancellation which causes the deadlock, one way to avoid this issue is to always return .Continue from the onRequestCancel of the promise. From the docs it seems this is recommend for other reasons as well.

I don't understand what makes the first example work, while the last two deadlock.

But it looks like I really should avoid returning .Complete(_) from onRequestCancel, which of course is what automaticallyCancelOnRequestCancel does?

I'm not 100% sure if this is a bug or a user error.

[mishagray]

So I think I fixed this here:
a1202c9

But it doesn't look like I merged this into the latest production branch.

It is in the new FutureKit 2.0 beta, which is where I have re-written all of the onCancel/onSuccess handlers to use protocols. It is slightly "breaking" (mainly if you ever tried using the FutureProtocol). But 2.0 seems to cleanup some of the onSuccess compiler-type-inference problems. (Not all.. but some).

The BIGGEST difference in 2.0, is that the handlers now come in "two" forms, one is that they return a new type (to map the future to a new type future) OR They return a CompletionType protocol object. And now things like FutureResult/Completion/ etc have been extended to CompletionType.

What's also cool, is you can extend a lot of 3rd party types (Result from ReactiveCocoa, Response from AlamoFire, etc..) into a CompletionType and basically "map" lots of other async "future-like" objects to Futures.

I've just been trapped in my new startup doing 70 hours of week of coding ... and haven't had time to update the notes etc.

I think I can cherrypick this fix back to the 1.x. Let me see.

[mishagray]

https://github.com/FutureKit/FutureKit/releases/tag/1.2.4 Can you verify this fixes your issue?

[heiberg]

Verified! Release 1.2.4 fixes all my reproducible deadlocks. Thanks! 🎉