From 856e46d8fde68b09992d62da8abd61f1fc7ed7a3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 1 Jul 2026 12:31:33 +0000 Subject: [PATCH 1/5] module loader: drive a synchronous fetch for require(esm) racing a concurrent import When a CommonJS require() targets an ESM module that a concurrent static import already started fetching, the registry entry exists with status Fetching and no record yet (its transpiler-thread fetch is still in flight). loadModuleSync's top-level loadModule path reuses that pending async fetch promise instead of forcing a synchronous fetch, so the load never completes synchronously and require() wrongly reports the plain (non-top-level-await) module as an unsupported async module: TypeError: require() async module "..." is unsupported. use "await import()" instead. This regressed in the module-loader rewrite (#29393); 1.3.11 returned the module. It reproduces with firebase-admin's auth path, where jwks-rsa require()s jose while the app also imports jose directly. functionEsmLoadSync now forces the synchronous fetch for an entry that is still Fetching with no record, mirroring the dependency-load handling in JSModuleLoader::hostLoadImportedModule, so the entry reaches Fetched before loadModuleSync links and evaluates it. A module that genuinely cannot complete synchronously (real top-level await, async onLoad plugin) leaves the fetch promise pending and still reaches the async-module throw. --- src/jsc/bindings/ZigGlobalObject.cpp | 37 +++++++++++++++++ test/regression/issue/33180.test.ts | 59 ++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 test/regression/issue/33180.test.ts diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index adb5ed6ecdda..b09c6bb1daa0 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -766,6 +766,43 @@ JSC_DEFINE_HOST_FUNCTION(functionEsmLoadSync, (JSC::JSGlobalObject * lexicalGlob RETURN_IF_EXCEPTION(scope, {}); return JSValue::encode(ns); } + + // A concurrent static import of this same module may have created the + // registry entry and kicked off an async (transpiler-thread) fetch that + // is still in flight. loadModuleSync's top-level path reuses that pending + // fetch promise instead of forcing a synchronous fetch, so the graph + // never completes synchronously and require() wrongly reports a plain + // (non-TLA) module as an unsupported async module. Drive the fetch + // synchronously here so the entry reaches Fetched before loadModuleSync + // links and evaluates it, mirroring the dependency-load handling in + // JSModuleLoader::hostLoadImportedModule. A module that genuinely can't + // complete synchronously (real top-level await, async onLoad plugin) + // leaves the fetch promise pending and correctly reaches the throw below. + if (entry->status() == JSC::ModuleRegistryEntry::Status::Fetching && !entry->record()) { + JSPromise* fetchPromise = entry->ensureFetchPromise(globalObject); + JSPromise* modulePromise = entry->ensureModulePromise(globalObject); + RETURN_IF_EXCEPTION(scope, {}); + if (modulePromise->status() == JSPromise::Status::Pending && fetchPromise->status() == JSPromise::Status::Pending) { + JSC::VM::SynchronousModuleQueue queue; + queue.prev = vm.m_synchronousModuleQueue; + vm.m_synchronousModuleQueue = &queue; + JSPromise* src = loader->fetch(globalObject, keyValue, nullptr, nullptr); + bool settled = false; + if (!scope.exception()) { + if (src->status() == JSPromise::Status::Fulfilled) { + fetchPromise->fulfillPromise(vm, src->result()); + settled = true; + } else if (src->status() == JSPromise::Status::Rejected) { + fetchPromise->rejectPromise(vm, src->result()); + settled = true; + } + if (settled && !scope.exception()) + JSC::JSModuleLoader::drainSynchronousModuleQueue(globalObject); + } + vm.m_synchronousModuleQueue = queue.prev; + RETURN_IF_EXCEPTION(scope, {}); + } + } } JSPromise* promise = loader->loadModuleSync(globalObject, key, nullptr, nullptr); diff --git a/test/regression/issue/33180.test.ts b/test/regression/issue/33180.test.ts new file mode 100644 index 000000000000..92b50cab662e --- /dev/null +++ b/test/regression/issue/33180.test.ts @@ -0,0 +1,59 @@ +// https://github.com/oven-sh/bun/issues/33180 +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +// A plain ESM module (no top-level await) that is statically imported while a +// sibling CommonJS module require()s it must load successfully. The static +// import kicks off a concurrent transpiler-thread fetch; the require() lands +// while that fetch is still in flight. loadModuleSync used to reuse the pending +// async fetch promise instead of forcing a synchronous fetch, so require() +// wrongly reported the plain module as an unsupported async module. The module +// body here is large enough that its off-thread transpile is reliably still +// running when the tiny CommonJS sibling evaluates. +function makeTarget() { + let src = ""; + for (let i = 0; i < 1000; i++) src += `export const v${i} = ${i};\n`; + src += `export const val = 42;\n`; + src += `export function getVal() { return val; }\n`; + return src; +} + +const files = { + "target.mjs": makeTarget(), + "cjs-req.cjs": ` + const t = require("./target.mjs"); + if (t.val !== 42 || t.getVal() !== 42) { + console.error("half-initialized namespace: val=" + t.val); + process.exit(3); + } + console.log("required-ok"); + module.exports = {}; + `, + "entry.mjs": ` + import "./target.mjs"; + import "./cjs-req.cjs"; + console.log("entry-done"); + `, +}; + +// Several independent attempts: without the fix the race reports the false +// "async module" error on essentially every run of a debug build, so any one +// attempt failing is enough. Each attempt is its own test so a single spawn +// stays well under the default timeout. +for (let attempt = 0; attempt < 4; attempt++) { + test(`require(esm) racing a concurrent static import loads synchronously (attempt ${attempt})`, async () => { + using dir = tempDir("issue-33180", files); + await using proc = Bun.spawn({ + cmd: [bunExe(), "entry.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + stdout: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("async module"); + expect(stdout).toContain("required-ok"); + expect(stdout).toContain("entry-done"); + expect(exitCode).toBe(0); + }); +} From ecb46556929b2a9447e7676e99878621d5a78dca Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 1 Jul 2026 12:39:29 +0000 Subject: [PATCH 2/5] review: trim comments to durable intent --- src/jsc/bindings/ZigGlobalObject.cpp | 14 +++----------- test/regression/issue/33180.test.ts | 17 +++++------------ 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index b09c6bb1daa0..7372ac109695 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -767,17 +767,9 @@ JSC_DEFINE_HOST_FUNCTION(functionEsmLoadSync, (JSC::JSGlobalObject * lexicalGlob return JSValue::encode(ns); } - // A concurrent static import of this same module may have created the - // registry entry and kicked off an async (transpiler-thread) fetch that - // is still in flight. loadModuleSync's top-level path reuses that pending - // fetch promise instead of forcing a synchronous fetch, so the graph - // never completes synchronously and require() wrongly reports a plain - // (non-TLA) module as an unsupported async module. Drive the fetch - // synchronously here so the entry reaches Fetched before loadModuleSync - // links and evaluates it, mirroring the dependency-load handling in - // JSModuleLoader::hostLoadImportedModule. A module that genuinely can't - // complete synchronously (real top-level await, async onLoad plugin) - // leaves the fetch promise pending and correctly reaches the throw below. + // A concurrent import may leave this entry Fetching with no record; drive + // the fetch synchronously so a sync-completable module reaches Fetched + // before loadModuleSync links it. Async fetches (TLA, plugin) stay pending. if (entry->status() == JSC::ModuleRegistryEntry::Status::Fetching && !entry->record()) { JSPromise* fetchPromise = entry->ensureFetchPromise(globalObject); JSPromise* modulePromise = entry->ensureModulePromise(globalObject); diff --git a/test/regression/issue/33180.test.ts b/test/regression/issue/33180.test.ts index 92b50cab662e..fe147ce75802 100644 --- a/test/regression/issue/33180.test.ts +++ b/test/regression/issue/33180.test.ts @@ -2,14 +2,9 @@ import { expect, test } from "bun:test"; import { bunEnv, bunExe, tempDir } from "harness"; -// A plain ESM module (no top-level await) that is statically imported while a -// sibling CommonJS module require()s it must load successfully. The static -// import kicks off a concurrent transpiler-thread fetch; the require() lands -// while that fetch is still in flight. loadModuleSync used to reuse the pending -// async fetch promise instead of forcing a synchronous fetch, so require() -// wrongly reported the plain module as an unsupported async module. The module -// body here is large enough that its off-thread transpile is reliably still -// running when the tiny CommonJS sibling evaluates. +// A plain (no top-level await) ESM module must stay require()-able while a +// concurrent static import is still fetching it off-thread. The target is large +// so its transpile reliably overlaps the tiny CommonJS sibling's require(). function makeTarget() { let src = ""; for (let i = 0; i < 1000; i++) src += `export const v${i} = ${i};\n`; @@ -36,10 +31,8 @@ const files = { `, }; -// Several independent attempts: without the fix the race reports the false -// "async module" error on essentially every run of a debug build, so any one -// attempt failing is enough. Each attempt is its own test so a single spawn -// stays well under the default timeout. +// Repeat as independent tests; without the fix the race throws the false +// "async module" error on essentially every debug run, so any attempt suffices. for (let attempt = 0; attempt < 4; attempt++) { test(`require(esm) racing a concurrent static import loads synchronously (attempt ${attempt})`, async () => { using dir = tempDir("issue-33180", files); From 4a6c22feebb48a53537f13b8e0ac82c4d057c7f7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 1 Jul 2026 13:09:01 +0000 Subject: [PATCH 3/5] test: run the independent spawn attempts concurrently --- test/regression/issue/33180.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/regression/issue/33180.test.ts b/test/regression/issue/33180.test.ts index fe147ce75802..f4fdc6953e7a 100644 --- a/test/regression/issue/33180.test.ts +++ b/test/regression/issue/33180.test.ts @@ -31,10 +31,11 @@ const files = { `, }; -// Repeat as independent tests; without the fix the race throws the false -// "async module" error on essentially every debug run, so any attempt suffices. +// Independent attempts (separate temp dirs + child processes, nothing shared); +// without the fix the race throws the false "async module" error on essentially +// every debug run, so any attempt suffices. for (let attempt = 0; attempt < 4; attempt++) { - test(`require(esm) racing a concurrent static import loads synchronously (attempt ${attempt})`, async () => { + test.concurrent(`require(esm) racing a concurrent static import loads synchronously (attempt ${attempt})`, async () => { using dir = tempDir("issue-33180", files); await using proc = Bun.spawn({ cmd: [bunExe(), "entry.mjs"], From a2be884ac9789dc2619c09b2991ad7c1a0d57b5c Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 1 Jul 2026 13:11:05 +0000 Subject: [PATCH 4/5] [autofix.ci] apply automated fixes --- test/regression/issue/33180.test.ts | 33 ++++++++++++++++------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/test/regression/issue/33180.test.ts b/test/regression/issue/33180.test.ts index f4fdc6953e7a..83e5d3beba00 100644 --- a/test/regression/issue/33180.test.ts +++ b/test/regression/issue/33180.test.ts @@ -35,19 +35,22 @@ const files = { // without the fix the race throws the false "async module" error on essentially // every debug run, so any attempt suffices. for (let attempt = 0; attempt < 4; attempt++) { - test.concurrent(`require(esm) racing a concurrent static import loads synchronously (attempt ${attempt})`, async () => { - using dir = tempDir("issue-33180", files); - await using proc = Bun.spawn({ - cmd: [bunExe(), "entry.mjs"], - env: bunEnv, - cwd: String(dir), - stderr: "pipe", - stdout: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stderr).not.toContain("async module"); - expect(stdout).toContain("required-ok"); - expect(stdout).toContain("entry-done"); - expect(exitCode).toBe(0); - }); + test.concurrent( + `require(esm) racing a concurrent static import loads synchronously (attempt ${attempt})`, + async () => { + using dir = tempDir("issue-33180", files); + await using proc = Bun.spawn({ + cmd: [bunExe(), "entry.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + stdout: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("async module"); + expect(stdout).toContain("required-ok"); + expect(stdout).toContain("entry-done"); + expect(exitCode).toBe(0); + }, + ); } From 8ff98bfe0542f88310ca4f14cbfa90b3781680bc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 1 Jul 2026 13:28:27 +0000 Subject: [PATCH 5/5] ci: retrigger