Skip to content

[API] Implement missing Gc API in branded collector - #98

Open
shruti2522 wants to merge 5 commits into
boa-dev:mainfrom
shruti2522:boa_api
Open

[API] Implement missing Gc API in branded collector#98
shruti2522 wants to merge 5 commits into
boa-dev:mainfrom
shruti2522:boa_api

Conversation

@shruti2522

@shruti2522 shruti2522 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Top of the stack (#98 -> #97 -> #96)

  • Implemented ptr_eq, is, cast_unchecked, from_raw, into_raw, AsRef and Default for Gc in mark_sweep_branded and null_collector_branded
  • Added type_name to GcBox allocation to support downcasting via Gc::is without 'static lifetime bounds
  • Renamed Gc::get method to inner_ref to avoid shadowing Deref trait methods

Motivation

These API additions fill the remaining functionality gaps in the branded collector.

Note: This branch (boa_api) is currently being used as the git dependency source for the active GC integration effort over at boa-dev/boa (see boa-dev/boa#5460).

Copilot AI lite review requested due to automatic review settings August 5, 2026 22:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fills out the branded collectors’ Gc API surface to better match the unbranded collectors, including pointer utilities and downcasting support, and updates existing tests to use Deref rather than a dedicated get() method.

Changes:

  • Added missing Gc APIs (ptr_eq, is, cast_unchecked, into_raw/from_raw, AsRef, Default) and renamed Gc::get to inner_ref.
  • Extended GcBox headers with a stored runtime type descriptor to support Gc::is without requiring 'static.
  • Relaxed PoolPointer::from_raw to support T: ?Sized and updated branded mark/sweep tests accordingly.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
oscars/src/collectors/null_collector_branded/mutation_ctx.rs Adds a global() accessor for a thread-local collector-backed mutation context.
oscars/src/collectors/null_collector_branded/gc.rs Adds pointer helpers, raw conversions, is, AsRef, and Default; renames getinner_ref.
oscars/src/collectors/null_collector_branded/gc_box.rs Stores a type descriptor in GcBox to support Gc::is.
oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs Adds a global() accessor for a thread-local collector-backed mutation context.
oscars/src/collectors/mark_sweep_branded/gc.rs Adds pointer helpers, raw conversions, is, AsRef, and Default; renames getinner_ref.
oscars/src/collectors/mark_sweep_branded/gc_box.rs Stores a type descriptor in GcBox to support Gc::is.
oscars/src/collectors/mark_sweep_branded/tests/mod.rs Updates tests to use Deref access instead of gc.get().
oscars/src/collectors/mark_sweep_branded/tests/ephemeron.rs Updates tests to use Deref access instead of val.get().
oscars/src/alloc/mempool3/alloc.rs Makes PoolPointer::from_raw accept T: ?Sized to support new branded APIs.
Suppressed comments (2)

oscars/src/collectors/mark_sweep_branded/gc.rs:117

  • This Default impl allocates via MutationContext::global(), which allows constructing Gc<'gc, T> outside a GcContext::mutate window. That defeats the whole purpose of the branded 'gc lifetime and makes it possible to keep a Gc alive across a collect() call, reintroducing the use-after-free scenario that tests/uaf.rs is explicitly designed to prevent.
impl<'gc, T: Trace + Finalize + Default + 'gc> Default for Gc<'gc, T> {
    fn default() -> Self {
        crate::collectors::mark_sweep_branded::MutationContext::global()
            .try_alloc(Default::default())
            .unwrap()

oscars/src/collectors/null_collector_branded/gc.rs:114

  • This Default impl allocates from a thread-local global collector, which allows producing Gc<'gc, T> values that can be treated as long-lived (potentially 'static). Since the underlying collector is thread-local and is dropped at thread exit, a Gc that escapes the thread can become dangling and lead to use-after-free.
impl<'gc, T: Trace + Finalize + Default + 'gc> Default for Gc<'gc, T> {
    fn default() -> Self {
        crate::collectors::null_collector_branded::MutationContext::global()
            .try_alloc(Default::default())
            .unwrap()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread oscars/src/collectors/null_collector_branded/gc.rs
Comment thread oscars/src/collectors/mark_sweep_branded/gc.rs
Comment thread oscars/src/collectors/null_collector_branded/mutation_ctx.rs
Comment thread oscars/src/collectors/mark_sweep_branded/mutation_ctx.rs
@shruti2522
shruti2522 force-pushed the boa_api branch 2 times, most recently from 7bae8c1 to 0948ed9 Compare August 5, 2026 23:12
@shruti2522
shruti2522 marked this pull request as draft August 9, 2026 23:38
@shruti2522
shruti2522 marked this pull request as ready for review August 10, 2026 02:33
@shruti2522 shruti2522 changed the title Implement missing Gc API in branded collector [API] Implement missing Gc API in branded collector Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants