WIP: task::scope #2576

Matthias247 · 2020-05-31T02:21:07Z

This change adds task::scope as a mechanism for supporting
structured concurrency as described in #1879.

In this version of the change scopes can be created using a freestanding
task::scope function, which creates a new detached scope (which is not
coupled to the parent scope), Scope::detached and Scope::with_parent,
and a ::child() method on an existing scope handle.

Since a few of of those methods are doing the same thing, some of those
might get dropped before the change is merged. For future extensibility
also a Scope::with_config and ScopeConfigBuilder are demonstrated in
this change. Those might also be part of the initial change.

This change adds task::scope as a mechanism for supporting structured concurrency as described in tokio-rs#1879. In this version of the change scopes can be created using a freestanding `task::scope` function, which creates a new detached scope (which is not coupled to the parent scope), `Scope::detached` and `Scope::with_parent`, and a `::child()` method on an existing scope handle. Since a few of of those methods are doing the same thing, some of those might get dropped before the change is merged. For future extensibility also a `Scope::with_config` and `ScopeConfigBuilder` are demonstrated in this change. Those might also be part of the initial change.

Matthias247 · 2020-05-31T02:21:25Z

This replaces #2153

Matthias247 · 2020-05-31T02:25:29Z

tokio/src/task/scope.rs

+        }
+
+        match self.drop_behavior {
+            ScopeDropBehavior::BlockToCompletion => {


We will probably want to leave out this version in the inital implementation since it raises a few ugly qeustions - eg around what to do in a singlethreaded runtime. I just left it here so far for demo purposes.

Agreed, and even in multi-threaded, it could be unexpected to have the threads see some hangs too.

seanmonstar · 2020-06-09T21:25:49Z

tokio/src/sync/wait_group.rs

+///
+/// Tasks can wait for tracked tasks to finish by obtaining a Future via `wait`.
+/// This Future will get fulfilled when no tasks are running anymore.
+pub(crate) struct WaitGroup {


I haven't reviewed this file, but I think the concept of a WaitGroup is probably worth being a separate PR that we can get in faster! Do you think this one is useful beyond the internal usage?

It could definitely be exposed on its own and is helpful to have. It only didn't happen in this change since it would require other discussions on how the public API of WaitGroup should look like (would it be what WaitGroup or SharedWaitGroup is here?)

seanmonstar · 2020-06-09T22:03:01Z

tokio/src/task/scope.rs

+        }
+
+        match self.drop_behavior {
+            ScopeDropBehavior::BlockToCompletion => {


Agreed, and even in multi-threaded, it could be unexpected to have the threads see some hangs too.

seanmonstar · 2020-06-09T22:04:19Z

tokio/src/task/scope.rs

+    Panic,
+    /// When a scope is dropped while tasks are outstanding, the process will be
+    /// aborted.
+    Abort,


Is there value in having the option vs someone just setting panic=abort in their Cargo.toml?

Not too much. Maybe if someone mainly cares about not misusing scopes. It's also a relict from the idea if to be able to borrow from the parent task inside child tasks. In order to guarantee that property a panic! wouldn't be strong enough.

That said those APIs are probably all not needed for a v1, and a reasonable default could be picked. The options are mostly presented tho show how a scope could be configured in case configurability is desired.

tokio/tests/task_scope.rs

seanmonstar · 2020-06-09T22:07:29Z

tokio/src/task/scope.rs

+    F: FnOnce(ScopeHandle) -> Fut,
+    Fut: Future<Output = R> + Send,
+{
+    Scope::detached().enter(scope_func).await


In your other PR about "implicit scopes", this would attach to the parent, but in this PR, this is supposed to be a global/detached scope, yes?

The "implicit scopes" change would also get a method where you can spawn a task which is not directly attached to the parent to opt-out of structured concurrency if you want to. Currently both options would then spawn a "global" task, but I think in the end both variants should then spawn the task into the "runtime scope", which is the top level scope. Thereby shutting down the runtime would automatically cancel the remaining tasks and wait for them to complete. This however requires a change to the runtime structs to include this top level scope, which is not included yet.

seanmonstar · 2020-06-09T22:10:02Z

tokio/src/task/scope.rs

+
+        let mut drop_behavior = match &self.parent {
+            Some(parent) => parent.scope.config.drop_behavior,
+            None => ScopeDropBehavior::Panic,


Should it really panic by default if you drop a scope without polling it to the end? I can imagine plenty of Rust patterns where you might start a task/scope at the beginning, and then try some other fallible method, and only afterwards scope.await. If you were using ?, it'd be pretty easy to drop the scope, and probably most would expect that the scope would just be canceled.

As written in a few other places: It's for obvious reasons not a great strategy. But there exists no perfect strategy for this problem, and therefore this seems to be the best compromise.

letting the background task continue to run (as proposed in RFC: structured concurrency #2596 ) will break structured concurrency and will not allow users to reason anymore about when subtasks (and their usage for certain resources) end

doing a blocking wait will not work in single-threaded environments, and might lead to high latency variances in multithreaded environments.

seanmonstar · 2020-06-09T22:28:51Z

In general, I think structured concurrency will be a huge help. I have actual experiences in the Linkerd proxy where we would accidentally leak tasks, and we really should be making it harder to do that. I want us to explore this space more!

At the same time, I think there's some questions around default behavior that we need answers to, and I'm not sure we can get them without trying scopes out in an actual application. I think the questions about implicit vs explicit (my guess is a mix), how to trigger cancels (explicit calls vs drop), whether scopes dropping should panic by default, and graceful vs forceful shutdown really need Rust experience. This is because while we have a lot of research in other languages, they also don't perfectly match Rust. We have Drop, and await, and Result vs panic!, which I think are all sufficiently different.

We also are trying to polish up a Tokio 1.0 this year. We only have a few months left. And we can't be changing the default behaviors once they've shipped. This doesn't dampen my desire to explore this, it just colors how I think we actually can.

What if we take this and publish tokio-scope as a separate library (or just scopes, periscope is a fun name, whatever). All the concepts can be implemented as a library to start. They can gain real world experience in an application. And then, we can add tokio::task::scope directly in a minor version of Tokio (maybe a headline feature of Tokio 1.1). Doing so makes it even easier to reach for, we can make some optimizations since we can reach into the Task object, and the main Tokio docs can start pointing it out. What do you think of this plan?

carllerche · 2020-07-24T05:13:23Z

I'm going to close this now as there is no way forward. More details here #2596 (comment)

Thanks again for all the work you did on this. Even though we didn't manage to get this through, I personally learned a lot on the topic and from your research. Also, once again, I'm sorry that didn't put more time into this sooner.

Matthias247 mentioned this pull request May 31, 2020

WIP: Add task::scope #2153

Closed

Matthias247 commented May 31, 2020

View reviewed changes

Matthias247 requested a review from carllerche May 31, 2020 02:25

Matthias247 mentioned this pull request Jun 2, 2020

WIP: task::scope using implicit scopes #2579

Closed

Darksonn added A-tokio Area: The main tokio crate C-enhancement Category: A PR with an enhancement or bugfix. M-task Module: tokio/task labels Jun 2, 2020

This was referenced Jun 5, 2020

RFC: structured concurrency via task::scope #2592

Closed

RFC: structured concurrency #2596

Closed

seanmonstar reviewed Jun 9, 2020

View reviewed changes

carllerche closed this Jul 24, 2020

alecmocatta mentioned this pull request Aug 18, 2020

Remove the 'static bound from everywhere constellation-rs/amadeus#100

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: task::scope #2576

WIP: task::scope #2576

Matthias247 commented May 31, 2020

Matthias247 commented May 31, 2020

Matthias247 May 31, 2020

seanmonstar Jun 9, 2020

seanmonstar Jun 9, 2020

Matthias247 Jun 9, 2020

seanmonstar Jun 9, 2020

seanmonstar Jun 9, 2020

Matthias247 Jun 9, 2020

seanmonstar Jun 9, 2020

Matthias247 Jun 9, 2020

seanmonstar Jun 9, 2020

Matthias247 Jun 9, 2020

seanmonstar commented Jun 9, 2020

carllerche commented Jul 24, 2020

WIP: task::scope #2576

WIP: task::scope #2576

Conversation

Matthias247 commented May 31, 2020

Matthias247 commented May 31, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanmonstar commented Jun 9, 2020

carllerche commented Jul 24, 2020