Skip to content

fix: kill Claude subprocess on stop/exit to prevent orphaned agents - #430

Open
aholston wants to merge 2 commits into
GreyDGL:mainfrom
aholston:fix/subprocess-cleanup
Open

fix: kill Claude subprocess on stop/exit to prevent orphaned agents#430
aholston wants to merge 2 commits into
GreyDGL:mainfrom
aholston:fix/subprocess-cleanup

Conversation

@aholston

Copy link
Copy Markdown

Summary

  • ClaudeCodeBackend now captures child PIDs after connecting and kills
    them explicitly in disconnect() and via atexit, so the claude CLI
    subprocess is cleaned up on any exit path
  • tui.py on_unmount now calls controller.stop() so the controller's
    disconnect path actually runs when the TUI closes
  • main.py installs a SIGTERM handler so docker stop triggers sys.exit()
    and therefore atexit cleanup

Root cause

When the Python controller exited (TUI closed, Ctrl+C, or docker stop),
the claude CLI subprocess it spawned was orphaned — re-parented to the
container's bash (PID 1) — and kept running indefinitely. The controller's
stop() method only set an in-memory flag, which does nothing if the Python
process is already gone.

Test plan

  • All existing tests pass (make test)
  • Start a session, quit via Ctrl+Q — verify no claude process remains (ps aux)
  • Start a session, run docker stop pentestgpt — verify process is gone on restart
  • Start a session, kill the Python process directly — verify subprocess is cleaned up via
    atexit

@gemini-code-assist

Copy link
Copy Markdown

Summary of Changes

Hello, 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 addresses a critical issue where claude CLI subprocesses were being orphaned and left running indefinitely after the main Python controller process exited. The changes ensure that these subprocesses are properly terminated across various exit paths, including TUI closure, Ctrl+C, and docker stop, by implementing explicit child process tracking and cleanup mechanisms.

Highlights

  • Subprocess Management: The ClaudeCodeBackend now actively tracks and terminates its child claude CLI subprocesses upon disconnection or application exit, preventing orphaned processes.
  • TUI Cleanup Integration: The TUI's on_unmount method was updated to ensure the controller's disconnect logic, including subprocess termination, is properly triggered when the TUI closes.
  • Docker Stop Handling: A SIGTERM handler has been implemented in main.py to gracefully shut down the application and trigger atexit cleanup when a docker stop command is issued.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • pentestgpt/core/backend.py
    • Added atexit, os, and signal imports for process management.
    • Implemented a new _get_child_pids function to identify direct child processes on Linux.
    • Introduced a _subprocess_pids attribute to ClaudeCodeBackend to track spawned subprocesses.
    • Created a _kill_subprocesses method to terminate all tracked PIDs.
    • Registered _kill_subprocesses with atexit for automatic cleanup upon program exit.
    • Modified the connect method to capture and track new child PIDs after establishing a connection.
    • Called _kill_subprocesses within the disconnect method to ensure termination on explicit disconnection.
    • Updated the resume method to also capture and track new child PIDs.
  • pentestgpt/interface/main.py
    • Imported the signal module.
    • Added a SIGTERM handler in the main() function to ensure sys.exit() is called, which triggers atexit cleanup, when the process receives a SIGTERM signal (e.g., from docker stop).
  • pentestgpt/interface/tui.py
    • Modified the on_unmount method to call self._controller.stop(), ensuring proper disconnection and subprocess termination when the TUI application closes.
    • Increased the _agent_thread.join() timeout from 2.0 to 5.0 seconds to allow more time for the agent thread to finish gracefully.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request effectively addresses the issue of orphaned claude subprocesses by implementing cleanup logic on exit, stop, and disconnect paths. The use of atexit, a SIGTERM handler for Docker, and explicit cleanup in the TUI unmount process are all solid improvements. My main feedback is a suggestion to refactor some duplicated code in pentestgpt/core/backend.py to improve maintainability.

Comment thread pentestgpt/core/backend.py Outdated
Comment on lines +277 to +281
pids_before = _get_child_pids()
result = self._client.connect()
if result is not None:
await result
self._subprocess_pids = _get_child_pids() - pids_before

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

There's significant code duplication between this resume method and the connect method. Much of the logic for setting up environment overrides, creating ClaudeAgentOptions, initializing ClaudeSDKClient, and connecting is nearly identical.

Consider refactoring the common logic into a private helper method, for example _initialize_and_connect(self, resume_session_id: str | None = None). This would improve maintainability by keeping the connection logic in one place.

The connect method could call await self._initialize_and_connect() and the resume method could call await self._initialize_and_connect(resume_session_id=session_id).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant