-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
oneshot Channel
#143741
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
oneshot Channel
#143741
Conversation
|
Maybe want to ask the unresolved questions at the ACP thread? To make sure it's seen by those already involved. (I'll take a closer look later) |
Because But mpmc is multiple producer multiple consumer, so it doesn't matter how many threads For this one, they can both be unconditionally |
|
Thank you for the answer! Please let me know if I'm interpreting this right: If all methods where synchronization must occur consume |
c7e668b to
1a515fd
Compare
|
I haven't gotten a chance to look into this unfortunately r? libs |
|
r? libs (based on this comment) |
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #147928) made this pull request unmergeable. Please resolve the merge conflicts. |
|
It's been a little while so Feel free to reroll if you want but you've reviewed a lot of sync module things. |
This comment has been minimized.
This comment has been minimized.
e32a0d2 to
c9cf24e
Compare
|
For reference I just changed the doctest to this: #![feature(oneshot_channel)]
use std::sync::oneshot::{self, RecvTimeoutError};
use std::thread;
use std::time::Duration;
let (sender, receiver) = oneshot::channel();
let send_failure = thread::spawn(move || {
// Simulate a long computation that takes longer than our timeout.
thread::sleep(Duration::from_millis(250));
// This will likely fail to send because we drop the receiver in the main thread.
sender.send("Goodbye!".to_string()).unwrap();
});
// Try to receive the message with a short timeout.
match receiver.recv_timeout(Duration::from_millis(10)) {
Ok(msg) => println!("Received: {}", msg),
Err(RecvTimeoutError::Timeout(rx)) => {
println!("Timed out waiting for message!");
// Note that you can reuse the receiver without dropping it.
drop(rx);
},
Err(RecvTimeoutError::Disconnected) => println!("Sender dropped!"),
}
send_failure.join().unwrap_err();@rustbot ready |
library/std/src/sync/oneshot.rs
Outdated
| #[unstable(feature = "oneshot_channel", issue = "143674")] | ||
| impl<T> fmt::Debug for TryRecvError<T> { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| "oneshot::TryRecvError(..)".fmt(f) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somethings gone wrong here, this issue is back...
|
argh I accidentally dropped the commit instead of squashing it... Thank you for catching that |
The `oneshot` channel is gated under the `oneshot_channel` feature. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Tests inspired by tests in the `oneshot` third-party crate.
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@rustbot ready |
|
Great, thank you! |
|
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 9e8a0c5 (parent) -> b7bcaa5 (this PR) Test differencesShow 67 test diffsStage 1
Stage 2
Additionally, 29 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard b7bcaa5c715ed07af2d74dac7ddb8786abeb4299 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (b7bcaa5): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 2.4%, secondary 2.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.1%, secondary -2.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.0%, secondary -0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 474.474s -> 474.563s (0.02%) |
Tracking Issue: #143674
This PR adds an experimental
oneshotmodule.Before talking about the API itself, I would prefer to get some of these questions below out of the way first. And as discussed in the ACP it would be
Unresolved Questions
Why exactly is it okay forSenderto beSync? Or basically, how do we boil down the discussion in ImplementSyncformpsc::Sender#111087 into a comment for theunsafe impl<T: Send> Sync for Sender<T> {}?Why ismpsc::Receiver!Syncbutmpmc::ReceiverisSync? Shouldoneshot::ReceiverbeSyncor not?is_readymethod as proposed in the tracking issue? If so, then the surface of this PR would likely need to increase to add apub(crate) fn is_disconnectedmethod tompmc(might even be a good idea to add that to all 3 channel flavors).mpmc, or should it be a wrapper around the internal crossbeam implementation?SenderandReceiveroperations be methods or associated methods? Sosender.send(msg)orSender::send(sender, msg)? The method syntax is more consistent with the rest of the ecosystem (namelytokio)