From f30ee05b0609b8f3aa488c94f3ff21733ed37053 Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Mon, 28 Sep 2026 16:29:29 -0300 Subject: A command's exit is told once its output is in, by the pty's state, not a timer The watcher woke the host a second time 60 ms after the exit by its own clock while the grace was counted from the reap: a host busy for 10 ms missed it, and the tag said running for ever and Kill did nothing; and under load exit 0 could land before the last output. As decided, no timer: the exit is told once it is reaped and the pty says nothing is left (poll: no POLLIN, and no POLLHUP, which means the end of file is on its way behind the output), else at that end of file, checked after each chunk of output. The tty host checks inside its step so the frame shows it. The four hosts' copies are one host_io.takeExits/commandEof, which close a told command's pty at its end of file (the fd and the GUI's reader leaked when the exit came first). A finished command pane whose pty a job it left still holds is not reused, so that job is not hung up; and the reset before a reuse is SGR 0, not DECSTR, which ghostty's stream does not implement. Co-Authored-By: Claude Opus 5.5 --- src/detached/server.zig | 28 +++++----------- src/exec.zig | 12 ++++--- src/gui/gui.zig | 33 +++++++------------ src/host_io.zig | 88 +++++++++++++++++++++++++++++++++++++------------ src/macos.zig | 41 +++++++++-------------- src/panes.zig | 3 ++ src/pardes.zig | 14 ++++++-- src/tty/tty.zig | 39 +++++++++------------- 8 files changed, 139 insertions(+), 119 deletions(-) diff --git a/src/detached/server.zig b/src/detached/server.zig index b47bda1b..8fc7b96e 100644 --- a/src/detached/server.zig +++ b/src/detached/server.zig @@ -484,7 +484,7 @@ pub const Session = struct { s.ninep, ) catch |err| return s.core.reportError(pane, "shell", err); const command = if (s.core.panes[pane]) |pn| pn.command != null else false; - s.ptys[pane] = .{ .fd = child.file.handle, .pid = child.pid, .cmd = .{ .watched = command and host_io.watchExit(child.pid) } }; + s.ptys[pane] = .{ .fd = child.file.handle, .pid = child.pid, .cmd = .{ .watched = command and host_io.watchExit(child.pid), .fd = child.file.handle } }; setNonblock(child.file.handle); var lbuf: [pardes.memory.limits.host_path_cap + 1]u8 = undefined; if (host_io.shellCwd(child.pid, &lbuf)) |wd| s.core.setCwd(pane, wd); @@ -697,11 +697,7 @@ pub const Session = struct { // A command's pty stays open until its child has exited: closing // it would hang up one that runs on without it. // Its exit, if it came first, is told now its output is in. - if (pt.cmd.watched) { - pt.cmd.eof = true; - s.core.update(.{ .eof = .{ .pane = pane } }); - return s.takeExits(); - } + if (pt.cmd.watched) return host_io.commandEof(s.core, &s.ptys, pane, s, closeWatched); // Unwatched, a command's exit is read here, as its end. const unwatched = if (s.core.panes[pane]) |pn| pn.command != null else false; var status: ?u8 = null; @@ -715,21 +711,13 @@ pub const Session = struct { s.core.update(.{ .eof = .{ .pane = pane } }); } - /// Each watched child that exited: reaped, the core told, and its pty - /// closed if its end of file came first. + /// The command panes' exits, told once their output is in (host_io). fn takeExits(s: *Session) void { - while (host_io.takeExited()) |pid| for (&s.ptys) |*pt| { - if (pt.fd < 0 or pt.pid != pid or pt.cmd.exited) continue; - pt.cmd.exit((host_io.reapExited(pid) orelse break).status); - pt.pid = 0; // reaped: no signal or retire may reach whoever gets it next - break; - }; - for (&s.ptys, 0..) |*pt, id| { - if (pt.fd < 0 or !pt.cmd.due()) continue; - pt.cmd.told = true; - s.core.update(.{ .exited = .{ .pane = @intCast(id), .status = pt.cmd.status } }); - if (pt.cmd.eof) s.closePty(@intCast(id)); - } + host_io.takeExits(s.core, &s.ptys, s, closeWatched); + } + + fn closeWatched(s: *Session, id: usize) void { + s.closePty(@intCast(id)); } fn harvest(s: *Session) void { diff --git a/src/exec.zig b/src/exec.zig index dfef2dfd..cd2b4089 100644 --- a/src/exec.zig +++ b/src/exec.zig @@ -544,9 +544,11 @@ fn runCommand(p: *Pardes, from: usize, line: []const u8) ?usize { } const src = p.panes[from] orelse return null; const dir = Pardes.paneDir(src); - const reuse: ?usize = if (src.command != null and src.command_done) from else for (p.panes, 0..) |slot, i| { + // Not one whose pty a job it left behind still prints to: reusing it + // would hang that job up. + const reuse: ?usize = if (src.command != null and src.command_done and !src.command_pty) from else for (p.panes, 0..) |slot, i| { const other = slot orelse continue; - if (other.command != null and other.command_done and std.mem.eql(u8, other.cwdSlice(), dir)) break i; + if (other.command != null and other.command_done and !other.command_pty and std.mem.eql(u8, other.cwdSlice(), dir)) break i; } else null; if (reuse) |id| { const pane = p.panes[id].?; @@ -558,13 +560,15 @@ fn runCommand(p: *Pardes, from: usize, line: []const u8) ?usize { pane.command = owned; pane.command_done = false; pane.command_status = null; + pane.command_pty = true; pane.body.mode = .tty; // What the last program left the emulator in goes first: the // alternate screen left (only if it is there: leaving restores a // saved cursor), mouse reports and bracketed paste off, the cursor - // shown, then a soft reset (DECSTR) for the rest. + // shown, colours reset. (Not DECSTR: ghostty's stream does not + // implement it.) if (panes.Terminal.onAlternateScreen(pane)) panes.Terminal.feedOutput(p, pane, "\x1b[?1049l"); - panes.Terminal.feedOutput(p, pane, "\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?2004l\x1b[?25h\x1b[!p"); + panes.Terminal.feedOutput(p, pane, "\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?2004l\x1b[?25h\x1b[0m"); echoCommand(p, pane, line); p.emit(.{ .spawn = .{ .pane = @intCast(id), .serial = pane.serial, .cwd = .from(pane.cwdSlice()) } }); noteRun(p, pane, "run", line); diff --git a/src/gui/gui.zig b/src/gui/gui.zig index 46bd5ffc..0cfe80f5 100644 --- a/src/gui/gui.zig +++ b/src/gui/gui.zig @@ -3878,29 +3878,20 @@ const Shell = struct { break :vt v; }; - /// Each watched child that exited: reaped, the core told, and its pty - /// closed if its end of file came first. + /// The command panes' exits, told once their output is in (host_io). fn takeExits(s: *Shell) void { - while (host_io.takeExited()) |pid| for (s.ptys) |*slot| { - const pt = if (slot.*) |*pt| pt else continue; - if (pt.pid != pid or pt.cmd.exited) continue; - pt.cmd.exit((host_io.reapExited(pid) orelse break).status); - pt.pid = 0; // reaped: no signal or retire may reach whoever gets it next - break; - }; - for (s.ptys, 0..) |*slot, id| { - const pt = if (slot.*) |*pt| pt else continue; - if (!pt.cmd.due()) continue; - pt.cmd.told = true; - s.core.update(.{ .exited = .{ .pane = @intCast(id), .status = pt.cmd.status } }); - if (pt.cmd.eof) s.closePty(@intCast(id)); - s.saw_event = true; - } + host_io.takeExits(s.core, s.ptys, s, closeWatched); + } + + fn closeWatched(s: *Shell, id: usize) void { + s.closePty(@intCast(id)); + s.saw_event = true; } fn drainQueue(s: *Shell) void { s.reconcilePtys(); - s.takeExits(); + // After this batch's output, which an exit is told behind. + defer s.takeExits(); var msgs = s.queue.take(); var check_files = false; for (msgs.slice()) |m| switch (m) { @@ -3928,9 +3919,7 @@ const Shell = struct { // closing it would hang up one that runs on without it. // Its exit, if it came first, is told now its output is in. if (pt.cmd.watched) { - pt.cmd.eof = true; - s.core.update(.{ .eof = .{ .pane = e.pane } }); - s.takeExits(); + host_io.commandEof(s.core, s.ptys, e.pane, s, closeWatched); break :eof; } // Unwatched, a command's exit is read here, as its end. @@ -4218,7 +4207,7 @@ fn spawnPane(ctx: ?*anyopaque, pane: u8, cwd: []const u8) void { 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); const command = s.core.panes[pane].?.command != null; - const pt: Pty = .{ .fd = child.file.handle, .pid = child.pid, .serial = s.core.panes[pane].?.serial, .cmd = .{ .watched = command and host_io.watchExit(child.pid) } }; + const pt: Pty = .{ .fd = child.file.handle, .pid = child.pid, .serial = s.core.panes[pane].?.serial, .cmd = .{ .watched = command and host_io.watchExit(child.pid), .fd = child.file.handle } }; s.ptys[pane] = pt; var lbuf: [pardes.memory.limits.host_path_cap + 1]u8 = undefined; if (host_io.shellCwd(pt.pid, &lbuf)) |wd| s.core.setCwd(pane, wd); diff --git a/src/host_io.zig b/src/host_io.zig index 68d35a9c..340513df 100644 --- a/src/host_io.zig +++ b/src/host_io.zig @@ -1426,12 +1426,6 @@ fn watchChild(pid: libc.pid_t) void { _ = libc.nanosleep(&ts, null); } wakeForExit(); - // Once more a little later: a job the command left holding its pty - // may keep its end of file from ever coming, and the host tells the - // exit then (`CommandWatch.due`). - const ts: libc.timespec = .{ .sec = 0, .nsec = (CommandWatch.grace_ms + 10) * std.time.ns_per_ms }; - _ = libc.nanosleep(&ts, null); - wakeForExit(); } fn wakeForExit() void { @@ -1440,33 +1434,85 @@ fn wakeForExit() void { if (exit_wake) |wake| wake.f(wake.ctx); } -/// Where a host's command pane is between its child's exit and its pty's -/// end of file. The exit is told once the output before it has been read, -/// at end of file -- the exit is often seen first, with output still in -/// the pty -- or after `grace_ms` without one, a job the command left in -/// the background holding the pty. +/// Where a host's command pane is between its child's exit and the end of +/// its output. The exit is told once the output before it is in: at the +/// pty's end of file; or, when something else still holds the pty (a job +/// the command left in the background), once nothing is waiting to be read +/// -- checked when the exit is seen and again after each chunk of output, +/// with no timer to miss. acme's waitthread tells an exit when it is +/// reaped (acme.c:587-700); a terminal also has output in flight. pub const CommandWatch = struct { - pub const grace_ms = 50; watched: bool = false, + /// The master, to ask whether output is still waiting. + fd: c_int = -1, eof: bool = false, exited: bool = false, told: bool = false, status: ?u8 = null, - at: i64 = 0, - - /// The child is reaped, with this status. - pub fn exit(w: *CommandWatch, status: ?u8) void { - w.exited = true; - w.status = status; - w.at = nowMs(); - } /// Whether the exit is to be told now. pub fn due(w: *const CommandWatch) bool { - return w.exited and !w.told and (w.eof or nowMs() - w.at >= grace_ms); + if (!w.exited or w.told) return false; + if (w.eof) return true; + var pfd = [1]libc.pollfd{.{ .fd = w.fd, .events = libc.POLL.IN, .revents = 0 }}; + if (libc.poll(&pfd, 1, 0) < 0) return true; + // Every holder of its terminal gone: the end of file comes, after + // the rest of the output. + if (pfd[0].revents & libc.POLL.HUP != 0) return false; + return pfd[0].revents & libc.POLL.IN == 0; } }; +/// A host's pty slot, whichever way it keeps it: an optional, or one whose +/// `fd` is -1 when empty. +fn SlotPty(comptime S: type) type { + return switch (@typeInfo(S)) { + .optional => |o| *o.child, + else => *S, + }; +} + +fn slotPty(slot: anytype) ?SlotPty(@TypeOf(slot.*)) { + return switch (@typeInfo(@TypeOf(slot.*))) { + .optional => if (slot.*) |*pt| pt else null, + else => if (slot.fd < 0) null else slot, + }; +} + +/// Each watched child that exited: reaped, and the core told once its +/// output is in, its pty closed (`close(ctx, id)`) if its end of file came +/// first. Every host runs this after an exit's wake and after each chunk +/// of a command's output; `ptys` is its slot array, each slot with a `pid` +/// and a `cmd`. +pub fn takeExits(core: *pardes.Pardes, ptys: anytype, ctx: anytype, comptime close: anytype) void { + while (takeExited()) |pid| for (ptys) |*slot| { + const pt = slotPty(slot) orelse continue; + if (!pt.cmd.watched or pt.pid != pid or pt.cmd.exited) continue; + pt.cmd.status = (reapExited(pid) orelse break).status; + pt.cmd.exited = true; + pt.pid = 0; // reaped: no signal or retire may reach whoever gets it next + break; + }; + for (ptys, 0..) |*slot, id| { + const pt = slotPty(slot) orelse continue; + if (!pt.cmd.watched or !pt.cmd.due()) continue; + pt.cmd.told = true; + const eof = pt.cmd.eof; + core.update(.{ .exited = .{ .pane = @intCast(id), .status = pt.cmd.status } }); + if (eof) close(ctx, id); + } +} + +/// A watched command's pty reached end of file: the core hears its output +/// is over; the pty is closed now if its exit was told, else when it is. +pub fn commandEof(core: *pardes.Pardes, ptys: anytype, id: usize, ctx: anytype, comptime close: anytype) void { + const pt = slotPty(&ptys[id]) orelse return; + pt.cmd.eof = true; + core.update(.{ .eof = .{ .pane = @intCast(id) } }); + if (pt.cmd.told) return close(ctx, id); + takeExits(core, ptys, ctx, close); +} + /// Reaps `pid` if it has exited: its status (null: unknown, or not ours /// to read), or null when it is still running -- a pid a watcher reported /// may since have been reaped by another and taken by a new child. diff --git a/src/macos.zig b/src/macos.zig index d6ceb535..70469618 100644 --- a/src/macos.zig +++ b/src/macos.zig @@ -132,31 +132,22 @@ const Pty = struct { }; /// A watched child exited: wake the host, which takes it (`takeExits`). +/// Called on a watcher thread holding host_io's exit lock, which +/// pardes_deinit takes to clear the wake: safe only because the runtime's +/// wakeup never waits for the main thread (AppDelegate's is a +/// DispatchQueue.main.async); one that did would deadlock the quit. fn wakeForExit(ctx: ?*anyopaque) void { const st: *State = @ptrCast(@alignCast(ctx orelse return)); wake(st); } -/// Each watched child that exited: reaped, the core told, and its pty -/// closed if its end of file came first. -fn takeExits(st: *State) bool { - var did = false; - while (host_io.takeExited()) |pid| for (&st.ptys) |*slot| { - const pt = if (slot.*) |*pt| pt else continue; - if (pt.pid != pid or pt.cmd.exited) continue; - pt.cmd.exit((host_io.reapExited(pid) orelse break).status); - pt.pid = 0; // reaped: no signal or retire may reach whoever gets it next - break; - }; - for (&st.ptys, 0..) |*slot, id| { - const pt = if (slot.*) |*pt| pt else continue; - if (!pt.cmd.due()) continue; - pt.cmd.told = true; - st.core.update(.{ .exited = .{ .pane = @intCast(id), .status = pt.cmd.status } }); - if (pt.cmd.eof) reap(st, @intCast(id)); - did = true; - } - return did; +/// The command panes' exits, told once their output is in (host_io). +fn takeExits(st: *State) void { + host_io.takeExits(st.core, &st.ptys, st, closeWatched); +} + +fn closeWatched(st: *State, id: usize) void { + reap(st, @intCast(id)); } const WatchedFile = struct { @@ -1302,7 +1293,9 @@ export fn pardes_watch_changed(pane: u8, generation: u32) void { fn drainInbox(st: *State) bool { var batch = st.inbox.take(st.io); - var did = takeExits(st) or batch.len > 0; + // After this batch's output, which an exit is told behind. + defer takeExits(st); + var did = batch.len > 0; for (batch.slice()) |msg| { defer msg.free(st.gpa); switch (msg) { @@ -1318,9 +1311,7 @@ fn drainInbox(st: *State) bool { // closing it would hang up one that runs on without it. // Its exit, if it came first, is told now its output is in. if (pt.cmd.watched) { - pt.cmd.eof = true; - st.core.update(.{ .eof = .{ .pane = e.pane } }); - _ = takeExits(st); + host_io.commandEof(st.core, &st.ptys, e.pane, st, closeWatched); continue; } // Unwatched, a command's exit is read here, as its end. @@ -2460,7 +2451,7 @@ fn spawnShell(ctx: ?*anyopaque, pane: u8, cwd: []const u8) void { .pid = child.pid, .gen = gen, .reader = .{ .any_future = null, .result = {} }, - .cmd = .{ .watched = (if (core.panes[pane]) |pn| pn.command != null else false) and host_io.watchExit(child.pid) }, + .cmd = .{ .watched = (if (core.panes[pane]) |pn| pn.command != null else false) and host_io.watchExit(child.pid), .fd = child.file.handle }, }; var lbuf: [1024]u8 = undefined; if (host_io.shellCwd(child.pid, &lbuf)) |wd| core.setCwd(pane, wd); diff --git a/src/panes.zig b/src/panes.zig index 9234912f..29fe0cb0 100644 --- a/src/panes.zig +++ b/src/panes.zig @@ -113,6 +113,9 @@ pub const Pane = struct { /// shown `exit ?`). command_done: bool = false, command_status: ?u8 = null, + /// Its pty is still open: a job the command left in the background may + /// be printing to it, and a reuse would hang it up. + command_pty: bool = false, /// The shell a terminal runs when it is not the configured one (`Tty /// fish`), owned. shell: ?[]u8 = null, diff --git a/src/pardes.zig b/src/pardes.zig index 9bf1c5f4..64c05fff 100644 --- a/src/pardes.zig +++ b/src/pardes.zig @@ -1632,10 +1632,16 @@ test "a command pane shows how its command ended, and the next command there run try std.testing.expect(other != dst); while (p.nextEffect()) |_| {} p.update(.{ .output = .{ .pane = @intCast(dst), .bytes = "compiled\r\n" } }); - // Its output ending is not its end: that is its child's exit. - p.update(.{ .eof = .{ .pane = @intCast(dst) } }); - try std.testing.expect(p.panes[dst] == pane and !pane.command_done); + // Its exit comes first, a job it left still printing: done, and not + // the next command's until its pty closes. p.update(.{ .exited = .{ .pane = @intCast(dst), .status = 2 } }); + try std.testing.expect(pane.command_done and pane.command_pty); + const busy = exec.execute(p, 1, "true") orelse return error.NoCommandPane; + try std.testing.expect(busy != dst); + while (p.nextEffect()) |_| {} + // Its output ending: now it may be reused. + p.update(.{ .eof = .{ .pane = @intCast(dst) } }); + try std.testing.expect(p.panes[dst] == pane and !pane.command_pty); try std.testing.expect(p.panes[dst] == pane); try std.testing.expect(pane.command_done); try std.testing.expect(std.mem.endsWith(u8, try tagline.tagPrefix(p, pane), "(make -j8) exit 2")); @@ -4536,6 +4542,7 @@ pub const Pardes = struct { errdefer p.gpa.free(owned); const pane = try panes.Terminal.create(p.gpa, p.screen_w, p.screen_h); pane.command = owned; + pane.command_pty = true; pane.body.mode = .tty; // Its directory is the one it was run for, and says which of the // directory's commands reuse it; the child's own cd does not move it. @@ -5118,6 +5125,7 @@ pub const Pardes = struct { }, .eof => |e| if (p.panes[e.pane]) |pane| if (pane.command != null) { // Output is over; the command is over when its child exits. + pane.command_pty = false; } else p.removePane(e.pane, null) catch |err| { pane.body.mode = .normal; p.reportError(e.pane, "terminal exited; Del retries close", err); diff --git a/src/tty/tty.zig b/src/tty/tty.zig index 14417cd0..f8f8cdc5 100644 --- a/src/tty/tty.zig +++ b/src/tty/tty.zig @@ -808,7 +808,6 @@ fn localSession( frames: while (!core.quit) { pardes.turn.restoreSettled(); try core.pump(host); - sh.takeExits(); if (core.takeRestore()) |rp| blk: { const bytes = filesystem.readRestore(gpa, rp, core.settings.dump_dir.get()) catch |err| { core.reportError(core.active, "Restore", err); @@ -988,6 +987,9 @@ const Shell = struct { batch += 1; } if (motion) |m| _ = s.apply(m); + // An exit's wake (a .nop) or output an exit waited behind: told + // here, inside the step, so the frame after it shows it. + s.takeExits(); s.reloadWatched(); } @@ -1015,6 +1017,8 @@ const Shell = struct { if (s.gens[pr.id] == pr.gen) core.update(.{ .output = .{ .pane = @intCast(pr.id), .bytes = pr.bytes } }); s.gpa.free(pr.bytes); + // An exit waiting behind this output may be told now. + s.takeExits(); return true; }, .pty_eof => |e| if (s.gens[e.id] == e.gen) { @@ -1024,9 +1028,7 @@ const Shell = struct { // closing it would hang up one that runs on without it. // Its exit, if it came first, is told now its output is in. if (pt.cmd.watched) { - pt.cmd.eof = true; - core.update(.{ .eof = .{ .pane = @intCast(e.id) } }); - s.takeExits(); + host_io.commandEof(core, &s.ptys, e.id, s, closeWatched); return true; } _ = libc.close(pt.file.handle); @@ -1225,7 +1227,7 @@ const Shell = struct { 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); const command = if (s.core.panes[pane]) |pn| pn.command != null else false; - s.ptys[pane] = .{ .file = child.file, .pid = child.pid, .reader = .{ .any_future = null, .result = {} }, .cmd = .{ .watched = command and host_io.watchExit(child.pid) } }; + s.ptys[pane] = .{ .file = child.file, .pid = child.pid, .reader = .{ .any_future = null, .result = {} }, .cmd = .{ .watched = command and host_io.watchExit(child.pid), .fd = child.file.handle } }; var lbuf: [pardes.memory.limits.host_path_cap + 1]u8 = undefined; if (host_io.shellCwd(child.pid, &lbuf)) |wd| s.core.setCwd(pane, wd); if (s.threads_ok) { @@ -1274,26 +1276,15 @@ const Shell = struct { return host_io.ttyTaken(pt.pid, pt.file.handle); } - /// Each watched child that exited: reaped, the core told, and its pty - /// closed if its end of file came first. + /// The command panes' exits, told once their output is in (host_io). fn takeExits(s: *@This()) void { - while (host_io.takeExited()) |pid| for (&s.ptys) |*slot| { - const pt = if (slot.*) |*pt| pt else continue; - if (pt.pid != pid or pt.cmd.exited) continue; - pt.cmd.exit((host_io.reapExited(pid) orelse break).status); - pt.pid = 0; // reaped: no signal or retire may reach whoever gets it next - break; - }; - for (&s.ptys, 0..) |*slot, id| { - const pt = if (slot.*) |*pt| pt else continue; - if (!pt.cmd.due()) continue; - pt.cmd.told = true; - s.core.update(.{ .exited = .{ .pane = @intCast(id), .status = pt.cmd.status } }); - if (pt.cmd.eof) { - _ = libc.close(pt.file.handle); - slot.* = null; - } - } + host_io.takeExits(s.core, &s.ptys, s, closeWatched); + } + + fn closeWatched(s: *@This(), id: usize) void { + const pt = &(s.ptys[id] orelse return); + _ = libc.close(pt.file.handle); + s.ptys[id] = null; } fn killJob(ctx: ?*anyopaque, pane: u8) bool { -- cgit v1.3