From b5073be895e3873e6fae40732ceb5ccae117b43a Mon Sep 17 00:00:00 2001 From: Flamki <9833ayush@gmail.com> Date: Sun, 29 Mar 2026 22:15:59 +0530 Subject: [PATCH] mark_sweep: add new_cyclic_in parity semantics and tests --- .../mark_sweep/internals/ephemeron.rs | 15 ++++++++ .../collectors/mark_sweep/internals/gc_box.rs | 7 ++++ oscars/src/collectors/mark_sweep/mod.rs | 36 +++++++++++++++++++ .../src/collectors/mark_sweep/pointers/gc.rs | 20 +++++++++++ .../collectors/mark_sweep/pointers/weak.rs | 8 +++++ oscars/src/collectors/mark_sweep/tests.rs | 19 ++++++++++ .../mark_sweep_arena2/internals/ephemeron.rs | 15 ++++++++ .../mark_sweep_arena2/internals/gc_box.rs | 7 ++++ .../src/collectors/mark_sweep_arena2/mod.rs | 35 ++++++++++++++++++ .../mark_sweep_arena2/pointers/gc.rs | 22 ++++++++++++ .../mark_sweep_arena2/pointers/weak.rs | 8 +++++ .../src/collectors/mark_sweep_arena2/tests.rs | 19 ++++++++++ 12 files changed, 211 insertions(+) diff --git a/oscars/src/collectors/mark_sweep/internals/ephemeron.rs b/oscars/src/collectors/mark_sweep/internals/ephemeron.rs index 52b389c..fd9afea 100644 --- a/oscars/src/collectors/mark_sweep/internals/ephemeron.rs +++ b/oscars/src/collectors/mark_sweep/internals/ephemeron.rs @@ -66,6 +66,21 @@ impl Ephemeron { } } +impl Ephemeron { + pub(crate) fn new_empty(color: TraceColor) -> Self { + Self { + value: GcBox::new_in((), color), + vtable: vtable_of::(), + key: WeakGcBox::new_empty(), + active: core::cell::Cell::new(true), + } + } + + pub(crate) fn set_key(&self, key: &Gc) { + self.key.inner_ptr.set(Some(key.inner_ptr)); + } +} + impl Ephemeron { pub(crate) fn trace_fn(&self) -> EphemeronTraceFn { self.vtable.trace_fn diff --git a/oscars/src/collectors/mark_sweep/internals/gc_box.rs b/oscars/src/collectors/mark_sweep/internals/gc_box.rs index 374cadb..92126a5 100644 --- a/oscars/src/collectors/mark_sweep/internals/gc_box.rs +++ b/oscars/src/collectors/mark_sweep/internals/gc_box.rs @@ -50,6 +50,13 @@ impl WeakGcBox { } } + pub fn new_empty() -> Self { + Self { + inner_ptr: Cell::new(None), + marker: PhantomData, + } + } + pub(crate) fn erased_inner_ptr(&self) -> Option>> { // SAFETY: `as_heap_ptr` returns a valid pointer to // `PoolItem` whose lifetime is tied to the pool diff --git a/oscars/src/collectors/mark_sweep/mod.rs b/oscars/src/collectors/mark_sweep/mod.rs index 607641b..2b183d7 100644 --- a/oscars/src/collectors/mark_sweep/mod.rs +++ b/oscars/src/collectors/mark_sweep/mod.rs @@ -51,6 +51,10 @@ pub trait Collector { value: V, ) -> Result>, PoolAllocError>; + fn alloc_empty_ephemeron_node<'gc, K: Trace + 'static>( + &'gc self, + ) -> Result>, PoolAllocError>; + // Register a weak map with the GC so it can prune dead entries #[doc(hidden)] fn track_weak_map(&self, map: core::ptr::NonNull); @@ -487,6 +491,38 @@ impl Collector for MarkSweepGarbageCollector { Ok(inner_ptr) } + fn alloc_empty_ephemeron_node<'gc, K: Trace + 'static>( + &'gc self, + ) -> Result>, crate::alloc::mempool3::PoolAllocError> { + if self.collect_needed.get() && !self.is_collecting.get() { + self.collect_needed.set(false); + self.collect(); + } + + let ephemeron = Ephemeron::::new_empty(self.trace_color.get()); + + let mut alloc = self.allocator.borrow_mut(); + let inner_ptr = alloc.try_alloc(ephemeron)?; + let needs_collect = !alloc.is_below_threshold(); + drop(alloc); + + if needs_collect { + self.collect_needed.set(true); + } + + let eph_ptr = inner_ptr + .as_ptr() + .cast::>>(); + + if self.is_collecting.get() { + self.pending_ephemeron_queue.borrow_mut().push(eph_ptr); + } else { + self.ephemeron_queue.borrow_mut().push(eph_ptr); + } + + Ok(inner_ptr) + } + fn track_weak_map( &self, map: core::ptr::NonNull< diff --git a/oscars/src/collectors/mark_sweep/pointers/gc.rs b/oscars/src/collectors/mark_sweep/pointers/gc.rs index c7d6bfd..fe027d8 100644 --- a/oscars/src/collectors/mark_sweep/pointers/gc.rs +++ b/oscars/src/collectors/mark_sweep/pointers/gc.rs @@ -35,6 +35,26 @@ impl Gc { gc } + #[must_use] + pub fn new_cyclic_in(collector: &C, data_fn: F) -> Self + where + C: Collector, + F: FnOnce(&crate::collectors::mark_sweep::WeakGc) -> T, + { + let weak = unsafe { + crate::collectors::mark_sweep::WeakGc::from_raw( + collector + .alloc_empty_ephemeron_node::() + .expect("Failed to allocate Ephemeron node") + .extend_lifetime(), + ) + }; + + let gc = Self::new_in(data_fn(&weak), collector); + weak.set_key(&gc); + gc + } + /// Converts a `Gc` into a raw [`PoolPointer`]. pub fn into_raw(this: Self) -> PoolPointer<'static, GcBox> { let ptr = this.inner_ptr(); diff --git a/oscars/src/collectors/mark_sweep/pointers/weak.rs b/oscars/src/collectors/mark_sweep/pointers/weak.rs index fa1aa8a..29c035c 100644 --- a/oscars/src/collectors/mark_sweep/pointers/weak.rs +++ b/oscars/src/collectors/mark_sweep/pointers/weak.rs @@ -34,4 +34,12 @@ impl WeakGc { pub fn upgrade(&self) -> Option> { self.inner_ptr.as_inner_ref().upgrade() } + + pub(crate) unsafe fn from_raw(inner_ptr: PoolPointer<'static, Ephemeron>) -> Self { + Self { inner_ptr } + } + + pub(crate) fn set_key(&self, key: &Gc) { + self.inner_ptr.as_inner_ref().set_key(key); + } } diff --git a/oscars/src/collectors/mark_sweep/tests.rs b/oscars/src/collectors/mark_sweep/tests.rs index dc3e5bf..bec45d0 100644 --- a/oscars/src/collectors/mark_sweep/tests.rs +++ b/oscars/src/collectors/mark_sweep/tests.rs @@ -149,6 +149,25 @@ fn ptr_eq_distinguishes_equal_values() { ); } +#[test] +fn new_cyclic_closure_cannot_upgrade_before_init() { + let collector = &mut MarkSweepGarbageCollector::default() + .with_page_size(256) + .with_heap_threshold(512); + + let mut saw_none = false; + let gc = Gc::new_cyclic_in(collector, |weak| { + saw_none = weak.upgrade().is_none(); + 7u32 + }); + + assert!( + saw_none, + "weak should not upgrade during new_cyclic construction" + ); + assert_eq!(*gc, 7u32, "new_cyclic should initialize final value"); +} + #[test] fn multi_gc() { let collector = &mut MarkSweepGarbageCollector::default() diff --git a/oscars/src/collectors/mark_sweep_arena2/internals/ephemeron.rs b/oscars/src/collectors/mark_sweep_arena2/internals/ephemeron.rs index 793fd0b..97fd8ac 100644 --- a/oscars/src/collectors/mark_sweep_arena2/internals/ephemeron.rs +++ b/oscars/src/collectors/mark_sweep_arena2/internals/ephemeron.rs @@ -62,6 +62,21 @@ impl Ephemeron { } } +impl Ephemeron { + pub(crate) fn new_empty(color: TraceColor) -> Self { + Self { + value: GcBox::new_in((), color), + vtable: vtable_of::(), + key: WeakGcBox::new_empty(), + active: core::cell::Cell::new(true), + } + } + + pub(crate) fn set_key(&self, key: &Gc) { + self.key.inner_ptr.set(Some(key.inner_ptr)); + } +} + impl Ephemeron { pub(crate) fn trace_fn(&self) -> EphemeronTraceFn { self.vtable.trace_fn diff --git a/oscars/src/collectors/mark_sweep_arena2/internals/gc_box.rs b/oscars/src/collectors/mark_sweep_arena2/internals/gc_box.rs index fe8c661..24e957e 100644 --- a/oscars/src/collectors/mark_sweep_arena2/internals/gc_box.rs +++ b/oscars/src/collectors/mark_sweep_arena2/internals/gc_box.rs @@ -44,6 +44,13 @@ impl WeakGcBox { } } + pub fn new_empty() -> Self { + Self { + inner_ptr: Cell::new(None), + marker: PhantomData, + } + } + pub(crate) fn erased_inner_ptr(&self) -> Option>> { // SAFETY: `&raw mut` prevents creating `&mut` reference into the // arena to avoid stacked borrows during Gc tracing diff --git a/oscars/src/collectors/mark_sweep_arena2/mod.rs b/oscars/src/collectors/mark_sweep_arena2/mod.rs index 8fc1437..7680db7 100644 --- a/oscars/src/collectors/mark_sweep_arena2/mod.rs +++ b/oscars/src/collectors/mark_sweep_arena2/mod.rs @@ -445,6 +445,41 @@ impl MarkSweepGarbageCollector { Ok(inner_ptr) } + fn alloc_empty_ephemeron_node< + 'gc, + K: crate::collectors::mark_sweep_arena2::trace::Trace + 'static, + >( + &'gc self, + ) -> Result>, crate::alloc::arena2::ArenaAllocError> { + if self.collect_needed.get() && !self.is_collecting.get() { + self.collect_needed.set(false); + self.collect(); + } + + let ephemeron = Ephemeron::::new_empty(self.trace_color.get()); + + let mut alloc = self.allocator.borrow_mut(); + let inner_ptr = alloc.try_alloc(ephemeron)?; + let needs_collect = !alloc.is_below_threshold(); + drop(alloc); + + if needs_collect { + self.collect_needed.set(true); + } + + let eph_ptr = inner_ptr + .as_ptr() + .cast::>>(); + + if self.is_collecting.get() { + self.pending_ephemeron_queue.borrow_mut().push(eph_ptr); + } else { + self.ephemeron_queue.borrow_mut().push(eph_ptr); + } + + Ok(inner_ptr) + } + fn track_weak_map( &self, map: core::ptr::NonNull< diff --git a/oscars/src/collectors/mark_sweep_arena2/pointers/gc.rs b/oscars/src/collectors/mark_sweep_arena2/pointers/gc.rs index cd88f28..1b3b7e8 100644 --- a/oscars/src/collectors/mark_sweep_arena2/pointers/gc.rs +++ b/oscars/src/collectors/mark_sweep_arena2/pointers/gc.rs @@ -38,6 +38,28 @@ impl Gc { gc } + #[must_use] + pub fn new_cyclic_in( + collector: &crate::collectors::mark_sweep_arena2::MarkSweepGarbageCollector, + data_fn: F, + ) -> Self + where + F: FnOnce(&crate::collectors::mark_sweep_arena2::WeakGc) -> T, + { + let weak = unsafe { + crate::collectors::mark_sweep_arena2::WeakGc::from_raw( + collector + .alloc_empty_ephemeron_node::() + .expect("Failed to allocate Ephemeron node") + .extend_lifetime(), + ) + }; + + let gc = Self::new_in(data_fn(&weak), collector); + weak.set_key(&gc); + gc + } + /// Converts a `Gc` into a raw [`ArenaPointer`]. pub fn into_raw(this: Self) -> ArenaPointer<'static, GcBox> { let ptr = this.inner_ptr(); diff --git a/oscars/src/collectors/mark_sweep_arena2/pointers/weak.rs b/oscars/src/collectors/mark_sweep_arena2/pointers/weak.rs index 0438098..d73dd36 100644 --- a/oscars/src/collectors/mark_sweep_arena2/pointers/weak.rs +++ b/oscars/src/collectors/mark_sweep_arena2/pointers/weak.rs @@ -37,4 +37,12 @@ impl WeakGc { pub fn upgrade(&self) -> Option> { self.inner_ptr.as_inner_ref().upgrade() } + + pub(crate) unsafe fn from_raw(inner_ptr: ArenaPointer<'static, Ephemeron>) -> Self { + Self { inner_ptr } + } + + pub(crate) fn set_key(&self, key: &Gc) { + self.inner_ptr.as_inner_ref().set_key(key); + } } diff --git a/oscars/src/collectors/mark_sweep_arena2/tests.rs b/oscars/src/collectors/mark_sweep_arena2/tests.rs index 4cada03..707a74a 100644 --- a/oscars/src/collectors/mark_sweep_arena2/tests.rs +++ b/oscars/src/collectors/mark_sweep_arena2/tests.rs @@ -160,6 +160,25 @@ fn ptr_eq_distinguishes_equal_values() { ); } +#[test] +fn new_cyclic_closure_cannot_upgrade_before_init() { + let collector = &mut MarkSweepGarbageCollector::default() + .with_arena_size(256) + .with_heap_threshold(512); + + let mut saw_none = false; + let gc = Gc::new_cyclic_in(collector, |weak| { + saw_none = weak.upgrade().is_none(); + 7u32 + }); + + assert!( + saw_none, + "weak should not upgrade during new_cyclic construction" + ); + assert_eq!(*gc, 7u32, "new_cyclic should initialize final value"); +} + #[test] fn multi_gc() { let collector = &mut MarkSweepGarbageCollector::default()