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.

Sign up for GitHub

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

Jump to bottom

document expectations for Waker::wake #93966

Merged
merged 3 commits into from
May 25, 2022
Merged

document expectations for Waker::wake #93966

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f52ebaa
document expectations for Waker::wake
rkuhn Feb 13, 2022
2946f7a
improve wording of Waker::wake documentation
rkuhn Feb 20, 2022
3d808d5
add caveat discussed in #74335
rkuhn May 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions library/core/src/task/wake.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,13 @@ unsafe impl Sync for Waker {}

impl Waker {
/// Wake up the task associated with this `Waker`.
///
/// Multiple wake-ups (through clones of this `Waker` or `wake_by_ref`) may be
/// coalesced into a single `poll` invocation by the runtime, and as long as
/// the runtime keeps running and the task is not finished it is expected that
/// each wake-up is followed by an invocation of `poll`, even in the absence of
/// other events. This makes it possible to yield to other tasks when running
/// potentially unbounded processing loops in order to maintain fairness.
Copy link
Member

Choose a reason for hiding this comment

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

I think this wording to me sounds like what you are getting at is that a call to wake() guarantees at least one call to poll (in the absence of Poll::Ready / runtime termination / panics / etc).

Maybe that's simpler than discussing coalescing etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding is that if I call cx.waker().wake_by_ref() twice in rapid succession, there will usually only be a single wake-up, hence “at least one call” is not correct. But with my proposed “is followed by” wording the coalescing can be left implicit if you find that more appropriate.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that makes sense. I think I had in mind that the interleaving is such that a wake() ensures at least one future call -- not necessarily each wake ensuring a distinct future call though.

Copy link
Member

Choose a reason for hiding this comment

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

"Guarantees there will be at least one call in the future" sounds correct to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review, I pushed an improvement that focuses more on the essence.

#[inline]
#[stable(feature = "futures_api", since = "1.36.0")]
pub fn wake(self) {
Expand Down