fix: replace persistent abort listener with AbortSignal.any() to prevent memory leak

[TimeToBuildBob]

Problem

makeTimeoutAbortSignal() attached an addEventListener("abort", ...) listener to existingSignal (which is AWClient.controller.signal) on every request, but never removed it. Since AWClient is a singleton in long-running consumers, controller.signal accumulates one listener per request — forever.

In practice: aw-watcher-web sends a heartbeat every ~5 seconds. After 5 days of browsing without restarting Firefox, this produced ~250,000 accumulated AbortController instances (reported by about:memory as ~123 MB of AbortController objects + ~645 MB of closures), causing 5 GB+ RSS and 100% CPU in the extensions process.

See: ActivityWatch/aw-watcher-web#222

Fix

Replace the manual setTimeout + addEventListener pattern with AbortSignal.any() + AbortSignal.timeout():

• AbortSignal.timeout(ms) is a self-managing built-in — no clearTimeout needed
• AbortSignal.any([...signals]) uses weak-reference semantics (per the WHATWG spec; verified in Blink/Gecko): the combined signal — and its closure references — become GC-eligible once the fetch holds no more references to it

Browser support: Chrome 116+, Firefox 115+, Safari 17.4+, Node.js 20.3+

Before / After

// Before — leaks one listener per request on existingSignal
const abortController = new AbortController();
const timeoutId = setTimeout(() => abortController.abort(), timeout);
existingSignal?.addEventListener("abort", () => abortController.abort()); // ← never removed!
return { signal: abortController.signal, timeoutId };

// After — no persistent listeners, GC-eligible after fetch completes
const signals: AbortSignal[] = [];
if (timeout !== undefined) signals.push(AbortSignal.timeout(timeout));
if (existingSignal !== undefined) signals.push(existingSignal);
return signals.length === 1 ? signals[0] : AbortSignal.any(signals);

[greptile-apps]

Greptile Summary

This PR fixes a genuine memory leak in makeTimeoutAbortSignal where an \"abort\" listener was added to the long-lived AWClient.controller.signal on every request but never removed. The replacement makeTimeoutSignal uses an explicit cleanup() closure that calls both clearTimeout and removeEventListener, invoked via .finally() on every request path. A focused unit test verifies the listener is properly cleaned up after a successful request.

Note: the PR title and the "Before/After" section of the description describe an AbortSignal.any() + AbortSignal.timeout() approach, but the merged code uses explicit removeEventListener cleanup instead — which is actually the better choice (broader compatibility, and it addresses both prior review concerns). The PR title and description should be updated to avoid confusing future readers of the git history.

Confidence Score: 5/5

Safe to merge — the fix is correct, idempotent, and well-tested; both prior review concerns are resolved by the explicit cleanup approach.

All execution paths (success, timeout, external abort, pre-aborted signal) correctly invoke the idempotent cleanup() exactly once, removing the listener and clearing the timer. No remaining P0/P1 findings; the only open item is a PR title/description mismatch that does not affect runtime behaviour.

No files require special attention.

Important Files Changed

Filename	Overview
src/aw-client.ts	Replaces the leaking makeTimeoutAbortSignal with makeTimeoutSignal using an idempotent cleanup() that calls clearTimeout and removeEventListener("abort", …) via .finally(); logic is correct across all abort/timeout/success paths.
src/test/test.ts	Adds a unit test that monkey-patches addEventListener/removeEventListener on the live signal to assert exactly one listener is added and removed per successful request; test is well-structured and correctly restores globals in finally.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["makeTimeoutSignal(timeout, existingSignal)"] --> B{Both undefined?}
    B -- Yes --> C["return { signal: undefined, cleanup: no-op }"]
    B -- No --> D["new AbortController()"]
    D --> E{timeout defined?}
    E -- Yes --> F["setTimeout → controller.abort() + cleanup()"]
    E -- No --> G{existingSignal defined?}
    F --> G
    G -- Yes --> H{existingSignal.aborted?}
    H -- Yes --> I["controller.abort() + cleanup() → early return"]
    H -- No --> J["addEventListener('abort', abortListener, {once:true})"]
    J --> K["return { signal: controller.signal, cleanup }"]
    G -- No --> K
    K --> L["fetch(…, { signal })"]
    L --> M[".finally(cleanup)"]
    M --> N["clearTimeout(timeoutId)\nremoveEventListener('abort', abortListener)"]

Loading

_{Reviews (2): Last reviewed commit: "style: run prettier on test.ts to fix li..." | Re-trigger Greptile}

[greptile-apps]

Browser/Node.js runtime compatibility gap

AbortSignal.any() requires Chrome 116+, Firefox 115+, Safari 17.4+ (March 2024), and Node.js 20.3+. On any older runtime, AbortSignal.any is undefined, so makeTimeoutSignal will throw TypeError: AbortSignal.any is not a function on every request where both a timeout and an existing signal are present — effectively breaking all network calls. Node.js 18 LTS (EOL April 2025) and Safari < 17.4 are the most likely affected environments.

The trade-off is explicitly documented in the PR description, but since package.json has no engines field and tsconfig.json targets es6, downstream consumers have no compile-time or install-time warning. Consider adding "engines": { "node": ">=20.3" } to package.json and/or a one-line runtime guard like:

if (typeof AbortSignal.any !== "function") {
    // fallback: return existingSignal or a basic timeout signal
}

[greptile-apps]

Timeout timer not cancelled after successful fetch

The old code called clearTimeout(timeoutId) in .finally() to immediately release the timer once the request completed. AbortSignal.timeout() provides no equivalent hook, so its internal timer continues running for the full timeout duration even after the response has been received. In Node.js 20+ this is benign because the timer is unref()-ed and won't delay process exit. In browsers it is also harmless. Still, for a this.timeout = 30_000 ms default, each completed request now holds a live (though side-effect-free) timer for up to 30 seconds, which could be surprising during profiling or in environments with very high request volume.

[ErikBjare]

@TimeToBuildBob Address review comments?

[TimeToBuildBob]

Addressed the review comments.

I dropped the AbortSignal.any() / AbortSignal.timeout() approach and replaced it with a compatibility-safe cleanup version:

• still uses a per-request AbortController
• still propagates aborts from AWClient.controller.signal
• now removes the propagated abort listener after the request finishes
• also clears the timeout in .finally() again

So this keeps the memory leak fix without requiring newer runtimes.

I also added a focused test that verifies the propagated abort listener count returns to zero after a successful request.

Commit pushed: 8bffa29

[ErikBjare]

@greptileai review