Skip to content

fix(core): address memory leaks in oauth flow, chat history, and shell parsing #24963

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

Closed
spencer426 wants to merge 5 commits into from
Closed

fix(core): address memory leaks in oauth flow, chat history, and shell parsing #24963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
200d240
fix(core): address memory leaks in oauth flow, chat history, and shel…
spencer426 Apr 8, 2026
9b0c10f
fix(core): clear tree-sitter initialization timeout on resolution
spencer426 Apr 8, 2026
a58035a
Merge branch 'main' into fix/memory-leaks
spencer426 Apr 8, 2026
a020164
fix(core): apply history truncation to ChatRecordingService
spencer426 Apr 8, 2026
e79064c
fix(core): refactor shell parser initialization state and timeout
spencer426 Apr 8, 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: 5 additions & 5 deletions packages/cli/src/ui/hooks/useAtCompletion.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { useEffect, useReducer, useRef } from 'react';
import { useEffect, useReducer, useRef, useCallback } from 'react';
import { setTimeout as setTimeoutPromise } from 'node:timers/promises';
import * as path from 'node:path';
import {
Expand Down Expand Up @@ -224,15 +224,15 @@ export function useAtCompletion(props: UseAtCompletionProps): void {
setIsLoadingSuggestions(state.isLoading);
}, [state.isLoading, setIsLoadingSuggestions]);

const resetFileSearchState = () => {
const resetFileSearchState = useCallback(() => {
fileSearchMap.current.clear();
initEpoch.current += 1;
dispatch({ type: 'RESET' });
};
}, []);

useEffect(() => {
resetFileSearchState();
}, [cwd, config]);
}, [cwd, config, resetFileSearchState]);

useEffect(() => {
const workspaceContext = config?.getWorkspaceContext?.();
Expand All @@ -242,7 +242,7 @@ export function useAtCompletion(props: UseAtCompletionProps): void {
workspaceContext.onDirectoriesChanged(resetFileSearchState);

return unsubscribe;
}, [config]);
}, [config, resetFileSearchState]);

// Reacts to user input (`pattern`) ONLY.
useEffect(() => {
Expand Down
7 changes: 6 additions & 1 deletion packages/cli/src/ui/hooks/useHistoryManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,12 @@ export function useHistory({
return prevHistory; // Don't add the duplicate
}
}
return [...prevHistory, newItem];
const newHistory = [...prevHistory, newItem];
// Enforce a hard limit of 1000 items to prevent unbounded memory growth
if (newHistory.length > 1000) {
return newHistory.slice(newHistory.length - 1000);
}
Comment on lines +88 to +90

@gemini-code-assist gemini-code-assist Bot Apr 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

While this change limits the history in the UI state, it does not address the unbounded growth in the ChatRecordingService itself, which was identified as the source of the ~350MB leak. The recordMessage method continues to append to an internal array without truncation. To resolve this, the messages state should be managed using the repository's standard LruCache implementation to prevent memory issues and ensure bounded growth. When implementing truncation, prefer a simple approach over complex logic if the inaccuracy is trivial.

References
  1. Avoid module-level global variables for state like caches to prevent race conditions and memory issues. Use session-scoped or instance-scoped state and leverage standard cache implementations like LRUCache.
  2. For caching, use the existing LruCache dependency instead of clearing the entire cache or implementing a custom LRU policy.
  3. When implementing truncation logic, a simpler approach is preferred over a more complex one if the potential inaccuracy is trivial compared to the overall buffer size.

return newHistory;
});

// Record UI-specific messages, but don't do it if we're actually loading
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/code_assist/oauth2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,14 +424,15 @@ async function authWithUserCode(client: OAuth2Client): Promise<boolean> {
'\n\n',
);

let authTimeoutId: NodeJS.Timeout | undefined;
const code = await new Promise<string>((resolve, reject) => {
const rl = readline.createInterface({
input: process.stdin,
output: createWorkingStdio().stdout,
terminal: true,
});

const timeout = setTimeout(() => {
authTimeoutId = setTimeout(() => {
rl.close();
reject(
new FatalAuthenticationError(
Expand All @@ -441,10 +442,11 @@ async function authWithUserCode(client: OAuth2Client): Promise<boolean> {
}, 300000); // 5 minute timeout

rl.question('Enter the authorization code: ', (code) => {
clearTimeout(timeout);
rl.close();
resolve(code.trim());
});
}).finally(() => {
if (authTimeoutId) clearTimeout(authTimeoutId);
});

if (!code) {
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/services/chatRecordingService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,12 @@ export class ChatRecordingService {
) {
const conversation = this.readConversation();
updateFn(conversation);
// Enforce a hard limit of 1000 items to prevent unbounded memory and file growth
if (conversation.messages.length > 1000) {
conversation.messages = conversation.messages.slice(
conversation.messages.length - 1000,
);
}
this.writeConversation(conversation);
}

Expand Down
13 changes: 11 additions & 2 deletions packages/core/src/utils/oauth-flow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ export function startCallbackServer(
portReject = reject;
});

let timeoutId: NodeJS.Timeout | undefined;

const responsePromise = new Promise<OAuthAuthorizationResponse>(
(resolve, reject) => {
let serverPort: number;
Expand Down Expand Up @@ -222,7 +224,7 @@ export function startCallbackServer(
});

// Timeout after 5 minutes
setTimeout(
timeoutId = setTimeout(
() => {
server.close();
reject(new Error('OAuth callback timeout'));
Expand All @@ -232,7 +234,14 @@ export function startCallbackServer(
},
);

return { port: portPromise, response: responsePromise };
return {
port: portPromise,
response: responsePromise.finally(() => {
if (timeoutId) {
clearTimeout(timeoutId);
}
}),
};
}

/**
Expand Down
37 changes: 30 additions & 7 deletions packages/core/src/utils/shell-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ export async function resolveExecutable(
let bashLanguage: Language | null = null;
let treeSitterInitialization: Promise<void> | null = null;
let treeSitterInitializationError: Error | null = null;
let parserState: 'uninitialized' | 'initializing' | 'initialized' | 'error' =
'uninitialized';

class ShellParserInitializationError extends Error {
constructor(cause: Error) {
Expand Down Expand Up @@ -163,16 +165,37 @@ async function loadBashLanguage(): Promise<void> {
}

export async function initializeShellParsers(): Promise<void> {
if (!treeSitterInitialization) {
treeSitterInitialization = loadBashLanguage().catch((error) => {
treeSitterInitialization = null;
// Log the error but don't throw, allowing the application to fall back to safe defaults (ASK_USER)
// or regex checks where appropriate.
debugLogger.debug('Failed to initialize shell parsers:', error);
if (parserState === 'uninitialized') {
parserState = 'initializing';
let timerId: NodeJS.Timeout | undefined;
const timeoutPromise = new Promise<void>((_, reject) => {
timerId = setTimeout(
() => reject(new Error('Tree-sitter initialization timed out')),
30000,
);
});
treeSitterInitialization = Promise.race([
loadBashLanguage(),
timeoutPromise,
Comment on lines +170 to +179

@gemini-code-assist gemini-code-assist Bot Apr 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The setTimeout for tree-sitter initialization is not cleared if the operation succeeds, creating a memory leak. Also, the 5-second timeout might be too short for some environments, leading to premature failures. Furthermore, avoid using the treeSitterInitialization promise object itself to track operation state; use an explicit state variable instead.

    let timeoutId: NodeJS.Timeout | undefined;
    const timeoutPromise = new Promise<void>((_, reject) => {
      timeoutId = setTimeout(
        () => reject(new Error('Tree-sitter initialization timed out')),
        5000,
      );
    });
    treeSitterInitialization = Promise.race([
      loadBashLanguage(),
      timeoutPromise,
    ])
      .finally(() => {
        if (timeoutId) clearTimeout(timeoutId);
      })
      .catch((error) => {
References
  1. Do not blindly apply short timeouts (like 5 seconds) to operations, as they may be too short and cause operations to abort prematurely.
  2. When managing the state of asynchronous operations, rely on an explicit state variable rather than checking for the existence of a promise object.

])
.finally(() => {
if (timerId) clearTimeout(timerId);
})
.then(() => {
parserState = 'initialized';
})
.catch((error) => {
parserState = 'error';
treeSitterInitialization = null;
// Log the error but don't throw, allowing the application to fall back to safe defaults (ASK_USER)
// or regex checks where appropriate.
debugLogger.debug('Failed to initialize shell parsers:', error);
});
}

await treeSitterInitialization;
if (treeSitterInitialization) {
await treeSitterInitialization;
}
}

export interface ParsedCommandDetail {
Expand Down
Loading