Repository navigation
fix(box): upgrading a box no longer leaves zombie processes under berthd - #194
Merged
Merged
Conversation
|
@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
marked this pull request as ready for review
October 28, 2026 09:51
There was a problem hiding this comment.
All reported issues were addressed across 15 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
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
force-pushed
the
box-upgrade-zombies
branch
from
October 28, 2026 22:02
82bdb51 to
b316271
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 noexec.Cmdfor 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:
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 withberth attach, runsberth upgrade --force, and counts berthd's zombie children:reaped 3 of 3 processes the build before this one leftDetachdrained them before the execIn every run
berth upgradeprinted "Sessions kept running", and the log shows a secondberthd servingfrom the same PID.Tests
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 exceptinternal/leadline/apiTestDetach, which fails the same way onmainon 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.
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.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. WhileBeforeRestartruns, 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