From 4eea02eb9ccb4287cd19b0925752c02f5948a1f8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:11:43 +0000 Subject: [PATCH 1/5] AbortSignal.timeout: keep the timer armed when the signal loses its observers eventListenersDidChange() cancelled the native timeout timer as soon as the signal had no abort listeners, native callbacks, pending activity, algorithms or dependent signals left. A signal the program still holds then never aborts: signal.aborted stays false after the deadline, throwIfAborted() never throws, and listeners or AbortSignal.any() dependents attached later never fire. Adding a listener for an unrelated event type was enough to trigger it, as was Bun.spawn() releasing the signal when its child exited. The cancel was added when the timer held a ref on the signal and was the only way to free an unobserved one. Since the wrapper became the sole owner, collecting an unobserved signal frees the timer through ~AbortSignal(), so the timer now lives until the signal aborts or is destroyed, as in Node and the browsers. --- src/jsc/bindings/webcore/AbortSignal.cpp | 13 +-- test/js/web/abort/abort.test.ts | 113 ++++++++++++++++++++++- 2 files changed, 117 insertions(+), 9 deletions(-) diff --git a/src/jsc/bindings/webcore/AbortSignal.cpp b/src/jsc/bindings/webcore/AbortSignal.cpp index 5ebd0d4aedde..97ed1c1f8cb6 100644 --- a/src/jsc/bindings/webcore/AbortSignal.cpp +++ b/src/jsc/bindings/webcore/AbortSignal.cpp @@ -72,6 +72,11 @@ Ref AbortSignal::timeout(ScriptExecutionContext& context, uint64_t // alive while an abort listener or an AbortSignal.any() dependent observes // the timeout. With no observer, collecting the wrapper destroys the // signal and ~AbortSignal() cancels and frees the timer. + // + // Those are the only two things that stop the timer (see markAborted() and + // ~AbortSignal()). Losing every observer must not: a signal JS still holds + // has to read as aborted once its deadline passes, whether or not anything + // was listening when it did. signal->m_timeout = AbortSignal__Timeout__create(bunVM(context.vm()), signal.ptr(), milliseconds); ASSERT(signal->m_timeout); return signal; @@ -297,14 +302,6 @@ void AbortSignal::eventListenersDidChange() else m_timeoutObserverCount.fetch_sub(1, std::memory_order_relaxed); } - - // When a timeout signal loses all observers there is nothing left to - // notify when the timer fires, so cancel it eagerly. - // JSAbortSignalOwner::isReachableFromOpaqueRoots then no longer keeps the - // wrapper alive and ~AbortSignal() runs on collection; this just frees the - // native timer sooner. - if (m_timeout && !aborted() && !hasTimeoutObserver()) - cancelTimer(); } uint32_t AbortSignal::addAbortAlgorithmToSignal(AbortSignal& signal, Ref&& algorithm) diff --git a/test/js/web/abort/abort.test.ts b/test/js/web/abort/abort.test.ts index 7e3ae3a0acc4..41ba41877b7c 100644 --- a/test/js/web/abort/abort.test.ts +++ b/test/js/web/abort/abort.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { writeFileSync } from "fs"; -import { bunEnv, bunExe, tempDir, tmpdirSync } from "harness"; +import { bunEnv, bunExe, isWindows, tempDir, tmpdirSync } from "harness"; import { tmpdir } from "os"; import { join } from "path"; @@ -180,3 +180,114 @@ describe("AbortSignal", () => { expect(exitCode).toBe(0); }); }); + +// https://dom.spec.whatwg.org/#dom-abortsignal-timeout: a timeout signal aborts +// once its deadline passes for as long as the signal exists. Whether anything +// was listening in the meantime only matters for GC, never for whether it fires. +// The native timer used to be cancelled as soon as the signal's listener count +// dropped to zero, leaving a signal the program still held stuck at +// aborted === false. Node and the browsers abort it in every case below. +describe.concurrent("AbortSignal.timeout() still fires after its observers go away", () => { + // Resolves after the deadline of a signal armed before this call with a + // shorter delay: the fence sits behind it in the timer heap, so by the time + // the fence fires that signal's own timer has had its turn. Waiting on a + // fence rather than on the signal under test keeps the latter unobserved. + function fence(ms: number): Promise { + const signal = AbortSignal.timeout(ms); + return new Promise(resolve => signal.addEventListener("abort", () => resolve(), { once: true })); + } + + function summarize(signal: AbortSignal) { + return { aborted: signal.aborted, reason: signal.reason?.name }; + } + + const churn: [string, (signal: AbortSignal) => void][] = [ + [ + "an abort listener was added and removed", + signal => { + const listener = () => {}; + signal.addEventListener("abort", listener); + signal.removeEventListener("abort", listener); + }, + ], + [ + "onabort was set and cleared", + signal => { + signal.onabort = () => {}; + signal.onabort = null; + }, + ], + [ + "its abort listener was removed through the listener's { signal } option", + signal => { + const controller = new AbortController(); + signal.addEventListener("abort", () => {}, { signal: controller.signal }); + controller.abort(); + }, + ], + [ + // Not even a removal: adding a listener for any other event type updates + // the listener bookkeeping while the abort listener count is still zero. + "a listener for an unrelated event type was added", + signal => { + signal.addEventListener("unrelated", () => {}); + }, + ], + ]; + + test.each(churn)("after %s", async (_, apply) => { + const signal = AbortSignal.timeout(1); + apply(signal); + await fence(20); + expect(summarize(signal)).toEqual({ aborted: true, reason: "TimeoutError" }); + }); + + test("consumers attached after the listener churn still see the abort", async () => { + const signal = AbortSignal.timeout(1); + const listener = () => {}; + signal.addEventListener("abort", listener); + signal.removeEventListener("abort", listener); + + const events: string[] = []; + signal.addEventListener("abort", event => events.push(event.type)); + const dependent = AbortSignal.any([signal]); + await fence(20); + + let thrown: DOMException | undefined; + try { + signal.throwIfAborted(); + } catch (error) { + thrown = error as DOMException; + } + expect({ events, dependent: summarize(dependent), thrown: thrown?.name }).toEqual({ + events: ["abort"], + dependent: { aborted: true, reason: "TimeoutError" }, + thrown: "TimeoutError", + }); + expect(thrown).toBe(signal.reason); + }); + + // Native consumers observe the signal through a native callback instead of a + // JS listener; Bun.spawn() registers one and releases it when the child exits. + test("after the Bun.spawn() child it was passed to has exited", async () => { + // The child has to be gone before the deadline. A shell exits in ~1ms + // (a debug build of bun takes 100ms+ just to start), so 500ms leaves + // plenty of room for a loaded CI machine. + const signal = AbortSignal.timeout(500); + const deadline = fence(600); + await using proc = Bun.spawn({ + cmd: isWindows ? [process.env.comspec || "cmd.exe", "/c", "exit", "0"] : ["/bin/sh", "-c", "exit 0"], + signal, + stdout: "ignore", + stderr: "ignore", + }); + expect({ exitCode: await proc.exited, ...summarize(signal) }).toEqual({ + exitCode: 0, + aborted: false, + reason: undefined, + }); + + await deadline; + expect(summarize(signal)).toEqual({ aborted: true, reason: "TimeoutError" }); + }); +}); From 466724b7170a9fd033f1bc366bcbfc59ae0e5c16 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 00:40:36 +0000 Subject: [PATCH 2/5] test: a Request-held timeout signal still fires after its wrapper is collected --- test/js/web/abort/abort.test.ts | 41 +++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/js/web/abort/abort.test.ts b/test/js/web/abort/abort.test.ts index 41ba41877b7c..86b5a26533f0 100644 --- a/test/js/web/abort/abort.test.ts +++ b/test/js/web/abort/abort.test.ts @@ -290,4 +290,45 @@ describe.concurrent("AbortSignal.timeout() still fires after its observers go aw await deadline; expect(summarize(signal)).toEqual({ aborted: true, reason: "TimeoutError" }); }); + + // A Request keeps its signal as a native ref and wraps it again on demand, so + // the signal's JS wrapper can be collected while the timeout is still + // observable through request.signal. The timer has to survive the wrapper as + // well; only the signal itself going away (or aborting) may stop it. + // Subprocess so heapStats() only sees this scenario's signals. + test("after its wrapper was collected while a Request still held the signal", async () => { + const src = ` + const { heapStats } = require("bun:jsc"); + const wrappers = () => heapStats().objectTypeCounts.AbortSignal ?? 0; + const N = 32; + // A full GC of the debug heap takes ~100ms under ASAN; the deadline only + // has to come after it. + const deadline = 1000; + const started = performance.now(); + const requests = []; + for (let i = 0; i < N; i++) { + requests.push(new Request("http://localhost/", { signal: AbortSignal.timeout(deadline) })); + } + const fence = AbortSignal.timeout(deadline + 100); + const withWrappers = wrappers(); + // Fresh stack first, so nothing conservatively scanned still points at a wrapper. + await new Promise(resolve => setImmediate(resolve)); + Bun.gc(true); + const collected = withWrappers - wrappers(); + const collectedBeforeDeadline = performance.now() - started < deadline; + await new Promise(resolve => fence.addEventListener("abort", resolve, { once: true })); + // request.signal wraps the native signal again. + const timedOut = requests.filter(r => r.signal.aborted && r.signal.reason.name === "TimeoutError").length; + console.log(JSON.stringify({ N, collected, collectedBeforeDeadline, timedOut })); + `; + await using proc = Bun.spawn({ cmd: [bunExe(), "-e", src], env: bunEnv, stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const { N, collected, collectedBeforeDeadline, timedOut } = JSON.parse(stdout); + // The wrappers have to be gone before the timers fire for the run to mean + // anything; allow a straggler or two in case something still pins one. + expect(collected).toBeGreaterThanOrEqual(N - 4); + expect({ collectedBeforeDeadline, timedOut }).toEqual({ collectedBeforeDeadline: true, timedOut: N }); + expect(exitCode).toBe(0); + }); }); From 5ddeb4d91d2bc15ace37914d2295cbf91c05aa68 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 00:45:33 +0000 Subject: [PATCH 3/5] AbortSignal.timeout: shorten the timer lifetime comment --- src/jsc/bindings/webcore/AbortSignal.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/jsc/bindings/webcore/AbortSignal.cpp b/src/jsc/bindings/webcore/AbortSignal.cpp index 97ed1c1f8cb6..3e0b379d60bb 100644 --- a/src/jsc/bindings/webcore/AbortSignal.cpp +++ b/src/jsc/bindings/webcore/AbortSignal.cpp @@ -71,12 +71,9 @@ Ref AbortSignal::timeout(ScriptExecutionContext& context, uint64_t // The JS wrapper is the sole owner; isReachableFromOpaqueRoots keeps it // alive while an abort listener or an AbortSignal.any() dependent observes // the timeout. With no observer, collecting the wrapper destroys the - // signal and ~AbortSignal() cancels and frees the timer. - // - // Those are the only two things that stop the timer (see markAborted() and - // ~AbortSignal()). Losing every observer must not: a signal JS still holds - // has to read as aborted once its deadline passes, whether or not anything - // was listening when it did. + // signal and ~AbortSignal() cancels and frees the timer. Nothing else may + // cancel it: whoever still holds the signal (JS, or a Request that hands it + // back out) must see it abort at its deadline, listeners or not. signal->m_timeout = AbortSignal__Timeout__create(bunVM(context.vm()), signal.ptr(), milliseconds); ASSERT(signal->m_timeout); return signal; From 8f6d13077a71d7293b1f3fbaafc85684f10f3254 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 00:49:22 +0000 Subject: [PATCH 4/5] AbortSignal.timeout: leave the timer lifetime comment as it was --- src/jsc/bindings/webcore/AbortSignal.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/jsc/bindings/webcore/AbortSignal.cpp b/src/jsc/bindings/webcore/AbortSignal.cpp index 3e0b379d60bb..88a58421d5b4 100644 --- a/src/jsc/bindings/webcore/AbortSignal.cpp +++ b/src/jsc/bindings/webcore/AbortSignal.cpp @@ -71,9 +71,7 @@ Ref AbortSignal::timeout(ScriptExecutionContext& context, uint64_t // The JS wrapper is the sole owner; isReachableFromOpaqueRoots keeps it // alive while an abort listener or an AbortSignal.any() dependent observes // the timeout. With no observer, collecting the wrapper destroys the - // signal and ~AbortSignal() cancels and frees the timer. Nothing else may - // cancel it: whoever still holds the signal (JS, or a Request that hands it - // back out) must see it abort at its deadline, listeners or not. + // signal and ~AbortSignal() cancels and frees the timer. signal->m_timeout = AbortSignal__Timeout__create(bunVM(context.vm()), signal.ptr(), milliseconds); ASSERT(signal->m_timeout); return signal; From 88adbf084fa3666a712b80327996ffd43a7f6a62 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 09:21:23 +0000 Subject: [PATCH 5/5] test: fs.watch() closing also used to cancel its timeout signal --- test/js/web/abort/abort.test.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/test/js/web/abort/abort.test.ts b/test/js/web/abort/abort.test.ts index 86b5a26533f0..c07b5856bd83 100644 --- a/test/js/web/abort/abort.test.ts +++ b/test/js/web/abort/abort.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { writeFileSync } from "fs"; +import { watch, writeFileSync } from "fs"; import { bunEnv, bunExe, isWindows, tempDir, tmpdirSync } from "harness"; import { tmpdir } from "os"; import { join } from "path"; @@ -233,6 +233,14 @@ describe.concurrent("AbortSignal.timeout() still fires after its observers go aw signal.addEventListener("unrelated", () => {}); }, ], + [ + // Native consumers observe the signal through a native callback rather + // than a JS listener; fs.watch() drops its callback synchronously in close(). + "the fs.watch() it was passed to was closed", + signal => { + watch(import.meta.dir, { signal }).close(); + }, + ], ]; test.each(churn)("after %s", async (_, apply) => { @@ -267,8 +275,8 @@ describe.concurrent("AbortSignal.timeout() still fires after its observers go aw expect(thrown).toBe(signal.reason); }); - // Native consumers observe the signal through a native callback instead of a - // JS listener; Bun.spawn() registers one and releases it when the child exits. + // Same native-callback release as fs.watch() above, but asynchronous: the + // subprocess lets go of the signal when the child exits. test("after the Bun.spawn() child it was passed to has exited", async () => { // The child has to be gone before the deadline. A shell exits in ~1ms // (a debug build of bun takes 100ms+ just to start), so 500ms leaves