From 9cf82af66a36942c202b2560bc26bd24da04445a Mon Sep 17 00:00:00 2001 From: Alexey Gerasev Date: Wed, 25 Sep 2024 17:58:07 +0700 Subject: [PATCH] Fix Heap construction from Vec of ZST, enforce exact capacity for Heap::new, bump version --- Cargo.toml | 4 ++-- src/rb/local.rs | 14 ++++++------ src/rb/macros.rs | 7 +++--- src/storage.rs | 30 +++++++++++++++---------- src/tests/mod.rs | 1 + src/tests/zero_sized.rs | 49 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 81 insertions(+), 24 deletions(-) create mode 100644 src/tests/zero_sized.rs diff --git a/Cargo.toml b/Cargo.toml index 2de06d9..14bb208 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,14 +7,14 @@ readme = "README.md" license = "MIT/Apache-2.0" [workspace.dependencies] -ringbuf = { path = ".", version = "0.4.4" } +ringbuf = { path = ".", version = "0.4.5" } [workspace] members = ["async", "blocking"] [package] name = "ringbuf" -version = "0.4.4" +version = "0.4.5" edition.workspace = true authors.workspace = true description = "Lock-free SPSC FIFO ring buffer with direct access to inner data" diff --git a/src/rb/local.rs b/src/rb/local.rs index 1145d52..40f0496 100644 --- a/src/rb/local.rs +++ b/src/rb/local.rs @@ -19,13 +19,13 @@ use core::{ ptr, }; -struct End { +struct Endpoint { index: Cell, held: Cell, } -impl End { - fn new(index: usize) -> Self { +impl Endpoint { + const fn new(index: usize) -> Self { Self { index: Cell::new(index), held: Cell::new(false), @@ -37,8 +37,8 @@ impl End { /// /// Slightly faster than multi-threaded version because it doesn't synchronize cache. pub struct LocalRb { - read: End, - write: End, + read: Endpoint, + write: Endpoint, storage: S, } @@ -53,8 +53,8 @@ impl LocalRb { assert!(!storage.is_empty()); Self { storage, - read: End::new(read), - write: End::new(write), + read: Endpoint::new(read), + write: Endpoint::new(write), } } /// Destructures ring buffer into underlying storage and `read` and `write` indices. diff --git a/src/rb/macros.rs b/src/rb/macros.rs index 26251d3..51e7d90 100644 --- a/src/rb/macros.rs +++ b/src/rb/macros.rs @@ -19,15 +19,16 @@ macro_rules! rb_impl_init { /// /// *Panics if allocation failed or `capacity` is zero.* pub fn new(capacity: usize) -> Self { - Self::try_new(capacity).unwrap() + unsafe { Self::from_raw_parts(crate::storage::Heap::::new(capacity), usize::default(), usize::default()) } } /// Creates a new instance of a ring buffer returning an error if allocation failed. /// /// *Panics if `capacity` is zero.* pub fn try_new(capacity: usize) -> Result { - let mut vec = alloc::vec::Vec::new(); + let mut vec = alloc::vec::Vec::>::new(); vec.try_reserve_exact(capacity)?; - Ok(unsafe { Self::from_raw_parts(vec.into(), usize::default(), usize::default()) }) + unsafe { vec.set_len(capacity) }; + Ok(unsafe { Self::from_raw_parts(vec.into_boxed_slice().into(), usize::default(), usize::default()) }) } } diff --git a/src/storage.rs b/src/storage.rs index bd504e9..6559446 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -2,7 +2,7 @@ use alloc::{boxed::Box, vec::Vec}; use core::{cell::UnsafeCell, marker::PhantomData, mem::MaybeUninit, ops::Range, ptr::NonNull, slice}; #[cfg(feature = "alloc")] -use core::{mem::forget, ptr}; +use core::{mem::ManuallyDrop, ptr}; /// Abstract storage for the ring buffer. /// @@ -104,7 +104,7 @@ unsafe impl Storage for Array { type Item = T; #[inline] fn as_mut_ptr(&self) -> *mut MaybeUninit { - self.data.get() as *mut _ + self.data.get().cast() } #[inline] fn len(&self) -> usize { @@ -122,7 +122,7 @@ unsafe impl Storage for Slice { type Item = T; #[inline] fn as_mut_ptr(&self) -> *mut MaybeUninit { - self.data.get() as *mut _ + self.data.get().cast() } #[inline] fn len(&self) -> usize { @@ -153,20 +153,25 @@ unsafe impl Storage for Heap { } #[cfg(feature = "alloc")] impl Heap { + /// Create a new heap storage with exact capacity. pub fn new(capacity: usize) -> Self { - Self { - ptr: Vec::::with_capacity(capacity).leak() as *mut _ as *mut MaybeUninit, - len: capacity, - } + let mut data = Vec::>::with_capacity(capacity); + // `data.capacity()` is not guaranteed to be equal to `capacity`. + // We enforce that by `set_len` and converting to boxed slice. + unsafe { data.set_len(capacity) }; + Self::from(data.into_boxed_slice()) } } #[cfg(feature = "alloc")] impl From>> for Heap { fn from(mut value: Vec>) -> Self { - let len = value.capacity(); - let ptr = value.as_mut_ptr(); - forget(value); - Self { ptr, len } + // Convert `value` to boxed slice of length equals to `value.capacity()` + // except for zero-sized types - for them length will be `value.len()` because `Vec::capacity` for ZST is undefined + // (see ). + if size_of::() != 0 { + unsafe { value.set_len(value.capacity()) }; + } + Self::from(value.into_boxed_slice()) } } #[cfg(feature = "alloc")] @@ -174,13 +179,14 @@ impl From]>> for Heap { fn from(value: Box<[MaybeUninit]>) -> Self { Self { len: value.len(), - ptr: Box::into_raw(value) as *mut MaybeUninit, + ptr: Box::into_raw(value).cast(), } } } #[cfg(feature = "alloc")] impl From> for Box<[MaybeUninit]> { fn from(value: Heap) -> Self { + let value = ManuallyDrop::new(value); unsafe { Box::from_raw(ptr::slice_from_raw_parts_mut(value.ptr, value.len)) } } } diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 5d67097..1f4f949 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -22,3 +22,4 @@ mod shared; mod skip; mod slice; mod unsized_; +mod zero_sized; diff --git a/src/tests/zero_sized.rs b/src/tests/zero_sized.rs new file mode 100644 index 0000000..8b22d84 --- /dev/null +++ b/src/tests/zero_sized.rs @@ -0,0 +1,49 @@ +use crate::{ + producer::Producer, + traits::{Consumer, Observer, Split}, + HeapRb, +}; + +#[test] +fn basic() { + let (mut prod, mut cons) = HeapRb::<()>::new(2).split(); + let obs = prod.observe(); + assert_eq!(obs.capacity().get(), 2); + + assert_eq!(obs.occupied_len(), 0); + assert_eq!(obs.vacant_len(), 2); + assert!(obs.is_empty()); + + assert!(cons.try_pop().is_none()); + + prod.try_push(()).unwrap(); + assert_eq!(obs.occupied_len(), 1); + assert_eq!(obs.vacant_len(), 1); + + prod.try_push(()).unwrap(); + assert_eq!(obs.occupied_len(), 2); + assert_eq!(obs.vacant_len(), 0); + assert!(obs.is_full()); + + assert!(prod.try_push(()).is_err()); + + cons.try_pop().unwrap(); + assert_eq!(obs.occupied_len(), 1); + assert_eq!(obs.vacant_len(), 1); + + prod.try_push(()).unwrap(); + assert_eq!(obs.occupied_len(), 2); + assert_eq!(obs.vacant_len(), 0); + assert!(obs.is_full()); + + cons.try_pop().unwrap(); + assert_eq!(obs.occupied_len(), 1); + assert_eq!(obs.vacant_len(), 1); + + cons.try_pop().unwrap(); + assert_eq!(obs.occupied_len(), 0); + assert_eq!(obs.vacant_len(), 2); + assert!(obs.is_empty()); + + assert!(cons.try_pop().is_none()); +}