SynchronousModuleQueue: replay diverted reactions against their own realm

[robobun]

Problem

JSModuleLoader::drainSynchronousModuleQueue runs every captured module-loader reaction with the caller's globalObject. That was fine for the original require(esm) path (one realm), but vm.m_synchronousModuleQueue is VM-wide: when another realm's module loader is entered while the queue is active, its reactions are diverted to the queue and then replayed against the wrong realm's module registry.

Repro

Reproducible on bun main today via require(esm):

// trigger.mjs  (evaluated under loadModuleSync, so the queue is active)
import url from 'url'; import path from 'path';
const mod = url.pathToFileURL(path.join(import.meta.dirname, 'counter.mjs')).href;
const realm = new ShadowRealm();
const p = realm.importValue(mod, 'getCounter');
export { realm, p, mod };

// counter.mjs
let n = 0; export const getCounter = () => n++;

// entry.cjs
const { realm, p, mod } = require('./trigger.mjs');
(async () => {
  console.log((await p)());                                     // 0
  console.log((await realm.importValue(mod, 'getCounter'))());  // expected 1, got 0
})();

The second importValue loads a fresh copy of counter.mjs because the first load was registered in the main realm's module map instead of the ShadowRealm's.

oven-sh/bun#34121 wraps every CommonJS entry in SynchronousModuleQueue, which makes this trip on Node's test/parallel/test-shadow-realm-module.js (a .cjs entry that calls realm.importValue from an immediately-invoked async function).

Cause

The four diversion points in JSPromise.cpp (performPromiseThenWithInternalMicrotask fulfilled/rejected, settleInlineInternalMicrotask, triggerPromiseReactions) all have the reaction's own globalObject in scope (realm() or a parameter, exactly what queueMicrotask would have used) but don't store it on SynchronousModuleTask, so drainSynchronousModuleQueue has no way to recover it and substitutes the caller's.

Fix

Add JSGlobalObject* globalObject to SynchronousModuleTask, populate it at the four diversion points, run against t.globalObject in the drain loop (both the runInternalMicrotask call and the exception-path queueMicrotask), and mark it in VM::visitAggregateImpl alongside the existing arg slots.

Verification

Built bun against this branch locally. Before:

a: 0
b: 0
BUG: module not cached in ShadowRealm

After:

a: 0
b: 1
OK

Also ran test/parallel/test-shadow-realm-module.js, test/js/bun/jsc/shadow.test.js, and the require(esm) sync-queue tests (require-esm-transitive-tla, require-esm-microtask-order, the require-esm-gc-roots body under collectContinuously=1) to confirm the single-realm path and GC visiting still work.

Unblocks oven-sh/bun#34121 for test-shadow-realm-module.js. Related: oven-sh/bun#34115.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 5 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: efd84d71-588b-4d68-84aa-c49bcc6f74e6

📥 Commits

Reviewing files that changed from the base of the PR and between 34c01d1 and 4e9f003.

📒 Files selected for processing (4)

• Source/JavaScriptCore/runtime/JSModuleLoader.cpp
• Source/JavaScriptCore/runtime/JSPromise.cpp
• Source/JavaScriptCore/runtime/VM.cpp
• Source/JavaScriptCore/runtime/VM.h

---

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

[claude]

I didn't find any issues, but this touches cross-realm promise-reaction dispatch and GC rooting in core JSC runtime code, so it's worth a human look.

What was reviewed:

• The four diversion sites in JSPromise.cpp each capture the same globalObject the fallback queueMicrotask would have used (realm() or the caller's param), so replay realm matches non-diverted behavior.
• Struct field order in SynchronousModuleTask matches all four brace-init lists.
• visitor.appendUnbarriered(t.globalObject) resolves to the JSCell* overload (precedent in ScriptExecutable.cpp); t.globalObject is never null at any append site.
• Exception-path re-queue in drainSynchronousModuleQueue also uses the captured realm, so remaining reactions land on the correct global's microtask queue.

Extended reasoning...

Overview

This PR threads a JSGlobalObject* through VM::SynchronousModuleTask so that when drainSynchronousModuleQueue replays diverted module-loader promise reactions, each runs against the realm it was diverted from rather than the drain caller's realm. Changes span four files: the struct definition in VM.h, four brace-initializer updates in JSPromise.cpp (all inside USE(BUN_JSC_ADDITIONS) synchronous-queue branches), the drain loop in JSModuleLoader.cpp (both the runInternalMicrotask call and the exception-path queueMicrotask), and a GC-visit line in VM::visitAggregateImpl.

Security risks

None identified. This is an internal correctness fix for which realm's module registry receives loader reactions. It doesn't touch parsing, auth, or untrusted-input handling; the captured globalObject is always the promise's own realm(), which was already reachable.

Level of scrutiny

High. While the mechanical change is small (~15 lines), it sits at the intersection of promise reaction dispatch, ES module loading, cross-realm (ShadowRealm) semantics, and GC rooting — all subtle areas of JSC's runtime. The Bun-specific SynchronousModuleQueue machinery is itself a delicate deviation from normal microtask ordering. A reviewer familiar with why the queue exists and how it interacts with ShadowRealm's module map should confirm the realm choice is correct in all cases (e.g., that realm() on the settling promise is always the right module-loader owner, not just coincidentally so in the repro).

Other factors

• The four globalObject values captured are exactly what each site would have passed to globalObject->queueMicrotask(...) on the non-diverted path, so behavior converges with the async path by construction.
• appendUnbarriered(JSCell*) exists and has precedent for global-object pointers, so the GC-visit addition compiles and behaves as intended; the author reports running require-esm-gc-roots under collectContinuously=1.
• The struct uses aggregate brace-init at all four sites; field order was verified to match the new layout (task, payload, globalObject, arg0, arg1, arg2).
• The PR description includes a concrete repro plus verification against Node's test-shadow-realm-module.js and existing sync-queue tests, which raises confidence but doesn't substitute for a domain expert's sign-off on cross-realm module-loader semantics.

[claude]

I didn't find any issues, but this touches cross-realm module-loader dispatch and GC root marking in JSC, so it's worth a human look.

Checked: all four m_synchronousModuleQueue->tasks.append sites now pass the same globalObject their fallback queueMicrotask call uses; struct field order matches every aggregate initializer.
Checked: visitor.appendUnbarriered(t.globalObject) binds to the JSCell* overload, consistent with the existing arg marking.
Checked: no other SynchronousModuleTask constructions exist in the tree; the drain loop's now-unused globalObject parameter is harmless.

Extended reasoning...

Overview

This PR fixes a cross-realm bug in Bun's synchronous module queue (require(esm) path). It adds a JSGlobalObject* field to VM::SynchronousModuleTask, populates it at the four diversion points in JSPromise.cpp, uses it in drainSynchronousModuleQueue (both the runInternalMicrotask call and the exception-path re-queue), and marks it in VM::visitAggregateImpl. All changes are under USE(BUN_JSC_ADDITIONS).

Security risks

None identified. The stored globalObject at each diversion point is the exact value the non-diverted path would have passed to queueMicrotask on the next line, so this restores parity rather than introducing new realm-crossing behavior. GC marking of the new field prevents a potential use-after-collect of a ShadowRealm global while its reaction sits in the queue.

Level of scrutiny

High. Although the diff is ~15 lines and mechanically consistent, it changes which realm module-loader reactions execute against and adds a GC root. Cross-realm semantics in JSC's module loader and promise-reaction dispatch are subtle enough that a maintainer familiar with the SynchronousModuleQueue design should confirm the fix, particularly the exception-path re-queue to rest.globalObject.

Other factors

The PR description includes a clear repro, before/after output, and lists the regression tests run (test-shadow-realm-module.js, require-esm-*, GC roots under collectContinuously=1). I verified via grep that there are no other construction sites for SynchronousModuleTask and that appendUnbarriered has a JSCell* overload. The change looks correct, but JSC runtime internals warrant human sign-off.

[github-actions]

Preview Builds

Commit	Release	Date
4e9f0037	autobuild-preview-pr-371-4e9f0037	2026-07-29 05:46:03 UTC