slide-rs / atom

Safe atomic pointers
Apache License 2.0
58 stars 7 forks source link

Unsound: `Atom` accepts an unsafe memory ordering #15

Open yvt opened 3 years ago

yvt commented 3 years ago

This crate has the same issue as https://github.com/mystor/atomic_ref/issues/5.

The following code segfaults on an AArch64 machine (but not on x86_64, which has a stronger memory model).

#![deny(unsafe_code)]
use atom::Atom;
use std::sync::atomic::Ordering;

fn main() {
    let channel = &*Box::leak(Box::new(Atom::<&&'static u32>::new(&&42u32)));

    std::thread::spawn(move || loop {
        if let Some(ptr) = channel.take(Ordering::Relaxed) {
            assert_eq!(**ptr, 42);
        }
    });

    for i in 0..10000000 {
        let b = Box::leak(Box::new(&42u32));
        channel.swap(b, Ordering::Relaxed);
    }
}
torkleyy commented 3 years ago

Thank you for the report, can you review #17 and tell me if it's fixed, please?

elidupree commented 2 years ago

I hope this can be fixed at some point. I want to use this crate, because Atom is exactly the synchronization primitive that I've wanted for several of my projects, and I appreciate the work that's gone into developing it. But I can't bring myself to use a dependency that has been "known to be unsound" for nearly a year without being updated...