fix(box): upgrading a box no longer leaves zombie processes under berthd
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.Attachnow returns awaitfunc instead of the*exec.Cmd, so the box can track its clients.Sessions.Detachcancels them all and waits, up to a time limit, for the handlers' deferredwait()to run. Sessions keep running, and the laptop reattaches to the new build as it does today.internal/childreaplists this process's children from/proc/*/staton Linux and from sysctl on macOS (it doesn't startps, which would itself be a child). It then reaps only those PIDs. It never callswait4(-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 anythingDetachdoesn'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:
| upgrade | attached | zombies 5 s | zombies 60 s | berthd.log |
|---|---|---|---|---|
| main → main (before) | 3 | 3 | 3 | — |
| main → this branch | 3 | 0 | 0 | reaped 3 of 3 processes the build before this one left |
| this branch → this branch (after) | 3 | 0 | 0 | nothing 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