Reap orphaned processes adopted by the wrapper

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)
 	}
 }

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/<pid>/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
+}

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())
+	}
+}

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)
@@ -97,13 +97,24 @@ 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()
+	}
+	_ = 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