Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TimeToBuildBob wants to merge 3 commits into
base: master
Choose a base branch
from
Open

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

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0ac117f
fix: replace persistent abort listener with AbortSignal.any()
TimeToBuildBob Apr 15, 2026
8bffa29
fix(abort): restore compatibility while cleaning up listeners
TimeToBuildBob Apr 16, 2026
aa2f775
style: run prettier on test.ts to fix lint CI
TimeToBuildBob Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 24 additions & 26 deletions src/aw-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,41 +83,39 @@
limit?: number;
}

function makeTimeoutAbortSignal(
function makeTimeoutSignal(
timeout?: number,
existingSignal?: AbortSignal,
) {
if (timeout === undefined)
return { signal: existingSignal, timeoutId: undefined };
const abortController = new AbortController();
const timeoutId = setTimeout(
() => abortController.abort(),
timeout || 10000,
);
// Sync with existing abort signal if it exists
if (existingSignal?.aborted) abortController.abort();
else
existingSignal?.addEventListener("abort", () =>
abortController.abort(),
);
return { signal: abortController.signal, timeoutId };
): AbortSignal | undefined {
// Use AbortSignal.any() + AbortSignal.timeout() instead of manually wiring an
// "abort" event listener onto existingSignal.
//
// The previous approach created a new AbortController per request and attached a
// persistent listener to existingSignal (AWClient.controller.signal). That listener
// was never removed, so after days of use — aw-watcher-web fires a heartbeat every
// ~5 s — hundreds of thousands of AbortController instances accumulated, causing
// 5 GB+ RSS and 100% CPU in the Firefox extensions process.
// See: https://github.com/ActivityWatch/aw-watcher-web/issues/222
//
// AbortSignal.timeout() and AbortSignal.any() are supported in:
// Chrome 116+, Firefox 115+, Safari 17.4+, Node.js 20.3+
if (timeout === undefined && existingSignal === undefined) return undefined;
const signals: AbortSignal[] = [];
if (timeout !== undefined) signals.push(AbortSignal.timeout(timeout));
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

if (existingSignal !== undefined) signals.push(existingSignal);
return signals.length === 1 ? signals[0] : AbortSignal.any(signals);
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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
}

}

async function fetchWithFailure(
input: string,
init: RequestInit,
timeout?: number,
): Promise<Response> {
const { signal, timeoutId } = makeTimeoutAbortSignal(
timeout,
init.signal || undefined,
);
return fetch(input, { ...init, signal })
.then((res) => {
if (res.status >= 300) throw new FetchError(res);
return res;
})
.finally(() => clearTimeout(timeoutId));
const signal = makeTimeoutSignal(timeout, init.signal || undefined);
return fetch(input, { ...init, signal }).then((res) => {
if (res.status >= 300) throw new FetchError(res);
return res;
});
}

export class AWClient {
Expand Down Expand Up @@ -170,7 +168,7 @@
).then((res) => res.json() as Promise<T>);
}

private async _post(endpoint: string, data: Record<string, any>) {

Check warning on line 171 in src/aw-client.ts

View workflow job for this annotation

GitHub Actions / lint (18.x) 

Unexpected any. Specify a different type
return fetchWithFailure(
`${this.apiURL}${endpoint}`,
{
Expand Down
Loading