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

Fix waker behavior when invoked before a poll finishes #54

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

jamesbornholt
Copy link
Member

It's possible for a task's waker to be invoked in the middle of a call
to that task's poll by the executor. We had accounted for that
possibility if the task called its own waker, but that's not good
enough: the waker can escape to other threads that can invoke it before
poll finishes (e.g., if the task blocks to acquire a lock).

This change fixes the waker behavior by clarifying the semantics of a
call to wake: a task whose waker is invoked should not be blocked
when it next returns Pending to the executor, and should be woken if
that has already happened. To do this, we introduce a new Sleeping
state for tasks, that has the same semantics as Blocked but that is
recognized by waker invocations, which will only unblock a task in
Sleeping state. This also removes the special case "woken by self"
behavior -- being woken by any thread should be enough to trigger this
sleep logic.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Member

@jorajeev jorajeev left a comment

Choose a reason for hiding this comment

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

This PR reminds me -- an interesting use of Shuttle could be to warn if your poll function could ever lead to blocking as this may be undesirable. Maybe we should think about adding an option to generate such a warning?

tests/asynch/waker.rs Show resolved Hide resolved
It's possible for a task's waker to be invoked in the middle of a call
to that task's `poll` by the executor. We had accounted for that
possibility if the task called *its own* waker, but that's not good
enough: the waker can escape to other threads that can invoke it before
`poll` finishes (e.g., if the task blocks to acquire a lock).

This change fixes the waker behavior by clarifying the semantics of a
call to `wake`: a task whose waker is invoked should not be blocked
*when it next returns Pending to the executor*, and should be woken if
that has already happened. To do this, we introduce a new `Sleeping`
state for tasks, that has the same semantics as `Blocked` but that is
recognized by waker invocations, which will only unblock a task in
`Sleeping` state. This also removes the special case "woken by self"
behavior -- being woken by *any* thread should be enough to trigger this
sleep logic.
@jamesbornholt
Copy link
Member Author

This PR reminds me -- an interesting use of Shuttle could be to warn if your poll function could ever lead to blocking as this may be undesirable.

I like this idea a whole bunch. Feels related to some of the recent clippy lints around holding a lock inside a future, e.g. rust-lang/rust-clippy#5439.

@jorajeev jorajeev merged commit b781b1e into awslabs:main Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants