boxlite-ai / boxlite-ai/boxlite
Investigate daemon-wide SIGCHLD zombie reaper for shim cleanup
- Dominant language
- Rust
- Stars
- 2.3k
- Forks
- 179
- Avg merge
- 23h 25m
- Merged PRs (30d)
- 121
Description
## Background
During the `CL84LvGx7RBE` incident investigation (see PR #520), the prod runner accumulated 7+ `` libkrun shim processes under the runner PID. Root cause: each `ProcessMonitor` only reaps the single PID it was constructed for, so failed/abandoned shim children from earlier init attempts never get a `waitpid()` and linger as zombies.
PR #520 originally landed a daemon-wide SIGCHLD reaper (`install_zombie_reaper`) to handle this — a background thread that calls `waitpid(-1, &status, WNOHANG)` every 5 s — and a unit test that forks a child, drops it without `wait()`, and asserts the reaper consumed it within 8 s.
After landing, the reaper was reverted (commit on the same PR) pending the design work below. The CleanupGuard preservation fix (the actual root cause of `CL84LvGx7RBE`) stayed.
## Why it was reverted
1. **Global side effect**: `waitpid(-1, WNOHANG)` races with any code in the same process that owns a `std::process::Child` handle and expects to call `.wait()`. If the reaper consumes the child first, the owner gets `ECHILD` and loses the exit code. `ProcessMonitor::try_wait` already returns `ProcessExit::Unknown` for this case, but other callers of `Child::wait()` across the daemon do not. We need to audit and harden those call sites OR pick a reaper strategy that scopes to a known PID set.
2. **Coarse cycle**: 5 s sleep between reap passes means the test had to wait up to 8 s for verification. Slow in CI; also means zombies linger for up to 5 s in production before getting cleaned up.
3. **No lifecycle**: the reaper thread is install-once via `AtomicBool` and runs for the entire process lifetime — no shutdown path, no per-test isolation, no clean way to verify it doesn't leak in long-running test binaries.
## What we still want to solve
Preventing zombie accumulation when:
- An init-pipeline attempt fails and the shim child is abandoned (no `ProcessMonitor::try_wait` ever runs to completion)
- Multiple retries happen in sequence (each spawns a new shim PID)
- The shim is reattached-to rather than spawned (no parent-child relationship)
Reproduction: trigger N init failures in a row on a real runner and grep `ps fwax` for `\[libkrun VM\] `.
## Acceptance criteria for a re-landing
- [ ] Reaper scopes itself to known shim PIDs (e.g. an explicit registry the init pipeline registers/unregisters from) — does NOT call `waitpid(-1, ...)` indiscriminately.
- [ ] Audit and document all `Child::wait()` call sites in `boxlite/src/**` and verify none of them race the reaper.
- [ ] Reaper has a shutdown path tied to `RuntimeImpl::shutdown` so test binaries don't leak threads.
- [ ] Verification test runs in < 2 s (poll-based, not sleep-based).
- [ ] An integration-style test demonstrates "N init failures leave 0 zombies" rather than relying only on a unit-level fork/exit pattern.
## Out of scope
- The CleanupGuard preservation fix is independent and stays. This issue is purely about the zombie-shim cleanup, which is a contributing factor but not the root cause of `CL84LvGx7RBE`.
## References
- PR #520 — original fix bundle
- Commits in #520: original landing in commit a7dc19ea, revert in commit 3b3f65cf
- Source: `src/boxlite/src/util/process.rs` (`ProcessMonitor` is the existing per-PID path)
Contributor guide
Research direction
Start with src/boxlite/src/util/process.rs and the existing ProcessMonitor path, then review PR #520 and its commits a7dc19ea and 3b3f65cf. Audit Child::wait() call sites under boxlite/src/** and the RuntimeImpl::shutdown entry point before choosing a scoped reaper design. Done means the acceptance tests pass, including sub-two-second verification and an integration test showing N failed init attempts leave no zombies.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- operating-systems
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100