-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
tokio: wake localset on spawn_local #3369
tokio: wake localset on spawn_local #3369
Conversation
Wake the `tokio::task::LocalSet` after `tokio::task::LocalSet::spawn_local` is called. Note this side effect in the `tokio::task::LocalSet::spawn_local` docs. Add test for spawn_local being woke in `task_local_set.rs` Fixes: tokio-rs#3117
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
tokio/src/task/local.rs
Outdated
/// Calls to this function will result in waking the current localset. | ||
/// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is necessary to have in the doc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to remove it if you don't think it is necessary. I added it because a side effect that changes program behavior is worth noting imo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is necessary. The "wake" is an implementation detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides, notes about changes in behavior go in the changelog instead. (You don't have to put it in this PR, we do them together when making the release.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense - Thanks for the review! Pushed the suggested changes.
Wake the
tokio::task::LocalSet
aftertokio::task::LocalSet::spawn_local
is called.Note this side effect in the
tokio::task::LocalSet::spawn_local
docs.Add test for spawn_local being woke in
task_local_set.rs
Fixes: #3117
Motivation
Fixes #3117
Solution
Add call to shared waker in localset after adding future to localset task queue.