Skip to content

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

Merged
sean-brydon merged 3 commits into
cosscom:mainfrom
Stener1:box-upgrade-zombies
Oct 29, 2026
Merged

sean-brydon merged 3 commits into
cosscom:mainfrom
Stener1:box-upgrade-zombies

Conversation

@Stener1

@Stener1 Stener1 commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

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:

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

View guided diff

@vercel

vercel Bot commented Oct 27, 2026

Copy link
Copy Markdown

@Stener1 is attempting to deploy a commit to the seanbrydonio's projects Team on Vercel.

A member of the Team first needs to authorize it.

@Stener1
Stener1 marked this pull request as ready for review October 28, 2026 09:51

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 15 files

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread .claude/skills/verify-berthd/scripts/upgrade-zombies.sh Outdated
Comment thread .claude/skills/verify-berthd/scripts/upgrade-zombies.sh Outdated
Comment thread .claude/skills/verify-berthd/scripts/upgrade-zombies.sh
Comment thread .claude/skills/verify-berthd/scripts/upgrade-zombies.sh Outdated
Stener1 and others added 2 commits October 28, 2026 22:02
berthd upgrades by exec'ing the new build in its own process. What the
old build had started (the terminals' tmux clients above all) stayed its
children, but the new build had no exec.Cmd for them and never waited:
each became a zombie once it ended, one more per terminal per upgrade.

Before the exec, BeforeRestart now ends the attached tmux clients and
waits for their handlers to have waited for them (Sessions.Detach). The
new build lists the children it inherited before it starts any of its
own and waits for exactly those as they end (internal/childreap), never
wait4(-1), which would take os/exec's exit statuses.

scripts/upgrade-zombies.sh in the verify skill reproduces it on a real
box: attach terminals, upgrade, count berthd's zombie children.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n a bad setup

From review: the script now waits at most 20 s for berthd to start and
prints its log if it doesn't, exits when fewer terminals attach than
asked for (fewer clients would read as fewer zombies), names the upload
for the box's own arch so it runs on arm64, and passes the berth path to
the nested shell as an argument so any state directory works.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018aGVrw5b32ZtF9v24E5wwU
@Stener1
Stener1 force-pushed the box-upgrade-zombies branch from 82bdb51 to b316271 Compare October 28, 2026 22:02
@sean-brydon
sean-brydon merged commit 4fd47dc into cosscom:main Oct 29, 2026
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants