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

WIP: implement cancellation and raw handles #2474

Merged
merged 1 commit into from Sep 9, 2020

Conversation

udoprog
Copy link
Contributor

@udoprog udoprog commented May 2, 2020

This is a WIP PR for implementing a simple form of cancellation, as mentioned here. Since work on structured concurrency is underway this might not be necessary, but I'm setting this up so that others can have a look and provide input.

While related to structured concurrency it does not replace it. It is only a low-level primitive necessary to perform some best-effort graceful shutdown (#1819) over spawned tasks.

Note that this does not allow for graceful termination of tasks spawned with spawn_blocking that are actively blocking, which includes most File operations.

Warning: Naming is provisional, and certain assumptions around task shutdown might not be correct since I'm still not super familiar with the task system.

Motivation

Support for some form of cancellation is necessary to support graceful termination . This doesn't replace structured concurrency, but should be considered more of an incremental step towards allowing application and library authors to support graceful termination of background tasks without adopting a different API.

Solution

Hooks into the existing task system, and extends completion polling with a variant which doesn't export the output value to support raw join handles. This allows us to cancel tasks (which are currently not being polled), and .await their termination. Hopefully providing their Output or a simple completion signal on a best-effort basis.

@Darksonn Darksonn added A-tokio Area: The main tokio crate C-enhancement Category: A PR with an enhancement or bugfix. M-task Module: tokio/task S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 2, 2020
Copy link
Member

@carllerche carllerche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, sorry for the delay. I am finally getting to this and it is definitely something that I think we should move forward with. The structured concurrency strategy has been put on hold indefinitely as we don't have a way forward with it.

I suggested splitting out the RawJoinHandle part. I would suggest starting a discussion issue for that.

As far as I can tell, the actual cancellation bit is not too involved as it can leverage shutdown(). I would probably recommend naming it something a bit more forceful like abort()?

What do you think?

/// }
/// }
/// ```
pub fn into_raw_handle(self) -> RawJoinHandle {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not necessarily against this, but could we decouple exposing RawJoinHandle as a public API from this PR? I would like to explore alternate solutions like providing a custom collection type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased and renamed to abort for now.

How do you want me to hide this from the public API?

Completely remove it, make private with #[allow(unused)], or #[doc(hidden)]?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely remove it in this PR. I would like to get abort landed. I don’t think a raw handle is the path forward. Lets discuss the use cases that motivated you that direction so we can figure out an option.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed, thanks!

@udoprog udoprog reopened this Aug 3, 2020
Copy link
Member

@carllerche carllerche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great 👍 thanks! Sorry for the delay.

@carllerche carllerche merged commit cbb14a7 into tokio-rs:master Sep 9, 2020
@udoprog udoprog deleted the cancel-raw-join-handles branch September 22, 2020 18:43
@najamelan
Copy link
Contributor

I have a question that I feel the documentation of this method should answer. Will the task be cancelled (read dropped) the next time it is polled? The next time the joinhandle get's polled? Does the joinhandle need to be awaited for it to be dropped or just the task polled?

Ok, sorry, that's 3 questions. You get the issue I think.

@udoprog
Copy link
Contributor Author

udoprog commented Sep 22, 2020

@najamelan The task is de-scheduled and dropped in the background, regardless if you poll it or not. The JoinHandle only gives you access to the output, but has to my understanding no bearing on the state of the task. Other than whether it can discard the output of the task, which it would if the JoinHandle is dropped.

@najamelan
Copy link
Contributor

Thanks for clarifying. What happens if the future is currently running? I know in futures (Abortable/RemoteHandle), the canceling happens in the poll method, so abort calls waker.wake() to reschedule it so it completes. Reading the docs in this PR I was kind of confused about the mechanics here.

@udoprog
Copy link
Contributor Author

udoprog commented Sep 22, 2020

Thanks for clarifying. What happens if the future is currently running? I know in futures (Abortable/RemoteHandle), the canceling happens in the poll method, so abort calls waker.wake() to reschedule it so it completes. Reading the docs in this PR I was kind of confused about the mechanics here.

Just for clarity: a task associated with a future is either being polled (i.e. running), or it's parked. If a task is cancelled in the middle of a poll the scheduler will notice this when the poll returns, and regardless of whether it returns Poll::Ready(..) or Poll::Pending finalize the task. Which involves dropping it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-enhancement Category: A PR with an enhancement or bugfix. M-task Module: tokio/task S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants