struct Unique: Copy more from std and use for Rav1dPictureDataComponentInner::ptr to make it Send + Sync

include/dav1d/picture.rs

@@ -29,6 +29,7 @@ use crate::src::error::Rav1dResult;
 use crate::src::pixels::Pixels;
 use crate::src::send_sync_non_null::SendSyncNonNull;
 use crate::src::strided::Strided;
+use crate::src::unique::Unique;
 use crate::src::with_offset::WithOffset;
 use libc::ptrdiff_t;
 use libc::uintptr_t;
@@ -94,7 +95,7 @@ impl From<Rav1dPictureParameters> for Dav1dPictureParameters {
 pub struct Dav1dPicture {
     pub seq_hdr: Option<NonNull<Dav1dSequenceHeader>>,
     pub frame_hdr: Option<NonNull<Dav1dFrameHeader>>,
-    pub data: [Option<NonNull<c_void>>; 3],
+    pub data: [Option<Unique<c_void>>; 3],
     pub stride: [ptrdiff_t; 2],
     pub p: Dav1dPictureParameters,
     pub m: Dav1dDataProps,
@@ -134,7 +135,7 @@ pub struct Rav1dPictureDataComponentInner {
     /// even if [`Self::stride`] is negative.
     ///
     /// It is aligned to [`RAV1D_PICTURE_ALIGNMENT`].
-    ptr: NonNull<u8>,
+    ptr: Unique<u8>,
 
     /// The length of [`Self::ptr`] in [`u8`] bytes.
     ///
@@ -151,12 +152,12 @@ impl Rav1dPictureDataComponentInner {
     /// # Safety
     ///
     /// `ptr`, `len`, and `stride` must follow the requirements of [`Dav1dPicAllocator::alloc_picture_callback`].
-    unsafe fn new(ptr: Option<NonNull<u8>>, len: usize, stride: isize) -> Self {
+    unsafe fn new(ptr: Option<Unique<u8>>, len: usize, stride: isize) -> Self {
         let ptr = match ptr {
             None => {
                 return Self {
                     // Ensure it is aligned enough.
-                    ptr: NonNull::<AlignedPixelChunk>::dangling().cast(),
+                    ptr: Unique::<AlignedPixelChunk>::dangling().cast(),
                     len: 0,
                     stride,
                 };
@@ -168,13 +169,15 @@ impl Rav1dPictureDataComponentInner {
         assert!(ptr.cast::<AlignedPixelChunk>().is_aligned());
 
         let ptr = if stride < 0 {
-            let ptr = ptr.as_ptr();
-            // SAFETY: According to `Dav1dPicAllocator::alloc_picture_callback`,
-            // if the `stride` is negative, this is how we get the start of the data.
-            // `.offset(-stride)` puts us at one element past the end of the slice,
-            // and `.sub(len)` puts us back at the start of the slice.
-            let ptr = unsafe { ptr.offset(-stride).sub(len) };
-            NonNull::new(ptr).unwrap()
+            ptr.map(|ptr| {
+                let ptr = ptr.as_ptr();
+                // SAFETY: According to `Dav1dPicAllocator::alloc_picture_callback`,
+                // if the `stride` is negative, this is how we get the start of the data.
+                // `.offset(-stride)` puts us at one element past the end of the slice,
+                // and `.sub(len)` puts us back at the start of the slice.
+                let ptr = unsafe { ptr.offset(-stride).sub(len) };
+                NonNull::new(ptr).unwrap()
+            })
         } else {
             ptr
         };
@@ -189,7 +192,7 @@ impl Rav1dPictureDataComponentInner {
     /// so it is sound to further subdivide it into disjoint `&mut`s.
     pub fn wrap_buf<BD: BitDepth>(buf: &mut [BD::Pixel], stride: usize) -> Self {
         let buf = AsBytes::as_bytes_mut(buf);
-        let ptr = NonNull::new(buf.as_mut_ptr()).unwrap();
+        let ptr = Unique::from_ref_mut(buf).cast();
         assert!(ptr.cast::<AlignedPixelChunk>().is_aligned());
         let len = buf.len();
         assert!(len % RAV1D_PICTURE_GUARANTEED_MULTIPLE == 0);
@@ -292,11 +295,14 @@ impl Rav1dPictureDataComponent {
         self.as_strided_mut_ptr::<BD>().cast_const()
     }
 
-    fn as_dav1d(&self) -> Option<NonNull<c_void>> {
+    fn as_dav1d(&self) -> Option<Unique<c_void>> {
         if self.byte_len() == 0 {
             None
         } else {
-            NonNull::new(self.as_strided_byte_mut_ptr().cast())
+            let ptr = NonNull::new(self.as_strided_byte_mut_ptr()).unwrap();
+            // SAFETY: The `ptr` originally comes from a `Unique` in `Rav1dPictureDataComponentInner::ptr`.
+            let ptr = unsafe { Unique::new(ptr) };
+            Some(ptr.cast())
         }
     }
 

lib.rs

@@ -56,6 +56,7 @@ pub mod src {
     pub(crate) mod relaxed_atomic;
     pub mod send_sync_non_null;
     pub(crate) mod strided;
+    pub mod unique;
     pub(crate) mod with_offset;
     pub(crate) mod wrap_fn_ptr;
     // TODO(kkysen) Temporarily `pub(crate)` due to a `pub use` until TAIT.

src/c_box.rs

@@ -1,12 +1,11 @@
 #![deny(unsafe_op_in_unsafe_fn)]
 
 use crate::src::send_sync_non_null::SendSyncNonNull;
+use crate::src::unique::Unique;
 use std::ffi::c_void;
-use std::marker::PhantomData;
 use std::ops::Deref;
 use std::pin::Pin;
 use std::ptr::drop_in_place;
-use std::ptr::NonNull;
 
 pub type FnFree = unsafe extern "C" fn(ptr: *const u8, cookie: Option<SendSyncNonNull<c_void>>);
 
@@ -38,37 +37,6 @@ impl Free {
     }
 }
 
-/// Same as [`core::ptr::Unique`].
-///
-/// A wrapper around a [`NonNull`]`<T>` that indicates that the possessor
-/// of this wrapper owns the referent.
-///
-/// [`Unique`]`<T>` behaves "as if" it were an instance of `T`.
-/// It implements [`Send`]/[`Sync`] if `T: `[`Send`]/[`Sync`].
-/// It also implies the kind of strong aliasing guarantees an instance of `T` can expect:
-/// the referent of the pointer should not be modified
-/// without a unique path to its owning [`Unique`].
-///
-/// Unlike [`NonNull`]`<T>`, `Unique<T>` is covariant over `T`.
-/// This should always be correct for any type which upholds [`Unique`]'s aliasing requirements.
-#[derive(Debug)]
-pub struct Unique<T: ?Sized> {
-    pointer: NonNull<T>,
-    // NOTE: this marker has no consequences for variance, but is necessary
-    // for dropck to understand that we logically own a `T`.
-    //
-    // For details, see:
-    // https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#phantom-data
-    _marker: PhantomData<T>,
-}
-
-/// SAFETY: [`Unique`] is [`Send`] if `T: `[`Send`]
-/// because the data it references is unaliased.
-unsafe impl<T: Send + ?Sized> Send for Unique<T> {}
-
-/// SAFETY: [`Unique`] is [`Sync`] if `T: `[`Sync`]
-unsafe impl<T: Sync + ?Sized> Sync for Unique<T> {}
-
 /// A C/custom [`Box`].
 ///
 /// That is, it is analogous to a [`Box`],
@@ -97,7 +65,7 @@ impl<T: ?Sized> AsRef<T> for CBox<T> {
             // so we can take `&` references of it.
             // Furthermore, `data` is never moved and is valid to dereference,
             // so this reference can live as long as `CBox` and still be valid the whole time.
-            Self::C { data, .. } => unsafe { data.pointer.as_ref() },
+            Self::C { data, .. } => unsafe { data.as_ref() },
         }
     }
 }
@@ -115,7 +83,7 @@ impl<T: ?Sized> Drop for CBox<T> {
         match self {
             Self::Rust(_) => {} // Drop normally.
             Self::C { data, free, .. } => {
-                let ptr = data.pointer.as_ptr();
+                let ptr = data.as_ptr();
                 // SAFETY: See below.
                 // The [`FnFree`] won't run Rust's `fn drop`,
                 // so we have to do this ourselves first.
@@ -135,14 +103,8 @@ impl<T: ?Sized> CBox<T> {
     /// until `free.free` is called on it, which must deallocate it.
     /// `free.free` is always called with `free.cookie`,
     /// which must be accessed thread-safely.
-    pub unsafe fn from_c(data: NonNull<T>, free: Free) -> Self {
-        Self::C {
-            data: Unique {
-                pointer: data,
-                _marker: PhantomData,
-            },
-            free,
-        }
+    pub unsafe fn from_c(data: Unique<T>, free: Free) -> Self {
+        Self::C { data, free }
     }
 
     pub fn from_box(data: Box<T>) -> Self {

src/data.rs

@@ -10,8 +10,8 @@ use crate::src::c_box::Free;
 use crate::src::error::Rav1dError::EINVAL;
 use crate::src::error::Rav1dResult;
 use crate::src::send_sync_non_null::SendSyncNonNull;
+use crate::src::unique::Unique;
 use std::ffi::c_void;
-use std::ptr::NonNull;
 
 impl From<CArc<[u8]>> for Rav1dData {
     fn from(data: CArc<[u8]>) -> Self {
@@ -36,7 +36,7 @@ impl Rav1dData {
     ///
     /// See [`CBox::from_c`]'s safety for `data`, `free_callback`, `cookie`.
     pub unsafe fn wrap(
-        data: NonNull<[u8]>,
+        data: Unique<[u8]>,
         free_callback: Option<FnFree>,
         cookie: Option<SendSyncNonNull<c_void>>,
     ) -> Rav1dResult<Self> {
@@ -53,7 +53,7 @@ impl Rav1dData {
     /// See [`CBox::from_c`]'s safety for `user_data`, `free_callback`, `cookie`.
     pub unsafe fn wrap_user_data(
         &mut self,
-        user_data: NonNull<u8>,
+        user_data: Unique<u8>,
         free_callback: Option<FnFree>,
         cookie: Option<SendSyncNonNull<c_void>>,
     ) -> Rav1dResult {

src/lib.rs

@@ -52,6 +52,7 @@ use crate::src::send_sync_non_null::SendSyncNonNull;
 use crate::src::thread_task::rav1d_task_delayed_fg;
 use crate::src::thread_task::rav1d_worker_task;
 use crate::src::thread_task::FRAME_ERROR;
+use crate::src::unique::Unique;
 use parking_lot::Mutex;
 use std::cmp;
 use std::ffi::c_char;
@@ -863,7 +864,7 @@ pub unsafe extern "C" fn dav1d_data_create(buf: Option<NonNull<Dav1dData>>, sz:
 #[no_mangle]
 pub unsafe extern "C" fn dav1d_data_wrap(
     buf: Option<NonNull<Dav1dData>>,
-    ptr: Option<NonNull<u8>>,
+    ptr: Option<Unique<u8>>,
     sz: usize,
     free_callback: Option<FnFree>,
     user_data: Option<SendSyncNonNull<c_void>>,
@@ -874,8 +875,10 @@ pub unsafe extern "C" fn dav1d_data_wrap(
         validate_input!((sz <= usize::MAX / 2, EINVAL))?;
         // SAFETY: `ptr` is the start of a `&[u8]` slice of length `sz`.
         let data = unsafe { slice::from_raw_parts(ptr.as_ptr(), sz) };
+        // SAFETY: `data` is unique since `ptr` was unique.
+        let data = unsafe { Unique::from_ref(data) };
         // SAFETY: `ptr`, and thus `data`, is valid to dereference until `free_callback` is called on it, which deallocates it.
-        let data = unsafe { Rav1dData::wrap(data.into(), free_callback, user_data) }?;
+        let data = unsafe { Rav1dData::wrap(data, free_callback, user_data) }?;
         let data_c = data.into();
         // SAFETY: `buf` is safe to write to.
         unsafe { buf.as_ptr().write(data_c) };
@@ -891,7 +894,7 @@ pub unsafe extern "C" fn dav1d_data_wrap(
 #[no_mangle]
 pub unsafe extern "C" fn dav1d_data_wrap_user_data(
     buf: Option<NonNull<Dav1dData>>,
-    user_data: Option<NonNull<u8>>,
+    user_data: Option<Unique<u8>>,
     free_callback: Option<FnFree>,
     cookie: Option<SendSyncNonNull<c_void>>,
 ) -> Dav1dResult {

src/picture.rs

@@ -26,15 +26,14 @@ use crate::src::log::Rav1dLog as _;
 use crate::src::log::Rav1dLogger;
 use crate::src::mem::MemPool;
 use crate::src::send_sync_non_null::SendSyncNonNull;
+use crate::src::unique::Unique;
 use bitflags::bitflags;
 use libc::ptrdiff_t;
 use parking_lot::Mutex;
 use std::ffi::c_int;
 use std::ffi::c_void;
 use std::mem;
-use std::ptr;
 use std::ptr::fn_addr_eq;
-use std::ptr::NonNull;
 use std::sync::atomic::AtomicU32;
 use std::sync::Arc;
 use to_method::To as _;
@@ -143,16 +142,16 @@ unsafe extern "C" fn dav1d_default_picture_alloc(
     // but this way is simpler and more uniform, especially when we move to slices.
     let data = [data0, data1, data2].map(|data| {
         if data.is_empty() {
-            ptr::null_mut()
+            None
         } else {
-            data.as_mut_ptr().cast()
+            Some(Unique::from_ref_mut(data).cast())
         }
     });
 
     // SAFETY: Guaranteed by safety preconditions.
     let p_c = unsafe { &mut *p_c };
     p_c.stride = stride;
-    p_c.data = data.map(NonNull::new);
+    p_c.data = data;
     p_c.allocator_data = Some(SendSyncNonNull::from_box(buf).cast::<c_void>());
     // The caller will create the real `Rav1dPicture` from the `Dav1dPicture` fields set above,
     // so we don't want to drop the `Rav1dPicture` we created for convenience here.