rust-lang / unsafe-code-guidelines

Forum for discussion about what unsafe code can and can't do
https://rust-lang.github.io/unsafe-code-guidelines
Apache License 2.0
657 stars 57 forks source link

Is a race between an unsynchronized read and write UB if the read value is always disregarded? #388

Open mikeleany opened 1 year ago

mikeleany commented 1 year ago

I apologize in advance if this or an equivalent question is considered elsewhere. I haven't been able to find it.

Consider the following code, which uses an atomic linked list to allocate memory frames. Note that deallocation isn't considered here for simplicity (which saves us from having to worry about the ABA problem).

use core::sync::atomic::{AtomicPtr, Ordering};
use core::ptr::NonNull;

#[repr(C, align(4096))]
struct Node {
    next: *mut Node,
}

#[repr(C, align(4096))]
pub struct Frame {
    data: [u8; 4096],
}

pub struct FrameAlloc {
    head: AtomicPtr<Node>,
}

impl FrameAlloc {
    pub fn alloc(&self) -> Option<NonNull<Frame>> {
        let ptr = self.head.fetch_update(Ordering::Acquire, Ordering::Acquire, |ptr| {
            if !ptr.is_null() {
                Some(unsafe { ptr.read().next })
            } else {
                None
            }
        }).ok()?;

        NonNull::new(ptr.cast::<Frame>())
    }
}

Now imagine that there are two concurrent calls to FrameAlloc::alloc as follows (with numbered lines indicating synchronization events):

  1. Thread 1 reads self.head and assigns ptr (in the closure) to point to some location X.
  2. Thread 2 reads self.head and also assigns its ptr to point to X.
    • Thread 2 reads from X (ptr.read().next) to get a pointer to some location Y.
  3. Thread 2 sees that self.head is unchanged and assigns it to point to Y.
    • Thread 2 returns a pointer to X (converted into an Option<NonNull<Frame>>).
    • Thread 2 writes to X.
    • Thread 1 reads from X (ptr.read().next), resulting in a race with the write by Thread 2.
  4. Thread 1 sees that self.head has changed and assigns ptr to point to Y. Note that the value read from X is now disregarded.

So the question is, does the race between steps 3 and 4, with Thread 2 writing to X and Thread 1 reading from X, result in undefined behavior, even though the value that was read is never used in circumstances where a race occurs.

Diggsey commented 1 year ago

I'm not sure enough about your question to answer (although I suspect it is unsound both by virtue of the fact that it does an unsynchronized read, and also because of the memory ordering on the update leading to unforeseen interleavings...)

However, there is another thing that might affect you - if you ever intend to free the memory returned from alloc, then this will race with other threads calling alloc (since the memory referred to by ptr may have been freed). Unless you are doing some kind of arena allocator (in which case you might want to attach some lifetimes) then there isn't a good way to synchronize the two.

RalfJung commented 1 year ago

So the question is, does the race between steps 3 and 4, with Thread 2 writing to X and Thread 1 reading from X, result in undefined behavior, even though the value that was read is never used in circumstances where a race occurs.

Yes. Following the C++ memory model, read-write data races in Rust are UB even if the read result is unused.