fix(daemon): harden /bench/run child lifecycle, bound request models - #221
Open
Artemowka22 wants to merge 2 commits into
Open
fix(daemon): harden /bench/run child lifecycle, bound request models#221Artemowka22 wants to merge 2 commits into
Artemowka22 wants to merge 2 commits into
Conversation
The SSE generator had no try/finally: a client disconnect mid-stream closed the generator and left the bench child running — holding the GPU this route exists to hand over, and writing the shared profile — and never reaped. Spawn the child in its own process group and guarantee teardown on any exit from the stream: SIGTERM the group, 5 s grace, SIGKILL escalation, unconditional reap (shielded so a second cancellation mid-teardown cannot skip it). Wire format is unchanged. Found via the freetoken-mlx downstream audit (docs/AUDIT.md, defect 1).
StartBody.port (and SwitchBody's, via inheritance) was an unbounded int flowing into serve lifecycle mutation; constrain it to a real socket port (1..65535, None = default). List args defaults move to Field(default_factory=list). Found via the freetoken-mlx downstream audit (docs/AUDIT.md, defect 5).
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
1 participant
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.
Summary
Two defects in the daemon control plane, found by source-level audit in a downstream port (agisota/freetoken-mlx, docs/AUDIT.md) and confirmed present here:
POST /bench/run: the bench child survives SSE cancellation. The streaming generator has notry/finally; when the client disconnects mid-stream, Starlette closes the generator and theft bench bwchild keeps running — holding GPU memory (the route exists precisely because the bench needs GPU exclusivity) and writing the shared profile, and it is never reaped. Fix: spawn the child in its own process group (start_new_session=True), wrap read/wait/emit intry/finally, and on teardown SIGTERM the group → 5 s grace → SIGKILL → guaranteed reap.Request models accept invalid input.
StartBody.port(and via inheritanceSwitchBody.port) is an unboundedint | Nonethat flows into serve lifecycle mutation; list defaults use mutable literals. Fix:portconstrained to 1..65535, list fields viaField(default_factory=list).Wire behavior of
/bench/run(progress/result/error events) is unchanged.Test plan
pytest tests/daemon/test_daemon_app.py(new, torch-free like the rest oftests/daemon/): port-bound validation matrix, child-killed-and-reaped on client disconnect mid-stream, success path still emits theresultSSE eventtests/daemon/suite passes