fix(runtime): reap detached box's full cgroup tree on stop/remove#876
Draft
G4614 wants to merge 2 commits into
Draft
fix(runtime): reap detached box's full cgroup tree on stop/remove#876G4614 wants to merge 2 commits into
G4614 wants to merge 2 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
A detached box's `stop` / `rm -f` left its VM running. `kill_process` only SIGKILLs the recorded pid — the outer bwrap launcher — and since boxlite-ai#851 stopped applying `--die-with-parent` to detached boxes, the inner pid-ns tree (inner bwrap + shim + VM) survives. The command returns success and the box record is removed, so the orphan VM is untracked — the root of detached-box accumulation on long-lived hosts (boxlite-ai#137/boxlite-ai#141). The whole tree lives in the box's cgroup, so reap it by id: - add `cgroup::kill_cgroup(box_id)`: write "1" to `<cgroup>/cgroup.kill` (cgroup v2); reaps every process in the box's cgroup atomically, regardless of pid-namespace / process-group structure. Best-effort, idempotent, no-op when the cgroup is gone/empty or absent. - `remove_box` force path: `kill_cgroup` before the single-pid fallback; keyed on id so it reaps even after recovery cleared `state.pid`. - `stop`: `kill_cgroup` as a final step after graceful shutdown — a no-op once the box has exited cleanly. `kill_process` stays as the fallback for hosts without the cgroup jailer (disabled / macOS seatbelt), where the box tree is a single shim process. Tests: - cgroup unit test (no root, CI): `kill_cgroup` on an absent cgroup is a no-op (returns false) — locks the best-effort contract for the no-jailer / macOS path. - detach integration test (Linux, VM): `detached_box_force_remove_reaps_whole_tree` asserts `rm -f` leaves zero box processes (scans /proc by box id). Reproduced two-sided as root: with the rm-f reap disabled the test FAILS (left: 2, right: 0 — the leaked inner bwrap + shim); restored, it passes. Verified as root (boxes boot once euid 0 bypasses the jailer RLIMIT_NPROC ceiling), repeated: rm -f detached x5 -> 3 procs -> 0; stop detached x2 -> 2 -> 0; non-detached run --rm prints output, exit 0, 0 leak (no regression). Compiles; cargo fmt clean. Full CI integration suites need a KVM runner and were not run here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`jailer::cgroup` is `#[cfg(target_os = "linux")]`, so the unconditional `kill_cgroup` calls in remove_box and stop() failed to compile on macOS (`error[E0433]: cannot find 'cgroup' in 'jailer'`). Gate both call sites with `#[cfg(target_os = "linux")]`, matching the existing cross-platform jailer call convention; macOS keeps the `kill_process` single-pid path (no pid-namespace nesting there, so it already reaps the box). Linux behavior unchanged.
01cadc6 to
4b5c0b7
Compare
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
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.
Problem
A detached box's
boxlite stop/rm -freturns success and removes thebox record, but leaves its VM running — an untracked orphan. This is the
runtime root of detached-box accumulation ("积压") on long-lived hosts
(#137/#141).
Cause:
kill_process(recorded_pid)only SIGKILLs the outer bwrap launcher(
shim.pidrecords it). A box's tree isouter bwrap → inner pid-ns bwrap → shim → VM. Since #851 stopped applying--die-with-parentto detached boxes,killing the outer one leaves the inner pid-ns tree alive.
Fix — reap the box's whole cgroup by id
cgroup::kill_cgroup(box_id)— write1to<cgroup>/cgroup.kill(cgroup v2). Tears down every process in the box's cgroup atomically,
regardless of pid-namespace / process-group structure. Best-effort,
idempotent — a no-op when the cgroup is gone/empty or absent.
remove_boxforce path —kill_cgroup(id)before the single-pidfallback; keyed on id, so it reaps even after recovery cleared
state.pid.stop—kill_cgroupas a final step after graceful guest shutdown; ano-op once the box has exited cleanly.
kill_processstays as the fallback for hosts without the cgroup jailer(disabled / macOS seatbelt), where the box tree is a single shim process.
Tests
kill_cgroupon an absent cgroup is a no-op(returns
false); locks the best-effort contract for the no-jailer / macOSpath.
detached_box_force_remove_reaps_whole_tree:boot a detached box, assert a live tree,
remove(force), assert 0 boxprocesses remain (
/procscan by box id). Reproduced two-sided as root:with the
rm -freap disabled it FAILS (left: 2, right: 0— the leakedinner bwrap + shim); restored, it passes.
Verified (root, repeated)
Boxes boot on the test host once
euid 0bypasses the jailer'sRLIMIT_NPROCceiling.
rm -fdetached ×5stopdetached ×2run --rmCompiles;
cargo fmtclean.cargo/CI integration suites (need a KVM runner withprocess headroom). macOS seatbelt path falls back to
kill_processand isunchanged.
Related
The test-side counterpart (recovery tests reaping their own stranded detached
trees) is #870. This PR fixes the underlying production leak that those tests
were tripping over.
🤖 Generated with Claude Code