Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run poll_drop destructors when invoking Task::cancel #2

Open
yoshuawuyts opened this issue Jul 20, 2020 · 2 comments
Open

Run poll_drop destructors when invoking Task::cancel #2

yoshuawuyts opened this issue Jul 20, 2020 · 2 comments

Comments

@yoshuawuyts
Copy link
Member

yoshuawuyts commented Jul 20, 2020

The Task::cancel API is used to cancel a task through the remote handle. The Drop::poll_drop_ready RFC proposes a mechanism through which async destructors can be added to the language. I'd like for Task::cancel (or a possibly a new API) to provide a way to cancel a task and provide a chance for the destructors to run.

Example Usage

In Tide we allow people to spin up multiple Server instances in the same application. One "async drop" becomes available what we'd like to do is make it so when Server is dropped it stops accepting new connections, and then allows the existing connections to finish responding. This can then be used to e.g. reload configuration, or gracefully exit the application pending an upgrade.

The way to run multiple Tide servers is by using several instances of async_std::task::spawn. I'm not too sure yet about whether concurrency adapters such as join and race need to be "Async Drop" aware as well. But it seems that at the very least Task::cancel should be possible to be invoked.

Implementation

The way to run "async drop" is by passing the future into an async context, and then dropping it. For example by passing it into a function with the following signature:

#[allow(dead_code)]
async fn async_drop<T>(x: T) {}

The way Task::cancel would work is when "cancel" is called, it would move the internal future into an async context, and then await that future. I'm unsure whether that requires an invocation of Box::pin(), but it's probably not unacceptable for cancellation to require an extra allocation.

Future developments

Another case I think is worth handling is:

let a = task::spawn(async { /* something */ });
let b = task::spawn(async { /* something */ });
a.race(b).await; // async destructors should run for the task that was dropped

There is not quite a precedent for this in thread::JoinHandle since synchronous code doesn't have suspension points, so intermediate cancellation isn't really a thing. But I think people would be surprised if async destructors didn't run in this setup, so I think it's at least worth considering if we can make this work.

@ghost
Copy link

ghost commented Jul 20, 2020

I think we need to wait for AsyncDrop to get implemented and stabilized, otherwise I don't see a way to implement this right now. But this is absolutely something I want to add to async-task as soon as we get AsyncDrop, no question about it!

@notgull
Copy link
Member

notgull commented Jul 4, 2022

Given that the linked RFC is dead, should this issue be closed? Async drop is an antipattern anyways IMO

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants