Skip to content

JSC: evaluate dynamically imported modules under the importer's async context #274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 2 commits into
base: main
Choose a base branch
from
Open

JSC: evaluate dynamically imported modules under the importer's async context #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7646eb
JSC: evaluate dynamically imported modules under the importer's async…
robobun Aug 12, 2026
a919cd4
JSC: keep the importer's async context across top-level-await depende…
robobun Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions Source/JavaScriptCore/runtime/CyclicModuleRecord.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "SourceProfiler.h"
#include "SymbolTableInlines.h"
#if USE(BUN_JSC_ADDITIONS)
#include "AsyncContextSwapScope.h"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
#include "SyntheticModuleRecord.h"
#endif
#include "UnlinkedModuleProgramCodeBlock.h"
Expand Down Expand Up @@ -555,7 +556,16 @@ void CyclicModuleRecord::executeAsync(JSGlobalObject* globalObject)
// 7. Let onRejected be CreateBuiltinFunction(rejectedClosure, 0, "", « »).
// Also handled in JSMicrotask.cpp.
// 8. Perform PerformPromiseThen(capability.[[Promise]], onFulfilled, onRejected).
#if USE(BUN_JSC_ADDITIONS)
// AsyncModuleExecutionFulfilled runs the bodies of the modules waiting on this one, so
// it has to run under the async context this evaluation was started under (for a
// dynamic import(), the importer's context installed by dynamicImportLoadSettled).
// Snapshot it alongside the module, as a promise reaction would; the
// AsyncModuleExecutionDone microtask unwraps the tuple and reinstalls it.
promise->performPromiseThenWithInternalMicrotask(vm, InternalMicrotask::AsyncModuleExecutionDone, nullptr, AsyncContextSwapScope::wrapWithCurrent(vm, globalObject, this));
Comment on lines +559 to +565

@claude claude Bot Aug 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not a defect — just noting a newly observable edge case: when two concurrent import()s under distinct als.run() stores share the same not-yet-evaluated TLA dependency D, wrapWithCurrent snapshots the context once (at D's executeAsync, under the first importer's store), and asyncExecutionFulfilled then runs both async parents' bodies under it — so the second importer's module observes the first importer's store. This matches Node/V8 (CPED is captured at D's single PerformPromiseThen), so it's the correct parity behavior; might be worth a companion test in oven-sh/bun#37933 pinning the first-importer-wins semantics, since the 60-concurrent-imports stress test gives each import a distinct TLA dep and wouldn't catch it.

Extended reasoning...

What happens

When two dynamic imports race for the same unevaluated top-level-await dependency, the second importer's module body runs under the first importer's async context rather than its own. Concretely:

D.mjs:  await 0;
M1.mjs: import './D.mjs'; console.log(als.getStore());
M2.mjs: import './D.mjs'; console.log(als.getStore());
index:  als.run(CTX1, () => import('./M1.mjs'));
        als.run(CTX2, () => import('./M2.mjs'));

M1 prints CTX1; M2 also prints CTX1 (not CTX2). Before this PR both printed undefined.

Step-by-step trace

  1. dynamicImportLoadSettled(M1) installs CTX1 (JSMicrotask.cpp:1505). innerModuleEvaluation(M1) recurses into D; D has TLA and pendingAsyncDependencies == 0, so D->executeAsync() runs (AbstractModuleRecord.cpp:1354). Inside it, wrapWithCurrent (CyclicModuleRecord.cpp:565) snapshots CTX1 into D's AsyncModuleExecutionDone reaction. Back in M1, pendingAsyncDependencies == 1, so M1 gets an asyncEvaluationOrder and is appended to D.[[AsyncParentModules]] without executing (AbstractModuleRecord.cpp:1346-1352).
  2. dynamicImportLoadSettled(M2) installs CTX2. innerModuleEvaluation(M2) reaches D whose status is now EvaluatingAsync; step 12.b.v appends M2 to D.[[AsyncParentModules]] and sets M2.pendingAsyncDependencies = 1 (AbstractModuleRecord.cpp:1319-1335). At line 1346 pendingAsyncDependencies > 0, so M2 does not execute either. The CTX2 scope is torn down; CTX2 is never snapshotted anywhere.
  3. D's await 0 resolves. AsyncModuleExecutionDone unwraps [D, CTX1] and reinstalls CTX1 (JSMicrotask.cpp:2214). asyncExecutionFulfilled(D) calls gatherAvailableAncestors, which collects {M1, M2}, sorts them, and runs m->execute() for each (CyclicModuleRecord.cpp:733) — both inside the single CTX1 scope.

Result: M2's top-level body observes als.getStore() === CTX1.

Why this is Node parity, not a defect

This is exactly what Node.js does, and matching Node is the PR's stated goal. V8 propagates AsyncLocalStorage via continuation-preserved embedder data, which is captured per promise reaction at PerformPromiseThen time. Per the ES spec, ExecuteAsyncModule(D) calls PerformPromiseThen once — during innerModuleEvaluation(M1), under CTX1 — and AsyncModuleExecutionFulfilled synchronously runs all of D's [[AsyncParentModules]] from that single reaction. So in Node, M2 also observes CTX1. wrapWithCurrent at executeAsync is the direct analogue of V8's CPED-at-PerformPromiseThen, and the observed behavior is the correct parity outcome.

This is also inherent to the ES module design: a TLA module has exactly one completion reaction, and gatherAvailableAncestors runs every waiting parent inside it. There is no per-async-parent capture point where M2's own context could be recorded without diverging from both the spec structure and Node. "Fixing" it (per-parent context storage on CyclicModuleRecord) would make Bun diverge from Node — the opposite of what oven-sh/bun#32693 asks for.

Why it's still worth a note

Before this PR M2 saw undefined; after, it sees a foreign store. That's a move from "wrong per Node" to "matches Node", which is strictly the fix — but it is a newly observable first-importer-wins behavior that nothing in the current test suite pins. The 60-concurrent-imports stress test in the Verification section gives each import a distinct TLA dependency, so it would not exercise this path. A short companion test in oven-sh/bun#37933 asserting M2 sees CTX1 (and confirming it against Node) would lock the parity in and make the limitation discoverable; alternatively a one-line addition to the comment at CyclicModuleRecord.cpp:560-564 noting that a shared TLA dependency's async parents all inherit the first evaluator's context would suffice.

Addressing the objection

One reviewer argued this should not be filed at all because it is intentional Node-matching behavior. That objection is correct on the substance — this is not a code defect and no code change is being requested. The comment is filed as a nit whose only actionable ask is documentation/test coverage of a corner case the PR newly makes observable, which is cheap and does not block merge.

#else
promise->performPromiseThenWithInternalMicrotask(vm, InternalMicrotask::AsyncModuleExecutionDone, nullptr, this);
#endif
// 9. Perform ! module.ExecuteModule(capability).
execute(globalObject, promise);
RETURN_IF_EXCEPTION(scope, void());
Expand Down
28 changes: 25 additions & 3 deletions Source/JavaScriptCore/runtime/JSMicrotask.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1192,7 +1192,7 @@ static void moduleLoadTopSettled(JSGlobalObject* globalObject, VM& vm, ThrowScop
innerLoadFlags.add(ModuleLoadFlag::UseImportMap);
if (context->dynamic()) {
#if USE(BUN_JSC_ADDITIONS)
combinedCell = ModuleLoaderPayload::create(vm, statePromise, context->deferred(), context->referrerAsyncOrder());
combinedCell = ModuleLoaderPayload::create(vm, statePromise, context->deferred(), context->referrerAsyncOrder(), context->importerAsyncContext());
#else
combinedCell = ModuleLoaderPayload::create(vm, statePromise, context->deferred());
#endif
Expand Down Expand Up @@ -1485,7 +1485,16 @@ static void dynamicImportLoadSettled(JSGlobalObject* globalObject, VM& vm, Throw
if (!deferred) {
// 6.c. Let evaluatePromise be module.Evaluate().
#if USE(BUN_JSC_ADDITIONS)
JSPromise* evaluatePromise = module->evaluate(globalObject, dynamicPayload->referrerAsyncOrder());
JSPromise* evaluatePromise = nullptr;
{
// The module bodies in this graph are the continuation of the import() call, so
// they run under the async context captured there (JSModuleLoader::loadModule).
// Modules held back by a top-level-await dependency execute later, from
// AsyncModuleExecutionDone, which reinstalls the context executeAsync snapshots
// while running inside this scope.
AsyncContextSwapScope asyncContextScope(vm, globalObject, dynamicPayload->importerAsyncContext());
evaluatePromise = module->evaluate(globalObject, dynamicPayload->referrerAsyncOrder());
}
Comment thread
claude[bot] marked this conversation as resolved.
#else
JSPromise* evaluatePromise = module->evaluate(globalObject);
#endif
Expand Down Expand Up @@ -1516,6 +1525,11 @@ static void dynamicImportLoadSettled(JSGlobalObject* globalObject, VM& vm, Throw

// For each Module Record dep of evaluationList, append dep.Evaluate() to asyncDepsEvaluationPromises.
MarkedArgumentBuffer asyncDepsEvaluationPromises;
#if USE(BUN_JSC_ADDITIONS)
// As above: the eagerly evaluated async dependencies are the continuation of the
// import.defer() call, so they run under the async context captured there.
AsyncContextSwapScope asyncContextScope(vm, globalObject, dynamicPayload->importerAsyncContext());
#endif
for (AbstractModuleRecord* dep : evaluationList) {
#if USE(BUN_JSC_ADDITIONS)
JSPromise* depPromise = dep->evaluate(globalObject, dynamicPayload->referrerAsyncOrder());
Expand Down Expand Up @@ -2188,7 +2202,15 @@ void runInternalMicrotask(JSGlobalObject* globalObject, VM& vm, InternalMicrotas
#endif

case InternalMicrotask::AsyncModuleExecutionDone: {
auto* module = uncheckedDowncast<JSModuleRecord>(arguments[2]);
// CyclicModuleRecord::executeAsync wraps the module together with Bun's
// async context in an InternalFieldTuple when a context was active as the
// module started executing, so that the waiting ancestors executed by
// asyncExecutionFulfilled run under that context too.
JSValue contextArg = arguments[2];
#if USE(BUN_JSC_ADDITIONS)
AsyncContextSwapScope asyncContextScope(vm, globalObject, AsyncContextSwapScope::unwrapContextTuple(contextArg));
#endif
auto* module = uncheckedDowncast<JSModuleRecord>(contextArg);
RELEASE_AND_RETURN(scope, asyncModuleExecutionDone(module->realm(), module, arguments[1], static_cast<JSPromise::Status>(payload)));
}

Expand Down
14 changes: 13 additions & 1 deletion Source/JavaScriptCore/runtime/JSModuleLoader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@
#include "SyntheticModuleRecord.h"
#include "TopExceptionScope.h"
#include "VMTrapsInlines.h"
#if USE(BUN_JSC_ADDITIONS)
#include "AsyncContextSwapScope.h"
#endif
#include <wtf/text/MakeString.h>

namespace JSC {
Expand Down Expand Up @@ -354,10 +357,19 @@ JSPromise* JSModuleLoader::loadModule(JSGlobalObject* globalObject, const Identi
JSPromise* promise = nullptr;

ScriptFetchParameters::Type type = parameters ? parameters->type() : ScriptFetchParameters::Type::JavaScript;
JSValue importerAsyncContext = jsUndefined();
#if USE(BUN_JSC_ADDITIONS)
// Preserve the caller's ScriptFetchParameters (HostDefined data) into the loading context
// before `parameters` is moved into fetch() below.
RefPtr<ScriptFetchParameters> contextParameters = parameters ? parameters : ScriptFetchParameters::create(type);

// A dynamic import() evaluates its module graph under the async context (AsyncLocalStorage
// store) that was active at the import() call site. We are still synchronous with that call
// here, so the slot holds the importer's context; the graph is linked and evaluated several
// internal microtasks later, by which point the slot has been reset. Capture it now, before
// fetch() below can run host code, and carry it through to dynamicImportLoadSettled.
if (flags.contains(ModuleLoadFlag::Dynamic))
importerAsyncContext = AsyncContextSwapScope::current(globalObject);
#endif

if (ModuleRegistryEntry* entry = getRegisteredMayBeNull(specifier, type)) {
Expand All @@ -384,7 +396,7 @@ JSPromise* JSModuleLoader::loadModule(JSGlobalObject* globalObject, const Identi
#else
AbstractModuleRecord::ModuleRequest request { specifier, ScriptFetchParameters::create(type) };
#endif
auto* context = ModuleLoadingContext::create(vm, request, WTF::move(scriptFetcher), flags, referrerAsyncOrder);
auto* context = ModuleLoadingContext::create(vm, request, WTF::move(scriptFetcher), flags, referrerAsyncOrder, importerAsyncContext);

JSPromise* intermediatePromise = JSPromise::create(vm, globalObject->promiseStructure());
intermediatePromise->markAsHandled();
Expand Down
11 changes: 8 additions & 3 deletions Source/JavaScriptCore/runtime/ModuleLoaderPayload.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,18 @@ namespace JSC {

const ClassInfo ModuleLoaderPayload::s_info = { "ModuleLoaderPayload"_s, nullptr, nullptr, nullptr, CREATE_METHOD_TABLE(ModuleLoaderPayload) };

ModuleLoaderPayload::ModuleLoaderPayload(VM& vm, Structure* structure, JSPromise* promise, bool deferred, int64_t referrerAsyncOrder)
ModuleLoaderPayload::ModuleLoaderPayload(VM& vm, Structure* structure, JSPromise* promise, bool deferred, int64_t referrerAsyncOrder, JSValue importerAsyncContext)
: Base(vm, structure)
, m_promise(promise, WriteBarrierEarlyInit)
#if USE(BUN_JSC_ADDITIONS)
, m_importerAsyncContext(importerAsyncContext, WriteBarrierEarlyInit)
, m_referrerAsyncOrder(referrerAsyncOrder)
#endif
, m_deferred(deferred)
{
#if !USE(BUN_JSC_ADDITIONS)
UNUSED_PARAM(referrerAsyncOrder);
UNUSED_PARAM(importerAsyncContext);
#endif
}

Expand All @@ -60,13 +62,16 @@ void ModuleLoaderPayload::visitChildrenImpl(JSCell* cell, Visitor& visitor)
Base::visitChildren(thisObject, visitor);
visitor.append(thisObject->m_promise);
visitor.append(thisObject->m_fulfillment);
#if USE(BUN_JSC_ADDITIONS)
visitor.append(thisObject->m_importerAsyncContext);
#endif
}

DEFINE_VISIT_CHILDREN(ModuleLoaderPayload);

ModuleLoaderPayload* ModuleLoaderPayload::create(VM& vm, JSPromise* promise, bool deferred, int64_t referrerAsyncOrder)
ModuleLoaderPayload* ModuleLoaderPayload::create(VM& vm, JSPromise* promise, bool deferred, int64_t referrerAsyncOrder, JSValue importerAsyncContext)
{
ModuleLoaderPayload* instance = new (NotNull, allocateCell<ModuleLoaderPayload>(vm)) ModuleLoaderPayload(vm, vm.moduleLoaderPayloadStructure.get(), promise, deferred, referrerAsyncOrder);
ModuleLoaderPayload* instance = new (NotNull, allocateCell<ModuleLoaderPayload>(vm)) ModuleLoaderPayload(vm, vm.moduleLoaderPayloadStructure.get(), promise, deferred, referrerAsyncOrder, importerAsyncContext);
instance->finishCreation(vm);
return instance;
}
Expand Down
7 changes: 5 additions & 2 deletions Source/JavaScriptCore/runtime/ModuleLoaderPayload.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class ModuleLoaderPayload final : public JSCell {
}

inline static Structure* createStructure(VM&, JSGlobalObject*, JSValue);
static ModuleLoaderPayload* create(VM&, JSPromise*, bool deferred = false, int64_t referrerAsyncOrder = -1);
static ModuleLoaderPayload* create(VM&, JSPromise*, bool deferred = false, int64_t referrerAsyncOrder = -1, JSValue importerAsyncContext = jsUndefined());

JSPromise* promise() const { return m_promise.get(); }

Expand All @@ -59,6 +59,8 @@ class ModuleLoaderPayload final : public JSCell {
bool deferred() const { return m_deferred; }
#if USE(BUN_JSC_ADDITIONS)
int64_t referrerAsyncOrder() const { return m_referrerAsyncOrder; }
// The async context (AsyncLocalStorage store) active at the import() call site.
JSValue importerAsyncContext() const { return m_importerAsyncContext.get(); }
Comment thread
coderabbitai[bot] marked this conversation as resolved.
#endif

bool decrementRemaining()
Expand All @@ -68,13 +70,14 @@ class ModuleLoaderPayload final : public JSCell {
}

private:
ModuleLoaderPayload(VM&, Structure*, JSPromise*, bool deferred, int64_t referrerAsyncOrder);
ModuleLoaderPayload(VM&, Structure*, JSPromise*, bool deferred, int64_t referrerAsyncOrder, JSValue importerAsyncContext);

void finishCreation(VM&);

WriteBarrier<JSPromise> m_promise;
WriteBarrier<Unknown> m_fulfillment;
#if USE(BUN_JSC_ADDITIONS)
WriteBarrier<Unknown> m_importerAsyncContext;
int64_t m_referrerAsyncOrder { -1 };
#endif
uint8_t m_remainingFulfillments { 2 };
Expand Down
11 changes: 8 additions & 3 deletions Source/JavaScriptCore/runtime/ModuleLoadingContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,24 +62,26 @@ ModuleLoadingContext* ModuleLoadingContext::create(VM& vm, Step step, const JSMo
return context;
}

ModuleLoadingContext::ModuleLoadingContext(VM& vm, Structure* structure, AbstractModuleRecord::ModuleRequest&& moduleRequest, RefPtr<ScriptFetcher> scriptFetcher, OptionSet<ModuleLoadFlag> flags, int64_t referrerAsyncOrder)
ModuleLoadingContext::ModuleLoadingContext(VM& vm, Structure* structure, AbstractModuleRecord::ModuleRequest&& moduleRequest, RefPtr<ScriptFetcher> scriptFetcher, OptionSet<ModuleLoadFlag> flags, int64_t referrerAsyncOrder, JSValue importerAsyncContext)
: Base(vm, structure)
, m_moduleRequest(WTF::move(moduleRequest))
, m_scriptFetcher(WTF::move(scriptFetcher))
#if USE(BUN_JSC_ADDITIONS)
, m_importerAsyncContext(importerAsyncContext, WriteBarrierEarlyInit)
, m_referrerAsyncOrder(referrerAsyncOrder)
#endif
, m_flags(flags)
{
#if !USE(BUN_JSC_ADDITIONS)
UNUSED_PARAM(referrerAsyncOrder);
UNUSED_PARAM(importerAsyncContext);
#endif
}

ModuleLoadingContext* ModuleLoadingContext::create(VM& vm, const AbstractModuleRecord::ModuleRequest& moduleRequest, RefPtr<ScriptFetcher> scriptFetcher, OptionSet<ModuleLoadFlag> flags, int64_t referrerAsyncOrder)
ModuleLoadingContext* ModuleLoadingContext::create(VM& vm, const AbstractModuleRecord::ModuleRequest& moduleRequest, RefPtr<ScriptFetcher> scriptFetcher, OptionSet<ModuleLoadFlag> flags, int64_t referrerAsyncOrder, JSValue importerAsyncContext)
{
AbstractModuleRecord::ModuleRequest requestCopy { moduleRequest };
auto* context = new (NotNull, allocateCell<ModuleLoadingContext>(vm)) ModuleLoadingContext(vm, vm.moduleLoadingContextStructure.get(), WTF::move(requestCopy), WTF::move(scriptFetcher), flags, referrerAsyncOrder);
auto* context = new (NotNull, allocateCell<ModuleLoadingContext>(vm)) ModuleLoadingContext(vm, vm.moduleLoadingContextStructure.get(), WTF::move(requestCopy), WTF::move(scriptFetcher), flags, referrerAsyncOrder, importerAsyncContext);
context->finishCreation(vm);
return context;
}
Expand All @@ -104,6 +106,9 @@ void ModuleLoadingContext::visitChildrenImpl(JSCell* cell, Visitor& visitor)
visitor.append(thisObject->m_entry);
visitor.append(thisObject->m_referrer);
visitor.append(thisObject->m_module);
#if USE(BUN_JSC_ADDITIONS)
visitor.append(thisObject->m_importerAsyncContext);
#endif
}

DEFINE_VISIT_CHILDREN(ModuleLoadingContext);
Expand Down
7 changes: 5 additions & 2 deletions Source/JavaScriptCore/runtime/ModuleLoadingContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class ModuleLoadingContext final : public JSCell {
};

static ModuleLoadingContext* create(VM&, Step, const JSModuleLoader::ModuleReferrer&, const AbstractModuleRecord::ModuleRequest&, JSCell* payload, ModuleRegistryEntry*, RefPtr<ScriptFetcher>);
static ModuleLoadingContext* create(VM&, const AbstractModuleRecord::ModuleRequest&, RefPtr<ScriptFetcher>, OptionSet<ModuleLoadFlag>, int64_t referrerAsyncOrder = -1);
static ModuleLoadingContext* create(VM&, const AbstractModuleRecord::ModuleRequest&, RefPtr<ScriptFetcher>, OptionSet<ModuleLoadFlag>, int64_t referrerAsyncOrder = -1, JSValue importerAsyncContext = jsUndefined());

Step step() const { return m_step; }
void setStep(Step s) { m_step = s; }
Expand All @@ -80,11 +80,13 @@ class ModuleLoadingContext final : public JSCell {
bool deferred() const { return m_flags.contains(ModuleLoadFlag::Deferred); }
#if USE(BUN_JSC_ADDITIONS)
int64_t referrerAsyncOrder() const { return m_referrerAsyncOrder; }
// The async context (AsyncLocalStorage store) active at the import() call site.
JSValue importerAsyncContext() const { return m_importerAsyncContext.get(); }
#endif

private:
ModuleLoadingContext(VM&, Structure*, Step, const JSModuleLoader::ModuleReferrer&, AbstractModuleRecord::ModuleRequest&&, JSCell* payload, ModuleRegistryEntry*, RefPtr<ScriptFetcher>);
ModuleLoadingContext(VM&, Structure*, AbstractModuleRecord::ModuleRequest&&, RefPtr<ScriptFetcher>, OptionSet<ModuleLoadFlag>, int64_t referrerAsyncOrder);
ModuleLoadingContext(VM&, Structure*, AbstractModuleRecord::ModuleRequest&&, RefPtr<ScriptFetcher>, OptionSet<ModuleLoadFlag>, int64_t referrerAsyncOrder, JSValue importerAsyncContext);

Step m_step { Step::Main };
AbstractModuleRecord::ModuleRequest m_moduleRequest;
Expand All @@ -94,6 +96,7 @@ class ModuleLoadingContext final : public JSCell {
WriteBarrier<Unknown> m_referrer;
WriteBarrier<AbstractModuleRecord> m_module;
#if USE(BUN_JSC_ADDITIONS)
WriteBarrier<Unknown> m_importerAsyncContext;
int64_t m_referrerAsyncOrder { -1 };
#endif
OptionSet<ModuleLoadFlag> m_flags;
Expand Down
Loading