Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit b93bf7d

Browse filesBrowse files
guybedfordaduh95
authored andcommitted
esm: fix source phase identity bug in loadCache eviction
PR-URL: #62415 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 12b04d1 commit b93bf7d
Copy full SHA for b93bf7d

4 files changed

+42-4Lines changed: 42 additions & 4 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/internal/modules/esm/module_job.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/module_job.js
+11-4Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,9 @@ class ModuleJobBase {
145145
*/
146146
syncLink(requestType) {
147147
// Store itself into the cache first before linking in case there are circular
148-
// references in the linking.
148+
// references in the linking. Track whether we're overwriting an existing entry
149+
// so we know whether to remove the temporary entry in the finally block.
150+
const hadPreviousEntry = this.loader.loadCache.get(this.url, this.type) !== undefined;
149151
this.loader.loadCache.set(this.url, this.type, this);
150152
const moduleRequests = this.module.getModuleRequests();
151153
// Modules should be aligned with the moduleRequests array in order.
@@ -169,9 +171,14 @@ class ModuleJobBase {
169171
}
170172
this.module.link(modules);
171173
} finally {
172-
// Restore it - if it succeeds, we'll reset in the caller; Otherwise it's
173-
// not cached and if the error is caught, subsequent attempt would still fail.
174-
this.loader.loadCache.delete(this.url, this.type);
174+
if (!hadPreviousEntry) {
175+
// Remove the temporary entry. On failure this ensures subsequent attempts
176+
// don't return a broken job. On success the caller
177+
// (#getOrCreateModuleJobAfterResolve) will re-insert under the correct key.
178+
this.loader.loadCache.delete(this.url, this.type);
179+
}
180+
// If there was a previous entry (ensurePhase() path), leave this in cache -
181+
// it is the upgraded job and the caller will not re-insert.
175182
}
176183

177184
return evaluationDepJobs;
Collapse file
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Regression test for source phase import identity with mixed eval/source
2+
// phase imports of the same module in one parent.
3+
import '../common/index.mjs';
4+
import { spawnSyncAndAssert } from '../common/child_process.js';
5+
import * as fixtures from '../common/fixtures.mjs';
6+
7+
spawnSyncAndAssert(
8+
process.execPath,
9+
['--no-warnings', fixtures.path('es-modules/test-wasm-source-phase-identity.mjs')],
10+
{ stdout: '', stderr: '', trim: true }
11+
);
Collapse file
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import * as mod1 from './simple.wasm';
2+
import * as mod2 from './simple.wasm';
3+
import source mod3 from './simple.wasm';
4+
import source mod4 from './simple.wasm';
5+
6+
export { mod1, mod2, mod3, mod4 };
Collapse file
+14Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { strictEqual } from 'node:assert';
2+
3+
// Pre-load simple.wasm at kSourcePhase to prime the loadCache.
4+
const preloaded = await import.source('./simple.wasm');
5+
strictEqual(preloaded instanceof WebAssembly.Module, true);
6+
7+
// Import a parent that has both eval-phase and source-phase imports of the
8+
// same wasm file, which triggers ensurePhase(kEvaluationPhase) on the cached
9+
// job and exposes the loadCache eviction bug.
10+
const { mod1, mod2, mod3, mod4 } =
11+
await import('./test-wasm-source-phase-identity-parent.mjs');
12+
13+
strictEqual(mod1, mod2, 'two eval-phase imports of the same wasm must be identical');
14+
strictEqual(mod3, mod4, 'two source-phase imports of the same wasm must be identical');

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.