a73x

0aed35be

fix: a successor reaps the pid-named leftovers a dead daemon or wall stranded

a73x   2026-09-02 09:46

Commit message
fix: a successor reaps the pid-named leftovers a dead daemon or wall stranded

The agent directory, the shim directory and the prompt socket are each
unlinked by their owner on the normal way out, and a SIGKILL or a closed
terminal window runs nothing: 31 agent directories and five prompt
sockets from dead pids were sitting on the dev box. No signal handler —
none runs on SIGKILL — but xdg.reapDeadPid, called by each creator first,
removes every entry under that prefix whose pid /proc no longer has, and
leaves a live pid alone even when it is no longer a mux.

Pinned per site by a unit test against a real dead pid, and by the boot
group: the SIGKILLed daemon's agent directory is there before the restart
and gone after it (108 scenarios now).
CLAUDE.md
Old New
@@ -187,6 +187,13 @@ own. Test fixtures in `test/`:
187 tile locally, because a pump parked in `dial` reads no ask, a pending pane 187 tile locally, because a pump parked in `dial` reads no ask, a pending pane
188 has no pump at all, and birthing onto an `unreachable` row is a designed 188 has no pump at all, and birthing onto an `unreachable` row is a designed
189 path. 189 path.
190 - **A pid-named leftover is reaped by its successor, never by a signal
191 handler.** The daemon's `mux-agent-PID-*` and `mux-shellint-PID-*`
192 directories and a wall's `mux-ask-PID.sock` are unlinked by their owner
193 on the normal way out; a SIGKILL or a closed terminal window runs
194 nothing, so the next daemon or wall to create one in that directory
195 calls `xdg.reapDeadPid` first and removes every entry whose pid `/proc`
196 no longer has. A live pid's entry stays even when it is no longer a mux.
190 - **A daemon lives until `mux d stop`; emptiness is not an exit.** `x` ends a 197 - **A daemon lives until `mux d stop`; emptiness is not an exit.** `x` ends a
191 session, never a box: `reap` and `pumpOnce` answer nothing, a shell's code 198 session, never a box: `reap` and `pumpOnce` answer nothing, a shell's code
192 reaches that shell's own clients as `exit_status`, and an emptied daemon 199 reaches that shell's own clients as `exit_status`, and an emptied daemon
docs/decisions.md
Old New
@@ -7820,3 +7820,17 @@ today's 1,875 stayed. And 31 `mux-agent-<pid>-<hash>` dirs from dead
7820 daemons, four of them left by the green ci run itself, so the leak in 7820 daemons, four of them left by the green ci run itself, so the leak in
7821 collab 68dc4c90 is reached by the suite's own legs and the leak sweep does 7821 collab 68dc4c90 is reached by the suite's own legs and the leak sweep does
7822 not count it; noted there, removed here. 7822 not count it; noted there, removed here.
7823
7824 **Both leaks fixed the same way, later the same day.** No signal handler:
7825 the unlink half of `retire` is async-signal-safe but the mutex half is
7826 not, and no handler runs on SIGKILL anyway. Instead `xdg.reapDeadPid`
7827 removes, from one directory, every entry named `<prefix><pid>…` whose pid
7828 `/proc` no longer has, and the three creators call it first — the agent
7829 directory, the shim directory, the prompt socket — so the successor that
7830 would otherwise sit beside the leftover is the one that asks the OS about
7831 its owner. A live pid's entry stays even when that pid is no longer a mux.
7832 Pinned by a unit test per site against a real dead pid (a `/bin/true`
7833 spawned and waited for — read its pid BEFORE the wait, `Child.wait` sets
7834 `id` to undefined, which cost one red round) and by the boot group's
7835 restart leg, which now asserts the SIGKILLed daemon's agent directory is
7836 there before the restart and gone after it.
src/client/askpass.zig
Old New
@@ -11,6 +11,7 @@ const std = @import("std");
11 // `serve_mod` and not `serve`: Listener has its own `serve` method, and 11 // `serve_mod` and not `serve`: Listener has its own `serve` method, and
12 // inside the struct the bare name is ambiguous. 12 // inside the struct the bare name is ambiguous.
13 const serve_mod = @import("serve"); 13 const serve_mod = @import("serve");
14 const xdg = @import("xdg");
14 15
15 /// Env var naming the socket. The mode word for the helper, too: ssh execs 16 /// Env var naming the socket. The mode word for the helper, too: ssh execs
16 /// its helper with the prompt as argv[1] and nothing else, so there is no 17 /// its helper with the prompt as argv[1] and nothing else, so there is no
@@ -154,6 +155,10 @@ pub const Listener = struct {
154 .{ runtime_dir, std.os.linux.getpid() }, 155 .{ runtime_dir, std.os.linux.getpid() },
155 ); 156 );
156 errdefer alloc.free(path); 157 errdefer alloc.free(path);
158 // Every wall that died by signal — a closed terminal window, a kill
159 // — left its socket here, and `retire` never ran for it. This wall
160 // is the first since to look, and asks the OS which owners are gone.
161 xdg.reapDeadPid(runtime_dir, "mux-ask-");
157 // `clobber_own`: a client that died without unlinking left a file, 162 // `clobber_own`: a client that died without unlinking left a file,
158 // and a pid comes round again. Nothing else may own this name — it 163 // and a pid comes round again. Nothing else may own this name — it
159 // has our pid in it. CLOEXEC because this process spawns the ssh 164 // has our pid in it. CLOEXEC because this process spawns the ssh
@@ -1031,3 +1036,19 @@ test "askpass.Listener: retire takes only its own socket, never a successor's at
1031 test { 1036 test {
1032 std.testing.refAllDeclsRecursive(@This()); 1037 std.testing.refAllDeclsRecursive(@This());
1033 } 1038 }
1039
1040 test "askpass.Listener: start reaps the socket a wall that died by signal left, not a live wall's" {
1041 var tmp = try testtmp.TmpDir.make();
1042 defer tmp.cleanup();
1043 const dead = try testtmp.deadPid();
1044 var nb: [48]u8 = undefined;
1045 const left = try std.fmt.bufPrint(&nb, "mux-ask-{d}.sock", .{dead});
1046 try tmp.dir.writeFile(.{ .sub_path = left, .data = "" });
1047 try tmp.dir.writeFile(.{ .sub_path = "mux-ask-1.sock", .data = "" });
1048
1049 var c = Counter{};
1050 const l = try Listener.start(std.testing.allocator, tmp.path(), c.hooks());
1051 defer l.stop();
1052 try std.testing.expectError(error.FileNotFound, tmp.dir.access(left, .{}));
1053 try tmp.dir.access("mux-ask-1.sock", .{});
1054 }
src/server/server_agent.zig
Old New
@@ -125,6 +125,9 @@ pub const AgentRelay = struct {
125 /// from inside the shell an absent `SSH_AUTH_SOCK` looks like no `-A`. 125 /// from inside the shell an absent `SSH_AUTH_SOCK` looks like no `-A`.
126 pub fn makeDir(alloc: std.mem.Allocator, sock_path: []const u8) ?[]const u8 { 126 pub fn makeDir(alloc: std.mem.Allocator, sock_path: []const u8) ?[]const u8 {
127 const parent = std.fs.path.dirname(sock_path) orelse "."; 127 const parent = std.fs.path.dirname(sock_path) orelse ".";
128 // A SIGKILLed predecessor left its directory here with nothing
129 // running to remove it; this daemon is the first since to look.
130 xdg.reapDeadPid(parent, "mux-agent-");
128 const dir = std.fmt.allocPrint( 131 const dir = std.fmt.allocPrint(
129 alloc, 132 alloc,
130 "{s}/mux-agent-{d}-{x:0>12}", 133 "{s}/mux-agent-{d}-{x:0>12}",
src/server/server_test_agent.zig
Old New
@@ -917,3 +917,22 @@ test "Server: ending one session unlinks ITS agent socket and leaves every other
917 try std.testing.expect(!isSocketAt(dp)); 917 try std.testing.expect(!isSocketAt(dp));
918 try std.testing.expect(isSocketAt(kp)); 918 try std.testing.expect(isSocketAt(kp));
919 } 919 }
920
921 test "AgentRelay.makeDir reaps a dead daemon's agent directory beside the socket, not a live one's" {
922 const testtmp = @import("testtmp");
923 var tmp = try testtmp.TmpDir.make();
924 defer tmp.cleanup();
925 const dead = try testtmp.deadPid();
926 var nb: [48]u8 = undefined;
927 const left = try std.fmt.bufPrint(&nb, "mux-agent-{d}-000000000000", .{dead});
928 try tmp.dir.makePath(left);
929 try tmp.dir.makePath("mux-agent-1-000000000000");
930 var sb: [64]u8 = undefined;
931 const sock = try std.fmt.bufPrint(&sb, "{s}/muxd.sock", .{tmp.path()});
932
933 const dir = @import("server_agent.zig").AgentRelay.makeDir(std.testing.allocator, sock) orelse
934 return error.TestUnexpectedResult;
935 defer std.testing.allocator.free(dir);
936 try std.testing.expectError(error.FileNotFound, tmp.dir.access(left, .{}));
937 try tmp.dir.access("mux-agent-1-000000000000", .{});
938 }
src/server/shellint.zig
Old New
@@ -156,6 +156,9 @@ pub fn install(
156 parent_dir: []const u8, 156 parent_dir: []const u8,
157 shell_path: []const u8, 157 shell_path: []const u8,
158 ) Injection { 158 ) Injection {
159 // What a SIGKILLed predecessor left under its own pid, reaped by the
160 // first daemon since to make one of these here.
161 xdg.reapDeadPid(parent_dir, "mux-shellint-");
159 // The pid keeps two daemons sharing one runtime directory legible in a 162 // The pid keeps two daemons sharing one runtime directory legible in a
160 // listing; the random half is not decoration. `parent_dir` is the socket's 163 // listing; the random half is not decoration. `parent_dir` is the socket's
161 // directory, a shared `/tmp` when `$XDG_RUNTIME_DIR` is unset, and a pid is 164 // directory, a shared `/tmp` when `$XDG_RUNTIME_DIR` is unset, and a pid is
@@ -572,3 +575,21 @@ test "prepare zsh: the shim directory is 0700 and the rc file 0600" {
572 test { 575 test {
573 std.testing.refAllDeclsRecursive(@This()); 576 std.testing.refAllDeclsRecursive(@This());
574 } 577 }
578
579 test "install reaps a dead daemon's shim directory and leaves a live daemon's" {
580 var arena = std.heap.ArenaAllocator.init(std.testing.allocator);
581 defer arena.deinit();
582 var t = try TmpPath.make();
583 defer t.deinit();
584 const testtmp = @import("testtmp");
585 const dead = try testtmp.deadPid();
586 const left = try std.fmt.allocPrint(arena.allocator(), "{s}/mux-shellint-{d}-000000000000", .{ t.dir, dead });
587 try std.fs.cwd().makePath(left);
588 const live = try std.fmt.allocPrint(arena.allocator(), "{s}/mux-shellint-1-000000000000", .{t.dir});
589 try std.fs.cwd().makePath(live);
590
591 const inj = install(arena.allocator(), t.dir, "/bin/bash");
592 try std.testing.expect(inj.dir != null);
593 try std.testing.expectError(error.FileNotFound, std.fs.cwd().access(left, .{}));
594 try std.fs.cwd().access(live, .{});
595 }
src/testtmp.zig
Old New
@@ -56,6 +56,18 @@ pub const TmpDir = struct {
56 } 56 }
57 }; 57 };
58 58
59 /// A pid nothing holds: `/bin/true`, spawned and waited for. What the
60 /// reapers ask the OS about, made real rather than guessed from pid_max.
61 pub fn deadPid() !std.posix.pid_t {
62 var child = std.process.Child.init(&.{"/bin/true"}, std.testing.allocator);
63 try child.spawn();
64 // Read before the wait: `wait` sets `id` to undefined once the child
65 // is reaped, and an undefined pid parsed out of a name is no pid.
66 const pid = child.id;
67 _ = try child.wait();
68 return pid;
69 }
70
59 test "TmpDir: a path short enough to bind a socket in" { 71 test "TmpDir: a path short enough to bind a socket in" {
60 var tmp = try TmpDir.make(); 72 var tmp = try TmpDir.make();
61 defer tmp.cleanup(); 73 defer tmp.cleanup();
src/xdg.zig
Old New
@@ -1,4 +1,5 @@
1 //! XDG-derived paths shared by every mode, plus key file creation. The `*From` 1 //! XDG-derived paths shared by every mode, key file creation, and the reaping
2 //! of what a dead process left under a pid-named entry. The `*From`
2 //! variants are PURE — environment handed in, nothing read — because that is 3 //! variants are PURE — environment handed in, nothing read — because that is
3 //! what makes them testable without setenv, which Zig tests cannot safely do. 4 //! what makes them testable without setenv, which Zig tests cannot safely do.
4 //! The un-suffixed wrappers are one line each. 5 //! The un-suffixed wrappers are one line each.
@@ -153,6 +154,37 @@ pub fn makeNewPrivateDir(dir: []const u8) !void {
153 try d.chmod(0o700); 154 try d.chmod(0o700);
154 } 155 }
155 156
157 /// Removes every entry of `parent` named `<prefix><pid>…` whose pid no
158 /// process holds. Owners unlink these on the way out — a daemon its agent
159 /// and shim directories, a wall its prompt socket — and a SIGKILL or a
160 /// closed terminal window is a way out that runs nothing, so the successor
161 /// creating the next such entry is what asks the OS whether each owner is
162 /// still there. A live pid is left alone even when it is no longer a mux:
163 /// deleting under a stranger is worse than one stale name. Best effort
164 /// throughout — a parent that cannot be read reaps nothing.
165 pub fn reapDeadPid(parent: []const u8, prefix: []const u8) void {
166 var d = std.fs.cwd().openDir(parent, .{ .iterate = true }) catch return;
167 defer d.close();
168 var it = d.iterate();
169 while (it.next() catch null) |entry| {
170 if (!std.mem.startsWith(u8, entry.name, prefix)) continue;
171 const rest = entry.name[prefix.len..];
172 var n: usize = 0;
173 while (n < rest.len and std.ascii.isDigit(rest[n])) n += 1;
174 // At least one digit, and the run must end at the name's own
175 // separator: `mux-agent-abc` and `mux-agent-12x` are somebody
176 // else's files, not a pid we can ask about.
177 if (n == 0) continue;
178 if (n < rest.len and rest[n] != '-' and rest[n] != '.') continue;
179 const pid = std.fmt.parseInt(u32, rest[0..n], 10) catch continue;
180 var buf: [32]u8 = undefined;
181 const proc = std.fmt.bufPrint(&buf, "/proc/{d}", .{pid}) catch continue;
182 const alive = if (std.fs.cwd().access(proc, .{})) true else |_| false;
183 if (alive) continue;
184 d.deleteTree(entry.name) catch {};
185 }
186 }
187
156 /// The same, for callers holding the path of the FILE that is going to 188 /// The same, for callers holding the path of the FILE that is going to
157 /// live there. A `path` with no directory component is a no-op. 189 /// live there. A `path` with no directory component is a no-op.
158 pub fn makePrivateParent(path: []const u8) !void { 190 pub fn makePrivateParent(path: []const u8) !void {
@@ -368,3 +400,41 @@ test "writeNewKey: creates 0600 with 32 bytes, refuses to overwrite" {
368 test { 400 test {
369 std.testing.refAllDeclsRecursive(@This()); 401 std.testing.refAllDeclsRecursive(@This());
370 } 402 }
403
404 test "reapDeadPid: a dead owner's entry goes; a live owner's, a stranger's and another prefix's stay" {
405 const testtmp = @import("testtmp");
406 var tmp = try testtmp.TmpDir.make();
407 defer tmp.cleanup();
408 const dead = try testtmp.deadPid();
409 var b0: [48]u8 = undefined;
410 var b1: [48]u8 = undefined;
411 var b2: [48]u8 = undefined;
412 var b3: [48]u8 = undefined;
413 const dead_dir = try std.fmt.bufPrint(&b0, "mux-agent-{d}-abc", .{dead});
414 const dead_sock = try std.fmt.bufPrint(&b1, "mux-ask-{d}.sock", .{dead});
415 const ours = try std.fmt.bufPrint(&b2, "mux-agent-{d}-abc", .{std.os.linux.getpid()});
416 const not_a_pid = try std.fmt.bufPrint(&b3, "mux-agent-{d}x", .{dead});
417 // A directory with something in it, so a plain rmdir would not do.
418 try tmp.dir.makePath(dead_dir);
419 var inner: [64]u8 = undefined;
420 try tmp.dir.writeFile(.{ .sub_path = try std.fmt.bufPrint(&inner, "{s}/agent-0.sock", .{dead_dir}), .data = "" });
421 try tmp.dir.writeFile(.{ .sub_path = dead_sock, .data = "" });
422 try tmp.dir.makePath(ours);
423 try tmp.dir.makePath("mux-agent-1-abc"); // pid 1 is alive in every pid namespace
424 try tmp.dir.makePath("mux-agent-abc");
425 try tmp.dir.makePath(not_a_pid);
426
427 reapDeadPid(tmp.path(), "mux-agent-");
428 try std.testing.expectError(error.FileNotFound, tmp.dir.access(dead_dir, .{}));
429 try tmp.dir.access(dead_sock, .{}); // another prefix: not this reaper's
430 try tmp.dir.access(ours, .{});
431 try tmp.dir.access("mux-agent-1-abc", .{});
432 try tmp.dir.access("mux-agent-abc", .{});
433 try tmp.dir.access(not_a_pid, .{});
434
435 reapDeadPid(tmp.path(), "mux-ask-");
436 try std.testing.expectError(error.FileNotFound, tmp.dir.access(dead_sock, .{}));
437
438 // An unreadable parent reaps nothing and says nothing.
439 reapDeadPid("/nonexistent/parent", "mux-agent-");
440 }
test/e2e.sh
Old New
@@ -170,8 +170,8 @@ done
170 # one of those and adds a convergence point would be pinning a fact every 170 # one of those and adds a convergence point would be pinning a fact every
171 # leg above already establishes. 171 # leg above already establishes.
172 172
173 [ "$OK_COUNT" = "107" ] || { 173 [ "$OK_COUNT" = "108" ] || {
174 echo "e2e FAIL: $OK_COUNT scenario checkpoints ran, the pin says 107 —" 174 echo "e2e FAIL: $OK_COUNT scenario checkpoints ran, the pin says 108 —"
175 echo " a scenario was added (update the pin) or silently lost" 175 echo " a scenario was added (update the pin) or silently lost"
176 exit 1 176 exit 1
177 } 177 }
test/e2e_01_boot.sh
Old New
@@ -368,10 +368,23 @@ wait_for "$OUT.m7b" "m7b-pre-restart" 20 || {
368 # SIGKILL leaves the socket file behind; the new daemon's stale-socket 368 # SIGKILL leaves the socket file behind; the new daemon's stale-socket
369 # recovery (ECONNREFUSED + S_ISSOCK -> unlink) is what lets it rebind here. 369 # recovery (ECONNREFUSED + S_ISSOCK -> unlink) is what lets it rebind here.
370 PAINTS_BEFORE=$(repaints "$OUT.m7b") 370 PAINTS_BEFORE=$(repaints "$OUT.m7b")
371 D3DEAD=$D3PID
371 hardkill "$D3PID" 372 hardkill "$D3PID"
372 sleep 0.5 373 sleep 0.5
374 # The SIGKILL also strands the dead daemon's agent directory beside the
375 # socket — nothing of it runs to remove one — and the successor on the same
376 # socket directory is what reaps it (xdg.reapDeadPid). Asserted on the
377 # directory itself, by the dead pid in its name, before and after.
378 AGENTLEFT=$(find "$(dirname "$SOCK3")" -maxdepth 1 -name "mux-agent-$D3DEAD-*" | wc -l)
379 [ "$AGENTLEFT" -eq 1 ] || {
380 echo "e2e FAIL: a SIGKILLed daemon left $AGENTLEFT agent directories for pid $D3DEAD (want 1)"; exit 1; }
373 start_daemon "$SOCK3" "$OUT.d3b.d" "daemon did not rebind the stale socket" --shell /bin/sh 381 start_daemon "$SOCK3" "$OUT.d3b.d" "daemon did not rebind the stale socket" --shell /bin/sh
374 D3PID=$DPID 382 D3PID=$DPID
383 AGENTLEFT=$(find "$(dirname "$SOCK3")" -maxdepth 1 -name "mux-agent-$D3DEAD-*" | wc -l)
384 [ "$AGENTLEFT" -eq 0 ] || {
385 echo "e2e FAIL: the restarted daemon left its SIGKILLed predecessor's agent directory:"
386 find "$(dirname "$SOCK3")" -maxdepth 1 -name "mux-agent-$D3DEAD-*"; exit 1; }
387 ok "a restarted daemon reaps the agent directory its SIGKILLed predecessor stranded"
375 388
376 await_repaint "$OUT.m7b" "$PAINTS_BEFORE" "m7b client never resumed into the restarted daemon" 389 await_repaint "$OUT.m7b" "$PAINTS_BEFORE" "m7b client never resumed into the restarted daemon"
377 pipe_send 'printf "m7b-%%s\\n" post-restart\n' 390 pipe_send 'printf "m7b-%%s\\n" post-restart\n'