From 8887f35f7432fbff94da7151a8b140c72a08875f Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 27 Oct 2026 19:04:26 -0700 Subject: [PATCH] Fix island signal listener bookkeeping and migrated timer refresh An island signal registration is now tagged with the raw listener it installed, so a once registration and an on registration of one function dispatch through their own listeners instead of both resolving to the plain function, and the once registration no longer fires indefinitely. The removal handler forgets the entry whose raw listener is no longer on process, so a once wrapper that removes itself leaves its runtime registration behind no longer; duplicate registrations of one function share a raw listener, and the most recent of those is removed. refresh() of a timer armed before Node loads re-arms at now + the delay the timer was created with. Its Node handle was created with the remaining delay at handoff and a due timer had no handle at all, so the handle is replaced rather than refreshed. The signal program registers one function once and on; a new timer program refreshes a timer most of whose delay passed before Node loaded. Co-Authored-By: Claude Opus 5.5 --- .../main.ts.json | 6 ++ packages/runtime/src/scr_async.c | 15 ++++- packages/runtime/src/scr_node_island_host.cjs | 55 ++++++++++++++++--- packages/runtime/src/scr_node_island_host.h | 55 ++++++++++++++++--- .../corpus/6804-node-island-signals/main.mjs | 10 ++++ .../6890-node-island-timer-refresh/echo.cjs | 3 + .../6890-node-island-timer-refresh/main.ts | 15 +++++ .../main.ts.json | 1 + 8 files changed, 145 insertions(+), 15 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6890-node-island-timer-refresh/main.ts.json create mode 100644 tests/corpus/6890-node-island-timer-refresh/echo.cjs create mode 100644 tests/corpus/6890-node-island-timer-refresh/main.ts create mode 100644 tests/harness/corpus-costs/6890-node-island-timer-refresh/main.ts.json diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6890-node-island-timer-refresh/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6890-node-island-timer-refresh/main.ts.json new file mode 100644 index 000000000..2da15efc8 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6890-node-island-timer-refresh/main.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/6890-node-island-timer-refresh/main.ts" + ], + "diags": [] +} diff --git a/packages/runtime/src/scr_async.c b/packages/runtime/src/scr_async.c index a48872221..9d07412c0 100644 --- a/packages/runtime/src/scr_async.c +++ b/packages/runtime/src/scr_async.c @@ -4868,6 +4868,19 @@ static void scr_hosted_timer_drop(uint32_t token) { scr_closure_release(cb); } +/* refresh(): re-arm to now + the delay the timer was created with, Node's + * Timeout.refresh(). A migrated timer's Node handle was created with its + * remaining delay at handoff, and a due one has no handle yet, so the + * handle is replaced rather than refreshed. A firing timer's flag makes the + * run loop keep the re-armed handle when its callback returns. */ +static void scr_hosted_timer_rearm(uint32_t token) { + ScrHJob *j = &scr_hjobs[token - 1]; + if (j->handle != 0) scr_loop_hosted->timer_clear(j->handle); + j->due = false; + j->handle = scr_loop_hosted->timer(scr_hosted_delay(j->delay_ms), j->repeat_ms, + j->reffed ? 1 : 0, token); +} + static bool scr_hosted_timer_op(unsigned long id, int op) { uint32_t token = scr_hmap_token(id * 2); ScrHJob *j = scr_hjob(token, SCR_HJ_TIMER); @@ -4889,7 +4902,7 @@ static bool scr_hosted_timer_op(unsigned long id, int op) { return true; case SCR_HOSTED_REFRESH: if (j->firing) j->refreshed = true; - if (j->handle != 0) scr_loop_hosted->timer_refresh(j->handle); + scr_hosted_timer_rearm(token); return true; default: return j->reffed; diff --git a/packages/runtime/src/scr_node_island_host.cjs b/packages/runtime/src/scr_node_island_host.cjs index c46bb7583..51644c234 100644 --- a/packages/runtime/src/scr_node_island_host.cjs +++ b/packages/runtime/src/scr_node_island_host.cjs @@ -276,31 +276,72 @@ process.on("newListener", (event, listener) => { const kind = eventKind(event); if (kind === 0) return; const id = nextListener++; - islandListeners.push({ event, listener, id }); + islandListeners.push({ event, listener, id, raw: undefined }); nativeHost.listen(kind, event, id, 1, 0); }); +function forget(i, kind) { + const entry = islandListeners[i]; + islandListeners.splice(i, 1); + nativeHost.listen(kind, entry.event, entry.id, 0, 0); +} + process.on("removeListener", (event, listener) => { const kind = eventKind(event); if (kind === 0) return; + // Node reports the removed registration as the raw listener it installed (a + // once wrapper removing itself reports the wrapper) or as the listener + // itself. The raw listener each entry recorded separates a once + // registration from an `on` registration of the same function: the removed + // one is the entry whose raw listener is no longer on `process`. + const live = process.rawListeners(event); + let same = -1; for (let i = islandListeners.length - 1; i >= 0; i--) { const entry = islandListeners[i]; - if (entry.event === event && entry.listener === listener) { - islandListeners.splice(i, 1); - nativeHost.listen(kind, event, entry.id, 0, 0); + if (entry.event !== event) continue; + if (entry.listener !== listener && entry.raw !== listener) continue; + if (entry.raw !== undefined && !live.includes(entry.raw)) { + forget(i, kind); return; } + if (same < 0) same = i; } + // Duplicate registrations of one function share a raw listener; Node + // removes the most recent, which is the last matching entry. + if (same >= 0) forget(same, kind); }); +// The raw listener is the function handed to the emitter (a once wrapper +// for process.once, which registers through process.on): tag each entry +// with it as its registration returns. Node emits newListener before it +// installs the listener, so the entry learns its raw only afterwards. +for (const name of ["on", "addListener", "prependListener"]) { + const add = process[name]; + process[name] = function addListener(event, listener) { + const start = islandListeners.length; + const result = Reflect.apply(add, this, arguments); + if (islandListeners.length > start) { + const registered = listener.listener ?? listener; + for (let i = start; i < islandListeners.length; i++) { + const entry = islandListeners[i]; + if (entry.event === event && entry.listener === registered && entry.raw === undefined) { + entry.raw = listener; + break; + } + } + } + return result; + }; +} + function emit(kind, id, name, arg) { const entry = islandListeners.find((e) => e.id === id); if (entry === undefined) return; // The registered function: the listener itself, or its once wrapper, // which removes itself before calling it. - const registered = process - .rawListeners(entry.event) - .find((raw) => (raw.listener ?? raw) === entry.listener); + const registered = + entry.raw ?? + process.rawListeners(entry.event).find((raw) => (raw.listener ?? raw) === entry.listener); if (registered === undefined) return; if (kind === EVENT_SIGNAL) Reflect.apply(registered, process, [name, arg]); else Reflect.apply(registered, process, [arg]); diff --git a/packages/runtime/src/scr_node_island_host.h b/packages/runtime/src/scr_node_island_host.h index 73262a8ce..ed7697b85 100644 --- a/packages/runtime/src/scr_node_island_host.h +++ b/packages/runtime/src/scr_node_island_host.h @@ -282,31 +282,72 @@ static const char scr_node_island_host_js[] = " const kind = eventKind(event);\012" " if (kind === 0) return;\012" " const id = nextListener++;\012" - " islandListeners.push({ event, listener, id });\012" + " islandListeners.push({ event, listener, id, raw: undefined });\012" " nativeHost.listen(kind, event, id, 1, 0);\012" "});\012" "\012" + "function forget(i, kind) {\012" + " const entry = islandListeners[i];\012" + " islandListeners.splice(i, 1);\012" + " nativeHost.listen(kind, entry.event, entry.id, 0, 0);\012" + "}\012" + "\012" "process.on(\"removeListener\", (event, listener) => {\012" " const kind = eventKind(event);\012" " if (kind === 0) return;\012" + " // Node reports the removed registration as the raw listener it installed (a\012" + " // once wrapper removing itself reports the wrapper) or as the listener\012" + " // itself. The raw listener each entry recorded separates a once\012" + " // registration from an `on` registration of the same function: the removed\012" + " // one is the entry whose raw listener is no longer on `process`.\012" + " const live = process.rawListeners(event);\012" + " let same = -1;\012" " for (let i = islandListeners.length - 1; i >= 0; i--) {\012" " const entry = islandListeners[i];\012" - " if (entry.event === event && entry.listener === listener) {\012" - " islandListeners.splice(i, 1);\012" - " nativeHost.listen(kind, event, entry.id, 0, 0);\012" + " if (entry.event !== event) continue;\012" + " if (entry.listener !== listener && entry.raw !== listener) continue;\012" + " if (entry.raw !== undefined && !live.includes(entry.raw)) {\012" + " forget(i, kind);\012" " return;\012" " }\012" + " if (same < 0) same = i;\012" " }\012" + " // Duplicate registrations of one function share a raw listener; Node\012" + " // removes the most recent, which is the last matching entry.\012" + " if (same >= 0) forget(same, kind);\012" "});\012" "\012" + "// The raw listener is the function handed to the emitter (a once wrapper\012" + "// for process.once, which registers through process.on): tag each entry\012" + "// with it as its registration returns. Node emits newListener before it\012" + "// installs the listener, so the entry learns its raw only afterwards.\012" + "for (const name of [\"on\", \"addListener\", \"prependListener\"]) {\012" + " const add = process[name];\012" + " process[name] = function addListener(event, listener) {\012" + " const start = islandListeners.length;\012" + " const result = Reflect.apply(add, this, arguments);\012" + " if (islandListeners.length > start) {\012" + " const registered = listener.listener \077\077 listener;\012" + " for (let i = start; i < islandListeners.length; i++) {\012" + " const entry = islandListeners[i];\012" + " if (entry.event === event && entry.listener === registered && entry.raw === undefined) {\012" + " entry.raw = listener;\012" + " break;\012" + " }\012" + " }\012" + " }\012" + " return result;\012" + " };\012" + "}\012" + "\012" "function emit(kind, id, name, arg) {\012" " const entry = islandListeners.find((e) => e.id === id);\012" " if (entry === undefined) return;\012" " // The registered function: the listener itself, or its once wrapper,\012" " // which removes itself before calling it.\012" - " const registered = process\012" - " .rawListeners(entry.event)\012" - " .find((raw) => (raw.listener \077\077 raw) === entry.listener);\012" + " const registered =\012" + " entry.raw \077\077\012" + " process.rawListeners(entry.event).find((raw) => (raw.listener \077\077 raw) === entry.listener);\012" " if (registered === undefined) return;\012" " if (kind === EVENT_SIGNAL) Reflect.apply(registered, process, [name, arg]);\012" " else Reflect.apply(registered, process, [arg]);\012" diff --git a/tests/corpus/6804-node-island-signals/main.mjs b/tests/corpus/6804-node-island-signals/main.mjs index aaed001e7..c6d91ad7b 100644 --- a/tests/corpus/6804-node-island-signals/main.mjs +++ b/tests/corpus/6804-node-island-signals/main.mjs @@ -48,6 +48,16 @@ async function main() { process.removeListener("SIGUSR2", native); process.removeListener("SIGUSR2", second); process.removeListener("SIGHUP", hup); + + console.log("-- one function registered once and on"); + island("listenOnce", "SIGUSR2", "dup"); + island("listen", "SIGUSR2", "dup"); + process.kill(process.pid, "SIGUSR2"); + await later(50); + console.log("-- the once registration is gone"); + process.kill(process.pid, "SIGUSR2"); + await later(50); + island("unlisten", "SIGUSR2", "dup"); console.log("done"); } diff --git a/tests/corpus/6890-node-island-timer-refresh/echo.cjs b/tests/corpus/6890-node-island-timer-refresh/echo.cjs new file mode 100644 index 000000000..76d5cde1d --- /dev/null +++ b/tests/corpus/6890-node-island-timer-refresh/echo.cjs @@ -0,0 +1,3 @@ +exports.echo = function (n) { + return n; +}; diff --git a/tests/corpus/6890-node-island-timer-refresh/main.ts b/tests/corpus/6890-node-island-timer-refresh/main.ts new file mode 100644 index 000000000..3f5b68ee5 --- /dev/null +++ b/tests/corpus/6890-node-island-timer-refresh/main.ts @@ -0,0 +1,15 @@ +// @requires-libnode +// refresh() of a timer armed before Node loads restarts it with the delay +// it was created with, not the remaining delay it had at handoff: a due +// timer it refreshes does not run before Node's loop starts. The timer +// armed after the refresh fires first. +import { callSync } from "@scriptc/node"; + +const slow = setTimeout(() => console.log("slow"), 1000); +// Most of the timer's delay passes before Node loads, so its runtime entry +// reaches the hosted loop with little time left (or already due). +const start = Date.now(); +while (Date.now() - start < 800) {} +console.log("loaded", callSync("./echo.cjs", "echo", 1)); +slow.refresh(); +setTimeout(() => console.log("fast"), 400); diff --git a/tests/harness/corpus-costs/6890-node-island-timer-refresh/main.ts.json b/tests/harness/corpus-costs/6890-node-island-timer-refresh/main.ts.json new file mode 100644 index 000000000..8affc810d --- /dev/null +++ b/tests/harness/corpus-costs/6890-node-island-timer-refresh/main.ts.json @@ -0,0 +1 @@ +{ "plain": 3300, "san": 23000 }