sync: Added WeakSender to sync::broadcast::channel
#7100
Conversation
| pub fn is_closed(&self) -> bool { | ||
| // Channel is closed when there are no strong senders left active | ||
| self.shared.num_tx.load(Acquire) == 0 |
There was a problem hiding this comment.
Man, the orderings in this file are a mess. Mixing SeqCst and non-SeqCst orderings on the same atomic is not a good idea. The correct orderings for counters is:
- Increments happen with relaxed.
- Decrements happen with acqrel.
- Checks for zero happen with acquire.
We shouldn't need SeqCst for num_tx ever.
There was a problem hiding this comment.
I'm sorry for that. There orderings and atomic operations are new for me so I copied it from sync/mpsc and tried to understand why the specific orderings where used there.
Thank you for some explanation on what should be used instead! I will change the code accordingly.
There was a problem hiding this comment.
Re-reading your comment I noticed that you said that we don't need SeqCst for num_tx ever. That was not part of my PR but should I adjust it now here in the PR too?
Edit.: I adjusted the orderings for num_tx too.
There was a problem hiding this comment.
Yeah, it's not your fault. This problem is pre-existing.
* Changed memory orderings of `num_weak_tx`: Increments happen with relaxed, decrements happen with acqrel and checks for zero happen with acquire. * Add asserts for `sync::broadcast::WeakSender` to `tests/async_send_sync.rs`
| //redirect.github.com/ Tries to convert a `WeakSender` into a [`Sender`]. This will return `Some` | ||
| //redirect.github.com/ if there are other `Sender` instances alive and the channel wasn't | ||
| //redirect.github.com/ previously dropped, otherwise `None` is returned. | ||
| pub fn upgrade(&self) -> Option<Sender<T>> { |
There was a problem hiding this comment.
This should probably be #[must_use] too.
| //redirect.github.com/ Tries to convert a `WeakSender` into a [`Sender`]. This will return `Some` | ||
| //redirect.github.com/ if there are other `Sender` instances alive and the channel wasn't | ||
| //redirect.github.com/ previously dropped, otherwise `None` is returned. |
There was a problem hiding this comment.
Markdown docs generally start with one really short summary, followed by more text.
| //redirect.github.com/ Tries to convert a `WeakSender` into a [`Sender`]. This will return `Some` | |
| //redirect.github.com/ if there are other `Sender` instances alive and the channel wasn't | |
| //redirect.github.com/ previously dropped, otherwise `None` is returned. | |
| //redirect.github.com/ Tries to convert a `WeakSender` into a [`Sender`]. | |
| //redirect.github.com/ | |
| //redirect.github.com/ This will return `Some` | |
| //redirect.github.com/ if there are other `Sender` instances alive and the channel wasn't | |
| //redirect.github.com/ previously dropped, otherwise `None` is returned. |
+ reflow to line length
Motivation
Closing issue #7003. Add a type
WeakSenderto thesync::broadcast::channelsimilar tosync::maps::channel.Solution
The new
WeakSendertype just stores anArc<Shared<T>>just like the normalSenderbut activeWeakSenderswill not prevent the channel from being closed if allSenders are dropped.Closes #7003.