diff --git a/node-graph/libraries/core-types/src/arena.rs b/node-graph/libraries/core-types/src/arena.rs index 6cda93d4e3..9a6f320482 100644 --- a/node-graph/libraries/core-types/src/arena.rs +++ b/node-graph/libraries/core-types/src/arena.rs @@ -30,7 +30,9 @@ struct DropEntry { // SAFETY: disjoint regions are handed out by an atomic bump; a region is written // only by its allocating caller before publication, and cross-thread hand-off is -// ordered by the Release/Acquire pair on the published handle word. +// ordered by the Release/Acquire pair on the published handle word. The stored +// payloads are `Send + Sync` by the bound on every allocating method, so drop +// glue running on the resetting thread and lent `&T` crossing threads are sound. unsafe impl Sync for Arena {} unsafe impl Send for Arena {} @@ -74,7 +76,7 @@ impl Arena { Some(start) } - pub fn alloc(&self, value: T) -> Option<(&T, ArenaWeak)> { + pub fn alloc(&self, value: T) -> Option<(&T, ArenaWeak)> { let offset = self.reserve(size_of::(), align_of::())?; // Built before the write so an unencodable offset drops `value` here // rather than stranding it in the arena without drop glue. @@ -92,7 +94,7 @@ impl Arena { Some((unsafe { &*ptr }, weak)) } - pub fn alloc_slice_copy(&self, src: &[T]) -> Option<&[T]> { + pub fn alloc_slice_copy(&self, src: &[T]) -> Option<&[T]> { let buf = self.alloc_scratch::(src.len())?; for (slot, &value) in buf.iter_mut().zip(src) { slot.write(value); @@ -104,7 +106,7 @@ impl Arena { // Not drop-tracked: callers must either consume every written lane or // restrict themselves to `Copy` payloads (leak, not UB, otherwise). #[allow(clippy::mut_from_ref)] - pub fn alloc_scratch(&self, len: usize) -> Option<&mut [MaybeUninit]> { + pub fn alloc_scratch(&self, len: usize) -> Option<&mut [MaybeUninit]> { let size = size_of::().checked_mul(len)?; let offset = self.reserve(size, align_of::())?; let ptr = unsafe { self.base().add(offset) }.cast::>(); @@ -115,7 +117,7 @@ impl Arena { pub fn reset(&mut self) { let base = self.base(); - for entry in self.drops.get_mut().unwrap().drain(..) { + for entry in self.drops.get_mut().unwrap().drain(..).rev() { // SAFETY: registered at alloc time; insert-only means the region was // never overwritten within this generation. unsafe { (entry.drop_fn)(base.add(entry.offset)) } @@ -273,6 +275,23 @@ mod tests { assert!(arena.alloc(0u32).is_some(), "the arena stays usable"); } + #[test] + fn reset_drops_dependents_before_their_dependencies() { + static ORDER: Mutex> = Mutex::new(Vec::new()); + struct Probe(u32); + impl Drop for Probe { + fn drop(&mut self) { + ORDER.lock().unwrap().push(self.0); + } + } + let mut arena = Arena::new(1024); + for id in 0..3 { + arena.alloc(Probe(id)).unwrap(); + } + arena.reset(); + assert_eq!(*ORDER.lock().unwrap(), vec![2, 1, 0], "later allocations may borrow earlier ones, so they drop first"); + } + #[test] fn drop_glue_runs_on_reset() { static DROPS: AtomicU32 = AtomicU32::new(0);