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

clarify the semantics of core::task::Waker::wake() #93961

Closed
rkuhn opened this issue Feb 13, 2022 · 5 comments · Fixed by #93966
Closed

clarify the semantics of core::task::Waker::wake() #93961

rkuhn opened this issue Feb 13, 2022 · 5 comments · Fixed by #93966

Comments

@rkuhn
Copy link
Contributor

rkuhn commented Feb 13, 2022

Based on reading e.g. the Tokio sources and knowledge passed down from async monks to novices, I assume that calling cx.waker().wake_by_ref() from the task that is currently being polled will ensure that the task is polled again soon, even if no external reason arises. However, this behaviour is not documented, at least not unambiguously — the documentation for core::future::Future only states that the waker provided by the context can be used to wake up the task, which may technically require the task to be “sleeping” for this to have an effect.

If there is consensus that wake_by_ref will always and unconditionally ensure one further call to poll, then we should document this expectation as part of the Future contract. Who agrees? Anyone against? Am I missing something?

@Thomasdezeeuw
Copy link
Contributor

If there is consensus that wake_by_ref will always and unconditionally ensure one further call to poll, then we should document this expectation as part of the Future contract.

I don't think you can guarantee that. A simple counter example would be shutting down the (async) runtime after a Future is awoken via Waker::wake or wake_by_ref. These kind of guarantees (or their lack of) is a property of the runtime, not Future or Waker.

@rkuhn
Copy link
Contributor Author

rkuhn commented Feb 13, 2022

The case of a vanishing runtime is outside the control of both the Future implementor and the runtime implementor, so I think we can carve that out of the discussion. The fact that futures can disappear at any moment does not make other guarantees worthless, like the one I’m after.

Saying that this is a runtime concern is also not going to work, at least not with the path Rust async has chosen. Cooperative scheduling can only work well if there is an agreed upon mechanism for temporarily yielding the CPU to other tasks, and with the Rust API of Future/Context/Waker there is no other option but to register oneself for later wake-up and then returning Poll::Pending.

@Thomasdezeeuw
Copy link
Contributor

If there is consensus that wake_by_ref will always and unconditionally ensure one further call to poll, then we should document this expectation as part of the Future contract.

The case of a vanishing runtime is outside the control of both the Future implementor and the runtime implementor, so I think we can carve that out of the discussion.

(my emphasis)

These two statements conflict. You can't just throw away part of the problem and still speak of always and unconditionally.

The fact that futures can disappear at any moment does not make other guarantees worthless, like the one I’m after.

Could you please restate the guarantee you'll looking for? Including (very reasonable) assumptions such as the runtime is still running.

Saying that this is a runtime concern is also not going to work, at least not with the path Rust async has chosen.

I'm not sure why this isn't going to work as a runtime concern? Could you elaborate?

Cooperative scheduling can only work well if there is an agreed upon mechanism for temporarily yielding the CPU to other tasks, and with the Rust API of Future/Context/Waker there is no other option but to register oneself for later wake-up and then returning Poll::Pending.

Isn't that the agreed upon mechanism already? Waker marks a Future as ready to be polled again. If the runtime (or waker implementation) decides it's not going to poll the Future any more their is little we can do. We could/should document a best-effort attempt to poll the Future again, but I don't think forcing guarantees here is the right path.

@rkuhn
Copy link
Contributor Author

rkuhn commented Feb 13, 2022

I think we’re in violent agreement here :-) And since you confirm the agreed upon mechanism, I’ll just open a PR where we can hammer out the precise wording.

@tmandry
Copy link
Member

tmandry commented Mar 29, 2022

Related to #74335

JohnTitor added a commit to JohnTitor/rust that referenced this issue May 24, 2022
document expectations for Waker::wake

fixes rust-lang#93961

Opened PR for a discussion on the precise wording.
@bors bors closed this as completed in f52ebaa May 25, 2022
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 a pull request may close this issue.

3 participants