From cb91ae53ca02dac5597b26cb3d22313aa268cbfc Mon Sep 17 00:00:00 2001 From: Bruno Dutra Date: Sun, 2 Jan 2022 20:28:42 +0100 Subject: [PATCH] fix rare and subtle bug before this change, it might have been possible, albeit very unlikely, that a receiver could report that the channel were disconnected even if there were messages in the buffer --- src/channel.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/channel.rs b/src/channel.rs index 2e0099e..cf7bc75 100644 --- a/src/channel.rs +++ b/src/channel.rs @@ -31,7 +31,7 @@ impl RingSender { /// * If the internal ring buffer is full, the oldest pending message is overwritten. /// * If the channel is disconnected, [`SendError::Disconnected`] is returned. pub fn send(&mut self, message: T) -> Result<(), SendError> { - if self.handle.receivers.load(Ordering::Relaxed) > 0 { + if self.handle.receivers.load(Ordering::Acquire) > 0 { self.handle.buffer.push(message); // A full memory barrier is necessary to prevent waitlist loads @@ -137,13 +137,15 @@ impl RingReceiver { /// * If the channel is disconnected and the internal ring buffer is empty, /// [`TryRecvError::Disconnected`] is returned. pub fn try_recv(&mut self) -> Result { - self.handle.buffer.pop().ok_or_else(|| { - if self.handle.senders.load(Ordering::Relaxed) > 0 { - TryRecvError::Empty - } else { - TryRecvError::Disconnected - } - }) + // We must check whether the channel is connected using acquire ordering before we look at + // the buffer, in order to ensure that the loads associated with popping from the buffer + // happen after the stores associated with a push into the buffer that may have happened + // immediately before the channel was disconnected. + if self.handle.senders.load(Ordering::Acquire) > 0 { + self.handle.buffer.pop().ok_or(TryRecvError::Empty) + } else { + self.handle.buffer.pop().ok_or(TryRecvError::Disconnected) + } } }