From 4b9cdea8a6262d8f4d4174f951015b27985ddb5c Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 3 Jun 2024 00:10:34 +0200 Subject: [PATCH] Revert "module: have a single hooks thread for all workers" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 22cb99d073b03414e33843550cca3bac2833a361. PR-URL: https://github.com/nodejs/node/pull/53183 Reviewed-By: Geoffrey Booth Reviewed-By: James M Snell Reviewed-By: Gerhard Stöbich --- lib/internal/main/worker_thread.js | 2 - lib/internal/modules/esm/hooks.js | 127 ++++++------------ lib/internal/modules/esm/loader.js | 19 +-- lib/internal/modules/esm/worker.js | 74 ++-------- lib/internal/worker.js | 32 +---- src/handle_wrap.cc | 3 +- test/common/index.mjs | 2 - test/es-module/test-esm-loader-mock.mjs | 7 +- test/es-module/test-esm-loader-threads.mjs | 74 ---------- test/es-module/test-esm-named-exports.js | 4 +- test/es-module/test-esm-named-exports.mjs | 9 +- test/es-module/test-esm-virtual-json.mjs | 3 +- .../builtin-named-exports.mjs | 13 +- .../es-module-loaders/hooks-exit-worker.mjs | 21 --- test/fixtures/es-module-loaders/hooks-log.mjs | 19 --- .../not-found-assert-loader.mjs | 17 ++- .../es-module-loaders/worker-fail-on-load.mjs | 1 - .../worker-fail-on-resolve.mjs | 1 - .../es-module-loaders/worker-log-again.mjs | 3 - .../worker-log-fail-worker-load.mjs | 12 -- .../worker-log-fail-worker-resolve.mjs | 12 -- .../fixtures/es-module-loaders/worker-log.mjs | 9 -- .../es-module-loaders/workers-spawned.mjs | 7 - 23 files changed, 79 insertions(+), 392 deletions(-) delete mode 100644 test/es-module/test-esm-loader-threads.mjs delete mode 100644 test/fixtures/es-module-loaders/hooks-exit-worker.mjs delete mode 100644 test/fixtures/es-module-loaders/hooks-log.mjs delete mode 100644 test/fixtures/es-module-loaders/worker-fail-on-load.mjs delete mode 100644 test/fixtures/es-module-loaders/worker-fail-on-resolve.mjs delete mode 100644 test/fixtures/es-module-loaders/worker-log-again.mjs delete mode 100644 test/fixtures/es-module-loaders/worker-log-fail-worker-load.mjs delete mode 100644 test/fixtures/es-module-loaders/worker-log-fail-worker-resolve.mjs delete mode 100644 test/fixtures/es-module-loaders/worker-log.mjs delete mode 100644 test/fixtures/es-module-loaders/workers-spawned.mjs diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index 32bccca9b53a72..aa329b9fe04f15 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -95,7 +95,6 @@ port.on('message', (message) => { filename, hasStdin, publicPort, - hooksPort, workerData, } = message; @@ -110,7 +109,6 @@ port.on('message', (message) => { } require('internal/worker').assignEnvironmentData(environmentData); - require('internal/worker').hooksPort = hooksPort; if (SharedArrayBuffer !== undefined) { // The counter is only passed to the workers created by the main thread, diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index f5833ad61cdb75..ba655116a0bb57 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -35,7 +35,7 @@ const { const { exitCodes: { kUnsettledTopLevelAwait } } = internalBinding('errors'); const { URL } = require('internal/url'); const { canParse: URLCanParse } = internalBinding('url'); -const { receiveMessageOnPort, isMainThread } = require('worker_threads'); +const { receiveMessageOnPort } = require('worker_threads'); const { isAnyArrayBuffer, isArrayBufferView, @@ -482,8 +482,6 @@ class HooksProxy { */ #worker; - #portToHooksThread; - /** * The last notification ID received from the worker. This is used to detect * if the worker has already sent a notification before putting the main @@ -501,38 +499,26 @@ class HooksProxy { #isReady = false; constructor() { - const { InternalWorker, hooksPort } = require('internal/worker'); + const { InternalWorker } = require('internal/worker'); + MessageChannel ??= require('internal/worker/io').MessageChannel; + const lock = new SharedArrayBuffer(SHARED_MEMORY_BYTE_LENGTH); this.#lock = new Int32Array(lock); - if (isMainThread) { - // Main thread is the only one that creates the internal single hooks worker - this.#worker = new InternalWorker(loaderWorkerId, { - stderr: false, - stdin: false, - stdout: false, - trackUnmanagedFds: false, - workerData: { - lock, - }, - }); - this.#worker.unref(); // ! Allows the process to eventually exit. - this.#worker.on('exit', process.exit); - this.#portToHooksThread = this.#worker; - } else { - this.#portToHooksThread = hooksPort; - } + this.#worker = new InternalWorker(loaderWorkerId, { + stderr: false, + stdin: false, + stdout: false, + trackUnmanagedFds: false, + workerData: { + lock, + }, + }); + this.#worker.unref(); // ! Allows the process to eventually exit. + this.#worker.on('exit', process.exit); } waitForWorker() { - // There is one Hooks instance for each worker thread. But only one of these Hooks instances - // has an InternalWorker. That was the Hooks instance created for the main thread. - // It means for all Hooks instances that are not on the main thread => they are ready because they - // delegate to the single InternalWorker anyway. - if (!isMainThread) { - return; - } - if (!this.#isReady) { const { kIsOnline } = require('internal/worker'); if (!this.#worker[kIsOnline]) { @@ -549,37 +535,6 @@ class HooksProxy { } } - #postMessageToWorker(method, type, transferList, args) { - this.waitForWorker(); - - MessageChannel ??= require('internal/worker/io').MessageChannel; - - const { - port1: fromHooksThread, - port2: toHooksThread, - } = new MessageChannel(); - - // Pass work to the worker. - debug(`post ${type} message to worker`, { method, args, transferList }); - const usedTransferList = [toHooksThread]; - if (transferList) { - ArrayPrototypePushApply(usedTransferList, transferList); - } - - this.#portToHooksThread.postMessage( - { - __proto__: null, - args, - lock: this.#lock, - method, - port: toHooksThread, - }, - usedTransferList, - ); - - return fromHooksThread; - } - /** * Invoke a remote method asynchronously. * @param {string} method Method to invoke @@ -588,7 +543,22 @@ class HooksProxy { * @returns {Promise} */ async makeAsyncRequest(method, transferList, ...args) { - const fromHooksThread = this.#postMessageToWorker(method, 'Async', transferList, args); + this.waitForWorker(); + + MessageChannel ??= require('internal/worker/io').MessageChannel; + const asyncCommChannel = new MessageChannel(); + + // Pass work to the worker. + debug('post async message to worker', { method, args, transferList }); + const finalTransferList = [asyncCommChannel.port2]; + if (transferList) { + ArrayPrototypePushApply(finalTransferList, transferList); + } + this.#worker.postMessage({ + __proto__: null, + method, args, + port: asyncCommChannel.port2, + }, finalTransferList); if (this.#numberOfPendingAsyncResponses++ === 0) { // On the next lines, the main thread will await a response from the worker thread that might @@ -597,11 +567,7 @@ class HooksProxy { // However we want to keep the process alive until the worker thread responds (or until the // event loop of the worker thread is also empty), so we ref the worker until we get all the // responses back. - if (this.#worker) { - this.#worker.ref(); - } else { - this.#portToHooksThread.ref(); - } + this.#worker.ref(); } let response; @@ -610,26 +576,18 @@ class HooksProxy { await AtomicsWaitAsync(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, this.#workerNotificationLastId).value; this.#workerNotificationLastId = AtomicsLoad(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION); - response = receiveMessageOnPort(fromHooksThread); + response = receiveMessageOnPort(asyncCommChannel.port1); } while (response == null); debug('got async response from worker', { method, args }, this.#lock); if (--this.#numberOfPendingAsyncResponses === 0) { // We got all the responses from the worker, its job is done (until next time). - if (this.#worker) { - this.#worker.unref(); - } else { - this.#portToHooksThread.unref(); - } - } - - if (response.message.status === 'exit') { - process.exit(response.message.body); + this.#worker.unref(); } - fromHooksThread.close(); - - return this.#unwrapMessage(response); + const body = this.#unwrapMessage(response); + asyncCommChannel.port1.close(); + return body; } /** @@ -640,7 +598,11 @@ class HooksProxy { * @returns {any} */ makeSyncRequest(method, transferList, ...args) { - const fromHooksThread = this.#postMessageToWorker(method, 'Sync', transferList, args); + this.waitForWorker(); + + // Pass work to the worker. + debug('post sync message to worker', { method, args, transferList }); + this.#worker.postMessage({ __proto__: null, method, args }, transferList); let response; do { @@ -649,7 +611,7 @@ class HooksProxy { AtomicsWait(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, this.#workerNotificationLastId); this.#workerNotificationLastId = AtomicsLoad(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION); - response = receiveMessageOnPort(fromHooksThread); + response = this.#worker.receiveMessageSync(); } while (response == null); debug('got sync response from worker', { method, args }); if (response.message.status === 'never-settle') { @@ -657,9 +619,6 @@ class HooksProxy { } else if (response.message.status === 'exit') { process.exit(response.message.body); } - - fromHooksThread.close(); - return this.#unwrapMessage(response); } diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index e060b36eccacab..afb0e9fd9ec1c6 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -41,7 +41,6 @@ const { ModuleWrap, kEvaluating, kEvaluated } = internalBinding('module_wrap'); const { urlToFilename, } = require('internal/modules/helpers'); -const { isMainThread } = require('worker_threads'); let defaultResolve, defaultLoad, defaultLoadSync, importMetaInitializer; /** @@ -608,11 +607,10 @@ class CustomizedModuleLoader { */ constructor() { getHooksProxy(); - _hasCustomizations = true; } /** - * Register a loader specifier. + * Register some loader specifier. * @param {string} originalSpecifier The specified URL path of the loader to * be registered. * @param {string} parentURL The parent URL from where the loader will be @@ -620,14 +618,10 @@ class CustomizedModuleLoader { * @param {any} [data] Arbitrary data to be passed from the custom loader * (user-land) to the worker. * @param {any[]} [transferList] Objects in `data` that are changing ownership - * @returns {{ format: string, url: URL['href'] } | undefined} + * @returns {{ format: string, url: URL['href'] }} */ register(originalSpecifier, parentURL, data, transferList) { - if (isMainThread) { - // Only the main thread has a Hooks instance with worker thread. All other Worker threads - // delegate their hooks to the HooksThread of the main thread. - return hooksProxy.makeSyncRequest('register', transferList, originalSpecifier, parentURL, data); - } + return hooksProxy.makeSyncRequest('register', transferList, originalSpecifier, parentURL, data); } /** @@ -725,12 +719,6 @@ function getHooksProxy() { return hooksProxy; } -let _hasCustomizations = false; -function hasCustomizations() { - return _hasCustomizations; -} - - let cascadedLoader; /** @@ -792,7 +780,6 @@ function register(specifier, parentURL = undefined, options) { module.exports = { createModuleLoader, - hasCustomizations, getHooksProxy, getOrInitializeCascadedLoader, register, diff --git a/lib/internal/modules/esm/worker.js b/lib/internal/modules/esm/worker.js index 088667f3c0d5d7..311d77fb099384 100644 --- a/lib/internal/modules/esm/worker.js +++ b/lib/internal/modules/esm/worker.js @@ -1,8 +1,6 @@ 'use strict'; const { - ArrayPrototypeFilter, - ArrayPrototypePush, AtomicsAdd, AtomicsNotify, DataViewPrototypeGetBuffer, @@ -99,21 +97,7 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) { // so it can detect the exit event. const { exit } = process; process.exit = function(code) { - const exitMsg = wrapMessage('exit', code ?? process.exitCode); - if (hooks) { - for (let i = 0; i < allThreadRegisteredHandlerPorts.length; i++) { - const { port: registeredPort } = allThreadRegisteredHandlerPorts[i]; - registeredPort.postMessage(exitMsg); - } - - for (const { port, lock: operationLock } of unsettledResponsePorts) { - port.postMessage(exitMsg); - // Wake all threads that have pending operations. - AtomicsAdd(operationLock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 1); - AtomicsNotify(operationLock, WORKER_TO_MAIN_THREAD_NOTIFICATION); - } - } - syncCommPort.postMessage(exitMsg); + syncCommPort.postMessage(wrapMessage('exit', code ?? process.exitCode)); AtomicsAdd(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 1); AtomicsNotify(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION); return ReflectApply(exit, this, arguments); @@ -161,11 +145,8 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) { const unsettledResponsePorts = new SafeSet(); process.on('beforeExit', () => { - for (const { port, lock: operationLock } of unsettledResponsePorts) { + for (const port of unsettledResponsePorts) { port.postMessage(wrapMessage('never-settle')); - // Wake all threads that have pending operations. - AtomicsAdd(operationLock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 1); - AtomicsNotify(operationLock, WORKER_TO_MAIN_THREAD_NOTIFICATION); } unsettledResponsePorts.clear(); @@ -183,59 +164,24 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) { setImmediate(() => {}); }); - let allThreadRegisteredHandlerPorts = []; - /** - * @callback registerHandler - * @param {MessagePort} toWorkerThread - Upon Worker creation a message channel between the new Worker - * and the Hooks thread is being initialized. This is the MessagePort that the Hooks thread will use post - * messages to the worker. The other MessagePort is passed to the new Worker itself via LOAD_SCRIPT message - */ - function registerHandler(toWorkerThread, registeredThreadId) { - toWorkerThread.on('message', handleMessage); - ArrayPrototypePush(allThreadRegisteredHandlerPorts, { port: toWorkerThread, registeredThreadId }); - } - - /** - * @callback registerHandler - * @param {number} unregisteredThreadId - the thread id of the worker thread that is being unregistered - * from the Hooks Thread - */ - function unregisterHandler(unregisteredThreadId) { - allThreadRegisteredHandlerPorts = ArrayPrototypeFilter( - allThreadRegisteredHandlerPorts, (el) => el.registeredThreadId !== unregisteredThreadId); - } - - function getMessageHandler(method) { - if (method === '#registerWorkerClient') { - return registerHandler; - } - if (method === '#unregisterWorkerClient') { - return unregisterHandler; - } - return hooks[method]; - } - /** * Handles incoming messages from the main thread or other workers. * @param {object} options - The options object. * @param {string} options.method - The name of the hook. * @param {Array} options.args - The arguments to pass to the method. * @param {MessagePort} options.port - The message port to use for communication. - * @param {Int32Array} options.lock - The shared memory where the caller expects to get awaken. */ - async function handleMessage({ method, args, port, lock: msgLock }) { + async function handleMessage({ method, args, port }) { // Each potential exception needs to be caught individually so that the correct error is sent to // the main thread. let hasError = false; let shouldRemoveGlobalErrorHandler = false; - const messageHandler = getMessageHandler(method); - assert(typeof messageHandler === 'function'); + assert(typeof hooks[method] === 'function'); if (port == null && !hasUncaughtExceptionCaptureCallback()) { // When receiving sync messages, we want to unlock the main thread when there's an exception. process.on('uncaughtException', errorHandler); shouldRemoveGlobalErrorHandler = true; } - const usedLock = msgLock ?? lock; // We are about to yield the execution with `await ReflectApply` below. In case the code // following the `await` never runs, we remove the message handler so the `beforeExit` event @@ -246,19 +192,17 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) { clearImmediate(immediate); immediate = setImmediate(checkForMessages).unref(); - const unsettledActionData = { port: port ?? syncCommPort, lock: usedLock }; - - unsettledResponsePorts.add(unsettledActionData); + unsettledResponsePorts.add(port ?? syncCommPort); let response; try { - response = await ReflectApply(messageHandler, hooks, args); + response = await ReflectApply(hooks[method], hooks, args); } catch (exception) { hasError = true; response = exception; } - unsettledResponsePorts.delete(unsettledActionData); + unsettledResponsePorts.delete(port ?? syncCommPort); // Send the method response (or exception) to the main thread. try { @@ -271,8 +215,8 @@ async function customizedModuleWorker(lock, syncCommPort, errorHandler) { (port ?? syncCommPort).postMessage(wrapMessage('error', exception)); } - AtomicsAdd(usedLock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 1); - AtomicsNotify(usedLock, WORKER_TO_MAIN_THREAD_NOTIFICATION); + AtomicsAdd(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 1); + AtomicsNotify(lock, WORKER_TO_MAIN_THREAD_NOTIFICATION); if (shouldRemoveGlobalErrorHandler) { process.off('uncaughtException', errorHandler); } diff --git a/lib/internal/worker.js b/lib/internal/worker.js index 766659d4464379..9eefaa97021756 100644 --- a/lib/internal/worker.js +++ b/lib/internal/worker.js @@ -132,7 +132,7 @@ class Worker extends EventEmitter { constructor(filename, options = kEmptyObject) { throwIfBuildingSnapshot('Creating workers'); super(); - const isInternal = this.#isInternal = arguments[2] === kIsInternal; + const isInternal = arguments[2] === kIsInternal; debug( `[${threadId}] create new worker`, filename, @@ -258,15 +258,6 @@ class Worker extends EventEmitter { ...new SafeArrayIterator(options.transferList)); this[kPublicPort] = port1; - const { - port1: toWorkerThread, - port2: toHooksThread, - } = new MessageChannel(); - if (!isInternal) { - // This is not an internal hooks thread => it needs a channel to the hooks thread: - // - send it one side of a channel here - ArrayPrototypePush(transferList, toHooksThread); - } ArrayPrototypeForEach(['message', 'messageerror'], (event) => { this[kPublicPort].on(event, (message) => this.emit(event, message)); }); @@ -281,20 +272,8 @@ class Worker extends EventEmitter { workerData: options.workerData, environmentData, publicPort: port2, - hooksPort: !isInternal ? toHooksThread : undefined, hasStdin: !!options.stdin, }, transferList); - - const loaderModule = require('internal/modules/esm/loader'); - const hasCustomizations = loaderModule.hasCustomizations(); - - if (!isInternal && hasCustomizations) { - // - send the second side of the channel to the hooks thread, - // also announce the threadId of the Worker that will use that port. - // This is needed for the cleanup stage - loaderModule.getHooksProxy().makeSyncRequest( - '#registerWorkerClient', [toWorkerThread], toWorkerThread, this.threadId); - } // Use this to cache the Worker's loopStart value once available. this[kLoopStartTime] = -1; this[kIsOnline] = false; @@ -314,12 +293,6 @@ class Worker extends EventEmitter { [kOnExit](code, customErr, customErrReason) { debug(`[${threadId}] hears end event for Worker ${this.threadId}`); - const loaderModule = require('internal/modules/esm/loader'); - const hasCustomizations = loaderModule.hasCustomizations(); - - if (!this.#isInternal && hasCustomizations) { - loaderModule.getHooksProxy()?.makeAsyncRequest('#unregisterWorkerClient', undefined, this.threadId); - } drainMessagePort(this[kPublicPort]); drainMessagePort(this[kPort]); this.removeAllListeners('message'); @@ -462,8 +435,6 @@ class Worker extends EventEmitter { return makeResourceLimits(this[kHandle].getResourceLimits()); } - #isInternal = false; - getHeapSnapshot(options) { const { HeapSnapshotStream, @@ -561,7 +532,6 @@ module.exports = { kIsOnline, isMainThread, SHARE_ENV, - hooksPort: undefined, resourceLimits: !isMainThread ? makeResourceLimits(resourceLimitsRaw) : {}, setEnvironmentData, diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc index 1fbe0178c7a10d..f651b81ca8ce6b 100644 --- a/src/handle_wrap.cc +++ b/src/handle_wrap.cc @@ -148,8 +148,7 @@ void HandleWrap::OnClose(uv_handle_t* handle) { wrap->OnClose(); wrap->handle_wrap_queue_.Remove(); - if (!env->isolate()->IsExecutionTerminating() && - !wrap->persistent().IsEmpty() && + if (!wrap->persistent().IsEmpty() && wrap->object() ->Has(env->context(), env->handle_onclose_symbol()) .FromMaybe(false)) { diff --git a/test/common/index.mjs b/test/common/index.mjs index 6b566a899a35bb..430527faf8f0ea 100644 --- a/test/common/index.mjs +++ b/test/common/index.mjs @@ -51,7 +51,6 @@ const { skipIfDumbTerminal, skipIfEslintMissing, skipIfInspectorDisabled, - skipIfWorker, spawnPromisified, } = common; @@ -107,6 +106,5 @@ export { skipIfDumbTerminal, skipIfEslintMissing, skipIfInspectorDisabled, - skipIfWorker, spawnPromisified, }; diff --git a/test/es-module/test-esm-loader-mock.mjs b/test/es-module/test-esm-loader-mock.mjs index 0d39f549581a54..164d0ac3775039 100644 --- a/test/es-module/test-esm-loader-mock.mjs +++ b/test/es-module/test-esm-loader-mock.mjs @@ -1,11 +1,6 @@ -import { skipIfWorker } from '../common/index.mjs'; +import '../common/index.mjs'; import assert from 'node:assert/strict'; import { mock } from '../fixtures/es-module-loaders/mock.mjs'; -// Importing mock.mjs above will call `register` to modify the loaders chain. -// Modifying the loader chain is not supported currently when running from a worker thread. -// Relevant PR: https://github.com/nodejs/node/pull/52706 -// See comment: https://github.com/nodejs/node/pull/52706/files#r1585144580 -skipIfWorker(); mock('node:events', { EventEmitter: 'This is mocked!' diff --git a/test/es-module/test-esm-loader-threads.mjs b/test/es-module/test-esm-loader-threads.mjs deleted file mode 100644 index 7310a9ac5b54ac..00000000000000 --- a/test/es-module/test-esm-loader-threads.mjs +++ /dev/null @@ -1,74 +0,0 @@ -import { spawnPromisified } from '../common/index.mjs'; -import * as fixtures from '../common/fixtures.mjs'; -import { strictEqual } from 'node:assert'; -import { execPath } from 'node:process'; -import { describe, it } from 'node:test'; - -describe('off-thread hooks', { concurrency: true }, () => { - it('uses only one hooks thread to support multiple application threads', async () => { - const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ - '--no-warnings', - '--import', - `data:text/javascript,${encodeURIComponent(` - import { register } from 'node:module'; - register(${JSON.stringify(fixtures.fileURL('es-module-loaders/hooks-log.mjs'))}); - `)}`, - fixtures.path('es-module-loaders/workers-spawned.mjs'), - ]); - - strictEqual(stderr, ''); - strictEqual(stdout.split('\n').filter((line) => line.startsWith('initialize')).length, 1); - strictEqual(stdout.split('\n').filter((line) => line === 'foo').length, 2); - strictEqual(stdout.split('\n').filter((line) => line === 'bar').length, 4); - // Calls to resolve/load: - // 1x main script: test/fixtures/es-module-loaders/workers-spawned.mjs - // 3x worker_threads - // => 1x test/fixtures/es-module-loaders/worker-log.mjs - // 2x test/fixtures/es-module-loaders/worker-log-again.mjs => once per worker-log.mjs Worker instance - // 2x test/fixtures/es-module-loaders/worker-log.mjs => once per worker-log.mjs Worker instance - // 4x test/fixtures/es-module-loaders/worker-log-again.mjs => 2x for each worker-log - // 6x module-named-exports.mjs => 2x worker-log.mjs + 4x worker-log-again.mjs - // =========================== - // 16 calls to resolve + 16 calls to load hook for the registered custom loader - strictEqual(stdout.split('\n').filter((line) => line.startsWith('hooked resolve')).length, 16); - strictEqual(stdout.split('\n').filter((line) => line.startsWith('hooked load')).length, 16); - strictEqual(code, 0); - strictEqual(signal, null); - }); - - it('propagates the exit code from worker thread import exiting from resolve hook', async () => { - const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ - '--no-warnings', - '--import', - `data:text/javascript,${encodeURIComponent(` - import { register } from 'node:module'; - register(${JSON.stringify(fixtures.fileURL('es-module-loaders/hooks-exit-worker.mjs'))}); - `)}`, - fixtures.path('es-module-loaders/worker-log-fail-worker-resolve.mjs'), - ]); - - strictEqual(stderr, ''); - strictEqual(stdout.split('\n').filter((line) => line.startsWith('resolve process-exit-module-resolve')).length, 1); - strictEqual(code, 42); - strictEqual(signal, null); - }); - - it('propagates the exit code from worker thread import exiting from load hook', async () => { - const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ - '--no-warnings', - '--import', - `data:text/javascript,${encodeURIComponent(` - import { register } from 'node:module'; - register(${JSON.stringify(fixtures.fileURL('es-module-loaders/hooks-exit-worker.mjs'))}); - `)}`, - fixtures.path('es-module-loaders/worker-log-fail-worker-load.mjs'), - ]); - - strictEqual(stderr, ''); - strictEqual(stdout.split('\n').filter((line) => line.startsWith('resolve process-exit-module-load')).length, 1); - strictEqual(stdout.split('\n').filter((line) => line.startsWith('load process-exit-on-load:///')).length, 1); - strictEqual(code, 43); - strictEqual(signal, null); - }); - -}); diff --git a/test/es-module/test-esm-named-exports.js b/test/es-module/test-esm-named-exports.js index 00b7aebbfd1f46..2c6f67288aa57c 100644 --- a/test/es-module/test-esm-named-exports.js +++ b/test/es-module/test-esm-named-exports.js @@ -1,9 +1,7 @@ // Flags: --import ./test/fixtures/es-module-loaders/builtin-named-exports.mjs 'use strict'; -const common = require('../common'); -common.skipIfWorker(); - +require('../common'); const { readFile, __fromLoader } = require('fs'); const assert = require('assert'); diff --git a/test/es-module/test-esm-named-exports.mjs b/test/es-module/test-esm-named-exports.mjs index 6e584b05aa204f..bbe9c96b92d9b8 100644 --- a/test/es-module/test-esm-named-exports.mjs +++ b/test/es-module/test-esm-named-exports.mjs @@ -1,10 +1,9 @@ // Flags: --import ./test/fixtures/es-module-loaders/builtin-named-exports.mjs -import { skipIfWorker } from '../common/index.mjs'; -import * as fs from 'fs'; +import '../common/index.mjs'; +import { readFile, __fromLoader } from 'fs'; import assert from 'assert'; import ok from '../fixtures/es-modules/test-esm-ok.mjs'; -skipIfWorker(); assert(ok); -assert(fs.readFile); -assert(fs.__fromLoader); +assert(readFile); +assert(__fromLoader); diff --git a/test/es-module/test-esm-virtual-json.mjs b/test/es-module/test-esm-virtual-json.mjs index 1064a6af5026cf..a42b037fc1f200 100644 --- a/test/es-module/test-esm-virtual-json.mjs +++ b/test/es-module/test-esm-virtual-json.mjs @@ -1,8 +1,7 @@ -import { skipIfWorker } from '../common/index.mjs'; +import '../common/index.mjs'; import * as fixtures from '../common/fixtures.mjs'; import { register } from 'node:module'; import assert from 'node:assert'; -skipIfWorker(); async function resolve(referrer, context, next) { const result = await next(referrer, context); diff --git a/test/fixtures/es-module-loaders/builtin-named-exports.mjs b/test/fixtures/es-module-loaders/builtin-named-exports.mjs index 4e22f631eba416..123b12c26bf0c9 100644 --- a/test/fixtures/es-module-loaders/builtin-named-exports.mjs +++ b/test/fixtures/es-module-loaders/builtin-named-exports.mjs @@ -1,4 +1,3 @@ -import { isMainThread } from '../../common/index.mjs'; import * as fixtures from '../../common/fixtures.mjs'; import { createRequire, register } from 'node:module'; @@ -11,10 +10,8 @@ Object.defineProperty(globalThis, GET_BUILTIN, { configurable: false, }); -if (isMainThread) { - register(fixtures.fileURL('es-module-loaders/builtin-named-exports-loader.mjs'), { - data: { - GET_BUILTIN, - }, - }); -} +register(fixtures.fileURL('es-module-loaders/builtin-named-exports-loader.mjs'), { + data: { + GET_BUILTIN, + }, +}); diff --git a/test/fixtures/es-module-loaders/hooks-exit-worker.mjs b/test/fixtures/es-module-loaders/hooks-exit-worker.mjs deleted file mode 100644 index d499a835e6456c..00000000000000 --- a/test/fixtures/es-module-loaders/hooks-exit-worker.mjs +++ /dev/null @@ -1,21 +0,0 @@ -import { writeFileSync } from 'node:fs'; - -export function resolve(specifier, context, next) { - writeFileSync(1, `resolve ${specifier}\n`); - if (specifier === 'process-exit-module-resolve') { - process.exit(42); - } - - if (specifier === 'process-exit-module-load') { - return { __proto__: null, shortCircuit: true, url: 'process-exit-on-load:///' } - } - return next(specifier, context); -} - -export function load(url, context, next) { - writeFileSync(1, `load ${url}\n`); - if (url === 'process-exit-on-load:///') { - process.exit(43); - } - return next(url, context); -} diff --git a/test/fixtures/es-module-loaders/hooks-log.mjs b/test/fixtures/es-module-loaders/hooks-log.mjs deleted file mode 100644 index 2d2512281e8bd5..00000000000000 --- a/test/fixtures/es-module-loaders/hooks-log.mjs +++ /dev/null @@ -1,19 +0,0 @@ -import { writeFileSync } from 'node:fs'; - -let initializeCount = 0; -let resolveCount = 0; -let loadCount = 0; - -export function initialize() { - writeFileSync(1, `initialize ${++initializeCount}\n`); -} - -export function resolve(specifier, context, next) { - writeFileSync(1, `hooked resolve ${++resolveCount} ${specifier}\n`); - return next(specifier, context); -} - -export function load(url, context, next) { - writeFileSync(1, `hooked load ${++loadCount} ${url}\n`); - return next(url, context); -} diff --git a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs index 7d53e31df918a7..bf66efbd0810e5 100644 --- a/test/fixtures/es-module-loaders/not-found-assert-loader.mjs +++ b/test/fixtures/es-module-loaders/not-found-assert-loader.mjs @@ -1,13 +1,16 @@ import assert from 'node:assert'; // A loader that asserts that the defaultResolve will throw "not found" +// (skipping the top-level main of course, and the built-in ones needed for run-worker). +let mainLoad = true; export async function resolve(specifier, { importAttributes }, next) { - if (specifier.startsWith('./not-found')) { - await assert.rejects(next(specifier), { code: 'ERR_MODULE_NOT_FOUND' }); - return { - url: 'node:fs', - importAttributes, - }; + if (mainLoad || specifier === 'path' || specifier === 'worker_threads') { + mainLoad = false; + return next(specifier); } - return next(specifier); + await assert.rejects(next(specifier), { code: 'ERR_MODULE_NOT_FOUND' }); + return { + url: 'node:fs', + importAttributes, + }; } diff --git a/test/fixtures/es-module-loaders/worker-fail-on-load.mjs b/test/fixtures/es-module-loaders/worker-fail-on-load.mjs deleted file mode 100644 index 46e88664a03c5c..00000000000000 --- a/test/fixtures/es-module-loaders/worker-fail-on-load.mjs +++ /dev/null @@ -1 +0,0 @@ -import 'process-exit-module-load'; diff --git a/test/fixtures/es-module-loaders/worker-fail-on-resolve.mjs b/test/fixtures/es-module-loaders/worker-fail-on-resolve.mjs deleted file mode 100644 index e8e7adde42585f..00000000000000 --- a/test/fixtures/es-module-loaders/worker-fail-on-resolve.mjs +++ /dev/null @@ -1 +0,0 @@ -import 'process-exit-module-resolve'; diff --git a/test/fixtures/es-module-loaders/worker-log-again.mjs b/test/fixtures/es-module-loaders/worker-log-again.mjs deleted file mode 100644 index 2969edc8dac382..00000000000000 --- a/test/fixtures/es-module-loaders/worker-log-again.mjs +++ /dev/null @@ -1,3 +0,0 @@ -import { bar } from './module-named-exports.mjs'; - -console.log(bar); diff --git a/test/fixtures/es-module-loaders/worker-log-fail-worker-load.mjs b/test/fixtures/es-module-loaders/worker-log-fail-worker-load.mjs deleted file mode 100644 index 81797da392cb7a..00000000000000 --- a/test/fixtures/es-module-loaders/worker-log-fail-worker-load.mjs +++ /dev/null @@ -1,12 +0,0 @@ -import { Worker } from 'worker_threads'; -import { foo } from './module-named-exports.mjs'; - -const workerURLFailOnLoad = new URL('./worker-fail-on-load.mjs', import.meta.url); -console.log(foo); - -// Spawn a worker that will fail to import a dependant module -new Worker(workerURLFailOnLoad); - -process.on('exit', (code) => { - console.log(`process exit code: ${code}`) -}); diff --git a/test/fixtures/es-module-loaders/worker-log-fail-worker-resolve.mjs b/test/fixtures/es-module-loaders/worker-log-fail-worker-resolve.mjs deleted file mode 100644 index b5ff238967f4ef..00000000000000 --- a/test/fixtures/es-module-loaders/worker-log-fail-worker-resolve.mjs +++ /dev/null @@ -1,12 +0,0 @@ -import { Worker } from 'worker_threads'; -import { foo } from './module-named-exports.mjs'; - -const workerURLFailOnResolve = new URL('./worker-fail-on-resolve.mjs', import.meta.url); -console.log(foo); - -// Spawn a worker that will fail to import a dependant module -new Worker(workerURLFailOnResolve); - -process.on('exit', (code) => { - console.log(`process exit code: ${code}`) -}); diff --git a/test/fixtures/es-module-loaders/worker-log.mjs b/test/fixtures/es-module-loaders/worker-log.mjs deleted file mode 100644 index 13290c37d07104..00000000000000 --- a/test/fixtures/es-module-loaders/worker-log.mjs +++ /dev/null @@ -1,9 +0,0 @@ -import { Worker } from 'worker_threads'; -import { foo } from './module-named-exports.mjs'; - -const workerURL = new URL('./worker-log-again.mjs', import.meta.url); -console.log(foo); - -// Spawn two workers -new Worker(workerURL); -new Worker(workerURL); diff --git a/test/fixtures/es-module-loaders/workers-spawned.mjs b/test/fixtures/es-module-loaders/workers-spawned.mjs deleted file mode 100644 index 439847656fe13e..00000000000000 --- a/test/fixtures/es-module-loaders/workers-spawned.mjs +++ /dev/null @@ -1,7 +0,0 @@ -import { Worker } from 'worker_threads'; - -const workerURL = new URL('./worker-log.mjs', import.meta.url); - -// Spawn two workers -new Worker(workerURL); -new Worker(workerURL);