Repository navigation
CI: stop leaking test temp dirs and orphaned daemons on the Linux runner - #1005
Conversation
Every `cargo test -p intendant --bins` run left ~150 dirs in the temp dir, and on the Linux CI runner (a shared tmpfs /tmp) they had piled up to tens of thousands. Measured per run, module by module: - web_gateway/listener.rs: the cfg(test) default_access_cert_dir() gave every test gateway its own TempDir but parked it in a process-lifetime static Vec, which is never dropped. One `.tmpXXXXXX` dir holding only `.authority-store.lock` per gateway: ~126 per run (web_gateway and peer tests), the bulk of the `.tmp*` residue. - session_supervisor test config: logs_home_override was a temp_dir().join(pid-nanos-seq) that nothing removed (~20 per run). - dashboard_control test runtime: same pattern for state_root. - access/org.rs: org_identity() mem::forget-ed its TempDir although the identity keeps its key in memory. test_support::thread_scratch_dir() hands out a TempDir owned by the calling thread, so each root is removed when its test finishes (after a tokio test body has dropped its runtime), without threading a guard through the helpers that build these configs. The logs home and state root stay not-yet-created children, as before. org.rs just lets the TempDir drop.
The Linux gate runs these on every landing, and none removed its mkdtemp scratch: per run the peer-sessions rig left ~380 inodes (two daemon homes, projects, logs), session-vitals ~200 and native-goal ~150. Together they were over half of the leaked inodes in the CI runner's /tmp. - peer-rig.py removes a scratch dir it created itself once its daemons are down; an explicit PEER_RIG_SCRATCH or a --browser run (the screenshot lands there) keeps it. - The two node drivers stop the daemon, wait for it to exit (it writes session logs into HOME while shutting down), then remove HOME and the project, instead of SIGTERM + immediate process.exit. - INTENDANT_KEEP_TEST_TMP=1 keeps the scratch after a failed run. - test-external-cu-session.py passes TMPDIR through its scrubbed daemon env, so the daemon's Xauthority dirs and Chromium temp land in the caller's (job-scoped) temp root instead of the shared /tmp.
The self-hosted Linux runner's /tmp is a RAM-backed tmpfs shared by both listeners and the operator, and nothing scoped CI temp to a job: by 2026-10 it held ~60k leaked test dirs (over 40% of its inodes, growing ~17k inodes a day) plus three CI daemons up to 23 days old. The Linux runner accounts have no job hooks yet. The daemons came from cancelled jobs. The runner's tree kill SIGKILLs the test binary, so kill_on_drop never runs and the e2e daemons it spawned are reparented to init; the external-CU acceptance daemon runs with a scrubbed environment and its Xvfb/Chromium children with it. - "Job-scoped temp dir" (Linux fleet only) points TMPDIR at $RUNNER_TEMP/tmp: per listener, and wiped by the runner at every job start and end. Measured cost of disk vs tmpfs: ~0.8s on the ~20s bins suite. - "Reap job-scoped processes and temp" runs last, always() (also after a cancellation): scripts/ci/reap_job_scope.py stops this account's processes that still reference the job temp dir (inherited TMPDIR, or a cwd/argv/HOME under it), sparing itself, its ancestors and the runner/sccache stack, TERM then KILL with start-time checks against pid reuse, then deletes the dir. It never fails the job and warns per process it had to stop. The other listener's jobs never reference this dir, and residue from before this change is not matched. - Unit tests (fake /proc, plus a live orphan on Linux) run in the Linux leg; scripts/ci/README.md documents the scope and interlock.
|
@codex review |
The janitor's tests call main(), whose ::warning:: lines surfaced as annotations on every Linux run (seen on this PR's first run) and would read as real leaks. Capture its stdout in the tests and assert on it.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The account paying for this security review has reached its Codex usage limits. The payer can check the Codex usage dashboard. For personal accounts, using credits requires enabling “Use credits for security reviews” in Code review settings. If you do not manage the paying account, contact this repository's admins. |
- Path matching requires a whole-path left boundary too (start, `=`, `:`, whitespace or a quote), so a longer path that merely contains the scope (e.g. under another mount) no longer matches. - Warn when the job dir could not be fully removed, and escape `%`, CR and LF in ::warning:: text (process command lines go there). - "Stop daemon (dashboard-boot)" waits for the daemon to exit, so the janitor never reports a daemon that is merely still shutting down. - The node smoke drivers never let a scratch-cleanup error turn a pass into a failure (rmSync retries, then logs), and every smoke only honours INTENDANT_KEEP_TEST_TMP=1, not any non-empty value. - README: a server shared across jobs must not run with the job TMPDIR.
|
Independent review (fresh-context reviewer; the Codex review hit its usage limit): no P0/P1 findings. Six P2s are addressed in 547152c: a whole-path left boundary in the janitor's path match, a warning when the job dir can't be fully removed, escaping of ::warning:: text, "Stop daemon (dashboard-boot)" now waits for the exit (avoids a false leak warning), smoke cleanup errors can no longer flip a pass to a failure, and only INTENDANT_KEEP_TEST_TMP=1 keeps scratch. A README note covers the case of a shared server started with the job TMPDIR. Not changed: the gateway test store stays owned by the spawning test thread (no current caller runs on a short-lived thread), and the Linux leg now runs tests with TMPDIR off /tmp by design. |
The leak
The self-hosted Linux runner's
/tmpis a 7.7 GB RAM-backed tmpfs shared by both listeners and the operator, and nothing scoped CI temp to a job. A read-only survey on 2026-10-21 found:.tmpXXXXXX), ~449k inodes in use (43% of the tmpfs), growing ~17k inodes a day. At that rate/tmpruns out of inodes in about five weeks unless the box reboots.intendantdaemons still running out of temp dirs: one for 23 days (with its Xvfb and Chromium), two for 1.5 days.Root causes
Inodes by source, with the cause measured by running each test module (and then each
peer::test) alone under its ownTMPDIR:tests/skills/peer-sessions-smoke/peer-rig.pymkdtempscratch (two daemon homes, projects, logs) never removed.tmpXXXXXX, mostly holding only.authority-store.lockweb_gateway/listener.rscfg(test)default_access_cert_dir()gave each test gateway aTempDirbut parked it in a process-lifetime static, which is never dropped: about 126 per--binsrun (web_gateway and peer tests). Alsoaccess/org.rsorg_identity()mem::forget-ed itsTempDirintendant-test-logs-home-*session_supervisortest config:temp_dir().join(pid-nanos-seq)that nothing removed (~20 per run)vitals-*,native-goal-*dashboard-boot-smoke-*,org.chromium.Chromium.*, dashboard-control state roots, per-process shell homesThe orphaned daemons came from cancelled jobs (concurrency
cancel-in-progress). The runner's worker log shows SIGINT and SIGTERM ignored, then "Kill entire process tree". That SIGKILLs the test binary, sokill_on_dropnever runs and its e2e daemons are reparented to init. The 23-day one is the external-CU acceptance daemon, which runs with a scrubbed environment and keeps its Xvfb and Chromium alive. The Linux runner accounts have no job hooks (only the Mac does), so nothing swept up afterwards.Fixes
At the source
test_support::thread_scratch_dir()provides aTempDirowned by the calling thread and removed when the test finishes (after a tokio test body has dropped its runtime). The test gateway access stores, supervisor logs homes and dashboard-control state roots use it, and the latter two stay not-yet-created children as before.org.rssimply lets itsTempDirdrop, because the identity keeps its key in memory.peer-rig.pyremoves the scratch dir it created once its daemons are down. An explicitPEER_RIG_SCRATCHor a--browserrun keeps it.INTENDANT_KEEP_TEST_TMP=1keeps smoke scratch after a failure.test-external-cu-session.pypassesTMPDIRinto its scrubbed daemon environment, so the daemon's Xauthority and Chromium temp land in the caller's temp root.Defensive CI (Linux fleet leg only; hosted runners and the Mac/Windows legs are unchanged)
Job-scoped temp dir: sets
TMPDIR=$RUNNER_TEMP/tmp. That dir belongs to one listener, and the runner wipes it at every job start and end.Reap job-scoped processes and temp: the last step, run with
always()so it also runs on cancellation.scripts/ci/reap_job_scope.pystops processes of this account that still reference the job temp dir (through an inheritedTMPDIR, or a cwd, argv entry or HOME under it), then deletes the dir. It never touches:It uses TERM, then KILL, and checks each process's start time first so a recycled pid is never signalled. It never fails the job; each process it had to stop is reported as a
::warning::, which is a leak signal to chase. Unit tests (a fake/proc, plus a live orphan on Linux) run in the Linux leg.scripts/ci/README.mddocuments the scope and the step-order interlock.Validation (Linux box, CI-like umask 022 and a job
TMPDIRunder a clean ancestor chain)cargo test -p intendant --bins: 6578 passed, 0 failed.test-external-cu-session.py --skip-expirypasses with theTMPDIRpassthrough.cargo clippy --workspace -- -D warnings: clean.cargo fmt --checkand actionlint: clean.TMPDIR, then removed the dir.browser_workspace::profile_templatetests require trusted-owner, non-group-writableTMPDIRancestors. That holds for the runner's$RUNNER_TEMPchain; they fail under a umask-002 scratch path whateverTMPDIRis.Not covered here (operator, one-time)
The existing
/tmpresidue and the three stale daemons predate the job scope, so the janitor deliberately does not match them. They need a one-time manual cleanup on the runner.