rust-lang / futures-rs

Zero-cost asynchronous programming in Rust
https://rust-lang.github.io/futures-rs/
Apache License 2.0
5.35k stars 619 forks source link

Add AbortRegistration::handle #2712

Closed ZhennanWu closed 1 year ago

ZhennanWu commented 1 year ago

Allow to create AbortHandle from AbortRegistration. The implementation is obvious. Since AbortHandle is already Clone, this API should not create too much problem.

Motivation

The intended use case is pretty niche. I'll try to explain. There is this single scenario where we have access to AbortRegistration, but can't have access to AbortHandle.

I'm trying to create a zero-cost abstraction over the different async executors. One major concern is to ensure the uniformity of cancellation propagation between different async executor backends. When I try to implement a cancel-on-drop semantic, due to limitations of async-std, it requires an Abortable wrapper to ensure the cancellation propagation. (as others have also found out).

Users may well combine a cancel-on-drop spawn with an Abortable, which results in two consecutive Abortable wrappers in async-std, which may be flattened into one. For zero-cost abstraction, a more specific "cancel-on-drop-with-Abortable" semantic is needed to avoid the extra cost on async-std. Which, unfortunately, has no other signature than the following.

pub trait SpawnAbortableScoped: Send + Sync + 'static {
    type JoinHandle<Output: Send + 'static>: JoinHandle<Output>;
    fn spawn_abortable_scoped<F, T: Send + 'static>(
        &self,
        f: F,
        reg: futures::future::AbortRegistration,
    ) -> Self::JoinHandle<Result<T, futures::future::Aborted>>
    where
        F: Future<Output = T> + Send + 'static;
}

We cannot add an AbortHandle parameter since other executors do not need AbortHandle to propagate the cancellation. Adding one violates zero-cost abstraction. However, on async-std we do need an AbortHandle for cancellation propagation. This prompts the need for this API.

taiki-e commented 1 year ago

Thanks for the PR. This looks good to me.

I thought new_* might be a little clearer on the name, but I have no strong opinion.

ZhennanWu commented 1 year ago

I thought new_* might be a little clearer on the name

The new_handle name under AbortRegistration may be a little confusing. How about naming it after StopSource::token(), so we name it as AbortRegistration::handle()?

Edit: I mean by "new_handle" it kind of suggests the creation of a entirely unrelated "new" abortable, especially when there is "AbortHandle::new_pair" around

ZhennanWu commented 1 year ago

@taiki-e I've changed the name to AbortRegistration::handle for now. After some implementation using this new API, now I think AbortRegistration::handle is indeed a better choice. Just like the StopSource::token()

Compare

let handle = abort_reg.create_handle()

to

let handle = abort_reg.new_handle()

to

let handle = abort_reg.handle()

The last one express the fixed dependency of handle on the registration, and its shared reference nature.