Skip to content

Narrow TLA-reentry skip to actual re-entry, not sibling static imports#216

Closed
robobun wants to merge 1 commit into
oven-sh:mainfrom
robobun:farm/269a9746/fix-tla-sibling-import
Closed

Narrow TLA-reentry skip to actual re-entry, not sibling static imports#216
robobun wants to merge 1 commit into
oven-sh:mainfrom
robobun:farm/269a9746/fix-tla-sibling-import

Conversation

@robobun

@robobun robobun commented May 5, 2026

Copy link
Copy Markdown
Collaborator

Problem

innerModuleEvaluation's Bun-specific skip for the spec-mandated async-dependency wait (11.c.v) was too broad: when a TLA leaf module (no async deps of its own, pendingAsyncDependencies always 0) is imported by a sibling in the same Evaluate() pass, the sibling skipped the wait and executed before the TLA body had progressed past its first await. Any binding declared after that await was still in TDZ, giving Cannot access X before initialization on the sibling's use.

Reproduction (oven-sh/bun#30259):

// root.ts
import { foo } from "./await.ts";
import "./child.ts";
void foo;

// await.ts
await 0;
export const foo = 123;

// child.ts
import { foo } from "./await.ts";
console.log(foo);

Fix

The skip was designed for the Nitro-style require(esm) re-entry where a TLA module's body is synchronously re-entered from within its own continuation. Additionally require that no entry in the cycle root's asyncParentModules is on the current DFS stack. When an ancestor is on stack, we're in the same Evaluate() pass that popped this SCC to EvaluatingAsync and the spec wait is mandatory.

This keeps the existing behaviour for the dynamic-import re-entry case (Nitro) while restoring spec-compliant waiting for sibling static imports.

Companion PR

oven-sh/bun# — bumps WEBKIT_VERSION once this is merged and autobuild publishes.

Fixes oven-sh/bun#30259.

innerModuleEvaluation's Bun-specific skip for the spec-mandated
async-dependency wait was too broad: when a TLA leaf module (no async
deps of its own, pendingAsyncDependencies always 0) is imported by a
sibling in the same Evaluate() pass, the sibling skipped the wait and
executed before the TLA body had progressed past its first await. Any
binding declared after that await was still in TDZ, giving
'Cannot access X before initialization' on the sibling's use.

The skip was designed for the Nitro-style require(esm) re-entry where
a TLA module's body is synchronously re-entered from within its own
continuation. Fix: additionally require that no entry in the cycle
root's asyncParentModules is on the current DFS stack. When an
ancestor is on stack, we're in the same Evaluate() pass that popped
this SCC to EvaluatingAsync and the spec wait is mandatory.

Fixes oven-sh/bun#30259.
@coderabbitai

coderabbitai Bot commented May 5, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 94fe77f6-9e8c-4552-909e-129b27354db9

📥 Commits

Reviewing files that changed from the base of the PR and between d5ed1db and 5882d99.

📒 Files selected for processing (1)
  • Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp

Walkthrough

Updated the top-level await dependency resolution logic in innerModuleEvaluation to check whether any async parent module is already on the current DFS stack. This prevents incorrect skipping of the spec-defined PendingAsyncDependencies increment, fixing a regression where subsequent imports of modules with top-level await would fail with ReferenceError.

Changes

TLA Dependency Skip Condition

Layer / File(s) Summary
Core Logic Fix
Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp
The "skip wait" condition in innerModuleEvaluation now additionally checks whether any record in cyclic->asyncParentModules() is present on the current DFS stack. If an async parent is on-stack, the skip is not performed, ensuring module.[[PendingAsyncDependencies]] is properly incremented and the async parent linkage is appended, fixing incorrect ReferenceError on repeated imports of modules with top-level await.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: narrowing a TLA-reentry skip condition to exclude sibling static imports, matching the core modification in the code.
Description check ✅ Passed The description provides clear problem statement, fix explanation, and reproduction case, though it lacks explicit Bugzilla reference and WebKit PR template formatting.
Linked Issues check ✅ Passed The code changes directly address issue #30259 by modifying the TLA-reentry skip logic to check asyncParentModules on the DFS stack, preventing premature execution before await bindings are initialized.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the TLA-reentry skip logic in innerModuleEvaluation, directly addressing the linked issue without unrelated modifications.

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


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

@robobun

robobun commented May 5, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as duplicate of #215 — same diagnosis of the root cause. sosukesuzuki's watermark-based fix is cleaner (uses the existing moduleAsyncEvaluationCount counter instead of walking asyncParentModules against the stack) and already has a Bun-side PR at oven-sh/bun#30262 with the WebKit version bumped to the preview autobuild. Happy to help review if useful.

@robobun robobun closed this May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

regression: when importing a module with top-level await multiple times, all but the first import throw ReferenceError

1 participant