Reap orphaned guest processes#305
Conversation
Prevent stale app and guest-agent PIDs from shielding unrelated adopted zombies after PID reuse. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 51996a0. Configure here.
| var status syscall.WaitStatus | ||
| _, _ = wait4(pid, &status, syscall.WNOHANG, nil) | ||
| } | ||
| } |
There was a problem hiding this comment.
Single-pass orphan reaping
Medium Severity
reapAdoptedZombies walks /proc once per wakeup and reaps only that snapshot. When many adopted children exit together, the kernel may deliver a single coalesced SIGCHLD, and zombies that appear after the scan starts can be missed with no further signal. Those entries can linger until a later unrelated wakeup (e.g. entrypoint exit).
Reviewed by Cursor Bugbot for commit 51996a0. Configure here.
There was a problem hiding this comment.
This should not miss exits that occur during a scan. Receiving from the buffered sigCh removes the pending notification before reapAdoptedZombies starts, leaving the channel slot empty. A child that exits during that scan therefore queues another SIGCHLD wakeup; additional exits may coalesce, but the subsequent /proc scan observes all zombies present at that point. We also exercised this with 32 children exiting in a burst across four real Cloud Hypervisor guest runs, and every adopted child was reaped. No change planned here.
sjmiller609
left a comment
There was a problem hiding this comment.
I was thinking this seems like something that only happens because we run custom PID 1, which is true. I was considering using some known software to handle the reaper instead of custom implementation, but I think it's not worth it. So I agree this path makes sense. 👍
We can keep in mind this concept to maybe have a different PID 1 like tini later on if issues like this are popping up more.
standard-software option: tini -s wrapping the entrypoint. tini is the ~50KB static binary behind docker run --init. with -s it registers as a child subreaper, so it doesn't need to be PID 1 — orphans from its subtree reparent to it and get reaped there. the change is small: embed a static tini in the initrd (the repo already go:embeds binaries in lib/system/) and change mode_exec.go:131 to exec.Command("/opt/hypeman/tini", "-s", "--", "/bin/sh", "-c", shellCmd). exit codes and signal forwarding are preserved, and the chromium-restart zombies get reaped before reaching PID 1.
why keep the custom reaper anyway: tini -s only covers the entrypoint's subtree. zombies parented to init itself still need handling — the fire-and-forget headers worker, and anything orphaned under guest-agent (hypeman exec spawns live there). so you'd need tini in two places plus a residual fix for init's own children; the /proc-scan reaper covers all of it uniformly. the "custom" part is ~130 tested lines in what is already a custom PID 1, versus a vendored third-party binary × 2 arches with ongoing build and supply-chain overhead. and the PR is already written and validated against real guests. merge it; keep tini in the back pocket.


summary
exec.Cmd.Wait/procparsing and real subreaper behavior with unit and Linux integration testswhy
The exec-mode init remains PID 1, so orphaned descendants are reparented to it. Without reaping them, exited subprocesses remain visible as zombies and can cause process-lifecycle checks to wait unnecessarily.
tests
go test ./lib/system/initgo test -race ./lib/system/initmake build-embeddedgo test ./...(not fully runnable in this environment: packages requiring embedded Cloud Hypervisor/Caddy assets fail setup, and image tests requiremkfs.erofs)manual end-to-end validation
Ran a temporary differential harness against real Alpine guests under Cloud Hypervisor; no harness code is included in this PR.
main: reproduced the bug—a short-lived orphan adopted by guest PID 1 remained in/procas stateZ, and a 32-process burst remained unreaped/procwithin the timeoutNote
Medium Risk
Changes PID 1 child lifecycle and signal/wait interaction in the guest init path; incorrect reaping could affect exit-code reporting for the app or guest-agent, though known-child exclusion is designed to prevent that.
Overview
Exec-mode guest init stays PID 1, so orphaned descendants were left as zombies and could confuse lifecycle checks. This PR adds a background orphan reaper that runs while the entrypoint is up.
The reaper listens for
SIGCHLDand rescans/procfor zombie children reparented to init, then reaps them with non-blockingwait4. Guest-agent and entrypoint PIDs are tracked in aknownChildrenset so the reaper does not steal exit statuses thatexec.Cmd.Waitmust consume; removals from that set trigger another rescan when a managed child exits.New unit tests cover
/proc/statparsing and selection logic; a Linux integration test exercises real subreaper behavior with a short-lived grandchild.Reviewed by Cursor Bugbot for commit 51996a0. Bugbot is set up for automated code reviews on this repo. Configure here.