Skip to content

fix: tear down the session's whole process tree on kill#98

Merged
kylecarbs merged 2 commits into
mainfrom
fix/kill-process-tree
Jul 5, 2026
Merged

fix: tear down the session's whole process tree on kill#98
kylecarbs merged 2 commits into
mainfrom
fix/kill-process-tree

Conversation

@kylecarbs

Copy link
Copy Markdown
Member

Summary

boo kill sent a single SIGHUP to the session shell's pid. Any descendant that ignored SIGHUP, was placed in its own process group by the shell's job control, or was backgrounded survived the session and kept holding ports and files, so a replacement session could fail with errors like Address already in use.

This snapshots the shell's descendant tree while it is still intact, hangs up the shell's process group, SIGTERMs every descendant by pid, then escalates to SIGKILL for any survivors after a bounded grace period. A per-process start time recorded at snapshot guards every bare-pid signal against pid reuse. Enumeration uses /proc on Linux and libproc on macOS, matching the existing src/cwd.zig approach.

Fixes #97.

Why not the one-line fix suggested in the issue?

The issue suggests kill(-child_pid, SIGHUP) (signal the process group). That is a reasonable improvement, but it does not fix the issue's own reproduction:

  • Under shell job control the foreground job gets its own process group, so a signal to the shell's process group never reaches it.
  • The reproduction process explicitly ignores SIGHUP, so even a delivered hangup is dropped. (The kernel already sends SIGHUP to the terminal's foreground group when the session leader dies, so a group-HUP adds little for a HUP-ignoring process.)

Reliably tearing such a process down requires enumerating the descendant tree and escalating to SIGKILL. The new integration test encodes this: it fails on main (orphaned descendant survived kill) and passes here.

Behavior notes

  • boo kill still returns promptly: the ack and the client exit notices are flushed before the teardown, so the grace period never blocks the caller (each session has its own daemon process).
  • The common case stays fast: teardown returns as soon as the tree drains, so it only reaches the full grace when a process actively ignores SIGTERM.

Known limitation (documented, same as tmux/screen)

A descendant that double-forks into its own session and reparents to init before the kill cannot be discovered from the shell and is not reached. This is called out in src/reap.zig's doc comment.

Implementation plan and decision log

Root cause (src/daemon.zig, quit command): posix.kill(w.child_pid, posix.SIG.HUP) targets a single pid (the shell). Grandchildren that ignore SIGHUP or left the shell's process group survive.

Options considered

  1. kill(-child_pid, SIGHUP) (issue suggestion). Rejected as insufficient: does not tear down the issue's reproduction (own process group + SIGHUP ignored). Verified by a failing test.
  2. Process-group SIGKILL. Still misses foreground jobs the shell placed in their own process groups.
  3. Full descendant-tree teardown with TERM then KILL escalation. Chosen: the only approach that tears down the reproduction.

Design

  • src/reap.zig terminateTree(alloc, leader):
    1. Snapshot the descendant tree before signaling (once the leader dies, children reparent to init and the links vanish). Linux: scan /proc, resolve to a fixpoint over parent links. macOS: BFS via proc_listchildpids.
    2. SIGHUP the leader's process group (-leader); child_pid == pgid because the shell ran setsid().
    3. SIGTERM every descendant by pid (reaches jobs in their own process groups).
    4. Poll up to a 2s grace, reaping the leader, and return early once nothing survives.
    5. SIGKILL remaining survivors.
  • pid-reuse guard: record each process's start time at snapshot (Linux /proc/<pid>/stat field 22; macOS proc_pidinfo/PROC_PIDTBSDINFO). Re-verify before every bare-pid signal so a delayed SIGKILL never hits a pid that was recycled. Zombies are treated as dead.
  • Bounded work (max_procs) so a fork bomb cannot make teardown scan without limit.
  • macOS struct layout locked with comptime size and offset asserts (matching src/cwd.zig), so drift fails the build loudly. Verified via cross-compile to both macOS arches.

Ordering change in the quit handler: notify clients and drainOutbound() before terminateTree, so the kill stays responsive while teardown escalates.

Test plan

  • zig build test -> 166/166 (adds reap.zig parseStat unit tests).
  • zig build test-integration -> 82/82 (adds kill tears down the whole process tree, not just the session shell).
  • zig build test-all -Doptimize=ReleaseSafe (CI release-safe gate) passes.
  • zig build -Dtarget=aarch64-macos and -Dtarget=x86_64-macos compile (darwin path + comptime layout asserts).
  • Confirmed the new test fails on main and passes on this branch.

Disclosure: this pull request was created by Coder Agents on behalf of @kylecarbs.

kylecarbs added 2 commits July 5, 2026 18:10
boo kill signaled only the session shell (a single SIGHUP to
child_pid), so grandchildren that ignore SIGHUP, were placed in their
own process group by the shell's job control, or were backgrounded
survived the session and kept holding ports and files. A replacement
session then failed with errors like "Address already in use".

Snapshot the shell's descendant tree while it is still intact, hang up
its process group, SIGTERM every descendant by pid (reaching jobs the
shell put in their own process groups), then SIGKILL any survivors
after a bounded grace. A per-process start time recorded at snapshot
guards each bare-pid signal against pid reuse. Enumeration is /proc on
Linux and libproc on macOS; a descendant that already double-forked
into its own session and reparented to init before the kill is still
out of reach, the same caveat tmux and screen carry.

Fixes #97.
proc_listallpids/proc_listchildpids return the number of pids, not a
byte count, so dividing the result by sizeof(pid) rounded a small tree
down to zero descendants and the teardown found nothing on macOS.

Enumerate the whole process table with proc_listallpids plus
proc_pidinfo (which yields ppid and start time together) and rebuild the
descendant tree with the same parent-link fixpoint used on Linux. Adds a
buildTree unit test covering transitive descendants and a self-loop.
@kylecarbs kylecarbs merged commit 5ce65d7 into main Jul 5, 2026
5 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.

boo kill leaves orphaned grandchildren alive (signals shell only, not process group)

1 participant