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

io: clarify clear_ready and clear_ready_matching documentation #6304

Merged
merged 5 commits into from
Jan 27, 2024
Merged

io: clarify clear_ready and clear_ready_matching documentation #6304

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a0a30aa
io: clarify documentation
maminrayej Jan 24, 2024
c80bf63
restore the original paragraph
maminrayej Jan 25, 2024
a8de03b
place the explanation at the end
maminrayej Jan 25, 2024
0fabcc5
reword the clarification
maminrayej Jan 26, 2024
c29f50a
remove leftover comment
maminrayej Jan 27, 2024
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
14 changes: 14 additions & 0 deletions tokio/src/io/async_fd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,9 @@ impl<'a, Inner: AsRawFd> AsyncFdReadyGuard<'a, Inner> {
/// _actually observes_ that the file descriptor is _not_ ready. Do not call
/// it simply because, for example, a read succeeded; it should be called
/// when a read is observed to block.
///
/// Note that that if a readiness notification occurs following the last operation
/// but prior to invoking `clear_ready`, it will not be cleared.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's really important to be very clear in the wording here, because the point we are making is tricky to understand.

Here's one suggested wording. This one tries to explain it twice with "in other words". I still don't love it though.

Suggested change
/// Note that that if a readiness notification occurs following the last operation
/// but prior to invoking `clear_ready`, it will not be cleared.
/// This method only clears readiness events that happened before the creation of this guard. In other words, if the IO resource becomes ready between the creation of the guard and this call to `clear_ready`, then the readiness is not actually cleared.

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually like that the timing is tied to the creation of the guard. I believe it makes it clear.

pub fn clear_ready(&mut self) {
if let Some(event) = self.event.take() {
self.async_fd.registration.clear_readiness(event);
Expand All @@ -835,6 +838,8 @@ impl<'a, Inner: AsRawFd> AsyncFdReadyGuard<'a, Inner> {
/// Indicates to tokio that the file descriptor no longer has a specific readiness.
/// The internal readiness flag will be cleared, and tokio will wait for the
/// next edge-triggered readiness notification from the OS.
/// This implies that if a readiness notification occurs following the last operation
/// but prior to invoking `clear_ready`, it will not be cleared.
///
Copy link
Contributor

Choose a reason for hiding this comment

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

This part isn't on all of them? But I think the other change is probably sufficient.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes it was a leftover comment. Thanks.

/// This function is useful in combination with the [`AsyncFd::ready`] method when a
/// combined interest like `Interest::READABLE | Interest::WRITABLE` is used.
Expand All @@ -846,6 +851,9 @@ impl<'a, Inner: AsRawFd> AsyncFdReadyGuard<'a, Inner> {
/// block. For example when a read blocks when using a combined interest,
/// only clear `Ready::READABLE`.
///
/// Note that that if a readiness notification occurs following the last operation
/// but prior to invoking `clear_ready`, it will not be cleared.
///
/// # Examples
///
/// Concurrently read and write to a [`std::net::TcpStream`] on the same task without
Expand Down Expand Up @@ -1042,6 +1050,9 @@ impl<'a, Inner: AsRawFd> AsyncFdReadyMutGuard<'a, Inner> {
/// _actually observes_ that the file descriptor is _not_ ready. Do not call
/// it simply because, for example, a read succeeded; it should be called
/// when a read is observed to block.
///
/// Note that that if a readiness notification occurs following the last operation
/// but prior to invoking `clear_ready`, it will not be cleared.
pub fn clear_ready(&mut self) {
if let Some(event) = self.event.take() {
self.async_fd.registration.clear_readiness(event);
Expand All @@ -1062,6 +1073,9 @@ impl<'a, Inner: AsRawFd> AsyncFdReadyMutGuard<'a, Inner> {
/// block. For example when a read blocks when using a combined interest,
/// only clear `Ready::READABLE`.
///
/// Note that that if a readiness notification occurs following the last operation
/// but prior to invoking `clear_ready`, it will not be cleared.
///
/// # Examples
///
/// Concurrently read and write to a [`std::net::TcpStream`] on the same task without
Expand Down
Loading