module-loader: narrow TLA re-entrancy skip to dynamic-import initiator

[robobun]

Fix for oven-sh/bun#30651.

What

The post-WebKit#30259 re-entrancy skip in innerModuleEvaluation's 11.c.v fires on asyncEvaluationOrder < asyncOrderWatermark, which is true whenever the TLA dep first suspended in a prior Evaluate() pass. An independent dynamic import() of a module that transitively depends on that still-suspended TLA gets a fresh watermark, so the second Evaluate() ends up skipping the spec wait and running against the dep's post-await TDZ bindings:

ReferenceError: Cannot access 'foo' before initialization.

Same TDZ hole as WebKit#30259 (fix for oven-sh/bun#30259), but reached through two separate dynamic imports rather than static siblings inside one Evaluate().

Minimal reproducer

// driver.mjs
const p1 = import("./entry1.mjs");
await new Promise(r => setTimeout(r, 10));
const p2 = import("./entry2.mjs");
await Promise.all([p1, p2]);

// entry1.mjs + entry2.mjs each
import { foo } from "./tla.mjs";
console.log(foo);

// tla.mjs
await new Promise(r => setTimeout(r, 100));
export const foo = 123;

On canary, entry2 throws ReferenceError: Cannot access 'foo' before initialization. Node runs it correctly. With this PR + the matching bun change, bun runs it correctly too.

Why the watermark alone is insufficient

    depWasAlreadyEvaluatingAsync     order < asyncOrderWatermark     pendingAsyncDependencies == 0
    deadlock (Nitro):  ✓             ✓                               ✓     → skip needed (old behaviour)
    bug   (parallel):  ✓             ✓                               ✓     → spec wait needed

All three existing conditions fire identically in both scenarios because the discriminator was designed for same-Evaluate() re-entrancy, not cross-Evaluate() re-entrancy.

Fix

Add a fourth discriminator: dep == dynamic-import-initiator. The initiator is the CyclicModuleRecord whose JS body is awaiting the import's result. In the Nitro self-deadlock, the target statically re-imports the initiator → match → keep skipping. In an unrelated parallel dynamic import, dep and initiator are different modules → no match → spec wait fires, and the importer correctly blocks until the dep settles.

Plumbing:

1. JSModuleLoader::requestImportModule resolves the referrer URL to a CyclicModuleRecord via the module registry.
2. loadModule (dynamic overload) stashes it on the freshly created ModuleLoadingContext.
3. moduleLoadTopSettled copies it from the context onto the ModuleLoaderPayload that outlives the context.
4. dynamicImportLoadSettled pushes it onto VM::m_modulesAwaitingDynamicImport (a HashCountedSet) immediately before the target's evaluate() call, and pops after.
5. AbstractModuleRecord::innerModuleEvaluation 11.c.v reads vm.isModuleAwaitingDynamicImport(cyclic) and only skips the spec wait when all four conditions hold.

All new state and logic is gated #if USE(BUN_JSC_ADDITIONS); non-Bun builds are unchanged.

Bun-side PR (including the referrer plumbing in ZigGlobalObject::moduleLoaderImportModule and the WEBKIT_VERSION bump once this PR lands) to follow.

[coderabbitai]

Walkthrough

Threads a dynamic-import initiator CyclicModuleRecord through loader/context/payload, records initiators in VM tracking, brackets evaluation with VM push/pop during dynamic-import settlement, and narrows the top-level-await self-skip to only apply when VM indicates the dependency is awaiting a dynamic import.

Changes

Dynamic Import Initiator Tracking

Layer / File(s)	Summary
VM tracking infrastructure for awaiting dynamic imports Source/JavaScriptCore/runtime/VM.h, Source/JavaScriptCore/runtime/VM.cpp	VM gains a HashCountedSet<CyclicModuleRecord*> (m_modulesAwaitingDynamicImport) and APIs: pushDynamicImportInitiator, popDynamicImportInitiator, isModuleAwaitingDynamicImport, and hasPendingDynamicImport for re-entrancy-aware tracking.
Loading context and payload initiator storage Source/JavaScriptCore/runtime/ModuleLoadingContext.h, Source/JavaScriptCore/runtime/ModuleLoadingContext.cpp, Source/JavaScriptCore/runtime/ModuleLoaderPayload.h, Source/JavaScriptCore/runtime/ModuleLoaderPayload.cpp	ModuleLoadingContext and ModuleLoaderPayload add WriteBarrier<CyclicModuleRecord> fields and accessor/mutator methods plus GC visitor updates to carry the initiator from loading through completion.
Module loader API threading Source/JavaScriptCore/runtime/JSModuleLoader.h, Source/JavaScriptCore/runtime/JSModuleLoader.cpp	loadModule gains an optional CyclicModuleRecord* dynamicImportInitiator parameter; continueDynamicImport is updated to accept ModuleLoaderPayload* (bun) instead of JSPromise*. Implementation resolves initiator from referrer during dynamic imports, stores it in the loading context, and routes the payload through completion handlers and microtasks.
Microtask execution with initiator tracking Source/JavaScriptCore/runtime/JSMicrotask.cpp	moduleLoadTopSettled copies the initiator from loading context to payload; dynamicImportLoadSettled extracts the initiator from the payload and brackets module->evaluate(globalObject) with VM pushDynamicImportInitiator/popDynamicImportInitiator to affect nested-dynamic-import behavior.
TLA deadlock detection refinement Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp	Refines the Bun-specific top-level await deadlock skip condition to check vm.hasPendingDynamicImport() and vm.isModuleAwaitingDynamicImport(cyclic), applying the self-skip only when the dependent is suspended at a dynamic import; otherwise follow the spec path that records the async parent and increments pending async dependencies.

• Possibly related issues:
  • bun test --isolate fails on dynamic import() of a top-level-await module (regression in 1.3.14) bun#31410: PR adds dynamic-import-initiator tracking and narrows the cyclic self-skip to the initiator-specific dynamic-import suspension, addressing the dynamic-import/TLA re-entrancy described in that issue.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is well-structured with clear sections explaining the bug, reproducer, root cause, and fix. However, it does not follow the WebKit template format (no Bugzilla link, no reviewer line, no file list).	Add a Bugzilla link at the top, include 'Reviewed by' line, and add a bullet-point list of modified files matching the WebKit PR template format.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main change: narrowing the TLA re-entrancy skip logic to specifically target dynamic-import initiators, addressing the core fix.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp`:
- Around line 1220-1240: The four-way condition combining
depWasAlreadyEvaluatingAsync, cyclic->asyncEvaluationOrder().order() <
asyncOrderWatermark, cyclic->pendingAsyncDependencies().value_or(1), and
vm.isModuleAwaitingDynamicImport(cyclic) is fragile and hard to reason about;
extract this predicate into a clearly named helper (e.g., shouldSkipTLA(cyclic,
asyncOrderWatermark, vm)) that returns the boolean and replace the inline
expression with that call, and add focused integration tests covering the
cross-Evaluate dynamic-import case (`#30651`) and the Nitro self-deadlock scenario
to assert the branch taken in both situations (use depWasAlreadyEvaluatingAsync,
asyncEvaluationOrder().order(), pendingAsyncDependencies(), and
vm.isModuleAwaitingDynamicImport(cyclic) states to construct the test cases).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0094871d-1929-480e-a6ba-f70504b8c879

📥 Commits

Reviewing files that changed from the base of the PR and between 5488984 and 6f24f5f7bcac2abec37a0030bbda17d75c197f66.

📒 Files selected for processing (10)

• Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp
• Source/JavaScriptCore/runtime/JSMicrotask.cpp
• Source/JavaScriptCore/runtime/JSModuleLoader.cpp
• Source/JavaScriptCore/runtime/JSModuleLoader.h
• Source/JavaScriptCore/runtime/ModuleLoaderPayload.cpp
• Source/JavaScriptCore/runtime/ModuleLoaderPayload.h
• Source/JavaScriptCore/runtime/ModuleLoadingContext.cpp
• Source/JavaScriptCore/runtime/ModuleLoadingContext.h
• Source/JavaScriptCore/runtime/VM.cpp
• Source/JavaScriptCore/runtime/VM.h

[github-actions]

Preview Builds

Commit	Release	Date
7dea873b	autobuild-preview-pr-230-7dea873b	2026-06-17 00:01:09 UTC
c7f29140	autobuild-preview-pr-230-c7f29140	2026-06-03 02:27:13 UTC
d8ec41d5	autobuild-preview-pr-230-d8ec41d5	2026-05-26 15:55:07 UTC
36cc1283	autobuild-preview-pr-230-36cc1283	2026-05-25 11:19:01 UTC
783ba19a	autobuild-preview-pr-230-783ba19a	2026-05-18 06:20:48 UTC
298067bc	autobuild-preview-pr-230-298067bc	2026-05-14 03:43:08 UTC
6f24f5f7	autobuild-preview-pr-230-6f24f5f7	2026-05-14 02:58:36 UTC

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Source/JavaScriptCore/runtime/VM.cpp`:
- Around line 2097-2112: Add VM-thread/API-lock assertions and a membership
check to harden dynamic-import helpers: in VM::pushDynamicImportInitiator,
VM::popDynamicImportInitiator and VM::isModuleAwaitingDynamicImport assert that
the VM/API lock (the VM thread ownership) is held before touching
m_modulesAwaitingDynamicImport; additionally, in VM::popDynamicImportInitiator
assert that m_modulesAwaitingDynamicImport.contains(module) is true before
calling remove so an unmatched pop immediately fails rather than silently no-op.
Use the project’s existing VM/API lock assertion macro or helper when adding
these checks.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a2e5ed28-b367-4f77-803a-1c8648f995cb

📥 Commits

Reviewing files that changed from the base of the PR and between 6f24f5f7bcac2abec37a0030bbda17d75c197f66 and c7f2914.

📒 Files selected for processing (10)

• Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp
• Source/JavaScriptCore/runtime/JSMicrotask.cpp
• Source/JavaScriptCore/runtime/JSModuleLoader.cpp
• Source/JavaScriptCore/runtime/JSModuleLoader.h
• Source/JavaScriptCore/runtime/ModuleLoaderPayload.cpp
• Source/JavaScriptCore/runtime/ModuleLoaderPayload.h
• Source/JavaScriptCore/runtime/ModuleLoadingContext.cpp
• Source/JavaScriptCore/runtime/ModuleLoadingContext.h
• Source/JavaScriptCore/runtime/VM.cpp
• Source/JavaScriptCore/runtime/VM.h

[coderabbitai]

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Assert VM-thread ownership and balanced push/pop here.

These helpers drive the re-entrancy exception, but they currently fail open: cross-thread access to m_modulesAwaitingDynamicImport is unchecked, and an unmatched popDynamicImportInitiator() silently removes nothing. Add API-lock assertions to all three helpers and assert membership before removing so a mis-bracketed caller trips immediately instead of corrupting the skip state.

Proposed hardening

 `#if` USE(BUN_JSC_ADDITIONS)
 void VM::pushDynamicImportInitiator(CyclicModuleRecord* module)
 {
+    ASSERT(currentThreadIsHoldingAPILock());
     if (module)
         m_modulesAwaitingDynamicImport.add(module);
 }
 
 void VM::popDynamicImportInitiator(CyclicModuleRecord* module)
 {
-    if (module)
+    ASSERT(currentThreadIsHoldingAPILock());
+    if (module) {
+        ASSERT(m_modulesAwaitingDynamicImport.contains(module));
         m_modulesAwaitingDynamicImport.remove(module);
+    }
 }
 
 bool VM::isModuleAwaitingDynamicImport(CyclicModuleRecord* module) const
 {
+    ASSERT(currentThreadIsHoldingAPILock());
     return module && m_modulesAwaitingDynamicImport.contains(module);
 }
 `#endif`

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	void VM::pushDynamicImportInitiator(CyclicModuleRecord* module)
	{
	if (module)
	m_modulesAwaitingDynamicImport.add(module);
	}
	void VM::popDynamicImportInitiator(CyclicModuleRecord* module)
	{
	if (module)
	m_modulesAwaitingDynamicImport.remove(module);
	}
	bool VM::isModuleAwaitingDynamicImport(CyclicModuleRecord* module) const
	{
	return module && m_modulesAwaitingDynamicImport.contains(module);
	}
	`#if` USE(BUN_JSC_ADDITIONS)
	void VM::pushDynamicImportInitiator(CyclicModuleRecord* module)
	{
	ASSERT(currentThreadIsHoldingAPILock());
	if (module)
	m_modulesAwaitingDynamicImport.add(module);
	}
	void VM::popDynamicImportInitiator(CyclicModuleRecord* module)
	{
	ASSERT(currentThreadIsHoldingAPILock());
	if (module) {
	ASSERT(m_modulesAwaitingDynamicImport.contains(module));
	m_modulesAwaitingDynamicImport.remove(module);
	}
	}
	bool VM::isModuleAwaitingDynamicImport(CyclicModuleRecord* module) const
	{
	ASSERT(currentThreadIsHoldingAPILock());
	return module && m_modulesAwaitingDynamicImport.contains(module);
	}
	`#endif`

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/runtime/VM.cpp` around lines 2097 - 2112, Add
VM-thread/API-lock assertions and a membership check to harden dynamic-import
helpers: in VM::pushDynamicImportInitiator, VM::popDynamicImportInitiator and
VM::isModuleAwaitingDynamicImport assert that the VM/API lock (the VM thread
ownership) is held before touching m_modulesAwaitingDynamicImport; additionally,
in VM::popDynamicImportInitiator assert that
m_modulesAwaitingDynamicImport.contains(module) is true before calling remove so
an unmatched pop immediately fails rather than silently no-op. Use the project’s
existing VM/API lock assertion macro or helper when adding these checks.

[robobun]

Superseded by the claude/tla-referrer-async-order WebKit change (which supersedes #228) and its Bun-side half oven-sh/bun#32437 (merged). That approach threads the referrer's asyncEvaluationOrder via JSModuleLoader::asyncEvaluationOrderForKey() instead of this PR's referrer-identifier + VM-level m_modulesAwaitingDynamicImport set.

Verified the WebKit#30651 / WebKit#30634 repro is fixed on current bun main. Closing this and the Bun-side oven-sh/bun#30656 as superseded.