Repository navigation
fix(webapp-testing): kill the server's process group so servers aren't orphaned - #1593
Open
romansrepairsltd-coder wants to merge 1 commit into
Conversation
…t orphaned with_server.py starts servers with shell=True and stops them with process.terminate(), which signals only the shell. A server started as `cd frontend && npm run dev` outlives cleanup and keeps holding its port, so the next run fails with EADDRINUSE. Put the shell and its children in one process group via start_new_session, then signal that group (SIGTERM, SIGKILL after the 5s timeout). Co-Authored-By: Claude Opus 5 <[email protected]>
Babysit review — LGTM ✅
|
98zc5g5jyw-arch
approved these changes
Aug 23, 2026
98zc5g5jyw-arch
left a comment
98zc5g5jyw-arch
left a comment
There was a problem hiding this comment.
Hermes Review
Verdict: Approve — correct and well-handled fix for orphaned server processes.
Verified
start_new_session=Trueputs the shell + children in a fresh process group;os.killpg(pgid, SIGTERM)then reaches the real server, not just the shell — fixes the orphaned-port problem described.ProcessLookupErroris caught both atgetpgidtime and aroundkillpg/terminate()— the fallback path is safe for already-dead processes.- SIGTERM → 5s wait → SIGKILL escalation is a sane cleanup ladder.
Suggestions (non-blocking)
- Consider also
os.setpgid-independent portability note:start_new_sessionis POSIX-only; on Windows this file's behavior changes (killpg unavailable) — a short comment or guard would help Windows contributors. process.terminate()in thepgid is Nonebranch can still raiseProcessLookupError, but it's inside thetry→ covered by theexcept (TimeoutExpired, ProcessLookupError)handler. Good.
Reviewed by Hermes Agent (cron babysitter)
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
2 participants
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.
Problem
scripts/with_server.pylaunches each server withshell=Trueand stops it withprocess.terminate(). That signals the shell, not the server the shell spawned. For any command of the documented multi-server form:the shell dies and the real server is orphaned, still bound to its port. The script prints
All servers stoppedwhile the process is very much alive, and the next run fails withEADDRINUSE/Address already in use.Reproduction
Fix
start_new_session=Trueputs the shell and everything it spawns into one process group.os.killpg—SIGTERMfirst,SIGKILLafter the existing 5s timeout.ProcessLookupErroris tolerated on both paths, for a server that already exited on its own.Behaviour is unchanged when the server is a direct child; this only extends the reach of the existing terminate/kill escalation.
Verification
Reproduced the orphan on Linux (Ubuntu 24.04, Python 3.12), confirmed the port stays bound after cleanup, applied the patch, and confirmed the port is released. Checked with a page whose content is set by JS, so the run also exercises the documented
networkidlepath via Playwright before teardown.🤖 Generated with Claude Code