Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions src/jsc/bindings/ModuleLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,15 @@ extern "C" void Bun__onFulfillAsyncModule(
JSC::JSPromise* promise = uncheckedDowncast<JSC::JSPromise>(JSC::JSValue::decode(encodedPromiseValue));

if (!res->success) {
// The module failed to load and will never evaluate, so no
// moduleLoaderEvaluate call will consume a dynamic-import async context
// captured for it (#32693); drop it here so it can't pin the store.
if (auto* map = globalObject->m_pendingDynamicImportAsyncContexts.get()) {
if (map->size()) {
map->remove(globalObject, JSC::jsString(vm, specifier->toWTFString(BunString::ZeroCopy)));
scope.assertNoException(); // JSMap::remove (non-allocating) cannot throw
}
Comment thread
robobun marked this conversation as resolved.
}
RELEASE_AND_RETURN(scope, promise->reject(vm, JSValue::decode(res->result.err.value)));
}

Expand Down
109 changes: 105 additions & 4 deletions src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3483,6 +3483,48 @@ JSC::Identifier GlobalObject::moduleLoaderResolve(JSGlobalObject* jsGlobalObject
}
}

// Record the AsyncLocalStorage context active at an import() call site, keyed by
// the resolved module key, so the imported module's top-level evaluation can run
// with it (see evaluateModuleWithCapturedAsyncContext). JSC's dynamic-import
// microtasks never restore m_asyncContextData; Node preserves it via V8's
// continuation-preserved embedder data. Returns the key to remove on cleanup, or
// null when nothing was recorded. When there is no active context, any entry a
// prior import of this key left behind without evaluating (e.g. a load that
// failed after its own fetch succeeded, so no fetch-failure seam fired) is
// dropped here, so a stale context is never applied to this evaluation.
static JSC::JSString* captureDynamicImportAsyncContext(Zig::GlobalObject* globalObject, JSC::VM& vm, const JSC::Identifier& resolvedIdentifier)
{
if (!globalObject->isAsyncContextTrackingEnabled())
return nullptr;
JSC::JSMap* map = globalObject->m_pendingDynamicImportAsyncContexts.get();
JSC::JSValue asyncContext = globalObject->m_asyncContextData.get()->getInternalField(0);
if (asyncContext.isUndefined()) {
// JSMap::remove cannot throw. Clears a stale entry left by a prior import
// of this key that never evaluated, so no context leaks into this one.
if (map && map->size())
map->remove(globalObject, jsString(vm, resolvedIdentifier.string()));
return nullptr;
}
if (!map) {
map = JSC::JSMap::create(vm, globalObject->mapStructure());
globalObject->m_pendingDynamicImportAsyncContexts.set(vm, globalObject, map);
}
JSC::JSString* key = jsString(vm, resolvedIdentifier.string());
// JSMap::set opens an internal throw scope (it can only fail on OOM). The
// caller checks the scope right after this returns, so don't add one here.
map->set(globalObject, key, asyncContext);
return key;
}
Comment thread
robobun marked this conversation as resolved.

static void dropDynamicImportAsyncContext(Zig::GlobalObject* globalObject, JSC::JSString* key)
{
if (!key)
return;
// JSMap::remove cannot throw.
if (auto* map = globalObject->m_pendingDynamicImportAsyncContexts.get())
map->remove(globalObject, key);
}

JSC::JSPromise* GlobalObject::moduleLoaderImportModule(JSGlobalObject* jsGlobalObject,
JSModuleLoader*,
JSString* moduleNameValue,
Expand Down Expand Up @@ -3532,10 +3574,16 @@ JSC::JSPromise* GlobalObject::moduleLoaderImportModule(JSGlobalObject* jsGlobalO
if (auto resolution = globalObject->onLoadPlugins.resolveVirtualModule(moduleName, sourceURL.protocolIsFile() ? sourceOriginStringHolder : String())) {
resolvedIdentifier = JSC::Identifier::fromString(vm, resolution.value());

JSC::JSString* asyncContextKey = captureDynamicImportAsyncContext(globalObject, vm, resolvedIdentifier);
RETURN_IF_EXCEPTION(scope, JSC::JSPromise::rejectedPromiseWithCaughtException(globalObject, scope));
auto result = JSC::importModule(globalObject, resolvedIdentifier, JSC::Identifier(), parameters, nullptr, /* deferred */ false, referrerAsyncOrder);
if (scope.exception()) [[unlikely]] {
return JSC::JSPromise::rejectedPromiseWithCaughtException(globalObject, scope);
}
if (asyncContextKey && (!result || result->status() != JSC::JSPromise::Status::Pending)) {
dropDynamicImportAsyncContext(globalObject, asyncContextKey);
scope.assertNoException(); // JSMap::remove (non-allocating) cannot throw
}
return result;
}
}
Expand Down Expand Up @@ -3588,15 +3636,30 @@ JSC::JSPromise* GlobalObject::moduleLoaderImportModule(JSGlobalObject* jsGlobalO
sourceOriginZ.deref();
}

JSC::JSString* asyncContextKey = captureDynamicImportAsyncContext(globalObject, vm, resolvedIdentifier);
RETURN_IF_EXCEPTION(scope, JSC::JSPromise::rejectedPromiseWithCaughtException(globalObject, scope));

// The C++ module loader now extracts `with.type` into a
// ScriptFetchParameters before calling this hook, so `parameters` is
// already the parsed RefPtr (or null). Just forward it.
auto result = JSC::importModule(globalObject, resolvedIdentifier,
JSC::Identifier(), WTF::move(parameters), nullptr, /* deferred */ false, referrerAsyncOrder);
if (scope.exception()) [[unlikely]] {
// A synchronous importModule failure leaves the captured entry; it is
// cleared on the next import of this key (see captureDynamicImportAsyncContext).
return JSC::JSPromise::rejectedPromiseWithCaughtException(globalObject, scope);
}

// Only a still-pending import reaches moduleLoaderEvaluate to consume the
// entry; a cached (already-evaluated) module settles synchronously and never
// re-evaluates, so drop its entry now. A pending import that later rejects
// without evaluating is cleaned up at its fetch-failure seam
// (Bun__onFulfillAsyncModule) or overwritten on the next import of the key.
if (asyncContextKey && (!result || result->status() != JSC::JSPromise::Status::Pending)) {
dropDynamicImportAsyncContext(globalObject, asyncContextKey);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
scope.assertNoException(); // JSMap::remove (non-allocating) cannot throw
}

ASSERT(result);
return result;
}
Expand Down Expand Up @@ -3703,13 +3766,51 @@ JSC::JSObject* GlobalObject::moduleLoaderCreateImportMetaProperties(JSGlobalObje
return Zig::ImportMetaObject::create(globalObject, key);
}

// Reinstate the AsyncLocalStorage context captured at this module's import()
// call site (see moduleLoaderImportModule) around its synchronous top-level
// evaluation. JSC drives dynamic-import evaluation from an internal microtask
// that never restores m_asyncContextData, so getStore() would otherwise be
// undefined during module init (#32693). A top-level-await module's post-await
// continuations resume through JSC's async-module machinery outside this hook
// and are not covered here.
static JSC::JSValue evaluateModuleWithCapturedAsyncContext(Zig::GlobalObject* globalObject,
JSModuleLoader* moduleLoader, JSValue key, JSValue moduleRecordValue,
RefPtr<JSC::ScriptFetcher>&& scriptFetcher, JSValue sentValue, JSValue resumeMode)
{
auto& vm = JSC::getVM(globalObject);

auto scope = DECLARE_THROW_SCOPE(vm);
JSC::JSValue capturedAsyncContext;
if (auto* map = globalObject->m_pendingDynamicImportAsyncContexts.get(); map && map->size()) {
// get/remove each open an internal throw scope (they can only fail on
// OOM), so check after each. get returns jsUndefined() when absent.
capturedAsyncContext = map->get(globalObject, key);
RETURN_IF_EXCEPTION(scope, {});
if (!capturedAsyncContext.isUndefined()) {
map->remove(globalObject, key);
RETURN_IF_EXCEPTION(scope, {});
}
}

if (!capturedAsyncContext || capturedAsyncContext.isUndefined() || !globalObject->isAsyncContextTrackingEnabled())
RELEASE_AND_RETURN(scope, moduleLoader->evaluateNonVirtual(globalObject, key, moduleRecordValue, WTF::move(scriptFetcher), sentValue, resumeMode));

auto* asyncContextData = globalObject->m_asyncContextData.get();
JSC::JSValue restoreAsyncContext = asyncContextData->getInternalField(0);
asyncContextData->putInternalField(vm, 0, capturedAsyncContext);
scope.release();
JSC::JSValue result = moduleLoader->evaluateNonVirtual(globalObject, key, moduleRecordValue, WTF::move(scriptFetcher), sentValue, resumeMode);
asyncContextData->putInternalField(vm, 0, restoreAsyncContext);
return result;
}

JSC::JSValue GlobalObject::moduleLoaderEvaluate(JSGlobalObject* lexicalGlobalObject,
JSModuleLoader* moduleLoader, JSValue key,
JSValue moduleRecordValue, RefPtr<JSC::ScriptFetcher> scriptFetcher,
JSValue sentValue, JSValue resumeMode)
{
return moduleLoader->evaluateNonVirtual(lexicalGlobalObject, key, moduleRecordValue,
WTF::move(scriptFetcher), sentValue, resumeMode);
return evaluateModuleWithCapturedAsyncContext(uncheckedDowncast<Zig::GlobalObject>(lexicalGlobalObject),
moduleLoader, key, moduleRecordValue, WTF::move(scriptFetcher), sentValue, resumeMode);
}

extern "C" bool Bun__VM__specifierIsEvalEntryPoint(void*, EncodedJSValue);
Expand All @@ -3724,8 +3825,8 @@ JSC::JSValue EvalGlobalObject::moduleLoaderEvaluate(JSGlobalObject* lexicalGloba
auto& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

JSC::JSValue result = moduleLoader->evaluateNonVirtual(lexicalGlobalObject, key, moduleRecordValue,
WTF::move(scriptFetcher), sentValue, resumeMode);
JSC::JSValue result = evaluateModuleWithCapturedAsyncContext(globalObject, moduleLoader, key,
moduleRecordValue, WTF::move(scriptFetcher), sentValue, resumeMode);
// The new C++ loader propagates the module body's throw out of
// evaluateNonVirtual; the old JS-side ModuleLoader.js swallowed it before
// dispatching here. Don't call back into native code (which opens an
Expand Down
5 changes: 5 additions & 0 deletions src/jsc/bindings/ZigGlobalObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace JSC {
class Structure;
class Identifier;
class JSMap;
class LazyClassStructure;
class ScriptFetcher;
class ScriptFetchParameters;
Expand Down Expand Up @@ -502,6 +503,10 @@ class GlobalObject : public Bun::GlobalScope {
\
V(public, WriteBarrier<Bun::JSNextTickQueue>, m_nextTickQueue) \
\
/* AsyncLocalStorage context captured at an import() call site, keyed by resolved module key, */ \
/* reinstated around the dynamically imported module's top-level evaluation (#32693). */ \
V(public, WriteBarrier<JSC::JSMap>, m_pendingDynamicImportAsyncContexts) \
\
/* WriteBarrier<Unknown> m_JSBunDebuggerValue; */ \
V(private, ThenablesArray, m_thenables) \
\
Expand Down
128 changes: 127 additions & 1 deletion test/js/node/async_hooks/AsyncLocalStorage.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AsyncLocalStorage, AsyncResource } from "async_hooks";
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe } from "harness";
import { bunEnv, bunExe, tempDir } from "harness";

describe("AsyncLocalStorage", () => {
test("throw inside of AsyncLocalStorage.run() will be passed out", () => {
Expand Down Expand Up @@ -567,3 +567,129 @@ describe("async context passes through", () => {
expect(a).toBe("value");
});
});

describe("dynamic import() preserves the AsyncLocalStorage context (#32693)", () => {
test("during the imported module's top-level evaluation", async () => {
using dir = tempDir("als-dynamic-import", {
"store.mjs": `
import { AsyncLocalStorage } from 'node:async_hooks';
export const store = new AsyncLocalStorage();
`,
// Imported lazily from inside store.run(). Its top-level code (module
// evaluation) must observe the store that was active at the import() site,
// matching Node. A nested run() inside the body must still scope correctly
// and restore to the imported context afterwards.
"imported.mjs": `
import { store } from './store.mjs';
console.log("eval:" + store.getStore());
store.run("NESTED", () => {
console.log("nested:" + store.getStore());
});
console.log("after-nested:" + store.getStore());
`,
// Imported with no active context: must evaluate with an undefined store,
// proving the captured context does not leak into unrelated imports.
"no-context.mjs": `
import { store } from './store.mjs';
console.log("no-context-eval:" + store.getStore());
`,
"index.mjs": `
import { store } from './store.mjs';
await store.run('CONTEXT', () => import('./imported.mjs'));
console.log("after-import:" + store.getStore());
await import('./no-context.mjs');
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "index.mjs"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stdout).toBe(
[
"eval:CONTEXT",
"nested:NESTED",
"after-nested:CONTEXT",
"after-import:undefined",
"no-context-eval:undefined",
"",
].join("\n"),
);
expect(exitCode).toBe(0);
expect(stderr).not.toContain("CONTEXT");
});

// A top-level-await module resumes through JSC's async-module machinery, which
// runs outside this hook; only the synchronous prefix observes the context. The
// guarantee here is that capturing the context does not crash that path.
test("a top-level-await module sees the context during its synchronous prefix and does not crash", async () => {
using dir = tempDir("als-dynamic-import-tla", {
"store.mjs": `
import { AsyncLocalStorage } from 'node:async_hooks';
export const store = new AsyncLocalStorage();
`,
"tla.mjs": `
import { store } from './store.mjs';
console.log("tla-sync:" + store.getStore());
await Promise.resolve();
console.log("done");
`,
"index.mjs": `
import { store } from './store.mjs';
await store.run('CONTEXT', () => import('./tla.mjs'));
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "index.mjs"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stdout).toBe("tla-sync:CONTEXT\ndone\n");
expect(exitCode).toBe(0);
});

// A dynamic import that fails to load never evaluates, so the captured context
// is cleaned up at the fetch-failure seam instead of being retained. Exercises
// that path and confirms it neither crashes nor disturbs a later import.
test("a dynamic import that fails to load is caught and does not disturb later imports", async () => {
using dir = tempDir("als-dynamic-import-fail", {
"store.mjs": `
import { AsyncLocalStorage } from 'node:async_hooks';
export const store = new AsyncLocalStorage();
`,
"bad.mjs": `export const x = ;`,
"good.mjs": `
import { store } from './store.mjs';
console.log("good-eval:" + store.getStore());
`,
"index.mjs": `
import { store } from './store.mjs';
const outcome = await store.run('A', () => import('./bad.mjs').then(() => 'loaded', () => 'caught'));
console.log("bad-import:" + outcome);
await store.run('B', () => import('./good.mjs'));
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "index.mjs"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stdout).toBe("bad-import:caught\ngood-eval:B\n");
expect(exitCode).toBe(0);
});
});
Loading