fix: Treat zombie-only process groups as terminated in E2E teardown - #393
Merged
jiangkuaixue123 merged 2 commits intoSep 24, 2026
Merged
jiangkuaixue123 merged 2 commits into
jiangkuaixue123 merged 2 commits into
Conversation
os.killpg(pgid, 0) succeeds while any group member is a zombie. When a container's PID 1 does not reap orphans (e.g. exec pytest), vLLM workers whose parent exited first stay zombies with ppid 1, so teardown reports 'process group still alive after SIGKILL' even though every real process has exited and no timeout can help. Add process_group_is_alive(), which scans /proc for a non-zombie member of the group and falls back to the killpg answer when /proc is missing or shows no member, and use it for both liveness checks in terminate_process_groups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: ronenkat <16743404+ronenkat@users.noreply.github.com>
ronenkat
requested review from
hsliuustc0106 and
jiangkuaixue123
as code owners
September 23, 2026 09:17
| if not process_entry.name.isdecimal(): | ||
| continue | ||
| try: | ||
| stat = (process_entry / "stat").read_text() |
Collaborator
There was a problem hiding this comment.
[P2] Parse /proc/*/stat as bytes, or treat decoding failures as an inconclusive scan. Linux permits a non-UTF-8 comm value in any process, including one outside the target group. read_text() then raises UnicodeDecodeError, which this except OSError does not catch. That propagates through both liveness checks and can skip the later SIGKILL/reap phase, failing the E2E teardown. This edge case is valid but has not been reproduced in CI.
Contributor
Author
There was a problem hiding this comment.
Thanks, fixed.
- process_group_is_alive now reads /proc//stat with read_bytes() and parses it as bytes (b")", b"Z", b"X"), matching find_processes_matching_environment. A non-UTF-8 process name can no longer raise an exception.
- If the process group field can't be parsed, int() raises ValueError, and the docstring now says so. Both liveness checks in terminate_process_groups catch (OSError, ValueError). A parse error is reported as a failure and the group is treated as alive, so SIGKILL and reaping still run.
- New unit tests cover a non-UTF-8 comm outside the group (ignored), a live member with a non-UTF-8 comm (group still counted as alive), and an unparsable process group (reported, then escalated). All three fail on the previous code.
Signed-off-by: ronenkat <16743404+ronenkat@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
E2E teardown can fail with
process group <pgid> still alive after SIGKILLeventhough vLLM shut down cleanly and GSM8K passed. When the container's PID 1 does
not reap orphans (e.g.
exec pytest), orphaned vLLM workers stay zombies, andos.killpg(pgid, 0)keeps reporting their group as alive. This PR makes theteardown liveness check ignore zombies.
Issue
Scope
tests/e2e/process_utils.py: newprocess_group_is_alive(), used byboth liveness checks in
terminate_process_groups; unit tests.shareProcessNamespace) alsoavoids the failure, but a pod
command:can bypass an imageENTRYPOINT,so the fix lives in the harness instead.
Implementation Notes
process_group_is_alive()keepsos.killpg(pgid, 0)as a fast path, thenscans
/proc/*/statfor a group member whose state is notZ/X.State and pgrp are parsed after the last
), becausecommcan contain).zombies. If
/procis missing (non-Linux) or shows no member (exitedmid-scan), it keeps the killpg answer and re-polls.
D-state processes still count as alive, so real driver hangs are stillreported.
proc_rootargument, defaulting to/proc, makes the check testable.Test Plan
pytest tests/unit/test_e2e_process_utils.py tests/unit/test_e2e_runner.py.test_deepseek_v2_lite[afd-graph-2a2f]with pytest as PID 1 (notini, no
shareProcessNamespace), 20s timeout, on the node where it failed.Test Result
vllm/vllm-openai:v0.26.0):tests/unit/test_e2e_process_utils.pyand
tests/unit/test_e2e_runner.py: 153 passed, 0 skipped. Two new testscover a zombie-only group (no failure) and a zombie beside a
D-state member(still reported).
mainstill alive after SIGKILL, also at 120sVLLM::Worker_DPwithppid=1in the checked group were observed and ignoredDocs Impact
Essential PR Checklist
🤖 Generated with Claude Code