Code review request: owner and holder pattern

code review
1

I have a use case where a value is shared by multiple reference-counting pointers, but only one specific pointer (the owner) will actually use the value, and the other pointers (the holders) can only access the value on destruction. Here is what I come up with (playground link):

pub mod owner_and_holder {
    use std::ops::{Deref, DerefMut};
    use std::sync::Arc;

    /// A reference-counting pointer to a `T` object, but cannot access it until destruction.
    pub struct Holder<T>
    where
        T: ?Sized,
    {
        inner: Arc<T>,
    }

    impl<T> Clone for Holder<T>
    where
        T: ?Sized,
    {
        fn clone(&self) -> Self {
            Self {
                inner: Arc::clone(&self.inner),
            }
        }
    }

    // SAFETY: Sending a `Holder<T>` to another thread may causing it to destruct on that another thread, which only
    // requires `T` being `Send`.
    unsafe impl<T> Send for Holder<T> where T: Send + ?Sized {}

    // Sharing a `Holder<T>` with another thread allows user to cloning it into that thread, which may causing it to
    // destruct on that thread, which only requires `T` being `Send`.
    unsafe impl<T> Sync for Holder<T> where T: Send + ?Sized {}

    /// A reference-counting pointer to a `T` object, with exclusive access to it, except for destructor, which is
    /// executed by the last `Owner<T>` or `Holder<T>` pointing to it.
    pub struct Owner<T>
    where
        T: ?Sized,
    {
        inner: Arc<T>,
    }

    impl<T> Owner<T>
    where
        T: ?Sized,
    {
        pub fn new(value: T) -> Self
        where
            T: Sized,
        {
            Self {
                inner: Arc::new(value),
            }
        }

        /// Creates a `Holder<T>` pointing to the the `T` value, does not have access to `T` until destruction, and
        /// extends the lifetime of `T` with it.
        pub fn holder(this: &Self) -> Holder<T> {
            Holder {
                inner: Arc::clone(&this.inner),
            }
        }
    }

    impl<T> Deref for Owner<T>
    where
        T: ?Sized,
    {
        type Target = T;

        fn deref(&self) -> &Self::Target {
            &self.inner
        }
    }

    impl<T> DerefMut for Owner<T>
    where
        T: ?Sized,
    {
        fn deref_mut(&mut self) -> &mut Self::Target {
            // SAFETY: Having a `Owner<T>` means that no `Holder<T>` is the last referencing pointer, which means we
            // have exclusive access to `T`.
            unsafe { &mut *Arc::as_ptr(&self.inner).cast_mut() }
        }
    }

    // SAFETY: `Owner<T>` has the same safety requirements as `T`.
    unsafe impl<T> Send for Owner<T> where T: Send + ?Sized {}

    // SAFETY: `Owner<T>` has the same safety requirements as `T`.
    unsafe impl<T> Sync for Owner<T> where T: Sync + ?Sized {}
}

fn main() {
    use owner_and_holder::Owner;

    let mut owner = Owner::new(42);
    let holder_1 = Owner::holder(&owner);

    *owner = 43;

    let holder_2 = holder_1.clone();

    assert_eq!(*owner, 43);

    drop(owner);
    drop(holder_1);
    drop(holder_2);
}

Is this implementation sound? Does this pattern have a known name?

2

I believe this impl has incorrect bounds:

    // SAFETY: `Owner<T>` has the same safety requirements as `T`.
    unsafe impl<T> Sync for Owner<T> where T: Sync + ?Sized {}

Because Owner::holder() exists, and Holder is Send, having an Owner<T> which is Sync allows T to eventually be dropped on another thread. Therefore, the bounds must be where T: Send + Sync — in other words, this unsafe impl should be removed, because the auto impl is correct.

I think the safety comment for unsafe impl<T> Send for Holder<T> would be improved by also contrasting it with the Send impl for Arc so it's clear what you are relaxing.

Other than that, your general plan seems OK to me, if correctly implemented.

4 Likes