Skip to content

Commit

Permalink
loader: use default loader as cascaded loader in the in loader worker
Browse files Browse the repository at this point in the history
Use the default loader as the cascaded loader in the loader worker.
Otherwise we spawn loader workers in the loader workers indefinitely.

PR-URL: nodejs#47620
Fixes: nodejs#47566
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
joyeecheung authored and targos committed Nov 10, 2023
1 parent d8687c4 commit ee9cef2
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 10 deletions.
5 changes: 4 additions & 1 deletion lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,10 @@ port.on('message', (message) => {
if (manifestSrc) {
require('internal/process/policy').setup(manifestSrc, manifestURL);
}
setupUserModules();
const isLoaderWorker =
doEval === 'internal' &&
filename === require('internal/modules/esm/utils').loaderWorkerId;
setupUserModules(isLoaderWorker);

if (!hasStdin)
process.stdin.push(null);
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const {
} = require('internal/modules/esm/resolve');
const {
getDefaultConditions,
loaderWorkerId,
} = require('internal/modules/esm/utils');
const { deserializeError } = require('internal/error_serdes');
const {
Expand Down Expand Up @@ -490,7 +491,7 @@ class HooksProxy {
const lock = new SharedArrayBuffer(SHARED_MEMORY_BYTE_LENGTH);
this.#lock = new Int32Array(lock);

this.#worker = new InternalWorker('internal/modules/esm/worker', {
this.#worker = new InternalWorker(loaderWorkerId, {
stderr: false,
stdin: false,
stdout: false,
Expand Down
5 changes: 4 additions & 1 deletion lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,10 @@ let emittedExperimentalWarning = false;
* @returns {DefaultModuleLoader | CustomizedModuleLoader}
*/
function createModuleLoader(useCustomLoadersIfPresent = true) {
if (useCustomLoadersIfPresent) {
if (useCustomLoadersIfPresent &&
// Don't spawn a new worker if we're already in a worker thread created by instantiating CustomizedModuleLoader;
// doing so would cause an infinite loop.
!require('internal/modules/esm/utils').isLoaderWorker()) {
const userLoaderPaths = getOptionValue('--experimental-loader');
if (userLoaderPaths.length > 0) {
if (!emittedExperimentalWarning) {
Expand Down
12 changes: 11 additions & 1 deletion lib/internal/modules/esm/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,22 @@ async function importModuleDynamicallyCallback(wrap, specifier, assertions) {
throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING();
}

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

function isLoaderWorker() {
return _isLoaderWorker;
}

async function initializeHooks() {
const customLoaderPaths = getOptionValue('--experimental-loader');

Expand Down Expand Up @@ -165,4 +173,6 @@ module.exports = {
initializeHooks,
getDefaultConditions,
getConditionsSet,
loaderWorkerId: 'internal/modules/esm/worker',
isLoaderWorker,
};
3 changes: 1 addition & 2 deletions lib/internal/modules/esm/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const { receiveMessageOnPort } = require('internal/worker/io');
const {
WORKER_TO_MAIN_THREAD_NOTIFICATION,
} = require('internal/modules/esm/shared_constants');
const { initializeESM, initializeHooks } = require('internal/modules/esm/utils');
const { initializeHooks } = require('internal/modules/esm/utils');


function transferArrayBuffer(hasError, source) {
Expand Down Expand Up @@ -80,7 +80,6 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) {


try {
initializeESM();
const initResult = await initializeHooks();
hooks = initResult.hooks;
preloadScripts = initResult.preloadScripts;
Expand Down
8 changes: 4 additions & 4 deletions lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,9 @@ function setupSymbolDisposePolyfill() {
Symbol.asyncDispose ??= SymbolAsyncDispose;
}

function setupUserModules() {
function setupUserModules(isLoaderWorker = false) {
initializeCJSLoader();
initializeESMLoader();
initializeESMLoader(isLoaderWorker);
const CJSLoader = require('internal/modules/cjs/loader');
assert(!CJSLoader.hasLoadedAnyUserCJSModule);
loadPreloadModules();
Expand Down Expand Up @@ -546,9 +546,9 @@ function initializeCJSLoader() {
initializeCJS();
}

function initializeESMLoader() {
function initializeESMLoader(isLoaderWorker) {
const { initializeESM } = require('internal/modules/esm/utils');
initializeESM();
initializeESM(isLoaderWorker);

// Patch the vm module when --experimental-vm-modules is on.
// Please update the comments in vm.js when this block changes.
Expand Down
77 changes: 77 additions & 0 deletions test/es-module/test-loaders-workers-spawned.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { spawnPromisified } from '../common/index.mjs';
import * as fixtures from '../common/fixtures.mjs';
import assert from 'node:assert';
import { execPath } from 'node:process';
import { describe, it } from 'node:test';

describe('Worker threads do not spawn infinitely', { concurrency: true }, () => {
it('should not trigger an infinite loop when using a loader exports no recognized hooks', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
fixtures.fileURL('empty.js'),
'--eval',
'setTimeout(() => console.log("hello"),99)',
]);

assert.strictEqual(stderr, '');
assert.match(stdout, /^hello\r?\n$/);
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
});

it('should support a CommonJS entry point and a loader that imports a CommonJS module', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-loader',
fixtures.fileURL('es-module-loaders/loader-with-dep.mjs'),
fixtures.path('print-delayed.js'),
]);

assert.strictEqual(stderr, '');
assert.match(stdout, /^delayed\r?\n$/);
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
});

it('should support --require and --import along with using a loader written in CJS and CJS entry point', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--eval',
'setTimeout(() => console.log("D"),99)',
'--import',
fixtures.fileURL('printC.js'),
'--experimental-loader',
fixtures.fileURL('printB.js'),
'--require',
fixtures.path('printA.js'),
]);

assert.strictEqual(stderr, '');
// The worker code should always run before the --import, but the console.log might arrive late.
assert.match(stdout, /^A\r?\nA\r?\n(B\r?\nC|C\r?\nB)\r?\nD\r?\n$/);
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
});

it('should support --require and --import along with using a loader written in ESM and ESM entry point', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--require',
fixtures.path('printA.js'),
'--experimental-loader',
'data:text/javascript,console.log("B")',
'--import',
fixtures.fileURL('printC.js'),
'--input-type=module',
'--eval',
'setTimeout(() => console.log("D"),99)',
]);

assert.strictEqual(stderr, '');
// The worker code should always run before the --import, but the console.log might arrive late.
assert.match(stdout, /^A\r?\nA\r?\n(B\r?\nC|C\r?\nB)\r?\nD\r?\n$/);
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
});
});
3 changes: 3 additions & 0 deletions test/fixtures/print-delayed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
setTimeout(() => {
console.log('delayed');
}, 100);

0 comments on commit ee9cef2

Please sign in to comment.