Sanity and correctnes of my toy `OnceLock` implementation?

code review
3

I'm definitely not qualified to audit unsafe code, but here are a few observations (which may be inaccurate or entirely incorrect [1]).

Your Once type leaks any initialized value. This is safe, but not friendly. It should have impl Drop unless the leaking is intentional for some specific reason (which is not documented).

This seems to have the wrong constraint:

By requiring Sync, this allows sending a type that is not Send but is Sync to another thread, violating its invariants. One such type is MutexGuard. It has this invariant for POSIX hosts, according to this legendary post: Example of a type that is not `Send`? - #3 by Yandros

Which means it can (in theory) be exploited to send a MutexGuard to another thread to unlock a mutex it didn't originally lock, in violation of the pthreads contract:

fn main()
    static M: Mutex<()> = Mutex::new(());
    let o = Once::new();
    o.get_or_init(|| M.lock().unwrap());
    thread::spawn(move || drop(o)).join().unwrap();
    M.lock().unwrap();
}

We create a static mutex and then wrap the guard in a Once and drop it in another thread.

This is currently sound as-written -- because Once does not impl Drop! It leaks the guard and the mutex is never unlocked. The last line deadlocks.

Changing the constraint to impl<T: Send> Send [2] will allow a sound Drop impl.

  1. Standard disclaimer. ↩︎

  2. Constraining the Sync impl with T: Sync is perfectly fine, AFAIK. ↩︎

3 Likes