Unsafe interior mutability with AtomicPtr

[josh1]

Hi,
I've recently come across a memory access requirement for one of my pet projects:

1. Concurrent field read and write access across multiple threads.
2. No Mutex / RwLock
3. Writes don't need to be reflected in current readers holding a reference to that field.
4. No need to worry about concurrent writes (i.e. read update interleaving)
5. No external mutability ('&mut')

So is basically a GC'ed like semantics - you get a reference to that field if someone replaces it with some new instance you will still be holding the 'old' value, and that's fine.

Since this is my first time using unsafe and pointers I would like to know if I'm missing something or perhaps doing something that's unnecessary ? mainly regarding correctness and not speed.
Is Box necessary here?

struct Container<T> {
    item: AtomicPtr<Rc<T>>,
}

impl<T> Container<T> {
    pub fn new(item: T) -> Self {
        let r = Self::box_leak(item);
        Self {
            item: AtomicPtr::new(r),
        }
    }

    fn box_leak<'a>(item: T) -> &'a mut Rc<T> {
        Box::leak(Box::new(Rc::new(item)))
    }

    pub fn replace(&self, new: T) {
        let r = Self::box_leak(new);
        let old = self.item.swap(r, Ordering::SeqCst);
        let oldbox = unsafe { Box::from_raw(old) };
        drop(oldbox); //just being explict
    }

    pub fn get(&self) -> Rc<T> {
        let ptr = self.item.load(Ordering::SeqCst);
        return unsafe { (*ptr).clone() };
    }
}

impl<T> Drop for Container<T> {
    fn drop(&mut self) {
        let old = self.item.load(Ordering::SeqCst);
        let oldbox = unsafe { Box::from_raw(old) };
        drop(oldbox); //just being explict
    }
}

Thank you

[steffahn]

This code is not sound.

If one thread (A) calls replace while another thread (B) call get, you can get the interleaving that

• (B) executes the item.load instruction
• (A) executes the whole replace, dropping the old Box<Rc<…>>
• (B) continues executing get, calling .clone() on the already freed Rc (use after free)

(Additionally, there’s the problem that Rc itself isn’t thread-safe either.)

Crates such as arc_swap - Rust address these issues by providing a primitive that can safely be read atomically.

Crates such as seize - Rust or crossbeam_epoch - Rust offer some more performant (but unsafe to use, AFAIR) tools for implementing atomic data structures in a way comparable to GCd languages.

[8573]

Is arc-swap what you would want?

arc-swap

Edit: it was edited into the post above already

[josh1]

Thanks for the reply much appreciated ! yeah it make sense, Rc was just sloppy code... but it seems like there's no way to continue with this approach since it's unsound, I will have to redo =(.
I did look into arc_swap as well as atomic just as source of inspiration, my priority here is to learn and not to be productive so I'm only using those crates as a general approach how to tackle these problems.
I'm trying to come up with the simplest possible approach in order to see how comfortable with Rust I am rather than just slapping another dependency and move on...

Thank you !

[H2CO3]

If you are learning, then first I think you should learn the safe way of doing things concurrently. It is exceedingly rare that you really need to reach for unsafe for concurrency. If you want to "learn the Rust way", then start with std concurrency primitives and commonly-used crates such as rayon and parking_lot.