diff --git a/notes/api-redesign-prototype/api_redesign_proposal.md b/notes/api-redesign-prototype/api_redesign_proposal.md index d70c852..0b0d8f6 100644 --- a/notes/api-redesign-prototype/api_redesign_proposal.md +++ b/notes/api-redesign-prototype/api_redesign_proposal.md @@ -6,7 +6,7 @@ Current `boa_gc` uses implicit rooting via `Clone`/`Drop` on `Gc`. Every clone touches root counts, adding overhead in hot VM paths. It also needs `thread_local`, blocking `no_std`. -This proposes lifetime-branded `Gc<'gc, T>` for zero cost pointers and explicit `Root` for persistence. +This proposes lifetime-branded `Gc<'gc, T>` for zero cost pointers and explicit `Root<'id, T>` for persistence. ## Core API @@ -31,62 +31,65 @@ pub struct GcRefCell { ### Weak Reference Separation ```rust -pub struct WeakGc { +pub struct WeakGc<'id, T: Trace + ?Sized> { ptr: NonNull>, + _marker: PhantomData<*mut &'id ()>, } -impl WeakGc { - pub fn upgrade<'gc>(&self, cx: &MutationContext<'gc>) -> Option> { ... } +impl<'id, T: Trace + ?Sized> WeakGc<'id, T> { + pub fn upgrade<'gc>(&self, cx: &MutationContext<'id, 'gc>) -> Option> { ... } } ``` -Weak references drop their tie to the single `'gc` lifetime. Instead, they are upgraded back into strong `Gc` pointers only when explicitly bound against an active safe `MutationContext<'gc>`. +Weak references carry the same `'id` brand as the context they came from. `upgrade` requires a matching `MutationContext<'id, 'gc>`, so cross-context upgrade is a compile error. The `'gc` lifetime ties the pointer to its collector. Copying is free, no root count manipulation. ### Root for Persistence ```rust -pub struct Root { - link: RootLink, // Intrusive list node (prev/next only), at offset 0 so bare link* == Root* - gc_ptr: NonNull>, // T: Sized keeps this thin for type erased offset_of! - /// Cross collector misuse detection only, plays no role in unlinking. - collector_id: u64, - _marker: PhantomData<*const ()>, +pub struct Root<'id, T: Trace> { + raw: NonNull>, } -impl Root { - pub fn get<'gc>(&self, cx: &MutationContext<'gc>) -> Gc<'gc, T> { - assert_eq!(self.collector_id, cx.collector.id); - // ... - } +#[repr(C)] +pub(crate) struct RootNode<'id, T: Trace> { + link: RootLink, // at offset 0, bare link* == RootNode* + gc_ptr: NonNull>, // T: Sized keeps this thin for type-erased offset_of! + _marker: PhantomData<*mut &'id ()>, +} + +impl<'id, T: Trace> Root<'id, T> { + pub fn get<'gc>(&self, _cx: &MutationContext<'id, 'gc>) -> Gc<'gc, T> { ... } } -impl Drop for Root { +impl<'id, T: Trace> Drop for Root<'id, T> { fn drop(&mut self) { - // O(1) self unlink: splice prev/next together, no list reference needed - if self.link.is_linked() { - unsafe { - RootLink::unlink(NonNull::from(&self.link)); + unsafe { + let node = Box::from_raw(self.raw.as_ptr()); + if node.link.is_linked() { + RootLink::unlink(NonNull::from(&node.link)); } } } } ``` -`Root` escapes the `'gc` lifetime. Returned as `Pin>>` for stable addresses (required by the intrusive list). Stores `collector_id` to catch cross-collector misuse at runtime — it is **not** used during unlink; `Drop` only touches the embedded `prev`/`next` pointers. +`Root<'id, T>` escapes the `'gc` lifetime but is tied to the `GcContext<'id>` that created it. The node is heap-allocated via `Box::into_raw`, keeping its address stable for the intrusive list without requiring `Pin` on the public API. `Drop` reclaims the allocation after unlinking. Cross-context misuse is a compile error, not a runtime panic. **No `Rc` required.** A root only needs its own embedded `prev`/`next` pointers to remove itself from the list. The `Collector` owns a **sentinel** node; insertion and removal are pure pointer surgery with no allocation and no reference counting. ### MutationContext ```rust -pub struct MutationContext<'gc> { +pub struct MutationContext<'id, 'gc> { collector: &'gc Collector, + _marker: PhantomData<*mut &'id ()>, } -impl<'gc> MutationContext<'gc> { +impl<'id, 'gc> MutationContext<'id, 'gc> { pub fn alloc(&self, value: T) -> Gc<'gc, T> { ... } - pub fn root(&self, gc: Gc<'gc, T>) -> Pin>> { ... } + pub fn alloc_weak(&self, value: T) -> WeakGc<'id, T> { ... } + pub fn root(&self, gc: Gc<'gc, T>) -> Root<'id, T> { ... } pub fn collect(&self) { ... } } ``` @@ -101,22 +104,24 @@ The `Collector` owns one **pinned sentinel** `RootLink` (a bare link node with n Collector::sentinel -> root_a.link -> root_b.link -> root_c.link -> None ``` -Roots insert themselves immediately after the sentinel via `RootLink::link_after`. During collection, `RootLink::iter_from_sentinel(sentinel)` starts from `sentinel.next`, so the sentinel itself is never yielded. For each link, `gc_ptr` is recovered via `offset_of!(Root, gc_ptr)` and used to mark the allocation. +Roots insert themselves immediately after the sentinel via `RootLink::link_after`. During collection, `RootLink::iter_from_sentinel(sentinel)` starts from `sentinel.next`, so the sentinel itself is never yielded. For each link, `gc_ptr` is recovered via `offset_of!(RootNode, gc_ptr)` and used to mark the allocation. A `debug_assert_eq!` with a second concrete type verifies the offset is stable across all `T: Sized`. ### Entry Point ```rust -pub struct GcContext { +pub struct GcContext<'id> { collector: Collector, + _marker: PhantomData<*mut &'id ()>, } -impl GcContext { - pub fn new() -> Self { ... } - pub fn mutate(&self, f: impl for<'gc> FnOnce(&MutationContext<'gc>) -> R) -> R { ... } +pub fn with_gc FnOnce(GcContext<'id>) -> R>(f: F) -> R { ... } + +impl<'id> GcContext<'id> { + pub fn mutate(&self, f: impl for<'gc> FnOnce(&MutationContext<'id, 'gc>) -> R) -> R { ... } } ``` -By owning the `Collector`, `GcContext` defines the entire host timeline. The `for<'gc>` pattern from gc-arena creates a unique lifetime isolating active context mutations per arena. +`with_gc` is the only way to create a `GcContext`. The `for<'id>` bound gives each context a fresh, unique lifetime that cannot unify with any other context's `'id`. `GcContext::mutate` threads that same `'id` into every `MutationContext` produced inside the closure. ### Tracing Mechanism ```rust @@ -139,7 +144,7 @@ Note: `trace` takes `&mut self` instead of `&self`, ensuring that potential movi | **Rooting** | Implicit (inc/dec on clone/drop) | Explicit (`Root`) | | **Copy cost** | Cell write | Zero | | **Drop cost** | TLS access (futex lock) | Zero (Copy type) | -| **Isolation** | Runtime only | Compile-time + runtime validation | +| **Isolation** | Runtime only | Compile-time only | ## Why This Works @@ -150,12 +155,12 @@ Note: `trace` takes `&mut self` instead of `&self`, ensuring that potential movi **Allocation**: Uses `mempool3::PoolAllocator` with size-class pooling instead of individual `Box` allocations, avoiding fragmentation. **Safety**: -- Cross-context caught at compile time for `Gc` -- Cross-collector caught at runtime for `Root` +- Cross-context use of `Gc`, `Root`, and `WeakGc` is a compile error, not a runtime panic +- No `collector_id` field, no atomic counter, no branch in `Root::get` - Explicit `!Send`/`!Sync` prevents threading bugs -- Intrusive sentinel based linked list for O(1) insertion and self-unlink +- Intrusive sentinel-based linked list for O(1) insertion and self-unlink - `Root` holds **no `Rc`**, unlink is pure pointer surgery on embedded `prev`/`next` -- `Pin>>` guarantees stable node addresses while linked +- Node address stability comes from `Box::into_raw`, `Pin` is not required on the public API ## Open Questions diff --git a/notes/api-redesign-prototype/prototype_findings.md b/notes/api-redesign-prototype/prototype_findings.md index 93b6c1a..30dcd61 100644 --- a/notes/api-redesign-prototype/prototype_findings.md +++ b/notes/api-redesign-prototype/prototype_findings.md @@ -1,6 +1,6 @@ # Prototype Findings -Prototyping lifetime-branded GC API for Boa. Testing if `Gc<'gc, T>` + `Root` is viable. +Prototyping lifetime-branded GC API for Boa. Testing if `Gc<'gc, T>` + `Root<'id, T>` is viable. Works, but migration will be challenging. @@ -51,28 +51,25 @@ Fix: `RefCell` inside collector, take `&self`. ```rust struct JsContext { - global_object: Root, // escapes 'gc + global_object: Root<'id, JsObject>, // escapes 'gc, tied to its GcContext<'id> } ``` -Root re-enters via `root.get(&cx)`. +Root re-enters via `root.get(cx)` where `cx: &MutationContext<'id, 'gc>` must share the same `'id`. -### Collector ID Validation +### Cross-Context Safety via `'id` Brand -Problem: `Root` from collector A used with context B → dangling pointer. +Problem: `Root` from context A used with context B -> dangling pointer. -Solution: Each collector gets unique ID, `Root` validates: +Solution: `with_gc` gives each context a fresh, unnamed `'id` lifetime via `for<'id>`. `Root<'id, T>` and `MutationContext<'id, 'gc>` share that brand, so the borrow checker rejects any mismatch at compile time: ```rust -impl Root { - pub fn get<'gc>(&self, cx: &MutationContext<'gc>) -> Gc<'gc, T> { - assert_eq!(self.collector_id, cx.collector.id); - // ... - } +impl<'id, T: Trace> Root<'id, T> { + pub fn get<'gc>(&self, _cx: &MutationContext<'id, 'gc>) -> Gc<'gc, T> { ... } } ``` -Catches cross-collector misuse where lifetimes can't help. +No runtime check, no `collector_id` field, no atomic counter. The compiler does all the work. ### Gc Access Safety @@ -103,7 +100,7 @@ Taking the `intrusive_collections` crate as inspiration, here is what we adopted 2. **O(1) Self Removal**: `unlink` drops nodes safely without a reference to the `Collector`. 3. **Double Unlink Protection**: `is_linked()` enforces safe dropping. 4. **Sentinel Node**: `Collector` owns a pinned `RootLink` as the list head. -5. **Type Erased Marking**: `Root` is `#[repr(C)]` with `link` at offset 0. The GC walks the links and recovers pointers using `offset_of!`. No `Trace` bound is needed. +5. **Type Erased Marking**: `RootNode` is `#[repr(C)]` with `link` at offset 0. The GC walks the links and recovers `gc_ptr` using `offset_of!(RootNode, gc_ptr)`. A `debug_assert_eq!` with a second concrete type checks the offset is stable across all `T: Sized`. No `Trace` bound is needed. #### Evolution of approaches @@ -132,17 +129,15 @@ Single threaded GC. Explicit bounds prevent cross thread bugs. ## Validated -**Compile-time isolation**: Borrow checker prevents mixing `Gc` from different contexts. - -**Runtime cross-collector detection**: `Root::get()` panics on wrong collector. +**Compile-time isolation**: Borrow checker prevents mixing `Gc`, `Root`, and `WeakGc` from different contexts. Cross-context use is a compile error, not a runtime panic. -**Root cleanup**: Drop removes from root list. +**Root cleanup**: Drop unlinking removes from root list. `Box::from_raw` reclaims the node allocation. **Interior Mutability Tracing**: Using `GcRefCell` allows `RefCell` semantics to persist efficiently while fulfilling `Trace` safety requirements without borrowing errors. -**Scopeless Weak Binding**: `WeakGc` survives successfully unbranded and can trace/upgrade against an arbitrary temporal `MutationContext` when actively touched again. +**Branded Weak Binding**: `WeakGc<'id, T>` carries the same context brand. `upgrade` requires a matching `MutationContext<'id, 'gc>`, so cross-context upgrade is also a compile error. -**Functional Builtin Prototyping**: Explicit tests matching exactly against definitions like `Array.prototype.push` (taking a `&Gc<'gc, GcRefCell>>` + `arg` buffer bound to `_cx: &MutationContext<'gc>`) compiled gracefully and safely. +**Functional Builtin Prototyping**: Explicit tests matching exactly against definitions like `Array.prototype.push` (taking a `&Gc<'gc, GcRefCell>>` + `arg` buffer bound to `_cx: &MutationContext<'id, 'gc>`) compiled gracefully and safely. ### Performance @@ -161,51 +156,20 @@ Single threaded GC. Explicit bounds prevent cross thread bugs. **Migration**: Boa has thousands of `Gc` uses. Need to add `'gc` everywhere. Phasing gradually starting with isolated systems can be done -### `Pin<&mut Root>` for Escaping Roots - -Raised during review: could we use `Pin<&mut Root>` instead of `Pin>>` to avoid a heap allocation per root? - -**No, not for escaping roots.** Stack allocation fails because: - -1. `Root` is created inside `mutate()`. -2. Escaping roots must outlive `mutate()`. -3. `Pin<&mut>` requires a stable address. - -We cannot move a `&mut` out of its closure frame without changing its address and violating `Pin` - -`Pin>` fixes this: the pointer moves out, but the heap allocation stays fixed. Cost belongs to one `Box` per root. - -#### Workaround: `root_in_place` - -Zero allocation is possible if the caller pre-allocates the `Root` slot on the outer stack: - -```rust -let mut slot = std::mem::MaybeUninit::>::uninit(); - -ctx.mutate(|cx| { - let obj = cx.alloc(JsObject { name: "global".into(), value: 0 }); - let root = cx.root_in_place(&mut slot, obj); -}); - -let root = unsafe { slot.assume_init_ref() }; -``` +### Root Node Stability via `Box::into_raw` -`root_in_place` writes into the slot, pins it, links it and returns `Pin<&mut Root>`. This matches V8's `HandleScope`: no allocation, O(1) creation. +`Pin>>` was the original approach: pinning kept the intrusive list node address stable. -**Reasons to skip this for now:** -1. Caller must know `T` upfront to size the `MaybeUninit` slot. -2. Requires `unsafe` to read the slot later. -3. `Pin>` is simpler and safer for validating the core API right now. +The current approach is simpler: `cx.root()` allocates the node with `Box::new`, calls `Box::into_raw` immediately, and stores the raw `NonNull` inside a thin `Root<'id, T>` handle. The heap address is stable by construction. `Drop` calls `Box::from_raw` to reclaim it after unlinking. -*We can prototype this later if needed.* +This removes `Pin` from the public API entirely. `root()` returns `Root<'id, T>` (one word on the stack), not `Pin>>`. The cost is still one heap allocation per escaping root, same as before. ## Conclusion -`Gc<'gc, T>` + `Root` is: -- **Sound**: Compile-time catches misuse -- **Runtime-safe**: Collector ID validation catches Root misuse -- **Fast**: Zero cost transient pointers +`Gc<'gc, T>` + `Root<'id, T>` is: +- **Sound**: Compile-time catches all cross-context misuse for `Gc`, `Root` and `WeakGc` +- **Fast**: Zero cost transient pointers, no atomic counters, no branch in `Root::get` - **Feasible**: Can coexist with current API Main risk is migration effort, we can go with the phased approach diff --git a/oscars/examples/api_prototype/gc.rs b/oscars/examples/api_prototype/gc.rs index bc64cd1..30cf34b 100644 --- a/oscars/examples/api_prototype/gc.rs +++ b/oscars/examples/api_prototype/gc.rs @@ -8,9 +8,6 @@ use core::mem::offset_of; use core::pin::Pin; use core::ptr::NonNull; use oscars::alloc::mempool3::PoolAllocator; -use std::sync::atomic::{AtomicU64, Ordering}; - -static NEXT_COLLECTOR_ID: AtomicU64 = AtomicU64::new(1); pub(crate) struct GcBox { pub(crate) marked: Cell, @@ -39,47 +36,50 @@ impl<'gc, T: Trace + 'gc> Gc<'gc, T> { /// Pinned root handle that keeps a GC allocation live across `mutate()` boundaries. /// /// Uses an intrusive linked list. `#[repr(C)]` with `link` first allows -/// casting `*mut RootLink` directly to `*mut Root` without pointer math. +/// casting `*mut RootLink` directly to `*mut RootNode` without pointer math. /// -/// `Pin>>` keeps the list link stable in memory. +/// `Root<'id, T>` wraps a raw pointer to keep the list link stable in memory +/// and avoid moving `Box`, preventing Stacked Borrows aliasing UB. /// `T: Sized` ensures `gc_ptr` is a single word thin pointer, making /// type-erased `*mut u8` collector reads sound +/// +/// The invariant `'id` lifetime ties this root to a specific `GcContext<'id>`, +/// preventing cross-context usage at compile time. #[must_use = "roots must be kept alive to prevent collection"] +pub struct Root<'id, T: Trace> { + raw: NonNull>, +} + #[repr(C)] -pub struct Root { +pub(crate) struct RootNode<'id, T: Trace> { /// Intrusive list node. Placed at offset 0 for direct base pointer casting. pub(crate) link: RootLink, /// GC allocation pointer. `T: Sized` ensures this is a thin pointer pub(crate) gc_ptr: NonNull>, - /// ID of the `Collector` that owns this root (for misuse detection). - pub(crate) collector_id: u64, - pub(crate) _marker: PhantomData<*const ()>, + /// Invariant lifetime marker tying this root to a specific `GcContext`. + pub(crate) _marker: PhantomData<*mut &'id ()>, } -impl Root { - pub fn get<'gc>(&self, cx: &MutationContext<'gc>) -> Gc<'gc, T> { - assert_eq!( - self.collector_id, cx.collector.id, - "root from different collector" - ); +impl<'id, T: Trace> Root<'id, T> { + /// Converts this root back into a `Gc<'gc, T>` within a mutation context. + /// + /// The `'id` lifetime on `Root` must match the `'id` lifetime on `MutationContext`. + pub fn get<'gc>(&self, _cx: &MutationContext<'id, 'gc>) -> Gc<'gc, T> { Gc { - ptr: self.gc_ptr, + ptr: unsafe { self.raw.as_ref().gc_ptr }, _marker: PhantomData, } } - - pub fn belongs_to(&self, cx: &MutationContext<'_>) -> bool { - self.collector_id == cx.collector.id - } } -impl Drop for Root { +impl<'id, T: Trace> Drop for Root<'id, T> { fn drop(&mut self) { // SAFETY: Node address is stable, neighbors outlive this node. // Unlinking touches only the embedded prev/next pointers. - if self.link.is_linked() { - unsafe { - RootLink::unlink(NonNull::from(&self.link)); + unsafe { + let node = Box::from_raw(self.raw.as_ptr()); + if node.link.is_linked() { + RootLink::unlink(NonNull::from(&node.link)); } } } @@ -115,7 +115,6 @@ struct PoolEntry { /// Roots insert after the sentinel on creation and self-unlink on drop (both O(1)). /// Marking walks the chain and reads `gc_ptr` via `offset_of!`. pub struct Collector { - pub(crate) id: u64, pool: RefCell>, pool_entries: RefCell>, /// Pinned sentinel: pure `RootLink`, no payload. Head of the root chain. @@ -126,7 +125,6 @@ pub struct Collector { impl Collector { pub fn new() -> Self { Self { - id: NEXT_COLLECTOR_ID.fetch_add(1, Ordering::Relaxed), pool: RefCell::new(PoolAllocator::default()), pool_entries: RefCell::new(Vec::new()), sentinel: Box::pin(RootLink::new()), @@ -187,7 +185,16 @@ impl Collector { // `gc_ptr` comes right after it. Because `T` is `Sized`, the distance to `gc_ptr` // is exactly the same no matter what generic type `T` is. We use `Root` as a // dummy type just to calculate this fixed offset. - let gc_ptr_offset = offset_of!(Root, gc_ptr); + let gc_ptr_offset = offset_of!(RootNode, gc_ptr); + // `#[repr(C)]` + `T: Sized` guarantees `gc_ptr` is always a single + // pointer-width field at the same offset regardless of `T`. Verify + // this with a second concrete type so any future repr change is caught + // immediately rather than silently corrupting the root traversal. + debug_assert_eq!( + gc_ptr_offset, + offset_of!(RootNode, gc_ptr), + "RootNode layout must be identical for all T: Sized" + ); let root_count = RootLink::iter_from_sentinel(sentinel_ptr).count(); println!( @@ -228,17 +235,43 @@ impl Drop for Collector { } } -pub struct GcContext { +/// The main GC context that owns a collector. +/// +/// The invariant `'id` lifetime uniquely identifies this context, ensuring +/// `Root<'id, T>` can only be used with its corresponding `MutationContext`. +pub struct GcContext<'id> { collector: Collector, + /// Invariant lifetime marker. Using `*mut &'id ()` makes it invariant. + _marker: PhantomData<*mut &'id ()>, } -impl GcContext { - pub fn new() -> Self { - Self { - collector: Collector::new(), - } - } - pub fn mutate(&self, f: impl for<'gc> FnOnce(&MutationContext<'gc>) -> R) -> R { +/// Creates a new GC context and passes it to the closure. +/// +/// The `for<'id>` bound creates a fresh `'id` for each context, +/// preventing cross-context usage of `Root` or `WeakGc`. +/// +/// # Example +/// ```ignore +/// with_gc(|ctx| { +/// ctx.mutate(|cx| { +/// let v = cx.alloc(42i32); +/// let root = cx.root(v); +/// assert_eq!(*root.get(cx).get(), 42); +/// }); +/// }); +/// ``` +pub fn with_gc FnOnce(GcContext<'id>) -> R>(f: F) -> R { + f(GcContext { + collector: Collector::new(), + _marker: PhantomData, + }) +} + +impl<'id> GcContext<'id> { + /// Runs a mutation closure with access to GC operations. + /// + /// The `'gc` lifetime brands pointers to this phase, while `'id` ties them to this context. + pub fn mutate(&self, f: impl for<'gc> FnOnce(&MutationContext<'id, 'gc>) -> R) -> R { let cx = MutationContext { collector: &self.collector, _marker: PhantomData, @@ -247,58 +280,59 @@ impl GcContext { } } -impl Default for GcContext { - fn default() -> Self { - Self::new() - } -} - -pub struct MutationContext<'gc> { +/// Context for GC mutations within a `mutate()` closure. +/// +/// `'id` ties this context to a specific `GcContext<'id>` (invariant lifetime). +/// `'gc` brands all `Gc<'gc, T>` pointers to this mutation phase. +pub struct MutationContext<'id, 'gc> { pub(crate) collector: &'gc Collector, - pub(crate) _marker: PhantomData<*const ()>, + /// Invariant lifetime marker for context identity. + pub(crate) _marker: PhantomData<*mut &'id ()>, } -impl<'gc> MutationContext<'gc> { +impl<'id, 'gc> MutationContext<'id, 'gc> { pub fn alloc(&self, value: T) -> Gc<'gc, T> { self.collector.alloc(value) } - pub fn alloc_weak(&self, value: T) -> WeakGc { + pub fn alloc_weak(&self, value: T) -> WeakGc<'id, T> { let gc = self.alloc(value); - WeakGc { ptr: gc.ptr } + WeakGc { + ptr: gc.ptr, + _marker: PhantomData, + } } - /// Roots a `Gc<'gc, T>` and returns a `Pin>>`. + /// Roots a `Gc<'gc, T>` and returns an opaque `Root<'id, T>` handle. /// - /// `Pin` is required to keep the link address stable while in the list. - /// Inserts after the sentinel (O(1)), self-unlinks on drop (O(1)). - pub fn root(&self, gc: Gc<'gc, T>) -> Pin>> { + /// The returned `Root` is tied to this specific `GcContext<'id>` at compile time. + /// + /// The returned handle encapsulates a dynamically allocated linked list node + /// whose address remains stable. Inserts after the sentinel (O(1)), self-unlinks on drop (O(1)). + pub fn root(&self, gc: Gc<'gc, T>) -> Root<'id, T> { let gc_ptr = gc.ptr; - let root = Box::pin(Root { + let node = Box::new(RootNode { link: RootLink::new(), gc_ptr, - collector_id: self.collector.id, _marker: PhantomData, }); + let raw = unsafe { NonNull::new_unchecked(Box::into_raw(node)) }; + // SAFETY: - // * root is pinned: address is stable for its lifetime. + // * root pointer address is stable for its lifetime. // * sentinel is pinned in Collector: outlives all roots. // * Insertion only touches sentinel.next and root.link.prev/next. unsafe { let sentinel_ptr = NonNull::new_unchecked(self.collector.sentinel.as_ref().get_ref() as *const RootLink as *mut RootLink); - let link_ptr = NonNull::from(&root.link); + let link_ptr = raw.cast::(); RootLink::link_after(sentinel_ptr, link_ptr); } - root - } - - pub fn collector_id(&self) -> u64 { - self.collector.id + Root { raw } } pub fn collect(&self) { diff --git a/oscars/examples/api_prototype/main.rs b/oscars/examples/api_prototype/main.rs index 4a21341..1756554 100644 --- a/oscars/examples/api_prototype/main.rs +++ b/oscars/examples/api_prototype/main.rs @@ -14,7 +14,7 @@ mod weak; use cell::GcRefCell; use gc::Gc; -use gc::{GcContext, MutationContext}; +use gc::{MutationContext, with_gc}; use trace::{Finalize, Trace, Tracer}; struct JsObject { @@ -46,10 +46,10 @@ impl<'gc> Finalize for JsArray<'gc> {} /// Replica of Boa Builtin Function: Array.prototype.push /// This fully proves that standalone builtin functions can accept the `'gc` /// context bounded pointers without lifetime errors or borrow checking issues -fn array_push<'gc>( +fn array_push<'id, 'gc>( this: &Gc<'gc, GcRefCell>>, args: &[Gc<'gc, JsObject>], - _cx: &MutationContext<'gc>, + _cx: &MutationContext<'id, 'gc>, ) -> usize { let mut array = this.get().borrow_mut(); @@ -63,54 +63,56 @@ fn array_push<'gc>( fn main() { println!("GC API Prototype Example (Redesign Additions)\n"); - let ctx = GcContext::new(); - // example 1: boa array migration println!("1. Boa Array Migration Example:\n"); - ctx.mutate(|cx| { - let val1 = cx.alloc(JsObject { - name: "item1".to_string(), - value: 42, - }); - let val2 = cx.alloc(JsObject { - name: "item2".to_string(), - value: 43, - }); + with_gc(|ctx| { + ctx.mutate(|cx| { + let val1 = cx.alloc(JsObject { + name: "item1".to_string(), + value: 42, + }); + let val2 = cx.alloc(JsObject { + name: "item2".to_string(), + value: 43, + }); - let array = cx.alloc(GcRefCell::new(JsArray { - elements: Vec::new(), - })); + let array = cx.alloc(GcRefCell::new(JsArray { + elements: Vec::new(), + })); - println!(" Calling array_push built-in replica:"); - let new_len = array_push(&array, &[val1, val2], cx); + println!(" Calling array_push built-in replica:"); + let new_len = array_push(&array, &[val1, val2], cx); - println!(" Returned length: {}", new_len); - println!( - " First element value: {}\n", - array.get().borrow().elements[0].get().value - ); + println!(" Returned length: {}", new_len); + println!( + " First element value: {}\n", + array.get().borrow().elements[0].get().value + ); + }); }); // example 2: weak refs println!("2. WeakGc upgrade example:\n"); - ctx.mutate(|cx| { - let target = cx.alloc(JsObject { - name: "target".to_string(), - value: 5, - }); - let _root = cx.root(target); // force it alive - let weak = cx.alloc_weak(JsObject { - name: "weak_data".to_string(), - value: 10, - }); + with_gc(|ctx| { + ctx.mutate(|cx| { + let target = cx.alloc(JsObject { + name: "target".to_string(), + value: 5, + }); + let _root = cx.root(target); // force it alive + let weak = cx.alloc_weak(JsObject { + name: "weak_data".to_string(), + value: 10, + }); - cx.collect(); + cx.collect(); - match weak.upgrade(cx) { - Some(gc) => println!(" Weak object is accessible: {}", gc.get().value), - None => println!(" Weak object was swept"), - } - println!(); + match weak.upgrade(cx) { + Some(gc) => println!(" Weak object is accessible: {}", gc.get().value), + None => println!(" Weak object was swept"), + } + println!(); + }); }); println!("Done!"); @@ -122,102 +124,112 @@ mod tests { #[test] fn context_mutate() { - let ctx = GcContext::new(); - ctx.mutate(|cx| { - let a = cx.alloc(42i32); - assert_eq!(*a.get(), 42); + with_gc(|ctx| { + ctx.mutate(|cx| { + let a = cx.alloc(42i32); + assert_eq!(*a.get(), 42); + }); }); } #[test] fn root_works_in_context() { - let ctx = GcContext::new(); - ctx.mutate(|cx| { - let obj = cx.alloc(123i32); - let root = cx.root(obj); - let gc = root.get(cx); - assert_eq!(*gc.get(), 123); + with_gc(|ctx| { + ctx.mutate(|cx| { + let obj = cx.alloc(123i32); + let root = cx.root(obj); + let gc = root.get(cx); + assert_eq!(*gc.get(), 123); + }); }); } - #[test] - fn root_rejects_different_collector() { - let ctx1 = GcContext::new(); - let ctx2 = GcContext::new(); - - let root = ctx1.mutate(|cx| { - let obj = cx.alloc(123i32); - cx.root(obj) - }); - - let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - ctx2.mutate(|cx| { - let _gc = root.get(cx); - }); - })); - assert!(result.is_err()); - } + // Cross-context root usage is rejected at compile time. + // See tests/ui/root_cross_context.rs for the trybuild verified version + // + // The following would fail to compile (each with_gc call produces a + // distinct non-unifiable 'id, so the root from ctx1 can't cross into ctx2): + // ```compile_fail + // with_gc(|ctx1| { + // with_gc(|ctx2| { + // let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32))); + // ctx2.mutate(|cx| { let _gc = root.get(cx); }); // ERROR + // }); + // }); + // ``` #[test] fn refcell_trace() { - let ctx = GcContext::new(); - ctx.mutate(|cx| { - let cell = cx.alloc(GcRefCell::new(100i32)); - *cell.get().borrow_mut() = 200; - assert_eq!(*cell.get().borrow(), 200); + with_gc(|ctx| { + ctx.mutate(|cx| { + let cell = cx.alloc(GcRefCell::new(100i32)); + *cell.get().borrow_mut() = 200; + assert_eq!(*cell.get().borrow(), 200); + }); }); } + // Root<'id, T> cannot outlive its GcContext<'id>: the invariant 'id + // lifetime ties them together inside the same with_gc closure. #[test] - fn root_outlives_context() { - // Ensures escaping roots do not trigger UAF after collector drops - let escaped_root = { - let ctx = GcContext::new(); - ctx.mutate(|cx| cx.root(cx.alloc(555i32))) - }; - drop(escaped_root); + fn root_requires_context_to_live() { + with_gc(|ctx| { + // Root created in the first mutate phase... + let root = ctx.mutate(|cx| cx.root(cx.alloc(555i32))); + + // ...is valid across subsequent mutate phases on the same context. + ctx.mutate(|cx| { + assert_eq!(*root.get(cx).get(), 555); + }); + + // root and ctx both drop at the end of this closure; the 'id brand + // prevents root from escaping into a scope where ctx is gone. + }); } #[test] fn weak_upgrade() { - let ctx = GcContext::new(); - ctx.mutate(|cx| { - let obj = cx.alloc(JsObject { - name: "test".into(), - value: 42, - }); - let weak = cx.alloc_weak(JsObject { - name: "weak".into(), - value: 10, - }); + with_gc(|ctx| { + ctx.mutate(|cx| { + let obj = cx.alloc(JsObject { + name: "test".into(), + value: 42, + }); + let weak = cx.alloc_weak(JsObject { + name: "weak".into(), + value: 10, + }); - // Sweep unrooted weak pointers. - cx.collect(); - assert!(weak.upgrade(cx).is_none()); + // Sweep unrooted weak pointers. + cx.collect(); + assert!(weak.upgrade(cx).is_none()); - // Rooted objects remain alive - let root = cx.root(obj); - cx.collect(); - let _ = root.get(cx); + // Rooted objects remain alive. + let root = cx.root(obj); + cx.collect(); + let _ = root.get(cx); + }); }); } #[test] fn bulk_allocation_cleanup() { - let ctx = GcContext::new(); - ctx.mutate(|cx| { - for i in 0..100 { - cx.alloc(JsObject { - name: "bulk".into(), - value: i, - }); - } + with_gc(|ctx| { + ctx.mutate(|cx| { + for i in 0..100 { + cx.alloc(JsObject { + name: "bulk".into(), + value: i, + }); + } + }); + // Deallocates automatically when the closure returns. }); - // Deallocates out of scope without leaking } #[test] #[cfg_attr(not(debug_assertions), ignore)] + #[cfg(not(miri))] fn compile_fail_tests() { let t = trybuild::TestCases::new(); t.compile_fail("examples/api_prototype/tests/ui/*.rs"); @@ -226,102 +238,108 @@ mod tests { // Tests verifying the lifetime-bounded `'gc` invariant prevents UAF #[test] fn unrooted_allocs_are_collected() { - let ctx = GcContext::new(); - ctx.mutate(|cx| { - let weak = cx.alloc_weak(JsObject { - name: "ephemeral".into(), - value: 999, - }); + with_gc(|ctx| { + ctx.mutate(|cx| { + let weak = cx.alloc_weak(JsObject { + name: "ephemeral".into(), + value: 999, + }); - cx.collect(); - assert!(weak.upgrade(cx).is_none()); + cx.collect(); + assert!(weak.upgrade(cx).is_none()); + }); }); } #[test] fn pinned_root_keeps_gc_alive() { - // Ensures Pin>> keeps allocations alive. - let ctx = GcContext::new(); - ctx.mutate(|cx| { - let obj = cx.alloc(JsObject { - name: "pinned".into(), - value: 42, - }); + // Ensures `Root` handle keeps allocations alive. + with_gc(|ctx| { + ctx.mutate(|cx| { + let obj = cx.alloc(JsObject { + name: "pinned".into(), + value: 42, + }); - let pinned_root = cx.root(obj); - cx.collect(); + let pinned_root = cx.root(obj); + cx.collect(); - let gc = pinned_root.get(cx); - assert_eq!(gc.get().value, 42); - assert_eq!(gc.get().name, "pinned"); + let gc = pinned_root.get(cx); + assert_eq!(gc.get().value, 42); + assert_eq!(gc.get().name, "pinned"); + }); }); } #[test] fn multiple_roots_are_independent() { - let ctx = GcContext::new(); - ctx.mutate(|cx| { - let obj1 = cx.alloc(100i32); - let obj2 = cx.alloc(200i32); + with_gc(|ctx| { + ctx.mutate(|cx| { + let obj1 = cx.alloc(100i32); + let obj2 = cx.alloc(200i32); - let root1 = cx.root(obj1); - let root2 = cx.root(obj2); + let root1 = cx.root(obj1); + let root2 = cx.root(obj2); - cx.collect(); + cx.collect(); - assert_eq!(*root1.get(cx).get(), 100); - assert_eq!(*root2.get(cx).get(), 200); + assert_eq!(*root1.get(cx).get(), 100); + assert_eq!(*root2.get(cx).get(), 200); - drop(root1); - cx.collect(); + drop(root1); + cx.collect(); - assert_eq!(*root2.get(cx).get(), 200); + assert_eq!(*root2.get(cx).get(), 200); + }); }); } #[test] fn root_get_requires_mut_ctx() { - // Ensures Root::get() requires a valid MutationContext<'gc>. - let ctx = GcContext::new(); - - let root = ctx.mutate(|cx| { - let obj = cx.alloc(JsObject { - name: "escaped".into(), - value: 123, + // Ensures Root::get() requires a valid MutationContext<'id, 'gc>. + // The root produced by the first mutate is usable in a second mutate + // on the same context because they share the same 'id brand. + with_gc(|ctx| { + let root = ctx.mutate(|cx| { + let obj = cx.alloc(JsObject { + name: "escaped".into(), + value: 123, + }); + cx.root(obj) }); - cx.root(obj) - }); - ctx.mutate(|cx| { - let gc = root.get(cx); - assert_eq!(gc.get().value, 123); + ctx.mutate(|cx| { + let gc = root.get(cx); + assert_eq!(gc.get().value, 123); + }); }); } #[test] fn gc_lifetime_tied_to_mut_ctx() { // Ensures Gc<'gc, T> cannot outlive the mutation phase. - let ctx = GcContext::new(); - ctx.mutate(|cx| { - let gc = cx.alloc(42i32); - assert_eq!(*gc.get(), 42); + with_gc(|ctx| { + ctx.mutate(|cx| { + let gc = cx.alloc(42i32); + assert_eq!(*gc.get(), 42); + }); }); } #[test] fn seq_mutations_independent() { - let ctx = GcContext::new(); - - let root = ctx.mutate(|cx| { - let obj = cx.alloc(1i32); - cx.root(obj) - }); + with_gc(|ctx| { + let root = ctx.mutate(|cx| { + let obj = cx.alloc(1i32); + cx.root(obj) + }); - ctx.mutate(|cx| { - let new_obj = cx.alloc(2i32); + ctx.mutate(|cx| { + let new_obj = cx.alloc(2i32); - assert_eq!(*root.get(cx).get(), 1); - assert_eq!(*new_obj.get(), 2); + assert_eq!(*root.get(cx).get(), 1); + assert_eq!(*new_obj.get(), 2); + }); }); } } diff --git a/oscars/examples/api_prototype/tests/ui/root_cross_context.rs b/oscars/examples/api_prototype/tests/ui/root_cross_context.rs new file mode 100644 index 0000000..845b5cb --- /dev/null +++ b/oscars/examples/api_prototype/tests/ui/root_cross_context.rs @@ -0,0 +1,53 @@ +//! Compile-fail test: `Root<'id, T>` cannot be used across `with_gc` contexts. +//! +//! Each `with_gc` call creates a unique, unnamed `'id` lifetime. The borrow checker +//! cannot unify two distinct `'id` variables, so passing a `Root<'id1, T>` to a +//!`MutationContext<'id2, '_>` is a type error. + +use core::marker::PhantomData; + +struct Root<'id, T> { + _marker: PhantomData<(*mut &'id (), T)>, +} + +impl<'id, T> Root<'id, T> { + fn get<'gc>(&self, _cx: &MutationContext<'id, 'gc>) -> &'gc T { + todo!() + } +} + +struct MutationContext<'id, 'gc> { + _marker: PhantomData<(*mut &'id (), &'gc ())>, +} + +impl<'id, 'gc> MutationContext<'id, 'gc> { + fn alloc(&self, _v: T) -> T { todo!() } + fn root(&self, _v: T) -> Root<'id, T> { todo!() } +} + +struct GcContext<'id> { + _marker: PhantomData<*mut &'id ()>, +} + +impl<'id> GcContext<'id> { + fn mutate(&self, f: impl for<'gc> FnOnce(&MutationContext<'id, 'gc>) -> R) -> R { + f(&MutationContext { _marker: PhantomData }) + } +} + +fn with_gc FnOnce(GcContext<'id>) -> R>(f: F) -> R { + f(GcContext { _marker: PhantomData }) +} + +fn main() { + // Each with_gc produces a fresh 'id; the root from ctx1 cannot be used + // with the MutationContext from ctx2, this must fail to compile. + with_gc(|ctx1| { + with_gc(|ctx2| { + let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32))); + ctx2.mutate(|cx| { + let _gc = root.get(cx); // ERROR: 'id mismatch + }); + }); + }); +} diff --git a/oscars/examples/api_prototype/tests/ui/root_cross_context.stderr b/oscars/examples/api_prototype/tests/ui/root_cross_context.stderr new file mode 100644 index 0000000..48d10a3 --- /dev/null +++ b/oscars/examples/api_prototype/tests/ui/root_cross_context.stderr @@ -0,0 +1,18 @@ +error: lifetime may not live long enough + --> examples/api_prototype/tests/ui/root_cross_context.rs:47:41 + | +45 | with_gc(|ctx1| { + | ---- lifetime `'2` appears in the type of `ctx1` +46 | with_gc(|ctx2| { + | ---- has type `GcContext<'1>` +47 | 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 + --> examples/api_prototype/tests/ui/root_cross_context.rs:47:41 + | +45 | with_gc(|ctx1| { + | ---- has type `GcContext<'1>` +46 | with_gc(|ctx2| { +47 | let root = ctx1.mutate(|cx| cx.root(cx.alloc(123i32))); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'static` diff --git a/oscars/examples/api_prototype/trace.rs b/oscars/examples/api_prototype/trace.rs index b2d32e6..3267e9f 100644 --- a/oscars/examples/api_prototype/trace.rs +++ b/oscars/examples/api_prototype/trace.rs @@ -22,6 +22,11 @@ impl Trace for i32 { } impl Finalize for i32 {} +impl Trace for u64 { + fn trace(&mut self, _: &mut Tracer) {} +} +impl Finalize for u64 {} + impl Trace for String { fn trace(&mut self, _: &mut Tracer) {} } diff --git a/oscars/examples/api_prototype/weak.rs b/oscars/examples/api_prototype/weak.rs index e8e3f47..4653067 100644 --- a/oscars/examples/api_prototype/weak.rs +++ b/oscars/examples/api_prototype/weak.rs @@ -3,12 +3,27 @@ use crate::trace::{Finalize, Trace, Tracer}; use core::marker::PhantomData; use core::ptr::NonNull; -pub struct WeakGc { +/// A weak GC pointer that does not prevent collection. +/// +/// The invariant `'id` brand guarantees it cannot be upgraded in a different context +/// or escape the [`with_gc`][crate::gc::with_gc] closure that created it. +pub struct WeakGc<'id, T: Trace + ?Sized> { pub(crate) ptr: NonNull>, + /// A strict `'id` marker that stops the compiler from secretly altering + /// lifetimes to bypass context checks. + pub(crate) _marker: PhantomData<*mut &'id ()>, } -impl WeakGc { - pub fn upgrade<'gc>(&self, _cx: &MutationContext<'gc>) -> Option> { +impl<'id, T: Trace + ?Sized> WeakGc<'id, T> { + /// Attempts to upgrade to a `Gc<'gc, T>`, returning `None` if collected. + /// + /// Requires `&MutationContext` to statically verify the correct realm and pool liveness. + pub fn upgrade<'gc>(&self, _cx: &MutationContext<'id, 'gc>) -> Option> { + // SAFETY: + // - Passing `_cx` guarantees the main GC context is still alive, + // meaning the memory pool holding this object hasn't been freed yet. + // - We only read the `marked` flag. Since it uses `Cell`, we can safely + // check it without breaking memory aliasing rules. unsafe { let marked = (*self.ptr.as_ptr()).marked.get(); if marked { @@ -23,9 +38,12 @@ impl WeakGc { } } -impl Clone for WeakGc { +impl<'id, T: Trace + ?Sized> Clone for WeakGc<'id, T> { fn clone(&self) -> Self { - Self { ptr: self.ptr } + Self { + ptr: self.ptr, + _marker: PhantomData, + } } }