Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions notes/mark_sweep_branded_implementation_notes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
**Date**: 2026-04-23

## Changes from API Redesign Proposal

### 1. Allocation ID for Weak References (ABA Protection)

**Added:**
- `alloc_id: usize` in `GcBox<T>` 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<K, V>` (traces values only)
- `BTreeSet<T>` (no-op, keys are immutable)
- 3-tuple and 4-tuple
- Comments for `Rc<T>`, `Arc<T>`, `Cell<Option<T>>`

**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<Option<T>> Requires T: Copy

**Fixed:**
```rust
impl<T: Copy + Trace> Trace for Cell<Option<T>>
```

**Why:**
`self.set(Some(v))` requires moving `v`, which needs `T: Copy`. Without this bound, code fails to compile.

**Alternative:**
Use `GcRefCell<T>` 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.
1 change: 1 addition & 0 deletions oscars/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
71 changes: 71 additions & 0 deletions oscars/src/collectors/mark_sweep_branded/cell.rs
Original file line number Diff line number Diff line change
@@ -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<T>`].
pub struct GcRefCell<T: Trace> {
inner: RefCell<T>,
}

impl<T: Trace> GcRefCell<T> {
/// 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<T: Trace> 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<T: Trace> Deref for GcRefMut<'_, T> {
type Target = T;
fn deref(&self) -> &T {
&self.0
}
}

impl<T: Trace> DerefMut for GcRefMut<'_, T> {
fn deref_mut(&mut self) -> &mut T {
&mut self.0
}
}

impl<T: Trace> Finalize for GcRefCell<T> {}

impl<T: Trace> Trace for GcRefCell<T> {
fn trace(&mut self, tracer: &mut Tracer) {
self.inner.get_mut().trace(tracer);
}
}
102 changes: 102 additions & 0 deletions oscars/src/collectors/mark_sweep_branded/gc.rs
Original file line number Diff line number Diff line change
@@ -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<GcBox<T>>,
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>` can't
// 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> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it may be better to move RootNode, Root, and RootLink all into their own root module. I think that may be easier to reason about.

/// Intrusive list link
pub(crate) link: RootLink,
/// Pointer to the allocation
pub(crate) gc_ptr: NonNull<GcBox<T>>,
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<RootNode<'id, T>>,
}

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>,
Comment thread
nekevss marked this conversation as resolved.
Outdated
) -> 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<T: Trace> Finalize for Gc<'_, T> {}
impl<T: Trace> Trace for Gc<'_, T> {
fn trace(&mut self, tracer: &mut crate::collectors::mark_sweep_branded::trace::Tracer) {
tracer.mark(self);
}
}
23 changes: 23 additions & 0 deletions oscars/src/collectors/mark_sweep_branded/gc_box.rs
Original file line number Diff line number Diff line change
@@ -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<T: ?Sized> {
/// Reachability flag set by the mark phase.
pub(crate) marked: Cell<bool>,
/// Type-erased trace function.
pub(crate) trace_fn: Option<TraceFn>,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there ever an instance where trace_fn would be None. This should be fine to just be TraceFn as it's never not set.

/// Allocation ID used to validate weak pointers.
pub(crate) alloc_id: usize,
/// The user value.
pub(crate) value: T,
}

impl<T: ?Sized> GcBox<T> {
pub(crate) const FREED_ALLOC_ID: usize = usize::MAX;
}
Loading