a73x

3b47d5d5

refactor: the daemon socket binds through serve

a73x   2026-08-31 21:58

Commit message
refactor: the daemon socket binds through serve

`Server.listener: std.net.Server` and `Server.path_id` become one
`bound: serve.Bound`. The descriptor had two homes before — a std.net.Server
whose only uses were `.stream.handle` and `accept` — and one owner is the
point: `Bound.close` is now the only thing that closes it, and the guarded
unlink in deinit is the shared one.

`initFromManifest` takes `serve.adopt`: no bind, no listen, no claim. That is
not decoration — with `serve.bind(.refuse_live)` there instead, four
server_test_upgrade cases fail with `error.DaemonAlreadyRunning`, the claim
probe finding the daemon's own inherited listener answering.

`init` keeps its early `sockpath.claim` before the fork and lets bind refuse
again at the bind; the repeat is the one that decides.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TWxBL1HBULH1ZwTNzzKTja
build.zig
Old New
@@ -196,7 +196,7 @@ const mod_table = [_]ModSpec{
196 // daemon itself when nobody handed it a --key — and for the shim 196 // daemon itself when nobody handed it a --key — and for the shim
197 // directory shell integration writes under the same 0700 policy. 197 // directory shell integration writes under the same 0700 policy.
198 // `pty` stays a row of its own: the ptyclient fixture consumes it. 198 // `pty` stays a row of its own: the ptyclient fixture consumes it.
199 .{ .name = "daemon", .path = "src/server/server.zig", .link_libc = true, .imports = &.{ "term", "pty", "sockpath", "quic", "xdg", "proxy" }, .test_imports = &.{ "testtmp", "dial" }, .quic_tests = true }, 199 .{ .name = "daemon", .path = "src/server/server.zig", .link_libc = true, .imports = &.{ "term", "pty", "sockpath", "serve", "quic", "xdg", "proxy" }, .test_imports = &.{ "testtmp", "dial" }, .quic_tests = true },
200 // The agent-facing client. It speaks frames and owns no terminal, which 200 // The agent-facing client. It speaks frames and owns no terminal, which
201 // is the whole point — it attaches at 0x0 and never claims the grid. 201 // is the whole point — it attaches at 0x0 and never claims the grid.
202 // The transport modules are the CLI client's, minus everything that 202 // The transport modules are the CLI client's, minus everything that
src/server/server.zig
Old New
@@ -12,6 +12,7 @@ const DeltaTracker = @import("term").delta.DeltaTracker;
12 const cmdmod = @import("cmd.zig"); 12 const cmdmod = @import("cmd.zig");
13 const shellint = @import("shellint.zig"); 13 const shellint = @import("shellint.zig");
14 const sockpath = @import("sockpath"); 14 const sockpath = @import("sockpath");
15 const serve = @import("serve");
15 const quic = @import("quic"); 16 const quic = @import("quic");
16 const xdg = @import("xdg"); 17 const xdg = @import("xdg");
17 // Re-exported for the daemon's own main (src/cli/main.zig) — the only 18 // Re-exported for the daemon's own main (src/cli/main.zig) — the only
@@ -421,13 +422,14 @@ pub const Server = struct {
421 spawn_shell: [:0]const u8, 422 spawn_shell: [:0]const u8,
422 spawn_shell_integration: bool, 423 spawn_shell_integration: bool,
423 spawn_extra_env: []const Pty.EnvPair, 424 spawn_extra_env: []const Pty.EnvPair,
424 listener: std.net.Server, 425 /// The listening socket and what its file was when we bound it, so
426 /// teardown can tell our socket from one that replaced it. One field
427 /// rather than a descriptor beside a std.net.Server holding the same
428 /// number: the fd has one owner, and `serve.Bound.close` is the only
429 /// thing that closes it. Not optional — init cannot return without one,
430 /// and an absent-means-false arm is the silent no-unlink 6090604 fixed.
431 bound: serve.Bound,
425 sock_path: []const u8, 432 sock_path: []const u8,
426 /// What the socket file was when we bound it, so teardown can tell our
427 /// socket from one that replaced it. See sockpath.PathId. Not optional:
428 /// init cannot return without one, and an absent-means-false arm is the
429 /// silent no-unlink 6090604 fixed.
430 path_id: sockpath.PathId,
431 /// The attached interactive clients, across every session; each sees 433 /// The attached interactive clients, across every session; each sees
432 /// every update of the one session it is attached to. 434 /// every update of the one session it is attached to.
433 clients: [max_clients]?ClientSlot = @splat(null), 435 clients: [max_clients]?ClientSlot = @splat(null),
@@ -501,7 +503,10 @@ pub const Server = struct {
501 503
502 pub fn init(alloc: std.mem.Allocator, opts: Options) !Server { 504 pub fn init(alloc: std.mem.Allocator, opts: Options) !Server {
503 // Before the shell is spawned, so refusing costs nobody a fork and 505 // Before the shell is spawned, so refusing costs nobody a fork and
504 // leaves no process to reap. 506 // leaves no process to reap. `serve.bind` below runs the same refusal
507 // again, and the repeat is not redundant: it is the one that decides,
508 // covering the window this early check opens by refusing before the
509 // fork rather than at the bind.
505 try sockpath.claim(opts.sock_path); 510 try sockpath.claim(opts.sock_path);
506 511
507 // Shell integration, decided and written before the fork: whatever 512 // Shell integration, decided and written before the fork: whatever
@@ -534,9 +539,7 @@ pub const Server = struct {
534 s0.eng.deinit(); 539 s0.eng.deinit();
535 } 540 }
536 541
537 const addr = try std.net.Address.initUnix(opts.sock_path); 542 const bound = try serve.bind(opts.sock_path, .{ .policy = .refuse_live });
538 const listener = try addr.listen(.{});
539 const path_id = try sockpath.PathId.of(opts.sock_path);
540 var srv: Server = .{ 543 var srv: Server = .{
541 .alloc = alloc, 544 .alloc = alloc,
542 .spawn_plan = plan, 545 .spawn_plan = plan,
@@ -544,9 +547,8 @@ pub const Server = struct {
544 .spawn_shell_integration = opts.shell_integration, 547 .spawn_shell_integration = opts.shell_integration,
545 .spawn_extra_env = opts.extra_env, 548 .spawn_extra_env = opts.extra_env,
546 .version = opts.version, 549 .version = opts.version,
547 .listener = listener, 550 .bound = bound,
548 .sock_path = opts.sock_path, 551 .sock_path = opts.sock_path,
549 .path_id = path_id,
550 .shellint_arena = shellint_arena, 552 .shellint_arena = shellint_arena,
551 .shellint_dir = plan.shellint_dir, 553 .shellint_dir = plan.shellint_dir,
552 .agents = .{ .dir = agent_dir }, 554 .agents = .{ .dir = agent_dir },
@@ -617,15 +619,12 @@ pub const Server = struct {
617 .spawn_shell_integration = d.shell_integration, 619 .spawn_shell_integration = d.shell_integration,
618 .spawn_extra_env = extra_env, 620 .spawn_extra_env = extra_env,
619 .version = version, 621 .version = version,
620 // The fd is the listener: no bind, no listen, no claim. The 622 // The fd is the listener: no bind, no listen, no claim. A claim
621 // address is rebuilt from the path only because std.net.Server 623 // here would probe the path, find our own inherited listener
622 // carries one; nothing reads it back. 624 // answering, and refuse the daemon its own socket. `adopt` only
623 .listener = .{ 625 // re-stamps the id from the file as found.
624 .listen_address = try std.net.Address.initUnix(sock_path), 626 .bound = try serve.adopt(d.listener_fd, sock_path),
625 .stream = .{ .handle = d.listener_fd },
626 },
627 .sock_path = sock_path, 627 .sock_path = sock_path,
628 .path_id = try sockpath.PathId.of(sock_path),
629 .shellint_arena = shellint_arena, 628 .shellint_arena = shellint_arena,
630 .shellint_dir = injection.dir, 629 .shellint_dir = injection.dir,
631 .agents = .{ .dir = agent_dir }, 630 .agents = .{ .dir = agent_dir },
@@ -807,15 +806,9 @@ pub const Server = struct {
807 .borrowed => {}, 806 .borrowed => {},
808 .none => {}, 807 .none => {},
809 } 808 }
810 // Unlink only if the path still names *our* socket: a newer daemon 809 // Close, then unlink only if the path still names *our* socket — the
811 // may have replaced the file, and deleting that one would steal its 810 // guard and its rationale live in serve.Bound.close now.
812 // clients. The stat comes AFTER the close, because a successor only 811 self.bound.close(self.sock_path);
813 // claims once nothing is listening — that narrows the race to the
814 // stat→unlink gap, which is the floor Linux gives for deleting by name.
815 self.listener.deinit();
816 if (self.path_id.stillAt(self.sock_path)) {
817 std.fs.cwd().deleteFile(self.sock_path) catch {};
818 }
819 // Two passes over one deadline: every child is asked to go before any 812 // Two passes over one deadline: every child is asked to go before any
820 // is waited on, so a table of shells that ignore TERM costs one 813 // is waited on, so a table of shells that ignore TERM costs one
821 // `term_grace_ms` and not one each. 814 // `term_grace_ms` and not one each.
@@ -871,7 +864,7 @@ pub const Server = struct {
871 for (&self.sessions.table, 0..) |*slot, si| { 864 for (&self.sessions.table, 0..) |*slot, si| {
872 fds[si] = pollIn(if (slot.*) |*s| s.pty.master else -1); 865 fds[si] = pollIn(if (slot.*) |*s| s.pty.master else -1);
873 } 866 }
874 fds[listener_idx] = pollIn(self.listener.stream.handle); 867 fds[listener_idx] = pollIn(self.bound.fd);
875 for (&self.clients, 0..) |*slot, i| { 868 for (&self.clients, 0..) |*slot, i| {
876 if (slot.*) |*c| { 869 if (slot.*) |*c| {
877 // POLLOUT only while something is owed: asking for it on an 870 // POLLOUT only while something is owed: asking for it on an
@@ -1033,21 +1026,27 @@ pub const Server = struct {
1033 } 1026 }
1034 1027
1035 fn acceptConn(self: *Server) void { 1028 fn acceptConn(self: *Server) void {
1036 const conn = self.listener.accept() catch return; 1029 // CLOEXEC on the accepted fd, which is what std.net.Server.accept
1030 // did here before: the daemon forks a shell per session and a live
1031 // client connection leaked into one outlives every close of ours.
1032 // Client connections are also the fds `mux d upgrade` DROPS, so
1033 // unlike the listener this one has no reason to cross an exec.
1034 const fd = std.posix.accept(self.bound.fd, null, null, std.posix.SOCK.CLOEXEC) catch return;
1035 const conn: std.net.Stream = .{ .handle = fd };
1037 // Nonblocking from the accept, and through the promotion into a 1036 // Nonblocking from the accept, and through the promotion into a
1038 // client slot: a blocking fd lets one peer that stops reading stop the 1037 // client slot: a blocking fd lets one peer that stops reading stop the
1039 // daemon's only loop. Every write here tolerates a short write. 1038 // daemon's only loop. Every write here tolerates a short write.
1040 setNonblocking(conn.stream.handle) catch { 1039 setNonblocking(conn.handle) catch {
1041 conn.stream.close(); 1040 conn.close();
1042 return; 1041 return;
1043 }; 1042 };
1044 for (&self.observers) |*slot| { 1043 for (&self.observers) |*slot| {
1045 if (slot.* == null) { 1044 if (slot.* == null) {
1046 slot.* = .{ .fd = conn.stream.handle, .since_ms = monoMs() }; 1045 slot.* = .{ .fd = conn.handle, .since_ms = monoMs() };
1047 return; 1046 return;
1048 } 1047 }
1049 } 1048 }
1050 conn.stream.close(); // out of slots 1049 conn.close(); // out of slots
1051 } 1050 }
1052 1051
1053 fn setNonblocking(fd: std.posix.fd_t) !void { 1052 fn setNonblocking(fd: std.posix.fd_t) !void {
@@ -2773,7 +2772,7 @@ pub const Server = struct {
2773 .writer_version = writer_version, 2772 .writer_version = writer_version,
2774 .writer_path = writer_path, 2773 .writer_path = writer_path,
2775 .sock_path = self.sock_path, 2774 .sock_path = self.sock_path,
2776 .listener_fd = self.listener.stream.handle, 2775 .listener_fd = self.bound.fd,
2777 .shellint_dir = self.shellint_dir, 2776 .shellint_dir = self.shellint_dir,
2778 .agent_dir = self.agents.dir, 2777 .agent_dir = self.agents.dir,
2779 .shell = self.spawn_shell, 2778 .shell = self.spawn_shell,
@@ -2952,7 +2951,7 @@ pub const Server = struct {
2952 /// The flag back on every fd `execUpgrade` cleared; the memfd it also 2951 /// The flag back on every fd `execUpgrade` cleared; the memfd it also
2953 /// cleared is the caller's to close. 2952 /// cleared is the caller's to close.
2954 pub fn sealAdoptedFds(self: *Server) void { 2953 pub fn sealAdoptedFds(self: *Server) void {
2955 sealFd(self.listener.stream.handle); 2954 sealFd(self.bound.fd);
2956 if (self.quicListener()) |q| sealFd(q.fd); 2955 if (self.quicListener()) |q| sealFd(q.fd);
2957 for (&self.sessions.table) |*slot| { 2956 for (&self.sessions.table) |*slot| {
2958 if (slot.*) |*s| { 2957 if (slot.*) |*s| {
@@ -2989,7 +2988,7 @@ pub const Server = struct {
2989 // failure that took an early exit. 2988 // failure that took an early exit.
2990 defer restoreCloexec(cleared.items); 2989 defer restoreCloexec(cleared.items);
2991 2990
2992 const listener_fd = self.listener.stream.handle; 2991 const listener_fd = self.bound.fd;
2993 clearCloexec(listener_fd) catch return; 2992 clearCloexec(listener_fd) catch return;
2994 cleared.append(a, listener_fd) catch return; 2993 cleared.append(a, listener_fd) catch return;
2995 2994
src/server/server_test_agent.zig
Old New
@@ -575,7 +575,7 @@ test "Server: a client closing a channel hangs up the agent connection without a
575 try std.testing.expect(echo == null); 575 try std.testing.expect(echo == null);
576 } 576 }
577 577
578 test "Server: an agent listener and the connections it accepts are close-on-exec" { 578 test "Server: the daemon listener, the clients it accepts, an agent listener and its channels are close-on-exec" {
579 const alloc = std.testing.allocator; 579 const alloc = std.testing.allocator;
580 580
581 var td = try h.TestDaemon.init(alloc, "agentcloexec", .{ .shell = "/bin/cat" }); 581 var td = try h.TestDaemon.init(alloc, "agentcloexec", .{ .shell = "/bin/cat" });
@@ -603,6 +603,19 @@ test "Server: an agent listener and the connections it accepts are close-on-exec
603 ); 603 );
604 const ch = td.srv.agents.chans[0] orelse return error.NoChannel; 604 const ch = td.srv.agents.chans[0] orelse return error.NoChannel;
605 try std.testing.expect(try std.posix.fcntl(ch.fd, std.posix.F.GETFD, 0) & flags != 0); 605 try std.testing.expect(try std.posix.fcntl(ch.fd, std.posix.F.GETFD, 0) & flags != 0);
606
607 // The daemon's own listener and the client connection it accepted, on
608 // the same grounds and unpinned until now: `serve.bind` and the raw
609 // `posix.accept` that replaced `std.net` both have to be ASKED for this
610 // flag, where std.net set it unconditionally, so a conversion can drop
611 // it and stay green everywhere else.
612 try std.testing.expect(try std.posix.fcntl(td.srv.bound.fd, std.posix.F.GETFD, 0) & flags != 0);
613 const seated = td.srv.clients[0] orelse return error.ClientNeverSeated;
614 const seated_fd = switch (seated.sink) {
615 .socket => |fd| fd,
616 .quic => return error.NotAUnixClient,
617 };
618 try std.testing.expect(try std.posix.fcntl(seated_fd, std.posix.F.GETFD, 0) & flags != 0);
606 } 619 }
607 620
608 test "Server: a client's agent channels die with the client" { 621 test "Server: a client's agent channels die with the client" {
src/server/server_test_upgrade.zig
Old New
@@ -315,7 +315,7 @@ test "sealAdoptedFds: the adopted fds are CLOEXEC again, and not one step before
315 315
316 // What execUpgrade does on the way out: the flag is cleared so the fds 316 // What execUpgrade does on the way out: the flag is cleared so the fds
317 // cross the exec. The adopting side must put it back. 317 // cross the exec. The adopting side must put it back.
318 try Server.clearCloexec(srv.listener.stream.handle); 318 try Server.clearCloexec(srv.bound.fd);
319 try Server.clearCloexec(srv.sessions.table[0].?.pty.master); 319 try Server.clearCloexec(srv.sessions.table[0].?.pty.master);
320 if (srv.sessions.table[0].?.agent_listener != -1) 320 if (srv.sessions.table[0].?.agent_listener != -1)
321 try Server.clearCloexec(srv.sessions.table[0].?.agent_listener); 321 try Server.clearCloexec(srv.sessions.table[0].?.agent_listener);
@@ -348,13 +348,13 @@ test "sealAdoptedFds: the adopted fds are CLOEXEC again, and not one step before
348 // these very descriptors, and a flag set here closes them at that exec. 348 // these very descriptors, and a flag set here closes them at that exec.
349 // Flagged in initFromManifest, this test was green and the rolled-back 349 // Flagged in initFromManifest, this test was green and the rolled-back
350 // daemon panicked adopting a manifest naming fds it no longer had. 350 // daemon panicked adopting a manifest naming fds it no longer had.
351 try std.testing.expect(!try hasCloexec(srv2.listener.stream.handle)); 351 try std.testing.expect(!try hasCloexec(srv2.bound.fd));
352 try std.testing.expect(!try hasCloexec(s2.pty.master)); 352 try std.testing.expect(!try hasCloexec(s2.pty.master));
353 if (s2.agent_listener != -1) try std.testing.expect(!try hasCloexec(s2.agent_listener)); 353 if (s2.agent_listener != -1) try std.testing.expect(!try hasCloexec(s2.agent_listener));
354 354
355 srv2.sealAdoptedFds(); 355 srv2.sealAdoptedFds();
356 356
357 try std.testing.expect(try hasCloexec(srv2.listener.stream.handle)); 357 try std.testing.expect(try hasCloexec(srv2.bound.fd));
358 try std.testing.expect(try hasCloexec(s2.pty.master)); 358 try std.testing.expect(try hasCloexec(s2.pty.master));
359 if (s2.agent_listener != -1) try std.testing.expect(try hasCloexec(s2.agent_listener)); 359 if (s2.agent_listener != -1) try std.testing.expect(try hasCloexec(s2.agent_listener));
360 } 360 }