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

sync: oneshot::Receiver::is_empty() #7153

Merged
merged 4 commits into from
Feb 16, 2025

Conversation

cratelyn
Copy link
Contributor

@cratelyn cratelyn commented Feb 13, 2025

this commit introduces a new method to tokio::sync::oneshot::Receiver<T>.

this method returns true if the channel has no messages waiting to be received. this is similar to the existing tokio::sync::mpsc::Receiver::is_empty() and tokio::sync::broadcast::Receiver::is_empty() methods.

see:

@github-actions github-actions bot added the R-loom-sync Run loom sync tests on this PR label Feb 13, 2025
@cratelyn cratelyn force-pushed the sync-oneshot.is_empty branch from 82b79f3 to 191c3ed Compare February 13, 2025 20:48
@cratelyn cratelyn marked this pull request as ready for review February 13, 2025 21:01
Copy link
Member

@maminrayej maminrayej left a comment

Choose a reason for hiding this comment

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

If I understand correctly, it's not safe to poll the receiver even if is_empty returns true, since you can consume the message with try_recv and incorrectly try to poll the receiver later on. I wonder whether it's worth it to document this fact on the is_empty method and redirect users to is_terminated implemented in #7152.

cratelyn added a commit to cratelyn/tokio that referenced this pull request Feb 15, 2025
> You could also add a test for `is_empty` before and after `try_recv`
> as well.

tokio-rs#7153 (comment)

Co-authored-by: M.Amin Rayej <[email protected]>
Signed-off-by: katelyn martin <[email protected]>
cratelyn added a commit to cratelyn/tokio that referenced this pull request Feb 15, 2025
NB: this commit means that this branch (tokio-rs#7153) is dependent upon
interfaces proposed in (tokio-rs#7152).

pr tokio-rs#7152 proposes a `is_terminated()` method for the oneshot channel's
receiver. that is the proper method to use to check whether or not it is
safe to poll a oneshot receiver as a future.

this commit adds a note cross-referencing this method to the
documentation of `is_empty()`, to help mitigate misuse. this method only
reports whether a value is currently waiting in the channel; a channel
that has already received and yielded a value to its receiver is also
empty, but would panic when polled.

Signed-off-by: katelyn martin <[email protected]>
@cratelyn cratelyn force-pushed the sync-oneshot.is_empty branch from b285698 to 3dd36d3 Compare February 15, 2025 20:06
cratelyn added a commit to cratelyn/tokio that referenced this pull request Feb 15, 2025
NB: this commit means that this branch (tokio-rs#7153) is dependent upon
interfaces proposed in (tokio-rs#7152).

pr tokio-rs#7152 proposes a `is_terminated()` method for the oneshot channel's
receiver. that is the proper method to use to check whether or not it is
safe to poll a oneshot receiver as a future.

this commit adds a note cross-referencing this method to the
documentation of `is_empty()`, to help mitigate misuse. this method only
reports whether a value is currently waiting in the channel; a channel
that has already received and yielded a value to its receiver is also
empty, but would panic when polled.

Signed-off-by: katelyn martin <[email protected]>
@cratelyn cratelyn force-pushed the sync-oneshot.is_empty branch from 3dd36d3 to b7d1b3e Compare February 15, 2025 20:15
@cratelyn
Copy link
Contributor Author

If I understand correctly, it's not safe to poll the receiver even if is_empty returns true, since you can consume the message with try_recv and incorrectly try to poll the receiver later on. I wonder whether it's worth it to document this fact on the is_empty method and redirect users to is_terminated implemented in #7152.

that's right, and i agree that this would be a good signpost to provide in order to mitigate misuse and clarify purpose.

note that this branch is now based upon #7152, to reflect that the documentation of is_empty() now references an interface proposed in that pull request.

b7d1b3e introduces additional documentation and accompanying examples to help draw attention to what exactly "empty" means, semantically.

@cratelyn

This comment was marked as outdated.

cratelyn and others added 4 commits February 16, 2025 14:37
this commit introduces a new method to
`tokio::sync::oneshot::Receiver<T>`.

this method returns true if the channel has no messages waiting to be
received. this is similar to the existing
`tokio::sync::mpsc::Receiver::is_empty()` and
`tokio::sync::broadcast::Receiver::is_empty()` methods.

see:
* https://docs.rs/tokio/latest/tokio/sync/broadcast/struct.Receiver.html#method.is_empty
* https://docs.rs/tokio/latest/tokio/sync/mpsc/struct.Receiver.html#method.is_empty
* tokio-rs#7137 (comment)

Signed-off-by: katelyn martin <[email protected]>
> You could also add a test for `is_empty` before and after `try_recv`
> as well.

tokio-rs#7153 (comment)

Co-authored-by: M.Amin Rayej <[email protected]>
Signed-off-by: katelyn martin <[email protected]>
this commit uses a `let else` binding to early return when the channel
has already terminated.

it then, additionally, removes a redundant `else if` branch.

Co-authored-by: M.Amin Rayej <[email protected]>
Signed-off-by: katelyn martin <[email protected]>
NB: this commit means that this branch (tokio-rs#7153) is dependent upon
interfaces proposed in (tokio-rs#7152).

pr tokio-rs#7152 proposes a `is_terminated()` method for the oneshot channel's
receiver. that is the proper method to use to check whether or not it is
safe to poll a oneshot receiver as a future.

this commit adds a note cross-referencing this method to the
documentation of `is_empty()`, to help mitigate misuse. this method only
reports whether a value is currently waiting in the channel; a channel
that has already received and yielded a value to its receiver is also
empty, but would panic when polled.

Signed-off-by: katelyn martin <[email protected]>
@cratelyn cratelyn force-pushed the sync-oneshot.is_empty branch from 85323ff to a59f884 Compare February 16, 2025 19:38
@cratelyn
Copy link
Contributor Author

this branch has been rebased, now that the prerequisite #7152 has been merged.

@maminrayej
Copy link
Member

Thanks.

@maminrayej maminrayej merged commit 383da87 into tokio-rs:master Feb 16, 2025
82 checks passed
@maminrayej maminrayej added A-tokio Area: The main tokio crate M-sync Module: tokio/sync labels Feb 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-sync Module: tokio/sync R-loom-sync Run loom sync tests on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants