Skip to content

fix(v1): reap env pool worker subprocesses#1995

Open
morluto wants to merge 5 commits into
PrimeIntellect-ai:mainfrom
morluto:fix/v1-pool-worker-subtree
Open

fix(v1): reap env pool worker subprocesses#1995
morluto wants to merge 5 commits into
PrimeIntellect-ai:mainfrom
morluto:fix/v1-pool-worker-subtree

Conversation

@morluto

@morluto morluto commented Jul 14, 2026

Copy link
Copy Markdown

Description

Fixes #1992.

Env-server pool workers now lead a POSIX session that owns their local subprocesses. SubprocessRuntime still gives each rollout its own process group, but those groups remain inside the worker session. Pool shutdown keeps the existing graceful SIGTERM window, then freezes and kills any processes left in the worker session.

Shutdown waits on multiprocessing sentinels without joining first. This keeps an exited worker's PID and session ID reserved until session cleanup finishes, avoiding stale descendant-PID snapshots and PID-reuse races.

Provider-side sandbox and tunnel registrations are outside the OS session and remain tracked in #1994.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Test improvement

Testing

  • All existing tests pass when running uv run pytest locally.
  • New tests have been added to cover the changes

Validation performed:

  • focused regression proof: PID-only escalation left a separate process group alive; this branch reaped it both when the worker ignored SIGTERM and when the worker exited during the grace period
  • uv run pytest tests/test_env_server.py — 15 passed
  • uv run ruff check --fix . — passed
  • uv run pre-commit run --all-files — Ruff passed; the generated-guide check reported pre-existing stale environments/AGENTS.md outputs
  • uv run ty check verifiers/utils/process_utils.py verifiers/v1/serve/pool.py — passed
  • autoreview — clean after resolving session ownership, late-fork, and PID-reuse findings

The repository asks contributors not to add unit tests; the regression proof was run as a temporary process-level script.

Checklist

  • My code follows the style guidelines of this project as outlined in AGENTS.md
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Additional Notes

Suggested review order:

  1. verifiers/v1/serve/pool.py: worker session creation and sentinel-based shutdown ordering
  2. verifiers/v1/runtimes/subprocess.py: per-rollout process groups remain inside the worker session
  3. verifiers/utils/process_utils.py: freeze-and-kill session cleanup

PR #1993 also changes pool.py for worker-death recovery. The changes are independent but may require a small rebase after either PR merges.


Note

Medium Risk
Changes POSIX signal handling and process-tree teardown during pool shutdown; mistakes could affect unrelated processes if session discovery misidentifies leaders, though the code guards non-leader workers by killing only the worker PID.

Overview
Fixes leaked rollout subprocesses when env-server pool workers shut down by treating each worker as a POSIX session leader and reaping the whole session instead of killing only the worker PID.

Pool workers now enter via _serve_pool_worker, which calls os.setsid() before serve_env, so the worker PID doubles as the session ID for cleanup. EnvServerPool._shutdown still sends graceful SIGTERM, then waits on multiprocessing sentinels (without joining first, so the zombie keeps the session identity stable), and escalates with kill_process_session rather than a bare process.kill(). Shutdown temporarily ignores SIGINT/SIGTERM on the broker while tearing workers down.

kill_process_session (new in process_utils) discovers session members via /proc with a ps fallback, freezes live PIDs with SIGSTOP until membership stabilizes (late forks), then iteratively SIGKILLs session members and joins the worker.

SubprocessRuntime switches from start_new_session=True to process_group=0, so each rollout still gets its own process group for killpg on cancel/cleanup, but those groups stay inside the worker session for pool-level reaping.

Reviewed by Cursor Bugbot for commit 55e7b71. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Reap env pool worker subprocesses by killing entire POSIX sessions on shutdown

  • Each pool worker now calls os.setsid() via a new _serve_pool_worker wrapper in pool.py, making it a session leader so all its descendants share a session ID.
  • EnvServerPool._shutdown waits on worker sentinels (10s timeout), then calls a new kill_process_session utility to freeze and SIGKILL every process in the worker's session, ensuring no orphaned subprocesses remain.
  • kill_process_session in process_utils.py discovers session members via /proc (or ps -A as fallback), SIGSTOPs live members to prevent forking, then iteratively SIGKILLs until the session is drained or a timeout is reached.
  • Subprocess children in subprocess.py now use process_group=0 instead of start_new_session=True, keeping them inside the worker's session so they are caught by session-level cleanup.
  • Behavioral Change: subprocesses spawned by the runtime are no longer in their own session; they share the worker's session and will be killed during pool shutdown.

Macroscope summarized 55e7b71.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Comment thread verifiers/utils/process_utils.py
Comment thread verifiers/utils/process_utils.py Outdated
Comment thread verifiers/utils/process_utils.py Outdated
Comment thread verifiers/utils/process_utils.py Outdated

@cursor cursor 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7d281db. Configure here.

Comment thread verifiers/v1/serve/pool.py Outdated
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.

Env-server pool hard shutdown can orphan worker subprocesses

1 participant