-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
2934b19 to
90740d3
Compare
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)
Why exactly is it okay for Sender to be Sync? Or basically, how do we boil down the discussion in #111087 into a comment for the unsafe impl<T: Send> Sync for Sender {}?
Why is mpsc::Receiver !Sync but mpmc::Receiver is Sync? Should oneshot::Receiver be Sync or not?
Because mpsc is multiple producer single consumer, and does the receiver operation via &self, so it would be incorrect to allow recv to be shared across threads because that would allow two threads to recv at the same time, which isn't allowed for a mpsc.
But mpmc is multiple producer multiple consumer, so it doesn't matter how many threads recv at the same time.
For this one, they can both be unconditionally Sync, since there is no API to go from &Sender<T> or &Reciever<T> to &T. (even via traits like Debug or Clone). In fact it looks like &Sender<T> and &Receiver<T> are entirely useless (only a Debug impl, which doesn't provide any information), which is just like Exclusive.. This is more evidence that both Sender<T> and Receiver<T> can be unconditionally Sync
Thank you for the answer!
Please let me know if I'm interpreting this right: If all methods where synchronization must occur consume self, we can say that both Sender and Receiver are Sync. For example, if threads share a &oneshot::Receiver, then none of them can call recv anyways. And this would still be compatible with the proposed is_ready method discussed in the tracking issue.
91df320 to
1d9d8fd
Compare
c7e668b to
1a515fd
Compare
I haven't gotten a chance to look into this unfortunately
r? libs
1a515fd to
4791be9
Compare
r? libs (based on this comment)
The `oneshot` channel is gated under the `oneshot_channel` feature.
Tests inspired by tests in the `oneshot` third-party crate.
4791be9 to
8020b5a
Compare
This PR was rebased onto a different master 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.
Uh oh!
There was an error while loading. Please reload this page.
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)