fix(core): ensure robust sandbox cleanup in all process execution paths#24763
fix(core): ensure robust sandbox cleanup in all process execution paths#24763
Conversation
Summary of ChangesHello, 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 improves the reliability and resource management of the sandboxed process execution system. By consistently calling cleanup routines in both successful and error-handling scenarios, it prevents resource leaks that previously led to intermittent test failures and potential system instability. Highlights
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. Footnotes
|
|
Hi @ehedlund, 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! |
|
Size Change: +1.56 kB (0%) Total Size: 34 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 introduces manual cleanup logic for sandboxed processes by invoking cleanup functions within 'close' and 'error' event handlers across several core services and tools. While this addresses basic resource management, the review feedback indicates that the current implementation is fragile. Specifically, resource leaks can still occur if errors happen during process initialization or before the event handlers are successfully attached. It is recommended to refactor these implementations to use 'try...finally' blocks to ensure that cleanup is executed reliably regardless of where an error occurs in the process lifecycle.
🚨 Action Required: Eval Regressions DetectedModel: The following trustworthy evaluations passed on
The check passed or was cleared for 61 other trustworthy evaluations. 🛠️ Troubleshooting & Fix Instructions1. Ask Gemini CLI to fix it (Recommended)Copy and paste this prompt to the agent: 2. Reproduce LocallyRun the following command to see the failure trajectory: GEMINI_MODEL=gemini-3-flash-preview npm run test:all_evals -- evals/cli_help_delegation.eval.ts --testNamePattern="should delegate to cli_help agent for subagent creation questions"3. Manual FixSee the Fixing Guide for detailed troubleshooting steps. This PR modifies files that affect the model's behavior (prompts, tools, or instructions).
This is an automated guidance message triggered by steering logic signatures. |
gemini-cli
bot
added
the
status/need-issue
ehedlund
changed the title
ehedlund
changed the title
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
4 participants
Summary
Fix memory leaks and resource exhaustion in sandboxed process execution. This PR adds missing sandbox cleanup calls and wraps all process execution logic in robust
try...finallyblocks to ensure sidecar processes and temporary files are reliably terminated across all success, error, and early-abort paths.Details
The original implementation missed calling
prepared.cleanup?.()in several execution paths, leading to resource leaks. Additionally, standard event handlers were insufficient to guarantee cleanup during synchronous throws or aborted generators.Changes included in this PR:
cleanupis invoked in previously unhandled success and error paths.try...finally: Upgraded all sandbox process execution methods to usetry...finallyblocks for guaranteed execution, regardless of how the process terminates.Affected areas:
SandboxedFileSystemService:read_fileandwrite_filepaths.GrepTool: Success and error paths.ToolRegistry: Success and error paths for both tool discovery and invocation.shell-utils:spawnAsyncandexecStreamingpaths.ShellExecutionService:childProcessFallbackandexecuteWithPtypaths.Related Issues
Mentioned in #24480 review.
How to Validate
Run the core tests that use the sandbox and shell execution:
npm test -w @google/gemini-cli-core -- src/utils/sandboxUtils.test.ts src/services/sandboxedFileSystemService.test.ts src/tools/grep.test.ts src/tools/tool-registry.test.ts src/services/shellExecutionService.test.tsPre-Merge Checklist