1 files changed, 82 insertions, 5 deletions

diff --git a/src/lsp/lsp_client.zig b/src/lsp/lsp_client.zig
index eca2691d..b1be590b 100644
--- a/src/lsp/lsp_client.zig
+++ b/src/lsp/lsp_client.zig
@@ -1050,6 +1050,54 @@ fn flat(arena: std.mem.Allocator, s: []const u8) []const u8 {
     return std.mem.trim(u8, buf.items, " ");
 }
 
+/// Close-on-exec by fcntl, the darwin route. Same three lines as fuse.zig's
+/// and nested.zig's, and here for the same reason they have their own: this
+/// file imports neither.
+fn setCloexec(fd: c_int) void {
+    const FD_CLOEXEC: c_int = 1;
+    _ = libc.fcntl(fd, libc.F.SETFD, FD_CLOEXEC);
+}
+
+/// The client's transport: an AF_UNIX stream pair with both ends close-on-exec
+/// and, on darwin, the parent end opted out of SIGPIPE. False if the host
+/// refused, which is a dead server and not a dead editor.
+///
+/// A named function rather than nine lines inside `ensure` because the one
+/// thing it encodes is a PLATFORM LIE, and a test has to be able to call
+/// exactly what the spawn calls. SOCK_CLOEXEC is a LINUX flag; zig spells
+/// `SOCK.CLOEXEC` for darwin too — as 0x10000000, with "does not exist on
+/// darwin but is used in std.net" in the comment beside it — and darwin's
+/// socketpair(2) validates `type` strictly, so asking for it there returns
+/// EPROTONOSUPPORT. Every server spawn on macOS failed on that line, before
+/// the fork: no binary probe, no handshake, no message row, just `NoServer` in
+/// 100µs from a client that had never once run on the platform it was written
+/// on. The end-to-end suite that would have caught it (test/snapshots/
+/// lsp-client.snap) only ever runs against the linux target, where the flag is
+/// real. fuse.zig and nested.zig already took the plain-socket-plus-fcntl
+/// route; this was the one caller that did not.
+///
+/// THE WINDOW THIS LEAVES, the same one host_io.zig states for the pty master:
+/// fcntl after socketpair is not atomic, so another thread that forks and
+/// execs in between inherits both ends. Linux closes it with the flag; darwin
+/// has no socketpair that takes one.
+fn transportPair(sv: *[2]libc.fd_t) bool {
+    const sock_type = if (comptime builtin.os.tag.isDarwin())
+        libc.SOCK.STREAM
+    else
+        libc.SOCK.STREAM | libc.SOCK.CLOEXEC;
+    if (libc.socketpair(libc.AF.UNIX, sock_type, 0, sv) != 0) return false;
+    if (comptime builtin.os.tag.isDarwin()) {
+        setCloexec(sv[0]);
+        // The child dup2s this onto 0 and 1, and dup2 CLEARS close-on-exec on
+        // the copy, so the server still gets the socket; this marks only the
+        // number itself, which the child's 3..1024 sweep closes anyway.
+        setCloexec(sv[1]);
+        const one: c_int = 1;
+        _ = libc.setsockopt(sv[0], libc.SOL.SOCKET, so_nosigpipe, @ptrCast(&one), @sizeOf(c_int));
+    }
+    return true;
+}
+
 // ----------------------------------------------------------- the connection
 
 /// Spawn-or-return, and tell an existing server about a new project root.
@@ -1106,19 +1154,17 @@ fn ensure(c: *Conn, si: usize, arena: std.mem.Allocator, req: lsp.Req, tr: *Trac
     const owned_root = sa.dupe(u8, root) catch return error.OutOfMemory;
 
     var sv: [2]libc.fd_t = undefined;
-    if (libc.socketpair(libc.AF.UNIX, libc.SOCK.STREAM | libc.SOCK.CLOEXEC, 0, &sv) != 0) {
+    if (!transportPair(&sv)) {
         sa.free(owned_root);
+        tr.note("STOP: socketpair for {s} failed", .{specs[si].name});
         return error.NoServer;
     }
-    if (comptime builtin.os.tag.isDarwin()) {
-        const one: c_int = 1;
-        _ = libc.setsockopt(sv[0], libc.SOL.SOCKET, so_nosigpipe, @ptrCast(&one), @sizeOf(c_int));
-    }
     const pid = libc.fork();
     if (pid < 0) {
         _ = libc.close(sv[0]);
         _ = libc.close(sv[1]);
         sa.free(owned_root);
+        tr.note("STOP: fork for {s} failed", .{specs[si].name});
         return error.NoServer;
     }
     if (pid == 0) {
@@ -2125,3 +2171,34 @@ test "specFor routes extensions and honours the disable env" {
     try std.testing.expect(specFor("/x/README.md") == null);
     try std.testing.expectEqualStrings("rust-analyzer", specs[specFor("/x/main.rs").?].name);
 }
+
+test "the transport this host actually gives us is a pair, and both ends are close-on-exec" {
+    // The spawn's FIRST fallible step, and for one release on macOS its last:
+    // `SOCK.CLOEXEC` is spelled for darwin in zig's libc bindings and rejected
+    // by darwin's socketpair(2), so this returned EPROTONOSUPPORT and no
+    // language server was ever forked on that platform. Nothing above the
+    // transport can notice — `ensure` reports the same `NoServer` a missing
+    // binary does — so the check belongs here, on the real function, in a test
+    // that runs on the host rather than on the linux target the snapshot
+    // suite cross-compiles to.
+    var sv: [2]libc.fd_t = undefined;
+    try std.testing.expect(transportPair(&sv));
+    defer {
+        _ = libc.close(sv[0]);
+        _ = libc.close(sv[1]);
+    }
+
+    // close-on-exec on both ends, however the platform got there: the flag on
+    // linux, fcntl on darwin. Without it every pty shell forked afterwards
+    // inherits the server's socket, which is the bug host_io.zig fixed for the
+    // pty master.
+    const FD_CLOEXEC: c_int = 1;
+    for (sv) |fd| try std.testing.expect(libc.fcntl(fd, libc.F.GETFD, @as(c_int, 0)) & FD_CLOEXEC != 0);
+
+    // and it is a connected PAIR, not two unrelated descriptors
+    const msg = "ping";
+    try std.testing.expectEqual(@as(isize, msg.len), libc.write(sv[0], msg, msg.len));
+    var got: [8]u8 = undefined;
+    try std.testing.expectEqual(@as(isize, msg.len), libc.read(sv[1], &got, got.len));
+    try std.testing.expectEqualStrings(msg, got[0..msg.len]);
+}