Revert "fix(mcp): ensure MCP transport is closed to prevent memory leaks" #18771
Conversation
|
Hi @skeshive, thank you so much for your contribution to Gemini CLI! We really appreciate the time and effort you've put into this. We're making some updates to our contribution process to improve how we track and review changes. Please take a moment to review our recent discussion post: Improving Our Contribution Process & Introducing New Guidelines. Key Update: Starting January 26, 2026, the Gemini CLI project will require all pull requests to be associated with an existing issue. Any pull requests not linked to an issue by that date will be automatically closed. Thank you for your understanding and for being a part of our community! |
Summary of ChangesHello @skeshive, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request reverts a prior change that introduced logic to close the MCP transport whenever an error occurred. The original intent was to prevent memory leaks, but it inadvertently caused a regression by treating all errors as critical, leading to premature connection closures. This revert ensures that only truly fatal errors lead to connection termination, resolving the reported issue. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Size Change: -810 B (0%) Total Size: 23.9 MB ℹ️ View Unchanged
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request reverts a previous change that aimed to fix a memory leak by closing the MCP transport on error, as the original fix was too aggressive. However, this revert re-introduces a high-severity resource leak vulnerability by removing all transport-closing logic from the onerror handlers while still marking connections as DISCONNECTED. This results in orphaned transports and child processes running indefinitely, leading to leaked resources in McpClient and connectAndDiscover when errors occur and the client is not properly cleaned up on reconnect. The feedback highlights the need to ensure resources are explicitly released when a connection transitions to a disconnected state.
| mcpClient.onerror = (error) => { | ||
| coreEvents.emitFeedback('error', `MCP ERROR (${mcpServerName}):`, error); | ||
| updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED); | ||
| // Close transport to prevent memory leaks | ||
| if (transport) { | ||
| try { | ||
| await transport.close(); | ||
| } catch { | ||
| // Ignore errors when closing transport on error | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
Similar to the issue in McpClient.connect, this onerror handler re-introduces a high-severity resource leak vulnerability. It updates the server status to DISCONNECTED but fails to close the mcpClient or its transport. Since mcpClient is local to connectAndDiscover, this leads to orphaned transports and potential process leaks if an error occurs. To remediate, mcpClient.close() should be called within the onerror handler to ensure resources are properly released when the connection is marked as disconnected.
mcpClient.onerror = (error) => {
coreEvents.emitFeedback('error', `MCP ERROR (${mcpServerName}):`, error);
updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED);
// Close the client to prevent resource leaks.
mcpClient?.close().catch(err => {
coreEvents.emitFeedback('error', `Error closing errored MCP client: ${mcpServerName}`, err);
});
};
gemini-cli
bot
added
the
status/need-issue
2 participants
Reverts #18054
This is causing #18738 since not all errors sent through onerror should be treated as fatal connection-closing issues