Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
76 changes: 58 additions & 18 deletions src/aw-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,32 +83,72 @@
limit?: number;
}

function makeTimeoutAbortSignal(
type TimeoutSignalResult = {
signal?: AbortSignal;
cleanup: () => void;
};

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 };
): TimeoutSignalResult {
// Create a per-request AbortController and explicitly clean up both the timeout
// and the propagated abort listener when the request finishes.
//
// The old code leaked because it added an "abort" listener to the long-lived
// AWClient.controller.signal on every request but never removed it. In consumers
// like aw-watcher-web (heartbeat every ~5 s), that accumulated hundreds of
// thousands of AbortController instances over days of browsing.
// See: https://github.com/ActivityWatch/aw-watcher-web/issues/222
if (timeout === undefined && existingSignal === undefined) {
return { signal: undefined, cleanup: () => undefined };
}

const controller = new AbortController();
let timeoutId: ReturnType<typeof setTimeout> | undefined;
let abortListener: (() => void) | undefined;

const cleanup = () => {
if (timeoutId !== undefined) {
clearTimeout(timeoutId);
timeoutId = undefined;
}
if (existingSignal !== undefined && abortListener !== undefined) {
existingSignal.removeEventListener("abort", abortListener);
abortListener = undefined;
}
};

if (timeout !== undefined) {
timeoutId = setTimeout(() => {
controller.abort();
cleanup();
}, timeout);
}

if (existingSignal !== undefined) {
if (existingSignal.aborted) {
controller.abort();
cleanup();
return { signal: controller.signal, cleanup };
}

abortListener = () => {
controller.abort();
cleanup();
};
existingSignal.addEventListener("abort", abortListener, { once: true });
}

return { signal: controller.signal, cleanup };
}

async function fetchWithFailure(
input: string,
init: RequestInit,
timeout?: number,
): Promise<Response> {
const { signal, timeoutId } = makeTimeoutAbortSignal(
const { signal, cleanup } = makeTimeoutSignal(
timeout,
init.signal || undefined,
);
Expand All @@ -117,7 +157,7 @@
if (res.status >= 300) throw new FetchError(res);
return res;
})
.finally(() => clearTimeout(timeoutId));
.finally(cleanup);
}

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

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

Check warning on line 213 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
61 changes: 61 additions & 0 deletions src/test/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,65 @@
awc.abort();
return caught;
});

it("cleans up propagated abort listeners after a successful request", async () => {
const awc = new AWClient(clientName, {
testing: true,
timeout: 30_000,
});

const signal = awc.controller.signal;
const originalAddEventListener = signal.addEventListener.bind(signal);
const originalRemoveEventListener =
signal.removeEventListener.bind(signal);
const originalFetch = global.fetch;

let activeAbortListeners = 0;
let addCalls = 0;
let removeCalls = 0;

signal.addEventListener = ((
type: string,
listener: any,

Check warning on line 254 in src/test/test.ts

View workflow job for this annotation

GitHub Actions / lint (18.x)

Unexpected any. Specify a different type
options?: any,

Check warning on line 255 in src/test/test.ts

View workflow job for this annotation

GitHub Actions / lint (18.x)

Unexpected any. Specify a different type
) => {
if (type === "abort" && listener !== null) {
activeAbortListeners += 1;
addCalls += 1;
}
originalAddEventListener(type, listener, options);
}) as typeof signal.addEventListener;

signal.removeEventListener = ((
type: string,
listener: any,

Check warning on line 266 in src/test/test.ts

View workflow job for this annotation

GitHub Actions / lint (18.x)

Unexpected any. Specify a different type
options?: any,

Check warning on line 267 in src/test/test.ts

View workflow job for this annotation

GitHub Actions / lint (18.x)

Unexpected any. Specify a different type
) => {
if (type === "abort" && listener !== null) {
activeAbortListeners -= 1;
removeCalls += 1;
}
originalRemoveEventListener(type, listener, options);
}) as typeof signal.removeEventListener;

global.fetch = (() =>
Promise.resolve(
new Response(JSON.stringify({ testing: true }), {
status: 200,
headers: { "Content-Type": "application/json" },
}),
)) as typeof fetch;

try {
const resp = await awc.getInfo();
assert.equal(resp.testing, true);
assert.equal(addCalls, 1);
assert.equal(removeCalls, 1);
assert.equal(activeAbortListeners, 0);
} finally {
signal.addEventListener = originalAddEventListener;
signal.removeEventListener = originalRemoveEventListener;
global.fetch = originalFetch;
}
});
});
Loading