[API] Implement missing Gc API in branded collector - #98
Open
shruti2522 wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
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
GcAPIs (ptr_eq,is,cast_unchecked,into_raw/from_raw,AsRef,Default) and renamedGc::gettoinner_ref. - Extended
GcBoxheaders with a stored runtime type descriptor to supportGc::iswithout requiring'static. - Relaxed
PoolPointer::from_rawto supportT: ?Sizedand 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 get → inner_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 get → inner_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
Defaultimpl allocates viaMutationContext::global(), which allows constructingGc<'gc, T>outside aGcContext::mutatewindow. That defeats the whole purpose of the branded'gclifetime and makes it possible to keep aGcalive across acollect()call, reintroducing the use-after-free scenario thattests/uaf.rsis 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
Defaultimpl allocates from a thread-local global collector, which allows producingGc<'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, aGcthat 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.
shruti2522
force-pushed
the
boa_api
branch
2 times, most recently
from
August 5, 2026 23:12
7bae8c1 to
0948ed9
Compare
shruti2522
marked this pull request as draft
August 9, 2026 23:38
shruti2522
force-pushed
the
boa_api
branch
2 times, most recently
from
August 10, 2026 02:22
e1cba1e to
b218a82
Compare
shruti2522
marked this pull request as ready for review
August 10, 2026 02:33
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 Allowed DST keys in Ephemeron methods Used core::ptr instead of std::ptr for no_std compatibility
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Top of the stack (#98 -> #97 -> #96)
ptr_eq,is,cast_unchecked,from_raw,into_raw,AsRefandDefaultforGcinmark_sweep_brandedandnull_collector_brandedtype_nametoGcBoxallocation to support downcasting viaGc::iswithout'staticlifetime boundsGc::getmethod toinner_refto avoid shadowingDereftrait methodsMotivation
These API additions fill the remaining functionality gaps in the branded collector.
Note: This branch (
boa_api) is currently being used as thegitdependency source for the active GC integration effort over atboa-dev/boa(see boa-dev/boa#5460).