a73x

6c06c07b

fix: reclaim the fd of an unadoptable agent socket at the seal point

a73x   2026-08-31 22:18

Commit message
fix: reclaim the fd of an unadoptable agent socket at the seal point

A session whose agent socket file vanished between the manifest and the
exec loses forwarding rather than the daemon, but the descriptor the
manifest carried was left open forever: no session names it, so
sealAdoptedFds could not find it, and execUpgrade had already cleared
CLOEXEC on it — every shell the upgraded daemon forked inherited a
listening socket.

Record it on the Server instead, and close it at the seal point. Not at
the failed adopt: until main.zig is past its last rollback point, a
rollback exec hands every manifest fd back to the old binary, this one
included.

Also two comment debts: dial.ask now says that a mid-wait read errno
folds into the same "no answer" null as EOF, and link.awaitFrame that a
zero deadline returns null off an fd or pipe without polling once.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TWxBL1HBULH1ZwTNzzKTja
src/dial.zig
Old New
@@ -73,7 +73,12 @@ pub fn detach(fd: std.posix.fd_t) !void {
73 /// no other connection to that daemon and wants none after the answer. 73 /// no other connection to that daemon and wants none after the answer.
74 /// 74 ///
75 /// Null is "no answer": the deadline ran out, or the peer closed without 75 /// Null is "no answer": the deadline ran out, or the peer closed without
76 /// sending one. A null `deadline_ms` waits forever, which is what a caller 76 /// sending one. A read that fails mid-wait — ECONNRESET on a daemon that
77 /// died with the question in flight — folds into that same null, where the
78 /// hand-rolled loop this replaced propagated the errno. Deliberate: the
79 /// caller's report is "the daemon never answered" either way, and one
80 /// wording for one fact beats two that must be kept in step.
81 /// A null `deadline_ms` waits forever, which is what a caller
77 /// wants when a daemon that has stopped replying should be a visible hang 82 /// wants when a daemon that has stopped replying should be a visible hang
78 /// rather than a report of an absent socket. 83 /// rather than a report of an absent socket.
79 /// 84 ///
src/link.zig
Old New
@@ -190,7 +190,12 @@ pub const Link = union(enum) {
190 /// forever), error.Closed when the peer is gone — callers own the 190 /// forever), error.Closed when the peer is gone — callers own the
191 /// wording for both (dial.ask maps Closed to its "no answer" null; muxa 191 /// wording for both (dial.ask maps Closed to its "no answer" null; muxa
192 /// maps it to DaemonGone/ConnectionLost). Non-matching frames go to the 192 /// maps it to DaemonGone/ConnectionLost). Non-matching frames go to the
193 /// sink (see Sink for ownership). 193 /// sink (see Sink for ownership). A `deadline_ms` of 0, or a deadline
194 /// already spent by the time the loop is re-entered, returns null off an
195 /// fd or pipe arm without polling once — a caller that means "hand me
196 /// whatever has already arrived" must pass a nonzero deadline. QUIC is
197 /// the exception: its read is eager, so a frame already buffered from an
198 /// earlier pump still comes back.
194 pub fn awaitFrame( 199 pub fn awaitFrame(
195 self: *Link, 200 self: *Link,
196 alloc: std.mem.Allocator, 201 alloc: std.mem.Allocator,
src/server/server.zig
Old New
@@ -479,6 +479,17 @@ pub const Server = struct {
479 /// directory the per-session sockets are bound in. See server_agent.zig 479 /// directory the per-session sockets are bound in. See server_agent.zig
480 /// for why it is handed a `*Server` rather than holding one. 480 /// for why it is handed a `*Server` rather than holding one.
481 agents: AgentRelay = .{}, 481 agents: AgentRelay = .{},
482 /// Descriptors the manifest handed over that no session could adopt — an
483 /// agent listener whose socket file vanished mid-upgrade. Closing one
484 /// where the adopt fails would be wrong: until the last rollback point
485 /// is behind us, a rollback exec hands EVERY manifest fd back to the old
486 /// binary, which expects this one among them. So the close is deferred
487 /// to `sealAdoptedFds`. It cannot be skipped either — `execUpgrade`
488 /// cleared CLOEXEC on these fds, and no session names them any more, so
489 /// one left open is a listening socket inherited by every shell this
490 /// daemon forks for the rest of its life. At most one per session.
491 orphaned_fds: [max_sessions]std.posix.fd_t = @splat(-1),
492 orphaned_n: usize = 0,
482 stats: upgrade.Counters = .{}, 493 stats: upgrade.Counters = .{},
483 // Set by the `upgrade_req` handler, checked by the run loop: the reply 494 // Set by the `upgrade_req` handler, checked by the run loop: the reply
484 // must drain before the exec, and a mid-handler exec would strand the 495 // must drain before the exec, and a mid-handler exec would strand the
@@ -729,10 +740,15 @@ pub const Server = struct {
729 .{ SessionTable.safeName(rec.name), err }, 740 .{ SessionTable.safeName(rec.name), err },
730 ); 741 );
731 alloc.free(dup); 742 alloc.free(dup);
732 // The descriptor is NOT closed, for the reason the 743 // The descriptor is NOT closed here, for the reason the
733 // errdefer above states: nothing the manifest handed over 744 // errdefer above states: nothing the manifest handed over
734 // is released on this path, because a later failure hands 745 // is released on this path, because a later failure hands
735 // every one of them back to the old binary. 746 // every one of them back to the old binary. It is
747 // recorded instead — no session names it now, so nothing
748 // else could find it again — and `sealAdoptedFds` closes
749 // it once that hand-back window has passed.
750 srv.orphaned_fds[srv.orphaned_n] = rec.agent_fd;
751 srv.orphaned_n += 1;
736 } 752 }
737 } 753 }
738 const n = @min(rec.name.len, proto.session_name_max); 754 const n = @min(rec.name.len, proto.session_name_max);
@@ -2986,7 +3002,12 @@ pub const Server = struct {
2986 } 3002 }
2987 3003
2988 /// The flag back on every fd `execUpgrade` cleared; the memfd it also 3004 /// The flag back on every fd `execUpgrade` cleared; the memfd it also
2989 /// cleared is the caller's to close. 3005 /// cleared is the caller's to close. The caller runs this only once the
3006 /// last rollback point is behind it, which is why the fds no session
3007 /// adopted are CLOSED here rather than sealed: before this point they
3008 /// still belong to the binary a rollback would exec, and after it
3009 /// nothing will ever name them again. Clearing the count keeps a second
3010 /// call from double-closing.
2990 pub fn sealAdoptedFds(self: *Server) void { 3011 pub fn sealAdoptedFds(self: *Server) void {
2991 sealFd(self.bound.fd); 3012 sealFd(self.bound.fd);
2992 if (self.quicListener()) |q| sealFd(q.fd); 3013 if (self.quicListener()) |q| sealFd(q.fd);
@@ -2996,6 +3017,8 @@ pub const Server = struct {
2996 if (s.agentFd() != -1) sealFd(s.agentFd()); 3017 if (s.agentFd() != -1) sealFd(s.agentFd());
2997 } 3018 }
2998 } 3019 }
3020 for (self.orphaned_fds[0..self.orphaned_n]) |fd| std.posix.close(fd);
3021 self.orphaned_n = 0;
2999 } 3022 }
3000 3023
3001 // The exec itself. NOTHING from `Server.deinit` runs here: the process 3024 // The exec itself. NOTHING from `Server.deinit` runs here: the process
src/server/server_test_upgrade.zig
Old New
@@ -188,6 +188,9 @@ test "initFromManifest: a session whose agent socket file vanished loses forward
188 srv.sessions.table[0].?.agentPath() orelse return error.NoAgentSocket, 188 srv.sessions.table[0].?.agentPath() orelse return error.NoAgentSocket,
189 ); 189 );
190 defer alloc.free(agent_path); 190 defer alloc.free(agent_path);
191 // The descriptor the manifest will carry, held here because after a
192 // failed adoption nothing on either Server names it any more.
193 const agent_fd = srv.sessions.table[0].?.agentFd();
191 194
192 const memfd = try std.posix.memfd_create("mux-goneagent-test", 0); 195 const memfd = try std.posix.memfd_create("mux-goneagent-test", 0);
193 defer std.posix.close(memfd); 196 defer std.posix.close(memfd);
@@ -235,6 +238,21 @@ test "initFromManifest: a session whose agent socket file vanished loses forward
235 const reply = (try awaitFrame(alloc, &srv2, obs.handle, .status_reply, 400)) orelse 238 const reply = (try awaitFrame(alloc, &srv2, obs.handle, .status_reply, 400)) orelse
236 return error.NoStatusReply; 239 return error.NoStatusReply;
237 reply.deinit(alloc); 240 reply.deinit(alloc);
241
242 // The orphan. It is deliberately STILL OPEN at this point: until the
243 // last rollback point in main.zig is behind us, a rollback exec hands
244 // every manifest descriptor back to the old binary, this one included.
245 // std.posix.fcntl is unreachable on EBADF, so ask the syscall directly.
246 const still_open = std.posix.system.fcntl(agent_fd, std.posix.F.GETFD, @as(usize, 0));
247 try std.testing.expectEqual(std.posix.E.SUCCESS, std.posix.errno(still_open));
248
249 // Past that point main.zig calls this, and it must CLOSE the orphan
250 // rather than seal it: execUpgrade cleared CLOEXEC on the fd and no
251 // session names it now, so one left open is a listening socket
252 // inherited by every shell this daemon forks from here on.
253 srv2.sealAdoptedFds();
254 const closed = std.posix.system.fcntl(agent_fd, std.posix.F.GETFD, @as(usize, 0));
255 try std.testing.expectEqual(std.posix.E.BADF, std.posix.errno(closed));
238 } 256 }
239 257
240 test "initFromManifest: the return watermark is re-stamped, never carried across seq spaces" { 258 test "initFromManifest: the return watermark is re-stamped, never carried across seq spaces" {