Skip to content

Commit 74564d9

Browse files
panvanodejs-github-bot
authored andcommitted
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 <panva.ip@gmail.com> Assisted-by: Codex PR-URL: #66354 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Aviv Keller <me@aviv.sh>
1 parent f53f438 commit 74564d9

2 files changed

Lines changed: 41 additions & 2 deletions

File tree

‎lib/internal/webworker.js‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ const {
6868
} = require('internal/worker');
6969

7070
const {
71+
MessageChannel,
7172
lazyMessageEvent,
7273
} = require('internal/worker/io');
7374

@@ -107,6 +108,7 @@ const kLocation = Symbol('kLocation');
107108
const kName = Symbol('kName');
108109
const kNavigator = Symbol('kNavigator');
109110
const kNavigatorBrand = Symbol('kNavigatorBrand');
111+
const kOutsidePort = Symbol('kOutsidePort');
110112
const kType = Symbol('kType');
111113
const kURL = Symbol('kURL');
112114
const kWorker = Symbol('kWorker');
@@ -808,6 +810,12 @@ class Worker extends EventTarget {
808810
this[kWorker] = null;
809811
const entry = resolveWorkerEntry(workerURL);
810812
if (entry === null) {
813+
// Even a failed worker has an outside port. Keep a closed port so
814+
// postMessage() still serializes its message and transfers objects.
815+
const { port1, port2 } = new MessageChannel();
816+
port1.close();
817+
port2.close();
818+
this[kOutsidePort] = port1;
811819
// "If the algorithm asynchronously completes with null or with a
812820
// script whose error to rethrow is non-null, then: Queue a global
813821
// task on the DOM manipulation task source given worker's relevant
@@ -837,7 +845,8 @@ class Worker extends EventTarget {
837845
},
838846
});
839847

840-
forwardMessageEvents(this[kWorker][kPublicPort], this);
848+
this[kOutsidePort] = this[kWorker][kPublicPort];
849+
forwardMessageEvents(this[kOutsidePort], this);
841850

842851
// "Set notHandled to the result of firing an event named error at
843852
// workerObject, using ErrorEvent, with the cancelable attribute
@@ -872,7 +881,7 @@ class Worker extends EventTarget {
872881
// invoked the respective postMessage(message, transfer) and
873882
// postMessage(message, options) on this's outside port, with the same
874883
// arguments, and returned the same return value."
875-
this[kWorker]?.postMessage(message, transfer);
884+
this[kOutsidePort].postMessage(message, transfer);
876885
}
877886

878887
// The following properties are non-standard, Node.js extensions
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Flags: --experimental-web-worker
2+
'use strict';
3+
4+
const common = require('../common');
5+
const assert = require('node:assert');
6+
7+
function checkSerialization(worker) {
8+
assert.throws(() => worker.postMessage(() => {}), { name: 'DataCloneError' });
9+
const buffer = new ArrayBuffer(8);
10+
assert.throws(() => worker.postMessage(null, [buffer, buffer]), { name: 'DataCloneError' });
11+
assert.strictEqual(buffer.byteLength, 8);
12+
worker.postMessage(null, [buffer]);
13+
assert.strictEqual(buffer.byteLength, 0);
14+
}
15+
16+
// A failed script fetch still leaves an outside port that serializes messages.
17+
{
18+
const worker = new Worker('data:text/plain,');
19+
worker.onerror = common.mustCall(() => checkSerialization(worker));
20+
checkSerialization(worker);
21+
}
22+
23+
// Serialization is required even after the backing thread has exited.
24+
{
25+
const worker = new Worker('data:text/javascript,close()');
26+
worker.onerror = common.mustNotCall('worker failed');
27+
process.once('worker', common.mustCall((thread) => {
28+
thread.once('exit', common.mustCall(() => checkSerialization(worker)));
29+
}));
30+
}

0 commit comments

Comments
 (0)