From cf73a4d6b418ee27429f606a6e3b07306d681d5a Mon Sep 17 00:00:00 2001 From: Sayan- <1415138+Sayan-@users.noreply.github.com> Date: Mon, 24 Aug 2026 20:44:59 +0000 Subject: [PATCH 1/2] Reap the log aggregator's tail processes tailFile started tail -F and never waited on it. The wrapper runs as pid 1 in both the container and the unikernel, so every exited tail stayed a zombie for the life of the instance. A scan that ends on a read error rather than EOF leaves tail alive, so kill it before waiting instead of parking the goroutine forever. --- server/cmd/wrapper/supervisord.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/server/cmd/wrapper/supervisord.go b/server/cmd/wrapper/supervisord.go index 4672db47..1d2ad256 100644 --- a/server/cmd/wrapper/supervisord.go +++ b/server/cmd/wrapper/supervisord.go @@ -97,6 +97,17 @@ func tailFile(path string) { for scanner.Scan() { fmt.Printf("[%s] %s\n", label, scanner.Text()) } + // A clean scan ends when tail closes its stdout, i.e. when it has exited. + // A scan that ends on an error (a log line past the 1MB cap, say) leaves + // tail running, so kill it rather than block here forever. + // + // Either way we have to collect it: the wrapper is pid 1 in both the + // container and the unikernel, so a tail nobody waits on stays a zombie + // for the life of the instance. + if scanner.Err() != nil { + _ = cmd.Process.Kill() + } + _ = cmd.Wait() } func runStream(label, name string, args ...string) error { From 339f1944c36f50d6be6d1044a4abe801b2c47e8c Mon Sep 17 00:00:00 2001 From: Sayan- <1415138+Sayan-@users.noreply.github.com> Date: Mon, 24 Aug 2026 20:48:06 +0000 Subject: [PATCH 2/2] Reap orphaned processes adopted by the wrapper The wrapper is pid 1 in both the container and the unikernel, so any process whose parent exits first gets reparented onto it. Nothing waited on those, so each one stayed a zombie holding a pid slot until the instance died. Chromium relaunches are the common source, via their crashpad handlers. Reaping cannot simply wait4(-1): os/exec waits on a specific pid, and losing that race drops the exit status main relies on to know when supervisord is done. Track the commands we start and only collect pids we don't own, keyed by command identity so a pid freed by Wait and reused before the release lands can't evict the new holder's entry. --- server/cmd/wrapper/main.go | 13 ++- server/cmd/wrapper/reap.go | 152 ++++++++++++++++++++++++++++++ server/cmd/wrapper/reap_test.go | 100 ++++++++++++++++++++ server/cmd/wrapper/supervisord.go | 6 +- 4 files changed, 263 insertions(+), 8 deletions(-) create mode 100644 server/cmd/wrapper/reap.go create mode 100644 server/cmd/wrapper/reap_test.go diff --git a/server/cmd/wrapper/main.go b/server/cmd/wrapper/main.go index 37b70e3a..7da559f0 100644 --- a/server/cmd/wrapper/main.go +++ b/server/cmd/wrapper/main.go @@ -76,6 +76,9 @@ func main() { startupCtx, cancelStartup := context.WithCancel(context.Background()) defer cancelStartup() + // Nothing else reaps the orphans we adopt as pid 1; start before we fork. + startReaper() + // /dev/shm: only mount when not running under Docker (Docker manages it). if os.Getenv("WITHDOCKER") == "" { _ = os.MkdirAll("/dev/shm", 0o1777) @@ -137,12 +140,12 @@ func main() { // supervisord — start in nodaemon mode so we own its lifecycle. // Without -n it forks and the parent exits with code 0, which would - // drop us out of supCmd.Wait() and the container would stop. + // drop us out of waitOwned(supCmd) and the container would stop. logf("starting supervisord") supCmd := exec.Command("supervisord", "-n", "-c", supervisorConf) supCmd.Stdout = os.Stdout supCmd.Stderr = os.Stderr - if err := supCmd.Start(); err != nil { + if err := startOwned(supCmd); err != nil { fatalf("supervisord start: %v", err) } // Install the shutdown goroutine now so it can clean up if a signal @@ -196,7 +199,7 @@ func main() { if err := prepareSnapshotStartPage(startupCtx, os.Getenv("INTERNAL_PORT")); err != nil { if errors.Is(err, context.Canceled) { logf("snapshot start page preparation canceled") - if err := supCmd.Wait(); err != nil { + if err := waitOwned(supCmd); err != nil { logf("supervisord exited: %v", err) } return @@ -217,7 +220,7 @@ func main() { forkIdentity, identityDeadline, ok := waitForForkIdentityIfEnabled(startupCtx, forkIdentityWait) if !ok { - if err := supCmd.Wait(); err != nil { + if err := waitOwned(supCmd); err != nil { logf("supervisord exited: %v", err) } return @@ -264,7 +267,7 @@ func main() { } // Block on supervisord; container exits when it does. - if err := supCmd.Wait(); err != nil { + if err := waitOwned(supCmd); err != nil { logf("supervisord exited: %v", err) } } diff --git a/server/cmd/wrapper/reap.go b/server/cmd/wrapper/reap.go new file mode 100644 index 00000000..f50487f8 --- /dev/null +++ b/server/cmd/wrapper/reap.go @@ -0,0 +1,152 @@ +package main + +import ( + "bytes" + "os" + "os/exec" + "os/signal" + "strconv" + "strings" + "sync" + "syscall" + "time" +) + +// The wrapper is pid 1 in both the container (ENTRYPOINT) and the unikernel +// (Kraftfile cmd), so any process whose parent dies first is reparented onto +// it — chromium's crashpad handlers are the common case, one per relaunch. +// Nothing waits on those, so each stays a zombie holding a pid slot until the +// instance dies, and every /proc walk on the host slows down with them. +// +// Reaping can't just be wait4(-1) on SIGCHLD: os/exec waits on a specific pid, +// and if the reaper collects one of our children first that Cmd.Wait fails +// with ECHILD and loses its exit status. So we track the commands we start and +// only collect pids we don't own. Ownership is keyed by command identity +// because Cmd.Wait frees the pid before releaseOwned runs, and a late release +// must not evict whatever took that pid next. +// +// Only commands whose exit status we act on need this — supervisord, whose +// exit ends the instance, and runStream, which runStreamFatal turns into a +// boot failure. The rest already throw their status away with `_ =`, so +// there's nothing for the reaper to take from them. +var owned = struct { + sync.Mutex + cmds map[int]*exec.Cmd +}{cmds: map[int]*exec.Cmd{}} + +// startOwned starts cmd and records it as ours to wait on. The lock is held +// across Start so a concurrent reap can't observe the child in the window +// between fork and registration. +func startOwned(cmd *exec.Cmd) error { + owned.Lock() + defer owned.Unlock() + if err := cmd.Start(); err != nil { + return err + } + owned.cmds[cmd.Process.Pid] = cmd + return nil +} + +// waitOwned waits on a command started by startOwned and releases its pid. +func waitOwned(cmd *exec.Cmd) error { + err := cmd.Wait() + releaseOwned(cmd) + return err +} + +// releaseOwned drops cmd's pid, but only while that pid still belongs to cmd. +func releaseOwned(cmd *exec.Cmd) { + pid := cmd.Process.Pid + owned.Lock() + if owned.cmds[pid] == cmd { + delete(owned.cmds, pid) + } + owned.Unlock() +} + +// runOwned is startOwned followed by waitOwned: the replacement for Cmd.Run. +func runOwned(cmd *exec.Cmd) error { + if err := startOwned(cmd); err != nil { + return err + } + return waitOwned(cmd) +} + +// startReaper drains adopted zombies for the life of the process. It does +// nothing when we aren't pid 1, since then orphans reparent elsewhere. +func startReaper() { + if os.Getpid() != 1 { + return + } + ch := make(chan os.Signal, 1) + signal.Notify(ch, syscall.SIGCHLD) + go func() { + // SIGCHLD coalesces, so a burst arriving mid-drain can leave + // stragglers. The ticker bounds how long one sits around. + tick := time.NewTicker(30 * time.Second) + defer tick.Stop() + for { + select { + case <-ch: + case <-tick.C: + } + reapOrphans() + } + }() +} + +func reapOrphans() { + for _, pid := range zombieChildren() { + owned.Lock() + if owned.cmds[pid] == nil { + var ws syscall.WaitStatus + // A pid that slipped away between the scan and here just comes + // back around on the next wakeup. + _, _ = syscall.Wait4(pid, &ws, syscall.WNOHANG, nil) + } + owned.Unlock() + } +} + +// zombieChildren returns the pids of our children currently in Z state. +func zombieChildren() []int { + entries, err := os.ReadDir("/proc") + if err != nil { + return nil + } + self := os.Getpid() + var zombies []int + for _, e := range entries { + pid, err := strconv.Atoi(e.Name()) + if err != nil { + continue + } + if state, ppid, ok := procStat(pid); ok && state == 'Z' && ppid == self { + zombies = append(zombies, pid) + } + } + return zombies +} + +// procStat reads a process's state and parent pid from /proc//stat. +// comm is the second field, parenthesized, and may itself contain spaces and +// parens, so the fixed-width fields are read from after its closing paren. +func procStat(pid int) (state byte, ppid int, ok bool) { + b, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat") + if err != nil { + return 0, 0, false + } + commEnd := bytes.LastIndexByte(b, ')') + if commEnd < 0 || commEnd+2 >= len(b) { + return 0, 0, false + } + fields := strings.Fields(string(b[commEnd+2:])) + if len(fields) < 2 { + return 0, 0, false + } + parent, err := strconv.Atoi(fields[1]) + if err != nil { + return 0, 0, false + } + return fields[0][0], parent, true +} diff --git a/server/cmd/wrapper/reap_test.go b/server/cmd/wrapper/reap_test.go new file mode 100644 index 00000000..51facffb --- /dev/null +++ b/server/cmd/wrapper/reap_test.go @@ -0,0 +1,100 @@ +package main + +import ( + "os" + "os/exec" + "testing" + "time" +) + +func waitForZombie(t *testing.T, pid int) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if state, ppid, ok := procStat(pid); ok && state == 'Z' && ppid == os.Getpid() { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatalf("pid %d never became a zombie child", pid) +} + +func isZombieChild(pid int) bool { + state, ppid, ok := procStat(pid) + return ok && state == 'Z' && ppid == os.Getpid() +} + +func TestReapOrphansCollectsUnownedZombie(t *testing.T) { + cmd := exec.Command("true") + if err := cmd.Start(); err != nil { + t.Fatalf("start: %v", err) + } + pid := cmd.Process.Pid + waitForZombie(t, pid) + + reapOrphans() + + if isZombieChild(pid) { + t.Fatalf("pid %d still a zombie after reapOrphans", pid) + } +} + +func TestReapOrphansLeavesOwnedZombie(t *testing.T) { + cmd := exec.Command("true") + if err := startOwned(cmd); err != nil { + t.Fatalf("startOwned: %v", err) + } + pid := cmd.Process.Pid + waitForZombie(t, pid) + + reapOrphans() + + if !isZombieChild(pid) { + t.Fatalf("reapOrphans collected owned pid %d", pid) + } + if err := waitOwned(cmd); err != nil { + t.Fatalf("waitOwned after reapOrphans: %v", err) + } + if owned.cmds[pid] != nil { + t.Fatalf("pid %d still tracked after waitOwned", pid) + } +} + +func TestReleaseOwnedKeepsLaterHolderOfSamePid(t *testing.T) { + first := exec.Command("true") + if err := startOwned(first); err != nil { + t.Fatalf("startOwned: %v", err) + } + pid := first.Process.Pid + if err := waitOwned(first); err != nil { + t.Fatalf("waitOwned: %v", err) + } + + // Stand in for the kernel handing this pid to a later command. + second := exec.Command("true") + owned.Lock() + owned.cmds[pid] = second + owned.Unlock() + + releaseOwned(first) + + owned.Lock() + defer owned.Unlock() + if owned.cmds[pid] != second { + t.Fatal("a stale release evicted the current holder of the pid") + } + delete(owned.cmds, pid) +} + +func TestProcStatReadsSelf(t *testing.T) { + state, ppid, ok := procStat(os.Getpid()) + if !ok { + t.Fatal("procStat failed on self") + } + if state == 'Z' { + t.Fatalf("self reported as zombie") + } + if ppid != os.Getppid() { + t.Fatalf("ppid = %d, want %d", ppid, os.Getppid()) + } +} diff --git a/server/cmd/wrapper/supervisord.go b/server/cmd/wrapper/supervisord.go index 1d2ad256..a907b5bb 100644 --- a/server/cmd/wrapper/supervisord.go +++ b/server/cmd/wrapper/supervisord.go @@ -88,7 +88,7 @@ func tailFile(path string) { return } cmd.Stderr = nil - if err := cmd.Start(); err != nil { + if err := startOwned(cmd); err != nil { return } label := filepath.Base(path) @@ -107,14 +107,14 @@ func tailFile(path string) { if scanner.Err() != nil { _ = cmd.Process.Kill() } - _ = cmd.Wait() + _ = waitOwned(cmd) } func runStream(label, name string, args ...string) error { cmd := exec.Command(name, args...) cmd.Stdout = prefixWriter{label: label, w: os.Stdout} cmd.Stderr = prefixWriter{label: label, w: os.Stderr} - return cmd.Run() + return runOwned(cmd) } // runStreamFatal is runStream + fatalf on non-zero exit. Use for scripts the