[dylan-conway]

What does this PR do?

Fixes a segfault in WTF::Vector::takeLast under JSFinalizationRegistry::takeDeadHoldingsValue ← runFinalizationCleanup ← Bun__runDeferredWork (Sentry BUN-4SGV). It happens with node:vm contexts and with bun test --isolate / --parallel, on every platform.

Cause

• A DeferredWorkTimer ticket's cells (the registry, a waitAsync promise, a wasm promise) are marked only by their realm, so the sweep frees them once the realm is dead.
• JSC cancels a dead realm's tickets at the end of each collection, in DeferredWorkTimer::cancelPendingWork(VM&). It walks m_pendingTickets, which is empty in Bun because onAddPendingWork takes each ticket into JSCTaskScheduler's maps.
• ~JSGlobalObject cancels them too, but a lazily swept global is destroyed later. A cleanup job that was already queued runs in between, on a registry that has been destroyed.

Fix

• JSVMClientData implements VM::ClientData::reconcileWeakReferencesAtGCEnd, the embedder hook JSC calls after marking and before sweeping.
• There, JSCTaskScheduler::cancelWorkOfDeadRealms finds the unmarked realms that still have pending tickets and calls JSGlobalObject::clearWeakTickets() on each. This is the fixing line. It is what ~JSGlobalObject calls: it unregisters the realm's Atomics.waitAsync waiters, then cancels its tickets through the existing onCancelPendingWork hook, which drops the map entry and releases the event loop ref.
• JSCDeferredWorkTask stores its JSVMClientData instead of reaching the VM through ticket->scriptExecutionOwner(), which is a freed cell for exactly these jobs. The null check in Bun__deleteDeferredWorkTask goes with it: ~VM deletes clientData last and never nulls it.

A second effect: Atomics.notify no longer counts or wakes the Atomics.waitAsync of a dead context.

No WebKit change. Supersedes #39994.

How did you verify your code works?

Two new tests in test/js/node/vm/vm.test.ts. Each also keeps one context alive and requires its callback to still run.

	1.4.2	fixed (debug + ASAN, Linux x64)
a FinalizationRegistry cleanup job is dropped when its context dies before the job runs	segfault (exit 139)	pass
Atomics.notify does not wake the Atomics.waitAsync of a context that died	woken: 5	woken: 1, pass

The first test also segfaults on canary 11c41c645. The FinalizationRegistry tests in test/cli/test/isolation.test.ts pass with the change.

[coderabbitai]

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 381d2626-b6a7-48c8-9196-13feb612285c

📥 Commits

Reviewing files that changed from the base of the PR and between 232cb41 and c549c32.

📒 Files selected for processing (1)

• test/js/node/vm/vm.test.ts

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

---

Walkthrough

The change adds end-of-GC cancellation of deferred work for unmarked realms. Deferred-work jobs now retain JSVMClientData for VM and scheduler access. Two subprocess tests cover FinalizationRegistry cleanup and Atomics.waitAsync behavior across VM contexts.

Changes

Dead-realm deferred work

Layer / File(s)	Summary
GC hook and dead-realm ticket cancellation src/jsc/bindings/BunClientData.h, src/jsc/bindings/BunClientData.cpp, src/jsc/bindings/JSCTaskScheduler.h, src/jsc/bindings/JSCTaskScheduler.cpp	JSVMClientData delegates end-of-GC reconciliation to JSCTaskScheduler. The scheduler checks pending tickets against VM heap marks and clears weak tickets for unmarked target realms.
Deferred-job lifecycle and regression tests src/jsc/bindings/JSCTaskScheduler.cpp, test/js/node/vm/vm.test.ts	Deferred-work jobs retain JSVMClientData for VM and scheduler access. Execution and shutdown cleanup use the stored client data. Subprocess tests cover FinalizationRegistry cleanup and Atomics.waitAsync across collected VM contexts.

Suggested reviewers: robobun

Priority: ⬆️ High

Merge Risk: ⚪ Minimal · up to c549c

The reviewed source shows dead-realm cancellation is serialized with event-loop execution, and the added regression tests cover live and dead VM contexts. No actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description includes both required sections. It clearly explains the crash, cause, fix, and verification results.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: cancelling deferred work for dead realms after garbage collection identifies them.
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.

---

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

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline test nits, I also looked at whether vm.heap.isMarked(realm) in cancelWorkOfDeadRealms could report a live old-generation realm as dead during an eden collection (it mirrors what upstream DeferredWorkTimer::cancelPendingWork(VM&) asks at the same GC-end boundary), whether the hook can race the mutator when the collector is concurrent (the end phase runs with the world stopped, and clearWeakTickets runs no JS), and whether the now-unconditional job->clientData dereference in Bun__deleteDeferredWorkTask can outlive JSVMClientData (~VM deletes it last, after the handle has drained or refused queued jobs). None of those turned into a bug, but since this touches GC-phase and lock-ordering code, a human look is still worthwhile.

Extended reasoning...

The change adds a GC-end hook on JSVMClientData that cancels DeferredWorkTimer tickets of unmarked vm.createContext realms, moves the scheduler lookup for deferred-work jobs from the (possibly freed) ticket owner cell to a stored JSVMClientData pointer, and drops a null check in the delete path; it touches no injection, auth, or data-exposure surface. It does touch GC-phase ordering, m_lock drop-then-reenter, and cross-thread job lifetime, and two verified test-quality findings are posted inline, so this is not a change to approve without a human reading it.

[claude]

I reviewed the updated PR and didn't find any bugs; the follow-up commit addresses both earlier nits (the dead contexts' cleanups are now counted and asserted as 0, and both tests run concurrently). Because the fix hooks the GC end phase and its safety rests on JSC ordering that isn't verifiable from this checkout, a human look from someone with the WebKit tree at hand would still be worthwhile.

What was reviewed:

• Retainer reasoning in cancelWorkOfDeadRealms: a ticket's target is marked only through its realm's visitChildren, so !heap.isMarked(realm) is the right discriminator and target()->globalObject() is only read on unswept cells as long as the hook precedes sweepInFinalize (the eager sweep of lower-tier precise allocations) — that ordering is the one claim I could not confirm here, since the vendored JavaScriptCore sources are not in the checkout.
• clearWeakTickets → onCancelPendingWork → Bun__VmHandle__refKeepAlive from a possibly-collector-thread end phase: the Rust side is documented as any-thread and the mutator is stopped, so no new thread-affinity problem.
• Dropping the null guard in Bun__deleteDeferredWorkTask: the old path already dereferenced the ticket's owner cell, so it assumed a live VM/clientData too; no weakening. The queued job for a cancelled ticket now falls through runPendingWork with wasPending == false and never touches the freed target.

Extended reasoning...

The change adds a JSC::VM::ClientData::reconcileWeakReferencesAtGCEnd override in src/jsc/bindings/BunClientData.{h,cpp} that calls a new JSCTaskScheduler::cancelWorkOfDeadRealms, which scans both pending-ticket maps for tickets whose global object is unmarked and calls JSGlobalObject::clearWeakTickets on each, and it reroutes the deferred-work job's VM/clientData access away from the ticket's possibly-freed owner cell. It touches no auth, injection, or data-exposure surface; the sensitive surface is GC lifetime and cross-thread event-loop keep-alive counting. The bug hunt exited on dry_streak, no third-party objections are recorded, no CODEOWNERS entry covers the changed files, and the follow-up commit addressed both of my earlier inline nits. I did not approve because the correctness hinges on JSC's end-phase ordering (hook before the eager sweep of the first eight precise cells per IsoSubspace) and on clearWeakTickets semantics that I could not verify without the WebKit sources, and a GC-end-phase hook is not a simple or mechanical change.

[robobun]

The two tests this PR added require fullGC() to collect every dropped context. That failed on PR builds 122945 and 122554 with deadContextCleanups: 5. #44544 makes both tests check the cancelled work per collected context.