Skip to content

Commit 856fcba

Browse files
committed
fix(runtime): parentPort once() detaches its own registration, relays ports, and a throwing stack accessor only drops the stack
A once() wrapper removed the first registration holding the same listener, which with on() and once() sharing a function could detach the wrong one and leave the once() wrapper firing forever; each registration is now its own entry and removeListener() takes the most recent, as Node does. The relay onto parentPort forwards event.ports. Reading `stack` off the error a scope onerror threw runs under a TryCatch so a throwing accessor cannot leave an exception pending past the rejection drain.
1 parent e642996 commit 856fcba

5 files changed

Lines changed: 82 additions & 8 deletions

File tree

‎NativeScript/runtime/NativeScriptException.mm‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,10 @@ static bool GiveWorkerOnErrorAChance(Isolate* isolate, Local<Context> context, L
660660
Local<Value> forwarded = thrown.IsEmpty() ? reason : thrown;
661661
std::string forwardedStack = stack;
662662
if (!thrown.IsEmpty()) {
663+
// `stack` may be an accessor that throws; that only costs
664+
// the stack, never the forward.
663665
forwardedStack = "";
666+
TryCatch stackTc(isolate_);
664667
Local<Value> thrownStack;
665668
if (thrown->IsObject() &&
666669
thrown.As<Object>()

‎NativeScript/runtime/js/node-worker-threads.js‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -254,32 +254,49 @@ class ParentPort extends EventTarget {
254254
throw new TypeError('The "listener" argument must be of type function');
255255
}
256256
const self = this;
257-
const wrapper = function (event) {
257+
const entry = { listener, wrapper: undefined };
258+
// A once registration detaches its own entry, not whichever entry happens
259+
// to hold the same listener: the same function may be on() and once() at
260+
// the same time.
261+
entry.wrapper = function (event) {
258262
if (once) {
259-
self.#remove(type, listener);
263+
self.#removeEntry(type, entry);
260264
}
261265
const arg = type === "message" || type === "messageerror" ? event.data : event;
262266
FunctionPrototypeCall(listener, self, arg);
263267
};
264268
const list = this.#wrappers[type] || (this.#wrappers[type] = []);
265-
ArrayPrototypePush(list, { listener, wrapper });
266-
FunctionPrototypeCall(addEventListener, this, type, wrapper);
269+
ArrayPrototypePush(list, entry);
270+
FunctionPrototypeCall(addEventListener, this, type, entry.wrapper);
267271
return this;
268272
}
269273

274+
// Node removes the most recently added registration of a listener.
270275
#remove(type, listener) {
271276
const list = this.#wrappers[type];
272277
if (list === undefined) {
273278
return;
274279
}
275-
for (let i = 0; i < list.length; i++) {
280+
for (let i = list.length - 1; i >= 0; i--) {
276281
if (list[i].listener === listener) {
277-
FunctionPrototypeCall(removeEventListener, this, type, list[i].wrapper);
278-
ArrayPrototypeSplice(list, i, 1);
282+
this.#removeEntry(type, list[i]);
279283
return;
280284
}
281285
}
282286
}
287+
288+
#removeEntry(type, entry) {
289+
const list = this.#wrappers[type];
290+
if (list === undefined) {
291+
return;
292+
}
293+
const index = ArrayPrototypeIndexOf(list, entry);
294+
if (index === -1) {
295+
return;
296+
}
297+
ArrayPrototypeSplice(list, index, 1);
298+
FunctionPrototypeCall(removeEventListener, this, type, entry.wrapper);
299+
}
283300
}
284301

285302
defineEventHandler(ParentPort.prototype, "message");
@@ -298,7 +315,7 @@ if (!isMainThread) {
298315
FunctionPrototypeCall(
299316
dispatchEvent,
300317
parentPort,
301-
new (getMessageEvent())(event.type, { data: event.data })
318+
new (getMessageEvent())(event.type, { data: event.data, ports: event.ports })
302319
);
303320
};
304321
FunctionPrototypeCall(addEventListener, globalEventTarget, "message", relay);

‎TestRunner/app/tests/MessagingTests.js‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,48 @@ describe("Messaging runtime edges", function () {
150150
worker.postMessage(2);
151151
});
152152

153+
it("lets the same listener be on() and once() at the same time", function (done) {
154+
var worker = new wt.Worker("~/tests/messaging/parentPortOnceWorker.js");
155+
var got = [];
156+
worker.on("message", function (value) {
157+
got.push(value);
158+
if (got.length === 3) {
159+
setTimeout(function () {
160+
// Three messages: the once() registration fires only
161+
// for the first, the on() one for all three.
162+
expect(got).toEqual([1, 2, 3, 4]);
163+
worker.terminate();
164+
done();
165+
}, SETTLE);
166+
}
167+
});
168+
worker.on("error", function (error) {
169+
fail("worker error: " + error.message);
170+
worker.terminate();
171+
done();
172+
});
173+
worker.postMessage("a");
174+
worker.postMessage("b");
175+
worker.postMessage("c");
176+
});
177+
178+
it("relays transferred ports to parentPort message events", function (done) {
179+
var worker = new wt.Worker("~/tests/messaging/parentPortPortsWorker.js");
180+
var channel = new MessageChannel();
181+
worker.on("message", function (value) {
182+
expect(value).toBe(1);
183+
channel.port1.close();
184+
worker.terminate();
185+
done();
186+
});
187+
worker.on("error", function (error) {
188+
fail("worker error: " + error.message);
189+
worker.terminate();
190+
done();
191+
});
192+
worker.postMessage(channel.port2, [channel.port2]);
193+
});
194+
153195
it("forwards the option bag to the runtime's Worker", function () {
154196
expect(function () {
155197
new wt.Worker("~/tests/messaging/parentPortWorker.js", {
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
var parentPort = require("node:worker_threads").parentPort;
2+
var count = 0;
3+
function listener() {
4+
count++;
5+
parentPort.postMessage(count);
6+
}
7+
parentPort.on("message", listener);
8+
parentPort.once("message", listener);
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
var parentPort = require("node:worker_threads").parentPort;
2+
parentPort.addEventListener("message", function (event) {
3+
parentPort.postMessage(event.ports.length);
4+
});

0 commit comments

Comments
 (0)