Is this a safe use of UnsafeCell?

help
1

I'm building a container for doing lazy initialization, and in order to allow the container to provide immutable references (i.e., to implement Deref), I need to use UnsafeCell so that I can still mutate the interior if the object needs to be initialized. I'm pretty sure that this is safe because UnsafeCell doesn't implement Sync, and so all of the immutable references to a given Lazy must be held by the same thread, but I'm still a bit unsure. Can anybody tell me if there's something I'm missing here wrt safety?

Here's the code:

use std::cell::UnsafeCell;
use std::ops::{Deref, DerefMut};

pub struct Lazy<T, F: Fn() -> T>(UnsafeCell<PrivateLazy<T, F>>);

enum PrivateLazy<T, F: Fn() -> T> {
    Uninitialized(F),
    Initialized(T),
}

impl<T, F: Fn() -> T> Lazy<T, F> {
    pub fn new(init: F) -> Lazy<T, F> {
        Lazy(UnsafeCell::new(PrivateLazy::Uninitialized(init)))
    }
}

impl<T, F: Fn() -> T> PrivateLazy<T, F> {
    fn eval(&mut self) {
        let t = match self {
            &mut PrivateLazy::Initialized(_) => return,
            &mut PrivateLazy::Uninitialized(ref mut init) => init(),
        };

        *self = PrivateLazy::Initialized(t);
    }
}

impl<T, F: Fn() -> T> Deref for Lazy<T, F> {
    type Target = T;

    fn deref(&self) -> &T {
        let interior = self.0.get();
        unsafe {
            (*interior).eval();
            match &*(interior as *const PrivateLazy<T, F>) {
                &PrivateLazy::Initialized(ref t) => t,
                &PrivateLazy::Uninitialized(_) => unreachable!(),
            }
        }
    }
}

impl<T, F: Fn() -> T> DerefMut for Lazy<T, F> {
    fn deref_mut(&mut self) -> &mut T {
        let interior = self.0.get();
        unsafe {
            (*interior).eval();
            match &mut *interior {
                &mut PrivateLazy::Initialized(ref mut t) => t,
                &mut PrivateLazy::Uninitialized(_) => unreachable!(),
            }
        }
    }
}
2

Nope, seems legit as written.

3

Awesome thanks!

4

What happens if there's reentrancy?

For example, if you store the Lazy object in a thread-local variable, and have the initialization function look up the object and deref it.

5

Ah, good point. However, I feel like reentrancy would never be safe with something like this because it would imply that your initialization routine depended on the thing it was initializing already being initialized, which is obviously impossible. So maybe something like RefCell might be better to catch those errors at runtime?

6

Seems like that would lead to a stack overflow, which is still safe. But, how would you arrange for that given Lazy needs the Fn on creation?

7 
thread_local!{
    static FOO: Lazy<u32, fn() -> u32> = Lazy::new(foo);
}

fn foo() -> u32 {
    FOO.with(|f| **f)
}

fn main() {
    println!("{}", foo());
}
3 Likes
8

Simple enough indeed. So, is there a way to forbid this statically (ie via type system)?

I think from a safety perspective this is fine since it'll overflow the stack. Of course it's undesirable to allow such footgun in the first place. Ahh, the problem of accepting arbitrary callables from outside ...

9

@vitalyd It'll still technically trigger undefined behavior, because a mutable reference to the Lazy would be created while one already exists.

@joshlf RefCell could work, and would avoid the need for unsafe code. There are also various ways you could stick it in as a third enum variant, but be careful of undefined behavior.

10

Yes, good point.

11

Ah yeah, I think I'll use RefCell at least in debug builds (this is in the hot path, so I want to avoid the RefCell overhead in release builds).

Thanks for all the good comments, folks!

12

FYI, the Shared combinator from futures can be used to perform thread-safe lazy initialisation.

13

Good to know! In this case, I actually explicitly want something that isn't thread-safe because I want to avoid the synchronization overhead.

14

So it looks like unfortunately RefCell won't actually work because borrow and borrow_mut return reference objects which live only for the lifetime of the borrow. I actually need the borrow to outlive the body of deref or deref_mut (the methods I'm implementing) and instead have the same lifetime as the Lazy object itself. The only way to accomplish that with RefCell would be to return these reference objects (Ref and RefMut) instead of references.

So UnsafeCell it will remain.