Is this (toy) use of UnsafeCell sound?

I would assume the following code is sound, but since I have been running into bad surprises in past, I would like to know if it's really sound:

use std::cell::UnsafeCell;

#[derive(Default)]
struct Inner {
    number: i32,
}

#[derive(Default)]
struct Outer {
    inner: UnsafeCell<Inner>,
}

impl Outer {
    fn increment(&self) {
        // SAFETY: There may be no concurrent access to `inner` because
        // only the `increment` and `get_number` methods access `inner` and
        // `Outer` is !Sync
        let inner: &mut Inner = unsafe { &mut *self.inner.get() };
        inner.number = inner.number.checked_add(1).unwrap();
    }
    fn get_number(&self) -> i32 {
        // SAFETY: There may be no concurrent access to `inner` because
        // only the `increment` and `get_number` methods access `inner` and
        // `Outer` is !Sync
        let inner: &Inner = unsafe { &*self.inner.get() };
        inner.number
    }
}

fn main() {
    let outer: Outer = Default::default();
    outer.increment();
    outer.increment();
    println!("{}", outer.get_number());
    let arc = std::sync::Arc::new(outer);
    let _arc2 = arc.clone();
    std::thread::spawn(move || {
        // Rust is smart enough to prohibit this:
        //_arc2.increment();
    });
    arc.increment();
}

(Playground)

2 Likes

This looks OK to me. (I'm still curious what real-world use case inspired it.)

1 Like

Your usage and safety invariants are essentially the same as Cell<i32>, so the general idea is certainly workable. I also don’t see any problems with your implementation, but don’t put too much stock in that assessment— I still get caught off-guard by some unsafe rules myself.

4 Likes

The use case is mmtkvdb::TxnRw, which is !Sync, and thus will not perform any internal locking when operating on it. (Use of UnsafeCell here; I think the code could definitely need better documentation though…) Feel free to see if you see any unsound code in mmtkvdb.

1 Like