From 990c2e1b184e9cb1fddd8eb1b05a472cfb113dde Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Mon, 28 Sep 2026 12:01:34 -0300 Subject: A closed terminal's shell is reaped on tty and macOS, killed if it ignores the hangup tty and macOS hung the pty up and called waitpid once without waiting, so a shell still exiting, or one that ignores SIGHUP, stayed a zombie or ran on with nobody reading it; tty also never reaped a shell that exited by itself, and its spawn into an occupied slot closed the old pty without ending the shell. host_io's retireShell says hangup, waits 100 ms on a thread (macOS has no host timer to poll from), then kills and reaps. The gui's own retired list, when full, left the slot holding the old shell and refused the next spawn into that pane; it now hands that shell to retireShell instead. Co-Authored-By: Claude Opus 5.5 --- src/gui/gui.zig | 4 ++++ src/host_io.zig | 47 +++++++++++++++++++++++++++++++++++++++++++++++ src/macos.zig | 5 ++--- src/pardes.zig | 26 ++++++++++++++++++++++++++ src/tty/tty.zig | 14 +++++--------- 5 files changed, 84 insertions(+), 12 deletions(-) diff --git a/src/gui/gui.zig b/src/gui/gui.zig index 1d5d3c05..2369dd28 100644 --- a/src/gui/gui.zig +++ b/src/gui/gui.zig @@ -3786,6 +3786,10 @@ const Shell = struct { s.ptys[pane] = null; return; }; + // No room to wait for it here: a slot kept for it would refuse the + // next shell spawned into this pane until it was reaped. + host_io.retireShell(pt.pid); + s.ptys[pane] = null; } fn stopPtys(s: *Shell) void { diff --git a/src/host_io.zig b/src/host_io.zig index 5d9dd413..81d784c5 100644 --- a/src/host_io.zig +++ b/src/host_io.zig @@ -1330,6 +1330,35 @@ const darwin = struct { } }; +/// A terminal pane's shell is done with (its pane gone, its pty closed or at +/// end of file): say hangup, for a shell that ignores the tty's, and see it +/// reaped, killed if it is still there 100 ms on. A thread does the waiting +/// so that no host loop needs a timer for it (macOS's has none); a thread +/// that cannot start kills and reaps it here. +/// ponytail: a shell still inside its 100 ms when the editor exits is left +/// to init; the gui and detached hosts keep their own retired lists, which +/// this could replace. +pub fn retireShell(pid: libc.pid_t) void { + if (pid <= 0) return; + _ = libc.kill(pid, libc.SIG.HUP); + if (libc.waitpid(pid, null, libc.W.NOHANG) != 0) return; + const thread = std.Thread.spawn(.{}, reapShell, .{ pid, 100 }) catch return reapShell(pid, 0); + thread.detach(); +} + +fn reapShell(pid: libc.pid_t, grace_ms: u32) void { + var waited: u32 = 0; + while (waited < grace_ms) : (waited += 5) { + // Not 0 is reaped, or someone else reaped it: either way not ours. + if (libc.waitpid(pid, null, libc.W.NOHANG) != 0) return; + const ts: libc.timespec = .{ .sec = 0, .nsec = 5 * std.time.ns_per_ms }; + _ = libc.nanosleep(&ts, null); + } + if (libc.waitpid(pid, null, libc.W.NOHANG) != 0) return; + _ = libc.kill(pid, libc.SIG.KILL); + while (libc.waitpid(pid, null, 0) < 0 and libc.errno(-1) == .INTR) {} +} + pub fn signalTty(shell_pid: libc.pid_t, master_fd: c_int, which: pardes.PtySignal) void { const sig = switch (which) { .int => libc.SIG.INT, @@ -1665,6 +1694,24 @@ test "Kill's signal stops the foreground job and never the shell" { try std.testing.expectEqual(@as(libc.pid_t, 0), libc.waitpid(sh.pid, null, libc.W.NOHANG)); } +test "a retired shell that ignores the hangup is killed and reaped, not left a zombie" { + const pid = libc.fork(); + if (pid == 0) { + const argv = [_:null]?[*:0]const u8{ "/bin/sh", "-c", "trap '' HUP; exec sleep 30" }; + const envp = [_:null]?[*:0]const u8{}; + _ = libc.execve("/bin/sh", &argv, &envp); + libc._exit(127); + } + try std.testing.expect(pid > 0); + sleepMs(100); // past the trap + retireShell(pid); + // A zombie still answers kill(pid, 0); only a reaped pid is gone. + var waited: i64 = 0; + while (libc.kill(pid, @enumFromInt(0)) == 0 and waited < 2000) : (waited += 10) sleepMs(10); + try std.testing.expect(libc.kill(pid, @enumFromInt(0)) != 0); + try std.testing.expect(waited >= 90); // it did ignore the hangup +} + test "a background job is not the tty's owner" { if (comptime !tty_probe_platform) return error.SkipZigTest; var sh = TestShell.start() orelse return error.SkipZigTest; diff --git a/src/macos.zig b/src/macos.zig index a0abc119..ebe8193b 100644 --- a/src/macos.zig +++ b/src/macos.zig @@ -2505,8 +2505,7 @@ fn startReader(st: *State, pt: *Pty, id: u8) void { /// The pane is gone: its shell goes with it. fn closePty(ctx: ?*anyopaque, pane: u8) void { const st = hostState(ctx); - const pt = st.ptys[pane] orelse return; - _ = libc.kill(pt.pid, posix.SIG.HUP); + if (st.ptys[pane] == null) return; reap(st, pane); st.gens[pane] +%= 1; } @@ -2516,7 +2515,7 @@ fn reap(st: *State, pane: u8) void { st.ptys[pane] = null; pt.reader.cancel(st.io) catch {}; _ = libc.close(pt.file.handle); - _ = libc.waitpid(pt.pid, null, posix.W.NOHANG); + host_io.retireShell(pt.pid); } fn readPty(st: *State, io: std.Io, pty: std.Io.File, id: u8, gen: u32) anyerror!void { diff --git a/src/pardes.zig b/src/pardes.zig index f3348ea5..59556649 100644 --- a/src/pardes.zig +++ b/src/pardes.zig @@ -1125,6 +1125,32 @@ test "Tty spawns a raw shell in the caller's directory" { try std.testing.expect(found); } +test "a terminal respawned into a closed pane's slot hangs the old shell up first" { + // Hosts keep a shell per slot: were the spawn first, the close would + // end the new shell and leave the old one running. + const p = try Pardes.init(std.testing.allocator, .{ .tty_only = true, .cols = 100, .rows = 30 }); + defer p.deinit(); + try std.testing.expect(p.executeBuiltinLine(0, "Tty")); + const slot = p.active; + while (p.nextEffect()) |_| {} + try p.removePane(slot, null); + try std.testing.expect(p.executeBuiltinLine(0, "Tty")); + try std.testing.expectEqual(slot, p.active); + var closed = false; + var spawned = false; + while (p.nextEffect()) |effect| switch (effect) { + .close_pty => |c| if (c.pane == slot) { + try std.testing.expect(!spawned); + closed = true; + }, + .spawn => |s| if (s.pane == slot) { + spawned = true; + }, + else => {}, + }; + try std.testing.expect(closed and spawned); +} + test "Tty9p marks only the new Linux terminal for a mounted shell" { if (comptime !hosted or @import("builtin").os.tag != .linux) return error.SkipZigTest; const p = try Pardes.init(std.testing.allocator, .{ .tty_only = true }); diff --git a/src/tty/tty.zig b/src/tty/tty.zig index b2768ed6..128a7ead 100644 --- a/src/tty/tty.zig +++ b/src/tty/tty.zig @@ -1007,6 +1007,7 @@ const Shell = struct { if (s.ptys[e.id]) |*pt| { pt.reader.await(s.io) catch {}; // reader just finished; join it or its future leaks _ = libc.close(pt.file.handle); + host_io.retireShell(pt.pid); s.ptys[e.id] = null; } core.update(.{ .eof = .{ .pane = @intCast(e.id) } }); @@ -1193,11 +1194,7 @@ const Shell = struct { fn spawn(ctx: ?*anyopaque, pane: u8, cwd: []const u8) void { const s = of(ctx); - if (s.ptys[pane]) |*old| { - old.reader.cancel(s.io) catch {}; - _ = libc.close(old.file.handle); - s.ptys[pane] = null; - } + closePty(ctx, pane); // a shell still in the slot goes first, reaped s.gens[pane] +%= 1; const child = host_io.forkShell(s.core, pane, s.prompt_rcs, s.core.shellBin(), cwd, s.core.screen_h, s.core.screen_w, s.fs) catch |err| return s.core.reportError(pane, "shell", err); s.ptys[pane] = .{ .file = child.file, .pid = child.pid, .reader = .{ .any_future = null, .result = {} } }; @@ -1229,8 +1226,8 @@ const Shell = struct { } /// The pane is gone: hang its pty up, which the kernel passes on to the - /// shell as SIGHUP, and say it too for a shell that ignores the tty's. - /// Output still in flight carries the old generation and is dropped. + /// shell as SIGHUP, and retire the shell. Output still in flight + /// carries the old generation and is dropped. fn closePty(ctx: ?*anyopaque, pane: u8) void { const s = of(ctx); var pt = s.ptys[pane] orelse return; @@ -1238,8 +1235,7 @@ const Shell = struct { s.gens[pane] +%= 1; pt.reader.cancel(s.io) catch {}; _ = libc.close(pt.file.handle); - _ = libc.kill(pt.pid, posix.SIG.HUP); - _ = libc.waitpid(pt.pid, null, posix.W.NOHANG); + host_io.retireShell(pt.pid); } fn ttyTaken(ctx: ?*anyopaque, pane: u8) bool { -- cgit v1.3