diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 929577c0da6d08..22032f79e90d44 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -145,7 +145,9 @@ class ModuleJobBase { */ syncLink(requestType) { // Store itself into the cache first before linking in case there are circular - // references in the linking. + // references in the linking. Track whether we're overwriting an existing entry + // so we know whether to remove the temporary entry in the finally block. + const hadPreviousEntry = this.loader.loadCache.get(this.url, this.type) !== undefined; this.loader.loadCache.set(this.url, this.type, this); const moduleRequests = this.module.getModuleRequests(); // Modules should be aligned with the moduleRequests array in order. @@ -169,9 +171,14 @@ class ModuleJobBase { } this.module.link(modules); } finally { - // Restore it - if it succeeds, we'll reset in the caller; Otherwise it's - // not cached and if the error is caught, subsequent attempt would still fail. - this.loader.loadCache.delete(this.url, this.type); + if (!hadPreviousEntry) { + // Remove the temporary entry. On failure this ensures subsequent attempts + // don't return a broken job. On success the caller + // (#getOrCreateModuleJobAfterResolve) will re-insert under the correct key. + this.loader.loadCache.delete(this.url, this.type); + } + // If there was a previous entry (ensurePhase() path), leave this in cache - + // it is the upgraded job and the caller will not re-insert. } return evaluationDepJobs; diff --git a/test/es-module/test-esm-wasm-source-phase-identity.mjs b/test/es-module/test-esm-wasm-source-phase-identity.mjs new file mode 100644 index 00000000000000..2d742dfcff61ab --- /dev/null +++ b/test/es-module/test-esm-wasm-source-phase-identity.mjs @@ -0,0 +1,11 @@ +// Regression test for source phase import identity with mixed eval/source +// phase imports of the same module in one parent. +import '../common/index.mjs'; +import { spawnSyncAndAssert } from '../common/child_process.js'; +import * as fixtures from '../common/fixtures.mjs'; + +spawnSyncAndAssert( + process.execPath, + ['--no-warnings', fixtures.path('es-modules/test-wasm-source-phase-identity.mjs')], + { stdout: '', stderr: '', trim: true } +); diff --git a/test/fixtures/es-modules/test-wasm-source-phase-identity-parent.mjs b/test/fixtures/es-modules/test-wasm-source-phase-identity-parent.mjs new file mode 100644 index 00000000000000..36d5765c17f0ad --- /dev/null +++ b/test/fixtures/es-modules/test-wasm-source-phase-identity-parent.mjs @@ -0,0 +1,6 @@ +import * as mod1 from './simple.wasm'; +import * as mod2 from './simple.wasm'; +import source mod3 from './simple.wasm'; +import source mod4 from './simple.wasm'; + +export { mod1, mod2, mod3, mod4 }; diff --git a/test/fixtures/es-modules/test-wasm-source-phase-identity.mjs b/test/fixtures/es-modules/test-wasm-source-phase-identity.mjs new file mode 100644 index 00000000000000..84cf6261139038 --- /dev/null +++ b/test/fixtures/es-modules/test-wasm-source-phase-identity.mjs @@ -0,0 +1,14 @@ +import { strictEqual } from 'node:assert'; + +// Pre-load simple.wasm at kSourcePhase to prime the loadCache. +const preloaded = await import.source('./simple.wasm'); +strictEqual(preloaded instanceof WebAssembly.Module, true); + +// Import a parent that has both eval-phase and source-phase imports of the +// same wasm file, which triggers ensurePhase(kEvaluationPhase) on the cached +// job and exposes the loadCache eviction bug. +const { mod1, mod2, mod3, mod4 } = + await import('./test-wasm-source-phase-identity-parent.mjs'); + +strictEqual(mod1, mod2, 'two eval-phase imports of the same wasm must be identical'); +strictEqual(mod3, mod4, 'two source-phase imports of the same wasm must be identical');