Skip to content

Commit aeab9f9

Browse files
authored
fix(reqresp): clear composed response timeout signals (#9029)
## Summary Open the Lodestar side of the memory-leak mitigation by clearing composed response-timeout signals in req/resp request handling. This is the Lodestar-local follow-up for the network-thread leak investigation in #8969. It addresses the req/resp timeout-signal retention path by replacing the `AbortSignal.any(...)` composition with a clearable composed signal and explicitly clearing listeners once the response stream is done. This PR is intentionally scoped to the req/resp fix only. The separate residual gossipsub topic-retention issue is being tracked upstream in `ChainSafe/js-libp2p-gossipsub#543`, and we can pick that up once a new libp2p release lands. ## Change - add `createRespSignal()` helper in `packages/reqresp/src/request/index.ts` - compose request signal + timeout signal via an `AbortController` - remove abort listeners explicitly via `respSignal.clear()` after the response closes ## Validation - `pnpm build` - `pnpm lint` *(warning-only: pre-existing unused biome suppression in `packages/light-client/test/unit/webEsmBundle.browser.test.ts`)* ## AI disclosure This PR was authored with AI assistance (Lodekeeper 🌟). --------- Co-authored-by: lodekeeper <lodekeeper@users.noreply.github.com>
1 parent 1ddf8e4 commit aeab9f9

1 file changed

Lines changed: 38 additions & 3 deletions

File tree

packages/reqresp/src/request/index.ts

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,42 @@ function scheduleStreamAbortIfNotClosed(stream: Stream, timeoutMs: number): void
3636
stream.addEventListener("close", onClose, {once: true});
3737
}
3838

39+
type ClearableSignal = AbortSignal & {clear: () => void};
40+
41+
function createRespSignal(signal: AbortSignal | undefined, timeoutMs: number): ClearableSignal {
42+
const timeoutSignal = AbortSignal.timeout(timeoutMs);
43+
const signals = signal ? [signal, timeoutSignal] : [timeoutSignal];
44+
const controller = new AbortController();
45+
46+
const clear = (): void => {
47+
for (const entry of signals) {
48+
entry.removeEventListener("abort", onAbort);
49+
}
50+
};
51+
52+
const onAbort = (): void => {
53+
if (controller.signal.aborted) {
54+
return;
55+
}
56+
const reason = signals.find((entry) => entry.aborted)?.reason;
57+
controller.abort(reason);
58+
clear();
59+
};
60+
61+
for (const entry of signals) {
62+
if (entry.aborted) {
63+
onAbort();
64+
break;
65+
}
66+
entry.addEventListener("abort", onAbort);
67+
}
68+
69+
const respSignal = controller.signal as ClearableSignal;
70+
respSignal.clear = clear;
71+
72+
return respSignal;
73+
}
74+
3975
export interface SendRequestOpts {
4076
/** The maximum time for complete response transfer. */
4177
respTimeoutMs?: number;
@@ -154,9 +190,7 @@ export async function* sendRequest(
154190
}
155191

156192
// RESP_TIMEOUT: Maximum time for complete response transfer
157-
const respSignal = signal
158-
? AbortSignal.any([signal, AbortSignal.timeout(RESP_TIMEOUT)])
159-
: AbortSignal.timeout(RESP_TIMEOUT);
193+
const respSignal = createRespSignal(signal, RESP_TIMEOUT);
160194

161195
let responseError: Error | null = null;
162196
let responseFullyConsumed = false;
@@ -199,6 +233,7 @@ export async function* sendRequest(
199233
}
200234
}
201235
}
236+
respSignal.clear();
202237
metrics?.outgoingClosedStreams?.inc({method});
203238
logger.verbose("Req stream closed", logCtx);
204239
}

0 commit comments

Comments
 (0)