Skip to content

fix(client): clean up abort listener in fetchWithTimeout to prevent memory leak #2151

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
lchetanlc wants to merge 2 commits into
base: main
Choose a base branch
from
+15 −2
Open

fix(client): clean up abort listener in fetchWithTimeout to prevent memory leak #2151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4bef397
fix(client): clean up abort listener in fetchWithTimeout to prevent m…
Aug 9, 2026
9a6a077
fix(client): delay abort listener cleanup until response consumption …
Aug 9, 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
10 changes: 8 additions & 2 deletions src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,13 @@ export class OpenAI {

const security = options.__security ?? { bearerAuth: true };
const controller = new AbortController();
const abort = this._makeAbort(controller);
let abortCleanup: (() => void) | undefined;
if (options.signal) {
options.signal.addEventListener('abort', abort);

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Remove abort listeners before retrying or throwing

Because this listener is installed for every attempt but only the final successfully parsed response returns its actionToRunOnComplete to defaultParseResponse, retry and terminal error paths in makeRequest never remove it. With a request-option AbortSignal.timeout() that gets a retryable 429/500 (or a final API/connection error), the earlier attempt's listener stays attached and Deno can still remain alive until the timeout; call the cleanup in those paths before retryRequest or throwing.

Useful? React with 👍 / 👎.

abortCleanup = () => options.signal!.removeEventListener('abort', abort);
}

const response = await this.fetchWithAuth(url, req, timeout, controller, security).catch(castToError);
const headersTime = Date.now();

Expand Down Expand Up @@ -949,7 +956,7 @@ export class OpenAI {
}),
);

return { response, options, controller, requestLogID, retryOfRequestLogID, startTime };
return { response, options, controller, actionToRunOnComplete: abortCleanup, requestLogID, retryOfRequestLogID, startTime };

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clean up raw Response paths

For successful requests that return a Response instead of parsed data—APIPromise.asResponse() and __binaryResponse resources such as files/audio—the only cleanup handle is stored in APIResponseProps and later invoked by defaultParseResponse. Those paths either bypass parsing or explicitly skip the cleanup, so a request with AbortSignal.timeout() can still keep Deno alive after the caller has consumed the response; remove the listener when the returned response body is consumed or cancelled.

Useful? React with 👍 / 👎.

}

getAPIList<Item, PageClass extends Pagination.AbstractPage<Item> = Pagination.AbstractPage<Item>>(
Expand Down Expand Up @@ -1008,7 +1015,6 @@ export class OpenAI {
): Promise<Response> {
const { signal, method, ...options } = init || {};
const abort = this._makeAbort(controller);
if (signal) signal.addEventListener('abort', abort, { once: true });

const timeout = setTimeout(abort, ms);

Expand Down
1 change: 1 addition & 0 deletions src/core/streaming.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export class Stream<Item> implements AsyncIterable<Item> {
controller: AbortController,
client?: OpenAI,
synthesizeEventData?: boolean,
cleanup?: () => void,

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Invoke stream cleanup when iteration finishes

For stream: true requests using a request-option signal (especially AbortSignal.timeout() in Deno), makeRequest now installs the abort listener and defaultParseResponse passes the cleanup callback here, but fromSSEResponse never invokes cleanup. After the SSE stream ends, is broken, or errors, the user's signal still retains the listener, so the memory/process-liveness leak this change is meant to fix remains for streaming responses; run the callback from the iterator's finally once the response is no longer being consumed.

Useful? React with 👍 / 👎.

): Stream<Item> {
let consumed = false;
const logger = client ? loggerFor(client) : console;
Expand Down
6 changes: 6 additions & 0 deletions src/internal/parse.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export type APIResponseProps = {
requestLogID: string;
retryOfRequestLogID: string | undefined;
startTime: number;
actionToRunOnComplete?: () => void;
};

export async function defaultParseResponse<T>(
Expand All @@ -33,6 +34,7 @@ export async function defaultParseResponse<T>(
props.controller,
client,
props.options.__synthesizeEventData,
props.actionToRunOnComplete,
) as any;
}

Expand All @@ -41,6 +43,7 @@ export async function defaultParseResponse<T>(
props.controller,
client,
props.options.__synthesizeEventData,
props.actionToRunOnComplete,
) as any;
}

Expand Down Expand Up @@ -80,6 +83,9 @@ export async function defaultParseResponse<T>(
durationMs: Date.now() - startTime,
}),
);
if (!props.options.stream && !props.options.__binaryResponse) {
props.actionToRunOnComplete?.();
}
return body;
}

Expand Down