From de78a167532d1f78611381accd8839fac066fd56 Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Thu, 1 Oct 2026 08:49:50 -0300 Subject: A language server goes with its children when reaped, and every server's process group is killed as pardes exits: no zls or `zig build --build-runner` outlives the editor The server is spawned with setsid, its own group's leader, but reap() signalled only its pid, and nothing reaped the servers at exit: the socket closing was left to tell them. zls busy in its build runner at a cold start did not notice, and lived on with its `zig build` for as long as no one killed it (found as three orphans 40 minutes after their sessions had gone). reap() now signals the group, and an atexit hook, registered at the first spawn, kills every server's group. Co-Authored-By: Claude Opus 5.5 --- src/lsp/lsp_client.zig | 69 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 64 insertions(+), 5 deletions(-) diff --git a/src/lsp/lsp_client.zig b/src/lsp/lsp_client.zig index a3f3a954..ce90b8bb 100644 --- a/src/lsp/lsp_client.zig +++ b/src/lsp/lsp_client.zig @@ -57,6 +57,7 @@ extern "c" fn access(path: [*:0]const u8, mode: c_int) c_int; /// std.posix.getenv is gone in 0.16 and std.process.Environ wants an Io extern "c" fn getenv(name: [*:0]const u8) ?[*:0]const u8; extern "c" fn usleep(usec: c_uint) c_int; +extern "c" fn atexit(f: *const fn () callconv(.c) void) c_int; extern "c" fn realpath(path: [*:0]const u8, resolved: [*]u8) ?[*:0]u8; // test-only libc (the seam's own tests build a real directory tree) extern "c" fn mkdtemp(template: [*:0]u8) ?[*:0]u8; @@ -1202,6 +1203,7 @@ fn ensure(c: *Conn, si: usize, arena: std.mem.Allocator, req: lsp.Req, tr: *Trac tr.note("STOP: socketpair for {s} failed", .{specs[si].name}); return error.NoServer; } + if (!exit_hook.swap(true, .acq_rel)) _ = atexit(killAllAtExit); const pid = libc.fork(); if (pid < 0) { _ = libc.close(sv[0]); @@ -1493,21 +1495,38 @@ fn shutdownIf(c: *Conn, g: u32) void { _ = libc.pthread_cond_broadcast(&c.cond); } -/// TERM, a bounded grace, then KILL. Never blocks unboundedly: a reaped or -/// foreign pid answers -1 immediately. +/// TERM, a bounded grace, then KILL, to the server's whole process group +/// (it is its group's leader, setsid at spawn): what it started -- zls's +/// `zig build --build-runner` -- goes with it, never left an orphan. Never +/// blocks unboundedly: a reaped or foreign pid answers -1 immediately. fn reap(pid: libc.pid_t) void { if (pid <= 0) return; - _ = libc.kill(pid, .TERM); + _ = libc.kill(-pid, .TERM); var tries: u8 = 0; while (tries < 20) : (tries += 1) { const w = libc.waitpid(pid, null, libc.W.NOHANG); - if (w == pid or w < 0) return; + if (w == pid or w < 0) { + // Its children may still be finishing their TERM. + _ = libc.kill(-pid, .KILL); + return; + } _ = usleep(10_000); } - _ = libc.kill(pid, .KILL); + _ = libc.kill(-pid, .KILL); _ = libc.waitpid(pid, null, 0); } +/// At this process's exit, every server's group is killed: a server busy +/// in a build (zls at a cold start) would not notice its socket close, and +/// lived on with its children after the editor. Registered once, at the +/// first spawn; signals only, no locks (other threads may hold them). +fn killAllAtExit() callconv(.c) void { + for (&conns) |*c| if (c.pid > 0) { + _ = libc.kill(-c.pid, .KILL); + }; +} +var exit_hook = std.atomic.Value(bool).init(false); + /// Free every per-document accumulation. Mutex held. fn forgetDocs(c: *Conn) void { for (c.extra_roots.items) |r| sa.free(r); @@ -2359,3 +2378,43 @@ test "a server that failed a moment ago is said to have failed, and when it is t try query(std.testing.allocator, arena.allocator(), .{ .kind = .hover, .path = "/x/a.zig", .source = "", .offset = 0 }, &out.writer); try std.testing.expect(std.mem.indexOf(u8, out.written(), "zls failed recently; retry in 5s") != null); } + +test "reaping a server takes what it started with it: its process group goes, its children too" { + var fds: [2]libc.fd_t = undefined; + if (libc.pipe(&fds) != 0) return error.SkipZigTest; + const pid = libc.fork(); + if (pid < 0) return error.SkipZigTest; + if (pid == 0) { + _ = setsid(); + const child = libc.fork(); + if (child == 0) { + while (true) _ = usleep(100_000); + } + var word: [4]u8 = @bitCast(@as(i32, child)); + _ = libc.write(fds[1], &word, 4); + while (true) _ = usleep(100_000); + } + _ = libc.close(fds[1]); + var word: [4]u8 = undefined; + try std.testing.expectEqual(@as(isize, 4), libc.read(fds[0], &word, 4)); + _ = libc.close(fds[0]); + const grandchild: libc.pid_t = @bitCast(word); + reap(pid); + // Gone, or a zombie waiting for its new parent to reap it: not running. + var tries: u32 = 0; + const running = while (tries < 200) : (tries += 1) { + var path: [64]u8 = undefined; + const stat_path = try std.fmt.bufPrintSentinel(&path, "/proc/{d}/stat", .{grandchild}, 0); + const fd = libc.open(stat_path, .{ .ACCMODE = .RDONLY }); + if (fd < 0) break false; + var buf: [256]u8 = undefined; + const n = libc.read(fd, &buf, buf.len); + _ = libc.close(fd); + if (n > 0) if (std.mem.lastIndexOfScalar(u8, buf[0..@intCast(n)], ')')) |close| { + if (buf[close + 2] == 'Z') break false; + }; + _ = usleep(10_000); + } else true; + if (running) _ = libc.kill(grandchild, .KILL); + try std.testing.expect(!running); +} -- cgit v1.3