From df660a83e0a3619c7f3a9eb498df63198ec45dab Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Tue, 5 May 2026 08:45:02 +0900 Subject: [PATCH] [JSC] innerModuleEvaluation: don't skip async-dep wait for siblings in same Evaluate() The Bun-specific skip at 11.c.v (avoid self-deadlock when require(esm) / dynamic import re-enters a TLA module from inside its own suspension) previously fired whenever the dep was already EvaluatingAsync with pendingAsyncDependencies == 0. That also matches a sibling static import within the *same* Evaluate() pass when the TLA dep has no async deps of its own: the dep is suspended at its first await and bindings declared after it are still TDZ, so the importer ran too early (oven-sh/bun#30259). Thread an asyncOrderWatermark (the VM's moduleAsyncEvaluationCount at the start of this Evaluate()) through innerModuleEvaluation and only skip when the cycle root's asyncEvaluationOrder predates it, i.e. the dep transitioned to EvaluatingAsync in a *prior* Evaluate(). Siblings that transition during the current DFS keep the spec wait. --- .../runtime/AbstractModuleRecord.cpp | 30 ++++++++++++++----- .../runtime/AbstractModuleRecord.h | 4 +++ .../runtime/CyclicModuleRecord.cpp | 7 +++++ Source/JavaScriptCore/runtime/VM.h | 3 ++ 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp b/Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp index 26d43e678c3a..909b3e346c71 100644 --- a/Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp +++ b/Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp @@ -1075,7 +1075,11 @@ static void checkSafeToRecurse(JSGlobalObject* globalObject, ThrowScope& scope) throwRangeError(globalObject, scope, "Maximum call stack size exceeded"_s); } +#if USE(BUN_JSC_ADDITIONS) +unsigned AbstractModuleRecord::innerModuleEvaluation(JSGlobalObject* globalObject, Vector& stack, unsigned index, int64_t asyncOrderWatermark) +#else unsigned AbstractModuleRecord::innerModuleEvaluation(JSGlobalObject* globalObject, Vector& stack, unsigned index) +#endif { // InnerModuleEvaluation(module, stack, index) // https://tc39.es/ecma262/#sec-innermoduleevaluation @@ -1146,13 +1150,21 @@ unsigned AbstractModuleRecord::innerModuleEvaluation(JSGlobalObject* globalObjec // skip when the cycle root's pendingAsyncDependencies is 0, i.e. its // ExecuteModule/ExecuteAsyncModule has already been called and the // bindings before its first await are initialised. For an SCC still - // queued behind an async dep the root's count is > 0. + // queued behind an async dep the root's count is > 0. That alone is + // still insufficient: a TLA dep with no async deps of its own has + // count 0 yet may have only run to its first await within this same + // DFS, leaving post-await bindings TDZ (#30259). We additionally + // require asyncEvaluationOrder() < asyncOrderWatermark, i.e. the + // dep entered EvaluatingAsync in a *prior* Evaluate() call. bool depWasAlreadyEvaluatingAsync = false; if (auto* depCyclic = dynamicDowncast(requiredModule)) depWasAlreadyEvaluatingAsync = depCyclic->status() == Status::EvaluatingAsync; -#endif + // 11.b. Set index to ? InnerModuleEvaluation(requiredModule, stack, index). + unsigned result = requiredModule->innerModuleEvaluation(globalObject, stack, index, asyncOrderWatermark); +#else // 11.b. Set index to ? InnerModuleEvaluation(requiredModule, stack, index). unsigned result = requiredModule->innerModuleEvaluation(globalObject, stack, index); +#endif RETURN_IF_EXCEPTION(scope, invalid); index = result; // 11.c. If requiredModule is a Cyclic Module Record, then @@ -1199,11 +1211,15 @@ unsigned AbstractModuleRecord::innerModuleEvaluation(JSGlobalObject* globalObjec #if USE(BUN_JSC_ADDITIONS) // See note above the recursive call: skip the spec-mandated // wait to avoid self-deadlock and match old-loader behaviour. - // Only skip when the cycle root's body has already been - // entered (pendingAsyncDependencies == 0); otherwise the SCC - // is still queued behind an async dep and its bindings are - // TDZ, so the wait is required. - if (!depWasAlreadyEvaluatingAsync || cyclic->pendingAsyncDependencies().value_or(1)) { + // Only skip when the cycle root entered EvaluatingAsync in a + // *prior* Evaluate() (order < asyncOrderWatermark) — a dep + // that became EvaluatingAsync earlier in *this* DFS is a + // sibling whose post-await bindings are still TDZ (#30259) — + // and its body has already been entered + // (pendingAsyncDependencies == 0). + if (!depWasAlreadyEvaluatingAsync + || cyclic->asyncEvaluationOrder().order() >= asyncOrderWatermark + || cyclic->pendingAsyncDependencies().value_or(1)) { #endif // 11.c.v.1. Set module.[[PendingAsyncDependencies]] to module.[[PendingAsyncDependencies]] + 1. module->setPendingAsyncDependencies(module->pendingAsyncDependencies().value() + 1); diff --git a/Source/JavaScriptCore/runtime/AbstractModuleRecord.h b/Source/JavaScriptCore/runtime/AbstractModuleRecord.h index dc02683422ca..12d3b8cd7be0 100644 --- a/Source/JavaScriptCore/runtime/AbstractModuleRecord.h +++ b/Source/JavaScriptCore/runtime/AbstractModuleRecord.h @@ -227,7 +227,11 @@ class AbstractModuleRecord : public JSInternalFieldObjectImpl<2> { WriteBarrier internalField(Field field) const { return Base::internalField(static_cast(field)); } void evaluateModuleSync(JSGlobalObject*); +#if USE(BUN_JSC_ADDITIONS) + unsigned innerModuleEvaluation(JSGlobalObject*, Vector& stack, unsigned index, int64_t asyncOrderWatermark); +#else unsigned innerModuleEvaluation(JSGlobalObject*, Vector& stack, unsigned index); +#endif unsigned innerModuleLinking(JSGlobalObject*, Vector& stack, unsigned index, RefPtr); DECLARE_VISIT_CHILDREN; diff --git a/Source/JavaScriptCore/runtime/CyclicModuleRecord.cpp b/Source/JavaScriptCore/runtime/CyclicModuleRecord.cpp index 27d72e0e2763..f9b94e8c6cd9 100644 --- a/Source/JavaScriptCore/runtime/CyclicModuleRecord.cpp +++ b/Source/JavaScriptCore/runtime/CyclicModuleRecord.cpp @@ -445,7 +445,14 @@ JSPromise* CyclicModuleRecord::evaluate(JSGlobalObject* globalObject) // 7. Set module.[[TopLevelCapability]] to capability. module->setTopLevelCapability(vm, capability); // 8. Let result be Completion(InnerModuleEvaluation(module, stack, 0)). +#if USE(BUN_JSC_ADDITIONS) + // Snapshot the async-order counter so the DFS can tell deps that were + // already EvaluatingAsync before this Evaluate() (re-entrant deadlock + // case, see note at 11.c.v) from siblings that transition during it. + module->innerModuleEvaluation(globalObject, stack, 0, vm.moduleAsyncEvaluationCount()); +#else module->innerModuleEvaluation(globalObject, stack, 0); +#endif // 9. If result is an abrupt completion, then if (Exception* exception = scope.exception()) { JSModuleLoader::attachErrorInfo(globalObject, exception, module, moduleKey(), moduleType(), JSModuleLoader::ModuleFailure::Kind::Evaluation); diff --git a/Source/JavaScriptCore/runtime/VM.h b/Source/JavaScriptCore/runtime/VM.h index 65cc6b52e305..285ea6988e1a 100644 --- a/Source/JavaScriptCore/runtime/VM.h +++ b/Source/JavaScriptCore/runtime/VM.h @@ -1225,6 +1225,9 @@ class VM : public ThreadSafeRefCountedWithSuppressingSaferCPPChecking { void notifyDebuggerHookInjected() { m_isDebuggerHookInjected = true; } bool isDebuggerHookInjected() const { return m_isDebuggerHookInjected; } int64_t incrementModuleAsyncEvaluationCount() { return m_moduleAsyncEvaluationCount++; } +#if USE(BUN_JSC_ADDITIONS) + int64_t moduleAsyncEvaluationCount() const { return m_moduleAsyncEvaluationCount; } +#endif #if ENABLE(WEBASSEMBLY_DEBUGGER) JS_EXPORT_PRIVATE Wasm::DebugState* NODELETE debugState();