Skip to content

Commit b77a6b2

Browse files
joyeecheungtargos
authored andcommitted
loader: use default loader as cascaded loader in the in loader worker
Use the default loader as the cascaded loader in the loader worker. Otherwise we spawn loader workers in the loader workers indefinitely. PR-URL: #47620 Backport-PR-URL: #50669 Fixes: #47566 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
1 parent ca20f59 commit b77a6b2

File tree

8 files changed

+106
-10
lines changed

8 files changed

+106
-10
lines changed

lib/internal/main/worker_thread.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,10 @@ port.on('message', (message) => {
132132
if (manifestSrc) {
133133
require('internal/process/policy').setup(manifestSrc, manifestURL);
134134
}
135-
setupUserModules();
135+
const isLoaderWorker =
136+
doEval === 'internal' &&
137+
filename === require('internal/modules/esm/utils').loaderWorkerId;
138+
setupUserModules(isLoaderWorker);
136139

137140
if (!hasStdin)
138141
process.stdin.push(null);

lib/internal/modules/esm/hooks.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ const {
5050
} = require('internal/modules/esm/resolve');
5151
const {
5252
getDefaultConditions,
53+
loaderWorkerId,
5354
} = require('internal/modules/esm/utils');
5455
const { deserializeError } = require('internal/error_serdes');
5556
const {
@@ -490,7 +491,7 @@ class HooksProxy {
490491
const lock = new SharedArrayBuffer(SHARED_MEMORY_BYTE_LENGTH);
491492
this.#lock = new Int32Array(lock);
492493

493-
this.#worker = new InternalWorker('internal/modules/esm/worker', {
494+
this.#worker = new InternalWorker(loaderWorkerId, {
494495
stderr: false,
495496
stdin: false,
496497
stdout: false,

lib/internal/modules/esm/loader.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,10 @@ let emittedExperimentalWarning = false;
417417
* @returns {DefaultModuleLoader | CustomizedModuleLoader}
418418
*/
419419
function createModuleLoader(useCustomLoadersIfPresent = true) {
420-
if (useCustomLoadersIfPresent) {
420+
if (useCustomLoadersIfPresent &&
421+
// Don't spawn a new worker if we're already in a worker thread created by instantiating CustomizedModuleLoader;
422+
// doing so would cause an infinite loop.
423+
!require('internal/modules/esm/utils').isLoaderWorker()) {
421424
const userLoaderPaths = getOptionValue('--experimental-loader');
422425
if (userLoaderPaths.length > 0) {
423426
if (!emittedExperimentalWarning) {

lib/internal/modules/esm/utils.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,14 +91,22 @@ async function importModuleDynamicallyCallback(wrap, specifier, assertions) {
9191
throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING();
9292
}
9393

94-
function initializeESM() {
94+
// This is configured during pre-execution. Specifically it's set to true for
95+
// the loader worker in internal/main/worker_thread.js.
96+
let _isLoaderWorker = false;
97+
function initializeESM(isLoaderWorker = false) {
98+
_isLoaderWorker = isLoaderWorker;
9599
initializeDefaultConditions();
96100
// Setup per-isolate callbacks that locate data or callbacks that we keep
97101
// track of for different ESM modules.
98102
setInitializeImportMetaObjectCallback(initializeImportMetaObject);
99103
setImportModuleDynamicallyCallback(importModuleDynamicallyCallback);
100104
}
101105

106+
function isLoaderWorker() {
107+
return _isLoaderWorker;
108+
}
109+
102110
async function initializeHooks() {
103111
const customLoaderPaths = getOptionValue('--experimental-loader');
104112

@@ -165,4 +173,6 @@ module.exports = {
165173
initializeHooks,
166174
getDefaultConditions,
167175
getConditionsSet,
176+
loaderWorkerId: 'internal/modules/esm/worker',
177+
isLoaderWorker,
168178
};

lib/internal/modules/esm/worker.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const { receiveMessageOnPort } = require('internal/worker/io');
2929
const {
3030
WORKER_TO_MAIN_THREAD_NOTIFICATION,
3131
} = require('internal/modules/esm/shared_constants');
32-
const { initializeESM, initializeHooks } = require('internal/modules/esm/utils');
32+
const { initializeHooks } = require('internal/modules/esm/utils');
3333

3434

3535
function transferArrayBuffer(hasError, source) {
@@ -80,7 +80,6 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) {
8080

8181

8282
try {
83-
initializeESM();
8483
const initResult = await initializeHooks();
8584
hooks = initResult.hooks;
8685
preloadScripts = initResult.preloadScripts;

lib/internal/process/pre_execution.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,9 @@ function setupSymbolDisposePolyfill() {
123123
Symbol.asyncDispose ??= SymbolAsyncDispose;
124124
}
125125

126-
function setupUserModules() {
126+
function setupUserModules(isLoaderWorker = false) {
127127
initializeCJSLoader();
128-
initializeESMLoader();
128+
initializeESMLoader(isLoaderWorker);
129129
const CJSLoader = require('internal/modules/cjs/loader');
130130
assert(!CJSLoader.hasLoadedAnyUserCJSModule);
131131
loadPreloadModules();
@@ -546,9 +546,9 @@ function initializeCJSLoader() {
546546
initializeCJS();
547547
}
548548

549-
function initializeESMLoader() {
549+
function initializeESMLoader(isLoaderWorker) {
550550
const { initializeESM } = require('internal/modules/esm/utils');
551-
initializeESM();
551+
initializeESM(isLoaderWorker);
552552

553553
// Patch the vm module when --experimental-vm-modules is on.
554554
// Please update the comments in vm.js when this block changes.
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import { spawnPromisified } from '../common/index.mjs';
2+
import * as fixtures from '../common/fixtures.mjs';
3+
import assert from 'node:assert';
4+
import { execPath } from 'node:process';
5+
import { describe, it } from 'node:test';
6+
7+
describe('Worker threads do not spawn infinitely', { concurrency: true }, () => {
8+
it('should not trigger an infinite loop when using a loader exports no recognized hooks', async () => {
9+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
10+
'--no-warnings',
11+
'--experimental-loader',
12+
fixtures.fileURL('empty.js'),
13+
'--eval',
14+
'setTimeout(() => console.log("hello"),99)',
15+
]);
16+
17+
assert.strictEqual(stderr, '');
18+
assert.match(stdout, /^hello\r?\n$/);
19+
assert.strictEqual(code, 0);
20+
assert.strictEqual(signal, null);
21+
});
22+
23+
it('should support a CommonJS entry point and a loader that imports a CommonJS module', async () => {
24+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
25+
'--no-warnings',
26+
'--experimental-loader',
27+
fixtures.fileURL('es-module-loaders/loader-with-dep.mjs'),
28+
fixtures.path('print-delayed.js'),
29+
]);
30+
31+
assert.strictEqual(stderr, '');
32+
assert.match(stdout, /^delayed\r?\n$/);
33+
assert.strictEqual(code, 0);
34+
assert.strictEqual(signal, null);
35+
});
36+
37+
it('should support --require and --import along with using a loader written in CJS and CJS entry point', async () => {
38+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
39+
'--no-warnings',
40+
'--eval',
41+
'setTimeout(() => console.log("D"),99)',
42+
'--import',
43+
fixtures.fileURL('printC.js'),
44+
'--experimental-loader',
45+
fixtures.fileURL('printB.js'),
46+
'--require',
47+
fixtures.path('printA.js'),
48+
]);
49+
50+
assert.strictEqual(stderr, '');
51+
// The worker code should always run before the --import, but the console.log might arrive late.
52+
assert.match(stdout, /^A\r?\nA\r?\n(B\r?\nC|C\r?\nB)\r?\nD\r?\n$/);
53+
assert.strictEqual(code, 0);
54+
assert.strictEqual(signal, null);
55+
});
56+
57+
it('should support --require and --import along with using a loader written in ESM and ESM entry point', async () => {
58+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
59+
'--no-warnings',
60+
'--require',
61+
fixtures.path('printA.js'),
62+
'--experimental-loader',
63+
'data:text/javascript,console.log("B")',
64+
'--import',
65+
fixtures.fileURL('printC.js'),
66+
'--input-type=module',
67+
'--eval',
68+
'setTimeout(() => console.log("D"),99)',
69+
]);
70+
71+
assert.strictEqual(stderr, '');
72+
// The worker code should always run before the --import, but the console.log might arrive late.
73+
assert.match(stdout, /^A\r?\nA\r?\n(B\r?\nC|C\r?\nB)\r?\nD\r?\n$/);
74+
assert.strictEqual(code, 0);
75+
assert.strictEqual(signal, null);
76+
});
77+
});

test/fixtures/print-delayed.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
setTimeout(() => {
2+
console.log('delayed');
3+
}, 100);

0 commit comments

Comments
 (0)