Skip to content

Commit c0d812f

Browse files
zook-botrefinery costing
authored andcommitted
Killed test runs leak their GOTMPDIR work dirs: 27 abandoned /var/tmp/gotmp/go-build* trees held 4.5G and ENOSPC-failed a push gate that passed 10/10 after reclaim (gc-68bao) (#176)
* fix(scripts): reclaim leaked go-build/go-link temp trees in the harness sweep (gc-68bao) go removes its per-invocation compile/link work dir under $GOTMPDIR only on a clean exit. A run the harness watchdog kills (SIGQUIT->SIGKILL), a signalled run, or an OOM kill leaks the tree, and nothing sweeps it. gc-68bao measured 27 such trees holding 4.5G that ENOSPC-failed a push gate which then passed 10/10 once they were reclaimed; the linker's go-link-* trees are the largest single hoard. The trees appear both under $GOTMPDIR and, when a run set no GOTMPDIR, directly under /var/tmp. The process sweep in scripts/lib/harness-reap.sh already reaps stranded test processes at gate start but never their leaked directories. This adds gc_harness_sweep_stale_build_dirs there, called from test-local-parallel (the one funnel all four heavy gate targets pass through) right after the process sweep and on the same age floor, so the reclaim runs on every gate without a new order or command. The reclaim is deliberately bounded to go's own go-build*/go-link* shapes. That bound is the safety argument: those prefixes are go-owned, never a checkout, a cache-with-value, or a comparison base a human wants kept, and are regenerated on the next build, so removing one needs no per-shape judgment. A tree is reclaimed only when it is owned by the invoking user, older than the go test budget, and referenced by no live process — the environ+cmdline+cwd /proc predicate proven safe in gc-68bao (it spared every live build across two concurrent gates). Every delete routes through the single gc_harness_reclaim_dir seam, mirroring gc_harness_kill_pid, so the destroy authority is reviewable in isolation and the self-test records decisions instead of removing real trees. Scope: gc-68bao's notes also describe a larger /var/tmp half — gocache-* and cold-build throwaway caches, 12.6G in one incident. A naive age+liveness scan of that class was measured against the real host and matched 662 owned dirs of mixed provenance, including comparison bases; encoding an unconditional rm -rf of that set on every fleet gate is an unrecoverable blast radius and a policy call, so it is filed as gc-ulys6 rather than shipped here. Validation: new self-test TestHarnessSweepReclaimsLeakedGoWorkDirsButSparesLiveAndProtected (reclaims an abandoned tree at either root; spares a live-held tree, a too-young tree, a non-go shape, and a .git-bearing dir). go vet clean; scripts harness/budget/tmpdir suites green. A print-only dry run over the real host /var/tmp selected exactly the ~10 real leaked go trees and none of the 662 other dirs. Follow-up: gc-ulys6 (the /var/tmp agent-cache half). * test(census): bump subprocess baselines for the new harness sweep test (gc-68bao) The fix commit added buildDirSweepHarness to scripts/harness_reap_test.go, a fifth exec.Command("bash", ...) call that shells out to exercise gc_harness_sweep_stale_build_dirs. That file is untagged (no //go:build constraint), so the new call lands in the untagged subprocess census and trips resourcecensus's "totals cannot grow" ratchet: TestRepositoryLedgerMatchesCensusAndDocumentation went red with the scanned count one above the checked baseline on three rows. The ledger, the bootstrap policy, and the rendered doc are three mirrors that comparePolicyFields/validateBaseline require to agree exactly, so the +1 is applied to all three: test/test-resources.toml and census.go's bootstrapPolicy (baseline_calls 649->650 all-source, 432->433 untagged source, 425->426 untagged Small), then TESTING.md regenerated via the -update flag. Only baseline_calls moves; baseline_files is unchanged because the call joined a file already counted, and the historical reported_* columns are left as-is. The new subprocess is a legitimate test harness that runs a shell function, so raising the ratcheted baseline is the sanctioned path (the fixed-sleep census is the one you shrink instead). Validation: full internal/testpolicy/resourcecensus package green without -update, go vet clean, and the deliverable's own TestHarnessSweepReclaimsLeakedGoWorkDirsButSparesLiveAndProtected still passes. * docs(scripts): state current behavior in harness-sweep comments The go work-dir sweep's living comments carried a bead id and incident history — measured hoard sizes, gate pass/fail counts, a follow-up policy note. Convention is that comments state what is true now and the constraints it rests on; history belongs in the bead and the commit log. Rewrite the four comment blocks in scripts/lib/harness-reap.sh, scripts/test-local-parallel, and scripts/harness_reap_test.go to state only the reclaim's behavior and safety contract: it reclaims stale same-UID go-build/go-link temp dirs after age and live-reference checks, scans each caller-named root at depth 1, and leaves broader cache-root cleanup out of scope. No behavior change. Addresses pre-open signoff finding (round 1) on polecat/gc-68bao. * fix(scripts): reclaim only go's exact go-build<n>/go-link-<n> shapes (gc-68bao) The sweep matched the bare go-build*/go-link* prefix, so an old, unreferenced human-named scratch dir such as go-build-base was selected and rm -rf'd. go names its work dirs with os.MkdirTemp(dir, "go-build") and os.MkdirTemp(dir, "go-link-"), which append a decimal suffix, so the only go-owned shapes are go-build<digits> and go-link-<digits>. Match exactly those. The regression test renames its spare fixtures to valid go shapes so the liveness/age/.git predicates (not shape) do the sparing, and adds prefix-but-not-generated-shape cases the old glob would have reclaimed. --------- Co-authored-by: refinery costing <refinery@local> Rebased 2026-09-15 onto upstream 83c8270: the resource-census +1 hunks (census.go, test-resources.toml, TESTING.md) were deltas on the previous sync's baselines and are re-derived over the merged tree in the census commit at the end of this rebase; the reaper, its test and the test-local-parallel sweep call merged unchanged. Claude-Session: https://claude.ai/code/session_01XfZfLDuwcBfs3TXnn1udn9
1 parent 7527859 commit c0d812f

3 files changed

Lines changed: 237 additions & 0 deletions

File tree

scripts/harness_reap_test.go

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"os"
77
"os/exec"
88
"path/filepath"
9+
"runtime"
910
"strconv"
1011
"strings"
1112
"syscall"
@@ -459,3 +460,124 @@ func TestGoTestShardLeavesNothingHoldingTheCallersPipe(t *testing.T) {
459460
t.Fatal("reading the shard runner's output blocked after the run finished: a descendant outlived the run holding the caller's pipe")
460461
}
461462
}
463+
464+
// buildDirSweepHarness runs gc_harness_sweep_stale_build_dirs against a fixture
465+
// scratch tree and returns its reclaim decisions. Two seams are overridden: the
466+
// liveness source (so the test supplies a process's referenced paths without a
467+
// real /proc) and the one function that deletes a tree (so decisions are
468+
// recorded instead of destroying real dirs), mirroring how the process sweep
469+
// overrides gc_harness_kill_pid.
470+
func buildDirSweepHarness(t *testing.T, minAge string, roots []string, referenced string) string {
471+
t.Helper()
472+
473+
dir := t.TempDir()
474+
refFixture := filepath.Join(dir, "referenced.txt")
475+
if err := os.WriteFile(refFixture, []byte(referenced), 0o644); err != nil {
476+
t.Fatalf("write referenced fixture: %v", err)
477+
}
478+
reclaimLog := filepath.Join(dir, "reclaims")
479+
480+
var rootArgs strings.Builder
481+
for _, r := range roots {
482+
fmt.Fprintf(&rootArgs, " %q", r)
483+
}
484+
485+
script := fmt.Sprintf(`
486+
set -euo pipefail
487+
source %q
488+
gc_harness_referenced_paths() { cat %q ; }
489+
gc_harness_reclaim_dir() { printf 'RECLAIMED %%s\n' "$1" >> %q ; }
490+
gc_harness_sweep_stale_build_dirs %s%s
491+
`, filepath.Join(repoRoot(t), "scripts", "lib", "harness-reap.sh"),
492+
refFixture, reclaimLog, minAge, rootArgs.String())
493+
494+
out, err := exec.Command("bash", "-c", script).CombinedOutput()
495+
if err != nil {
496+
t.Fatalf("build-dir sweep failed: %v\n%s", err, out)
497+
}
498+
reclaims, readErr := os.ReadFile(reclaimLog)
499+
if readErr != nil && !os.IsNotExist(readErr) {
500+
t.Fatalf("read reclaim log: %v", readErr)
501+
}
502+
return string(reclaims) + string(out)
503+
}
504+
505+
// TestHarnessSweepReclaimsLeakedGoWorkDirsButSparesLiveAndProtected verifies the
506+
// go work-dir sweep: go removes its work dir only on a clean exit, so a run the
507+
// harness watchdog or a signal kills leaks the tree and nothing else sweeps it.
508+
// The sweep must reclaim an abandoned go-build<n>/go-link-<n> tree at any of the
509+
// scratch roots it is given while sparing a tree a live build still holds, one
510+
// too young to be abandoned, anything carrying a .git pointer, and — because the
511+
// sweep rm -rf's whatever it matches — any dir that is not go's exact generated
512+
// shape, including a go-prefixed but human-named comparison base.
513+
func TestHarnessSweepReclaimsLeakedGoWorkDirsButSparesLiveAndProtected(t *testing.T) {
514+
if runtime.GOOS != "linux" {
515+
t.Skip("go work-dir reclaim reads /proc for liveness; Linux-only")
516+
}
517+
518+
root := t.TempDir() // stands in for /var/tmp
519+
gotmp := filepath.Join(root, "gotmp") // stands in for $GOTMPDIR
520+
521+
old := time.Now().Add(-time.Hour)
522+
fresh := time.Now()
523+
524+
mk := func(parent, name string, mtime time.Time, dotGit bool) string {
525+
p := filepath.Join(parent, name)
526+
if err := os.MkdirAll(p, 0o755); err != nil {
527+
t.Fatalf("mkdir %s: %v", p, err)
528+
}
529+
if dotGit {
530+
if err := os.WriteFile(filepath.Join(p, ".git"), []byte("gitdir: /elsewhere\n"), 0o644); err != nil {
531+
t.Fatalf("write .git in %s: %v", p, err)
532+
}
533+
}
534+
if err := os.Chtimes(p, mtime, mtime); err != nil {
535+
t.Fatalf("chtimes %s: %v", p, err)
536+
}
537+
return p
538+
}
539+
540+
reclaimAbandoned := mk(root, "go-build2147480000", old, false)
541+
reclaimLink := mk(root, "go-link-4242", old, false)
542+
// Spared by a real predicate, not by shape: each carries a valid
543+
// go-build<n>/go-link-<n> name so the shape gate passes and the
544+
// liveness/age/.git predicate is what does the sparing.
545+
spareLive := mk(root, "go-build770001", old, false)
546+
spareFresh := mk(root, "go-build550002", fresh, false)
547+
spareWorktree := mk(root, "go-build330003", old, true)
548+
// Spared by shape. go builds its work dirs with os.MkdirTemp(dir,
549+
// "go-build") and os.MkdirTemp(dir, "go-link-"), which append a decimal
550+
// suffix, so the only go-owned shapes are go-build<digits> and
551+
// go-link-<digits>. These share a go prefix but are not that shape — a
552+
// human-named comparison base, or a digit-then-letter tail — and one carries
553+
// no go prefix at all (a real cache dir). All are old and unreferenced, so
554+
// shape is the only thing standing between them and an rm -rf.
555+
spareHumanBuild := mk(root, "go-build-base", old, false)
556+
spareHumanLink := mk(root, "go-link-base", old, false)
557+
spareDigitThenLetters := mk(root, "go-build12ab", old, false)
558+
spareNonGo := mk(root, "gocache-tk-nnx2gd", old, false)
559+
560+
// The /var/tmp scan does not descend, so a tree leaked under $GOTMPDIR is
561+
// only reclaimed through gotmp being passed as its own root. gotmp itself is
562+
// a child of root but is spared there: its name is not a go work-dir shape.
563+
mk(root, "gotmp", old, false)
564+
reclaimNested := mk(gotmp, "go-build9998887", old, false)
565+
566+
// A live build holds spareLive as its working directory; the seam feeds that
567+
// path to the sweep as the sole referenced path.
568+
got := buildDirSweepHarness(t, "60", []string{root, gotmp}, spareLive+"\n")
569+
570+
for _, want := range []string{reclaimAbandoned, reclaimLink, reclaimNested} {
571+
if !strings.Contains(got, "RECLAIMED "+want+"\n") {
572+
t.Errorf("sweep left an abandoned go work dir behind: %s\nlog:\n%s", want, got)
573+
}
574+
}
575+
for _, spared := range []string{
576+
spareLive, spareFresh, spareWorktree,
577+
spareHumanBuild, spareHumanLink, spareDigitThenLetters, spareNonGo,
578+
} {
579+
if strings.Contains(got, "RECLAIMED "+spared+"\n") {
580+
t.Errorf("sweep reclaimed a dir it must spare: %s\nlog:\n%s", spared, got)
581+
}
582+
}
583+
}

scripts/lib/harness-reap.sh

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,3 +289,108 @@ gc_harness_test_socket_holders() {
289289
}
290290
'
291291
}
292+
293+
# gc_harness_referenced_paths prints every filesystem path a live process of the
294+
# invoking user still references — env values, argv, and cwd — one per line. It
295+
# is the liveness half of gc_harness_sweep_stale_build_dirs: a work dir whose
296+
# path appears here belongs to a running build and must be spared. Reading
297+
# /proc/<pid>/{environ,cmdline,cwd} is the liveness predicate. Only the invoking
298+
# user's process files are readable, which is exactly the set that can own a
299+
# 0700 go work dir, so a different user's live build can never be misread as
300+
# absent. Factored out as the one data source so the self-test can inject a
301+
# process table without a real /proc.
302+
#
303+
# stderr is discarded at the loop, not per command: opening another user's
304+
# /proc/<pid>/environ is denied, and that open failure is the shell's, emitted
305+
# before a trailing per-command 2>/dev/null on the same line can take effect —
306+
# so on a shared host it would otherwise flood every caller with one
307+
# "Permission denied" per foreign process. A denied entry is simply not one of
308+
# ours and contributes nothing to the set.
309+
gc_harness_referenced_paths() {
310+
local p
311+
for p in /proc/[0-9]*; do
312+
tr '\0' '\n' < "$p/environ" || true
313+
tr '\0' '\n' < "$p/cmdline" || true
314+
readlink "$p/cwd" || true
315+
done 2>/dev/null
316+
}
317+
318+
# gc_harness_reclaim_dir removes one abandoned work-dir tree. Every reclaim goes
319+
# through here — the one place the sweep deletes a directory — mirroring
320+
# gc_harness_kill_pid for process kills: it keeps the destroy authority
321+
# reviewable in isolation and gives the self-test a seam to record decisions
322+
# instead of removing real trees.
323+
gc_harness_reclaim_dir() {
324+
local dir="${1:-}"
325+
[[ -n "$dir" ]] || return 0
326+
rm -rf -- "$dir" 2>/dev/null || true
327+
}
328+
329+
# gc_harness_sweep_stale_build_dirs reclaims go's per-invocation compile and
330+
# link temp trees that a killed run leaked. go removes its work dir only on a
331+
# clean exit, so a run ended by the watchdog above, a signal, or an OOM kill
332+
# leaves the tree behind and nothing else sweeps it. The trees appear both under
333+
# $GOTMPDIR and, when a run set no GOTMPDIR, directly under its TMPDIR, so every
334+
# scratch root the caller names is scanned at depth 1.
335+
#
336+
# go builds those dirs with os.MkdirTemp(dir, "go-build") and
337+
# os.MkdirTemp(dir, "go-link-"), each of which appends a decimal random suffix,
338+
# so the only go-owned shapes are go-build<digits> and go-link-<digits>. The
339+
# sweep matches exactly those, and that exactness is the whole safety argument:
340+
# a go-owned work dir is never a checkout, a cache-with-value, or a comparison
341+
# base a human wants kept, and is regenerated on the next build, so reclaiming
342+
# one is always safe and needs no per-shape judgment. Matching the bare
343+
# go-build/go-link prefix instead would also select human-named scratch such as
344+
# go-build-base, which this path would then rm -rf. Broader cache-root cleanup —
345+
# arbitrary scratch dirs of mixed provenance, which can include comparison bases
346+
# a human wants kept — is out of scope here. A tree is reclaimed only when it is
347+
# owned by the invoking user, older than min_age_seconds, and referenced by no
348+
# live process. The age floor is the caller's go test budget, so a concurrently
349+
# starting build is never in scope, and the liveness check spares an in-flight
350+
# build regardless of age. Set GC_TEST_NO_BUILD_DIR_SWEEP=1 to skip the reclaim
351+
# entirely.
352+
gc_harness_sweep_stale_build_dirs() {
353+
local min_age_seconds="${1:-3600}"
354+
if (( $# > 0 )); then shift; fi
355+
[[ "${GC_TEST_NO_BUILD_DIR_SWEEP:-0}" != "1" ]] || return 0
356+
[[ $# -gt 0 ]] || return 0
357+
# Liveness assessment needs procfs. Without it a dir cannot be proven unused,
358+
# and reclaiming on an empty liveness set would delete a live build, so the
359+
# whole sweep skips rather than guess — the leak is a Linux-host condition.
360+
[[ -e /proc/self/cmdline ]] || return 0
361+
[[ "$min_age_seconds" =~ ^[0-9]+$ ]] || return 0
362+
363+
local uid now referenced
364+
uid="$(id -u)"
365+
now="$(date +%s)"
366+
referenced="$(gc_harness_referenced_paths)"
367+
368+
local reclaimed=0
369+
local root child base mtime
370+
for root in "$@"; do
371+
[[ -n "$root" && -d "$root" ]] || continue
372+
while IFS= read -r child; do
373+
[[ -n "$child" ]] || continue
374+
base="${child##*/}"
375+
# Match only go's generated shapes, not the bare go-build/go-link prefix:
376+
# this path rm -rf's what it matches, and the prefix alone also selects
377+
# human-named scratch such as go-build-base.
378+
[[ "$base" =~ ^go-build[0-9]+$ || "$base" =~ ^go-link-[0-9]+$ ]] || continue
379+
# A go work dir never carries a .git pointer; skipping one is defence in
380+
# depth so a name collision can never turn the sweep on a checkout.
381+
[[ -e "$child/.git" ]] && continue
382+
mtime="$(stat -c %Y "$child" 2>/dev/null)" || continue
383+
(( now - mtime >= min_age_seconds )) || continue
384+
if [[ -n "$referenced" ]] && grep -qF -- "$child" <<< "$referenced"; then
385+
continue
386+
fi
387+
printf 'harness sweep: reclaiming abandoned go work dir %s (age %ss, no live holder)\n' \
388+
"$child" "$(( now - mtime ))" >&2
389+
gc_harness_reclaim_dir "$child"
390+
reclaimed=$(( reclaimed + 1 ))
391+
done < <(find "$root" -maxdepth 1 -mindepth 1 -type d -uid "$uid" 2>/dev/null || true)
392+
done
393+
394+
(( reclaimed == 0 )) || printf 'harness sweep: reclaimed %s abandoned go work dir(s)\n' "$reclaimed" >&2
395+
return 0
396+
}

scripts/test-local-parallel

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,16 @@ export TEST_LOCAL_NICE="$nice_prefix"
287287
sweep_min_age_seconds="$(gc_harness_duration_seconds "$go_test_timeout")" || sweep_min_age_seconds=3600
288288
gc_harness_sweep_stale_orphans "$sweep_min_age_seconds"
289289

290+
# Reclaim go's leaked compile/link temp before adding load of our own. go
291+
# deletes its per-invocation work dir only on a clean exit, so a run the
292+
# watchdog above, a signal, or an OOM kill ends leaks go-build*/go-link* under
293+
# $GOTMPDIR — or under /var/tmp when the run set no GOTMPDIR — and nothing else
294+
# sweeps them. Same age floor as the process sweep, so an in-flight build is
295+
# never in scope; a bare session reports no GOTMPDIR, so fall back to the shim's
296+
# on-disk root.
297+
gc_harness_sweep_stale_build_dirs "$sweep_min_age_seconds" \
298+
"${gotmpdir_val:-/var/tmp/gotmp}" /var/tmp
299+
290300
echo "Running ${#jobspecs[@]} ${mode} job(s) with LOCAL_TEST_JOBS=${local_jobs} inner_p=${inner_p}"
291301

292302
set +e

0 commit comments

Comments
 (0)