esm: fix source phase identity bug in loadCache eviction

guybedford · aduh95 · commit 1cdd23c9f317 · 2026-03-30T17:20:28.000+02:00
PR-URL: #62415 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
diff --git 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
@@ -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
@@ -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
@@ -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');
