Check synchronization when using rtrb::Consumer::is_abandoned()

[mgeier]

Hi, rtrb maintainer here!

Since you are my largest user according to crates.io, I thought I'd let you know about a potential problem in the .is_abandoned() method: mgeier/rtrb#114

The method is used here:

minitrace-rust/minitrace/src/util/spsc.rs

Line 64 in 6c764fa

Err(_) if self.rx.is_abandoned() => Err(ChannelClosed),

I'm not sure about your requirements, but if you need synchronization between threads, this is broken since Rust 1.74 and you should use an acquire fence to restore the previous behavior.

I might fix the behavior in rtrb at some point, but this needs some refactoring, and this might take some time.

If you don't need synchronization, feel free to ignore this message.

[andylokandy]

Thank you! I think we don't thread synchronization here. The is_abandoned() will eventually return true after the Producer is dropped, right?

[mgeier]

Yes, it will return true eventually ... but ... if you access (read or write) some memory (after the call to is_abandoned()) that has been written to before the Producer was dropped (without any additional synchronization), you are invoking undefined behavior.

This is an illustration of the situation:

static mut V: u32 = 0;
let (p, c) = rtrb::RingBuffer::<u8>::new(7);
let t = std::thread::spawn(move || {
    unsafe { V = 10 };
    drop(p);
});
if c.is_abandoned() {
    std::sync::atomic::fence(std::sync::atomic::Ordering::Acquire);
    unsafe { V = 20 };
}
t.join().unwrap();

Without the fence, this has undefined behavior (but only since Rust 1.74).

AFAIU, this is only a problem if you have unsafe code that relies on the synchronization of is_abandoned() for soundness. In such a case, a fence can be used to restore the synchronization.

[andylokandy]

I see. We don't rely on such assumption on rtrb. Thank you for your advice and your amazing crate!