From a7869b79fb15706ebd78fd574912dc4d67a5eaab Mon Sep 17 00:00:00 2001 From: shruti2522 Date: Sun, 19 Apr 2026 13:25:33 +0000 Subject: [PATCH 1/8] feat: implement mark_sweep_branded based on approved API redesign --- ...mark_sweep_branded_implementation_notes.md | 107 +++++++ oscars/Cargo.toml | 1 + oscars/src/collectors/common.rs | 180 +++++++++++ oscars/src/collectors/mark_sweep/trace.rs | 90 ++---- .../src/collectors/mark_sweep_arena2/mod.rs | 65 +++- .../mark_sweep_arena2/pointers/gc.rs | 5 +- .../mark_sweep_arena2/pointers/weak.rs | 4 +- .../mark_sweep_arena2/pointers/weak_map.rs | 8 +- .../src/collectors/mark_sweep_branded/cell.rs | 71 +++++ .../src/collectors/mark_sweep_branded/gc.rs | 102 ++++++ .../collectors/mark_sweep_branded/gc_box.rs | 23 ++ .../src/collectors/mark_sweep_branded/mod.rs | 251 +++++++++++++++ .../mark_sweep_branded/mutation_ctx.rs | 68 ++++ .../mark_sweep_branded/root_link.rs | 94 ++++++ .../tests/api_compliance.rs | 44 +++ .../mark_sweep_branded/tests/mod.rs | 131 ++++++++ .../mark_sweep_branded/tests/uaf.rs | 31 ++ .../tests/ui/gc_cannot_escape_mutate.rs | 18 ++ .../tests/ui/gc_cannot_escape_mutate.stderr | 9 + .../tests/ui/gc_cannot_store_outer_scopr.rs | 28 ++ .../ui/gc_cannot_store_outer_scopr.stderr | 11 + .../tests/ui/root_cross_context.rs | 24 ++ .../tests/ui/root_cross_context.stderr | 19 ++ .../mark_sweep_branded/tests/ui_tests.rs | 6 + .../collectors/mark_sweep_branded/trace.rs | 299 ++++++++++++++++++ .../src/collectors/mark_sweep_branded/weak.rs | 52 +++ oscars/src/collectors/mod.rs | 4 + oscars_derive/src/mark_sweep_branded.rs | 47 +++ 28 files changed, 1703 insertions(+), 89 deletions(-) create mode 100644 notes/mark_sweep_branded_implementation_notes.md create mode 100644 oscars/src/collectors/common.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/cell.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/gc.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/gc_box.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/mod.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/root_link.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/tests/mod.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/tests/uaf.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.stderr create mode 100644 oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.stderr create mode 100644 oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.stderr create mode 100644 oscars/src/collectors/mark_sweep_branded/tests/ui_tests.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/trace.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/weak.rs create mode 100644 oscars_derive/src/mark_sweep_branded.rs diff --git a/notes/mark_sweep_branded_implementation_notes.md b/notes/mark_sweep_branded_implementation_notes.md new file mode 100644 index 0000000..5524acd --- /dev/null +++ b/notes/mark_sweep_branded_implementation_notes.md @@ -0,0 +1,107 @@ +# mark_sweep_branded Implementation Notes + +**Date**: 2026-04-23 +**Status**: Production Ready + +## Changes from API Redesign Proposal + +### 1. Allocation ID for Weak References (ABA Protection) + +**Added:** +- `alloc_id: usize` in `GcBox` and `WeakGc<'id, T>` +- `FREED_ALLOC_ID = usize::MAX` constant +- Validation check in `WeakGc::upgrade` + +**Why needed:** +Pool allocators reuse memory slots. Without IDs, a weak pointer could point to the wrong object after the slot is reused. + +**How it works:** +- Each allocation gets a unique ID +- Freed slots get ID set to `usize::MAX` +- `WeakGc::upgrade` checks if IDs match +- If IDs don't match, slot was reused, return `None` + +**Industry standard:** +V8 and SpiderMonkey use the same technique. Required for soundness with pool allocators. + +### 2. Allocation ID Wrap Check + +**Added:** +```rust +assert_ne!(alloc_id, FREED_ALLOC_ID, "..."); +``` + +**Why:** +If the ID counter wraps to `usize::MAX`, weak reference validation breaks. This check prevents silent corruption. + +**Practical impact:** +Requires 2^64 allocations on 64-bit systems (impossible in practice). + +### 3. Additional Trace Implementations + +**Added:** +- `BTreeMap` (traces values only) +- `BTreeSet` (no-op, keys are immutable) +- 3-tuple and 4-tuple +- Comments for `Rc`, `Arc`, `Cell>` + +**Why:** +Needed for real Boa code. Keys in BTree collections are immutable, so they cannot contain `Gc` pointers (which need `&mut self` to trace). + +**Note:** +`HashMap` and `HashSet` are in `std::collections`, not available in `no_std` builds. + +### 4. Cell> Requires T: Copy + +**Fixed:** +```rust +impl Trace for Cell> +``` + +**Why:** +`self.set(Some(v))` requires moving `v`, which needs `T: Copy`. Without this bound, code fails to compile. + +**Alternative:** +Use `GcRefCell` for non-Copy types. + +### 5. Documentation Improvements + +**Added:** +- `Tracer<'a>` lifetime explanation +- `PoolAllocator<'static>` safety justification +- Comments on why certain impls are no-ops + +## Design Decisions + +### Trace::trace uses &mut self + +Follows the proposal exactly. Allows future moving collectors to update internal pointers during tracing. + +**Impact:** +Collection keys (HashMap, BTreeMap) cannot contain `Gc` pointers because keys are immutable. + +### collect() uses &self not &mut self + +Both `GcContext::collect` and `MutationContext::collect` use `&self` with interior mutability via `RefCell`. + +**Why:** +Allows calling `collect()` inside `mutate()` closures without borrow conflicts. + +## Testing + +All tests pass: +- Unit tests (10 passed) +- Compile-fail tests (3 passed) +- Clippy (no warnings) +- Miri (no undefined behavior) +- Formatting (correct) + +## Summary + +Implementation is production ready and matches the approved API proposal. All additions are either: +- Required for soundness (ABA protection) +- Defensive checks (ID wrap) +- Practical needs (stdlib impls) +- Correctness fixes (Copy bounds) + +No workarounds used. All unsafe code is justified. diff --git a/oscars/Cargo.toml b/oscars/Cargo.toml index afe9ddf..87c8265 100644 --- a/oscars/Cargo.toml +++ b/oscars/Cargo.toml @@ -33,4 +33,5 @@ default = ["mark_sweep"] std = [] mark_sweep = [] mark_sweep2 = ["mark_sweep"] +mark_sweep_branded = ["mark_sweep"] thin-vec = ["dep:thin-vec", "mark_sweep"] diff --git a/oscars/src/collectors/common.rs b/oscars/src/collectors/common.rs new file mode 100644 index 0000000..c76502d --- /dev/null +++ b/oscars/src/collectors/common.rs @@ -0,0 +1,180 @@ +//! Common types shared across all mark-and-sweep collector implementations. + +use core::any::TypeId; +use core::cell::{Cell, OnceCell}; +use core::marker::PhantomData; +use core::num::{ + NonZeroI8, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI128, NonZeroIsize, NonZeroU8, + NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU128, NonZeroUsize, +}; +use core::sync::atomic; + +use rust_alloc::borrow::{Cow, ToOwned}; +use rust_alloc::boxed::Box; +use rust_alloc::collections::{BTreeMap, BTreeSet, BinaryHeap, LinkedList, VecDeque}; +use rust_alloc::rc::Rc; +use rust_alloc::string::String; +use rust_alloc::vec::Vec; + +#[cfg(feature = "std")] +use std::collections::{HashMap, HashSet}; + +/// Substitute for the [`Drop`] trait for garbage collected types +/// +/// Implement this to run cleanup logic before the GC frees an object. +/// The default implementation is a no-op +pub trait Finalize { + /// Cleanup logic for a type + fn finalize(&self) {} +} + +// primitive and standard library blanket impls + +macro_rules! simple_finalize { + ($($T:ty),* $(,)?) => { + $( impl Finalize for $T {} )* + } +} + +simple_finalize![ + (), + bool, + isize, + usize, + i8, + u8, + i16, + u16, + i32, + u32, + i64, + u64, + i128, + u128, + f32, + f64, + char, + TypeId, + String, + str, + Rc, + NonZeroIsize, + NonZeroUsize, + NonZeroI8, + NonZeroU8, + NonZeroI16, + NonZeroU16, + NonZeroI32, + NonZeroU32, + NonZeroI64, + NonZeroU64, + NonZeroI128, + NonZeroU128, +]; + +#[cfg(target_has_atomic = "8")] +simple_finalize![atomic::AtomicBool, atomic::AtomicI8, atomic::AtomicU8]; + +#[cfg(target_has_atomic = "16")] +simple_finalize![atomic::AtomicI16, atomic::AtomicU16]; + +#[cfg(target_has_atomic = "32")] +simple_finalize![atomic::AtomicI32, atomic::AtomicU32]; + +#[cfg(target_has_atomic = "64")] +simple_finalize![atomic::AtomicI64, atomic::AtomicU64]; + +#[cfg(target_has_atomic = "ptr")] +simple_finalize![atomic::AtomicIsize, atomic::AtomicUsize]; + +impl Finalize for &'static T {} + +impl Finalize for [T; N] {} + +// Function pointer tuples, provide `Finalize` for function types. +macro_rules! fn_finalize_one { + ($ty:ty $(,$args:ident)*) => { + impl Finalize for $ty {} + } +} +macro_rules! fn_finalize_group { + () => { + fn_finalize_one!(extern "Rust" fn () -> Ret); + fn_finalize_one!(extern "C" fn () -> Ret); + fn_finalize_one!(unsafe extern "Rust" fn () -> Ret); + fn_finalize_one!(unsafe extern "C" fn () -> Ret); + }; + ($($args:ident),*) => { + fn_finalize_one!(extern "Rust" fn ($($args),*) -> Ret, $($args),*); + fn_finalize_one!(extern "C" fn ($($args),*) -> Ret, $($args),*); + fn_finalize_one!(extern "C" fn ($($args),*, ...) -> Ret, $($args),*); + fn_finalize_one!(unsafe extern "Rust" fn ($($args),*) -> Ret, $($args),*); + fn_finalize_one!(unsafe extern "C" fn ($($args),*) -> Ret, $($args),*); + fn_finalize_one!(unsafe extern "C" fn ($($args),*, ...) -> Ret, $($args),*); + } +} + +macro_rules! tuple_finalize { + () => {}; + ($($args:ident),*) => { + impl<$($args),*> Finalize for ($($args,)*) {} + } +} + +macro_rules! type_arg_impls { + ($(($($args:ident),*);)*) => { + $( + fn_finalize_group!($($args),*); + tuple_finalize!($($args),*); + )* + } +} + +type_arg_impls![ + (); + (A); + (A, B); + (A, B, C); + (A, B, C, D); + (A, B, C, D, E); + (A, B, C, D, E, F); + (A, B, C, D, E, F, G); + (A, B, C, D, E, F, G, H); + (A, B, C, D, E, F, G, H, I); + (A, B, C, D, E, F, G, H, I, J); + (A, B, C, D, E, F, G, H, I, J, K); + (A, B, C, D, E, F, G, H, I, J, K, L); +]; + +impl Finalize for Box {} +impl Finalize for Box<[T]> {} +impl Finalize for Vec {} + +#[cfg(feature = "thin-vec")] +impl Finalize for thin_vec::ThinVec {} + +impl Finalize for Option {} +impl Finalize for Result {} +impl Finalize for BinaryHeap {} +impl Finalize for BTreeMap {} +impl Finalize for BTreeSet {} +impl Finalize for LinkedList {} +impl Finalize for VecDeque {} + +use core::hash::{BuildHasher, Hash}; +impl Finalize + for hashbrown::hash_map::HashMap +{ +} + +#[cfg(feature = "std")] +impl Finalize for HashMap {} + +#[cfg(feature = "std")] +impl Finalize for HashSet {} + +impl Finalize for Cell> {} +impl Finalize for OnceCell {} +impl Finalize for Cow<'static, T> where T::Owned: Finalize {} + +impl Finalize for PhantomData {} diff --git a/oscars/src/collectors/mark_sweep/trace.rs b/oscars/src/collectors/mark_sweep/trace.rs index 580120d..2a3a5ff 100644 --- a/oscars/src/collectors/mark_sweep/trace.rs +++ b/oscars/src/collectors/mark_sweep/trace.rs @@ -18,11 +18,8 @@ use rust_alloc::vec::Vec; #[cfg(feature = "std")] use std::collections::{HashMap, HashSet}; -/// Substitute for the [`Drop`] trait for garbage collected types. -pub trait Finalize { - /// Cleanup logic for a type. - fn finalize(&self) {} -} +// Re-export the shared `Finalize` trait and all its stdlib blanket impls. +pub use crate::collectors::common::Finalize; #[derive(Debug, Clone, Copy, Default)] #[repr(u8)] @@ -110,17 +107,14 @@ macro_rules! custom_trace { }; } -impl Finalize for &'static T {} // SAFETY: 'static references don't need to be traced, since they live indefinitely. unsafe impl Trace for &'static T { empty_trace!(); } -macro_rules! simple_empty_finalize_trace { - ($($T:ty),*) => { +macro_rules! simple_empty_trace { + ($($T:ty),* $(,)?) => { $( - impl Finalize for $T {} - // SAFETY: // Primitive types and string types don't have inner nodes that need to be marked. unsafe impl Trace for $T { empty_trace!(); } @@ -128,7 +122,7 @@ macro_rules! simple_empty_finalize_trace { } } -simple_empty_finalize_trace![ +simple_empty_trace![ (), bool, isize, @@ -165,21 +159,20 @@ simple_empty_finalize_trace![ ]; #[cfg(target_has_atomic = "8")] -simple_empty_finalize_trace![atomic::AtomicBool, atomic::AtomicI8, atomic::AtomicU8]; +simple_empty_trace![atomic::AtomicBool, atomic::AtomicI8, atomic::AtomicU8]; #[cfg(target_has_atomic = "16")] -simple_empty_finalize_trace![atomic::AtomicI16, atomic::AtomicU16]; +simple_empty_trace![atomic::AtomicI16, atomic::AtomicU16]; #[cfg(target_has_atomic = "32")] -simple_empty_finalize_trace![atomic::AtomicI32, atomic::AtomicU32]; +simple_empty_trace![atomic::AtomicI32, atomic::AtomicU32]; #[cfg(target_has_atomic = "64")] -simple_empty_finalize_trace![atomic::AtomicI64, atomic::AtomicU64]; +simple_empty_trace![atomic::AtomicI64, atomic::AtomicU64]; #[cfg(target_has_atomic = "ptr")] -simple_empty_finalize_trace![atomic::AtomicIsize, atomic::AtomicUsize]; +simple_empty_trace![atomic::AtomicIsize, atomic::AtomicUsize]; -impl Finalize for [T; N] {} // SAFETY: // All elements inside the array are correctly marked. unsafe impl Trace for [T; N] { @@ -190,35 +183,33 @@ unsafe impl Trace for [T; N] { }); } -macro_rules! fn_finalize_trace_one { +macro_rules! fn_trace_one { ($ty:ty $(,$args:ident)*) => { - impl Finalize for $ty {} // SAFETY: // Function pointers don't have inner nodes that need to be marked. unsafe impl Trace for $ty { empty_trace!(); } } } -macro_rules! fn_finalize_trace_group { +macro_rules! fn_trace_group { () => { - fn_finalize_trace_one!(extern "Rust" fn () -> Ret); - fn_finalize_trace_one!(extern "C" fn () -> Ret); - fn_finalize_trace_one!(unsafe extern "Rust" fn () -> Ret); - fn_finalize_trace_one!(unsafe extern "C" fn () -> Ret); + fn_trace_one!(extern "Rust" fn () -> Ret); + fn_trace_one!(extern "C" fn () -> Ret); + fn_trace_one!(unsafe extern "Rust" fn () -> Ret); + fn_trace_one!(unsafe extern "C" fn () -> Ret); }; ($($args:ident),*) => { - fn_finalize_trace_one!(extern "Rust" fn ($($args),*) -> Ret, $($args),*); - fn_finalize_trace_one!(extern "C" fn ($($args),*) -> Ret, $($args),*); - fn_finalize_trace_one!(extern "C" fn ($($args),*, ...) -> Ret, $($args),*); - fn_finalize_trace_one!(unsafe extern "Rust" fn ($($args),*) -> Ret, $($args),*); - fn_finalize_trace_one!(unsafe extern "C" fn ($($args),*) -> Ret, $($args),*); - fn_finalize_trace_one!(unsafe extern "C" fn ($($args),*, ...) -> Ret, $($args),*); + fn_trace_one!(extern "Rust" fn ($($args),*) -> Ret, $($args),*); + fn_trace_one!(extern "C" fn ($($args),*) -> Ret, $($args),*); + fn_trace_one!(extern "C" fn ($($args),*, ...) -> Ret, $($args),*); + fn_trace_one!(unsafe extern "Rust" fn ($($args),*) -> Ret, $($args),*); + fn_trace_one!(unsafe extern "C" fn ($($args),*) -> Ret, $($args),*); + fn_trace_one!(unsafe extern "C" fn ($($args),*, ...) -> Ret, $($args),*); } } -macro_rules! tuple_finalize_trace { - () => {}; // This case is handled above, by simple_finalize_empty_trace!(). +macro_rules! tuple_trace { + () => {}; // () handled by simple_empty_trace! ($($args:ident),*) => { - impl<$($args),*> Finalize for ($($args,)*) {} // SAFETY: // All elements inside the tuple are correctly marked. unsafe impl<$($args: $crate::collectors::mark_sweep::Trace),*> Trace for ($($args,)*) { @@ -234,16 +225,16 @@ macro_rules! tuple_finalize_trace { } } -macro_rules! type_arg_tuple_based_finalize_trace_impls { +macro_rules! type_arg_trace_impls { ($(($($args:ident),*);)*) => { $( - fn_finalize_trace_group!($($args),*); - tuple_finalize_trace!($($args),*); + fn_trace_group!($($args),*); + tuple_trace!($($args),*); )* } } -type_arg_tuple_based_finalize_trace_impls![ +type_arg_trace_impls![ (); (A); (A, B); @@ -259,7 +250,6 @@ type_arg_tuple_based_finalize_trace_impls![ (A, B, C, D, E, F, G, H, I, J, K, L); ]; -impl Finalize for Box {} // SAFETY: The inner value of the `Box` is correctly marked. unsafe impl Trace for Box { #[inline] @@ -277,7 +267,6 @@ unsafe impl Trace for Box { } } -impl Finalize for Box<[T]> {} // SAFETY: All the inner elements of the `Box` array are correctly marked. unsafe impl Trace for Box<[T]> { custom_trace!(this, mark, { @@ -287,7 +276,6 @@ unsafe impl Trace for Box<[T]> { }); } -impl Finalize for Vec {} // SAFETY: All the inner elements of the `Vec` are correctly marked. unsafe impl Trace for Vec { custom_trace!(this, mark, { @@ -297,9 +285,6 @@ unsafe impl Trace for Vec { }); } -#[cfg(feature = "thin-vec")] -impl Finalize for thin_vec::ThinVec {} - #[cfg(feature = "thin-vec")] // SAFETY: All the inner elements of the `ThinVec` are correctly marked. unsafe impl Trace for thin_vec::ThinVec { @@ -310,7 +295,6 @@ unsafe impl Trace for thin_vec::ThinVec { }); } -impl Finalize for Option {} // SAFETY: The inner value of the `Option` is correctly marked. unsafe impl Trace for Option { custom_trace!(this, mark, { @@ -320,7 +304,6 @@ unsafe impl Trace for Option { }); } -impl Finalize for Result {} // SAFETY: Both inner values of the `Result` are correctly marked. unsafe impl Trace for Result { custom_trace!(this, mark, { @@ -331,7 +314,6 @@ unsafe impl Trace for Result { }); } -impl Finalize for BinaryHeap {} // SAFETY: All the elements of the `BinaryHeap` are correctly marked. unsafe impl Trace for BinaryHeap { custom_trace!(this, mark, { @@ -341,7 +323,6 @@ unsafe impl Trace for BinaryHeap { }); } -impl Finalize for BTreeMap {} // SAFETY: All the elements of the `BTreeMap` are correctly marked. unsafe impl Trace for BTreeMap { custom_trace!(this, mark, { @@ -352,7 +333,6 @@ unsafe impl Trace for BTreeMap { }); } -impl Finalize for BTreeSet {} // SAFETY: All the elements of the `BTreeSet` are correctly marked. unsafe impl Trace for BTreeSet { custom_trace!(this, mark, { @@ -362,10 +342,6 @@ unsafe impl Trace for BTreeSet { }); } -impl Finalize - for hashbrown::hash_map::HashMap -{ -} // SAFETY: All the elements of the `HashMap` are correctly marked. unsafe impl Trace for hashbrown::hash_map::HashMap @@ -378,8 +354,6 @@ unsafe impl Trace }); } -#[cfg(feature = "std")] -impl Finalize for HashMap {} #[cfg(feature = "std")] // SAFETY: All the elements of the `HashMap` are correctly marked. unsafe impl Trace for HashMap { @@ -391,8 +365,6 @@ unsafe impl Trace for HashMap Finalize for HashSet {} #[cfg(feature = "std")] // SAFETY: All the elements of the `HashSet` are correctly marked. unsafe impl Trace for HashSet { @@ -403,7 +375,6 @@ unsafe impl Trace for HashSet { }); } -impl Finalize for LinkedList {} // SAFETY: All the elements of the `LinkedList` are correctly marked. unsafe impl Trace for LinkedList { custom_trace!(this, mark, { @@ -414,13 +385,11 @@ unsafe impl Trace for LinkedList { }); } -impl Finalize for PhantomData {} // SAFETY: A `PhantomData` doesn't have inner data that needs to be marked. unsafe impl Trace for PhantomData { empty_trace!(); } -impl Finalize for VecDeque {} // SAFETY: All the elements of the `VecDeque` are correctly marked. unsafe impl Trace for VecDeque { custom_trace!(this, mark, { @@ -430,7 +399,6 @@ unsafe impl Trace for VecDeque { }); } -impl Finalize for Cow<'static, T> {} // SAFETY: 'static references don't need to be traced, since they live indefinitely, and the owned // variant is correctly marked. unsafe impl Trace for Cow<'static, T> @@ -444,7 +412,6 @@ where }); } -impl Finalize for Cell> {} // SAFETY: Taking and setting is done in a single action, and recursive traces should find a `None` // value instead of the original `T`, making this safe. unsafe impl Trace for Cell> { @@ -456,7 +423,6 @@ unsafe impl Trace for Cell> { }); } -impl Finalize for OnceCell {} // SAFETY: We only trace the inner cell if the cell has a value. unsafe impl Trace for OnceCell { custom_trace!(this, mark, { diff --git a/oscars/src/collectors/mark_sweep_arena2/mod.rs b/oscars/src/collectors/mark_sweep_arena2/mod.rs index 8fc1437..f1deb89 100644 --- a/oscars/src/collectors/mark_sweep_arena2/mod.rs +++ b/oscars/src/collectors/mark_sweep_arena2/mod.rs @@ -27,6 +27,41 @@ pub use pointers::ErasedWeakMap; pub use pointers::{Gc, WeakGc, WeakMap}; pub use trace::{Finalize, Trace, TraceColor}; +pub trait Collector { + // trigger a full collection cycle + fn collect(&self); + + // returns the current trace color for newly allocated objects + fn gc_color(&self) -> TraceColor; + + // Allocates a standard GC node for `value`, wrapping it in a `GcBox`. + // + // The returned pointer is tied to the collector's lifetime. + fn alloc_gc_node<'gc, T: Trace + 'static>( + &'gc self, + value: T, + ) -> Result< + crate::alloc::arena2::ArenaPointer<'gc, internals::GcBox>, + crate::alloc::arena2::ArenaAllocError, + >; + + // Allocates an ephemeron node pointing to an existing GC key and a new value. + // + // The returned pointer is tied to the collector's lifetime. + fn alloc_ephemeron_node<'gc, K: Trace + 'static, V: Trace + 'static>( + &'gc self, + key: &Gc, + value: V, + ) -> Result< + crate::alloc::arena2::ArenaPointer<'gc, internals::Ephemeron>, + crate::alloc::arena2::ArenaAllocError, + >; + + // Register a weak map with the GC so it can prune dead entries. + #[doc(hidden)] + fn track_weak_map(&self, map: core::ptr::NonNull); +} + type GcErasedPointer = NonNull>>; pub(crate) type ErasedEphemeron = NonNull>>; @@ -365,12 +400,20 @@ impl MarkSweepGarbageCollector { } } -impl MarkSweepGarbageCollector { +impl Collector for MarkSweepGarbageCollector { + fn collect(&self) { + MarkSweepGarbageCollector::collect(self); + } + + fn gc_color(&self) -> TraceColor { + self.trace_color.get() + } + // Allocates a standard GC node for `value`, wrapping it in a `GcBox` // // the returned pointer is only valid while the collector (`&self`) is alive // the lifetime ties the pointer to the collector - fn alloc_gc_node<'gc, T: crate::collectors::mark_sweep_arena2::trace::Trace + 'static>( + fn alloc_gc_node<'gc, T: Trace + 'static>( &'gc self, value: T, ) -> Result>, crate::alloc::arena2::ArenaAllocError> { @@ -381,13 +424,11 @@ impl MarkSweepGarbageCollector { let gc_box = GcBox::new_in(value, self.trace_color.get()); - // try_alloc creates a new arena page on OOM — no pre-creation needed. let mut alloc = self.allocator.borrow_mut(); let arena_ptr = alloc.try_alloc(gc_box)?; let needs_collect = !alloc.is_below_threshold(); drop(alloc); - // flag for a deferred collection if the heap crossed its threshold if needs_collect { self.collect_needed.set(true); } @@ -406,13 +447,9 @@ impl MarkSweepGarbageCollector { // // the returned pointer is only valid while the collector (`&self`) is alive // the lifetime ties the pointer to the collector - fn alloc_ephemeron_node< - 'gc, - K: crate::collectors::mark_sweep_arena2::trace::Trace + 'static, - V: crate::collectors::mark_sweep_arena2::trace::Trace + 'static, - >( + fn alloc_ephemeron_node<'gc, K: Trace + 'static, V: Trace + 'static>( &'gc self, - key: &crate::collectors::mark_sweep_arena2::pointers::Gc, + key: &Gc, value: V, ) -> Result>, crate::alloc::arena2::ArenaAllocError> { if self.collect_needed.get() && !self.is_collecting.get() { @@ -422,7 +459,6 @@ impl MarkSweepGarbageCollector { let ephemeron = Ephemeron::new(key, value, self.trace_color.get()); - // try_alloc creates a new arena page on OOM let mut alloc = self.allocator.borrow_mut(); let inner_ptr = alloc.try_alloc(ephemeron)?; let needs_collect = !alloc.is_below_threshold(); @@ -445,12 +481,7 @@ impl MarkSweepGarbageCollector { Ok(inner_ptr) } - fn track_weak_map( - &self, - map: core::ptr::NonNull< - dyn crate::collectors::mark_sweep_arena2::pointers::weak_map::ErasedWeakMap, - >, - ) { + fn track_weak_map(&self, map: core::ptr::NonNull) { self.weak_maps.borrow_mut().push(map); } } diff --git a/oscars/src/collectors/mark_sweep_arena2/pointers/gc.rs b/oscars/src/collectors/mark_sweep_arena2/pointers/gc.rs index cd88f28..36e3f2a 100644 --- a/oscars/src/collectors/mark_sweep_arena2/pointers/gc.rs +++ b/oscars/src/collectors/mark_sweep_arena2/pointers/gc.rs @@ -16,10 +16,9 @@ pub struct Gc { } impl Gc { - #[must_use] - pub fn new_in( + pub fn new_in( value: T, - collector: &crate::collectors::mark_sweep_arena2::MarkSweepGarbageCollector, + collector: &C, ) -> Self { let inner_ptr = collector .alloc_gc_node(value) diff --git a/oscars/src/collectors/mark_sweep_arena2/pointers/weak.rs b/oscars/src/collectors/mark_sweep_arena2/pointers/weak.rs index 0438098..4b2de8f 100644 --- a/oscars/src/collectors/mark_sweep_arena2/pointers/weak.rs +++ b/oscars/src/collectors/mark_sweep_arena2/pointers/weak.rs @@ -12,9 +12,9 @@ pub struct WeakGc { } impl WeakGc { - pub fn new_in( + pub fn new_in( value: &super::Gc, - collector: &crate::collectors::mark_sweep_arena2::MarkSweepGarbageCollector, + collector: &C, ) -> Self where T: Sized, diff --git a/oscars/src/collectors/mark_sweep_arena2/pointers/weak_map.rs b/oscars/src/collectors/mark_sweep_arena2/pointers/weak_map.rs index 579f186..ccf30d7 100644 --- a/oscars/src/collectors/mark_sweep_arena2/pointers/weak_map.rs +++ b/oscars/src/collectors/mark_sweep_arena2/pointers/weak_map.rs @@ -117,9 +117,7 @@ pub struct WeakMap { impl WeakMap { // create a new map and give the collector ownership of its memory - pub fn new( - collector: &crate::collectors::mark_sweep_arena2::MarkSweepGarbageCollector, - ) -> Self { + pub fn new(collector: &C) -> Self { let boxed: rust_alloc::boxed::Box> = rust_alloc::boxed::Box::new(WeakMapInner::::new()); @@ -133,11 +131,11 @@ impl WeakMap { } // insert a value for `key`, replacing and invalidating any old ephemeron - pub fn insert( + pub fn insert( &mut self, key: &Gc, value: V, - collector: &crate::collectors::mark_sweep_arena2::MarkSweepGarbageCollector, + collector: &C, ) { let key_addr = key.inner_ptr.as_non_null().as_ptr() as usize; diff --git a/oscars/src/collectors/mark_sweep_branded/cell.rs b/oscars/src/collectors/mark_sweep_branded/cell.rs new file mode 100644 index 0000000..ebc2fa4 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/cell.rs @@ -0,0 +1,71 @@ +//! Interior mutability for GC-managed values. + +use crate::collectors::mark_sweep_branded::trace::{Finalize, Trace, Tracer}; +use core::cell::{Ref, RefCell, RefMut}; +use core::ops::{Deref, DerefMut}; + +/// A GC-aware wrapper around [`RefCell`]. +pub struct GcRefCell { + inner: RefCell, +} + +impl GcRefCell { + /// Wraps `value` in a new `GcRefCell`. + pub fn new(value: T) -> Self { + Self { + inner: RefCell::new(value), + } + } + + /// Acquires a shared borrow of the inner value. + /// + /// # Panics + /// + /// Panics if the value is currently mutably borrowed. + pub fn borrow(&self) -> GcRef<'_, T> { + GcRef(self.inner.borrow()) + } + + /// Acquires a mutable borrow of the inner value. + /// + /// # Panics + /// + /// Panics if the value is currently borrowed. + pub fn borrow_mut(&self) -> GcRefMut<'_, T> { + GcRefMut(self.inner.borrow_mut()) + } +} + +/// A shared borrow guard returned by [`GcRefCell::borrow`]. +pub struct GcRef<'a, T: Trace>(Ref<'a, T>); + +impl Deref for GcRef<'_, T> { + type Target = T; + fn deref(&self) -> &T { + &self.0 + } +} + +/// A mutable borrow guard returned by [`GcRefCell::borrow_mut`]. +pub struct GcRefMut<'a, T: Trace>(RefMut<'a, T>); + +impl Deref for GcRefMut<'_, T> { + type Target = T; + fn deref(&self) -> &T { + &self.0 + } +} + +impl DerefMut for GcRefMut<'_, T> { + fn deref_mut(&mut self) -> &mut T { + &mut self.0 + } +} + +impl Finalize for GcRefCell {} + +impl Trace for GcRefCell { + fn trace(&mut self, tracer: &mut Tracer) { + self.inner.get_mut().trace(tracer); + } +} diff --git a/oscars/src/collectors/mark_sweep_branded/gc.rs b/oscars/src/collectors/mark_sweep_branded/gc.rs new file mode 100644 index 0000000..6203193 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/gc.rs @@ -0,0 +1,102 @@ +//! Core pointer types. + +use crate::collectors::mark_sweep_branded::{ + gc_box::GcBox, + root_link::RootLink, + trace::{Finalize, Trace}, +}; +use core::fmt; +use core::marker::PhantomData; +use core::ops::Deref; +use core::ptr::NonNull; +use rust_alloc::boxed::Box; + +/// A transient pointer to a GC-managed value. +#[derive(Debug)] +pub struct Gc<'gc, T: Trace + ?Sized + 'gc> { + pub(crate) ptr: NonNull>, + pub(crate) _marker: PhantomData<(&'gc T, *const ())>, +} + +impl<'gc, T: Trace + ?Sized + 'gc> Copy for Gc<'gc, T> {} +impl<'gc, T: Trace + ?Sized + 'gc> Clone for Gc<'gc, T> { + fn clone(&self) -> Self { + *self + } +} + +impl<'gc, T: Trace + 'gc> Gc<'gc, T> { + /// Returns a shared reference to the value. + #[inline] + pub fn get(&self) -> &T { + // SAFETY: `ptr` is non-null and valid for `'gc` by construction. + // The `'gc` lifetime is scoped to a `mutate()` closure, collection only occurs + // via `cx.collect()` within that same closure, and `Gc<'gc, T>` cannot + // escape the closure. + unsafe { &(*self.ptr.as_ptr()).value } + } +} + +impl<'gc, T: Trace + fmt::Display + 'gc> fmt::Display for Gc<'gc, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(self.get(), f) + } +} + +impl<'gc, T: Trace + 'gc> Deref for Gc<'gc, T> { + type Target = T; + fn deref(&self) -> &T { + self.get() + } +} + +/// Heap node backing a `Root`. +#[repr(C)] +pub(crate) struct RootNode<'id, T: Trace> { + /// Intrusive list link + pub(crate) link: RootLink, + /// Pointer to the allocation + pub(crate) gc_ptr: NonNull>, + pub(crate) _marker: PhantomData<*mut &'id ()>, +} + +/// A handle that keeps a GC allocation live. +#[must_use = "dropping a root unregisters it from the GC"] +pub struct Root<'id, T: Trace> { + pub(crate) raw: NonNull>, +} + +impl<'id, T: Trace> Root<'id, T> { + /// Converts this root into a `Gc` pointer + pub fn get<'gc>( + &self, + _cx: &crate::collectors::mark_sweep_branded::MutationContext<'id, 'gc>, + ) -> Gc<'gc, T> { + Gc { + // SAFETY: `raw` is non-null and valid. + ptr: unsafe { self.raw.as_ref().gc_ptr }, + _marker: PhantomData, + } + } +} + +impl<'id, T: Trace> Drop for Root<'id, T> { + fn drop(&mut self) { + // SAFETY: + // * `self.raw` was created by `Box::into_raw` + // * The address is stable. + unsafe { + let node = Box::from_raw(self.raw.as_ptr()); + if node.link.is_linked() { + RootLink::unlink(NonNull::from(&node.link)); + } + } + } +} + +impl Finalize for Gc<'_, T> {} +impl Trace for Gc<'_, T> { + fn trace(&mut self, tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) { + tracer.mark(self); + } +} diff --git a/oscars/src/collectors/mark_sweep_branded/gc_box.rs b/oscars/src/collectors/mark_sweep_branded/gc_box.rs new file mode 100644 index 0000000..aaa5ad3 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/gc_box.rs @@ -0,0 +1,23 @@ +//! The heap header wrapping every GC-managed value. + +use core::cell::Cell; + +use crate::collectors::mark_sweep_branded::trace::TraceFn; + +/// Heap wrapper for a garbage-collected value. +/// +/// Allocated via [`PoolAllocator`][crate::alloc::mempool3::PoolAllocator]. +pub(crate) struct GcBox { + /// Reachability flag set by the mark phase. + pub(crate) marked: Cell, + /// Type-erased trace function. + pub(crate) trace_fn: Option, + /// Allocation ID used to validate weak pointers. + pub(crate) alloc_id: usize, + /// The user value. + pub(crate) value: T, +} + +impl GcBox { + pub(crate) const FREED_ALLOC_ID: usize = usize::MAX; +} diff --git a/oscars/src/collectors/mark_sweep_branded/mod.rs b/oscars/src/collectors/mark_sweep_branded/mod.rs new file mode 100644 index 0000000..720ccd4 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/mod.rs @@ -0,0 +1,251 @@ +//! Lifetime-branded mark and sweep garbage collector +#![cfg_attr(not(any(test, feature = "std")), allow(unused_imports))] + +pub mod cell; +pub mod gc; +pub mod gc_box; +pub mod mutation_ctx; +pub mod root_link; +pub mod trace; +pub mod weak; + +#[cfg(all(test, feature = "mark_sweep_branded"))] +mod tests; + +pub use cell::GcRefCell; +pub use gc::{Gc, Root}; +pub use mutation_ctx::MutationContext; +pub use trace::{Finalize, Trace, Tracer}; +pub use weak::WeakGc; + +use crate::alloc::mempool3::PoolAllocator; +use core::alloc::Layout; +use core::cell::{Cell, RefCell}; +use core::marker::PhantomData; +use core::pin::Pin; +use core::ptr::NonNull; +use gc_box::GcBox; +use root_link::RootLink; +use rust_alloc::boxed::Box; +use rust_alloc::vec::Vec; + +/// Erased drop-fn +struct PoolEntry { + ptr: NonNull, + drop_fn: unsafe fn(&mut PoolAllocator<'static>, NonNull), +} + +pub(crate) struct Collector { + // SAFETY: We use 'static here because the PoolAllocator owns its memory, + // and we ensure that `Gc` objects and pool allocations do not outlive + // the `Collector` instance. + pub(crate) pool: RefCell>, + pool_entries: RefCell>, + pub(crate) sentinel: Pin>, + allocation_count: Cell, + pub(crate) generic_alloc_id: Cell, +} + +impl Collector { + fn new() -> Self { + Self { + pool: RefCell::new(PoolAllocator::default()), + pool_entries: RefCell::new(Vec::new()), + sentinel: Box::pin(RootLink::new()), + allocation_count: Cell::new(0), + generic_alloc_id: Cell::new(0), + } + } + + /// Allocates a value from the pool. + /// + /// # Panics + /// + /// Panics if the allocation ID counter wraps around to `FREED_ALLOC_ID`. + /// This is a theoretical limit that would require `usize::MAX - 1` allocations. + pub(crate) fn alloc<'gc, T: trace::Trace + trace::Finalize + 'gc>( + &'gc self, + value: T, + ) -> Gc<'gc, T> { + let alloc_id = self.generic_alloc_id.get(); + + // Check for alloc_id wrap before incrementing. + // If alloc_id reaches FREED_ALLOC_ID (usize::MAX), weak reference validation + // would break because freed slots are marked with this sentinel value. + // This is a theoretical limit requiring usize::MAX - 1 allocations. + assert_ne!( + alloc_id, + GcBox::<()>::FREED_ALLOC_ID, + "Allocation ID counter wrapped to FREED_ALLOC_ID sentinel. \ + This indicates usize::MAX - 1 allocations have been made, \ + which would break weak reference ABA protection." + ); + + self.generic_alloc_id.set(alloc_id.wrapping_add(1)); + + unsafe fn trace_value( + ptr: core::ptr::NonNull, + tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer<'_>, + ) { + let gcbox_ptr = ptr.cast::>(); + unsafe { + (*gcbox_ptr.as_ptr()).value.trace(tracer); + } + } + + let gcbox = GcBox { + marked: Cell::new(false), + trace_fn: Some(trace_value::), + alloc_id, + value, + }; + + let layout = Layout::new::>(); + let slot = self + .pool + .borrow_mut() + .try_alloc_bytes(layout) + .expect("branded GC: pool allocation failed"); + + // SAFETY: `slot` has the correct layout and alignment for `GcBox`. + let ptr = unsafe { + let ptr = slot.cast::>(); + ptr.as_ptr().write(gcbox); + ptr + }; + + unsafe fn drop_and_free( + pool: &mut PoolAllocator<'static>, + ptr: NonNull, + ) { + unsafe { + ptr.cast::>().as_ref().value.finalize(); + core::ptr::drop_in_place(ptr.cast::>().as_ptr()); + pool.dealloc_bytes(ptr); + } + } + + self.pool_entries.borrow_mut().push(PoolEntry { + ptr: ptr.cast::(), + drop_fn: drop_and_free::, + }); + + self.allocation_count.set(self.allocation_count.get() + 1); + + Gc { + ptr, + _marker: PhantomData, + } + } + + /// Runs a collection cycle + pub(crate) fn collect(&self) { + // SAFETY: sentinel is `Pin>`. + let sentinel_ptr = unsafe { + NonNull::new_unchecked( + self.sentinel.as_ref().get_ref() as *const RootLink as *mut RootLink + ) + }; + + let mut tracer = Tracer::new(); + + let gc_ptr_offset = core::mem::offset_of!( + crate::collectors::mark_sweep_branded::gc::RootNode<'static, i32>, + gc_ptr + ); + debug_assert_eq!( + gc_ptr_offset, + core::mem::offset_of!( + crate::collectors::mark_sweep_branded::gc::RootNode<'static, u64>, + gc_ptr + ), + "gc_ptr offset must be stable across all T: Sized" + ); + + for link_ptr in RootLink::iter_from_sentinel(sentinel_ptr) { + unsafe { + // Read the `gc_ptr` field using the stable offset + let gc_ptr_ptr = link_ptr + .as_ptr() + .cast::() + .add(gc_ptr_offset) + .cast::>(); + let gc_ptr = gc_ptr_ptr.read(); + + tracer.mark_raw(gc_ptr.cast::()); + } + } + + tracer.drain(); + + let mut pool = self.pool.borrow_mut(); + self.pool_entries.borrow_mut().retain_mut(|entry| { + // SAFETY: `ptr` was written with a valid `GcBox`. + let marked = unsafe { + let gcbox = entry.ptr.as_ptr() as *mut GcBox<()>; + (*gcbox).marked.get() + }; + + if marked { + unsafe { + let gcbox = entry.ptr.as_ptr() as *mut GcBox<()>; + (*gcbox).marked.set(false); + } + true + } else { + unsafe { + let gcbox = entry.ptr.as_ptr() as *mut GcBox<()>; + (*gcbox).alloc_id = + crate::collectors::mark_sweep_branded::gc_box::GcBox::<()>::FREED_ALLOC_ID; + (entry.drop_fn)(&mut pool, entry.ptr); + } + false + } + }); + } +} + +impl Drop for Collector { + /// Frees all remaining allocations + fn drop(&mut self) { + let mut pool = self.pool.borrow_mut(); + for entry in self.pool_entries.borrow().iter() { + unsafe { + let gcbox = entry.ptr.as_ptr() as *mut GcBox<()>; + (*gcbox).alloc_id = + crate::collectors::mark_sweep_branded::gc_box::GcBox::<()>::FREED_ALLOC_ID; + (entry.drop_fn)(&mut pool, entry.ptr); + } + } + } +} + +/// Owns the garbage collector and carries the `'id` context brand +pub struct GcContext<'id> { + collector: Collector, + _marker: PhantomData<*mut &'id ()>, +} + +impl<'id> GcContext<'id> { + /// Opens a mutation window and passes a [`MutationContext`] to `f`. + /// Triggers a gc cycle + pub fn collect(&self) { + self.collector.collect(); + } + + pub fn mutate(&self, f: impl for<'gc> FnOnce(&MutationContext<'id, 'gc>) -> R) -> R { + let cx = MutationContext { + collector: &self.collector, + _marker: PhantomData, + }; + f(&cx) + } +} + +/// Creates a new GC context. +pub fn with_gc FnOnce(GcContext<'id>) -> R>(f: F) -> R { + f(GcContext { + collector: Collector::new(), + _marker: PhantomData, + }) +} diff --git a/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs b/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs new file mode 100644 index 0000000..0d3bea2 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs @@ -0,0 +1,68 @@ +//! `MutationContext<'id, 'gc>` handle. + +use crate::collectors::mark_sweep_branded::{ + Collector, + gc::{Gc, Root, RootNode}, + root_link::RootLink, + trace::{Finalize, Trace}, + weak::WeakGc, +}; +use core::marker::PhantomData; +use core::ptr::NonNull; +use rust_alloc::boxed::Box; + +/// Handle for GC allocations +pub struct MutationContext<'id, 'gc> { + pub(crate) collector: &'gc Collector, + pub(crate) _marker: PhantomData<*mut &'id ()>, +} + +impl<'id, 'gc> MutationContext<'id, 'gc> { + /// Allocates a value on the GC heap. + /// + /// # Panics + /// + /// Panics if the pool allocator fails to allocate. + pub fn alloc(&self, value: T) -> Gc<'gc, T> { + self.collector.alloc(value) + } + + /// Downgrades a `Gc` into a weak reference. + pub fn alloc_weak(&self, gc: Gc<'gc, T>) -> WeakGc<'id, T> { + let alloc_id = unsafe { (*gc.ptr.as_ptr()).alloc_id }; + WeakGc { + ptr: gc.ptr, + alloc_id, + _marker: PhantomData, + } + } + + /// Promotes a `Gc` pointer to a `Root` + pub fn root(&self, gc: Gc<'gc, T>) -> Root<'id, T> { + let gc_ptr = gc.ptr; + + let node = Box::new(RootNode { + link: RootLink::new(), + gc_ptr, + _marker: PhantomData, + }); + + let raw = unsafe { NonNull::new_unchecked(Box::into_raw(node)) }; + + // SAFETY: `raw` points to a stable `RootNode`. + unsafe { + let sentinel_ptr = NonNull::new_unchecked(self.collector.sentinel.as_ref().get_ref() + as *const RootLink + as *mut RootLink); + let link_ptr = raw.cast::(); + RootLink::link_after(sentinel_ptr, link_ptr); + } + + Root { raw } + } + + /// Triggers a gc cycle. + pub fn collect(&self) { + self.collector.collect(); + } +} diff --git a/oscars/src/collectors/mark_sweep_branded/root_link.rs b/oscars/src/collectors/mark_sweep_branded/root_link.rs new file mode 100644 index 0000000..747e63b --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/root_link.rs @@ -0,0 +1,94 @@ +//! Intrusive singly-linked root list. + +use core::cell::Cell; +use core::ptr::NonNull; + +/// Intrusive link node +pub(crate) struct RootLink { + prev: Cell>>, + next: Cell>>, +} + +impl RootLink { + /// Creates a new, unlinked node. + pub(crate) const fn new() -> Self { + Self { + prev: Cell::new(None), + next: Cell::new(None), + } + } + + /// Returns `true` if this node is currently part of a list. + #[inline] + pub(crate) fn is_linked(&self) -> bool { + self.prev.get().is_some() + } + + /// Inserts `node` immediately after `anchor`. + /// + /// # Safety + /// + /// Both `anchor` and `node` must remain at stable addresses until unlinked. + pub(crate) unsafe fn link_after(anchor: NonNull, node: NonNull) { + unsafe { + let anchor_ref = anchor.as_ref(); + let node_ref = node.as_ref(); + let old_next = anchor_ref.next.get(); + + node_ref.prev.set(Some(anchor)); + node_ref.next.set(old_next); + anchor_ref.next.set(Some(node)); + + if let Some(next) = old_next { + next.as_ref().prev.set(Some(node)); + } + } + } + + /// Removes `node` from the list. + /// + /// # Safety + /// + /// `node` must currently be linked. + pub(crate) unsafe fn unlink(node: NonNull) { + unsafe { + let node_ref = node.as_ref(); + let prev = node_ref.prev.get(); + let next = node_ref.next.get(); + + if let Some(p) = prev { + p.as_ref().next.set(next); + } + if let Some(n) = next { + n.as_ref().prev.set(prev); + } + + node_ref.prev.set(None); + node_ref.next.set(None); + } + } + + /// Returns an iterator over all nodes after `sentinel`. + pub(crate) fn iter_from_sentinel( + sentinel: NonNull, + ) -> impl Iterator> { + struct Iter { + current: Option>, + } + + impl Iterator for Iter { + type Item = NonNull; + + fn next(&mut self) -> Option { + let node = self.current?; + // SAFETY: nodes are pinned/heap-stable and valid during collection. + self.current = unsafe { node.as_ref().next.get() }; + Some(node) + } + } + + // SAFETY: sentinel is pinned in Collector and outlives the iteration. + let first = unsafe { sentinel.as_ref().next.get() }; + Iter { current: first } + } +} diff --git a/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs b/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs new file mode 100644 index 0000000..fe51a8d --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs @@ -0,0 +1,44 @@ +#[cfg(test)] +mod tests { + use crate::collectors::mark_sweep_branded::gc::RootNode; + use crate::collectors::mark_sweep_branded::root_link::RootLink; + + #[test] + fn root_link_is_thin() { + // According to the API redesign RFC, RootLink should just be two pointers (prev, next) + // with no virtual table overhead (no trace_fn fat pointers). + assert_eq!( + core::mem::size_of::(), + core::mem::size_of::<*const ()>() * 2, + "RootLink must be exactly two words (prev and next pointers)" + ); + } + + #[test] + fn root_node_layout_guarantees() { + // The RFC relies on offset_of!(RootNode<()>, gc_ptr) working exactly due to #[repr(C)] + // with `link` at offset 0. + // We verify that the gc_ptr is always immediately following the link, regardless of T. + + let link_size = core::mem::size_of::(); + + // Assert offset is correct via offset_of-like macro conceptually + let make_dummy = || RootNode:: { + link: RootLink::new(), + gc_ptr: core::ptr::NonNull::dangling(), + _marker: core::marker::PhantomData, + }; + + let node = make_dummy(); + let base_ptr = &node as *const _ as usize; + let link_ptr = &node.link as *const _ as usize; + let gc_ptr_addr = &node.gc_ptr as *const _ as usize; + + assert_eq!(base_ptr, link_ptr, "RootLink must be at offset 0"); + assert_eq!( + gc_ptr_addr - base_ptr, + link_size, + "gc_ptr must immediately follow RootLink" + ); + } +} diff --git a/oscars/src/collectors/mark_sweep_branded/tests/mod.rs b/oscars/src/collectors/mark_sweep_branded/tests/mod.rs new file mode 100644 index 0000000..b54064e --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/tests/mod.rs @@ -0,0 +1,131 @@ +use super::*; + +#[derive(Debug)] +struct JsObject { + name: rust_alloc::string::String, + value: i32, +} + +impl crate::collectors::mark_sweep_branded::Trace for JsObject { + fn trace(&mut self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {} +} +impl crate::collectors::mark_sweep_branded::Finalize for JsObject {} + +#[test] +fn unrooted_alloc_is_swept() { + with_gc(|ctx| { + let weak = ctx.mutate(|cx| { + cx.alloc_weak(cx.alloc(JsObject { + name: "ephemeral".into(), + value: 999, + })) + }); + ctx.collect(); + ctx.mutate(|cx| { + assert!(weak.upgrade(cx).is_none()); + }); + }); +} + +#[test] +fn rooted_alloc_survives_collection() { + with_gc(|ctx| { + let root = ctx.mutate(|cx| { + cx.root(cx.alloc(JsObject { + name: "pinned".into(), + value: 42, + })) + }); + ctx.collect(); + ctx.mutate(|cx| { + let gc = root.get(cx); + assert_eq!(gc.get().value, 42); + assert_eq!(gc.get().name, "pinned"); + }); + }); +} + +#[test] +fn weak_upgrade_after_collection_without_root_is_none() { + with_gc(|ctx| { + let weak = ctx.mutate(|cx| { + cx.alloc_weak(cx.alloc(JsObject { + name: "weak".into(), + value: 10, + })) + }); + ctx.collect(); + ctx.mutate(|cx| { + assert!(weak.upgrade(cx).is_none()); + }); + }); +} + +#[test] +fn weak_upgrade_with_live_root_is_some() { + with_gc(|ctx| { + let (root, weak) = ctx.mutate(|cx| { + let obj = cx.alloc(JsObject { + name: "strong".into(), + value: 7, + }); + let root = cx.root(obj); + + let weak = cx.alloc_weak(cx.alloc(JsObject { + name: "weak_entry".into(), + value: 77, + })); + (root, weak) + }); + ctx.collect(); + ctx.mutate(|cx| { + assert!(weak.upgrade(cx).is_none()); + assert_eq!(root.get(cx).get().value, 7); + }); + }); +} + +#[test] +fn multiple_roots_are_independent() { + with_gc(|ctx| { + let (root1, root2) = ctx.mutate(|cx| { + let obj1 = cx.alloc(100i32); + let obj2 = cx.alloc(200i32); + (cx.root(obj1), cx.root(obj2)) + }); + + ctx.collect(); + + ctx.mutate(|cx| { + assert_eq!(*root1.get(cx).get(), 100); + assert_eq!(*root2.get(cx).get(), 200); + }); + + drop(root1); + ctx.collect(); + + ctx.mutate(|cx| { + assert_eq!(*root2.get(cx).get(), 200); + }); + }); +} + +#[test] +fn root_escapes_closure_safely() { + with_gc(|ctx| { + let root = ctx.mutate(|cx| { + let obj = cx.alloc(555i32); + cx.root(obj) + }); + + ctx.collect(); + + ctx.mutate(|cx| { + assert_eq!(*root.get(cx).get(), 555); + }); + }); +} + +mod api_compliance; +mod uaf; +mod ui_tests; diff --git a/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs b/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs new file mode 100644 index 0000000..fad39e2 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs @@ -0,0 +1,31 @@ +use crate::collectors::mark_sweep::Finalize; +use crate::collectors::mark_sweep_branded::Trace; +use crate::collectors::mark_sweep_branded::with_gc; +use core::cell::Cell; + +struct DetectDrop<'a>(&'a Cell); + +impl<'a> Trace for DetectDrop<'a> { + fn trace(&mut self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {} +} + +impl Finalize for DetectDrop<'_> {} + +impl Drop for DetectDrop<'_> { + fn drop(&mut self) { + self.0.set(true); + } +} + +#[test] +fn test_uaf() { + with_gc(|cx| { + let dropped = Cell::new(false); + cx.mutate(|mcx| { + let _gc = mcx.alloc(DetectDrop(&dropped)); + }); + cx.collect(); // Garbage collects 'gc' because it isn't rooted! + assert!(dropped.get(), "It wasn't collected!"); + // The pointer _gc is safely out of scope here! Compiler prevents accessing it. + }); +} diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.rs b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.rs new file mode 100644 index 0000000..0e29f6f --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.rs @@ -0,0 +1,18 @@ +//! Compile-fail: `Gc<'gc, T>` cannot escape the `mutate()` closure. +//! +//! `for<'gc>` in `GcContext::mutate` makes `'gc` universally quantified inside +//! the closure. The return type `R` cannot mention `'gc`, so the borrow checker +//! rejects any attempt to return a `Gc<'gc, T>` from `mutate`. + +use oscars::collectors::mark_sweep_branded::with_gc; + +fn main() { + with_gc(|ctx| { + // The closure must return R for some R that does not mention 'gc. + // Returning Gc<'gc, i32> directly attempts to leak a shorter lifetime + // into the outer scope. The compiler must reject this. + let _escaped = ctx.mutate(|cx| { + cx.alloc(42i32) // ERROR: Gc<'gc, i32> cannot escape mutate() + }); + }); +} diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.stderr b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.stderr new file mode 100644 index 0000000..37ad639 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.stderr @@ -0,0 +1,9 @@ +error: lifetime may not live long enough + --> src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.rs:15:13 + | +14 | let _escaped = ctx.mutate(|cx| { + | --- return type of closure is oscars::collectors::mark_sweep_branded::Gc<'2, i32> + | | + | has type `&MutationContext<'_, '1>` +15 | cx.alloc(42i32) // ERROR: Gc<'gc, i32> cannot escape mutate() + | ^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.rs b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.rs new file mode 100644 index 0000000..648db6a --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.rs @@ -0,0 +1,28 @@ + +//! Compile-fail: `Gc<'gc, T>` cannot be stored in a location that outlives +//! the `mutate()` closure. +//! +//! Even if the caller does not return the `Gc` directly, storing it in an +//! outer `let` binding that outlives the closure is equally rejected: the +//! `'gc` lifetime of the `Gc` is shorter than the outer binding's scope. + +use oscars::collectors::mark_sweep_branded::{Gc, with_gc}; + +struct Holder<'a> { + gc: Gc<'a, i32>, +} + +fn main() { + with_gc(|ctx| { + let mut holder: Option> = None; + + ctx.mutate(|cx| { + let gc = cx.alloc(42i32); + // ERROR: `cx` (and therefore `gc`'s `'gc` lifetime) does not live + // long enough to be stored in `holder`. + holder = Some(Holder { gc }); + }); + + let _ = holder; + }); +} diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.stderr b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.stderr new file mode 100644 index 0000000..08e66af --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.stderr @@ -0,0 +1,11 @@ +error[E0521]: borrowed data escapes outside of closure + --> src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.rs:23:13 + | +17 | let mut holder: Option> = None; + | ---------- `holder` declared here, outside of the closure body +18 | +19 | ctx.mutate(|cx| { + | -- `cx` is a reference that is only valid in the closure body +... +23 | holder = Some(Holder { gc }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `cx` escapes the closure body here diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs b/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs new file mode 100644 index 0000000..408c8a3 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs @@ -0,0 +1,24 @@ + +//! Compile-fail: `Root<'id, T>` from one `with_gc` context cannot be used +//! inside a different `with_gc` context. +//! +//! `with_gc` has a `for<'id>` bound, so every call produces a fresh, unnamed +//! `'id` lifetime. `Root<'id1, T>` and `MutationContext<'id2, '_>` carry +//! distinct, non-unifiable `'id` variables, so the borrow checker rejects +//! `root.get(cx)` when `root` and `cx` come from different contexts. + +use oscars::collectors::mark_sweep_branded::with_gc; + +fn main() { + with_gc(|ctx1| { + with_gc(|ctx2| { + // root carries 'id of ctx1 + let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32))); + + ctx2.mutate(|cx| { + // ERROR: `'id` of `root` (ctx1) != `'id` of `cx` (ctx2) + let _gc = root.get(cx); + }); + }); + }); +} diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.stderr b/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.stderr new file mode 100644 index 0000000..eac1f09 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.stderr @@ -0,0 +1,19 @@ +error: lifetime may not live long enough + --> src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs:16:41 + | +13 | with_gc(|ctx1| { + | ---- lifetime `'2` appears in the type of `ctx1` +14 | with_gc(|ctx2| { + | ---- has type `GcContext<'1>` +15 | // root carries 'id of ctx1 +16 | let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32))); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` + +error: lifetime may not live long enough + --> src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs:16:41 + | +13 | with_gc(|ctx1| { + | ---- has type `GcContext<'1>` +... +16 | let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32))); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'static` diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui_tests.rs b/oscars/src/collectors/mark_sweep_branded/tests/ui_tests.rs new file mode 100644 index 0000000..c17d420 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui_tests.rs @@ -0,0 +1,6 @@ +#[test] +#[cfg(not(miri))] +fn ui() { + let t = trybuild::TestCases::new(); + t.compile_fail("src/collectors/mark_sweep_branded/tests/ui/*.rs"); +} diff --git a/oscars/src/collectors/mark_sweep_branded/trace.rs b/oscars/src/collectors/mark_sweep_branded/trace.rs new file mode 100644 index 0000000..464fd78 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/trace.rs @@ -0,0 +1,299 @@ +//! Trace and Finalize traits for the lifetime branded GC + +use crate::collectors::mark_sweep_branded::gc::Gc; +use core::cell::{Cell, OnceCell}; +use core::marker::PhantomData; +use rust_alloc::borrow::{Cow, ToOwned}; +use rust_alloc::boxed::Box; +use rust_alloc::collections::{BTreeMap, BTreeSet, LinkedList, VecDeque}; +use rust_alloc::string::String; +use rust_alloc::vec::Vec; + +// Re-export the shared `Finalize` trait and standard library implementations. +pub use crate::collectors::common::Finalize; + +/// Trait for tracing garbage collected values. +/// +/// # Safety +/// +/// Use `Tracer::mark` for every reachable `Gc` pointer. +pub trait Trace { + /// Marks all `Gc` pointers reachable from `self`. + fn trace(&mut self, tracer: &mut Tracer); +} + +pub(crate) type TraceFn = unsafe fn(core::ptr::NonNull, &mut Tracer<'_>); + +/// Callback handle passed to `Trace::trace`. +/// +/// The `'a` lifetime ties the tracer to the collection cycle, +/// preventing it from being stored or escaping the collector. +pub struct Tracer<'a> { + pub(crate) worklist: Vec<(core::ptr::NonNull, TraceFn)>, + pub(crate) _marker: PhantomData<&'a ()>, +} + +impl<'a> Tracer<'a> { + pub(crate) fn new() -> Self { + Self { + worklist: Vec::new(), + _marker: PhantomData, + } + } + + pub(crate) fn drain(&mut self) { + // Note: Using `pop()` processes the worklist in LIFO order (Depth-First Search). + // While correct, heap-allocated object graphs often exhibit better cache locality + // with Breadth-First Search. This could be evaluated with a `VecDeque` in the future. + while let Some((ptr, trace_fn)) = self.worklist.pop() { + unsafe { + (trace_fn)(ptr, self); + } + } + } + + /// Marks `gc` as reachable. + #[inline] + pub fn mark(&mut self, gc: &Gc<'_, T>) { + // SAFETY: `gc.ptr` is a valid `GcBox`. + unsafe { + if !(*gc.ptr.as_ptr()).marked.replace(true) { + unsafe fn trace_value( + ptr: core::ptr::NonNull, + tracer: &mut Tracer<'_>, + ) { + let gcbox_ptr = + ptr.cast::>(); + unsafe { + (*gcbox_ptr.as_ptr()).value.trace(tracer); + } + } + + self.worklist.push((gc.ptr.cast::(), trace_value::)); + } + } + } + + /// Marks a raw allocation as reachable. + /// + /// # Safety + /// + /// `ptr` must be a valid pointer to a `GcBox` managed by this collector. + #[inline] + pub(crate) fn mark_raw(&mut self, ptr: core::ptr::NonNull) { + let boxed_ptr = ptr.cast::>(); + + unsafe { + if !(*boxed_ptr.as_ptr()).marked.replace(true) { + // Call the trace function. + if let Some(trace_fn) = (*boxed_ptr.as_ptr()).trace_fn { + self.worklist.push((ptr, trace_fn)); + } + } + } + } +} + +impl Trace for &T { + #[inline] + fn trace(&mut self, _tracer: &mut Tracer) {} +} + +// primitive + std-lib Trace impls + +macro_rules! empty_trace { + ($($T:ty),* $(,)?) => { + $( + impl Trace for $T { + #[inline] + fn trace(&mut self, _tracer: &mut Tracer) {} + } + )* + }; +} + +empty_trace![ + (), + bool, + isize, + usize, + i8, + u8, + i16, + u16, + i32, + u32, + i64, + u64, + i128, + u128, + f32, + f64, + char, + String, + core::num::NonZeroIsize, + core::num::NonZeroUsize, + core::num::NonZeroI8, + core::num::NonZeroU8, + core::num::NonZeroI16, + core::num::NonZeroU16, + core::num::NonZeroI32, + core::num::NonZeroU32, + core::num::NonZeroI64, + core::num::NonZeroU64, + core::num::NonZeroI128, + core::num::NonZeroU128, +]; + +impl Trace for [T; N] { + fn trace(&mut self, tracer: &mut Tracer) { + for v in self.iter_mut() { + v.trace(tracer); + } + } +} + +impl Trace for Box { + fn trace(&mut self, tracer: &mut Tracer) { + (**self).trace(tracer); + } +} + +impl Trace for Option { + fn trace(&mut self, tracer: &mut Tracer) { + if let Some(v) = self { + v.trace(tracer); + } + } +} + +impl Trace for Result { + fn trace(&mut self, tracer: &mut Tracer) { + match self { + Ok(v) => v.trace(tracer), + Err(e) => e.trace(tracer), + } + } +} + +impl Trace for Vec { + fn trace(&mut self, tracer: &mut Tracer) { + for v in self.iter_mut() { + v.trace(tracer); + } + } +} + +impl Trace for VecDeque { + fn trace(&mut self, tracer: &mut Tracer) { + for v in self.iter_mut() { + v.trace(tracer); + } + } +} + +impl Trace for LinkedList { + fn trace(&mut self, tracer: &mut Tracer) { + for v in self.iter_mut() { + v.trace(tracer); + } + } +} + +impl Trace for PhantomData { + #[inline] + fn trace(&mut self, _tracer: &mut Tracer) {} +} + +// Cell> requires T: Copy to safely take and restore the value. +// For non-Copy types, use GcRefCell instead. +impl Trace for Cell> { + fn trace(&mut self, tracer: &mut Tracer) { + if let Some(mut v) = self.get_mut().take() { + v.trace(tracer); + self.set(Some(v)); + } + } +} + +impl Trace for OnceCell { + fn trace(&mut self, tracer: &mut Tracer) { + if let Some(v) = self.get_mut() { + v.trace(tracer); + } + } +} + +impl Trace for Cow<'static, T> +where + T::Owned: Trace, +{ + fn trace(&mut self, tracer: &mut Tracer) { + if let Cow::Owned(v) = self { + v.trace(tracer); + } + } +} + +impl Trace for (A,) { + #[inline] + fn trace(&mut self, tracer: &mut Tracer) { + self.0.trace(tracer); + } +} + +impl Trace for (A, B) { + #[inline] + fn trace(&mut self, tracer: &mut Tracer) { + self.0.trace(tracer); + self.1.trace(tracer); + } +} + +impl Trace for (A, B, C) { + #[inline] + fn trace(&mut self, tracer: &mut Tracer) { + self.0.trace(tracer); + self.1.trace(tracer); + self.2.trace(tracer); + } +} + +impl Trace for (A, B, C, D) { + #[inline] + fn trace(&mut self, tracer: &mut Tracer) { + self.0.trace(tracer); + self.1.trace(tracer); + self.2.trace(tracer); + self.3.trace(tracer); + } +} + +// Rc and Arc do not contain Gc pointers (they use reference counting, not GC). +// If you need to store Gc pointers inside Rc/Arc, wrap them in a GC-allocated +// struct instead. +impl Trace for rust_alloc::rc::Rc { + #[inline] + fn trace(&mut self, _tracer: &mut Tracer) {} +} + +impl Trace for rust_alloc::sync::Arc { + #[inline] + fn trace(&mut self, _tracer: &mut Tracer) {} +} + +impl Trace for BTreeMap { + fn trace(&mut self, tracer: &mut Tracer) { + for v in self.values_mut() { + v.trace(tracer); + } + } +} + +impl Trace for BTreeSet { + #[inline] + fn trace(&mut self, _tracer: &mut Tracer) { + // BTreeSet keys are immutable and cannot contain Gc pointers + // that need tracing (Gc requires &mut self to trace). + } +} diff --git a/oscars/src/collectors/mark_sweep_branded/weak.rs b/oscars/src/collectors/mark_sweep_branded/weak.rs new file mode 100644 index 0000000..660cc69 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/weak.rs @@ -0,0 +1,52 @@ +//! `WeakGc<'id, T>` for weak references. + +use crate::collectors::mark_sweep_branded::{ + gc::Gc, + gc_box::GcBox, + trace::{Finalize, Trace}, +}; +use core::marker::PhantomData; +use core::ptr::NonNull; + +/// A weak reference to a GC managed value +pub struct WeakGc<'id, T: Trace + ?Sized> { + pub(crate) ptr: NonNull>, + pub(crate) alloc_id: usize, + pub(crate) _marker: PhantomData<*mut &'id ()>, +} + +impl<'id, T: Trace> WeakGc<'id, T> { + /// Attempts to upgrade to a strong `Gc<'gc, T>`. + pub fn upgrade<'gc>( + &self, + _cx: &crate::collectors::mark_sweep_branded::MutationContext<'id, 'gc>, + ) -> Option> { + // SAFETY: `_cx` proves the `Collector` is alive. + // `alloc_id` confirms the allocation is still valid. + // The allocator does not unmap memory, so reading a recycled block's `alloc_id` is safe + let is_valid = unsafe { (*self.ptr.as_ptr()).alloc_id == self.alloc_id }; + + if is_valid { + Some(Gc { + ptr: self.ptr, + _marker: PhantomData, + }) + } else { + None + } + } +} + +impl<'id, T: Trace + ?Sized> Clone for WeakGc<'id, T> { + fn clone(&self) -> Self { + *self + } +} + +impl<'id, T: Trace + ?Sized> Copy for WeakGc<'id, T> {} + +impl<'id, T: Trace> Finalize for WeakGc<'id, T> {} +impl<'id, T: Trace> Trace for WeakGc<'id, T> { + // Weak references do not mark their target; upgrade() returning None after collection is the intended behaviour. + fn trace(&mut self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {} +} diff --git a/oscars/src/collectors/mod.rs b/oscars/src/collectors/mod.rs index c8bfd75..6172a2b 100644 --- a/oscars/src/collectors/mod.rs +++ b/oscars/src/collectors/mod.rs @@ -1,4 +1,8 @@ //! This module contains various collector implementations. +pub mod common; pub mod mark_sweep; pub mod mark_sweep_arena2; + +#[cfg(feature = "mark_sweep_branded")] +pub mod mark_sweep_branded; diff --git a/oscars_derive/src/mark_sweep_branded.rs b/oscars_derive/src/mark_sweep_branded.rs new file mode 100644 index 0000000..37d4c5e --- /dev/null +++ b/oscars_derive/src/mark_sweep_branded.rs @@ -0,0 +1,47 @@ +use quote::quote; +use synstructure::{AddBounds, Structure, decl_derive}; + +decl_derive! { + [Trace, attributes(oscars_gc, unsafe_ignore_trace)] => + /// Derive the `Trace` trait for mark_sweep_branded collector. + derive_trace +} + +/// Derives the `Trace` trait for mark_sweep_branded. +fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { + s.filter(|bi| { + !bi.ast() + .attrs + .iter() + .any(|attr| attr.path().is_ident("unsafe_ignore_trace")) + }); + + let trace_body = s.each(|bi| { + quote!(::oscars::collectors::mark_sweep_branded::Trace::trace(#bi, tracer)) + }); + + s.add_bounds(AddBounds::Fields); + s.bound_impl( + quote!(::oscars::collectors::mark_sweep_branded::Trace), + quote! { + #[inline] + fn trace(&mut self, tracer: &mut ::oscars::collectors::mark_sweep_branded::Tracer) { + match *self { #trace_body } + } + }, + ) +} + +decl_derive! { + [Finalize] => + /// Derive the `Finalize` trait for mark_sweep_branded collector. + derive_finalize +} + +/// Derives the `Finalize` trait for mark_sweep_branded. +fn derive_finalize(s: Structure<'_>) -> proc_macro2::TokenStream { + s.unbound_impl( + quote!(::oscars::collectors::mark_sweep_branded::Finalize), + quote!(), + ) +} From 222a786f8e8f4b82999dc97f3f5fe81e55a19c17 Mon Sep 17 00:00:00 2001 From: shruti2522 Date: Thu, 23 Apr 2026 18:20:39 +0000 Subject: [PATCH 2/8] fix --- ...mark_sweep_branded_implementation_notes.md | 31 +------------------ .../src/collectors/mark_sweep_branded/gc.rs | 2 +- .../src/collectors/mark_sweep_branded/mod.rs | 5 ++- .../mark_sweep_branded/mutation_ctx.rs | 2 +- .../src/collectors/mark_sweep_branded/weak.rs | 2 +- oscars_derive/src/mark_sweep_branded.rs | 4 +-- 6 files changed, 7 insertions(+), 39 deletions(-) diff --git a/notes/mark_sweep_branded_implementation_notes.md b/notes/mark_sweep_branded_implementation_notes.md index 5524acd..b02dcd5 100644 --- a/notes/mark_sweep_branded_implementation_notes.md +++ b/notes/mark_sweep_branded_implementation_notes.md @@ -1,7 +1,4 @@ -# mark_sweep_branded Implementation Notes - **Date**: 2026-04-23 -**Status**: Production Ready ## Changes from API Redesign Proposal @@ -62,14 +59,7 @@ impl Trace for Cell> `self.set(Some(v))` requires moving `v`, which needs `T: Copy`. Without this bound, code fails to compile. **Alternative:** -Use `GcRefCell` for non-Copy types. - -### 5. Documentation Improvements - -**Added:** -- `Tracer<'a>` lifetime explanation -- `PoolAllocator<'static>` safety justification -- Comments on why certain impls are no-ops +Use `GcRefCell` for non Copy types. ## Design Decisions @@ -86,22 +76,3 @@ Both `GcContext::collect` and `MutationContext::collect` use `&self` with interi **Why:** Allows calling `collect()` inside `mutate()` closures without borrow conflicts. - -## Testing - -All tests pass: -- Unit tests (10 passed) -- Compile-fail tests (3 passed) -- Clippy (no warnings) -- Miri (no undefined behavior) -- Formatting (correct) - -## Summary - -Implementation is production ready and matches the approved API proposal. All additions are either: -- Required for soundness (ABA protection) -- Defensive checks (ID wrap) -- Practical needs (stdlib impls) -- Correctness fixes (Copy bounds) - -No workarounds used. All unsafe code is justified. diff --git a/oscars/src/collectors/mark_sweep_branded/gc.rs b/oscars/src/collectors/mark_sweep_branded/gc.rs index 6203193..3bef4a3 100644 --- a/oscars/src/collectors/mark_sweep_branded/gc.rs +++ b/oscars/src/collectors/mark_sweep_branded/gc.rs @@ -31,7 +31,7 @@ impl<'gc, T: Trace + 'gc> Gc<'gc, T> { pub fn get(&self) -> &T { // SAFETY: `ptr` is non-null and valid for `'gc` by construction. // The `'gc` lifetime is scoped to a `mutate()` closure, collection only occurs - // via `cx.collect()` within that same closure, and `Gc<'gc, T>` cannot + // via `cx.collect()` within that same closure and `Gc<'gc, T>` can't // escape the closure. unsafe { &(*self.ptr.as_ptr()).value } } diff --git a/oscars/src/collectors/mark_sweep_branded/mod.rs b/oscars/src/collectors/mark_sweep_branded/mod.rs index 720ccd4..1174eb2 100644 --- a/oscars/src/collectors/mark_sweep_branded/mod.rs +++ b/oscars/src/collectors/mark_sweep_branded/mod.rs @@ -38,7 +38,7 @@ struct PoolEntry { pub(crate) struct Collector { // SAFETY: We use 'static here because the PoolAllocator owns its memory, // and we ensure that `Gc` objects and pool allocations do not outlive - // the `Collector` instance. + // the `Collector` instance pub(crate) pool: RefCell>, pool_entries: RefCell>, pub(crate) sentinel: Pin>, @@ -61,7 +61,7 @@ impl Collector { /// /// # Panics /// - /// Panics if the allocation ID counter wraps around to `FREED_ALLOC_ID`. + /// Panics if the allocation ID counter wraps around to `FREED_ALLOC_ID` /// This is a theoretical limit that would require `usize::MAX - 1` allocations. pub(crate) fn alloc<'gc, T: trace::Trace + trace::Finalize + 'gc>( &'gc self, @@ -72,7 +72,6 @@ impl Collector { // Check for alloc_id wrap before incrementing. // If alloc_id reaches FREED_ALLOC_ID (usize::MAX), weak reference validation // would break because freed slots are marked with this sentinel value. - // This is a theoretical limit requiring usize::MAX - 1 allocations. assert_ne!( alloc_id, GcBox::<()>::FREED_ALLOC_ID, diff --git a/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs b/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs index 0d3bea2..9fe85e1 100644 --- a/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs +++ b/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs @@ -27,7 +27,7 @@ impl<'id, 'gc> MutationContext<'id, 'gc> { self.collector.alloc(value) } - /// Downgrades a `Gc` into a weak reference. + /// Downgrades a `Gc` into a weak reference pub fn alloc_weak(&self, gc: Gc<'gc, T>) -> WeakGc<'id, T> { let alloc_id = unsafe { (*gc.ptr.as_ptr()).alloc_id }; WeakGc { diff --git a/oscars/src/collectors/mark_sweep_branded/weak.rs b/oscars/src/collectors/mark_sweep_branded/weak.rs index 660cc69..5b266b5 100644 --- a/oscars/src/collectors/mark_sweep_branded/weak.rs +++ b/oscars/src/collectors/mark_sweep_branded/weak.rs @@ -47,6 +47,6 @@ impl<'id, T: Trace + ?Sized> Copy for WeakGc<'id, T> {} impl<'id, T: Trace> Finalize for WeakGc<'id, T> {} impl<'id, T: Trace> Trace for WeakGc<'id, T> { - // Weak references do not mark their target; upgrade() returning None after collection is the intended behaviour. + // Weak references do not mark their target, upgrade() returning None after collection is the intended behaviour. fn trace(&mut self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {} } diff --git a/oscars_derive/src/mark_sweep_branded.rs b/oscars_derive/src/mark_sweep_branded.rs index 37d4c5e..4f81abb 100644 --- a/oscars_derive/src/mark_sweep_branded.rs +++ b/oscars_derive/src/mark_sweep_branded.rs @@ -7,7 +7,6 @@ decl_derive! { derive_trace } -/// Derives the `Trace` trait for mark_sweep_branded. fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { s.filter(|bi| { !bi.ast() @@ -34,11 +33,10 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { decl_derive! { [Finalize] => - /// Derive the `Finalize` trait for mark_sweep_branded collector. + /// Derive the `Finalize` trait for mark_sweep_branded collector derive_finalize } -/// Derives the `Finalize` trait for mark_sweep_branded. fn derive_finalize(s: Structure<'_>) -> proc_macro2::TokenStream { s.unbound_impl( quote!(::oscars::collectors::mark_sweep_branded::Finalize), From 974c72bc7385281d3e920c7900d5d23fcedb03a2 Mon Sep 17 00:00:00 2001 From: Shruti Sharma <98698727+shruti2522@users.noreply.github.com> Date: Tue, 21 Apr 2026 06:57:29 +0530 Subject: [PATCH 3/8] add ephemeron support --- oscars/src/alloc/mempool3/mod.rs | 61 +++++++ .../mark_sweep_branded/ephemeron.rs | 48 ++++++ .../src/collectors/mark_sweep_branded/gc.rs | 23 +-- .../collectors/mark_sweep_branded/gc_box.rs | 2 +- .../src/collectors/mark_sweep_branded/mod.rs | 160 +++++++++++++----- .../mark_sweep_branded/mutation_ctx.rs | 31 +++- .../mark_sweep_branded/root_link.rs | 54 ++++-- .../mark_sweep_branded/tests/ephemeron.rs | 109 ++++++++++++ .../mark_sweep_branded/tests/mod.rs | 1 + .../collectors/mark_sweep_branded/trace.rs | 26 +-- .../src/collectors/mark_sweep_branded/weak.rs | 19 ++- 11 files changed, 439 insertions(+), 95 deletions(-) create mode 100644 oscars/src/collectors/mark_sweep_branded/ephemeron.rs create mode 100644 oscars/src/collectors/mark_sweep_branded/tests/ephemeron.rs diff --git a/oscars/src/alloc/mempool3/mod.rs b/oscars/src/alloc/mempool3/mod.rs index 9382548..159626d 100644 --- a/oscars/src/alloc/mempool3/mod.rs +++ b/oscars/src/alloc/mempool3/mod.rs @@ -248,6 +248,67 @@ impl<'alloc> PoolAllocator<'alloc> { } } + /// Allocates a raw slot of the given size without type constraints. + /// Returns a pointer to uninitialized memory. + /// + /// # Safety + /// Caller must initialize the memory before use and ensure proper cleanup. + pub fn alloc_slot_raw(&mut self, size: usize) -> Result, PoolAllocError> { + let sc_idx = size_class_index_for(size); + let slot_size = SIZE_CLASSES.get(sc_idx).copied().unwrap_or(size); + + let cached_idx = self.alloc_cache[sc_idx].get(); + if cached_idx < self.slot_pools.len() { + let pool = &self.slot_pools[cached_idx]; + if pool.slot_size == slot_size + && let Some(slot_ptr) = pool.alloc_slot() + { + return Ok(slot_ptr); + } + } + + // try existing pools with matching slot_size first + for (i, pool) in self.slot_pools.iter().enumerate().rev() { + if pool.slot_size == slot_size + && let Some(slot_ptr) = pool.alloc_slot() + { + self.alloc_cache[sc_idx].set(i); + return Ok(slot_ptr); + } + } + + // need a new pool for this size class + // try the recycle list first + if let Some(pos) = self + .recycled_pools + .iter() + .rposition(|p| p.slot_size == slot_size) + { + let pool = self.recycled_pools.swap_remove(pos); + let slot_ptr = pool.alloc_slot().ok_or(PoolAllocError::OutOfMemory)?; + let insert_idx = self.slot_pools.len(); + let (base, end) = pool.slot_range(); + let spos = self.sorted_ranges.partition_point(|&(b, _, _)| b < base); + self.sorted_ranges.insert(spos, (base, end, insert_idx)); + self.slot_pools.push(pool); + self.alloc_cache[sc_idx].set(insert_idx); + return Ok(slot_ptr); + } + + // Allocate a fresh page from the OS + let total = self.page_size.max(slot_size * 4); + let new_pool = SlotPool::try_init(slot_size, total, 16)?; + self.current_heap_size += new_pool.layout.size(); + let slot_ptr = new_pool.alloc_slot().ok_or(PoolAllocError::OutOfMemory)?; + let insert_idx = self.slot_pools.len(); + let (base, end) = new_pool.slot_range(); + let spos = self.sorted_ranges.partition_point(|&(b, _, _)| b < base); + self.sorted_ranges.insert(spos, (base, end, insert_idx)); + self.slot_pools.push(new_pool); + self.alloc_cache[sc_idx].set(insert_idx); + Ok(slot_ptr) + } + /// drops the value at `ptr` and returns the slot to the allocator /// /// # Safety diff --git a/oscars/src/collectors/mark_sweep_branded/ephemeron.rs b/oscars/src/collectors/mark_sweep_branded/ephemeron.rs new file mode 100644 index 0000000..9b75638 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/ephemeron.rs @@ -0,0 +1,48 @@ +use crate::{ + alloc::mempool3::PoolItem, + collectors::mark_sweep_branded::{ + gc::Gc, + gc_box::GcBox, + mutation_ctx::MutationContext, + trace::{Finalize, Trace, Tracer}, + }, +}; +use core::marker::PhantomData; +use core::ptr::NonNull; + +pub struct Ephemeron<'id, K: Trace, V: Trace> { + pub(crate) key_ptr: NonNull>>, + pub(crate) key_alloc_id: usize, + pub(crate) value_ptr: NonNull>>, + pub(crate) _marker: PhantomData<*mut &'id ()>, +} + +impl<'id, K: Trace, V: Trace> Ephemeron<'id, K, V> { + /// Returns the value if the key is alive. + pub fn get_value<'gc>(&self, _cx: &MutationContext<'id, 'gc>) -> Option> { + // SAFETY: `_cx` proves the collector is alive; alloc_id guards ABA. + let key_alive = unsafe { (*self.key_ptr.as_ptr()).0.alloc_id == self.key_alloc_id }; + if key_alive { + Some(Gc { + ptr: self.value_ptr, + _marker: PhantomData, + }) + } else { + None + } + } +} + +impl<'id, K: Trace, V: Trace> Clone for Ephemeron<'id, K, V> { + fn clone(&self) -> Self { + *self + } +} + +impl<'id, K: Trace, V: Trace> Copy for Ephemeron<'id, K, V> {} + +impl<'id, K: Trace, V: Trace> Finalize for Ephemeron<'id, K, V> {} + +impl<'id, K: Trace, V: Trace> Trace for Ephemeron<'id, K, V> { + fn trace(&mut self, _tracer: &mut Tracer) {} +} diff --git a/oscars/src/collectors/mark_sweep_branded/gc.rs b/oscars/src/collectors/mark_sweep_branded/gc.rs index 3bef4a3..85f5641 100644 --- a/oscars/src/collectors/mark_sweep_branded/gc.rs +++ b/oscars/src/collectors/mark_sweep_branded/gc.rs @@ -1,9 +1,13 @@ //! Core pointer types. -use crate::collectors::mark_sweep_branded::{ - gc_box::GcBox, - root_link::RootLink, - trace::{Finalize, Trace}, +use crate::{ + alloc::mempool3::PoolItem, + collectors::mark_sweep_branded::{ + gc_box::GcBox, + mutation_ctx::MutationContext, + root_link::RootLink, + trace::{Finalize, Trace}, + }, }; use core::fmt; use core::marker::PhantomData; @@ -14,7 +18,7 @@ use rust_alloc::boxed::Box; /// A transient pointer to a GC-managed value. #[derive(Debug)] pub struct Gc<'gc, T: Trace + ?Sized + 'gc> { - pub(crate) ptr: NonNull>, + pub(crate) ptr: NonNull>>, pub(crate) _marker: PhantomData<(&'gc T, *const ())>, } @@ -33,7 +37,7 @@ impl<'gc, T: Trace + 'gc> Gc<'gc, T> { // The `'gc` lifetime is scoped to a `mutate()` closure, collection only occurs // via `cx.collect()` within that same closure and `Gc<'gc, T>` can't // escape the closure. - unsafe { &(*self.ptr.as_ptr()).value } + unsafe { &(*self.ptr.as_ptr()).0.value } } } @@ -56,7 +60,7 @@ pub(crate) struct RootNode<'id, T: Trace> { /// Intrusive list link pub(crate) link: RootLink, /// Pointer to the allocation - pub(crate) gc_ptr: NonNull>, + pub(crate) gc_ptr: NonNull>>, pub(crate) _marker: PhantomData<*mut &'id ()>, } @@ -68,10 +72,7 @@ pub struct Root<'id, T: Trace> { impl<'id, T: Trace> Root<'id, T> { /// Converts this root into a `Gc` pointer - pub fn get<'gc>( - &self, - _cx: &crate::collectors::mark_sweep_branded::MutationContext<'id, 'gc>, - ) -> Gc<'gc, T> { + pub fn get<'gc>(&self, _cx: &MutationContext<'id, 'gc>) -> Gc<'gc, T> { Gc { // SAFETY: `raw` is non-null and valid. ptr: unsafe { self.raw.as_ref().gc_ptr }, diff --git a/oscars/src/collectors/mark_sweep_branded/gc_box.rs b/oscars/src/collectors/mark_sweep_branded/gc_box.rs index aaa5ad3..4ee8da7 100644 --- a/oscars/src/collectors/mark_sweep_branded/gc_box.rs +++ b/oscars/src/collectors/mark_sweep_branded/gc_box.rs @@ -11,7 +11,7 @@ pub(crate) struct GcBox { /// Reachability flag set by the mark phase. pub(crate) marked: Cell, /// Type-erased trace function. - pub(crate) trace_fn: Option, + pub(crate) trace_fn: TraceFn, /// Allocation ID used to validate weak pointers. pub(crate) alloc_id: usize, /// The user value. diff --git a/oscars/src/collectors/mark_sweep_branded/mod.rs b/oscars/src/collectors/mark_sweep_branded/mod.rs index 1174eb2..7ce9e09 100644 --- a/oscars/src/collectors/mark_sweep_branded/mod.rs +++ b/oscars/src/collectors/mark_sweep_branded/mod.rs @@ -2,6 +2,7 @@ #![cfg_attr(not(any(test, feature = "std")), allow(unused_imports))] pub mod cell; +pub mod ephemeron; pub mod gc; pub mod gc_box; pub mod mutation_ctx; @@ -13,20 +14,19 @@ pub mod weak; mod tests; pub use cell::GcRefCell; +pub use ephemeron::Ephemeron; pub use gc::{Gc, Root}; pub use mutation_ctx::MutationContext; pub use trace::{Finalize, Trace, Tracer}; pub use weak::WeakGc; use crate::alloc::mempool3::PoolAllocator; -use core::alloc::Layout; use core::cell::{Cell, RefCell}; use core::marker::PhantomData; -use core::pin::Pin; +use core::mem; use core::ptr::NonNull; use gc_box::GcBox; -use root_link::RootLink; -use rust_alloc::boxed::Box; +use root_link::{RootLink, RootSentinel}; use rust_alloc::vec::Vec; /// Erased drop-fn @@ -35,15 +35,23 @@ struct PoolEntry { drop_fn: unsafe fn(&mut PoolAllocator<'static>, NonNull), } +/// Type-erased ephemeron registration. +pub(crate) struct EphemeronEntry { + pub(crate) key_ptr: NonNull, + pub(crate) key_alloc_id: usize, + pub(crate) value_ptr: NonNull, +} + pub(crate) struct Collector { // SAFETY: We use 'static here because the PoolAllocator owns its memory, // and we ensure that `Gc` objects and pool allocations do not outlive // the `Collector` instance pub(crate) pool: RefCell>, pool_entries: RefCell>, - pub(crate) sentinel: Pin>, + pub(crate) sentinel: RootSentinel, allocation_count: Cell, pub(crate) generic_alloc_id: Cell, + pub(crate) ephemerons: RefCell>, } impl Collector { @@ -51,12 +59,27 @@ impl Collector { Self { pool: RefCell::new(PoolAllocator::default()), pool_entries: RefCell::new(Vec::new()), - sentinel: Box::pin(RootLink::new()), + sentinel: RootSentinel::new(), allocation_count: Cell::new(0), generic_alloc_id: Cell::new(0), + ephemerons: RefCell::new(Vec::new()), } } + /// Registers an ephemeron key/value pair for processing during collection. + pub(crate) fn register_ephemeron( + &self, + key_ptr: NonNull, + key_alloc_id: usize, + value_ptr: NonNull, + ) { + self.ephemerons.borrow_mut().push(EphemeronEntry { + key_ptr, + key_alloc_id, + value_ptr, + }); + } + /// Allocates a value from the pool. /// /// # Panics @@ -86,41 +109,56 @@ impl Collector { ptr: core::ptr::NonNull, tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer<'_>, ) { - let gcbox_ptr = ptr.cast::>(); + use crate::alloc::mempool3::PoolItem; + let pool_item_ptr = ptr.cast::>>(); unsafe { - (*gcbox_ptr.as_ptr()).value.trace(tracer); + (*pool_item_ptr.as_ptr()).0.value.trace(tracer); } } - let gcbox = GcBox { - marked: Cell::new(false), - trace_fn: Some(trace_value::), - alloc_id, - value, - }; + // Allocate a raw slot for PoolItem> + let size = mem::size_of::>>(); - let layout = Layout::new::>(); - let slot = self - .pool - .borrow_mut() - .try_alloc_bytes(layout) + let mut pool = self.pool.borrow_mut(); + let slot_ptr = pool + .alloc_slot_raw(size) .expect("branded GC: pool allocation failed"); - // SAFETY: `slot` has the correct layout and alignment for `GcBox`. + // SAFETY: slot_ptr points to uninitialized memory of the correct size and alignment. + // We initialize it here before releasing the borrow. let ptr = unsafe { - let ptr = slot.cast::>(); - ptr.as_ptr().write(gcbox); - ptr + use crate::alloc::mempool3::PoolItem; + let pool_item_ptr = slot_ptr.cast::>>(); + + // Initialize the PoolItem> in place + core::ptr::write( + pool_item_ptr.as_ptr(), + PoolItem(GcBox { + marked: Cell::new(false), + trace_fn: trace_value::, + alloc_id, + value, + }), + ); + + pool_item_ptr }; + drop(pool); + unsafe fn drop_and_free( pool: &mut PoolAllocator<'static>, ptr: NonNull, ) { + use crate::alloc::mempool3::PoolItem; unsafe { - ptr.cast::>().as_ref().value.finalize(); - core::ptr::drop_in_place(ptr.cast::>().as_ptr()); - pool.dealloc_bytes(ptr); + let typed_ptr = ptr.cast::>>(); + // Finalize the value + (*typed_ptr.as_ptr()).0.value.finalize(); + // Drop the PoolItem> in place + core::ptr::drop_in_place(typed_ptr.as_ptr()); + // Return the slot to the pool + pool.free_slot(ptr); } } @@ -139,12 +177,7 @@ impl Collector { /// Runs a collection cycle pub(crate) fn collect(&self) { - // SAFETY: sentinel is `Pin>`. - let sentinel_ptr = unsafe { - NonNull::new_unchecked( - self.sentinel.as_ref().get_ref() as *const RootLink as *mut RootLink - ) - }; + let sentinel_ptr = self.sentinel.as_ptr(); let mut tracer = Tracer::new(); @@ -177,41 +210,81 @@ impl Collector { tracer.drain(); + // Phase 2: ephemeron fixpoint. + // If marking a value causes new keys of other ephemerons to become + // reachable, we must iterate until no further values are marked. + loop { + let mut any_newly_marked = false; + for entry in self.ephemerons.borrow().iter() { + use crate::alloc::mempool3::PoolItem; + unsafe { + let key_box = entry.key_ptr.cast::>>(); + // Skip entries invalidated by a previous collection cycle. + if (*key_box.as_ptr()).0.alloc_id != entry.key_alloc_id { + continue; + } + if (*key_box.as_ptr()).0.marked.get() { + let value_box = entry.value_ptr.cast::>>(); + if !(*value_box.as_ptr()).0.marked.replace(true) { + let trace_fn = (*value_box.as_ptr()).0.trace_fn; + tracer.worklist.push((entry.value_ptr, trace_fn)); + any_newly_marked = true; + } + } + } + } + if !any_newly_marked { + break; + } + tracer.drain(); + } + + use crate::alloc::mempool3::PoolItem; let mut pool = self.pool.borrow_mut(); self.pool_entries.borrow_mut().retain_mut(|entry| { - // SAFETY: `ptr` was written with a valid `GcBox`. + // SAFETY: `ptr` was written with a valid `PoolItem>`. let marked = unsafe { - let gcbox = entry.ptr.as_ptr() as *mut GcBox<()>; - (*gcbox).marked.get() + let pool_item = entry.ptr.as_ptr() as *mut PoolItem>; + (*pool_item).0.marked.get() }; if marked { unsafe { - let gcbox = entry.ptr.as_ptr() as *mut GcBox<()>; - (*gcbox).marked.set(false); + let pool_item = entry.ptr.as_ptr() as *mut PoolItem>; + (*pool_item).0.marked.set(false); } true } else { unsafe { - let gcbox = entry.ptr.as_ptr() as *mut GcBox<()>; - (*gcbox).alloc_id = + let pool_item = entry.ptr.as_ptr() as *mut PoolItem>; + (*pool_item).0.alloc_id = crate::collectors::mark_sweep_branded::gc_box::GcBox::<()>::FREED_ALLOC_ID; (entry.drop_fn)(&mut pool, entry.ptr); } false } }); + + // Phase 3: remove ephemeron entries whose key was swept this cycle. + self.ephemerons.borrow_mut().retain(|entry| { + use crate::alloc::mempool3::PoolItem; + unsafe { + let key_box = entry.key_ptr.cast::>>(); + (*key_box.as_ptr()).0.alloc_id == entry.key_alloc_id + } + }); } } impl Drop for Collector { /// Frees all remaining allocations fn drop(&mut self) { + use crate::alloc::mempool3::PoolItem; let mut pool = self.pool.borrow_mut(); for entry in self.pool_entries.borrow().iter() { unsafe { - let gcbox = entry.ptr.as_ptr() as *mut GcBox<()>; - (*gcbox).alloc_id = + let pool_item = entry.ptr.as_ptr() as *mut PoolItem>; + (*pool_item).0.alloc_id = crate::collectors::mark_sweep_branded::gc_box::GcBox::<()>::FREED_ALLOC_ID; (entry.drop_fn)(&mut pool, entry.ptr); } @@ -239,6 +312,11 @@ impl<'id> GcContext<'id> { }; f(&cx) } + + #[cfg(test)] + pub(crate) fn ephemeron_count(&self) -> usize { + self.collector.ephemerons.borrow().len() + } } /// Creates a new GC context. diff --git a/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs b/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs index 9fe85e1..adb2a52 100644 --- a/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs +++ b/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs @@ -2,6 +2,7 @@ use crate::collectors::mark_sweep_branded::{ Collector, + ephemeron::Ephemeron, gc::{Gc, Root, RootNode}, root_link::RootLink, trace::{Finalize, Trace}, @@ -29,7 +30,7 @@ impl<'id, 'gc> MutationContext<'id, 'gc> { /// Downgrades a `Gc` into a weak reference pub fn alloc_weak(&self, gc: Gc<'gc, T>) -> WeakGc<'id, T> { - let alloc_id = unsafe { (*gc.ptr.as_ptr()).alloc_id }; + let alloc_id = unsafe { (*gc.ptr.as_ptr()).0.alloc_id }; WeakGc { ptr: gc.ptr, alloc_id, @@ -51,9 +52,7 @@ impl<'id, 'gc> MutationContext<'id, 'gc> { // SAFETY: `raw` points to a stable `RootNode`. unsafe { - let sentinel_ptr = NonNull::new_unchecked(self.collector.sentinel.as_ref().get_ref() - as *const RootLink - as *mut RootLink); + let sentinel_ptr = self.collector.sentinel.as_ptr(); let link_ptr = raw.cast::(); RootLink::link_after(sentinel_ptr, link_ptr); } @@ -61,6 +60,30 @@ impl<'id, 'gc> MutationContext<'id, 'gc> { Root { raw } } + /// Creates an ephemeron binding `key` to `value`. + /// + /// The value is kept alive by the collector as long as the key remains + /// reachable from a root. Once the key is collected, `get_value` returns + /// `None` and the value is eligible for collection on the next cycle. + pub fn alloc_ephemeron( + &self, + key: Gc<'gc, K>, + value: Gc<'gc, V>, + ) -> Ephemeron<'id, K, V> { + let key_alloc_id = unsafe { (*key.ptr.as_ptr()).0.alloc_id }; + self.collector.register_ephemeron( + key.ptr.cast::(), + key_alloc_id, + value.ptr.cast::(), + ); + Ephemeron { + key_ptr: key.ptr, + key_alloc_id, + value_ptr: value.ptr, + _marker: core::marker::PhantomData, + } + } + /// Triggers a gc cycle. pub fn collect(&self) { self.collector.collect(); diff --git a/oscars/src/collectors/mark_sweep_branded/root_link.rs b/oscars/src/collectors/mark_sweep_branded/root_link.rs index 747e63b..ddfd227 100644 --- a/oscars/src/collectors/mark_sweep_branded/root_link.rs +++ b/oscars/src/collectors/mark_sweep_branded/root_link.rs @@ -69,26 +69,44 @@ impl RootLink { } /// Returns an iterator over all nodes after `sentinel`. - pub(crate) fn iter_from_sentinel( - sentinel: NonNull, - ) -> impl Iterator> { - struct Iter { - current: Option>, - } + pub(crate) fn iter_from_sentinel(sentinel: NonNull) -> RootLinkIter { + // SAFETY: sentinel is pinned in Collector and outlives the iteration. + let first = unsafe { sentinel.as_ref().next.get() }; + RootLinkIter { current: first } + } +} - impl Iterator for Iter { - type Item = NonNull; +/// Iterator over root links in the intrusive list. +pub(crate) struct RootLinkIter { + current: Option>, +} - fn next(&mut self) -> Option { - let node = self.current?; - // SAFETY: nodes are pinned/heap-stable and valid during collection. - self.current = unsafe { node.as_ref().next.get() }; - Some(node) - } - } +impl Iterator for RootLinkIter { + type Item = NonNull; - // SAFETY: sentinel is pinned in Collector and outlives the iteration. - let first = unsafe { sentinel.as_ref().next.get() }; - Iter { current: first } + fn next(&mut self) -> Option { + let node = self.current?; + // SAFETY: nodes are pinned/heap-stable and valid during collection. + self.current = unsafe { node.as_ref().next.get() }; + Some(node) + } +} + +/// Sentinel node for the root list. +#[repr(transparent)] +pub(crate) struct RootSentinel(core::pin::Pin>); + +impl RootSentinel { + /// Creates a new sentinel node + pub(crate) fn new() -> Self { + Self(rust_alloc::boxed::Box::pin(RootLink::new())) + } + + /// Returns a pointer to the underlying RootLink + pub(crate) fn as_ptr(&self) -> NonNull { + // SAFETY: The sentinel is pinned and the pointer is derived from a valid Box. + unsafe { + NonNull::new_unchecked(self.0.as_ref().get_ref() as *const RootLink as *mut RootLink) + } } } diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ephemeron.rs b/oscars/src/collectors/mark_sweep_branded/tests/ephemeron.rs new file mode 100644 index 0000000..1e8a9a3 --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/tests/ephemeron.rs @@ -0,0 +1,109 @@ +use super::*; + +#[test] +fn ephemeron_value_survives_when_key_is_rooted() { + with_gc(|ctx| { + let (root_key, eph) = ctx.mutate(|cx| { + let key = cx.alloc(1u32); + let value = cx.alloc(42u32); + let root_key = cx.root(key); + let eph = cx.alloc_ephemeron(key, value); + (root_key, eph) + }); + + ctx.collect(); + + ctx.mutate(|cx| { + let val = eph + .get_value(cx) + .expect("value must be alive while key is rooted"); + assert_eq!(*val.get(), 42); + drop(root_key); + }); + }); +} + +#[test] +fn ephemeron_value_collected_when_key_unrooted() { + with_gc(|ctx| { + let eph = ctx.mutate(|cx| { + let key = cx.alloc(1u32); + let value = cx.alloc(99u32); + cx.alloc_ephemeron(key, value) + }); + + ctx.collect(); + + ctx.mutate(|cx| { + assert!( + eph.get_value(cx).is_none(), + "value must be gone after key is swept" + ); + }); + }); +} + +#[test] +fn ephemeron_chain_fixpoint() { + // Ephemeron(root_a -> b) and Ephemeron(b -> c): + // root_a alive -> b survives via first ephemeron. + // b then alive -> c survives via second ephemeron + // This requires the collector to run multiple fixpoint passes. + with_gc(|ctx| { + let (root_a, eph_ab, eph_bc) = ctx.mutate(|cx| { + let a = cx.alloc(1u32); + let b = cx.alloc(2u32); + let c = cx.alloc(3u32); + let root_a = cx.root(a); + let eph_ab = cx.alloc_ephemeron(a, b); + let eph_bc = cx.alloc_ephemeron(b, c); + (root_a, eph_ab, eph_bc) + }); + + ctx.collect(); + + ctx.mutate(|cx| { + let b_val = eph_ab.get_value(cx).expect("b must survive: a is rooted"); + assert_eq!(*b_val.get(), 2); + let c_val = eph_bc + .get_value(cx) + .expect("c must survive: b is kept alive by ephemeron"); + assert_eq!(*c_val.get(), 3); + }); + + drop(root_a); + ctx.collect(); + + ctx.mutate(|cx| { + assert!( + eph_ab.get_value(cx).is_none(), + "b must be gone after a is dropped" + ); + assert!( + eph_bc.get_value(cx).is_none(), + "c must be gone after b is gone" + ); + }); + }); +} + +#[test] +fn ephemeron_entry_cleaned_up_after_sweep() { + // Verify the collector's internal ephemeron list shrinks after dead entries are swept. + with_gc(|ctx| { + ctx.mutate(|cx| { + let key = cx.alloc(0u32); + let value = cx.alloc(0u32); + cx.alloc_ephemeron(key, value); + }); + assert_eq!(ctx.ephemeron_count(), 1); + + ctx.collect(); + + assert_eq!( + ctx.ephemeron_count(), + 0, + "dead ephemeron entry must be removed after sweep" + ); + }); +} diff --git a/oscars/src/collectors/mark_sweep_branded/tests/mod.rs b/oscars/src/collectors/mark_sweep_branded/tests/mod.rs index b54064e..cf88fc4 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/mod.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/mod.rs @@ -127,5 +127,6 @@ fn root_escapes_closure_safely() { } mod api_compliance; +mod ephemeron; mod uaf; mod ui_tests; diff --git a/oscars/src/collectors/mark_sweep_branded/trace.rs b/oscars/src/collectors/mark_sweep_branded/trace.rs index 464fd78..b8abf41 100644 --- a/oscars/src/collectors/mark_sweep_branded/trace.rs +++ b/oscars/src/collectors/mark_sweep_branded/trace.rs @@ -1,6 +1,6 @@ //! Trace and Finalize traits for the lifetime branded GC -use crate::collectors::mark_sweep_branded::gc::Gc; +use crate::{alloc::mempool3::PoolItem, collectors::mark_sweep_branded::gc::Gc}; use core::cell::{Cell, OnceCell}; use core::marker::PhantomData; use rust_alloc::borrow::{Cow, ToOwned}; @@ -55,17 +55,18 @@ impl<'a> Tracer<'a> { /// Marks `gc` as reachable. #[inline] pub fn mark(&mut self, gc: &Gc<'_, T>) { - // SAFETY: `gc.ptr` is a valid `GcBox`. + // SAFETY: `gc.ptr` is a valid `PoolItem>`. unsafe { - if !(*gc.ptr.as_ptr()).marked.replace(true) { + if !(*gc.ptr.as_ptr()).0.marked.replace(true) { unsafe fn trace_value( ptr: core::ptr::NonNull, tracer: &mut Tracer<'_>, ) { - let gcbox_ptr = - ptr.cast::>(); + let pool_item_ptr = ptr + .cast::>>( + ); unsafe { - (*gcbox_ptr.as_ptr()).value.trace(tracer); + (*pool_item_ptr.as_ptr()).0.value.trace(tracer); } } @@ -78,17 +79,16 @@ impl<'a> Tracer<'a> { /// /// # Safety /// - /// `ptr` must be a valid pointer to a `GcBox` managed by this collector. + /// `ptr` must be a valid pointer to a `PoolItem>` managed by this collector. #[inline] pub(crate) fn mark_raw(&mut self, ptr: core::ptr::NonNull) { - let boxed_ptr = ptr.cast::>(); + let pool_item_ptr = + ptr.cast::>>(); unsafe { - if !(*boxed_ptr.as_ptr()).marked.replace(true) { - // Call the trace function. - if let Some(trace_fn) = (*boxed_ptr.as_ptr()).trace_fn { - self.worklist.push((ptr, trace_fn)); - } + if !(*pool_item_ptr.as_ptr()).0.marked.replace(true) { + let trace_fn = (*pool_item_ptr.as_ptr()).0.trace_fn; + self.worklist.push((ptr, trace_fn)); } } } diff --git a/oscars/src/collectors/mark_sweep_branded/weak.rs b/oscars/src/collectors/mark_sweep_branded/weak.rs index 5b266b5..c7c5666 100644 --- a/oscars/src/collectors/mark_sweep_branded/weak.rs +++ b/oscars/src/collectors/mark_sweep_branded/weak.rs @@ -1,16 +1,21 @@ //! `WeakGc<'id, T>` for weak references. - -use crate::collectors::mark_sweep_branded::{ - gc::Gc, - gc_box::GcBox, - trace::{Finalize, Trace}, +//! +//! For weak key-value mappings (e.g., weak hash maps) see +//! [`Ephemeron`][crate::collectors::mark_sweep_branded::Ephemeron]. +use crate::{ + alloc::mempool3::PoolItem, + collectors::mark_sweep_branded::{ + gc::Gc, + gc_box::GcBox, + trace::{Finalize, Trace}, + }, }; use core::marker::PhantomData; use core::ptr::NonNull; /// A weak reference to a GC managed value pub struct WeakGc<'id, T: Trace + ?Sized> { - pub(crate) ptr: NonNull>, + pub(crate) ptr: NonNull>>, pub(crate) alloc_id: usize, pub(crate) _marker: PhantomData<*mut &'id ()>, } @@ -24,7 +29,7 @@ impl<'id, T: Trace> WeakGc<'id, T> { // SAFETY: `_cx` proves the `Collector` is alive. // `alloc_id` confirms the allocation is still valid. // The allocator does not unmap memory, so reading a recycled block's `alloc_id` is safe - let is_valid = unsafe { (*self.ptr.as_ptr()).alloc_id == self.alloc_id }; + let is_valid = unsafe { (*self.ptr.as_ptr()).0.alloc_id == self.alloc_id }; if is_valid { Some(Gc { From 4928a182244c3c677f5b87cdff33920d2f74fd17 Mon Sep 17 00:00:00 2001 From: shruti2522 Date: Fri, 24 Apr 2026 19:57:36 +0000 Subject: [PATCH 4/8] replace Tracer with TraceColor --- .../src/collectors/mark_sweep_branded/cell.rs | 6 +- .../mark_sweep_branded/ephemeron.rs | 4 +- .../src/collectors/mark_sweep_branded/gc.rs | 4 +- .../src/collectors/mark_sweep_branded/mod.rs | 21 +-- .../mark_sweep_branded/tests/mod.rs | 2 +- .../mark_sweep_branded/tests/uaf.rs | 2 +- .../collectors/mark_sweep_branded/trace.rs | 155 +++++++++--------- .../src/collectors/mark_sweep_branded/weak.rs | 6 +- oscars_derive/src/mark_sweep_branded.rs | 4 +- 9 files changed, 102 insertions(+), 102 deletions(-) diff --git a/oscars/src/collectors/mark_sweep_branded/cell.rs b/oscars/src/collectors/mark_sweep_branded/cell.rs index ebc2fa4..249efc6 100644 --- a/oscars/src/collectors/mark_sweep_branded/cell.rs +++ b/oscars/src/collectors/mark_sweep_branded/cell.rs @@ -1,6 +1,6 @@ //! Interior mutability for GC-managed values. -use crate::collectors::mark_sweep_branded::trace::{Finalize, Trace, Tracer}; +use crate::collectors::mark_sweep_branded::trace::{Finalize, Trace, TraceColor}; use core::cell::{Ref, RefCell, RefMut}; use core::ops::{Deref, DerefMut}; @@ -65,7 +65,7 @@ impl DerefMut for GcRefMut<'_, T> { impl Finalize for GcRefCell {} impl Trace for GcRefCell { - fn trace(&mut self, tracer: &mut Tracer) { - self.inner.get_mut().trace(tracer); + fn trace(&self, color: &TraceColor) { + self.inner.borrow().trace(color); } } diff --git a/oscars/src/collectors/mark_sweep_branded/ephemeron.rs b/oscars/src/collectors/mark_sweep_branded/ephemeron.rs index 9b75638..14ee162 100644 --- a/oscars/src/collectors/mark_sweep_branded/ephemeron.rs +++ b/oscars/src/collectors/mark_sweep_branded/ephemeron.rs @@ -4,7 +4,7 @@ use crate::{ gc::Gc, gc_box::GcBox, mutation_ctx::MutationContext, - trace::{Finalize, Trace, Tracer}, + trace::{Finalize, Trace, TraceColor}, }, }; use core::marker::PhantomData; @@ -44,5 +44,5 @@ impl<'id, K: Trace, V: Trace> Copy for Ephemeron<'id, K, V> {} impl<'id, K: Trace, V: Trace> Finalize for Ephemeron<'id, K, V> {} impl<'id, K: Trace, V: Trace> Trace for Ephemeron<'id, K, V> { - fn trace(&mut self, _tracer: &mut Tracer) {} + fn trace(&self, _color: &TraceColor) {} } diff --git a/oscars/src/collectors/mark_sweep_branded/gc.rs b/oscars/src/collectors/mark_sweep_branded/gc.rs index 85f5641..c1b8d5a 100644 --- a/oscars/src/collectors/mark_sweep_branded/gc.rs +++ b/oscars/src/collectors/mark_sweep_branded/gc.rs @@ -97,7 +97,7 @@ impl<'id, T: Trace> Drop for Root<'id, T> { impl Finalize for Gc<'_, T> {} impl Trace for Gc<'_, T> { - fn trace(&mut self, tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) { - tracer.mark(self); + fn trace(&self, color: &crate::collectors::mark_sweep_branded::trace::TraceColor) { + color.mark(self); } } diff --git a/oscars/src/collectors/mark_sweep_branded/mod.rs b/oscars/src/collectors/mark_sweep_branded/mod.rs index 7ce9e09..4de08aa 100644 --- a/oscars/src/collectors/mark_sweep_branded/mod.rs +++ b/oscars/src/collectors/mark_sweep_branded/mod.rs @@ -17,7 +17,7 @@ pub use cell::GcRefCell; pub use ephemeron::Ephemeron; pub use gc::{Gc, Root}; pub use mutation_ctx::MutationContext; -pub use trace::{Finalize, Trace, Tracer}; +pub use trace::{Finalize, Trace, TraceColor}; pub use weak::WeakGc; use crate::alloc::mempool3::PoolAllocator; @@ -107,12 +107,12 @@ impl Collector { unsafe fn trace_value( ptr: core::ptr::NonNull, - tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer<'_>, + color: &crate::collectors::mark_sweep_branded::trace::TraceColor<'_>, ) { use crate::alloc::mempool3::PoolItem; let pool_item_ptr = ptr.cast::>>(); unsafe { - (*pool_item_ptr.as_ptr()).0.value.trace(tracer); + (*pool_item_ptr.as_ptr()).0.value.trace(color); } } @@ -179,7 +179,7 @@ impl Collector { pub(crate) fn collect(&self) { let sentinel_ptr = self.sentinel.as_ptr(); - let mut tracer = Tracer::new(); + let color = TraceColor::new(); let gc_ptr_offset = core::mem::offset_of!( crate::collectors::mark_sweep_branded::gc::RootNode<'static, i32>, @@ -204,11 +204,11 @@ impl Collector { .cast::>(); let gc_ptr = gc_ptr_ptr.read(); - tracer.mark_raw(gc_ptr.cast::()); + color.mark_raw(gc_ptr.cast::()); } } - tracer.drain(); + color.drain(); // Phase 2: ephemeron fixpoint. // If marking a value causes new keys of other ephemerons to become @@ -224,19 +224,14 @@ impl Collector { continue; } if (*key_box.as_ptr()).0.marked.get() { - let value_box = entry.value_ptr.cast::>>(); - if !(*value_box.as_ptr()).0.marked.replace(true) { - let trace_fn = (*value_box.as_ptr()).0.trace_fn; - tracer.worklist.push((entry.value_ptr, trace_fn)); - any_newly_marked = true; - } + any_newly_marked |= color.mark_raw(entry.value_ptr); } } } if !any_newly_marked { break; } - tracer.drain(); + color.drain(); } use crate::alloc::mempool3::PoolItem; diff --git a/oscars/src/collectors/mark_sweep_branded/tests/mod.rs b/oscars/src/collectors/mark_sweep_branded/tests/mod.rs index cf88fc4..3bc4d6d 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/mod.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/mod.rs @@ -7,7 +7,7 @@ struct JsObject { } impl crate::collectors::mark_sweep_branded::Trace for JsObject { - fn trace(&mut self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {} + fn trace(&self, _color: &crate::collectors::mark_sweep_branded::trace::TraceColor) {} } impl crate::collectors::mark_sweep_branded::Finalize for JsObject {} diff --git a/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs b/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs index fad39e2..3539e98 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs @@ -6,7 +6,7 @@ use core::cell::Cell; struct DetectDrop<'a>(&'a Cell); impl<'a> Trace for DetectDrop<'a> { - fn trace(&mut self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {} + fn trace(&self, _color: &crate::collectors::mark_sweep_branded::trace::TraceColor) {} } impl Finalize for DetectDrop<'_> {} diff --git a/oscars/src/collectors/mark_sweep_branded/trace.rs b/oscars/src/collectors/mark_sweep_branded/trace.rs index b8abf41..9ce5c8f 100644 --- a/oscars/src/collectors/mark_sweep_branded/trace.rs +++ b/oscars/src/collectors/mark_sweep_branded/trace.rs @@ -1,7 +1,7 @@ //! Trace and Finalize traits for the lifetime branded GC use crate::{alloc::mempool3::PoolItem, collectors::mark_sweep_branded::gc::Gc}; -use core::cell::{Cell, OnceCell}; +use core::cell::{Cell, OnceCell, RefCell}; use core::marker::PhantomData; use rust_alloc::borrow::{Cow, ToOwned}; use rust_alloc::boxed::Box; @@ -16,79 +16,87 @@ pub use crate::collectors::common::Finalize; /// /// # Safety /// -/// Use `Tracer::mark` for every reachable `Gc` pointer. +/// Use `TraceColor::mark` for every reachable `Gc` pointer. pub trait Trace { /// Marks all `Gc` pointers reachable from `self`. - fn trace(&mut self, tracer: &mut Tracer); + fn trace(&self, color: &TraceColor); } -pub(crate) type TraceFn = unsafe fn(core::ptr::NonNull, &mut Tracer<'_>); +pub(crate) type TraceFn = unsafe fn(core::ptr::NonNull, &TraceColor<'_>); -/// Callback handle passed to `Trace::trace`. +/// Opaque token threaded through a collection cycle. /// -/// The `'a` lifetime ties the tracer to the collection cycle, +/// The `'a` lifetime ties the color to the collection cycle, /// preventing it from being stored or escaping the collector. -pub struct Tracer<'a> { - pub(crate) worklist: Vec<(core::ptr::NonNull, TraceFn)>, +pub struct TraceColor<'a> { + pub(crate) worklist: RefCell, TraceFn)>>, pub(crate) _marker: PhantomData<&'a ()>, } -impl<'a> Tracer<'a> { +impl<'a> TraceColor<'a> { pub(crate) fn new() -> Self { Self { - worklist: Vec::new(), + worklist: RefCell::new(Vec::new()), _marker: PhantomData, } } - pub(crate) fn drain(&mut self) { + pub(crate) fn drain(&self) { // Note: Using `pop()` processes the worklist in LIFO order (Depth-First Search). // While correct, heap-allocated object graphs often exhibit better cache locality // with Breadth-First Search. This could be evaluated with a `VecDeque` in the future. - while let Some((ptr, trace_fn)) = self.worklist.pop() { - unsafe { - (trace_fn)(ptr, self); + loop { + // Drop the borrow before calling trace_fn to allow re-entrant marks. + let item = self.worklist.borrow_mut().pop(); + match item { + Some((ptr, trace_fn)) => unsafe { (trace_fn)(ptr, self) }, + None => break, } } } /// Marks `gc` as reachable. #[inline] - pub fn mark(&mut self, gc: &Gc<'_, T>) { + pub fn mark(&self, gc: &Gc<'_, T>) { // SAFETY: `gc.ptr` is a valid `PoolItem>`. unsafe { if !(*gc.ptr.as_ptr()).0.marked.replace(true) { unsafe fn trace_value( ptr: core::ptr::NonNull, - tracer: &mut Tracer<'_>, + color: &TraceColor<'_>, ) { let pool_item_ptr = ptr .cast::>>( ); unsafe { - (*pool_item_ptr.as_ptr()).0.value.trace(tracer); + (*pool_item_ptr.as_ptr()).0.value.trace(color); } } - self.worklist.push((gc.ptr.cast::(), trace_value::)); + self.worklist + .borrow_mut() + .push((gc.ptr.cast::(), trace_value::)); } } } - /// Marks a raw allocation as reachable. + /// Marks a raw allocation as reachable, returning `true` if newly marked. /// /// # Safety /// /// `ptr` must be a valid pointer to a `PoolItem>` managed by this collector. #[inline] - pub(crate) fn mark_raw(&mut self, ptr: core::ptr::NonNull) { + pub(crate) fn mark_raw(&self, ptr: core::ptr::NonNull) -> bool { let pool_item_ptr = ptr.cast::>>(); unsafe { if !(*pool_item_ptr.as_ptr()).0.marked.replace(true) { let trace_fn = (*pool_item_ptr.as_ptr()).0.trace_fn; - self.worklist.push((ptr, trace_fn)); + self.worklist.borrow_mut().push((ptr, trace_fn)); + true + } else { + false } } } @@ -96,7 +104,7 @@ impl<'a> Tracer<'a> { impl Trace for &T { #[inline] - fn trace(&mut self, _tracer: &mut Tracer) {} + fn trace(&self, _color: &TraceColor) {} } // primitive + std-lib Trace impls @@ -106,7 +114,7 @@ macro_rules! empty_trace { $( impl Trace for $T { #[inline] - fn trace(&mut self, _tracer: &mut Tracer) {} + fn trace(&self, _color: &TraceColor) {} } )* }; @@ -146,80 +154,79 @@ empty_trace![ ]; impl Trace for [T; N] { - fn trace(&mut self, tracer: &mut Tracer) { - for v in self.iter_mut() { - v.trace(tracer); + fn trace(&self, color: &TraceColor) { + for v in self.iter() { + v.trace(color); } } } impl Trace for Box { - fn trace(&mut self, tracer: &mut Tracer) { - (**self).trace(tracer); + fn trace(&self, color: &TraceColor) { + (**self).trace(color); } } impl Trace for Option { - fn trace(&mut self, tracer: &mut Tracer) { + fn trace(&self, color: &TraceColor) { if let Some(v) = self { - v.trace(tracer); + v.trace(color); } } } impl Trace for Result { - fn trace(&mut self, tracer: &mut Tracer) { + fn trace(&self, color: &TraceColor) { match self { - Ok(v) => v.trace(tracer), - Err(e) => e.trace(tracer), + Ok(v) => v.trace(color), + Err(e) => e.trace(color), } } } impl Trace for Vec { - fn trace(&mut self, tracer: &mut Tracer) { - for v in self.iter_mut() { - v.trace(tracer); + fn trace(&self, color: &TraceColor) { + for v in self.iter() { + v.trace(color); } } } impl Trace for VecDeque { - fn trace(&mut self, tracer: &mut Tracer) { - for v in self.iter_mut() { - v.trace(tracer); + fn trace(&self, color: &TraceColor) { + for v in self.iter() { + v.trace(color); } } } impl Trace for LinkedList { - fn trace(&mut self, tracer: &mut Tracer) { - for v in self.iter_mut() { - v.trace(tracer); + fn trace(&self, color: &TraceColor) { + for v in self.iter() { + v.trace(color); } } } impl Trace for PhantomData { #[inline] - fn trace(&mut self, _tracer: &mut Tracer) {} + fn trace(&self, _color: &TraceColor) {} } -// Cell> requires T: Copy to safely take and restore the value. +// Cell> requires T: Copy to safely read the value via Cell::get(). // For non-Copy types, use GcRefCell instead. impl Trace for Cell> { - fn trace(&mut self, tracer: &mut Tracer) { - if let Some(mut v) = self.get_mut().take() { - v.trace(tracer); - self.set(Some(v)); + fn trace(&self, color: &TraceColor) { + if let Some(v) = self.get() { + v.trace(color); } } } impl Trace for OnceCell { - fn trace(&mut self, tracer: &mut Tracer) { - if let Some(v) = self.get_mut() { - v.trace(tracer); + fn trace(&self, color: &TraceColor) { + if let Some(v) = self.get() { + v.trace(color); } } } @@ -228,44 +235,44 @@ impl Trace for Cow<'static, T> where T::Owned: Trace, { - fn trace(&mut self, tracer: &mut Tracer) { + fn trace(&self, color: &TraceColor) { if let Cow::Owned(v) = self { - v.trace(tracer); + v.trace(color); } } } impl Trace for (A,) { #[inline] - fn trace(&mut self, tracer: &mut Tracer) { - self.0.trace(tracer); + fn trace(&self, color: &TraceColor) { + self.0.trace(color); } } impl Trace for (A, B) { #[inline] - fn trace(&mut self, tracer: &mut Tracer) { - self.0.trace(tracer); - self.1.trace(tracer); + fn trace(&self, color: &TraceColor) { + self.0.trace(color); + self.1.trace(color); } } impl Trace for (A, B, C) { #[inline] - fn trace(&mut self, tracer: &mut Tracer) { - self.0.trace(tracer); - self.1.trace(tracer); - self.2.trace(tracer); + fn trace(&self, color: &TraceColor) { + self.0.trace(color); + self.1.trace(color); + self.2.trace(color); } } impl Trace for (A, B, C, D) { #[inline] - fn trace(&mut self, tracer: &mut Tracer) { - self.0.trace(tracer); - self.1.trace(tracer); - self.2.trace(tracer); - self.3.trace(tracer); + fn trace(&self, color: &TraceColor) { + self.0.trace(color); + self.1.trace(color); + self.2.trace(color); + self.3.trace(color); } } @@ -274,26 +281,26 @@ impl Trace for (A, B, C, D) { // struct instead. impl Trace for rust_alloc::rc::Rc { #[inline] - fn trace(&mut self, _tracer: &mut Tracer) {} + fn trace(&self, _color: &TraceColor) {} } impl Trace for rust_alloc::sync::Arc { #[inline] - fn trace(&mut self, _tracer: &mut Tracer) {} + fn trace(&self, _color: &TraceColor) {} } impl Trace for BTreeMap { - fn trace(&mut self, tracer: &mut Tracer) { - for v in self.values_mut() { - v.trace(tracer); + fn trace(&self, color: &TraceColor) { + for v in self.values() { + v.trace(color); } } } impl Trace for BTreeSet { #[inline] - fn trace(&mut self, _tracer: &mut Tracer) { + fn trace(&self, _color: &TraceColor) { // BTreeSet keys are immutable and cannot contain Gc pointers - // that need tracing (Gc requires &mut self to trace). + // that need tracing (Gc requires &self to trace). } } diff --git a/oscars/src/collectors/mark_sweep_branded/weak.rs b/oscars/src/collectors/mark_sweep_branded/weak.rs index c7c5666..e5bc596 100644 --- a/oscars/src/collectors/mark_sweep_branded/weak.rs +++ b/oscars/src/collectors/mark_sweep_branded/weak.rs @@ -1,7 +1,5 @@ //! `WeakGc<'id, T>` for weak references. -//! -//! For weak key-value mappings (e.g., weak hash maps) see -//! [`Ephemeron`][crate::collectors::mark_sweep_branded::Ephemeron]. + use crate::{ alloc::mempool3::PoolItem, collectors::mark_sweep_branded::{ @@ -53,5 +51,5 @@ impl<'id, T: Trace + ?Sized> Copy for WeakGc<'id, T> {} impl<'id, T: Trace> Finalize for WeakGc<'id, T> {} impl<'id, T: Trace> Trace for WeakGc<'id, T> { // Weak references do not mark their target, upgrade() returning None after collection is the intended behaviour. - fn trace(&mut self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {} + fn trace(&self, _color: &crate::collectors::mark_sweep_branded::trace::TraceColor) {} } diff --git a/oscars_derive/src/mark_sweep_branded.rs b/oscars_derive/src/mark_sweep_branded.rs index 4f81abb..235ccfb 100644 --- a/oscars_derive/src/mark_sweep_branded.rs +++ b/oscars_derive/src/mark_sweep_branded.rs @@ -16,7 +16,7 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { }); let trace_body = s.each(|bi| { - quote!(::oscars::collectors::mark_sweep_branded::Trace::trace(#bi, tracer)) + quote!(::oscars::collectors::mark_sweep_branded::Trace::trace(#bi, color)) }); s.add_bounds(AddBounds::Fields); @@ -24,7 +24,7 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { quote!(::oscars::collectors::mark_sweep_branded::Trace), quote! { #[inline] - fn trace(&mut self, tracer: &mut ::oscars::collectors::mark_sweep_branded::Tracer) { + fn trace(&self, color: &::oscars::collectors::mark_sweep_branded::TraceColor) { match *self { #trace_body } } }, From 2fbadae1ea76364f759b50e44a24134228509cbf Mon Sep 17 00:00:00 2001 From: shruti2522 Date: Fri, 24 Apr 2026 20:40:17 +0000 Subject: [PATCH 5/8] remove pool_entries Vec and allocation_count --- oscars/src/alloc/mempool3/alloc.rs | 12 +++ oscars/src/alloc/mempool3/mod.rs | 7 ++ .../collectors/mark_sweep_branded/gc_box.rs | 8 +- .../src/collectors/mark_sweep_branded/mod.rs | 83 +++++++++---------- 4 files changed, 64 insertions(+), 46 deletions(-) diff --git a/oscars/src/alloc/mempool3/alloc.rs b/oscars/src/alloc/mempool3/alloc.rs index d7ea091..9da0a95 100644 --- a/oscars/src/alloc/mempool3/alloc.rs +++ b/oscars/src/alloc/mempool3/alloc.rs @@ -310,6 +310,18 @@ impl SlotPool { self.live.set(self.live.get().saturating_sub(1)); } + /// Iterates over all live (allocated) slot pointers in this pool. + pub(crate) fn iter_live(&self) -> impl Iterator> + '_ { + (0..self.slot_count).filter_map(move |i| { + let chunk = self.bitmap_chunk(i); + if chunk.get() & (1u64 << (i % 64)) != 0 { + Some(self.slot_ptr(i)) + } else { + None + } + }) + } + /// returns true when the pool is empty and safe to drop /// `live` tracks the count, so no bitmap scan is needed pub fn run_drop_check(&self) -> bool { diff --git a/oscars/src/alloc/mempool3/mod.rs b/oscars/src/alloc/mempool3/mod.rs index 159626d..93b21d4 100644 --- a/oscars/src/alloc/mempool3/mod.rs +++ b/oscars/src/alloc/mempool3/mod.rs @@ -121,6 +121,13 @@ impl<'alloc> PoolAllocator<'alloc> { self.current_heap_size } + /// Iterates over every live slot pointer across all slot pools. + /// + /// Yields one `NonNull` per allocated (not yet freed) slot. + pub fn iter_live_slots(&self) -> impl Iterator> + '_ { + self.slot_pools.iter().flat_map(|pool| pool.iter_live()) + } + pub fn is_below_threshold(&self) -> bool { // keep 25% headroom so collection fires before the last page fills let margin = self.heap_threshold / 4; diff --git a/oscars/src/collectors/mark_sweep_branded/gc_box.rs b/oscars/src/collectors/mark_sweep_branded/gc_box.rs index 4ee8da7..73bc2b6 100644 --- a/oscars/src/collectors/mark_sweep_branded/gc_box.rs +++ b/oscars/src/collectors/mark_sweep_branded/gc_box.rs @@ -1,17 +1,23 @@ //! The heap header wrapping every GC-managed value. use core::cell::Cell; +use core::ptr::NonNull; +use crate::alloc::mempool3::PoolAllocator; use crate::collectors::mark_sweep_branded::trace::TraceFn; +pub(crate) type DropFn = unsafe fn(&mut PoolAllocator<'static>, NonNull); + /// Heap wrapper for a garbage-collected value. /// -/// Allocated via [`PoolAllocator`][crate::alloc::mempool3::PoolAllocator]. +/// Allocated via [`PoolAllocator`]. pub(crate) struct GcBox { /// Reachability flag set by the mark phase. pub(crate) marked: Cell, /// Type-erased trace function. pub(crate) trace_fn: TraceFn, + /// Type-erased finalize and free fn + pub(crate) drop_fn: DropFn, /// Allocation ID used to validate weak pointers. pub(crate) alloc_id: usize, /// The user value. diff --git a/oscars/src/collectors/mark_sweep_branded/mod.rs b/oscars/src/collectors/mark_sweep_branded/mod.rs index 4de08aa..89e567d 100644 --- a/oscars/src/collectors/mark_sweep_branded/mod.rs +++ b/oscars/src/collectors/mark_sweep_branded/mod.rs @@ -25,16 +25,10 @@ use core::cell::{Cell, RefCell}; use core::marker::PhantomData; use core::mem; use core::ptr::NonNull; -use gc_box::GcBox; +use gc_box::{DropFn, GcBox}; use root_link::{RootLink, RootSentinel}; use rust_alloc::vec::Vec; -/// Erased drop-fn -struct PoolEntry { - ptr: NonNull, - drop_fn: unsafe fn(&mut PoolAllocator<'static>, NonNull), -} - /// Type-erased ephemeron registration. pub(crate) struct EphemeronEntry { pub(crate) key_ptr: NonNull, @@ -47,9 +41,7 @@ pub(crate) struct Collector { // and we ensure that `Gc` objects and pool allocations do not outlive // the `Collector` instance pub(crate) pool: RefCell>, - pool_entries: RefCell>, pub(crate) sentinel: RootSentinel, - allocation_count: Cell, pub(crate) generic_alloc_id: Cell, pub(crate) ephemerons: RefCell>, } @@ -58,9 +50,7 @@ impl Collector { fn new() -> Self { Self { pool: RefCell::new(PoolAllocator::default()), - pool_entries: RefCell::new(Vec::new()), sentinel: RootSentinel::new(), - allocation_count: Cell::new(0), generic_alloc_id: Cell::new(0), ephemerons: RefCell::new(Vec::new()), } @@ -136,6 +126,7 @@ impl Collector { PoolItem(GcBox { marked: Cell::new(false), trace_fn: trace_value::, + drop_fn: drop_and_free::, alloc_id, value, }), @@ -162,13 +153,6 @@ impl Collector { } } - self.pool_entries.borrow_mut().push(PoolEntry { - ptr: ptr.cast::(), - drop_fn: drop_and_free::, - }); - - self.allocation_count.set(self.allocation_count.get() + 1); - Gc { ptr, _marker: PhantomData, @@ -234,33 +218,34 @@ impl Collector { color.drain(); } + // Phase 3: sweep all slots. Collect unmarked ones, then invalidate and free them. use crate::alloc::mempool3::PoolItem; - let mut pool = self.pool.borrow_mut(); - self.pool_entries.borrow_mut().retain_mut(|entry| { - // SAFETY: `ptr` was written with a valid `PoolItem>`. - let marked = unsafe { - let pool_item = entry.ptr.as_ptr() as *mut PoolItem>; - (*pool_item).0.marked.get() - }; - - if marked { - unsafe { - let pool_item = entry.ptr.as_ptr() as *mut PoolItem>; - (*pool_item).0.marked.set(false); - } - true - } else { + let dead: Vec<(NonNull, DropFn)> = { + let pool = self.pool.borrow(); + pool.iter_live_slots() + .filter_map(|ptr| unsafe { + let gc_box = &(*ptr.cast::>>().as_ptr()).0; + if gc_box.marked.get() { + gc_box.marked.set(false); + None + } else { + Some((ptr, gc_box.drop_fn)) + } + }) + .collect() + }; + { + let mut pool = self.pool.borrow_mut(); + for (ptr, drop_fn) in dead { unsafe { - let pool_item = entry.ptr.as_ptr() as *mut PoolItem>; - (*pool_item).0.alloc_id = - crate::collectors::mark_sweep_branded::gc_box::GcBox::<()>::FREED_ALLOC_ID; - (entry.drop_fn)(&mut pool, entry.ptr); + (*ptr.cast::>>().as_ptr()).0.alloc_id = + GcBox::<()>::FREED_ALLOC_ID; + (drop_fn)(&mut pool, ptr); } - false } - }); + } - // Phase 3: remove ephemeron entries whose key was swept this cycle. + // Phase 4: remove ephemeron entries whose key was swept this cycle. self.ephemerons.borrow_mut().retain(|entry| { use crate::alloc::mempool3::PoolItem; unsafe { @@ -275,13 +260,21 @@ impl Drop for Collector { /// Frees all remaining allocations fn drop(&mut self) { use crate::alloc::mempool3::PoolItem; + let all: Vec<(NonNull, DropFn)> = self + .pool + .borrow() + .iter_live_slots() + .map(|ptr| unsafe { + let drop_fn = (*ptr.cast::>>().as_ptr()).0.drop_fn; + (ptr, drop_fn) + }) + .collect(); let mut pool = self.pool.borrow_mut(); - for entry in self.pool_entries.borrow().iter() { + for (ptr, drop_fn) in all { unsafe { - let pool_item = entry.ptr.as_ptr() as *mut PoolItem>; - (*pool_item).0.alloc_id = - crate::collectors::mark_sweep_branded::gc_box::GcBox::<()>::FREED_ALLOC_ID; - (entry.drop_fn)(&mut pool, entry.ptr); + (*ptr.cast::>>().as_ptr()).0.alloc_id = + GcBox::<()>::FREED_ALLOC_ID; + (drop_fn)(&mut pool, ptr); } } } From f5d3154bced46585ecc4c51fd6d3efab8eb7149c Mon Sep 17 00:00:00 2001 From: shruti2522 Date: Fri, 24 Apr 2026 21:01:54 +0000 Subject: [PATCH 6/8] implement tri-color marking --- ...mark_sweep_branded_implementation_notes.md | 78 ------- oscars/src/alloc/mempool3/alloc.rs | 7 +- oscars/src/alloc/mempool3/mod.rs | 61 ------ .../src/collectors/mark_sweep_branded/cell.rs | 6 +- .../mark_sweep_branded/ephemeron.rs | 4 +- .../src/collectors/mark_sweep_branded/gc.rs | 25 ++- .../collectors/mark_sweep_branded/gc_box.rs | 46 ++++- .../src/collectors/mark_sweep_branded/mod.rs | 149 ++++++++------ .../mark_sweep_branded/mutation_ctx.rs | 14 +- .../mark_sweep_branded/root_link.rs | 13 +- .../tests/api_compliance.rs | 2 + .../mark_sweep_branded/tests/mod.rs | 2 +- .../mark_sweep_branded/tests/uaf.rs | 2 +- .../collectors/mark_sweep_branded/trace.rs | 190 +++++++++--------- .../src/collectors/mark_sweep_branded/weak.rs | 2 +- 15 files changed, 267 insertions(+), 334 deletions(-) delete mode 100644 notes/mark_sweep_branded_implementation_notes.md diff --git a/notes/mark_sweep_branded_implementation_notes.md b/notes/mark_sweep_branded_implementation_notes.md deleted file mode 100644 index b02dcd5..0000000 --- a/notes/mark_sweep_branded_implementation_notes.md +++ /dev/null @@ -1,78 +0,0 @@ -**Date**: 2026-04-23 - -## Changes from API Redesign Proposal - -### 1. Allocation ID for Weak References (ABA Protection) - -**Added:** -- `alloc_id: usize` in `GcBox` and `WeakGc<'id, T>` -- `FREED_ALLOC_ID = usize::MAX` constant -- Validation check in `WeakGc::upgrade` - -**Why needed:** -Pool allocators reuse memory slots. Without IDs, a weak pointer could point to the wrong object after the slot is reused. - -**How it works:** -- Each allocation gets a unique ID -- Freed slots get ID set to `usize::MAX` -- `WeakGc::upgrade` checks if IDs match -- If IDs don't match, slot was reused, return `None` - -**Industry standard:** -V8 and SpiderMonkey use the same technique. Required for soundness with pool allocators. - -### 2. Allocation ID Wrap Check - -**Added:** -```rust -assert_ne!(alloc_id, FREED_ALLOC_ID, "..."); -``` - -**Why:** -If the ID counter wraps to `usize::MAX`, weak reference validation breaks. This check prevents silent corruption. - -**Practical impact:** -Requires 2^64 allocations on 64-bit systems (impossible in practice). - -### 3. Additional Trace Implementations - -**Added:** -- `BTreeMap` (traces values only) -- `BTreeSet` (no-op, keys are immutable) -- 3-tuple and 4-tuple -- Comments for `Rc`, `Arc`, `Cell>` - -**Why:** -Needed for real Boa code. Keys in BTree collections are immutable, so they cannot contain `Gc` pointers (which need `&mut self` to trace). - -**Note:** -`HashMap` and `HashSet` are in `std::collections`, not available in `no_std` builds. - -### 4. Cell> Requires T: Copy - -**Fixed:** -```rust -impl Trace for Cell> -``` - -**Why:** -`self.set(Some(v))` requires moving `v`, which needs `T: Copy`. Without this bound, code fails to compile. - -**Alternative:** -Use `GcRefCell` for non Copy types. - -## Design Decisions - -### Trace::trace uses &mut self - -Follows the proposal exactly. Allows future moving collectors to update internal pointers during tracing. - -**Impact:** -Collection keys (HashMap, BTreeMap) cannot contain `Gc` pointers because keys are immutable. - -### collect() uses &self not &mut self - -Both `GcContext::collect` and `MutationContext::collect` use `&self` with interior mutability via `RefCell`. - -**Why:** -Allows calling `collect()` inside `mutate()` closures without borrow conflicts. diff --git a/oscars/src/alloc/mempool3/alloc.rs b/oscars/src/alloc/mempool3/alloc.rs index 9da0a95..2b1941f 100644 --- a/oscars/src/alloc/mempool3/alloc.rs +++ b/oscars/src/alloc/mempool3/alloc.rs @@ -61,14 +61,17 @@ impl<'pool> ErasedPoolPointer<'pool> { /// typed pointer into a pool slot #[derive(Debug, Clone, Copy)] #[repr(transparent)] -pub struct PoolPointer<'pool, T>(NonNull>, PhantomData<&'pool T>); +pub struct PoolPointer<'pool, T>(NonNull>, PhantomData<(&'pool (), *mut T)>); impl<'pool, T> PoolPointer<'pool, T> { pub(crate) unsafe fn from_raw(raw: NonNull>) -> Self { Self(raw, PhantomData) } - pub fn as_inner_ref(&self) -> &'pool T { + pub fn as_inner_ref(&self) -> &'pool T + where + T: 'pool, + { // SAFETY: pointer is valid and properly aligned unsafe { &(*self.0.as_ptr()).0 } } diff --git a/oscars/src/alloc/mempool3/mod.rs b/oscars/src/alloc/mempool3/mod.rs index 93b21d4..f879ac7 100644 --- a/oscars/src/alloc/mempool3/mod.rs +++ b/oscars/src/alloc/mempool3/mod.rs @@ -255,67 +255,6 @@ impl<'alloc> PoolAllocator<'alloc> { } } - /// Allocates a raw slot of the given size without type constraints. - /// Returns a pointer to uninitialized memory. - /// - /// # Safety - /// Caller must initialize the memory before use and ensure proper cleanup. - pub fn alloc_slot_raw(&mut self, size: usize) -> Result, PoolAllocError> { - let sc_idx = size_class_index_for(size); - let slot_size = SIZE_CLASSES.get(sc_idx).copied().unwrap_or(size); - - let cached_idx = self.alloc_cache[sc_idx].get(); - if cached_idx < self.slot_pools.len() { - let pool = &self.slot_pools[cached_idx]; - if pool.slot_size == slot_size - && let Some(slot_ptr) = pool.alloc_slot() - { - return Ok(slot_ptr); - } - } - - // try existing pools with matching slot_size first - for (i, pool) in self.slot_pools.iter().enumerate().rev() { - if pool.slot_size == slot_size - && let Some(slot_ptr) = pool.alloc_slot() - { - self.alloc_cache[sc_idx].set(i); - return Ok(slot_ptr); - } - } - - // need a new pool for this size class - // try the recycle list first - if let Some(pos) = self - .recycled_pools - .iter() - .rposition(|p| p.slot_size == slot_size) - { - let pool = self.recycled_pools.swap_remove(pos); - let slot_ptr = pool.alloc_slot().ok_or(PoolAllocError::OutOfMemory)?; - let insert_idx = self.slot_pools.len(); - let (base, end) = pool.slot_range(); - let spos = self.sorted_ranges.partition_point(|&(b, _, _)| b < base); - self.sorted_ranges.insert(spos, (base, end, insert_idx)); - self.slot_pools.push(pool); - self.alloc_cache[sc_idx].set(insert_idx); - return Ok(slot_ptr); - } - - // Allocate a fresh page from the OS - let total = self.page_size.max(slot_size * 4); - let new_pool = SlotPool::try_init(slot_size, total, 16)?; - self.current_heap_size += new_pool.layout.size(); - let slot_ptr = new_pool.alloc_slot().ok_or(PoolAllocError::OutOfMemory)?; - let insert_idx = self.slot_pools.len(); - let (base, end) = new_pool.slot_range(); - let spos = self.sorted_ranges.partition_point(|&(b, _, _)| b < base); - self.sorted_ranges.insert(spos, (base, end, insert_idx)); - self.slot_pools.push(new_pool); - self.alloc_cache[sc_idx].set(insert_idx); - Ok(slot_ptr) - } - /// drops the value at `ptr` and returns the slot to the allocator /// /// # Safety diff --git a/oscars/src/collectors/mark_sweep_branded/cell.rs b/oscars/src/collectors/mark_sweep_branded/cell.rs index 249efc6..ebc2fa4 100644 --- a/oscars/src/collectors/mark_sweep_branded/cell.rs +++ b/oscars/src/collectors/mark_sweep_branded/cell.rs @@ -1,6 +1,6 @@ //! Interior mutability for GC-managed values. -use crate::collectors::mark_sweep_branded::trace::{Finalize, Trace, TraceColor}; +use crate::collectors::mark_sweep_branded::trace::{Finalize, Trace, Tracer}; use core::cell::{Ref, RefCell, RefMut}; use core::ops::{Deref, DerefMut}; @@ -65,7 +65,7 @@ impl DerefMut for GcRefMut<'_, T> { impl Finalize for GcRefCell {} impl Trace for GcRefCell { - fn trace(&self, color: &TraceColor) { - self.inner.borrow().trace(color); + fn trace(&mut self, tracer: &mut Tracer) { + self.inner.get_mut().trace(tracer); } } diff --git a/oscars/src/collectors/mark_sweep_branded/ephemeron.rs b/oscars/src/collectors/mark_sweep_branded/ephemeron.rs index 14ee162..9b75638 100644 --- a/oscars/src/collectors/mark_sweep_branded/ephemeron.rs +++ b/oscars/src/collectors/mark_sweep_branded/ephemeron.rs @@ -4,7 +4,7 @@ use crate::{ gc::Gc, gc_box::GcBox, mutation_ctx::MutationContext, - trace::{Finalize, Trace, TraceColor}, + trace::{Finalize, Trace, Tracer}, }, }; use core::marker::PhantomData; @@ -44,5 +44,5 @@ impl<'id, K: Trace, V: Trace> Copy for Ephemeron<'id, K, V> {} impl<'id, K: Trace, V: Trace> Finalize for Ephemeron<'id, K, V> {} impl<'id, K: Trace, V: Trace> Trace for Ephemeron<'id, K, V> { - fn trace(&self, _color: &TraceColor) {} + fn trace(&mut self, _tracer: &mut Tracer) {} } diff --git a/oscars/src/collectors/mark_sweep_branded/gc.rs b/oscars/src/collectors/mark_sweep_branded/gc.rs index c1b8d5a..92226eb 100644 --- a/oscars/src/collectors/mark_sweep_branded/gc.rs +++ b/oscars/src/collectors/mark_sweep_branded/gc.rs @@ -1,7 +1,7 @@ //! Core pointer types. use crate::{ - alloc::mempool3::PoolItem, + alloc::mempool3::{PoolAllocator, PoolItem}, collectors::mark_sweep_branded::{ gc_box::GcBox, mutation_ctx::MutationContext, @@ -13,7 +13,8 @@ use core::fmt; use core::marker::PhantomData; use core::ops::Deref; use core::ptr::NonNull; -use rust_alloc::boxed::Box; + +pub(crate) type RootDropFn = unsafe fn(&mut PoolAllocator<'static>, NonNull); /// A transient pointer to a GC-managed value. #[derive(Debug)] @@ -61,6 +62,10 @@ pub(crate) struct RootNode<'id, T: Trace> { pub(crate) link: RootLink, /// Pointer to the allocation pub(crate) gc_ptr: NonNull>>, + /// Type-erased drop function for freeing this RootNode + pub(crate) drop_fn: RootDropFn, + /// Raw pointer to the Collector for freeing this node + pub(crate) collector_ptr: *const crate::collectors::mark_sweep_branded::Collector, pub(crate) _marker: PhantomData<*mut &'id ()>, } @@ -83,21 +88,21 @@ impl<'id, T: Trace> Root<'id, T> { impl<'id, T: Trace> Drop for Root<'id, T> { fn drop(&mut self) { - // SAFETY: - // * `self.raw` was created by `Box::into_raw` - // * The address is stable. unsafe { - let node = Box::from_raw(self.raw.as_ptr()); - if node.link.is_linked() { - RootLink::unlink(NonNull::from(&node.link)); + let node_ref = self.raw.as_ref(); + if node_ref.link.is_linked() { + RootLink::unlink(NonNull::from(&node_ref.link)); } + // SAFETY: collector_ptr is valid for the lifetime of the GcContext + let collector = &*node_ref.collector_ptr; + collector.free_root_node(self.raw.cast::(), node_ref.drop_fn); } } } impl Finalize for Gc<'_, T> {} impl Trace for Gc<'_, T> { - fn trace(&self, color: &crate::collectors::mark_sweep_branded::trace::TraceColor) { - color.mark(self); + fn trace(&mut self, tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) { + tracer.mark(self); } } diff --git a/oscars/src/collectors/mark_sweep_branded/gc_box.rs b/oscars/src/collectors/mark_sweep_branded/gc_box.rs index 73bc2b6..f4a300d 100644 --- a/oscars/src/collectors/mark_sweep_branded/gc_box.rs +++ b/oscars/src/collectors/mark_sweep_branded/gc_box.rs @@ -3,17 +3,29 @@ use core::cell::Cell; use core::ptr::NonNull; -use crate::alloc::mempool3::PoolAllocator; -use crate::collectors::mark_sweep_branded::trace::TraceFn; +use crate::alloc::mempool3::{PoolAllocator, PoolItem}; +use crate::collectors::mark_sweep_branded::trace::{Trace, TraceFn, Tracer}; pub(crate) type DropFn = unsafe fn(&mut PoolAllocator<'static>, NonNull); +/// The tri-color marking state of a [`GcBox`] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[repr(u8)] +pub(crate) enum GcColor { + /// Not yet reached by mark phase + White = 0, + /// Reached and queued in the worklist, children not yet traced. + Gray = 1, + /// Reached and dequeued from the worklist, all children traced + Black = 2, +} + /// Heap wrapper for a garbage-collected value. /// /// Allocated via [`PoolAllocator`]. pub(crate) struct GcBox { - /// Reachability flag set by the mark phase. - pub(crate) marked: Cell, + /// tricolor marking state, updated by the mark phase + pub(crate) color: Cell, /// Type-erased trace function. pub(crate) trace_fn: TraceFn, /// Type-erased finalize and free fn @@ -27,3 +39,29 @@ pub(crate) struct GcBox { impl GcBox { pub(crate) const FREED_ALLOC_ID: usize = usize::MAX; } + +impl GcBox { + /// Create a [`GcBox`] for `value`, `color` starts as [`GcColor::White`] + pub(crate) fn new(value: T, trace_fn: TraceFn, drop_fn: DropFn, alloc_id: usize) -> Self { + Self { + color: Cell::new(GcColor::White), + trace_fn, + drop_fn, + alloc_id, + value, + } + } +} + +/// type-erased trace function for a `GcBox` slot. +/// +/// # Safety +/// +/// `ptr` must point to a live `PoolItem>` in the pool allocator +pub(crate) unsafe fn trace_value(ptr: NonNull, tracer: &mut Tracer<'_>) { + let pool_item_ptr = ptr.cast::>>(); + unsafe { + (*pool_item_ptr.as_ptr()).0.color.set(GcColor::Black); + (*pool_item_ptr.as_ptr()).0.value.trace(tracer); + } +} diff --git a/oscars/src/collectors/mark_sweep_branded/mod.rs b/oscars/src/collectors/mark_sweep_branded/mod.rs index 89e567d..6a4060b 100644 --- a/oscars/src/collectors/mark_sweep_branded/mod.rs +++ b/oscars/src/collectors/mark_sweep_branded/mod.rs @@ -17,15 +17,14 @@ pub use cell::GcRefCell; pub use ephemeron::Ephemeron; pub use gc::{Gc, Root}; pub use mutation_ctx::MutationContext; -pub use trace::{Finalize, Trace, TraceColor}; +pub use trace::{Finalize, Trace, Tracer}; pub use weak::WeakGc; use crate::alloc::mempool3::PoolAllocator; use core::cell::{Cell, RefCell}; use core::marker::PhantomData; -use core::mem; use core::ptr::NonNull; -use gc_box::{DropFn, GcBox}; +use gc_box::{DropFn, GcBox, GcColor}; use root_link::{RootLink, RootSentinel}; use rust_alloc::vec::Vec; @@ -41,6 +40,8 @@ pub(crate) struct Collector { // and we ensure that `Gc` objects and pool allocations do not outlive // the `Collector` instance pub(crate) pool: RefCell>, + /// Dedicated pool for RootNode allocations + pub(crate) root_pool: RefCell>, pub(crate) sentinel: RootSentinel, pub(crate) generic_alloc_id: Cell, pub(crate) ephemerons: RefCell>, @@ -50,6 +51,7 @@ impl Collector { fn new() -> Self { Self { pool: RefCell::new(PoolAllocator::default()), + root_pool: RefCell::new(PoolAllocator::default()), sentinel: RootSentinel::new(), generic_alloc_id: Cell::new(0), ephemerons: RefCell::new(Vec::new()), @@ -70,6 +72,46 @@ impl Collector { }); } + /// Allocates a RootNode from the dedicated root pool. + pub(crate) fn alloc_root_node<'id, T: trace::Trace>( + &self, + gc_ptr: NonNull>>, + ) -> NonNull> { + unsafe fn drop_and_free( + pool: &mut PoolAllocator<'static>, + ptr: NonNull, + ) { + use crate::alloc::mempool3::PoolItem; + unsafe { + let typed_ptr = ptr.cast::>>(); + core::ptr::drop_in_place(typed_ptr.as_ptr()); + pool.free_slot(ptr); + } + } + + let mut pool = self.root_pool.borrow_mut(); + let ptr = pool + .try_alloc(gc::RootNode { + link: root_link::RootLink::new(), + gc_ptr, + drop_fn: drop_and_free::, + collector_ptr: self as *const Collector, + _marker: PhantomData, + }) + .expect("root pool allocation failed"); + + // SAFETY: PoolItem is repr(transparent) over T; pointer address is identical. + ptr.as_ptr().cast::>() + } + + /// Frees a RootNode back to the root pool. + pub(crate) fn free_root_node(&self, ptr: NonNull, drop_fn: gc::RootDropFn) { + let mut pool = self.root_pool.borrow_mut(); + unsafe { + (drop_fn)(&mut pool, ptr); + } + } + /// Allocates a value from the pool. /// /// # Panics @@ -95,48 +137,6 @@ impl Collector { self.generic_alloc_id.set(alloc_id.wrapping_add(1)); - unsafe fn trace_value( - ptr: core::ptr::NonNull, - color: &crate::collectors::mark_sweep_branded::trace::TraceColor<'_>, - ) { - use crate::alloc::mempool3::PoolItem; - let pool_item_ptr = ptr.cast::>>(); - unsafe { - (*pool_item_ptr.as_ptr()).0.value.trace(color); - } - } - - // Allocate a raw slot for PoolItem> - let size = mem::size_of::>>(); - - let mut pool = self.pool.borrow_mut(); - let slot_ptr = pool - .alloc_slot_raw(size) - .expect("branded GC: pool allocation failed"); - - // SAFETY: slot_ptr points to uninitialized memory of the correct size and alignment. - // We initialize it here before releasing the borrow. - let ptr = unsafe { - use crate::alloc::mempool3::PoolItem; - let pool_item_ptr = slot_ptr.cast::>>(); - - // Initialize the PoolItem> in place - core::ptr::write( - pool_item_ptr.as_ptr(), - PoolItem(GcBox { - marked: Cell::new(false), - trace_fn: trace_value::, - drop_fn: drop_and_free::, - alloc_id, - value, - }), - ); - - pool_item_ptr - }; - - drop(pool); - unsafe fn drop_and_free( pool: &mut PoolAllocator<'static>, ptr: NonNull, @@ -144,26 +144,33 @@ impl Collector { use crate::alloc::mempool3::PoolItem; unsafe { let typed_ptr = ptr.cast::>>(); - // Finalize the value (*typed_ptr.as_ptr()).0.value.finalize(); - // Drop the PoolItem> in place core::ptr::drop_in_place(typed_ptr.as_ptr()); - // Return the slot to the pool pool.free_slot(ptr); } } + let mut pool = self.pool.borrow_mut(); + let ptr = pool + .try_alloc(GcBox::new( + value, + gc_box::trace_value::, + drop_and_free::, + alloc_id, + )) + .expect("branded GC: pool allocation failed"); + + drop(pool); + Gc { - ptr, + ptr: ptr.as_ptr(), _marker: PhantomData, } } /// Runs a collection cycle pub(crate) fn collect(&self) { - let sentinel_ptr = self.sentinel.as_ptr(); - - let color = TraceColor::new(); + let mut tracer = Tracer::new(); let gc_ptr_offset = core::mem::offset_of!( crate::collectors::mark_sweep_branded::gc::RootNode<'static, i32>, @@ -178,7 +185,7 @@ impl Collector { "gc_ptr offset must be stable across all T: Sized" ); - for link_ptr in RootLink::iter_from_sentinel(sentinel_ptr) { + for link_ptr in self.sentinel.iter() { unsafe { // Read the `gc_ptr` field using the stable offset let gc_ptr_ptr = link_ptr @@ -188,11 +195,11 @@ impl Collector { .cast::>(); let gc_ptr = gc_ptr_ptr.read(); - color.mark_raw(gc_ptr.cast::()); + tracer.mark_raw(gc_ptr.cast::()); } } - color.drain(); + tracer.drain(); // Phase 2: ephemeron fixpoint. // If marking a value causes new keys of other ephemerons to become @@ -207,15 +214,15 @@ impl Collector { if (*key_box.as_ptr()).0.alloc_id != entry.key_alloc_id { continue; } - if (*key_box.as_ptr()).0.marked.get() { - any_newly_marked |= color.mark_raw(entry.value_ptr); + if (*key_box.as_ptr()).0.color.get() != GcColor::White { + any_newly_marked |= tracer.mark_raw(entry.value_ptr); } } } if !any_newly_marked { break; } - color.drain(); + tracer.drain(); } // Phase 3: sweep all slots. Collect unmarked ones, then invalidate and free them. @@ -225,8 +232,8 @@ impl Collector { pool.iter_live_slots() .filter_map(|ptr| unsafe { let gc_box = &(*ptr.cast::>>().as_ptr()).0; - if gc_box.marked.get() { - gc_box.marked.set(false); + if gc_box.color.get() == GcColor::Black { + gc_box.color.set(GcColor::White); None } else { Some((ptr, gc_box.drop_fn)) @@ -260,6 +267,28 @@ impl Drop for Collector { /// Frees all remaining allocations fn drop(&mut self) { use crate::alloc::mempool3::PoolItem; + + // Free all root nodes first + let all_roots: Vec<(NonNull, gc::RootDropFn)> = self + .root_pool + .borrow() + .iter_live_slots() + .map(|ptr| unsafe { + let drop_fn = (*ptr.cast::>>().as_ptr()) + .0 + .drop_fn; + (ptr, drop_fn) + }) + .collect(); + let mut root_pool = self.root_pool.borrow_mut(); + for (ptr, drop_fn) in all_roots { + unsafe { + (drop_fn)(&mut root_pool, ptr); + } + } + drop(root_pool); + + // Then free all GC allocations let all: Vec<(NonNull, DropFn)> = self .pool .borrow() diff --git a/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs b/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs index adb2a52..acbc385 100644 --- a/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs +++ b/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs @@ -3,14 +3,12 @@ use crate::collectors::mark_sweep_branded::{ Collector, ephemeron::Ephemeron, - gc::{Gc, Root, RootNode}, + gc::{Gc, Root}, root_link::RootLink, trace::{Finalize, Trace}, weak::WeakGc, }; use core::marker::PhantomData; -use core::ptr::NonNull; -use rust_alloc::boxed::Box; /// Handle for GC allocations pub struct MutationContext<'id, 'gc> { @@ -40,15 +38,7 @@ impl<'id, 'gc> MutationContext<'id, 'gc> { /// Promotes a `Gc` pointer to a `Root` pub fn root(&self, gc: Gc<'gc, T>) -> Root<'id, T> { - let gc_ptr = gc.ptr; - - let node = Box::new(RootNode { - link: RootLink::new(), - gc_ptr, - _marker: PhantomData, - }); - - let raw = unsafe { NonNull::new_unchecked(Box::into_raw(node)) }; + let raw = self.collector.alloc_root_node(gc.ptr); // SAFETY: `raw` points to a stable `RootNode`. unsafe { diff --git a/oscars/src/collectors/mark_sweep_branded/root_link.rs b/oscars/src/collectors/mark_sweep_branded/root_link.rs index ddfd227..ec064b8 100644 --- a/oscars/src/collectors/mark_sweep_branded/root_link.rs +++ b/oscars/src/collectors/mark_sweep_branded/root_link.rs @@ -67,13 +67,6 @@ impl RootLink { node_ref.next.set(None); } } - - /// Returns an iterator over all nodes after `sentinel`. - pub(crate) fn iter_from_sentinel(sentinel: NonNull) -> RootLinkIter { - // SAFETY: sentinel is pinned in Collector and outlives the iteration. - let first = unsafe { sentinel.as_ref().next.get() }; - RootLinkIter { current: first } - } } /// Iterator over root links in the intrusive list. @@ -109,4 +102,10 @@ impl RootSentinel { NonNull::new_unchecked(self.0.as_ref().get_ref() as *const RootLink as *mut RootLink) } } + + /// Returns an iterator over all root nodes after this sentinel. + pub(crate) fn iter(&self) -> RootLinkIter { + let first = self.0.as_ref().next.get(); + RootLinkIter { current: first } + } } diff --git a/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs b/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs index fe51a8d..5a34c72 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs @@ -26,6 +26,8 @@ mod tests { let make_dummy = || RootNode:: { link: RootLink::new(), gc_ptr: core::ptr::NonNull::dangling(), + drop_fn: |_, _| {}, + collector_ptr: core::ptr::null(), _marker: core::marker::PhantomData, }; diff --git a/oscars/src/collectors/mark_sweep_branded/tests/mod.rs b/oscars/src/collectors/mark_sweep_branded/tests/mod.rs index 3bc4d6d..cf88fc4 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/mod.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/mod.rs @@ -7,7 +7,7 @@ struct JsObject { } impl crate::collectors::mark_sweep_branded::Trace for JsObject { - fn trace(&self, _color: &crate::collectors::mark_sweep_branded::trace::TraceColor) {} + fn trace(&mut self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {} } impl crate::collectors::mark_sweep_branded::Finalize for JsObject {} diff --git a/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs b/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs index 3539e98..fad39e2 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs @@ -6,7 +6,7 @@ use core::cell::Cell; struct DetectDrop<'a>(&'a Cell); impl<'a> Trace for DetectDrop<'a> { - fn trace(&self, _color: &crate::collectors::mark_sweep_branded::trace::TraceColor) {} + fn trace(&mut self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {} } impl Finalize for DetectDrop<'_> {} diff --git a/oscars/src/collectors/mark_sweep_branded/trace.rs b/oscars/src/collectors/mark_sweep_branded/trace.rs index 9ce5c8f..50c49b0 100644 --- a/oscars/src/collectors/mark_sweep_branded/trace.rs +++ b/oscars/src/collectors/mark_sweep_branded/trace.rs @@ -1,7 +1,10 @@ //! Trace and Finalize traits for the lifetime branded GC -use crate::{alloc::mempool3::PoolItem, collectors::mark_sweep_branded::gc::Gc}; -use core::cell::{Cell, OnceCell, RefCell}; +use crate::{ + alloc::mempool3::PoolItem, + collectors::mark_sweep_branded::{gc::Gc, gc_box::GcColor}, +}; +use core::cell::{Cell, OnceCell}; use core::marker::PhantomData; use rust_alloc::borrow::{Cow, ToOwned}; use rust_alloc::boxed::Box; @@ -16,66 +19,67 @@ pub use crate::collectors::common::Finalize; /// /// # Safety /// -/// Use `TraceColor::mark` for every reachable `Gc` pointer. +/// Use `Tracer::mark` for every reachable `Gc` pointer. pub trait Trace { /// Marks all `Gc` pointers reachable from `self`. - fn trace(&self, color: &TraceColor); + fn trace(&mut self, tracer: &mut Tracer); } -pub(crate) type TraceFn = unsafe fn(core::ptr::NonNull, &TraceColor<'_>); +pub(crate) type TraceFn = unsafe fn(core::ptr::NonNull, &mut Tracer<'_>); -/// Opaque token threaded through a collection cycle. +/// Worklist-driven mark context for a stop-the-world collection cycle. /// -/// The `'a` lifetime ties the color to the collection cycle, +/// Implements the classic tri-color marking invariant +/// (see `GcColor` for the per-object states): +/// +/// - `mark()` transitions `White → Gray` and enqueues the object. +/// - `drain()` dequeues each Gray entry; `gc_box::trace_value` transitions +/// it `Gray → Black` and recurses into its children. +/// - The sweep phase reclaims all remaining White objects and resets +/// Black → White, restoring the invariant for the next cycle. +/// +/// The worklist provides iterative traversal, preventing stack overflow on +/// deeply nested object graphs. +/// +/// The `'a` lifetime ties the tracer to the collection cycle, /// preventing it from being stored or escaping the collector. -pub struct TraceColor<'a> { - pub(crate) worklist: RefCell, TraceFn)>>, +pub struct Tracer<'a> { + pub(crate) worklist: Vec<(core::ptr::NonNull, TraceFn)>, pub(crate) _marker: PhantomData<&'a ()>, } -impl<'a> TraceColor<'a> { +impl<'a> Tracer<'a> { pub(crate) fn new() -> Self { Self { - worklist: RefCell::new(Vec::new()), + worklist: Vec::new(), _marker: PhantomData, } } - pub(crate) fn drain(&self) { + pub(crate) fn drain(&mut self) { // Note: Using `pop()` processes the worklist in LIFO order (Depth-First Search). // While correct, heap-allocated object graphs often exhibit better cache locality // with Breadth-First Search. This could be evaluated with a `VecDeque` in the future. - loop { - // Drop the borrow before calling trace_fn to allow re-entrant marks. - let item = self.worklist.borrow_mut().pop(); - match item { - Some((ptr, trace_fn)) => unsafe { (trace_fn)(ptr, self) }, - None => break, - } + while let Some((ptr, trace_fn)) = self.worklist.pop() { + // SAFETY: ptr is a live PoolItem> whose TraceFn was stored at allocation. + // pop() releases the borrow on self.worklist before the call, allowing mark() + // to push new entries re-entrantly. + unsafe { (trace_fn)(ptr, self) } } } - /// Marks `gc` as reachable. + /// Marks `gc` as reachable (White → Gray). #[inline] - pub fn mark(&self, gc: &Gc<'_, T>) { + pub fn mark(&mut self, gc: &Gc<'_, T>) { // SAFETY: `gc.ptr` is a valid `PoolItem>`. unsafe { - if !(*gc.ptr.as_ptr()).0.marked.replace(true) { - unsafe fn trace_value( - ptr: core::ptr::NonNull, - color: &TraceColor<'_>, - ) { - let pool_item_ptr = ptr - .cast::>>( - ); - unsafe { - (*pool_item_ptr.as_ptr()).0.value.trace(color); - } - } - - self.worklist - .borrow_mut() - .push((gc.ptr.cast::(), trace_value::)); + let gc_box = &(*gc.ptr.as_ptr()).0; + if gc_box.color.get() == GcColor::White { + gc_box.color.set(GcColor::Gray); + self.worklist.push(( + gc.ptr.cast::(), + crate::collectors::mark_sweep_branded::gc_box::trace_value::, + )); } } } @@ -86,14 +90,16 @@ impl<'a> TraceColor<'a> { /// /// `ptr` must be a valid pointer to a `PoolItem>` managed by this collector. #[inline] - pub(crate) fn mark_raw(&self, ptr: core::ptr::NonNull) -> bool { + pub(crate) fn mark_raw(&mut self, ptr: core::ptr::NonNull) -> bool { let pool_item_ptr = ptr.cast::>>(); unsafe { - if !(*pool_item_ptr.as_ptr()).0.marked.replace(true) { - let trace_fn = (*pool_item_ptr.as_ptr()).0.trace_fn; - self.worklist.borrow_mut().push((ptr, trace_fn)); + let gc_box = &(*pool_item_ptr.as_ptr()).0; + if gc_box.color.get() == GcColor::White { + let trace_fn = gc_box.trace_fn; + gc_box.color.set(GcColor::Gray); + self.worklist.push((ptr, trace_fn)); true } else { false @@ -104,7 +110,7 @@ impl<'a> TraceColor<'a> { impl Trace for &T { #[inline] - fn trace(&self, _color: &TraceColor) {} + fn trace(&mut self, _tracer: &mut Tracer) {} } // primitive + std-lib Trace impls @@ -114,7 +120,7 @@ macro_rules! empty_trace { $( impl Trace for $T { #[inline] - fn trace(&self, _color: &TraceColor) {} + fn trace(&mut self, _tracer: &mut Tracer) {} } )* }; @@ -154,79 +160,79 @@ empty_trace![ ]; impl Trace for [T; N] { - fn trace(&self, color: &TraceColor) { - for v in self.iter() { - v.trace(color); + fn trace(&mut self, tracer: &mut Tracer) { + for v in self.iter_mut() { + v.trace(tracer); } } } impl Trace for Box { - fn trace(&self, color: &TraceColor) { - (**self).trace(color); + fn trace(&mut self, tracer: &mut Tracer) { + (**self).trace(tracer); } } impl Trace for Option { - fn trace(&self, color: &TraceColor) { + fn trace(&mut self, tracer: &mut Tracer) { if let Some(v) = self { - v.trace(color); + v.trace(tracer); } } } impl Trace for Result { - fn trace(&self, color: &TraceColor) { + fn trace(&mut self, tracer: &mut Tracer) { match self { - Ok(v) => v.trace(color), - Err(e) => e.trace(color), + Ok(v) => v.trace(tracer), + Err(e) => e.trace(tracer), } } } impl Trace for Vec { - fn trace(&self, color: &TraceColor) { - for v in self.iter() { - v.trace(color); + fn trace(&mut self, tracer: &mut Tracer) { + for v in self.iter_mut() { + v.trace(tracer); } } } impl Trace for VecDeque { - fn trace(&self, color: &TraceColor) { - for v in self.iter() { - v.trace(color); + fn trace(&mut self, tracer: &mut Tracer) { + for v in self.iter_mut() { + v.trace(tracer); } } } impl Trace for LinkedList { - fn trace(&self, color: &TraceColor) { - for v in self.iter() { - v.trace(color); + fn trace(&mut self, tracer: &mut Tracer) { + for v in self.iter_mut() { + v.trace(tracer); } } } impl Trace for PhantomData { #[inline] - fn trace(&self, _color: &TraceColor) {} + fn trace(&mut self, _tracer: &mut Tracer) {} } // Cell> requires T: Copy to safely read the value via Cell::get(). // For non-Copy types, use GcRefCell instead. impl Trace for Cell> { - fn trace(&self, color: &TraceColor) { - if let Some(v) = self.get() { - v.trace(color); + fn trace(&mut self, tracer: &mut Tracer) { + if let Some(mut v) = self.get() { + v.trace(tracer); } } } impl Trace for OnceCell { - fn trace(&self, color: &TraceColor) { - if let Some(v) = self.get() { - v.trace(color); + fn trace(&mut self, tracer: &mut Tracer) { + if let Some(v) = self.get_mut() { + v.trace(tracer); } } } @@ -235,44 +241,44 @@ impl Trace for Cow<'static, T> where T::Owned: Trace, { - fn trace(&self, color: &TraceColor) { + fn trace(&mut self, tracer: &mut Tracer) { if let Cow::Owned(v) = self { - v.trace(color); + v.trace(tracer); } } } impl Trace for (A,) { #[inline] - fn trace(&self, color: &TraceColor) { - self.0.trace(color); + fn trace(&mut self, tracer: &mut Tracer) { + self.0.trace(tracer); } } impl Trace for (A, B) { #[inline] - fn trace(&self, color: &TraceColor) { - self.0.trace(color); - self.1.trace(color); + fn trace(&mut self, tracer: &mut Tracer) { + self.0.trace(tracer); + self.1.trace(tracer); } } impl Trace for (A, B, C) { #[inline] - fn trace(&self, color: &TraceColor) { - self.0.trace(color); - self.1.trace(color); - self.2.trace(color); + fn trace(&mut self, tracer: &mut Tracer) { + self.0.trace(tracer); + self.1.trace(tracer); + self.2.trace(tracer); } } impl Trace for (A, B, C, D) { #[inline] - fn trace(&self, color: &TraceColor) { - self.0.trace(color); - self.1.trace(color); - self.2.trace(color); - self.3.trace(color); + fn trace(&mut self, tracer: &mut Tracer) { + self.0.trace(tracer); + self.1.trace(tracer); + self.2.trace(tracer); + self.3.trace(tracer); } } @@ -281,26 +287,26 @@ impl Trace for (A, B, C, D) { // struct instead. impl Trace for rust_alloc::rc::Rc { #[inline] - fn trace(&self, _color: &TraceColor) {} + fn trace(&mut self, _tracer: &mut Tracer) {} } impl Trace for rust_alloc::sync::Arc { #[inline] - fn trace(&self, _color: &TraceColor) {} + fn trace(&mut self, _tracer: &mut Tracer) {} } impl Trace for BTreeMap { - fn trace(&self, color: &TraceColor) { - for v in self.values() { - v.trace(color); + fn trace(&mut self, tracer: &mut Tracer) { + for v in self.values_mut() { + v.trace(tracer); } } } impl Trace for BTreeSet { #[inline] - fn trace(&self, _color: &TraceColor) { + fn trace(&mut self, _tracer: &mut Tracer) { // BTreeSet keys are immutable and cannot contain Gc pointers - // that need tracing (Gc requires &self to trace). + // that need tracing (Gc requires &mut self to trace). } } diff --git a/oscars/src/collectors/mark_sweep_branded/weak.rs b/oscars/src/collectors/mark_sweep_branded/weak.rs index e5bc596..8bc2dbe 100644 --- a/oscars/src/collectors/mark_sweep_branded/weak.rs +++ b/oscars/src/collectors/mark_sweep_branded/weak.rs @@ -51,5 +51,5 @@ impl<'id, T: Trace + ?Sized> Copy for WeakGc<'id, T> {} impl<'id, T: Trace> Finalize for WeakGc<'id, T> {} impl<'id, T: Trace> Trace for WeakGc<'id, T> { // Weak references do not mark their target, upgrade() returning None after collection is the intended behaviour. - fn trace(&self, _color: &crate::collectors::mark_sweep_branded::trace::TraceColor) {} + fn trace(&mut self, _tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {} } From a98ad8e6feb4a0e37a82e8baa18e411b6c776786 Mon Sep 17 00:00:00 2001 From: shruti2522 Date: Mon, 4 May 2026 12:29:30 +0000 Subject: [PATCH 7/8] refactor: use PoolPointer, make Ephemeron::key_ptr Option --- oscars/src/alloc/mempool3/alloc.rs | 30 ++++-- .../mark_sweep_branded/ephemeron.rs | 13 +-- .../src/collectors/mark_sweep_branded/gc.rs | 19 +++- .../src/collectors/mark_sweep_branded/mod.rs | 91 +++++++------------ .../mark_sweep_branded/mutation_ctx.rs | 42 +++++---- .../tests/api_compliance.rs | 4 +- .../mark_sweep_branded/tests/ephemeron.rs | 18 ++-- .../mark_sweep_branded/tests/mod.rs | 60 +++++++----- .../mark_sweep_branded/tests/uaf.rs | 2 +- .../tests/ui/gc_cannot_escape_mutate.rs | 2 +- .../tests/ui/gc_cannot_escape_mutate.stderr | 4 +- .../tests/ui/gc_cannot_store_outer_scopr.rs | 2 +- .../tests/ui/root_cross_context.rs | 2 +- .../tests/ui/root_cross_context.stderr | 8 +- .../collectors/mark_sweep_branded/trace.rs | 4 +- .../src/collectors/mark_sweep_branded/weak.rs | 7 +- 16 files changed, 165 insertions(+), 143 deletions(-) diff --git a/oscars/src/alloc/mempool3/alloc.rs b/oscars/src/alloc/mempool3/alloc.rs index 2b1941f..64e5ba3 100644 --- a/oscars/src/alloc/mempool3/alloc.rs +++ b/oscars/src/alloc/mempool3/alloc.rs @@ -59,15 +59,24 @@ impl<'pool> ErasedPoolPointer<'pool> { } /// typed pointer into a pool slot -#[derive(Debug, Clone, Copy)] #[repr(transparent)] -pub struct PoolPointer<'pool, T>(NonNull>, PhantomData<(&'pool (), *mut T)>); +pub struct PoolPointer<'pool, T: ?Sized>(NonNull>, PhantomData<(&'pool (), *mut T)>); -impl<'pool, T> PoolPointer<'pool, T> { - pub(crate) unsafe fn from_raw(raw: NonNull>) -> Self { - Self(raw, PhantomData) +impl<'pool, T: ?Sized> core::fmt::Debug for PoolPointer<'pool, T> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "PoolPointer({:p})", self.0.as_ptr()) } +} + +impl<'pool, T: ?Sized> Clone for PoolPointer<'pool, T> { + fn clone(&self) -> Self { + *self + } +} +impl<'pool, T: ?Sized> Copy for PoolPointer<'pool, T> {} + +impl<'pool, T: ?Sized> PoolPointer<'pool, T> { pub fn as_inner_ref(&self) -> &'pool T where T: 'pool, @@ -80,7 +89,10 @@ impl<'pool, T> PoolPointer<'pool, T> { self.0 } - pub fn to_erased(self) -> ErasedPoolPointer<'pool> { + pub fn to_erased(self) -> ErasedPoolPointer<'pool> + where + T: Sized, + { ErasedPoolPointer(self.0.cast::(), PhantomData) } @@ -95,6 +107,12 @@ impl<'pool, T> PoolPointer<'pool, T> { } } +impl<'pool, T> PoolPointer<'pool, T> { + pub(crate) unsafe fn from_raw(raw: NonNull>) -> Self { + Self(raw, PhantomData) + } +} + // ==== SlotPool ==== // impl core::fmt::Debug for SlotPool { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { diff --git a/oscars/src/collectors/mark_sweep_branded/ephemeron.rs b/oscars/src/collectors/mark_sweep_branded/ephemeron.rs index 9b75638..e77771b 100644 --- a/oscars/src/collectors/mark_sweep_branded/ephemeron.rs +++ b/oscars/src/collectors/mark_sweep_branded/ephemeron.rs @@ -1,5 +1,5 @@ use crate::{ - alloc::mempool3::PoolItem, + alloc::mempool3::PoolPointer, collectors::mark_sweep_branded::{ gc::Gc, gc_box::GcBox, @@ -8,20 +8,21 @@ use crate::{ }, }; use core::marker::PhantomData; -use core::ptr::NonNull; pub struct Ephemeron<'id, K: Trace, V: Trace> { - pub(crate) key_ptr: NonNull>>, + pub(crate) key_ptr: Option>>, pub(crate) key_alloc_id: usize, - pub(crate) value_ptr: NonNull>>, + pub(crate) value_ptr: PoolPointer<'static, GcBox>, pub(crate) _marker: PhantomData<*mut &'id ()>, } impl<'id, K: Trace, V: Trace> Ephemeron<'id, K, V> { /// Returns the value if the key is alive. pub fn get_value<'gc>(&self, _cx: &MutationContext<'id, 'gc>) -> Option> { - // SAFETY: `_cx` proves the collector is alive; alloc_id guards ABA. - let key_alive = unsafe { (*self.key_ptr.as_ptr()).0.alloc_id == self.key_alloc_id }; + // SAFETY: `_cx` proves the collector is alive, alloc_id guards ABA + let key_alive = self + .key_ptr + .is_some_and(|p| unsafe { (*p.as_ptr().as_ptr()).0.alloc_id == self.key_alloc_id }); if key_alive { Some(Gc { ptr: self.value_ptr, diff --git a/oscars/src/collectors/mark_sweep_branded/gc.rs b/oscars/src/collectors/mark_sweep_branded/gc.rs index 92226eb..57693ca 100644 --- a/oscars/src/collectors/mark_sweep_branded/gc.rs +++ b/oscars/src/collectors/mark_sweep_branded/gc.rs @@ -1,7 +1,7 @@ //! Core pointer types. use crate::{ - alloc::mempool3::{PoolAllocator, PoolItem}, + alloc::mempool3::{PoolAllocator, PoolPointer}, collectors::mark_sweep_branded::{ gc_box::GcBox, mutation_ctx::MutationContext, @@ -19,7 +19,7 @@ pub(crate) type RootDropFn = unsafe fn(&mut PoolAllocator<'static>, NonNull) /// A transient pointer to a GC-managed value. #[derive(Debug)] pub struct Gc<'gc, T: Trace + ?Sized + 'gc> { - pub(crate) ptr: NonNull>>, + pub(crate) ptr: PoolPointer<'static, GcBox>, pub(crate) _marker: PhantomData<(&'gc T, *const ())>, } @@ -38,7 +38,7 @@ impl<'gc, T: Trace + 'gc> Gc<'gc, T> { // The `'gc` lifetime is scoped to a `mutate()` closure, collection only occurs // via `cx.collect()` within that same closure and `Gc<'gc, T>` can't // escape the closure. - unsafe { &(*self.ptr.as_ptr()).0.value } + unsafe { &(*self.ptr.as_ptr().as_ptr()).0.value } } } @@ -61,7 +61,7 @@ pub(crate) struct RootNode<'id, T: Trace> { /// Intrusive list link pub(crate) link: RootLink, /// Pointer to the allocation - pub(crate) gc_ptr: NonNull>>, + pub(crate) gc_ptr: PoolPointer<'static, GcBox>, /// Type-erased drop function for freeing this RootNode pub(crate) drop_fn: RootDropFn, /// Raw pointer to the Collector for freeing this node @@ -69,6 +69,17 @@ pub(crate) struct RootNode<'id, T: Trace> { pub(crate) _marker: PhantomData<*mut &'id ()>, } +/// Type-erased version of [`RootNode`] for use during collection. +/// +/// Since [`RootNode`] is `repr(C)` and `link` is always the first field, +/// a `NonNull` from the sentinel iterator can be safely cast to +/// `NonNull` to read `gc_ptr` without knowing `T`. +#[repr(C)] +pub(crate) struct ErasedRootNode { + pub(crate) link: RootLink, + pub(crate) gc_ptr: PoolPointer<'static, GcBox<()>>, +} + /// A handle that keeps a GC allocation live. #[must_use = "dropping a root unregisters it from the GC"] pub struct Root<'id, T: Trace> { diff --git a/oscars/src/collectors/mark_sweep_branded/mod.rs b/oscars/src/collectors/mark_sweep_branded/mod.rs index 6a4060b..4085b0f 100644 --- a/oscars/src/collectors/mark_sweep_branded/mod.rs +++ b/oscars/src/collectors/mark_sweep_branded/mod.rs @@ -20,19 +20,18 @@ pub use mutation_ctx::MutationContext; pub use trace::{Finalize, Trace, Tracer}; pub use weak::WeakGc; -use crate::alloc::mempool3::PoolAllocator; +use crate::alloc::mempool3::{PoolAllocError, PoolAllocator, PoolPointer}; use core::cell::{Cell, RefCell}; use core::marker::PhantomData; use core::ptr::NonNull; use gc_box::{DropFn, GcBox, GcColor}; -use root_link::{RootLink, RootSentinel}; +use root_link::RootSentinel; use rust_alloc::vec::Vec; /// Type-erased ephemeron registration. pub(crate) struct EphemeronEntry { - pub(crate) key_ptr: NonNull, - pub(crate) key_alloc_id: usize, - pub(crate) value_ptr: NonNull, + pub(crate) key_ptr: Option>>, + pub(crate) value_ptr: PoolPointer<'static, GcBox<()>>, } pub(crate) struct Collector { @@ -61,13 +60,11 @@ impl Collector { /// Registers an ephemeron key/value pair for processing during collection. pub(crate) fn register_ephemeron( &self, - key_ptr: NonNull, - key_alloc_id: usize, - value_ptr: NonNull, + key_ptr: PoolPointer<'static, GcBox<()>>, + value_ptr: PoolPointer<'static, GcBox<()>>, ) { self.ephemerons.borrow_mut().push(EphemeronEntry { - key_ptr, - key_alloc_id, + key_ptr: Some(key_ptr), value_ptr, }); } @@ -75,7 +72,7 @@ impl Collector { /// Allocates a RootNode from the dedicated root pool. pub(crate) fn alloc_root_node<'id, T: trace::Trace>( &self, - gc_ptr: NonNull>>, + gc_ptr: PoolPointer<'static, GcBox>, ) -> NonNull> { unsafe fn drop_and_free( pool: &mut PoolAllocator<'static>, @@ -121,7 +118,7 @@ impl Collector { pub(crate) fn alloc<'gc, T: trace::Trace + trace::Finalize + 'gc>( &'gc self, value: T, - ) -> Gc<'gc, T> { + ) -> Result, PoolAllocError> { let alloc_id = self.generic_alloc_id.get(); // Check for alloc_id wrap before incrementing. @@ -151,51 +148,32 @@ impl Collector { } let mut pool = self.pool.borrow_mut(); - let ptr = pool - .try_alloc(GcBox::new( - value, - gc_box::trace_value::, - drop_and_free::, - alloc_id, - )) - .expect("branded GC: pool allocation failed"); + let ptr = pool.try_alloc(GcBox::new( + value, + gc_box::trace_value::, + drop_and_free::, + alloc_id, + ))?; drop(pool); - Gc { - ptr: ptr.as_ptr(), + Ok(Gc { + ptr: unsafe { ptr.extend_lifetime() }, _marker: PhantomData, - } + }) } /// Runs a collection cycle pub(crate) fn collect(&self) { let mut tracer = Tracer::new(); - let gc_ptr_offset = core::mem::offset_of!( - crate::collectors::mark_sweep_branded::gc::RootNode<'static, i32>, - gc_ptr - ); - debug_assert_eq!( - gc_ptr_offset, - core::mem::offset_of!( - crate::collectors::mark_sweep_branded::gc::RootNode<'static, u64>, - gc_ptr - ), - "gc_ptr offset must be stable across all T: Sized" - ); - for link_ptr in self.sentinel.iter() { unsafe { - // Read the `gc_ptr` field using the stable offset - let gc_ptr_ptr = link_ptr - .as_ptr() - .cast::() - .add(gc_ptr_offset) - .cast::>(); - let gc_ptr = gc_ptr_ptr.read(); - - tracer.mark_raw(gc_ptr.cast::()); + // SAFETY: link_ptr points to the `link` field which is first in repr(C) RootNode. + // Casting to ErasedRootNode (also repr(C), same first two fields) lets us read + // gc_ptr without knowing T, avoiding manual offset arithmetic. + let erased = link_ptr.cast::(); + tracer.mark_raw((*erased.as_ptr()).gc_ptr.as_ptr().cast::()); } } @@ -207,15 +185,12 @@ impl Collector { loop { let mut any_newly_marked = false; for entry in self.ephemerons.borrow().iter() { - use crate::alloc::mempool3::PoolItem; + let Some(key_ptr) = entry.key_ptr else { + continue; + }; unsafe { - let key_box = entry.key_ptr.cast::>>(); - // Skip entries invalidated by a previous collection cycle. - if (*key_box.as_ptr()).0.alloc_id != entry.key_alloc_id { - continue; - } - if (*key_box.as_ptr()).0.color.get() != GcColor::White { - any_newly_marked |= tracer.mark_raw(entry.value_ptr); + if (*key_ptr.as_ptr().as_ptr()).0.color.get() != GcColor::White { + any_newly_marked |= tracer.mark_raw(entry.value_ptr.as_ptr().cast::()); } } } @@ -253,12 +228,12 @@ impl Collector { } // Phase 4: remove ephemeron entries whose key was swept this cycle. + // A swept key has alloc_id set to FREED_ALLOC_ID by the sweep above. + // Using the Option lets us express the invalid state without a stored alloc_id. self.ephemerons.borrow_mut().retain(|entry| { - use crate::alloc::mempool3::PoolItem; - unsafe { - let key_box = entry.key_ptr.cast::>>(); - (*key_box.as_ptr()).0.alloc_id == entry.key_alloc_id - } + entry.key_ptr.is_some_and(|key_ptr| unsafe { + (*key_ptr.as_ptr().as_ptr()).0.alloc_id != GcBox::<()>::FREED_ALLOC_ID + }) }); } } diff --git a/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs b/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs index acbc385..8024b6d 100644 --- a/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs +++ b/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs @@ -1,12 +1,16 @@ //! `MutationContext<'id, 'gc>` handle. -use crate::collectors::mark_sweep_branded::{ - Collector, - ephemeron::Ephemeron, - gc::{Gc, Root}, - root_link::RootLink, - trace::{Finalize, Trace}, - weak::WeakGc, +use crate::{ + alloc::mempool3::{PoolAllocError, PoolPointer}, + collectors::mark_sweep_branded::{ + Collector, + ephemeron::Ephemeron, + gc::{Gc, Root}, + gc_box::GcBox, + root_link::RootLink, + trace::{Finalize, Trace}, + weak::WeakGc, + }, }; use core::marker::PhantomData; @@ -18,17 +22,13 @@ pub struct MutationContext<'id, 'gc> { impl<'id, 'gc> MutationContext<'id, 'gc> { /// Allocates a value on the GC heap. - /// - /// # Panics - /// - /// Panics if the pool allocator fails to allocate. - pub fn alloc(&self, value: T) -> Gc<'gc, T> { + pub fn alloc(&self, value: T) -> Result, PoolAllocError> { self.collector.alloc(value) } /// Downgrades a `Gc` into a weak reference pub fn alloc_weak(&self, gc: Gc<'gc, T>) -> WeakGc<'id, T> { - let alloc_id = unsafe { (*gc.ptr.as_ptr()).0.alloc_id }; + let alloc_id = unsafe { (*gc.ptr.as_ptr().as_ptr()).0.alloc_id }; WeakGc { ptr: gc.ptr, alloc_id, @@ -60,14 +60,16 @@ impl<'id, 'gc> MutationContext<'id, 'gc> { key: Gc<'gc, K>, value: Gc<'gc, V>, ) -> Ephemeron<'id, K, V> { - let key_alloc_id = unsafe { (*key.ptr.as_ptr()).0.alloc_id }; - self.collector.register_ephemeron( - key.ptr.cast::(), - key_alloc_id, - value.ptr.cast::(), - ); + let key_alloc_id = unsafe { (*key.ptr.as_ptr().as_ptr()).0.alloc_id }; + // SAFETY: GcBox and GcBox are erased to GcBox<()>, the collector + // only reads the fixed size prefix fields via this pointer + let erased_key: PoolPointer<'static, GcBox<()>> = + unsafe { key.ptr.to_erased().to_typed_pool_pointer::>() }; + let erased_value: PoolPointer<'static, GcBox<()>> = + unsafe { value.ptr.to_erased().to_typed_pool_pointer::>() }; + self.collector.register_ephemeron(erased_key, erased_value); Ephemeron { - key_ptr: key.ptr, + key_ptr: Some(key.ptr), key_alloc_id, value_ptr: value.ptr, _marker: core::marker::PhantomData, diff --git a/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs b/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs index 5a34c72..4c50235 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs @@ -25,7 +25,9 @@ mod tests { // Assert offset is correct via offset_of-like macro conceptually let make_dummy = || RootNode:: { link: RootLink::new(), - gc_ptr: core::ptr::NonNull::dangling(), + gc_ptr: unsafe { + crate::alloc::mempool3::PoolPointer::from_raw(core::ptr::NonNull::dangling()) + }, drop_fn: |_, _| {}, collector_ptr: core::ptr::null(), _marker: core::marker::PhantomData, diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ephemeron.rs b/oscars/src/collectors/mark_sweep_branded/tests/ephemeron.rs index 1e8a9a3..a96e015 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/ephemeron.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/ephemeron.rs @@ -4,8 +4,8 @@ use super::*; fn ephemeron_value_survives_when_key_is_rooted() { with_gc(|ctx| { let (root_key, eph) = ctx.mutate(|cx| { - let key = cx.alloc(1u32); - let value = cx.alloc(42u32); + let key = cx.alloc(1u32).unwrap(); + let value = cx.alloc(42u32).unwrap(); let root_key = cx.root(key); let eph = cx.alloc_ephemeron(key, value); (root_key, eph) @@ -27,8 +27,8 @@ fn ephemeron_value_survives_when_key_is_rooted() { fn ephemeron_value_collected_when_key_unrooted() { with_gc(|ctx| { let eph = ctx.mutate(|cx| { - let key = cx.alloc(1u32); - let value = cx.alloc(99u32); + let key = cx.alloc(1u32).unwrap(); + let value = cx.alloc(99u32).unwrap(); cx.alloc_ephemeron(key, value) }); @@ -51,9 +51,9 @@ fn ephemeron_chain_fixpoint() { // This requires the collector to run multiple fixpoint passes. with_gc(|ctx| { let (root_a, eph_ab, eph_bc) = ctx.mutate(|cx| { - let a = cx.alloc(1u32); - let b = cx.alloc(2u32); - let c = cx.alloc(3u32); + let a = cx.alloc(1u32).unwrap(); + let b = cx.alloc(2u32).unwrap(); + let c = cx.alloc(3u32).unwrap(); let root_a = cx.root(a); let eph_ab = cx.alloc_ephemeron(a, b); let eph_bc = cx.alloc_ephemeron(b, c); @@ -92,8 +92,8 @@ fn ephemeron_entry_cleaned_up_after_sweep() { // Verify the collector's internal ephemeron list shrinks after dead entries are swept. with_gc(|ctx| { ctx.mutate(|cx| { - let key = cx.alloc(0u32); - let value = cx.alloc(0u32); + let key = cx.alloc(0u32).unwrap(); + let value = cx.alloc(0u32).unwrap(); cx.alloc_ephemeron(key, value); }); assert_eq!(ctx.ephemeron_count(), 1); diff --git a/oscars/src/collectors/mark_sweep_branded/tests/mod.rs b/oscars/src/collectors/mark_sweep_branded/tests/mod.rs index cf88fc4..0214d0e 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/mod.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/mod.rs @@ -15,10 +15,13 @@ impl crate::collectors::mark_sweep_branded::Finalize for JsObject {} fn unrooted_alloc_is_swept() { with_gc(|ctx| { let weak = ctx.mutate(|cx| { - cx.alloc_weak(cx.alloc(JsObject { - name: "ephemeral".into(), - value: 999, - })) + cx.alloc_weak( + cx.alloc(JsObject { + name: "ephemeral".into(), + value: 999, + }) + .unwrap(), + ) }); ctx.collect(); ctx.mutate(|cx| { @@ -31,10 +34,13 @@ fn unrooted_alloc_is_swept() { fn rooted_alloc_survives_collection() { with_gc(|ctx| { let root = ctx.mutate(|cx| { - cx.root(cx.alloc(JsObject { - name: "pinned".into(), - value: 42, - })) + cx.root( + cx.alloc(JsObject { + name: "pinned".into(), + value: 42, + }) + .unwrap(), + ) }); ctx.collect(); ctx.mutate(|cx| { @@ -49,10 +55,13 @@ fn rooted_alloc_survives_collection() { fn weak_upgrade_after_collection_without_root_is_none() { with_gc(|ctx| { let weak = ctx.mutate(|cx| { - cx.alloc_weak(cx.alloc(JsObject { - name: "weak".into(), - value: 10, - })) + cx.alloc_weak( + cx.alloc(JsObject { + name: "weak".into(), + value: 10, + }) + .unwrap(), + ) }); ctx.collect(); ctx.mutate(|cx| { @@ -65,16 +74,21 @@ fn weak_upgrade_after_collection_without_root_is_none() { fn weak_upgrade_with_live_root_is_some() { with_gc(|ctx| { let (root, weak) = ctx.mutate(|cx| { - let obj = cx.alloc(JsObject { - name: "strong".into(), - value: 7, - }); + let obj = cx + .alloc(JsObject { + name: "strong".into(), + value: 7, + }) + .unwrap(); let root = cx.root(obj); - let weak = cx.alloc_weak(cx.alloc(JsObject { - name: "weak_entry".into(), - value: 77, - })); + let weak = cx.alloc_weak( + cx.alloc(JsObject { + name: "weak_entry".into(), + value: 77, + }) + .unwrap(), + ); (root, weak) }); ctx.collect(); @@ -89,8 +103,8 @@ fn weak_upgrade_with_live_root_is_some() { fn multiple_roots_are_independent() { with_gc(|ctx| { let (root1, root2) = ctx.mutate(|cx| { - let obj1 = cx.alloc(100i32); - let obj2 = cx.alloc(200i32); + let obj1 = cx.alloc(100i32).unwrap(); + let obj2 = cx.alloc(200i32).unwrap(); (cx.root(obj1), cx.root(obj2)) }); @@ -114,7 +128,7 @@ fn multiple_roots_are_independent() { fn root_escapes_closure_safely() { with_gc(|ctx| { let root = ctx.mutate(|cx| { - let obj = cx.alloc(555i32); + let obj = cx.alloc(555i32).unwrap(); cx.root(obj) }); diff --git a/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs b/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs index fad39e2..f15bc57 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs @@ -22,7 +22,7 @@ fn test_uaf() { with_gc(|cx| { let dropped = Cell::new(false); cx.mutate(|mcx| { - let _gc = mcx.alloc(DetectDrop(&dropped)); + let _gc = mcx.alloc(DetectDrop(&dropped)).unwrap(); }); cx.collect(); // Garbage collects 'gc' because it isn't rooted! assert!(dropped.get(), "It wasn't collected!"); diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.rs b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.rs index 0e29f6f..7552163 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.rs @@ -12,7 +12,7 @@ fn main() { // Returning Gc<'gc, i32> directly attempts to leak a shorter lifetime // into the outer scope. The compiler must reject this. let _escaped = ctx.mutate(|cx| { - cx.alloc(42i32) // ERROR: Gc<'gc, i32> cannot escape mutate() + cx.alloc(42i32).unwrap() // ERROR: Gc<'gc, i32> cannot escape mutate() }); }); } diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.stderr b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.stderr index 37ad639..8796e27 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.stderr +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.stderr @@ -5,5 +5,5 @@ error: lifetime may not live long enough | --- return type of closure is oscars::collectors::mark_sweep_branded::Gc<'2, i32> | | | has type `&MutationContext<'_, '1>` -15 | cx.alloc(42i32) // ERROR: Gc<'gc, i32> cannot escape mutate() - | ^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` +15 | cx.alloc(42i32).unwrap() // ERROR: Gc<'gc, i32> cannot escape mutate() + | ^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.rs b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.rs index 648db6a..efda0e0 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.rs @@ -17,7 +17,7 @@ fn main() { let mut holder: Option> = None; ctx.mutate(|cx| { - let gc = cx.alloc(42i32); + let gc = cx.alloc(42i32).unwrap(); // ERROR: `cx` (and therefore `gc`'s `'gc` lifetime) does not live // long enough to be stored in `holder`. holder = Some(Holder { gc }); diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs b/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs index 408c8a3..8c8e259 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs @@ -13,7 +13,7 @@ fn main() { with_gc(|ctx1| { with_gc(|ctx2| { // root carries 'id of ctx1 - let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32))); + let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32).unwrap())); ctx2.mutate(|cx| { // ERROR: `'id` of `root` (ctx1) != `'id` of `cx` (ctx2) diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.stderr b/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.stderr index eac1f09..8848272 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.stderr +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.stderr @@ -6,8 +6,8 @@ error: lifetime may not live long enough 14 | with_gc(|ctx2| { | ---- has type `GcContext<'1>` 15 | // root carries 'id of ctx1 -16 | let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32))); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` +16 | let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32).unwrap())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` error: lifetime may not live long enough --> src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs:16:41 @@ -15,5 +15,5 @@ error: lifetime may not live long enough 13 | with_gc(|ctx1| { | ---- has type `GcContext<'1>` ... -16 | let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32))); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'static` +16 | let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32).unwrap())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'static` diff --git a/oscars/src/collectors/mark_sweep_branded/trace.rs b/oscars/src/collectors/mark_sweep_branded/trace.rs index 50c49b0..214a94a 100644 --- a/oscars/src/collectors/mark_sweep_branded/trace.rs +++ b/oscars/src/collectors/mark_sweep_branded/trace.rs @@ -73,11 +73,11 @@ impl<'a> Tracer<'a> { pub fn mark(&mut self, gc: &Gc<'_, T>) { // SAFETY: `gc.ptr` is a valid `PoolItem>`. unsafe { - let gc_box = &(*gc.ptr.as_ptr()).0; + let gc_box = &(*gc.ptr.as_ptr().as_ptr()).0; if gc_box.color.get() == GcColor::White { gc_box.color.set(GcColor::Gray); self.worklist.push(( - gc.ptr.cast::(), + gc.ptr.as_ptr().cast::(), crate::collectors::mark_sweep_branded::gc_box::trace_value::, )); } diff --git a/oscars/src/collectors/mark_sweep_branded/weak.rs b/oscars/src/collectors/mark_sweep_branded/weak.rs index 8bc2dbe..bb60895 100644 --- a/oscars/src/collectors/mark_sweep_branded/weak.rs +++ b/oscars/src/collectors/mark_sweep_branded/weak.rs @@ -1,7 +1,7 @@ //! `WeakGc<'id, T>` for weak references. use crate::{ - alloc::mempool3::PoolItem, + alloc::mempool3::PoolPointer, collectors::mark_sweep_branded::{ gc::Gc, gc_box::GcBox, @@ -9,11 +9,10 @@ use crate::{ }, }; use core::marker::PhantomData; -use core::ptr::NonNull; /// A weak reference to a GC managed value pub struct WeakGc<'id, T: Trace + ?Sized> { - pub(crate) ptr: NonNull>>, + pub(crate) ptr: PoolPointer<'static, GcBox>, pub(crate) alloc_id: usize, pub(crate) _marker: PhantomData<*mut &'id ()>, } @@ -27,7 +26,7 @@ impl<'id, T: Trace> WeakGc<'id, T> { // SAFETY: `_cx` proves the `Collector` is alive. // `alloc_id` confirms the allocation is still valid. // The allocator does not unmap memory, so reading a recycled block's `alloc_id` is safe - let is_valid = unsafe { (*self.ptr.as_ptr()).0.alloc_id == self.alloc_id }; + let is_valid = unsafe { (*self.ptr.as_ptr().as_ptr()).0.alloc_id == self.alloc_id }; if is_valid { Some(Gc { From c6aa49c1956654ddca58f9620d1e3efe88f74db3 Mon Sep 17 00:00:00 2001 From: shruti2522 Date: Tue, 5 May 2026 12:27:36 +0000 Subject: [PATCH 8/8] refactor: move RootNode, Root and RootLink into separate root module --- oscars/src/alloc/mempool3/mod.rs | 1 + .../src/collectors/mark_sweep_branded/gc.rs | 63 +----- .../src/collectors/mark_sweep_branded/mod.rs | 70 +++---- .../mark_sweep_branded/mutation_ctx.rs | 28 ++- .../src/collectors/mark_sweep_branded/root.rs | 195 ++++++++++++++++++ .../mark_sweep_branded/root_link.rs | 111 ---------- .../tests/api_compliance.rs | 3 +- .../mark_sweep_branded/tests/ephemeron.rs | 22 +- .../mark_sweep_branded/tests/mod.rs | 23 ++- .../mark_sweep_branded/tests/uaf.rs | 2 +- .../tests/ui/gc_cannot_escape_mutate.rs | 2 +- .../tests/ui/gc_cannot_escape_mutate.stderr | 4 +- .../tests/ui/gc_cannot_store_outer_scopr.rs | 2 +- .../tests/ui/root_cross_context.rs | 2 +- .../tests/ui/root_cross_context.stderr | 8 +- 15 files changed, 270 insertions(+), 266 deletions(-) create mode 100644 oscars/src/collectors/mark_sweep_branded/root.rs delete mode 100644 oscars/src/collectors/mark_sweep_branded/root_link.rs diff --git a/oscars/src/alloc/mempool3/mod.rs b/oscars/src/alloc/mempool3/mod.rs index f879ac7..90a8f23 100644 --- a/oscars/src/alloc/mempool3/mod.rs +++ b/oscars/src/alloc/mempool3/mod.rs @@ -19,6 +19,7 @@ pub enum PoolAllocError { LayoutError(LayoutError), OutOfMemory, AlignmentNotPossible, + AllocIdExhausted, } impl From for PoolAllocError { diff --git a/oscars/src/collectors/mark_sweep_branded/gc.rs b/oscars/src/collectors/mark_sweep_branded/gc.rs index 57693ca..032d540 100644 --- a/oscars/src/collectors/mark_sweep_branded/gc.rs +++ b/oscars/src/collectors/mark_sweep_branded/gc.rs @@ -1,20 +1,15 @@ //! Core pointer types. use crate::{ - alloc::mempool3::{PoolAllocator, PoolPointer}, + alloc::mempool3::PoolPointer, collectors::mark_sweep_branded::{ gc_box::GcBox, - mutation_ctx::MutationContext, - root_link::RootLink, trace::{Finalize, Trace}, }, }; use core::fmt; use core::marker::PhantomData; use core::ops::Deref; -use core::ptr::NonNull; - -pub(crate) type RootDropFn = unsafe fn(&mut PoolAllocator<'static>, NonNull); /// A transient pointer to a GC-managed value. #[derive(Debug)] @@ -55,62 +50,6 @@ impl<'gc, T: Trace + 'gc> Deref for Gc<'gc, T> { } } -/// Heap node backing a `Root`. -#[repr(C)] -pub(crate) struct RootNode<'id, T: Trace> { - /// Intrusive list link - pub(crate) link: RootLink, - /// Pointer to the allocation - pub(crate) gc_ptr: PoolPointer<'static, GcBox>, - /// Type-erased drop function for freeing this RootNode - pub(crate) drop_fn: RootDropFn, - /// Raw pointer to the Collector for freeing this node - pub(crate) collector_ptr: *const crate::collectors::mark_sweep_branded::Collector, - pub(crate) _marker: PhantomData<*mut &'id ()>, -} - -/// Type-erased version of [`RootNode`] for use during collection. -/// -/// Since [`RootNode`] is `repr(C)` and `link` is always the first field, -/// a `NonNull` from the sentinel iterator can be safely cast to -/// `NonNull` to read `gc_ptr` without knowing `T`. -#[repr(C)] -pub(crate) struct ErasedRootNode { - pub(crate) link: RootLink, - pub(crate) gc_ptr: PoolPointer<'static, GcBox<()>>, -} - -/// A handle that keeps a GC allocation live. -#[must_use = "dropping a root unregisters it from the GC"] -pub struct Root<'id, T: Trace> { - pub(crate) raw: NonNull>, -} - -impl<'id, T: Trace> Root<'id, T> { - /// Converts this root into a `Gc` pointer - pub fn get<'gc>(&self, _cx: &MutationContext<'id, 'gc>) -> Gc<'gc, T> { - Gc { - // SAFETY: `raw` is non-null and valid. - ptr: unsafe { self.raw.as_ref().gc_ptr }, - _marker: PhantomData, - } - } -} - -impl<'id, T: Trace> Drop for Root<'id, T> { - fn drop(&mut self) { - unsafe { - let node_ref = self.raw.as_ref(); - if node_ref.link.is_linked() { - RootLink::unlink(NonNull::from(&node_ref.link)); - } - // SAFETY: collector_ptr is valid for the lifetime of the GcContext - let collector = &*node_ref.collector_ptr; - collector.free_root_node(self.raw.cast::(), node_ref.drop_fn); - } - } -} - impl Finalize for Gc<'_, T> {} impl Trace for Gc<'_, T> { fn trace(&mut self, tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) { diff --git a/oscars/src/collectors/mark_sweep_branded/mod.rs b/oscars/src/collectors/mark_sweep_branded/mod.rs index 4085b0f..3f78a24 100644 --- a/oscars/src/collectors/mark_sweep_branded/mod.rs +++ b/oscars/src/collectors/mark_sweep_branded/mod.rs @@ -6,7 +6,7 @@ pub mod ephemeron; pub mod gc; pub mod gc_box; pub mod mutation_ctx; -pub mod root_link; +pub mod root; pub mod trace; pub mod weak; @@ -15,8 +15,9 @@ mod tests; pub use cell::GcRefCell; pub use ephemeron::Ephemeron; -pub use gc::{Gc, Root}; +pub use gc::Gc; pub use mutation_ctx::MutationContext; +pub use root::Root; pub use trace::{Finalize, Trace, Tracer}; pub use weak::WeakGc; @@ -25,7 +26,7 @@ use core::cell::{Cell, RefCell}; use core::marker::PhantomData; use core::ptr::NonNull; use gc_box::{DropFn, GcBox, GcColor}; -use root_link::RootSentinel; +use root::RootSentinel; use rust_alloc::vec::Vec; /// Type-erased ephemeron registration. @@ -69,40 +70,24 @@ impl Collector { }); } - /// Allocates a RootNode from the dedicated root pool. - pub(crate) fn alloc_root_node<'id, T: trace::Trace>( + /// Allocates a RootNode from the dedicated root pool and links it into the root list. + pub(crate) fn try_alloc_root_node<'id, T: trace::Trace>( &self, gc_ptr: PoolPointer<'static, GcBox>, - ) -> NonNull> { - unsafe fn drop_and_free( - pool: &mut PoolAllocator<'static>, - ptr: NonNull, - ) { - use crate::alloc::mempool3::PoolItem; - unsafe { - let typed_ptr = ptr.cast::>>(); - core::ptr::drop_in_place(typed_ptr.as_ptr()); - pool.free_slot(ptr); - } - } - + ) -> Result>, PoolAllocError> { let mut pool = self.root_pool.borrow_mut(); - let ptr = pool - .try_alloc(gc::RootNode { - link: root_link::RootLink::new(), - gc_ptr, - drop_fn: drop_and_free::, - collector_ptr: self as *const Collector, - _marker: PhantomData, - }) - .expect("root pool allocation failed"); - + let ptr = pool.try_alloc(root::RootNode::new_in(gc_ptr, self))?; // SAFETY: PoolItem is repr(transparent) over T; pointer address is identical. - ptr.as_ptr().cast::>() + let raw = ptr.as_ptr().cast::>(); + // SAFETY: `raw` points to a stable `RootNode` allocated in the pool. + unsafe { + root::RootLink::link_after(self.sentinel.as_ptr(), raw.cast::()); + } + Ok(raw) } /// Frees a RootNode back to the root pool. - pub(crate) fn free_root_node(&self, ptr: NonNull, drop_fn: gc::RootDropFn) { + pub(crate) fn free_root_node(&self, ptr: NonNull, drop_fn: root::RootDropFn) { let mut pool = self.root_pool.borrow_mut(); unsafe { (drop_fn)(&mut pool, ptr); @@ -111,11 +96,12 @@ impl Collector { /// Allocates a value from the pool. /// - /// # Panics + /// # Errors /// - /// Panics if the allocation ID counter wraps around to `FREED_ALLOC_ID` - /// This is a theoretical limit that would require `usize::MAX - 1` allocations. - pub(crate) fn alloc<'gc, T: trace::Trace + trace::Finalize + 'gc>( + /// Returns `Err(PoolAllocError::AllocIdExhausted)` if the allocation ID counter + /// has reached `FREED_ALLOC_ID` (`usize::MAX`). This is a theoretical limit + /// that would require `usize::MAX - 1` allocations. + pub(crate) fn try_alloc<'gc, T: trace::Trace + trace::Finalize + 'gc>( &'gc self, value: T, ) -> Result, PoolAllocError> { @@ -124,13 +110,9 @@ impl Collector { // Check for alloc_id wrap before incrementing. // If alloc_id reaches FREED_ALLOC_ID (usize::MAX), weak reference validation // would break because freed slots are marked with this sentinel value. - assert_ne!( - alloc_id, - GcBox::<()>::FREED_ALLOC_ID, - "Allocation ID counter wrapped to FREED_ALLOC_ID sentinel. \ - This indicates usize::MAX - 1 allocations have been made, \ - which would break weak reference ABA protection." - ); + if alloc_id == GcBox::<()>::FREED_ALLOC_ID { + return Err(PoolAllocError::AllocIdExhausted); + } self.generic_alloc_id.set(alloc_id.wrapping_add(1)); @@ -172,7 +154,7 @@ impl Collector { // SAFETY: link_ptr points to the `link` field which is first in repr(C) RootNode. // Casting to ErasedRootNode (also repr(C), same first two fields) lets us read // gc_ptr without knowing T, avoiding manual offset arithmetic. - let erased = link_ptr.cast::(); + let erased = link_ptr.cast::(); tracer.mark_raw((*erased.as_ptr()).gc_ptr.as_ptr().cast::()); } } @@ -244,12 +226,12 @@ impl Drop for Collector { use crate::alloc::mempool3::PoolItem; // Free all root nodes first - let all_roots: Vec<(NonNull, gc::RootDropFn)> = self + let all_roots: Vec<(NonNull, root::RootDropFn)> = self .root_pool .borrow() .iter_live_slots() .map(|ptr| unsafe { - let drop_fn = (*ptr.cast::>>().as_ptr()) + let drop_fn = (*ptr.cast::>>().as_ptr()) .0 .drop_fn; (ptr, drop_fn) diff --git a/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs b/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs index 8024b6d..7c9d788 100644 --- a/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs +++ b/oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs @@ -5,9 +5,9 @@ use crate::{ collectors::mark_sweep_branded::{ Collector, ephemeron::Ephemeron, - gc::{Gc, Root}, + gc::Gc, gc_box::GcBox, - root_link::RootLink, + root::Root, trace::{Finalize, Trace}, weak::WeakGc, }, @@ -22,8 +22,11 @@ pub struct MutationContext<'id, 'gc> { impl<'id, 'gc> MutationContext<'id, 'gc> { /// Allocates a value on the GC heap. - pub fn alloc(&self, value: T) -> Result, PoolAllocError> { - self.collector.alloc(value) + pub fn try_alloc( + &self, + value: T, + ) -> Result, PoolAllocError> { + self.collector.try_alloc(value) } /// Downgrades a `Gc` into a weak reference @@ -37,17 +40,12 @@ impl<'id, 'gc> MutationContext<'id, 'gc> { } /// Promotes a `Gc` pointer to a `Root` - pub fn root(&self, gc: Gc<'gc, T>) -> Root<'id, T> { - let raw = self.collector.alloc_root_node(gc.ptr); - - // SAFETY: `raw` points to a stable `RootNode`. - unsafe { - let sentinel_ptr = self.collector.sentinel.as_ptr(); - let link_ptr = raw.cast::(); - RootLink::link_after(sentinel_ptr, link_ptr); - } - - Root { raw } + pub fn root( + &self, + gc: Gc<'gc, T>, + ) -> Result, PoolAllocError> { + let raw = self.collector.try_alloc_root_node(gc.ptr)?; + Ok(Root { raw }) } /// Creates an ephemeron binding `key` to `value`. diff --git a/oscars/src/collectors/mark_sweep_branded/root.rs b/oscars/src/collectors/mark_sweep_branded/root.rs new file mode 100644 index 0000000..a77fa1c --- /dev/null +++ b/oscars/src/collectors/mark_sweep_branded/root.rs @@ -0,0 +1,195 @@ +use crate::{ + alloc::mempool3::{PoolAllocator, PoolPointer}, + collectors::mark_sweep_branded::{ + gc::Gc, gc_box::GcBox, mutation_ctx::MutationContext, trace::Trace, + }, +}; +use core::cell::Cell; +use core::marker::PhantomData; +use core::ptr::NonNull; + +pub(crate) type RootDropFn = unsafe fn(&mut PoolAllocator<'static>, NonNull); + +/// Intrusive link node +pub(crate) struct RootLink { + prev: Cell>>, + next: Cell>>, +} + +impl RootLink { + pub(crate) const fn new() -> Self { + Self { + prev: Cell::new(None), + next: Cell::new(None), + } + } + + /// Returns true if this node is currently part of a list + #[inline] + pub(crate) fn is_linked(&self) -> bool { + self.prev.get().is_some() + } + + /// Inserts `node` immediately after `anchor` + /// + /// # Safety + /// + /// Both `anchor` and `node` must remain at stable addresses until unlinked. + pub(crate) unsafe fn link_after(anchor: NonNull, node: NonNull) { + unsafe { + let anchor_ref = anchor.as_ref(); + let node_ref = node.as_ref(); + let old_next = anchor_ref.next.get(); + + node_ref.prev.set(Some(anchor)); + node_ref.next.set(old_next); + anchor_ref.next.set(Some(node)); + + if let Some(next) = old_next { + next.as_ref().prev.set(Some(node)); + } + } + } + + /// Removes `node` from the list. + /// + /// # Safety + /// + /// `node` must currently be linked. + pub(crate) unsafe fn unlink(node: NonNull) { + unsafe { + let node_ref = node.as_ref(); + let prev = node_ref.prev.get(); + let next = node_ref.next.get(); + + if let Some(p) = prev { + p.as_ref().next.set(next); + } + if let Some(n) = next { + n.as_ref().prev.set(prev); + } + + node_ref.prev.set(None); + node_ref.next.set(None); + } + } +} + +pub(crate) struct RootLinkIter { + current: Option>, +} + +impl Iterator for RootLinkIter { + type Item = NonNull; + + fn next(&mut self) -> Option { + let node = self.current?; + // SAFETY: nodes are pinned and valid during collection. + self.current = unsafe { node.as_ref().next.get() }; + Some(node) + } +} + +/// Sentinel node for the root list +#[repr(transparent)] +pub(crate) struct RootSentinel(core::pin::Pin>); + +impl RootSentinel { + pub(crate) fn new() -> Self { + Self(rust_alloc::boxed::Box::pin(RootLink::new())) + } + + /// Returns a pointer to the underlying RootLink + pub(crate) fn as_ptr(&self) -> NonNull { + // SAFETY: The sentinel is pinned and the pointer is derived from a valid Box. + unsafe { + NonNull::new_unchecked(self.0.as_ref().get_ref() as *const RootLink as *mut RootLink) + } + } + + /// Returns an iterator over all root nodes after this sentinel. + pub(crate) fn iter(&self) -> RootLinkIter { + let first = self.0.as_ref().next.get(); + RootLinkIter { current: first } + } +} + +/// Heap node backing a [`Root`] +#[repr(C)] +pub(crate) struct RootNode<'id, T: Trace> { + pub(crate) link: RootLink, + /// Pointer to the allocation + pub(crate) gc_ptr: PoolPointer<'static, GcBox>, + /// Type-erased drop function for freeing this RootNode + pub(crate) drop_fn: RootDropFn, + /// Raw pointer to the Collector for freeing this node + pub(crate) collector_ptr: *const crate::collectors::mark_sweep_branded::Collector, + pub(crate) _marker: PhantomData<*mut &'id ()>, +} + +impl<'id, T: Trace> RootNode<'id, T> { + /// Creates a new [`RootNode`] initialised for the given `gc_ptr` and `collector` + pub(crate) fn new_in( + gc_ptr: PoolPointer<'static, GcBox>, + collector: &crate::collectors::mark_sweep_branded::Collector, + ) -> Self { + unsafe fn drop_and_free(pool: &mut PoolAllocator<'static>, ptr: NonNull) { + use crate::alloc::mempool3::PoolItem; + unsafe { + let typed_ptr = ptr.cast::>>(); + core::ptr::drop_in_place(typed_ptr.as_ptr()); + pool.free_slot(ptr); + } + } + + Self { + link: RootLink::new(), + gc_ptr, + drop_fn: drop_and_free::, + collector_ptr: collector as *const _, + _marker: PhantomData, + } + } +} + +/// Type-erased version of [`RootNode`] for use during collection. +/// +/// Since [`RootNode`] is `repr(C)` and `link` is always the first field, +/// a `NonNull` from the sentinel iterator can be safely cast to +/// `NonNull` to read `gc_ptr` without knowing `T`. +#[repr(C)] +pub(crate) struct ErasedRootNode { + pub(crate) link: RootLink, + pub(crate) gc_ptr: PoolPointer<'static, GcBox<()>>, +} + +/// A handle that keeps a GC allocation live. +#[must_use = "dropping a root unregisters it from the GC"] +pub struct Root<'id, T: Trace> { + pub(crate) raw: NonNull>, +} + +impl<'id, T: Trace> Root<'id, T> { + /// Converts this root into a `Gc` pointer + pub fn get<'gc>(&self, _cx: &MutationContext<'id, 'gc>) -> Gc<'gc, T> { + Gc { + // SAFETY: `raw` is non null and valid + ptr: unsafe { self.raw.as_ref().gc_ptr }, + _marker: PhantomData, + } + } +} + +impl<'id, T: Trace> Drop for Root<'id, T> { + fn drop(&mut self) { + unsafe { + let node_ref = self.raw.as_ref(); + if node_ref.link.is_linked() { + RootLink::unlink(NonNull::from(&node_ref.link)); + } + // SAFETY: collector_ptr is valid for the lifetime of the GcContext + let collector = &*node_ref.collector_ptr; + collector.free_root_node(self.raw.cast::(), node_ref.drop_fn); + } + } +} diff --git a/oscars/src/collectors/mark_sweep_branded/root_link.rs b/oscars/src/collectors/mark_sweep_branded/root_link.rs deleted file mode 100644 index ec064b8..0000000 --- a/oscars/src/collectors/mark_sweep_branded/root_link.rs +++ /dev/null @@ -1,111 +0,0 @@ -//! Intrusive singly-linked root list. - -use core::cell::Cell; -use core::ptr::NonNull; - -/// Intrusive link node -pub(crate) struct RootLink { - prev: Cell>>, - next: Cell>>, -} - -impl RootLink { - /// Creates a new, unlinked node. - pub(crate) const fn new() -> Self { - Self { - prev: Cell::new(None), - next: Cell::new(None), - } - } - - /// Returns `true` if this node is currently part of a list. - #[inline] - pub(crate) fn is_linked(&self) -> bool { - self.prev.get().is_some() - } - - /// Inserts `node` immediately after `anchor`. - /// - /// # Safety - /// - /// Both `anchor` and `node` must remain at stable addresses until unlinked. - pub(crate) unsafe fn link_after(anchor: NonNull, node: NonNull) { - unsafe { - let anchor_ref = anchor.as_ref(); - let node_ref = node.as_ref(); - let old_next = anchor_ref.next.get(); - - node_ref.prev.set(Some(anchor)); - node_ref.next.set(old_next); - anchor_ref.next.set(Some(node)); - - if let Some(next) = old_next { - next.as_ref().prev.set(Some(node)); - } - } - } - - /// Removes `node` from the list. - /// - /// # Safety - /// - /// `node` must currently be linked. - pub(crate) unsafe fn unlink(node: NonNull) { - unsafe { - let node_ref = node.as_ref(); - let prev = node_ref.prev.get(); - let next = node_ref.next.get(); - - if let Some(p) = prev { - p.as_ref().next.set(next); - } - if let Some(n) = next { - n.as_ref().prev.set(prev); - } - - node_ref.prev.set(None); - node_ref.next.set(None); - } - } -} - -/// Iterator over root links in the intrusive list. -pub(crate) struct RootLinkIter { - current: Option>, -} - -impl Iterator for RootLinkIter { - type Item = NonNull; - - fn next(&mut self) -> Option { - let node = self.current?; - // SAFETY: nodes are pinned/heap-stable and valid during collection. - self.current = unsafe { node.as_ref().next.get() }; - Some(node) - } -} - -/// Sentinel node for the root list. -#[repr(transparent)] -pub(crate) struct RootSentinel(core::pin::Pin>); - -impl RootSentinel { - /// Creates a new sentinel node - pub(crate) fn new() -> Self { - Self(rust_alloc::boxed::Box::pin(RootLink::new())) - } - - /// Returns a pointer to the underlying RootLink - pub(crate) fn as_ptr(&self) -> NonNull { - // SAFETY: The sentinel is pinned and the pointer is derived from a valid Box. - unsafe { - NonNull::new_unchecked(self.0.as_ref().get_ref() as *const RootLink as *mut RootLink) - } - } - - /// Returns an iterator over all root nodes after this sentinel. - pub(crate) fn iter(&self) -> RootLinkIter { - let first = self.0.as_ref().next.get(); - RootLinkIter { current: first } - } -} diff --git a/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs b/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs index 4c50235..4258d44 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/api_compliance.rs @@ -1,7 +1,6 @@ #[cfg(test)] mod tests { - use crate::collectors::mark_sweep_branded::gc::RootNode; - use crate::collectors::mark_sweep_branded::root_link::RootLink; + use crate::collectors::mark_sweep_branded::root::{RootLink, RootNode}; #[test] fn root_link_is_thin() { diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ephemeron.rs b/oscars/src/collectors/mark_sweep_branded/tests/ephemeron.rs index a96e015..b793f95 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/ephemeron.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/ephemeron.rs @@ -4,9 +4,9 @@ use super::*; fn ephemeron_value_survives_when_key_is_rooted() { with_gc(|ctx| { let (root_key, eph) = ctx.mutate(|cx| { - let key = cx.alloc(1u32).unwrap(); - let value = cx.alloc(42u32).unwrap(); - let root_key = cx.root(key); + let key = cx.try_alloc(1u32).unwrap(); + let value = cx.try_alloc(42u32).unwrap(); + let root_key = cx.root(key).unwrap(); let eph = cx.alloc_ephemeron(key, value); (root_key, eph) }); @@ -27,8 +27,8 @@ fn ephemeron_value_survives_when_key_is_rooted() { fn ephemeron_value_collected_when_key_unrooted() { with_gc(|ctx| { let eph = ctx.mutate(|cx| { - let key = cx.alloc(1u32).unwrap(); - let value = cx.alloc(99u32).unwrap(); + let key = cx.try_alloc(1u32).unwrap(); + let value = cx.try_alloc(99u32).unwrap(); cx.alloc_ephemeron(key, value) }); @@ -51,10 +51,10 @@ fn ephemeron_chain_fixpoint() { // This requires the collector to run multiple fixpoint passes. with_gc(|ctx| { let (root_a, eph_ab, eph_bc) = ctx.mutate(|cx| { - let a = cx.alloc(1u32).unwrap(); - let b = cx.alloc(2u32).unwrap(); - let c = cx.alloc(3u32).unwrap(); - let root_a = cx.root(a); + let a = cx.try_alloc(1u32).unwrap(); + let b = cx.try_alloc(2u32).unwrap(); + let c = cx.try_alloc(3u32).unwrap(); + let root_a = cx.root(a).unwrap(); let eph_ab = cx.alloc_ephemeron(a, b); let eph_bc = cx.alloc_ephemeron(b, c); (root_a, eph_ab, eph_bc) @@ -92,8 +92,8 @@ fn ephemeron_entry_cleaned_up_after_sweep() { // Verify the collector's internal ephemeron list shrinks after dead entries are swept. with_gc(|ctx| { ctx.mutate(|cx| { - let key = cx.alloc(0u32).unwrap(); - let value = cx.alloc(0u32).unwrap(); + let key = cx.try_alloc(0u32).unwrap(); + let value = cx.try_alloc(0u32).unwrap(); cx.alloc_ephemeron(key, value); }); assert_eq!(ctx.ephemeron_count(), 1); diff --git a/oscars/src/collectors/mark_sweep_branded/tests/mod.rs b/oscars/src/collectors/mark_sweep_branded/tests/mod.rs index 0214d0e..f766a73 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/mod.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/mod.rs @@ -16,7 +16,7 @@ fn unrooted_alloc_is_swept() { with_gc(|ctx| { let weak = ctx.mutate(|cx| { cx.alloc_weak( - cx.alloc(JsObject { + cx.try_alloc(JsObject { name: "ephemeral".into(), value: 999, }) @@ -35,12 +35,13 @@ fn rooted_alloc_survives_collection() { with_gc(|ctx| { let root = ctx.mutate(|cx| { cx.root( - cx.alloc(JsObject { + cx.try_alloc(JsObject { name: "pinned".into(), value: 42, }) .unwrap(), ) + .unwrap() }); ctx.collect(); ctx.mutate(|cx| { @@ -56,7 +57,7 @@ fn weak_upgrade_after_collection_without_root_is_none() { with_gc(|ctx| { let weak = ctx.mutate(|cx| { cx.alloc_weak( - cx.alloc(JsObject { + cx.try_alloc(JsObject { name: "weak".into(), value: 10, }) @@ -75,15 +76,15 @@ fn weak_upgrade_with_live_root_is_some() { with_gc(|ctx| { let (root, weak) = ctx.mutate(|cx| { let obj = cx - .alloc(JsObject { + .try_alloc(JsObject { name: "strong".into(), value: 7, }) .unwrap(); - let root = cx.root(obj); + let root = cx.root(obj).unwrap(); let weak = cx.alloc_weak( - cx.alloc(JsObject { + cx.try_alloc(JsObject { name: "weak_entry".into(), value: 77, }) @@ -103,9 +104,9 @@ fn weak_upgrade_with_live_root_is_some() { fn multiple_roots_are_independent() { with_gc(|ctx| { let (root1, root2) = ctx.mutate(|cx| { - let obj1 = cx.alloc(100i32).unwrap(); - let obj2 = cx.alloc(200i32).unwrap(); - (cx.root(obj1), cx.root(obj2)) + let obj1 = cx.try_alloc(100i32).unwrap(); + let obj2 = cx.try_alloc(200i32).unwrap(); + (cx.root(obj1).unwrap(), cx.root(obj2).unwrap()) }); ctx.collect(); @@ -128,8 +129,8 @@ fn multiple_roots_are_independent() { fn root_escapes_closure_safely() { with_gc(|ctx| { let root = ctx.mutate(|cx| { - let obj = cx.alloc(555i32).unwrap(); - cx.root(obj) + let obj = cx.try_alloc(555i32).unwrap(); + cx.root(obj).unwrap() }); ctx.collect(); diff --git a/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs b/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs index f15bc57..5eab874 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/uaf.rs @@ -22,7 +22,7 @@ fn test_uaf() { with_gc(|cx| { let dropped = Cell::new(false); cx.mutate(|mcx| { - let _gc = mcx.alloc(DetectDrop(&dropped)).unwrap(); + let _gc = mcx.try_alloc(DetectDrop(&dropped)).unwrap(); }); cx.collect(); // Garbage collects 'gc' because it isn't rooted! assert!(dropped.get(), "It wasn't collected!"); diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.rs b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.rs index 7552163..afbbcd8 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.rs @@ -12,7 +12,7 @@ fn main() { // Returning Gc<'gc, i32> directly attempts to leak a shorter lifetime // into the outer scope. The compiler must reject this. let _escaped = ctx.mutate(|cx| { - cx.alloc(42i32).unwrap() // ERROR: Gc<'gc, i32> cannot escape mutate() + cx.try_alloc(42i32).unwrap() // ERROR: Gc<'gc, i32> cannot escape mutate() }); }); } diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.stderr b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.stderr index 8796e27..5c677b2 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.stderr +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_escape_mutate.stderr @@ -5,5 +5,5 @@ error: lifetime may not live long enough | --- return type of closure is oscars::collectors::mark_sweep_branded::Gc<'2, i32> | | | has type `&MutationContext<'_, '1>` -15 | cx.alloc(42i32).unwrap() // ERROR: Gc<'gc, i32> cannot escape mutate() - | ^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` +15 | cx.try_alloc(42i32).unwrap() // ERROR: Gc<'gc, i32> cannot escape mutate() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.rs b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.rs index efda0e0..eb56a61 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/gc_cannot_store_outer_scopr.rs @@ -17,7 +17,7 @@ fn main() { let mut holder: Option> = None; ctx.mutate(|cx| { - let gc = cx.alloc(42i32).unwrap(); + let gc = cx.try_alloc(42i32).unwrap(); // ERROR: `cx` (and therefore `gc`'s `'gc` lifetime) does not live // long enough to be stored in `holder`. holder = Some(Holder { gc }); diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs b/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs index 8c8e259..f4e3365 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs @@ -13,7 +13,7 @@ fn main() { with_gc(|ctx1| { with_gc(|ctx2| { // root carries 'id of ctx1 - let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32).unwrap())); + let root = ctx1.mutate(|cx| cx.root(cx.try_alloc(123i32).unwrap()).unwrap()); ctx2.mutate(|cx| { // ERROR: `'id` of `root` (ctx1) != `'id` of `cx` (ctx2) diff --git a/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.stderr b/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.stderr index 8848272..e4dc38e 100644 --- a/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.stderr +++ b/oscars/src/collectors/mark_sweep_branded/tests/ui/root_cross_context.stderr @@ -6,8 +6,8 @@ error: lifetime may not live long enough 14 | with_gc(|ctx2| { | ---- has type `GcContext<'1>` 15 | // root carries 'id of ctx1 -16 | let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32).unwrap())); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` +16 | let root = ctx1.mutate(|cx| cx.root(cx.try_alloc(123i32).unwrap()).unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` error: lifetime may not live long enough --> src/collectors/mark_sweep_branded/tests/ui/root_cross_context.rs:16:41 @@ -15,5 +15,5 @@ error: lifetime may not live long enough 13 | with_gc(|ctx1| { | ---- has type `GcContext<'1>` ... -16 | let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32).unwrap())); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'static` +16 | let root = ctx1.mutate(|cx| cx.root(cx.try_alloc(123i32).unwrap()).unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'static`