Skip to content

Enhance documentation on wake call memory ordering#154401

Open
xmh0511 wants to merge 5 commits intorust-lang:mainfrom
xmh0511:main
Open

Enhance documentation on wake call memory ordering#154401
xmh0511 wants to merge 5 commits intorust-lang:mainfrom
xmh0511:main

Conversation

@xmh0511
Copy link
Copy Markdown
Contributor

@xmh0511 xmh0511 commented Mar 26, 2026

View all comments

Add documentation about memory ordering requirements for wake calls. Try to fix #128920

Add documentation about memory ordering requirements for wake calls.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 26, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 26, 2026

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: libs
  • libs expanded to 7 candidates

@rust-log-analyzer

This comment has been minimized.

Comment thread library/alloc/src/task.rs Outdated
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 27, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 27, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@Darksonn
Copy link
Copy Markdown
Member

In general I would say that this documentation makes more sense on the Waker struct or the Future trait. The Wake trait is just a utility trait, and should not be the primary location for these kinds of docs (though it can be mentioned there too).

I would also relax the wording a bit. You use the word 'runtime must' and so on, but I think it would be useful to instead phrase it along the lines of 'to avoid missed wakeups, the runtime must' or similar to make it clear what the consequences of getting it wrong are.

xmh0511 added 3 commits March 31, 2026 10:05
Clarify wake behavior in documentation regarding missed wakeups.
Clarify wakeup requirements for the executor in documentation.
@rust-log-analyzer

This comment has been minimized.

Copy link
Copy Markdown
Member

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Overall looks good to me.

View changes since this review

Comment thread library/alloc/src/task.rs Outdated
@rust-log-analyzer
Copy link
Copy Markdown
Collaborator

The job tidy failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Diff in /checkout/library/core/src/task/wake.rs:432:
     /// executor’s choice which task to run and the executor may choose to run the
     /// current task again.
     ///
-    /// To avoid missed wakeups, the runtime must ensure that for any call to `wake`, 
+    /// To avoid missed wakeups, the runtime must ensure that for any call to `wake`,
     /// there is a subsequent call to `poll` such that the returned `wake` _happens-before_
-    /// the beginning of the invocation of `poll`. 
+    /// the beginning of the invocation of `poll`.
     /// In particular, this means that if a task self-wakes (invokes `wake` on itself during `poll`),
-    /// then `poll` must be invoked again because the call to `wake` _happens-after_ the beginning 
-    /// of the current invocation of `poll`. 
-    /// 
+    /// then `poll` must be invoked again because the call to `wake` _happens-after_ the beginning
+    /// of the current invocation of `poll`.
+    ///
     /// [`poll()`]: crate::future::Future::poll
     #[inline]
     #[stable(feature = "futures_api", since = "1.36.0")]
fmt: checked 6765 files
Bootstrap failed while executing `test src/tools/tidy tidyselftest --extra-checks=py,cpp,js,spellcheck`
Build completed unsuccessfully in 0:00:51

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Happens-before relationship between wake and poll

5 participants