add `task::LocalSet` API for running `!Send` futures #1733

hawkw · 2019-11-05T19:29:38Z

Motivation

In earlier versions of tokio, the current_thread::Runtime type could
be used to run !Send futures. However, PR #1716 merged the
current-thread and threadpool runtimes into a single type, which can no
longer run !Send futures. There is still a need in some cases to
support futures that don't implement Send, and the tokio-compat
crate requires this in order to provide APIs that existed in tokio
0.1.

Solution

This branch implements the API described by @carllerche in
#1716 (comment). It
adds a new LocalSet type and spawn_local function to tokio::task.
The LocalSet type is used to group together a set of tasks which must
run on the same thread and don't implement Send. These are available
when a new "rt-util" feature flag is enabled.

Currently, the local task set is run by passing it a reference to a
Runtime and a future to block_on. In the future, we may also want
to investigate allowing spawned futures to construct their own local
task sets, which would be executed on the worker that the future is
executing on.

In order to implement the new API, I've made some internal changes to
the task module and Schedule trait to support scheduling both Send
and !Send futures.

hawkw · 2019-11-05T19:30:52Z

I would love any feedback on the API surface of the new local module — if there's anything we can do to make it more ergonomic to use, that would be great. In particular, I wasn't really sure if we would prefer the free function for spawning on the local task set to be used as tokio::spawn_local, or as tokio::local::spawn. I could go either way...

tokio/src/executor/task/mod.rs

tokio/src/local.rs

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

This adds a marker type to the `Task` struct and `Schedule` trait indicating whether or not the task is `Send`, and changes `Task` to only implement `Send` when the `Send` marker is present. Schedulers must indicate whether they are capable of scheduling `Send` or `!Send` tasks. This solution is singnificantly better than the previous one of using an unsafe wrapper to make `!Send` tasks implement `Send`, which was necessary due to the bounds on the `Task` struct. Although the previous solution was correct, since the `!Send` tasks were only ever scheduled locally, the compiler could no longer verify that this was the case. Now, the typesystem actually _helps_ us ensure that schedulers that can only schedule `Send` tasks will never schedule `!Send` tasks. Signed-off-by: Eliza Weisman <eliza@buoyant.io>

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

These assertions asserted that the scheduler was current in all calls to its methods. However, the `TaskSet::spawn_local` method allows scheduling tasks to be run on the `TaskSet` when it is _not_ currently running. Therefore, these assertions are wrong. This test moves the assertion to the _right_ place: when the scheduler actually _runs_ scheduled tasks. Signed-off-by: Eliza Weisman <eliza@buoyant.io>

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

hawkw · 2019-11-14T18:16:22Z

@carllerche BTW, the local task set now requires the "rt-full" feature flag. this is because it now requires the "sync" feature (due to the use of AtomicWaker), which I didn't think "rt-core" should require, and we didn't want to add a separate feature for local. I think that ideally, it would be possible to enable just the single-threaded runtime + LocalSet without also enabling the threadpool runtime, but I think that can wait until we give all the feature flags a pass.

Does that seem right to you?

@jonhoo

thanks @jonhoo! Signed-off-by: Eliza Weisman <eliza@buoyant.io>

tokio/tests/rt_common.rs

tokio/src/task/local.rs

carllerche · 2019-11-15T23:12:34Z

tokio/src/task/local.rs

+        }
+    }
+
+    fn with<F>(&self, f: impl FnOnce() -> F) -> F {


What happens when you nest with calls? Looks like bad things might happen? Either we want to allow it or disallow it via panic.

tokio/src/task/local.rs

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

Copied from `basic_scheduler` Signed-off-by: Eliza Weisman <eliza@buoyant.io>

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

tokio/src/task/mod.rs

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

hawkw added the C-enhancement Category: A PR with an enhancement or bugfix. label Nov 5, 2019

hawkw requested review from carllerche, jonhoo and taiki-e November 5, 2019 19:29

hawkw self-assigned this Nov 5, 2019

jonhoo reviewed Nov 6, 2019

View reviewed changes

tokio/src/executor/task/mod.rs Outdated Show resolved Hide resolved

jonhoo reviewed Nov 6, 2019

View reviewed changes

tokio/src/executor/task/mod.rs Outdated Show resolved Hide resolved

jonhoo reviewed Nov 6, 2019

View reviewed changes

tokio/src/local.rs Outdated Show resolved Hide resolved

jonhoo reviewed Nov 6, 2019

View reviewed changes

tokio/src/local.rs Outdated Show resolved Hide resolved

hawkw added 19 commits November 6, 2019 10:33

first pass on local task set

7eee2ec

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

it works!

366ecbd

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

change API to work with multiple block ons

82857dd

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

shutdown on drop

edd8ca4

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

add docs

c1e4e4a

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

re-export local mod from root, feature flag

968eea5

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

placate clippy

983b6b7

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

fix doctest

fb99e68

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

*actually* fix doctest

d6ff66e

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

fix missing pin-project without default features

17c7f7e

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

fix unused code when local is disabled

78d41b8

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

add send markers to debug output

057f415

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

fdsgkhsdfhjkgljkfdg;jklagdsf

f00f099

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

fix more feature-flaggy sadness

157e2e2

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

rename marker types

2a88b5f

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

make markers void structs

eef8727

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

remove duplicate file

1c0cea1

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

hawkw force-pushed the eliza/local-spawn branch from df978a8 to 1c0cea1 Compare November 6, 2019 18:45

hawkw added 2 commits November 14, 2019 10:12

put clippy attribute in the right place

7a9c84e

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

fix unused code based on feature flags

f1211e3

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

review feedback: rename ScheduleSend -> ScheduleSendOnly

6ff8c4f

thanks @jonhoo! Signed-off-by: Eliza Weisman <eliza@buoyant.io>

hawkw mentioned this pull request Nov 15, 2019

[feature] Constraining a future to a specific OS thread #1583

Closed

carllerche reviewed Nov 15, 2019

View reviewed changes

tokio/tests/rt_common.rs Show resolved Hide resolved

carllerche reviewed Nov 15, 2019

View reviewed changes

tokio/src/task/local.rs Show resolved Hide resolved

carllerche reviewed Nov 15, 2019

View reviewed changes

tokio/src/task/local.rs Outdated Show resolved Hide resolved

hawkw added 7 commits November 18, 2019 10:11

Merge master + update some things

b21cca7

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

Avoid starving the remote queue

17d520a

Copied from `basic_scheduler` Signed-off-by: Eliza Weisman <eliza@buoyant.io>

allow tasks to be cancelled remotely

cfa2808

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

update tests to use schedulers that actually do stuff

6f43e7e

fix accidental hang in test

f1c82c5

add test for sending a local future's JoinHandle

b919c95

fix yielding

fe93b45

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

hawkw mentioned this pull request Nov 22, 2019

compat: support 0.1 blocking #1722

Closed

asonix mentioned this pull request Nov 26, 2019

Please update futures-rs to 0.3.1 actix/actix#298

Closed

hawkw added 5 commits November 26, 2019 13:00

Merge branch 'master' into eliza/local-spawn

66f4a45

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

fix bad merge

40374f7

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

fix bad merge 2: the fixening

7260998

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

more cleanup

e40faca

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

rm empty file committed accidentally

9c805b6

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

carllerche reviewed Nov 26, 2019

View reviewed changes

tokio/src/task/mod.rs Outdated Show resolved Hide resolved

hawkw added 2 commits November 26, 2019 15:30

move to rt-util feature

04b0189

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

panic if nested

0528537

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

carllerche approved these changes Nov 27, 2019

View reviewed changes

fix AtomicWaker being enabled when not used

3418bfb

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

hawkw merged commit 38e602f into master Nov 27, 2019

carllerche deleted the eliza/local-spawn branch November 28, 2019 04:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add `task::LocalSet` API for running `!Send` futures #1733

add `task::LocalSet` API for running `!Send` futures #1733

hawkw commented Nov 5, 2019 •

edited

hawkw commented Nov 5, 2019

hawkw commented Nov 14, 2019

carllerche Nov 15, 2019

add task::LocalSet API for running !Send futures #1733

add task::LocalSet API for running !Send futures #1733

Conversation

hawkw commented Nov 5, 2019 • edited

Motivation

Solution

hawkw commented Nov 5, 2019

hawkw commented Nov 14, 2019

carllerche Nov 15, 2019

Choose a reason for hiding this comment

add `task::LocalSet` API for running `!Send` futures #1733

add `task::LocalSet` API for running `!Send` futures #1733

hawkw commented Nov 5, 2019 •

edited