fix(gc): pinned objects are marked, traced and scanned as roots (use-after-free in cross-thread promises)

[proggeramlug]

Fixes a use-after-free: the marker never marked or traced pinned objects, so anything reachable only through a pinned object got freed.

The bug

Every mark entry treated GC_FLAG_PINNED as "already marked" and returned without tracing. That covered try_mark_value, try_mark_raw_root_addr, mark_field_into_worklist, try_mark_young_user_ptr_as_seed, the root scanners, the copying minor's malloc/long-lived mark, the incremental mark barrier, and the budgeted cycle's block persistence.

• js_promise_new_cross_thread (promise/then.rs: gc_malloc + pin) loses its then/await reaction closure after one full collection, and the next allocation reuses the memory. Callers: bcrypt, sharp, container compose_ffi, the worker_threads shim, turnloop_client, thread spawn.
• The Eden-pinned async_bridge promise (fetch/zlib/ws) survived only because the full trace force-marked recent blocks that held a pinned header.

The fix

• A pin means only "don't move, don't sweep." Pinned objects are marked and traced like any other; every PINNED early-out is gone.
• Pinned objects are roots, found without a side table:
  • Arena objects: the header bit plus a per-block pinned_summary bit, set only by pin_object / pin_object_non_young (the gc_pin_sites gate enforces this).
  • gc_malloc objects: a per-thread flag over the malloc list the GC already walks.
  • Tenured non-young pins: they set a tiny thread-local flag. The next full collection places them in their block summary. This keeps the non-young pin path as light as fix(gc): pin long-lived and malloc-resident objects without the space classifier (#7650) #7655 made it (the perf(gc): stop traversing the young graph twice — skip the copying minor's eligibility preflight when its answer is already known (#7645) #7650 macOS link issue).
  • Leaf pins (strings, buffers) aren't flagged, so the cache pins cost nothing.
• The crutch is removed: the "force-mark recent blocks with a pinned header" code is gone.
• One shared walk: the arena block header walk is shared (arena/walk.rs::for_each_block_header). The address-classification gate passes on code alone, with no allowlist entry.

Tests

gc/tests/pinned_roots.rs has 18 tests. They cover 4 parent births × {full, copying minor, forced evacuation}, pinned and unpinned, plus the cross-thread promise then callback and the aged async_bridge promise across 4 full collections, and a full-mark trace-through test for every non-young pin. Sabotaging either half of the fix turns them red.

Verification (Linux x86_64, vs main)

• runtime 4775/0 (serial); codegen 2326/0
• gc-root-dominance: 40/40 seeded, stale 0 (unchanged)
• gc_call_effects --check identical; gc_pin_sites, root_holders, address classification, file size and fmt pass
• all 18 perry-ext crates build and link
• parity: gc, promise, worker, bcrypt and thread show the same fail sets as base (worker 8/0, bcrypt 3/0)
• tsc instructions +0.76%: the pinned-block scan is 0.00% in a profile; the rest comes with 82 vs 80 full collections at a slightly lower live size (a GC-pacing shift, tracked separately). Zod +0.17%.

Summary by CodeRabbit

• Bug Fixes
  • Pinned objects now remain stationary without being incorrectly treated as already marked during garbage collection. Their reachable children are traced across full, minor, and forced-evacuation collections, preventing those children from being freed prematurely.
  • Pinned objects and their reachable promise reactions are preserved across repeated collections.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 943eac4f-6937-4c05-9b25-dd71d39a42ef

📥 Commits

Reviewing files that changed from the base of the PR and between c968266 and bf0731e.

📒 Files selected for processing (2)

• crates/perry-runtime/src/gc/mod.rs
• scripts/gc_runtime_root_holders.json

 ______________________________________________________________________________________________________________________________________
< DRY - Don't Repeat Yourself. Every piece of knowledge must have a single, unambiguous, authoritative representation within a system. >
 --------------------------------------------------------------------------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: d7334255-485c-4b8f-8eb4-4b5e853c09c8

📥 Commits

Reviewing files that changed from the base of the PR and between 989c551 and c968266.

📒 Files selected for processing (17)

• changelog.d/11664-gc-pinned-objects-are-roots.md
• crates/perry-runtime/src/arena/block.rs
• crates/perry-runtime/src/arena/mod.rs
• crates/perry-runtime/src/arena/pinned.rs
• crates/perry-runtime/src/arena/promote.rs
• crates/perry-runtime/src/arena/quarantine.rs
• crates/perry-runtime/src/arena/walk.rs
• crates/perry-runtime/src/gc/barrier/mod.rs
• crates/perry-runtime/src/gc/copying.rs
• crates/perry-runtime/src/gc/cycle.rs
• crates/perry-runtime/src/gc/mod.rs
• crates/perry-runtime/src/gc/pin.rs
• crates/perry-runtime/src/gc/roots.rs
• crates/perry-runtime/src/gc/tests/mod.rs
• crates/perry-runtime/src/gc/tests/pinned_roots.rs
• crates/perry-runtime/src/gc/trace.rs
• scripts/gc_runtime_root_holders.json

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

The runtime now discovers non-leaf pinned objects as roots and traces their children during garbage collection. Arena summaries and malloc-registry summaries support pinned-object discovery. Marking paths and block-persistence scans no longer treat pinned objects as marked by default.

Changes

Pinned GC roots

Layer / File(s)	Summary
Arena summaries and header traversal crates/perry-runtime/src/arena/block.rs, crates/perry-runtime/src/arena/mod.rs, crates/perry-runtime/src/arena/pinned.rs, crates/perry-runtime/src/arena/promote.rs, crates/perry-runtime/src/arena/quarantine.rs, crates/perry-runtime/src/arena/walk.rs	Arena blocks track whether they may contain pinned headers. Header traversal is shared by arena walkers, and pinned-header collection scans summarized blocks and refreshes their summaries.
Pinned root recording and scanning crates/perry-runtime/src/gc/pin.rs, crates/perry-runtime/src/gc/mod.rs	Pin setters record non-leaf pinned objects. GC initialization registers a scanner that collects pinned arena and malloc objects and visits them during marking and fix-up passes.
Tracing semantics and survival validation crates/perry-runtime/src/gc/barrier/mod.rs, crates/perry-runtime/src/gc/copying.rs, crates/perry-runtime/src/gc/cycle.rs, crates/perry-runtime/src/gc/roots.rs, crates/perry-runtime/src/gc/trace.rs, crates/perry-runtime/src/gc/tests/*, crates/perry-runtime/src/gc/tests/mod.rs, scripts/gc_runtime_root_holders.json, changelog.d/11664-gc-pinned-objects-are-roots.md	Marking and block-persistence paths use updated pinned-object handling. Tests cover child and promise-reaction survival across collection modes and allocation origins, including sabotage cases for the prior behaviors.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant PinSetter
  participant ArenaSummary
  participant GCInitializer
  participant PinnedRootScanner
  participant MarkingVisitor
  participant ChildObject
  PinSetter->>ArenaSummary: record pinned header
  GCInitializer->>PinnedRootScanner: register root scanner
  PinnedRootScanner->>ArenaSummary: collect pinned headers
  PinnedRootScanner->>MarkingVisitor: visit pinned object
  MarkingVisitor->>ChildObject: trace child reference

Loading

Suggested reviewers: claude

Merge Risk: ⚪ Minimal · up to c9682

The inspected cross-thread promise paths retain their GC roots. No actionable issue is established that would prevent merging after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to c9682

The change addresses a use-after-free affecting objects reachable through pinned promises. The ordinary collection path is supported by the reviewed code and tests, but it remains unclear whether a pin made late in an incremental collection is traced before its children can be reclaimed.

Retained concerns

• Medium · security · inferred: A pin installed after a budgeted collection’s final root remark may not be traced in that collection: pin registration records a future root, but does not visibly shade the object for the active cycle. Reachability and regression against the former fallback remain unproven.

Security review details

Security Blast Radius

• inferred — A missed trace can affect objects reachable only through a pinned parent in the collecting runtime thread, including promise reactions; the reviewed paths do not establish cross-thread heap ownership transfer.

Security Findings and Attack Paths

• inferred — The unresolved attack-relevant transition is a newly effective pin after the final root remark but before sweep. No reachable caller or resulting use-after-free in that window was established.

Trust Boundaries and Controls

• observed — Root-scanner registration and the malloc-object registry are thread-local. The reviewed cross-thread Promise path keeps allocation and pin registration with the owning thread.

Resilience and Maintainability Implications

• observed — The active mark barrier is kept through the finalize-to-sweep gap and its seeds are drained before the sweep snapshot; the inspected pin setters publish summaries rather than invoking that barrier.

Hardening Proposals

• proposed — Establish whether pins can become newly effective after final remark; if they can, ensure that transition shades the pinned object and traces its children before sweep.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 61.02% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 59 functions across 15 files. (2 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the pinned-object GC fix and the associated use-after-free in cross-thread promises. It is specific and related to the primary change.
Description check	✅ Passed	The description provides a detailed summary, concrete changes, root cause, test coverage, and verification results. It does not use the template headings or include the explicit checklist and related-…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 61.02% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 59 functions across 15 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}