From 53a4c678a32b6d1b92102e1cdb451507cae2ac97 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sun, 27 Sep 2026 19:02:12 +0200 Subject: [PATCH 1/6] worker: fix postMessage overload resolution Web IDL overload resolution treats functions as objects, accepts a null iterator as missing, and retrieves the iterator method only once. Reuse that method during sequence conversion in both postMessage entry points. Signed-off-by: Filip Skokan Assisted-by: Codex --- lib/internal/webidl.js | 9 ++-- lib/internal/webworker.js | 9 ++-- .../test-webworker-postmessage-overloads.js | 53 +++++++++++++++++++ 3 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 test/parallel/test-webworker-postmessage-overloads.js diff --git a/lib/internal/webidl.js b/lib/internal/webidl.js index 88f4205a1c05..819b5af205b7 100644 --- a/lib/internal/webidl.js +++ b/lib/internal/webidl.js @@ -815,7 +815,7 @@ function createDictionaryConverter( * @returns {Converter} */ function createSequenceConverter(converter) { - return function(V, options = kEmptyObject) { + return function(V, options = kEmptyObject, method = undefined) { // Web IDL sequence conversion step 1: require an ECMA-262 Object. if (type(V) !== 'Object') { throw makeException( @@ -823,8 +823,11 @@ function createSequenceConverter(converter) { options); } - // Step 2: GetMethod(V, %Symbol.iterator%). - const method = V[SymbolIterator]; + // Step 2: GetMethod(V, %Symbol.iterator%). Overload resolution can + // supply the method it already retrieved, avoiding a second lookup. + if (method === undefined) { + method = V[SymbolIterator]; + } // Step 3: throw if the iterator method is undefined, null, or not callable. if (typeof method !== 'function') { throw makeException( diff --git a/lib/internal/webworker.js b/lib/internal/webworker.js index d7e768372def..228fb194d296 100644 --- a/lib/internal/webworker.js +++ b/lib/internal/webworker.js @@ -319,9 +319,12 @@ function fetchClassicScriptSourceSync(url, blob) { // sequence overload, everything else is converted as a // StructuredSerializeOptions dictionary. function normalizeTransfer(transferOrOptions, options) { - if (typeof transferOrOptions === 'object' && transferOrOptions !== null && - transferOrOptions[SymbolIterator] !== undefined) { - return converters['sequence'](transferOrOptions, options); + if ((typeof transferOrOptions === 'object' && transferOrOptions !== null) || + typeof transferOrOptions === 'function') { + const method = transferOrOptions[SymbolIterator]; + if (method != null) { + return converters['sequence'](transferOrOptions, options, method); + } } return converters.StructuredSerializeOptions(transferOrOptions, options) .transfer; diff --git a/test/parallel/test-webworker-postmessage-overloads.js b/test/parallel/test-webworker-postmessage-overloads.js new file mode 100644 index 000000000000..08021251584b --- /dev/null +++ b/test/parallel/test-webworker-postmessage-overloads.js @@ -0,0 +1,53 @@ +// Flags: --experimental-web-worker +'use strict'; + +const common = require('../common'); +const assert = require('node:assert'); +const { isMainThread } = require('node:worker_threads'); +const { pathToFileURL } = require('node:url'); + +function checkPostMessage(post) { + // A null iterator selects the dictionary overload. + const dictionaryBuffer = new ArrayBuffer(8); + post(null, { [Symbol.iterator]: null, transfer: [dictionaryBuffer] }); + assert.strictEqual(dictionaryBuffer.byteLength, 0); + + // Callable objects can also be iterable transfer lists. + const functionBuffer = new ArrayBuffer(8); + function transfer() {} + transfer[Symbol.iterator] = function*() { yield functionBuffer; }; + post(null, transfer); + assert.strictEqual(functionBuffer.byteLength, 0); + + // Overload resolution must reuse the iterator method it retrieved. + const getterBuffer = new ArrayBuffer(8); + const iterable = {}; + Object.defineProperty(iterable, Symbol.iterator, { + get: common.mustCall(() => common.mustCall(function*() { + assert.strictEqual(this, iterable); + yield getterBuffer; + })), + }); + post(null, iterable); + assert.strictEqual(getterBuffer.byteLength, 0); + + // A present, non-callable iterator must fail before reading the dictionary. + for (const value of [{}, function() {}]) { + value[Symbol.iterator] = 1; + Object.defineProperty(value, 'transfer', { get: common.mustNotCall() }); + assert.throws(() => post(null, value), TypeError); + } +} + +if (isMainThread) { + const worker = new Worker(pathToFileURL(__filename)); + worker.onerror = common.mustNotCall('worker failed'); + const done = common.mustCall(() => worker.terminate()); + worker.onmessage = ({ data }) => { + if (data === 'done') done(); + }; + checkPostMessage(worker.postMessage.bind(worker)); +} else { + checkPostMessage(globalThis.postMessage); + globalThis.postMessage('done'); +} From d3e37cf56e36ed207439c785e43800286180dba5 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sun, 27 Sep 2026 19:02:12 +0200 Subject: [PATCH 2/6] worker: convert importScripts arguments first Web IDL converts every argument before entering the method algorithm. Perform all USVString conversions before rejecting module workers or parsing URLs, so later conversions can throw or revoke blob URLs first. Signed-off-by: Filip Skokan Assisted-by: Codex --- lib/internal/webworker.js | 8 ++- ...test-webworker-importscripts-conversion.js | 67 +++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-webworker-importscripts-conversion.js diff --git a/lib/internal/webworker.js b/lib/internal/webworker.js index 228fb194d296..e798c814f284 100644 --- a/lib/internal/webworker.js +++ b/lib/internal/webworker.js @@ -507,6 +507,11 @@ class WorkerGlobalScope extends EventTarget { importScripts(...urls) { validateThisInternalField(this, kLocation, 'WorkerGlobalScope'); const prefix = "Failed to execute 'importScripts' on 'WorkerGlobalScope'"; + // Web IDL converts every argument before running the method steps. + for (let i = 0; i < urls.length; i++) { + urls[i] = converters.USVString( + urls[i], { prefix, context: `Argument ${i + 1}` }); + } // "To import scripts into worker global scope, given a // WorkerGlobalScope object worker global scope, a list of scalar value // strings urls, and an optional perform the fetch hook performFetch:" @@ -525,8 +530,7 @@ class WorkerGlobalScope extends EventTarget { const urlRecords = []; // "For each url of urls:" for (let i = 0; i < urls.length; i++) { - const url = converters.USVString( - urls[i], { prefix, context: `Argument ${i + 1}` }); + const url = urls[i]; // "Let urlRecord be the result of encoding-parsing a URL given url, // relative to settings object." const urlRecord = URLParse(url, this[kLocation].href); diff --git a/test/parallel/test-webworker-importscripts-conversion.js b/test/parallel/test-webworker-importscripts-conversion.js new file mode 100644 index 000000000000..5ea252814284 --- /dev/null +++ b/test/parallel/test-webworker-importscripts-conversion.js @@ -0,0 +1,67 @@ +// Flags: --experimental-web-worker +'use strict'; + +const common = require('../common'); +const assert = require('node:assert'); + +for (const type of ['classic', 'module']) { + const source = ` + const results = []; + const conversions = []; + const sentinel = new Error('conversion'); + try { + importScripts('https://[', { + toString() { conversions.push('converted'); throw sentinel; } + }); + } catch (error) { + results.push([conversions, error === sentinel]); + } + const order = []; + try { + importScripts( + { toString() { order.push(1); return 'https://['; } }, + { toString() { order.push(2); return 'data:text/javascript,'; } } + ); + } catch (error) { + results.push([order, error.name]); + } + postMessage(results); + `; + const worker = new Worker(`data:text/javascript,${encodeURIComponent(source)}`, { type }); + worker.onerror = common.mustNotCall('worker failed'); + worker.onmessage = common.mustCall(({ data }) => { + worker.terminate(); + assert.deepStrictEqual(data, [ + [['converted'], true], + [[1, 2], type === 'module' ? 'TypeError' : 'SyntaxError'], + ]); + }); +} + +if (common.hasCrypto) { + // URL parsing must capture blob entries after all arguments are converted. + const source = ` + self.ran = false; + const url = URL.createObjectURL(new Blob(['self.ran = true'], { + type: 'text/javascript' + })); + let errorName; + try { + importScripts(url, { + toString() { + URL.revokeObjectURL(url); + return 'data:text/javascript,'; + } + }); + } catch (error) { + errorName = error.name; + } + postMessage([self.ran, errorName]); + `; + const worker = new Worker(`data:text/javascript,${encodeURIComponent(source)}`); + worker.onerror = common.mustNotCall('worker failed'); + worker.onmessage = common.mustCall(({ data }) => { + worker.terminate(); + assert.deepStrictEqual(data, [false, 'NetworkError']); + }); +} From c186605b00be32a0f49846c5f7b11948e9588a5e Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sun, 27 Sep 2026 19:02:12 +0200 Subject: [PATCH 3/6] worker: preserve blob module URLs Evaluate blob module sources under their original URL instead of rewrapping them in a data URL. This preserves import.meta.url and module identity while retaining the captured source after URL revocation. Signed-off-by: Filip Skokan Assisted-by: Codex --- lib/internal/webworker.js | 22 ++----------------- .../test-webworker-blob-module-url.js | 19 ++++++++++++++++ 2 files changed, 21 insertions(+), 20 deletions(-) create mode 100644 test/parallel/test-webworker-blob-module-url.js diff --git a/lib/internal/webworker.js b/lib/internal/webworker.js index e798c814f284..cf5f1df63726 100644 --- a/lib/internal/webworker.js +++ b/lib/internal/webworker.js @@ -20,8 +20,6 @@ const { SymbolFor, SymbolIterator, SymbolToStringTag, - TypedArrayPrototypeGetLength, - Uint8Array, globalThis, } = primordials; @@ -77,10 +75,6 @@ const { vm_dynamic_import_default_internal, } = internalBinding('symbols'); -const { - base64Slice, -} = internalBinding('buffer'); - const { hasOpenSSL, } = internalBinding('config'); @@ -757,10 +751,9 @@ const convertWorkerOptions = createDictionaryConverter( /** * @param {URL} workerURL The parsed URL of the worker script. - * @param {'classic'|'module'} type The worker's type. * @returns {{ value?: URL, source?: string }|null} */ -function resolveWorkerEntry(workerURL, type) { +function resolveWorkerEntry(workerURL) { switch (workerURL.protocol) { case 'file:': return { value: workerURL }; @@ -779,17 +772,6 @@ function resolveWorkerEntry(workerURL, type) { if (data === undefined) { return null; } - if (type === 'module') { - // The blob's contents are re-wrapped as a data: URL so that the - // module loader evaluates them as a module script. - // TODO(@avivkeller): What if we update the ESM loader to accept blob: - // urls? - const bytes = new Uint8Array(data); - return { - value: new URL('data:text/javascript;base64,' + base64Slice( - bytes, 0, TypedArrayPrototypeGetLength(bytes))), - }; - } return { source: utf8Decode(data) }; } default: @@ -824,7 +806,7 @@ class Worker extends EventTarget { } this[kWorker] = null; - const entry = resolveWorkerEntry(workerURL, options.type); + const entry = resolveWorkerEntry(workerURL); if (entry === null) { // "If the algorithm asynchronously completes with null or with a // script whose error to rethrow is non-null, then: Queue a global diff --git a/test/parallel/test-webworker-blob-module-url.js b/test/parallel/test-webworker-blob-module-url.js new file mode 100644 index 000000000000..7e51b7274464 --- /dev/null +++ b/test/parallel/test-webworker-blob-module-url.js @@ -0,0 +1,19 @@ +// Flags: --experimental-web-worker +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) common.skip('missing crypto'); + +const assert = require('node:assert'); +const source = ` + import value from 'data:text/javascript,export default 42'; + postMessage([import.meta.url, location.href, value]); +`; +const url = URL.createObjectURL(new Blob([source], { type: 'text/javascript' })); +const worker = new Worker(url, { type: 'module' }); +URL.revokeObjectURL(url); +worker.onerror = common.mustNotCall('worker failed'); +worker.onmessage = common.mustCall(({ data }) => { + worker.terminate(); + assert.deepStrictEqual(data, [url, url, 42]); +}); From 2ee9a6608d869124b6e92212bd9a2ba984f0204b Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sun, 27 Sep 2026 19:02:12 +0200 Subject: [PATCH 4/6] worker: serialize messages after exit The outside port must serialize and transfer messages even when it has no peer. Retain that port after the backing thread exits and create a closed port when the entry script fetch fails. This preserves clone errors and transfer side effects in both cases. Signed-off-by: Filip Skokan Assisted-by: Codex --- lib/internal/webworker.js | 13 ++++++-- .../test-webworker-postmessage-lifecycle.js | 30 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-webworker-postmessage-lifecycle.js diff --git a/lib/internal/webworker.js b/lib/internal/webworker.js index cf5f1df63726..744fe4de0684 100644 --- a/lib/internal/webworker.js +++ b/lib/internal/webworker.js @@ -68,6 +68,7 @@ const { } = require('internal/worker'); const { + MessageChannel, lazyMessageEvent, } = require('internal/worker/io'); @@ -107,6 +108,7 @@ const kLocation = Symbol('kLocation'); const kName = Symbol('kName'); const kNavigator = Symbol('kNavigator'); const kNavigatorBrand = Symbol('kNavigatorBrand'); +const kOutsidePort = Symbol('kOutsidePort'); const kType = Symbol('kType'); const kURL = Symbol('kURL'); const kWorker = Symbol('kWorker'); @@ -808,6 +810,12 @@ class Worker extends EventTarget { this[kWorker] = null; const entry = resolveWorkerEntry(workerURL); if (entry === null) { + // Even a failed worker has an outside port. Keep a closed port so + // postMessage() still serializes its message and transfers objects. + const { port1, port2 } = new MessageChannel(); + port1.close(); + port2.close(); + this[kOutsidePort] = port1; // "If the algorithm asynchronously completes with null or with a // script whose error to rethrow is non-null, then: Queue a global // task on the DOM manipulation task source given worker's relevant @@ -837,7 +845,8 @@ class Worker extends EventTarget { }, }); - forwardMessageEvents(this[kWorker][kPublicPort], this); + this[kOutsidePort] = this[kWorker][kPublicPort]; + forwardMessageEvents(this[kOutsidePort], this); // "Set notHandled to the result of firing an event named error at // workerObject, using ErrorEvent, with the cancelable attribute @@ -872,7 +881,7 @@ class Worker extends EventTarget { // invoked the respective postMessage(message, transfer) and // postMessage(message, options) on this's outside port, with the same // arguments, and returned the same return value." - this[kWorker]?.postMessage(message, transfer); + this[kOutsidePort].postMessage(message, transfer); } // The following properties are non-standard, Node.js extensions diff --git a/test/parallel/test-webworker-postmessage-lifecycle.js b/test/parallel/test-webworker-postmessage-lifecycle.js new file mode 100644 index 000000000000..51acd57fa627 --- /dev/null +++ b/test/parallel/test-webworker-postmessage-lifecycle.js @@ -0,0 +1,30 @@ +// Flags: --experimental-web-worker +'use strict'; + +const common = require('../common'); +const assert = require('node:assert'); + +function checkSerialization(worker) { + assert.throws(() => worker.postMessage(() => {}), { name: 'DataCloneError' }); + const buffer = new ArrayBuffer(8); + assert.throws(() => worker.postMessage(null, [buffer, buffer]), { name: 'DataCloneError' }); + assert.strictEqual(buffer.byteLength, 8); + worker.postMessage(null, [buffer]); + assert.strictEqual(buffer.byteLength, 0); +} + +// A failed script fetch still leaves an outside port that serializes messages. +{ + const worker = new Worker('data:text/plain,'); + worker.onerror = common.mustCall(() => checkSerialization(worker)); + checkSerialization(worker); +} + +// Serialization is required even after the backing thread has exited. +{ + const worker = new Worker('data:text/javascript,close()'); + worker.onerror = common.mustNotCall('worker failed'); + process.once('worker', common.mustCall((thread) => { + thread.once('exit', common.mustCall(() => checkSerialization(worker))); + })); +} From 5a556695bc0d1999e5ece52d0b8ef8a5c4f7ece7 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sun, 27 Sep 2026 19:02:13 +0200 Subject: [PATCH 5/6] worker: discard queued messages on termination Close the outside port and remove its message forwarding listeners synchronously. Closing the port alone is asynchronous and can leave queued messages dispatching when terminate() runs inside a listener. Allow the current event to finish while discarding subsequent messages, including those drained when the backing thread exits. Signed-off-by: Filip Skokan Assisted-by: Codex --- lib/internal/webworker.js | 21 ++++++-- .../test-webworker-postmessage-lifecycle.js | 48 +++++++++++++++++++ 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/lib/internal/webworker.js b/lib/internal/webworker.js index 744fe4de0684..1361b6daa6e3 100644 --- a/lib/internal/webworker.js +++ b/lib/internal/webworker.js @@ -109,6 +109,7 @@ const kName = Symbol('kName'); const kNavigator = Symbol('kNavigator'); const kNavigatorBrand = Symbol('kNavigatorBrand'); const kOutsidePort = Symbol('kOutsidePort'); +const kRemoveMessageListeners = Symbol('kRemoveMessageListeners'); const kType = Symbol('kType'); const kURL = Symbol('kURL'); const kWorker = Symbol('kWorker'); @@ -133,15 +134,21 @@ function createErrorEvent(init) { // "If this throws an exception, catch it, fire an event named messageerror // at messageEventTarget, using MessageEvent, and then return." function forwardMessageEvents(port, target) { - port.on('message', (data) => + const onMessage = (data) => target.dispatchEvent(lazyMessageEvent('message', { data, ports: port[kCurrentlyReceivingPorts], - }))); - port.on('messageerror', (data) => + })); + const onMessageError = (data) => target.dispatchEvent(lazyMessageEvent('messageerror', { data, - }))); + })); + port.on('message', onMessage); + port.on('messageerror', onMessageError); + return () => { + port.removeListener('message', onMessage); + port.removeListener('messageerror', onMessageError); + }; } // Web IDL [Replaceable]: the setter replaces the accessor with an own @@ -846,7 +853,7 @@ class Worker extends EventTarget { }); this[kOutsidePort] = this[kWorker][kPublicPort]; - forwardMessageEvents(this[kOutsidePort], this); + this[kRemoveMessageListeners] = forwardMessageEvents(this[kOutsidePort], this); // "Set notHandled to the result of firing an event named error at // workerObject, using ErrorEvent, with the cancelable attribute @@ -865,6 +872,10 @@ class Worker extends EventTarget { validateThisInternalField(this, kWorker, 'Worker'); // "The terminate() method steps are to terminate a worker given this's // worker." + // Closing a port is asynchronous. Remove its forwarding listeners too, + // since terminate() can run while the backing thread drains queued messages. + this[kRemoveMessageListeners]?.(); + this[kOutsidePort].close(); this[kWorker]?.terminate(); } diff --git a/test/parallel/test-webworker-postmessage-lifecycle.js b/test/parallel/test-webworker-postmessage-lifecycle.js index 51acd57fa627..e4199ed53726 100644 --- a/test/parallel/test-webworker-postmessage-lifecycle.js +++ b/test/parallel/test-webworker-postmessage-lifecycle.js @@ -28,3 +28,51 @@ function checkSerialization(worker) { thread.once('exit', common.mustCall(() => checkSerialization(worker))); })); } + +// Queue messages before terminating so the test does not depend on thread speed. +{ + const source = ` + onmessage = ({ data }) => { + for (let i = 0; i < 3; i++) postMessage(i); + const state = new Int32Array(data); + Atomics.store(state, 0, 1); + Atomics.notify(state, 0); + }; + `; + const worker = new Worker(`data:text/javascript,${encodeURIComponent(source)}`); + worker.onerror = common.mustNotCall('worker failed'); + worker.onmessage = common.mustNotCall('message delivered after terminate()'); + const state = new Int32Array(new SharedArrayBuffer(4)); + worker.postMessage(state.buffer); + assert.notStrictEqual(Atomics.wait(state, 0, 0, common.platformTimeout(10000)), 'timed-out'); + worker.terminate(); + worker.terminate(); + checkSerialization(worker); +} + +// terminate() can run during message dispatch, including while the backing +// thread is draining messages on exit. Finish this event but discard later ones. +for (const closeAfterPosting of [false, true]) { + const source = ` + onmessage = ({ data }) => { + for (let i = 0; i < 3; i++) postMessage(i); + const state = new Int32Array(data); + Atomics.store(state, 0, 1); + Atomics.notify(state, 0); + if (${closeAfterPosting}) close(); + }; + `; + const worker = new Worker(`data:text/javascript,${encodeURIComponent(source)}`); + worker.onerror = common.mustNotCall('worker failed'); + worker.addEventListener('message', common.mustCall(({ data }) => { + assert.strictEqual(data, 0); + worker.terminate(); + checkSerialization(worker); + })); + worker.addEventListener('message', common.mustCall(({ data }) => { + assert.strictEqual(data, 0); + })); + const state = new Int32Array(new SharedArrayBuffer(4)); + worker.postMessage(state.buffer); + assert.notStrictEqual(Atomics.wait(state, 0, 0, common.platformTimeout(10000)), 'timed-out'); +} From 06ef7b4e0ce17fe8b667116975fe1c7651196f4b Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 28 Sep 2026 13:23:00 +0200 Subject: [PATCH 6/6] fixup! worker: fix postMessage overload resolution Signed-off-by: Filip Skokan Assisted-by: Codex --- test/parallel/test-webworker-postmessage-overloads.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-webworker-postmessage-overloads.js b/test/parallel/test-webworker-postmessage-overloads.js index 08021251584b..17b83191b309 100644 --- a/test/parallel/test-webworker-postmessage-overloads.js +++ b/test/parallel/test-webworker-postmessage-overloads.js @@ -3,7 +3,6 @@ const common = require('../common'); const assert = require('node:assert'); -const { isMainThread } = require('node:worker_threads'); const { pathToFileURL } = require('node:url'); function checkPostMessage(post) { @@ -39,7 +38,8 @@ function checkPostMessage(post) { } } -if (isMainThread) { +// The test runner can also execute this file in a regular Node worker. +if (typeof globalThis.DedicatedWorkerGlobalScope === 'undefined') { const worker = new Worker(pathToFileURL(__filename)); worker.onerror = common.mustNotCall('worker failed'); const done = common.mustCall(() => worker.terminate());