fix(box): upgrading a box no longer leaves zombie processes under berthd

Merged
ST
2h ago
+556-14

Summary

berthd upgrades itself by exec'ing the new build in its own process (internal/box/upgrade.go). The process id stays the same, so everything the old build had started is still berthd's child. The new build has no exec.Cmd for any of it and never waits for it. Each terminal's tmux client ends when its pty closes on exec and then stays a zombie, so every attached terminal adds one zombie per upgrade. A box that had been up five days had 151 of them: tmux clients, plus a few git, node and headless-chrome processes that were in flight during an upgrade.

The fix has two parts:

 handleUpgrade → BeforeRestart
   shares.StopAll()
   Helpers.Stop()
+  Sessions.Detach(2s)        # end attached tmux clients, refuse new ones,
+                             # wait until their handlers have Wait()ed them
   ln.Close()
   syscall.Exec(new build)

 serve (the new build)
+  inherited := childreap.Inherited()   # first thing, before it starts any child
   …
+  go childreap.Reap(ctx, inherited, 5s)  # wait4(pid, WNOHANG) on exactly those
  • Sessions.Attach now returns a wait func instead of the *exec.Cmd, so the box can track its clients. Sessions.Detach cancels them all and waits, up to a time limit, for the handlers' deferred wait() to run. Sessions keep running, and the laptop reattaches to the new build as it does today.
  • internal/childreap lists this process's children from /proc/*/stat on Linux and from sysctl on macOS (it doesn't start ps, which would itself be a child). It then reaps only those PIDs. It never calls wait4(-1), which would take exit statuses that os/exec is waiting for. A PID can't be reused while it's still unreaped, so the new build never reaps a child of its own. This is the backstop for anything Detach doesn't cover (git, browsers), and for a box coming from a build that doesn't have this fix.

Evidence

Repro on a real box (new verify-berthd/scripts/upgrade-zombies.sh FROM TO). It starts a throwaway box on its own state, tmux server and ports, attaches 3 terminals with berth attach, runs berth upgrade --force, and counts berthd's zombie children:

upgradeattachedzombies 5 szombies 60 sberthd.log
main → main (before)333—
main → this branch300reaped 3 of 3 processes the build before this one left
this branch → this branch (after)300nothing inherited: Detach drained them before the exec

In every run berth upgrade printed "Sessions kept running", and the log shows a second berthd serving from the same PID.

Tests

internal/childreap  (re-execs its own test binary, the way an upgrade does)
  TestWithoutReapThemTheyStayZombies          2 children left before exec → both still zombies after: the bug
  TestChildrenFromBeforeAnExecAreReaped       same, with Reap → 2 reaped, 0 left; a child the new
                                              process starts meanwhile keeps its exit status (Wait() == nil)
  TestAPidThatIsNotAChildIsDropped
internal/box
  TestDetachEndsAttachedClientsAndWaitsForThem  2 attached clients → ended and reaped well within the
                                                time limit, attach refused afterwards, session still running

The existing attach tests now wait for their clients (they used to Process.Kill() them and leave zombies).

go vet ./... is clean. go test -race ./... passes in every package except internal/leadline/api TestDetach, which fails the same way on main on this machine and doesn't depend on the packages changed here. The docs build passes.

Running in production

Since 2026-10-28 this change, together with #195, has run on one of my own boxes through my personal update channel (upstream's latest release plus my open PRs). The box is busy: 35–37 coding-agent sessions and about 20 attached terminals.

  • The bug, live: the app's own "update boxes automatically" upgrade between two release builds without this fix left 3 zombie tmux clients under berthd.
  • The fix, live: the first start of a build with this PR logged berthd: reaped 17 of 17 processes the build before this one left. Zombies under berthd were then 0, and all sessions survived with the same names.
  • Repeated upgrades: an upgrade between two patched builds with terminals attached showed 0 zombies at 5, 15, 30 and 60 s. So did three app-driven upgrades away from patched builds.

Merge Danger

Door: two-way

Reverting the commit restores the old behaviour. Nothing is stored, and there are no migrations.

Blast Radius: upgrades

Sessions.Attach's signature changed; its only caller is the attach handler. While BeforeRestart runs, a new attach is refused with "the box is restarting" for up to 2 s. Before this change the same request would have been dropped by the exec anyway. The reaper runs only when berthd starts with children already present, which happens only after a self-upgrade.

🤖 Generated with Claude Code