a73x

36800478

fix: the client's agent probe sends through client_os, and the review's comment repairs

a73x   2026-09-03 15:10

Commit message
fix: the client's agent probe sends through client_os, and the review's comment repairs

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SakwJEwD9dXBoRP5kWbemW
CLAUDE.md
Old New
@@ -79,8 +79,9 @@ rather than a shell the product runs. Rule 6 is the one `posix.fork` site,
79 and it names its file in `except` rather than in the file. The three keep 79 and it names its file in `except` rather than in the file. The three keep
80 their numbers because the in-file `folder rule N exemption:` lines cite them 80 their numbers because the in-file `folder rule N exemption:` lines cite them
81 by number. Rule 7 is the platform ban: `std.os.linux`, `/proc`, `memfd`, 81 by number. Rule 7 is the platform ban: `std.os.linux`, `/proc`, `memfd`,
82 `close_range`, `exit_group`, the `SO_PEERCRED` socket option and the two 82 `close_range`, `exit_group`, the `SO_PEERCRED` socket option, the two
83 Linux-only pty ioctls may appear only under `src/os/`, comments included — 83 Linux-only pty ioctls and `MSG_NOSIGNAL` may appear only under `src/os/`,
84 comments included —
84 the roots `server_os.zig` and `client_os.zig` are the contract and their 85 the roots `server_os.zig` and `client_os.zig` are the contract and their
85 `_linux` children the spellings (spec 2026-09-03). Rule 4's three remaining 86 `_linux` children the spellings (spec 2026-09-03). Rule 4's three remaining
86 debts are the markers in `engine.zig`, `protocol.zig` and `keymap.zig`, each 87 debts are the markers in `engine.zig`, `protocol.zig` and `keymap.zig`, each
build.zig
Old New
@@ -437,7 +437,11 @@ const source_bans = [_]SourceBan{
437 // the leading T so they catch `std.posix.T.IOCGPTN` as well as 437 // the leading T so they catch `std.posix.T.IOCGPTN` as well as
438 // `TIOCGPTN` — `std.posix.T` exists, so that first spelling names no 438 // `TIOCGPTN` — `std.posix.T` exists, so that first spelling names no
439 // `std.os.linux` and would otherwise be a Linux-ism that passes. 439 // `std.os.linux` and would otherwise be a Linux-ism that passes.
440 .needles = &.{ "std.os.linux", "/proc", "memfd", "close_range", "exit_group", "so.peercred", "so_peercred", "iocsptlck", "iocgptn" }, 440 // `nosignal` catches `std.posix.MSG.NOSIGNAL`, a flag Linux and the
441 // BSDs spell differently and macOS does not have at all: a send that
442 // must not signal goes through `server_os.sendNoSigNoWait` or
443 // `client_os.sendNoSig`, whichever side is asking.
444 .needles = &.{ "std.os.linux", "/proc", "memfd", "close_range", "exit_group", "so.peercred", "so_peercred", "iocsptlck", "iocgptn", "nosignal" },
441 .why = "a call whose spelling differs by OS belongs in src/os/, behind a " ++ 445 .why = "a call whose spelling differs by OS belongs in src/os/, behind a " ++
442 "server_os or client_os operation whose doc names what it guarantees; " ++ 446 "server_os or client_os operation whose doc names what it guarantees; " ++
443 "everything else builds for every OS from the same line", 447 "everything else builds for every OS from the same line",
docs/decisions.md
Old New
@@ -8148,16 +8148,21 @@ or a pty, and an app that links the engine and a client must not either.
8148 **The gate is folder rule 7.** No production line under `src/`, 8148 **The gate is folder rule 7.** No production line under `src/`,
8149 `src/engine/`, `src/client/`, `src/tui/`, `src/server/` or `src/cli/` may 8149 `src/engine/`, `src/client/`, `src/tui/`, `src/server/` or `src/cli/` may
8150 spell `std.os.linux`, `/proc`, `memfd`, `close_range`, `exit_group`, 8150 spell `std.os.linux`, `/proc`, `memfd`, `close_range`, `exit_group`,
8151 `so.peercred`, `so_peercred`, `iocsptlck` or `iocgptn`. `src/os/` is absent 8151 `so.peercred`, `so_peercred`, `iocsptlck`, `iocgptn` or `nosignal`.
8152 from that folder list on purpose: its children may spell anything, and its 8152 `src/os/` is absent from that folder list on purpose: its children may
8153 roots have no reason to. Comments count, as they do for rule 4, because a 8153 spell anything, and its roots have no reason to. Comments count, as they
8154 comment naming a Linux mechanism is one that goes stale the day a second 8154 do for rule 4, because a comment naming a Linux mechanism is one that goes
8155 arm exists. Four needles are spelled to catch a name in both the form Zig 8155 stale the day a second arm exists. Four needles are spelled to catch a name in both the form Zig
8156 writes it and the form C and our own prose do — `so.peercred` and 8156 writes it and the form C and our own prose do — `so.peercred` and
8157 `so_peercred` for `std.posix.SO.PEERCRED` and `SO_PEERCRED`, where a bare 8157 `so_peercred` for `std.posix.SO.PEERCRED` and `SO_PEERCRED`, where a bare
8158 `peercred` would have banned `client_os.peerCred`, the very operation 8158 `peercred` would have banned `client_os.peerCred`, the very operation
8159 callers are supposed to reach for; `iocsptlck` and `iocgptn` drop the 8159 callers are supposed to reach for; `iocsptlck` and `iocgptn` drop the
8160 leading T so they catch `std.posix.T.IOCGPTN` as well as `TIOCGPTN`. 8160 leading T so they catch `std.posix.T.IOCGPTN` as well as `TIOCGPTN`.
8161 `nosignal` is the fifth and is there for a different reason: it catches
8162 `std.posix.MSG.NOSIGNAL`, a flag Linux and the BSDs spell differently and
8163 macOS does not have at all, so a send that must not signal goes through
8164 `server_os.sendNoSigNoWait` or `client_os.sendNoSig`, whichever side is
8165 asking.
8161 8166
8162 **Measured before the design, on a Linux host.** zig 0.15.2 cross-compiles 8167 **Measured before the design, on a Linux host.** zig 0.15.2 cross-compiles
8163 a libc program using `posix_openpt`, `kqueue`, `libproc` and `dyld` to 8168 a libc program using `posix_openpt`, `kqueue`, `libproc` and `dyld` to
src/cli/main.zig
Old New
@@ -855,10 +855,14 @@ fn remoteUpgradeCmd(alloc: std.mem.Allocator, host: []const u8, allow_same: bool
855 push.stdout_behavior = .Inherit; 855 push.stdout_behavior = .Inherit;
856 push.stderr_behavior = .Inherit; 856 push.stderr_behavior = .Inherit;
857 try push.spawn(); 857 try push.spawn();
858 // The RUNNING image rather than a saved path: the bytes streamed are 858 // The running image rather than a saved path: "push this binary" can
859 // what is executing here, the only thing "push this binary" can honestly 859 // only honestly mean the bytes executing here. How close the ask lands
860 // mean, even if the file it was started from has since been replaced. 860 // depends on the OS. On Linux `openSelfExe` opens the running INODE, so
861 // `openSelfExe` is the portable spelling of that ask. 861 // the bytes streamed are what is executing even after the file it was
862 // started from has been renamed over. On an OS that can only open by
863 // path, it streams whatever that path holds now — the same file in the
864 // ordinary case, and a replacement's bytes if something swapped the
865 // binary mid-push.
862 const streamed: bool = blk: { 866 const streamed: bool = blk: {
863 var img = std.fs.openSelfExe(.{}) catch break :blk false; 867 var img = std.fs.openSelfExe(.{}) catch break :blk false;
864 defer img.close(); 868 defer img.close();
src/cli/mux_main.zig
Old New
@@ -21,6 +21,7 @@ const wall = @import("wall");
21 const hosts = @import("client").hosts; 21 const hosts = @import("client").hosts;
22 const layoutfile = @import("client").layoutfile; 22 const layoutfile = @import("client").layoutfile;
23 const cliflags = @import("cliflags"); 23 const cliflags = @import("cliflags");
24 const client_os = @import("client_os");
24 const TmpDir = @import("testtmp").TmpDir; 25 const TmpDir = @import("testtmp").TmpDir;
25 26
26 /// Root help page for the binary. It lists the mode words first, then documents 27 /// Root help page for the binary. It lists the mode words first, then documents
@@ -140,9 +141,11 @@ fn agentReachable(path: []const u8) bool {
140 const fd = client.connectAgent(path) orelse return false; 141 const fd = client.connectAgent(path) orelse return false;
141 defer std.posix.close(fd); 142 defer std.posix.close(fd);
142 143
143 // Suppress SIGPIPE because this probe runs before the client installs signal 144 // Through `client_os.sendNoSig` because this probe runs before the client
144 // handling and the peer may already have closed the socket. 145 // installs signal handling and the peer may already have closed the socket;
145 _ = std.posix.send(fd, &agent_request_identities, std.posix.MSG.NOSIGNAL) catch return false; 146 // the operation's contract is that a closed peer comes back as an error
147 // rather than as a signal.
148 _ = client_os.sendNoSig(fd, &agent_request_identities) catch return false;
146 149
147 var pfd = [_]std.posix.pollfd{.{ .fd = fd, .events = std.posix.POLL.IN, .revents = 0 }}; 150 var pfd = [_]std.posix.pollfd{.{ .fd = fd, .events = std.posix.POLL.IN, .revents = 0 }};
148 const ready = std.posix.poll(&pfd, agent_probe_ms) catch return true; 151 const ready = std.posix.poll(&pfd, agent_probe_ms) catch return true;
src/os/client_os.zig
Old New
@@ -21,11 +21,34 @@ pub fn getpid() std.posix.pid_t {
21 /// private, the uid here is what stops another local user raising a prompt 21 /// private, the uid here is what stops another local user raising a prompt
22 /// and reading the answer, and the pid is what attributes a prompt to the 22 /// and reading the answer, and the pid is what attributes a prompt to the
23 /// ssh THIS wall spawned. 23 /// ssh THIS wall spawned.
24 ///
25 /// Unlike `server_os.peerCred`, a pid of 0 — what a kernel reports for a
26 /// peer it cannot name — passes through this root unjudged, because the
27 /// consumer already has the rule: `askpass.dialOwner` walks up from the
28 /// peer under `at > 0`, so a 0 ends the walk without matching anything.
29 /// Rejecting it here as well would be a second copy of one rule.
24 pub const PeerCred = struct { uid: std.posix.uid_t, pid: std.posix.pid_t }; 30 pub const PeerCred = struct { uid: std.posix.uid_t, pid: std.posix.pid_t };
25 pub fn peerCred(fd: std.posix.socket_t) ?PeerCred { 31 pub fn peerCred(fd: std.posix.socket_t) ?PeerCred {
26 return impl.peerCred(fd); 32 return impl.peerCred(fd);
27 } 33 }
28 34
35 /// A BLOCKING send that cannot raise SIGPIPE. `server_os.sendNoSigNoWait`
36 /// is the daemon's twin and is NON-blocking, because a stalled client must
37 /// never stall the pump; this one waits for room, because its caller is the
38 /// agent probe, which writes five bytes and then polls for the answer. The
39 /// daemon's name carries the difference so neither side is reached for by
40 /// habit.
41 ///
42 /// The signal half is the reason the operation exists here at all. The
43 /// probe runs before the client installs any signal handling, and the peer
44 /// may already have closed: the daemon accepts a forwarded agent socket
45 /// and only then closes it when no attached client is offering an agent.
46 /// A send that signalled would end the client outright instead of handing
47 /// back BrokenPipe for the probe to read as "no agent".
48 pub fn sendNoSig(fd: std.posix.socket_t, bytes: []const u8) std.posix.SendError!usize {
49 return impl.sendNoSig(fd, bytes);
50 }
51
29 /// The parent of `pid`, or 0 when the OS will not say or `pid` is not 52 /// The parent of `pid`, or 0 when the OS will not say or `pid` is not
30 /// positive. One step of the walk from an askpass helper up to the ssh a 53 /// positive. One step of the walk from an askpass helper up to the ssh a
31 /// dial spawned. 54 /// dial spawned.
@@ -51,15 +74,22 @@ pub fn setWinSize(fd: std.posix.fd_t, ws: std.posix.winsize) error{Unsupported}!
51 return impl.setWinSize(fd, ws); 74 return impl.setWinSize(fd, ws);
52 } 75 }
53 76
54 /// A real master/slave pty pair, the OS answering about the OS. Test-only: 77 /// A real master/slave pty pair, the OS answering about the OS. Named here
55 /// the wall never opens a pty, it lives in one. Named here rather than 78 /// rather than returned anonymously because an anonymous struct in the root
56 /// returned anonymously because an anonymous struct in the root and one in 79 /// and one in an arm are two distinct types, and the arm could then never
57 /// an arm are two distinct types, and the arm could then never satisfy the 80 /// satisfy the contract.
58 /// contract.
59 pub const PtyPair = struct { master: std.posix.fd_t, slave: std.posix.fd_t }; 81 pub const PtyPair = struct { master: std.posix.fd_t, slave: std.posix.fd_t };
60 pub fn openPtyPair() error{Unsupported}!PtyPair { 82
61 return impl.openPtyPair(); 83 /// Declared ONLY in a test binary, so this row's header stays true of every
62 } 84 /// shipped build: the wall never opens a pty, it lives in one, and a client
85 /// that could open one is a client an app might link a pty through. The two
86 /// callers that need a real terminal to size — this file's own test and
87 /// `interact.ptsPair` — are reached only from test blocks, and a test build
88 /// is the compilation where `builtin.is_test` holds and this decl exists.
89 /// A production line that reached for it gets the message below instead.
90 pub const openPtyPair = if (builtin.is_test) impl.openPtyPair else @compileError(
91 "client_os.openPtyPair is test-only: the client side links no pty",
92 );
63 93
64 test "client_os: the arm compiles and answers for the process it is in" { 94 test "client_os: the arm compiles and answers for the process it is in" {
65 try std.testing.expect(getpid() > 0); 95 try std.testing.expect(getpid() > 0);
@@ -91,6 +121,49 @@ test "client_os.winSize reads what setWinSize wrote, off a real pty" {
91 try std.testing.expectEqual(@as(u16, 17), ws.row); 121 try std.testing.expectEqual(@as(u16, 17), ws.row);
92 } 122 }
93 123
124 test "client_os.sendNoSig: a closed peer is an error, not a signal" {
125 // Judged in a CHILD, because this process cannot be asked. Zig's own
126 // startup code installs a no-op SIGPIPE handler in every binary it
127 // starts, the test runner included, so a plain send with no
128 // MSG_NOSIGNAL also returns BrokenPipe here — a test written in this
129 // process stays green with the flag deleted, which is the one mistake
130 // it exists to catch. The child puts SIGPIPE back at SIG_DFL first, so
131 // a send that raises the signal DIES and the parent reads a status that
132 // never exited.
133 //
134 // A raw fork rather than the server row's `forkPty`: this row links no
135 // pty and no fork by design, and build.zig's folder rule 6 reads only
136 // production lines, so the call is legal exactly here. The price is
137 // that the child inherits fd 1 and fd 2, and fd 1 is the build runner's
138 // protocol stream, which one stray byte wedges — so pointing both at
139 // /dev/null is the first thing the child does, before anything that
140 // could print.
141 const pid = try std.posix.fork();
142 if (pid == 0) {
143 const devnull = std.posix.open("/dev/null", .{ .ACCMODE = .RDWR }, 0) catch std.c._exit(2);
144 std.posix.dup2(devnull, std.posix.STDOUT_FILENO) catch std.c._exit(2);
145 std.posix.dup2(devnull, std.posix.STDERR_FILENO) catch std.c._exit(2);
146 var dfl: std.posix.Sigaction = .{
147 .handler = .{ .handler = std.posix.SIG.DFL },
148 .mask = std.posix.sigemptyset(),
149 .flags = 0,
150 };
151 std.posix.sigaction(std.posix.SIG.PIPE, &dfl, null);
152 var csp: [2]std.posix.fd_t = undefined;
153 if (std.c.socketpair(std.posix.AF.UNIX, std.posix.SOCK.STREAM, 0, &csp) != 0) std.c._exit(2);
154 std.posix.close(csp[1]);
155 _ = sendNoSig(csp[0], "x") catch |e| std.c._exit(if (e == error.BrokenPipe) 0 else 2);
156 // A send that SUCCEEDED to a closed peer is as wrong as one that
157 // signalled, and neither is 0. `_exit` rather than an exit that runs
158 // atexit handlers: this child is a copy of a test runner mid-run and
159 // must flush nothing of its parent's.
160 std.c._exit(2);
161 }
162 const status = std.posix.waitpid(pid, 0).status;
163 try std.testing.expect(std.posix.W.IFEXITED(status));
164 try std.testing.expectEqual(@as(u32, 0), std.posix.W.EXITSTATUS(status));
165 }
166
94 test { 167 test {
95 std.testing.refAllDeclsRecursive(@This()); 168 std.testing.refAllDeclsRecursive(@This());
96 } 169 }
src/os/client_os_linux.zig
Old New
@@ -13,6 +13,10 @@ pub fn peerCred(fd: std.posix.socket_t) ?root.PeerCred {
13 return .{ .uid = cred.uid, .pid = cred.pid }; 13 return .{ .uid = cred.uid, .pid = cred.pid };
14 } 14 }
15 15
16 pub fn sendNoSig(fd: std.posix.socket_t, bytes: []const u8) std.posix.SendError!usize {
17 return std.posix.send(fd, bytes, std.posix.MSG.NOSIGNAL);
18 }
19
16 /// `/proc/<pid>/stat` field 4. Parsed from the LAST ')' rather than by 20 /// `/proc/<pid>/stat` field 4. Parsed from the LAST ')' rather than by
17 /// counting spaces: field 2 is the executable's name, unquoted, and a 21 /// counting spaces: field 2 is the executable's name, unquoted, and a
18 /// program free to call itself `a b) c` is a program free to move every 22 /// program free to call itself `a b) c` is a program free to move every
src/os/server_os.zig
Old New
@@ -34,12 +34,17 @@ pub fn peerCred(fd: std.posix.socket_t) ?PeerCred {
34 return cred; 34 return cred;
35 } 35 }
36 36
37 /// A non-blocking send that cannot raise SIGPIPE: a client that hung up 37 /// A NON-BLOCKING send that cannot raise SIGPIPE: a client that hung up
38 /// mid-frame is an error the pump handles, never a signal that ends the 38 /// mid-frame is an error the pump handles, never a signal that ends the
39 /// daemon. The daemon also ignores SIGPIPE process-wide; this is the half 39 /// daemon, and a client that stopped reading must not stall the pump
40 /// either. The daemon also ignores SIGPIPE process-wide; this is the half
40 /// that does not depend on the order of that ignore against a fork. 41 /// that does not depend on the order of that ignore against a fork.
41 pub fn sendNoSig(fd: std.posix.socket_t, bytes: []const u8) std.posix.SendError!usize { 42 /// The name says NoWait because `client_os.sendNoSig` is the client-side
42 return impl.sendNoSig(fd, bytes); 43 /// operation and it BLOCKS: the two differ in that one respect, and a
44 /// shared name would let a caller that moved between them assume the
45 /// other's behaviour.
46 pub fn sendNoSigNoWait(fd: std.posix.socket_t, bytes: []const u8) std.posix.SendError!usize {
47 return impl.sendNoSigNoWait(fd, bytes);
43 } 48 }
44 49
45 /// The socket type of an fd, for refusing to adopt a stream fd as the 50 /// The socket type of an fd, for refusing to adopt a stream fd as the
@@ -357,7 +362,7 @@ test "server_os.anonFd: no path names it, and it is not CLOEXEC" {
357 try std.testing.expectEqualStrings("abc", &buf); 362 try std.testing.expectEqualStrings("abc", &buf);
358 } 363 }
359 364
360 test "server_os.sendNoSig: a closed peer is an error, not a signal" { 365 test "server_os.sendNoSigNoWait: a closed peer is an error, not a signal" {
361 // Judged in a CHILD, because this process cannot be asked. Zig's own 366 // Judged in a CHILD, because this process cannot be asked. Zig's own
362 // startup code installs a no-op SIGPIPE handler in every binary it 367 // startup code installs a no-op SIGPIPE handler in every binary it
363 // starts, the test runner included, so a plain send with no 368 // starts, the test runner included, so a plain send with no
@@ -365,8 +370,11 @@ test "server_os.sendNoSig: a closed peer is an error, not a signal" {
365 // process stays green with the flag deleted, which is the one mistake 370 // process stays green with the flag deleted, which is the one mistake
366 // it exists to catch. The child puts SIGPIPE back at SIG_DFL first, so 371 // it exists to catch. The child puts SIGPIPE back at SIG_DFL first, so
367 // a send that raises the signal DIES and the parent reads a status that 372 // a send that raises the signal DIES and the parent reads a status that
368 // never exited. `forkPty` rather than a raw fork: this file is scanned 373 // never exited. `forkPty` rather than a raw fork because of the child's
369 // by build.zig's folder rule 6, whose one exemption is the Linux arm. 374 // stdio: a raw-forked child shares fd 1 with this process, and fd 1 of
375 // a test binary is the build runner's protocol stream, which one stray
376 // byte wedges. A `forkPty` child gets its own stdio on the slave, so
377 // anything it prints goes to a pty nobody reads.
370 const f = try forkPty(.{ .row = 24, .col = 80, .xpixel = 0, .ypixel = 0 }); 378 const f = try forkPty(.{ .row = 24, .col = 80, .xpixel = 0, .ypixel = 0 });
371 if (f.pid == 0) { 379 if (f.pid == 0) {
372 var dfl: std.posix.Sigaction = .{ 380 var dfl: std.posix.Sigaction = .{
@@ -378,7 +386,7 @@ test "server_os.sendNoSig: a closed peer is an error, not a signal" {
378 var csp: [2]std.posix.fd_t = undefined; 386 var csp: [2]std.posix.fd_t = undefined;
379 if (std.c.socketpair(std.posix.AF.UNIX, std.posix.SOCK.STREAM, 0, &csp) != 0) exitNow(2); 387 if (std.c.socketpair(std.posix.AF.UNIX, std.posix.SOCK.STREAM, 0, &csp) != 0) exitNow(2);
380 std.posix.close(csp[1]); 388 std.posix.close(csp[1]);
381 _ = sendNoSig(csp[0], "x") catch |e| exitNow(if (e == error.BrokenPipe) 0 else 2); 389 _ = sendNoSigNoWait(csp[0], "x") catch |e| exitNow(if (e == error.BrokenPipe) 0 else 2);
382 // A send that SUCCEEDED to a closed peer is as wrong as one that 390 // A send that SUCCEEDED to a closed peer is as wrong as one that
383 // signalled, and neither is 0. 391 // signalled, and neither is 0.
384 exitNow(2); 392 exitNow(2);
src/os/server_os_linux.zig
Old New
@@ -17,7 +17,7 @@ pub fn peerCred(fd: std.posix.socket_t) ?root.PeerCred {
17 return .{ .uid = cred.uid, .pid = cred.pid }; 17 return .{ .uid = cred.uid, .pid = cred.pid };
18 } 18 }
19 19
20 pub fn sendNoSig(fd: std.posix.socket_t, bytes: []const u8) std.posix.SendError!usize { 20 pub fn sendNoSigNoWait(fd: std.posix.socket_t, bytes: []const u8) std.posix.SendError!usize {
21 return std.posix.send(fd, bytes, std.posix.MSG.DONTWAIT | std.posix.MSG.NOSIGNAL); 21 return std.posix.send(fd, bytes, std.posix.MSG.DONTWAIT | std.posix.MSG.NOSIGNAL);
22 } 22 }
23 23
src/os/spawn.zig
Old New
@@ -6,8 +6,11 @@ const builtin = @import("builtin");
6 6
7 /// The kernel's link to the running image, used only when the resolved 7 /// The kernel's link to the running image, used only when the resolved
8 /// path is no longer executable — Linux keeps a live link after a rename- 8 /// path is no longer executable — Linux keeps a live link after a rename-
9 /// over. On an OS with no such link the fallback is the resolved path 9 /// over, so the exec runs the image already running. On an OS with no such
10 /// itself, and an exec of a replaced image fails where it always would. 10 /// link the fallback is the resolved path itself, and an exec after a
11 /// rename-over runs the NEW file at that path rather than the running one.
12 /// That is the ordinary meaning of the path and not a failure, but it does
13 /// mean the started daemon can be a different build from the starter.
11 pub const self_exe: []const u8 = switch (builtin.os.tag) { 14 pub const self_exe: []const u8 = switch (builtin.os.tag) {
12 .linux => "/proc/self/exe", 15 .linux => "/proc/self/exe",
13 else => "", 16 else => "",
src/server/server.zig
Old New
@@ -177,7 +177,7 @@ const Sink = union(enum) {
177 /// unbounded growth just moved into the listener. 177 /// unbounded growth just moved into the listener.
178 fn send(self: Sink, bytes: []const u8) !usize { 178 fn send(self: Sink, bytes: []const u8) !usize {
179 return switch (self) { 179 return switch (self) {
180 .socket => |fd| server_os.sendNoSig(fd, bytes), 180 .socket => |fd| server_os.sendNoSigNoWait(fd, bytes),
181 .quic => |q| q.listener.send(q.id, bytes), 181 .quic => |q| q.listener.send(q.id, bytes),
182 }; 182 };
183 } 183 }
@@ -3016,7 +3016,11 @@ pub const Server = struct {
3016 3016
3017 // Clear FD_CLOEXEC on a descriptor so it survives execve. The upgrade 3017 // Clear FD_CLOEXEC on a descriptor so it survives execve. The upgrade
3018 // exec keeps the listener, QUIC UDP, pty masters, agent listeners and 3018 // exec keeps the listener, QUIC UDP, pty masters, agent listeners and
3019 // the manifest carrier; all are CLOEXEC by default and must be cleared. 3019 // the manifest carrier. Most of those are CLOEXEC by default; the
3020 // carrier is not (`server_os.anonFd` never sets the flag) and is on the
3021 // list because `restoreCloexec` re-seals every fd here after a failed
3022 // exec, so a carrier missing from it would stay inheritable by the
3023 // shells this daemon spawns next.
3020 pub fn clearCloexec(fd: std.posix.fd_t) !void { 3024 pub fn clearCloexec(fd: std.posix.fd_t) !void {
3021 const flags = try std.posix.fcntl(fd, std.posix.F.GETFD, 0); 3025 const flags = try std.posix.fcntl(fd, std.posix.F.GETFD, 0);
3022 _ = try std.posix.fcntl(fd, std.posix.F.SETFD, flags & ~@as(usize, std.posix.FD_CLOEXEC)); 3026 _ = try std.posix.fcntl(fd, std.posix.F.SETFD, flags & ~@as(usize, std.posix.FD_CLOEXEC));
src/server/server_test_session.zig
Old New
@@ -778,13 +778,15 @@ test "Server: a table of TERM-ignoring shells costs one grace, not one each" {
778 return error.TeardownGraceNotShared; 778 return error.TeardownGraceNotShared;
779 } 779 }
780 780
781 // Ask the OS, not the daemon: a reaped child has no /proc entry, so 781 // Ask the OS, not the daemon: a pid that was reaped names no process at
782 // this catches both a survivor and a zombie the daemon claimed to have 782 // all, so `kill(pid, 0)` must fail. That catches a survivor, and it
783 // waited for. 783 // catches a zombie the daemon claimed to have waited for too — a zombie
784 // is still a process table entry and `kill(pid, 0)` on one SUCCEEDS,
785 // which is the same verdict the /proc read this replaces gave, since a
786 // zombie keeps its /proc directory. Spelled through `kill` rather than
787 // /proc because every OS has it.
784 for (kids[0..n]) |pid| { 788 for (kids[0..n]) |pid| {
785 var pb: [32]u8 = undefined; 789 try std.testing.expectError(error.ProcessNotFound, std.posix.kill(pid, 0));
786 const proc = try std.fmt.bufPrint(&pb, "/proc/{d}", .{pid});
787 try std.testing.expectError(error.FileNotFound, std.fs.cwd().access(proc, .{}));
788 } 790 }
789 } 791 }
790 792
test/e2e_lib.sh
Old New
@@ -1369,6 +1369,15 @@ s = socket.socket(socket.AF_UNIX); s.bind(sys.argv[1]); s.listen(1); time.sleep(
1369 [ -S "$_osock" ] || { 1369 [ -S "$_osock" ] || {
1370 echo "e2e FAIL: oracle: the listener bound no socket at $_osock"; exit 1; } 1370 echo "e2e FAIL: oracle: the listener bound no socket at $_osock"; exit 1; }
1371 1371
1372 # The subject is only the process this means to ask about once it has
1373 # opened fds 5 and 6 and forked `sleep`; until then `pid_fd_count` and
1374 # `pid_children` would be graded against a shell still starting up, and
1375 # the self-test would fail for a reason that is nothing to do with the
1376 # helpers. A child is the LAST of those steps, so waiting for one waits
1377 # for all of them.
1378 wait_until 50 "oracle: the subject shell never forked its sleep" \
1379 '[ -n "$(pid_children "$_opid")" ]'
1380
1372 pid_alive "$_opid" || { 1381 pid_alive "$_opid" || {
1373 echo "e2e FAIL: oracle: pid_alive says a live shell is dead"; exit 1; } 1382 echo "e2e FAIL: oracle: pid_alive says a live shell is dead"; exit 1; }
1374 [ "$(pid_comm "$_opid")" = sh ] || { 1383 [ "$(pid_comm "$_opid")" = sh ] || {