Repository navigation
fix(services): clean up timed out command groups - #341
codeforester wants to merge 5 commits into
Conversation
bb51d45 to
5bd5c0f
Compare
af8cc64 to
5e91f7d
Compare
5bd5c0f to
bb33199
Compare
5e91f7d to
27996ba
Compare
bb33199 to
4e039b1
Compare
codeforester
left a comment
There was a problem hiding this comment.
Reviewed head 4e039b1 against #333 (stacked on #340). No blocking findings. I loaded this branch's run_command_check and exercised the timeout path:
| command | result | time | leftover sleep 30 |
|---|---|---|---|
sh -c 'sleep 30 & sleep 30' |
failed, "timed out after 3.0 seconds" | 3.1 s | 0 |
sh -c "(trap '' TERM; sleep 30) & wait" |
failed (SIGTERM ignored, so it escalated to SIGKILL) | 5.2 s | 0 |
true |
ok | 0.0 s | 0 |
/nonexistent/x |
failed, ENOENT |
0.0 s | 0 |
So start_new_session=True plus group termination reaps descendants, including ones that ignore SIGTERM.
Worth noting: terminate_process_group is shared with services stop, and stopped() now means "the whole process group is gone" rather than "the leader exited", with process_is_running(pid) no longer consulted. That's stricter and more correct, but it changes stop's success criterion for services that leave helpers behind. A one-line CHANGELOG/contract note would help. Descendants that call setsid() still escape the group, which is unavoidable; worth a sentence in the docs. Ready in stack order.
27996ba to
c614c0b
Compare
4e039b1 to
122d43f
Compare
Summary
Run command health checks in an owned process group and terminate/reap the entire group when the bounded timeout expires. This prevents descendants from surviving a completed timeout while preserving subsequent service rows.
Fixes #333
Validation
bats tests/services_test.bats --filter 'command timeouts'python3 -m compileall -q bingit diff --checkThe focused regression covers descendants with detached stdio and descendants inheriting the command pipes, and verifies that neither can write after timeout.