Add `poll_cancel` to `Future`

Proposal

Add the following method to the Future trait:

fn poll_cancel(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<()> {
    Poll::Ready(())
}

The result would be:

pub trait Future {
    type Output;
    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output>;
    fn poll_cancel(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<()> {
        Poll::Ready(())
    }
}

Future contract

The new contract for Future is:

  • All futures are cancellable.
  • Once poll_cancel is called, poll may never be called again.
  • A polled future may not be dropped without poll_cancel returning ready.

Guarantees of poll_cancel:

  • Cancelling an unpolled future is safe.
  • Cancelling a finished future is safe.
  • Cancelling a cancelled future is safe.

Thus, a future must either be driven to completion or be cancelled before it can get dropped. Also, any future can be safely cancelled at any moment in its lifetime.

Motivation

Currently futures are all assumed to be trivially cancellable. This is to say that the can be cancelled in a synchronous operation (drop). However, this abstraction has turned out to be too limitating for certain asynchronous APIs, including, most notably io_uring. These APIs require cancellation to be an asynchronous operation (async drop).

Providing a poll_cancel method to the Future trait will help the asynchronous ecosystem to migrate to a more flexible abstraction for future cancellation.

Async

Asynchronous functions and blocks will have to generate a state machine that can keep track of a future in mid cancellation as a possible state.

Compatibility

Even though adding a default method to a trait is not a breaking change, changing the contract for the Future trait is breaking. This however, should only affect crates implementing a runtime, which are a minority. Libraries implementing futures will not be affected, since all futures already adhere to this contract.

Related topics

EDIT:

When I say "safe", I don't mean "memory safe", I mean safe from panicking or blocking. The entire API should always be memory safe.

1 Like

Any proposal that involves breaking backwards compatibility of a stable API is a complete nonstarter.

12 Likes

I believe the benefits that such a method would offer would outweigh the costs. And as I said, this would only be a breaking change for runtimes such as tokio, which are quite few compared to future implementors and consumers of those runtimes.

You could add this as a new trait (let's call it DelayedDropFuture, which would also provide all the Future combinators that are possible), add a blanket impl of DelayedDropFuture for any T: Future, and then change runtimes to only require DelayedDropFuture instead of Future.

Async functions would then generate a DelayedDropFuture if any of the awaits are such a future, or alternatively an async(DelayedDropFuture) syntax could be required.

I guess this has already been proposed though.

2 Likes

[citation needed]

While it may be true for async-generated Futures, it is still possible to implement Future manually, in which case this would be a change of contract.

Breaking stable API contracts (especially unsafe, safety-critical ones) is a complete nonstarter.

The most I personally believe we'll get for asynchronous cancellation is something along the lines of

  • poll_cancel or similar is added with a default Poll::Ready
  • It is recommended but not required to poll poll_cancel to completion before dropping a future
  • If poll_cancel is not called, the drop implementation may block "unnecessarily"
  • If poll_cancel is not specialized ("legacy" impls), the drop implementation may block "unnecessarily"
  • If poll_cancel is specialized and polled to completion, the drop implementation should not block

Any requirement to not support silent blocking unwind cancellation via drop for normal futures is a complete non-starter, because current code is allowed to do so.

14 Likes

https://rust-lang.github.io/wg-async/vision/roadmap/scopes.html

Especially

https://rust-lang.github.io/wg-async/vision/roadmap/scopes/capability/variant_async_trait.html

The problem I see with this approach is that there is no way to safely write a default impementation of DelayedDropFuture for T: Future. Suppose it were this:

impl<T: Future> DelayedDropFuture for T {
    fn poll_cancel(...)  -> Poll<()> {
        Poll::Ready(())
    }
}

Then user defined futures (with async) would block on drop. So it would be better todrive the future to completion to cancel it, like this:

impl<T: Future> DelayedDropFuture for T {
    fn poll_cancel(...)  -> Poll<()> {
        match self.poll() {
            Poll::Ready(_) => Poll::Ready(())
            _ => Poll::Pending
        }
    }
}

But then you are not actually cancelling the future. So I don't think this implementation can actually be written, unless async code returns an impl DelayedDropFuture instead of a impl Future. But then again, that would be a worse breaking change.

I don't think that API is terribly different to the one I described. Although I can imagine in the future people panicking instead of blocking.

futures that would "block on drop" would not implement the Future trait, but only DelayedDropFuture (which would include async-generated futures that await futures not implementing Future).

It's not a breaking change to change async to not implement Future when futures that don't implement Future are awaited (that code would currently not compile) or when the user specifies async(DelayedDropFuture) (that code would currently not parse).

1 Like

No, this also breaks any pinning API such as Box::pin and pin_mut! because they can be used to pin a future and poll it entirely from safe code, where it's not an hard requirement to call poll_cancel nor Drop::drop. To solve this you need to either:

  • mark Future::poll and Future::poll_cancel as unsafe functions;
  • change the requirements for pinning to account for poll_cancel.

Both changes are highly breaking changes since the first one breaks any manual implementation of Futures, while the second makes a bunch of highly used APIs like Box::pin, impl<T> From<Box<T>> for Pin<Box<T>> and pin_mut! unsound.

3 Likes

To be clear, I'm not suggesting than this code should be UB:

let fut =  Box::pin(future()); 
fut.poll(); 
fut.poll_cancel(); 
fut.poll(); // wrong: polling after cancelling
drop(fut); // wrong: dropping without cancelling being ready

I'm suggesting that the previous code may panic at runtime or may block on drop. Future implementations should always be safe, even if the Future contract is not respected.

I guess I misinterpreted this part then:

io_uring (with user-provided buffers, i.e. &mut [u8]) requires destructors to run for soundness, so if poll_cancel is not required to run how is it relevant for io_uring then?

3 Likes

The biggest issue I see with this proposal is that it seems to target a different language than Rust: saying that a value cannot be dropped under some circumstances is equivalent to needing a linear type system. The reason is that with affine types (which is all we got) there is no way for the compiler to assist people in keeping the contract.

I’d advocate for the opposite: every value must be well-behaved when it is dropped, since Rust expends a substantial fraction of its complexity budget for making sure that the compiler can enforce this property. This is ingrained in the language’s design — drop glue is the only thing that is implicit in this otherwise completely explicit language.


On the topic raised as motivation, I’d suggest exploring a different angle. What if Rust had a well-established Executor trait (or similar) that would allow a Drop implementation to spawn a new task on the current thread pool?

That's really backward compatible contract for Future::poll_cancel, and it's the compromise the thread was looking for: on the one side it doesn't change the current behavior of futures, and on the other side it does provide all the guarantees this proposal aims to provide for cases that used the poll_cancel properly.

It would then either require an addition to current std::task::Context, or some with context...

1 Like

I think it is an interesting option worth exploring, but I don't know how it would look.

a buffer-borrowing io_uring implementation could block in Drop or abort() when used incorrectly.

That requires the destructor to run, which is not something that's guaranteed to happen. Thus it's not sound.

And how do you detect it?

1 Like

If the future owns the buffer, it is fine for the destructor to never run as the memory won't be reused anyway as per the Pin invariants.

1 Like