add gc support to Rust owned objects

[anonrig]

Adds gc support to Rust owned objects using the existing GC implementation, rather than using cppgc heap, which the draft implementation used: #5638.

[anonrig]

/bigbonk roast this pr

[anonrig]

/bigbonk roast this pr

[anonrig]

/bigbonk roast this pr

[anonrig]

/bigbonk roast this pr

[ask-bonk]

@anonrig Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[anonrig]

/bigbonk roast this pr

[ask-bonk]

Review Update

Re-reviewing after a thorough read of the full diff. My earlier review (review #3905780686) had several incorrect findings that I want to correct, plus new findings from this deeper pass.

Corrections to prior review

• Issue #1 (was High): unwrap/unwrap_ref null handling — This was already correctly handled. WrappableRc::from_js() at v8.rs:1580 returns Option<Self> and checks handle.get().is_null(). Both unwrap and unwrap_ref propagate None via ?. Apologies for the false alarm.
• Issue #2 (was Medium): Docstring mismatch — Fixed. The docstring and signature now both describe Option<Self>.
• Issue #3 (was Medium): WeakRef::get() safety — No longer applicable. The get() method was removed; the API now uses upgrade() -> Option<Ref<R>> which is sound.
• Issue #5 (was Low): todo!() in proc macro — Fixed. Now uses proper syn::Error at lib.rs:389-393.

New findings below (posted as inline comments)

This review was generated by an AI assistant and may contain inaccuracies.

[ask-bonk]

[MEDIUM] Missing copyright header on new file.

This file is new in this PR but is missing the project copyright/license header. Per the review checklist, every new .rs file must begin with:

// Copyright (c) 2026 Cloudflare, Inc.
// Licensed under the Apache 2.0 license found in the LICENSE file or at:
//     https://opensource.org/licenses/Apache-2.0

Same applies to gc.rs and resource_conversion.rs.

[ask-bonk]

[MEDIUM] resolve_resource performs an unchecked type cast.

trait_ptr.data.cast::<R>() reinterprets the Rc::into_raw pointer as R with no runtime verification that the Wrappable actually holds type R. The C++ side only checks WORKERD_RUST_WRAPPABLE_TAG (confirming "this is a Rust object") but doesn't verify which specific Rust type it holds.

The safety contract ("caller must ensure the type R matches") is upheld by the proc-macro-generated code, but this is a one-line type confusion away from silent memory corruption. Consider storing a TypeId in an additional data slot and adding a debug_assert! check:

debug_assert_eq!(
    stored_type_id,
    TypeId::of::<R>(),
    "type mismatch in resolve_resource"
);

This would catch type confusion bugs during development/testing with zero cost in release builds.

[ask-bonk]

[LOW] as_pin_mut safety comment could be strengthened.

Invariant #3 ("Non-reentrant FFI") is not entirely accurate: wrappable_visit_ref at ffi.c++:454 calls wrappable.visitRef(*gcVisitor, parentMaybe, *ref_strong), which traces child Wrappables, which calls wrappable_invoke_trace, which calls back into Rust's GarbageCollected::trace(). However, the re-entry creates a Pin<&mut Wrappable> for a different Wrappable (the child), not the same one, so the &mut uniqueness invariant holds.

Consider clarifying the comment to say: "C++ methods may re-enter Rust for different Wrappables during tracing, but never create a second Pin<&mut Wrappable> for the same object."

[ask-bonk]

[LOW] Stale API reference.

get() is mentioned here but WeakRef only exposes upgrade() and is_alive() — there is no get() method.

Suggested change

	Weak reference that doesn't prevent GC collection. Use `upgrade()` to get a `Ref<R>` if the resource is still alive, or `get()` for a temporary `&R`.
	Weak reference that doesn't prevent GC collection. Use `upgrade()` to get a `Ref<R>` if the resource is still alive.

[ask-bonk]

Review posted successfully. Here's a summary of what was posted:

Corrections to prior review:

• The "High" severity null-handling issue was already correctly handled — WrappableRc::from_js() returns Option<Self> with a null check, and both unwrap/unwrap_ref propagate None via ?.
• The docstring mismatch, WeakRef::get() safety, and todo!() issues were all already resolved in the current code.

New findings (5 inline comments):

1. [MEDIUM] Missing copyright headers on 3 new files (resource.rs, gc.rs, resource_conversion.rs) — per the project's review checklist, new .rs files need the copyright header.

2. [MEDIUM] resolve_resource unchecked type cast (v8.rs:1709) — trait_ptr.data.cast::<R>() has no runtime type verification. The WORKERD_RUST_WRAPPABLE_TAG check only confirms "this is a Rust object," not which specific Rust type. Suggested adding a debug_assert! with TypeId for development-time safety.

3. [LOW] Fat pointer transmute tracking (v8.rs:1319) — The inline TODO about ptr_metadata stabilization should be turned into a concrete tracking issue for visibility.

4. [LOW] as_pin_mut safety comment clarification (v8.rs:1651) — The "non-reentrant FFI" invariant should clarify that re-entry occurs for different Wrappables during tracing, which is safe.

5. [LOW] Stale get() reference in README (README.md:17) — Mentions a WeakRef::get() method that doesn't exist; should reference upgrade() instead. Included a suggested edit.

github run