Constructing a !Sized UnsafeCell

[DzenanJupic]

I'm currently working on an atomic, append-only array-vec, that I now wanted to test using loom.

Besides the API, one of the major differences between std::cell::UnsafeCell and loom::cell::UnsafeCell is that the std version is repr(transparent). This allows i.e. casting between [UnsafeCell<T>] and [T] or between UnsafeCell<[T]> and [T].

The loom version is not repr(transparent) (and appears to not be able to be), which leads to an interesting problem when trying to cast the slice of potentially uninitialized elements into a slice of elements.

My initial design looked somewhat like this:

pub struct Array<T> {
    elements: Box<[UnsafeCell<mem::MaybeUninit<T>>]>,
    // ... other stuff, not important for now
}

impl<T> Array<T> {
    pub fn from_vec(vec: Vec<T>) -> Self {
        let len = vec.len();
        let elements = {
            let mut vec = mem::ManuallyDrop::new(vec);
            let cap = vec.capacity();
            let ptr = vec.as_mut_ptr() as *mut UnsafeCell<mem::MaybeUninit<T>>;

            // SAFETY:
            // - We took the values from a real vec we own, so they are valid
            // - We leak the original vec, so there's no double free
            // - `UnsafeCell<ManuallyDrop<T>>` has the same layout as `T`            // << This is no longer true
            unsafe { Vec::<UnsafeCell<mem::MaybeUninit<T>>>::from_raw_parts(ptr, len, cap) }
        };

        Self {
            elements: elements.into_boxed_slice(),
            // ...
        }
    }

    pub fn as_slice(&self) -> &[T] {
        let slice = &self.elements[..self.len()];

        // SAFETY:
        // `len` will only ever be increased when a new element was
        // pushed to the array. Since we never remove any elements,
        // we know that all elements until `len` are initialized, and
        // that the lifetime of the slice is the same as the one of
        // `&self`. Also, `[UnsafeCell<ManuallyDrop<T>>]` has the 
        // same layout as `[T]`.                                               // << This is no longer true
        unsafe { &*(slice as *const _ as *const _) }
    }
}

My first idea to solve the problem was to lift the UnsafeCell out of the slice: Box<UnsafeCell<[MaybeUninit<T>]>>. This would lead to less ergonomic code when reading, since I now have to construct slices from pointers, but should still work. The only problem is, that I cannot create such a slice, since loom::cell::UnsafeCell::new (of course) requires T to be sized, and I of course also cannot cast between a Box<[MaybeUninit<T>]> and a Box<UnsafeCell<[MaybeUninit<T>]>> due to the same reasons as above:

impl<T> Array<T> {
    pub fn from_vec(vec: Vec<T>) -> Self {
        let len = vec.len();
        let elements = {
            let mut vec = mem::ManuallyDrop::new(vec);
            let cap = vec.capacity();
            let ptr = vec.as_mut_ptr() as *mut UnsafeCell<mem::MaybeUninit<T>>;

            // SAFETY:
            // - We took the values from a real vec we own, so they are valid
            // - We leak the original vec, so there's no double free
            // - `ManuallyDrop<T>` has the same layout as `T`
            unsafe { Vec::<mem::MaybeUninit<T>>::from_raw_parts(ptr, len, cap) }
        };

        let boxed: Box<[MaybeUninit<T>]> = elements.into_boxed_slice();
        let elements: Box<UnsafeCell<[MaybeUninit<T>]>> = /* What do I do here? */;

        Self {
            elements,
            // ...
        }
    }
}

The problem is, that lifting the UnsafeCell yet another layer to UnsafeCell<Box<[MaybeUninit<T>]>> breaks soundness, since I can not guarantee sound read access to the whole slice, only a small part of it:

impl<T> Array<T> {
    pub fn as_slice(&self) -> &[T] {
        self.elements.with(|boxed: *const Box<[MaybeUninit<T>]>| {
                let slice: &[MaybeUninit<T>] = unsafe { &**boxed };   //  << unsound, since someone else might currently hold an exclusive reference to the uninitialized end of the slice
                todo!()
        })
    }
}

So, as far as I can see, it's currently impossible - or at least I cannot think of any method - to represent that type in a way so that soundness is not broken during the loom tests.

Is there a know solution for that? Or am I just missing something obvious here?
Here's a playground link to the first code snippet, in case someone wants to play with it.

[Darksonn]

I recommend that you use a type like this one:

use loom::cell::UnsafeCell as LoomCell;
use std::cell::UnsafeCell as StdCell;
use std::mem::MaybeUninit;

struct CellSlice<T> {
    inner: LoomCell<Box<StdCell<[MaybeUninit<T>]>>>,
}

impl<T> CellSlice<T> {
    fn with<F, R>(&self, f: F) -> R
    where
        F: FnOnce(*const T, usize) -> R,
    {
        self.inner.with(|ptr| {
            let ptr = unsafe { StdCell::raw_get(&**ptr) };
            let len = unsafe { (*ptr).len() };
            f(ptr.cast(), len)
        })
    }

    fn with_mut<F, R>(&self, f: F) -> R
    where
        F: FnOnce(*mut T, usize) -> R,
    {
        self.inner.with_mut(|ptr| {
            let ptr = unsafe { StdCell::raw_get(&**ptr) };
            let len = unsafe { (*ptr).len() };
            f(ptr.cast(), len)
        })
    }
}

[DzenanJupic]

Wouldn't that lead to a test failure in case someone is reading the already initialized elements, while someone else is writing a new element? [playground]

CellSlice

use loom::cell::UnsafeCell as LoomCell;
use std::cell::UnsafeCell as StdCell;
use std::mem::{self, MaybeUninit};

struct CellSlice<T> {
    inner: LoomCell<Box<StdCell<[MaybeUninit<T>]>>>,
}

#[allow(dead_code)]
impl<T> CellSlice<T> {
    fn new() -> Self {
        let boxed: Box<StdCell<[MaybeUninit<T>]>> = unsafe {
            let boxed = mem::ManuallyDrop::new(Vec::<T>::new().into_boxed_slice());
            std::ptr::read(&boxed as *const _ as *const _)
        };

        Self {
            inner: LoomCell::new(boxed),
        }
    }

    fn with<F, R>(&self, f: F) -> R
        where
            F: FnOnce(*const T, usize) -> R,
    {
        self.inner.with(|ptr| {
            let ptr = unsafe { StdCell::raw_get(&**ptr) };
            let len = unsafe { (*ptr).len() };
            f(ptr.cast(), len)
        })
    }

    fn with_mut<F, R>(&self, f: F) -> R
        where
            F: FnOnce(*mut T, usize) -> R,
    {
        self.inner.with_mut(|ptr| {
            let ptr = unsafe { StdCell::raw_get(&**ptr) };
            let len = unsafe { (*ptr).len() };
            f(ptr.cast(), len)
        })
    }
}

#[test]
fn concurrent_read_and_write() {
    loom::model(|| {
        let cell_slice = std::sync::Arc::new(CellSlice::<String>::new());

        let read = {
            let cell_slice = cell_slice.clone();
            loom::thread::spawn(move || {
                let _ = cell_slice.with(|_, _| {});
            })
        };
        let write = {
            let cell_slice = cell_slice.clone();
            loom::thread::spawn(move || {
                let _ = cell_slice.with_mut(|_, _| {});
            })
        };

        read.join().unwrap();
        write.join().unwrap();
    });
}

[Darksonn]

True, it would not work with per-field concurrency checking. To do that, perhaps you could use a struct like this:

struct CellSlice<T> {
    fields: Box<StdCell<[MaybeUninit<T>]>>,
    loom_check: Box<[LoomCell<()>]>,
}

Then, when accessing field i, you do so inside a with or with_mut call on the ith cell in loom_check.

[DzenanJupic]

That's perfect, thanks. Even allows to keep using the 'normal' UnsafeCell interface. Should have thought about that earlier.