Skip to content

fix(services): clean up timed out command groups - #341

Open
codeforester wants to merge 5 commits into
mainfrom
bug/333-20261006-terminate-owned-health-command-descendants-when-a-check-time
Open

codeforester wants to merge 5 commits into
mainfrom
bug/333-20261006-terminate-owned-health-command-descendants-when-a-check-time

Conversation

@codeforester

Copy link
Copy Markdown
Collaborator

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 bin
  • git diff --check

The focused regression covers descendants with detached stdio and descendants inheriting the command pipes, and verifies that neither can write after timeout.

@codeforester
codeforester force-pushed the bug/333-20261006-terminate-owned-health-command-descendants-when-a-check-time branch from bb51d45 to 5bd5c0f Compare October 6, 2026 02:35
@codeforester
codeforester force-pushed the bug/332-20261006-keep-service-health-aggregation-intact-after-command-launch branch from af8cc64 to 5e91f7d Compare October 6, 2026 02:38
@codeforester
codeforester force-pushed the bug/333-20261006-terminate-owned-health-command-descendants-when-a-check-time branch from 5bd5c0f to bb33199 Compare October 6, 2026 02:38
@codeforester
codeforester force-pushed the bug/332-20261006-keep-service-health-aggregation-intact-after-command-launch branch from 5e91f7d to 27996ba Compare October 6, 2026 12:55
@codeforester
codeforester force-pushed the bug/333-20261006-terminate-owned-health-command-descendants-when-a-check-time branch from bb33199 to 4e039b1 Compare October 6, 2026 12:55

@codeforester codeforester left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@codeforester
codeforester force-pushed the bug/332-20261006-keep-service-health-aggregation-intact-after-command-launch branch from 27996ba to c614c0b Compare October 6, 2026 13:25
@codeforester
codeforester force-pushed the bug/333-20261006-terminate-owned-health-command-descendants-when-a-check-time branch from 4e039b1 to 122d43f Compare October 6, 2026 13:25
Base automatically changed from bug/332-20261006-keep-service-health-aggregation-intact-after-command-launch to main October 6, 2026 14:28
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.

1 participant