Move child exit handling logic to apprt thread (#7705)

Fixes #7500
Supersedes #7582

This commit moves the child exit handling logic from the IO thead to the
apprt thread. The IO thread now only sends a `child_exited` message to
the apprt thread with metadata about the exit conditions (exit code,
runtime).

From there, the apprt thread can handle the exit situation however is
necessary. This commit doesn't change the behavior but it does fix the
issue #7500. The behavior is: exit immediately, show abnormal exit
message, wait for user input, etc.

This also gets us closer to #7649.

src/Surface.zig

@@ -237,6 +237,7 @@ const DerivedConfig = struct {
     /// For docs for these, see the associated config they are derived from.
     original_font_size: f32,
     keybind: configpkg.Keybinds,
+    abnormal_command_exit_runtime_ms: u32,
     clipboard_read: configpkg.ClipboardAccess,
     clipboard_write: configpkg.ClipboardAccess,
     clipboard_trim_trailing_spaces: bool,
@@ -255,6 +256,7 @@ const DerivedConfig = struct {
     macos_option_as_alt: ?configpkg.OptionAsAlt,
     selection_clear_on_typing: bool,
     vt_kam_allowed: bool,
+    wait_after_command: bool,
     window_padding_top: u32,
     window_padding_bottom: u32,
     window_padding_left: u32,
@@ -301,6 +303,7 @@ const DerivedConfig = struct {
         return .{
             .original_font_size = config.@"font-size",
             .keybind = try config.keybind.clone(alloc),
+            .abnormal_command_exit_runtime_ms = config.@"abnormal-command-exit-runtime",
             .clipboard_read = config.@"clipboard-read",
             .clipboard_write = config.@"clipboard-write",
             .clipboard_trim_trailing_spaces = config.@"clipboard-trim-trailing-spaces",
@@ -319,6 +322,7 @@ const DerivedConfig = struct {
             .macos_option_as_alt = config.@"macos-option-as-alt",
             .selection_clear_on_typing = config.@"selection-clear-on-typing",
             .vt_kam_allowed = config.@"vt-kam-allowed",
+            .wait_after_command = config.@"wait-after-command",
             .window_padding_top = config.@"window-padding-y".top_left,
             .window_padding_bottom = config.@"window-padding-y".bottom_right,
             .window_padding_left = config.@"window-padding-x".top_left,
@@ -911,11 +915,7 @@ pub fn handleMessage(self: *Surface, msg: Message) !void {
 
         .close => self.close(),
 
-        // Close without confirmation.
-        .child_exited => {
-            self.child_exited = true;
-            self.close();
-        },
+        .child_exited => |v| self.childExited(v),
 
         .desktop_notification => |notification| {
             if (!self.config.desktop_notifications) {
@@ -948,6 +948,136 @@ pub fn handleMessage(self: *Surface, msg: Message) !void {
     }
 }
 
+fn childExited(self: *Surface, info: apprt.surface.Message.ChildExited) void {
+    // Mark our flag that we exited immediately
+    self.child_exited = true;
+
+    // If our runtime was below some threshold then we assume that this
+    // was an abnormal exit and we show an error message.
+    if (info.runtime_ms <= self.config.abnormal_command_exit_runtime_ms) runtime: {
+        // On macOS, our exit code detection doesn't work, possibly
+        // because of our `login` wrapper. More investigation required.
+        if (comptime builtin.target.os.tag.isDarwin()) break :runtime;
+
+        // If the exit code is 0 then we it was a good exit.
+        if (info.exit_code == 0) break :runtime;
+        log.warn("abnormal process exit detected, showing error message", .{});
+
+        // Update our terminal to note the abnormal exit. In the future we
+        // may want the apprt to handle this to show some native GUI element.
+        self.childExitedAbnormally(info) catch |err| {
+            log.err("error handling abnormal child exit err={}", .{err});
+            return;
+        };
+
+        return;
+    }
+
+    // We output a message so that the user knows whats going on and
+    // doesn't think their terminal just froze. We show this unconditionally
+    // on close even if `wait_after_command` is false and the surface closes
+    // immediately because if a user does an `undo` to restore a closed
+    // surface then they will see this message and know the process has
+    // completed.
+    terminal: {
+        self.renderer_state.mutex.lock();
+        defer self.renderer_state.mutex.unlock();
+        const t: *terminal.Terminal = self.renderer_state.terminal;
+        t.carriageReturn();
+        t.linefeed() catch break :terminal;
+        t.printString("Process exited. Press any key to close the terminal.") catch
+            break :terminal;
+        t.modes.set(.cursor_visible, false);
+    }
+
+    // Waiting after command we stop here. The terminal is updated, our
+    // state is updated, and now its up to the user to decide what to do.
+    if (self.config.wait_after_command) return;
+
+    // If we aren't waiting after the command, then we exit immediately
+    // with no confirmation.
+    self.close();
+}
+
+/// Called when the child process exited abnormally.
+fn childExitedAbnormally(
+    self: *Surface,
+    info: apprt.surface.Message.ChildExited,
+) !void {
+    var arena = ArenaAllocator.init(self.alloc);
+    defer arena.deinit();
+    const alloc = arena.allocator();
+
+    // Build up our command for the error message
+    const command = try std.mem.join(alloc, " ", switch (self.io.backend) {
+        .exec => |*exec| exec.subprocess.args,
+    });
+    const runtime_str = try std.fmt.allocPrint(alloc, "{d} ms", .{info.runtime_ms});
+
+    self.renderer_state.mutex.lock();
+    defer self.renderer_state.mutex.unlock();
+    const t: *terminal.Terminal = self.renderer_state.terminal;
+
+    // No matter what move the cursor back to the column 0.
+    t.carriageReturn();
+
+    // Reset styles
+    try t.setAttribute(.{ .unset = {} });
+
+    // If there is data in the viewport, we want to scroll down
+    // a little bit and write a horizontal rule before writing
+    // our message. This lets the use see the error message the
+    // command may have output.
+    const viewport_str = try t.plainString(alloc);
+    if (viewport_str.len > 0) {
+        try t.linefeed();
+        for (0..t.cols) |_| try t.print(0x2501);
+        t.carriageReturn();
+        try t.linefeed();
+        try t.linefeed();
+    }
+
+    // Output our error message
+    try t.setAttribute(.{ .@"8_fg" = .bright_red });
+    try t.setAttribute(.{ .bold = {} });
+    try t.printString("Ghostty failed to launch the requested command:");
+    try t.setAttribute(.{ .unset = {} });
+
+    t.carriageReturn();
+    try t.linefeed();
+    try t.linefeed();
+    try t.printString(command);
+    try t.setAttribute(.{ .unset = {} });
+
+    t.carriageReturn();
+    try t.linefeed();
+    try t.linefeed();
+    try t.printString("Runtime: ");
+    try t.setAttribute(.{ .@"8_fg" = .red });
+    try t.printString(runtime_str);
+    try t.setAttribute(.{ .unset = {} });
+
+    // We don't print this on macOS because the exit code is always 0
+    // due to the way we launch the process.
+    if (comptime !builtin.target.os.tag.isDarwin()) {
+        const exit_code_str = try std.fmt.allocPrint(alloc, "{d}", .{info.exit_code});
+        t.carriageReturn();
+        try t.linefeed();
+        try t.printString("Exit Code: ");
+        try t.setAttribute(.{ .@"8_fg" = .red });
+        try t.printString(exit_code_str);
+        try t.setAttribute(.{ .unset = {} });
+    }
+
+    t.carriageReturn();
+    try t.linefeed();
+    try t.linefeed();
+    try t.printString("Press any key to close the window.");
+
+    // Hide the cursor
+    t.modes.set(.cursor_visible, false);
+}
+
 /// Called when the terminal detects there is a password input prompt.
 fn passwordInput(self: *Surface, v: bool) !void {
     {
@@ -1953,6 +2083,14 @@ pub fn keyCallback(
         if (self.io.terminal.modes.get(.disable_keyboard)) return .consumed;
     }
 
+    // If our process is exited and we press a key then we close the
+    // surface. We may want to eventually move this to the apprt rather
+    // than in core.
+    if (self.child_exited and event.action == .press) {
+        self.close();
+        return .closed;
+    }
+
     // If this input event has text, then we hide the mouse if configured.
     // We only do this on pressed events to avoid hiding the mouse when we
     // change focus due to a keybinding (i.e. switching tabs).

src/apprt/surface.zig

@@ -43,8 +43,9 @@ pub const Message = union(enum) {
     close: void,
 
     /// The child process running in the surface has exited. This may trigger
-    /// a surface close, it may not.
-    child_exited: void,
+    /// a surface close, it may not. Additional details about the child
+    /// command are given in the `ChildExited` struct.
+    child_exited: ChildExited,
 
     /// Show a desktop notification.
     desktop_notification: struct {
@@ -89,6 +90,11 @@ pub const Message = union(enum) {
 
         // This enum is a placeholder for future title styles.
     };
+
+    pub const ChildExited = struct {
+        exit_code: u32,
+        runtime_ms: u64,
+    };
 };
 
 /// A surface mailbox.

src/termio/Exec.zig

@@ -10,6 +10,7 @@ const Allocator = std.mem.Allocator;
 const ArenaAllocator = std.heap.ArenaAllocator;
 const posix = std.posix;
 const xev = @import("../global.zig").xev;
+const apprt = @import("../apprt.zig");
 const build_config = @import("../build_config.zig");
 const configpkg = @import("../config.zig");
 const crash = @import("../crash/main.zig");
@@ -153,8 +154,6 @@ pub fn threadEnter(
     // Setup our threadata backend state to be our own
     td.backend = .{ .exec = .{
         .start = process_start,
-        .abnormal_runtime_threshold_ms = io.config.abnormal_runtime_threshold_ms,
-        .wait_after_command = io.config.wait_after_command,
         .write_stream = stream,
         .process = process,
         .read_thread = read_thread,
@@ -273,83 +272,6 @@ pub fn resize(
     return try self.subprocess.resize(grid_size, screen_size);
 }
 
-/// Called when the child process exited abnormally but before the surface
-/// is notified.
-pub fn childExitedAbnormally(
-    self: *Exec,
-    gpa: Allocator,
-    t: *terminal.Terminal,
-    exit_code: u32,
-    runtime_ms: u64,
-) !void {
-    var arena = ArenaAllocator.init(gpa);
-    defer arena.deinit();
-    const alloc = arena.allocator();
-
-    // Build up our command for the error message
-    const command = try std.mem.join(alloc, " ", self.subprocess.args);
-    const runtime_str = try std.fmt.allocPrint(alloc, "{d} ms", .{runtime_ms});
-
-    // No matter what move the cursor back to the column 0.
-    t.carriageReturn();
-
-    // Reset styles
-    try t.setAttribute(.{ .unset = {} });
-
-    // If there is data in the viewport, we want to scroll down
-    // a little bit and write a horizontal rule before writing
-    // our message. This lets the use see the error message the
-    // command may have output.
-    const viewport_str = try t.plainString(alloc);
-    if (viewport_str.len > 0) {
-        try t.linefeed();
-        for (0..t.cols) |_| try t.print(0x2501);
-        t.carriageReturn();
-        try t.linefeed();
-        try t.linefeed();
-    }
-
-    // Output our error message
-    try t.setAttribute(.{ .@"8_fg" = .bright_red });
-    try t.setAttribute(.{ .bold = {} });
-    try t.printString("Ghostty failed to launch the requested command:");
-    try t.setAttribute(.{ .unset = {} });
-
-    t.carriageReturn();
-    try t.linefeed();
-    try t.linefeed();
-    try t.printString(command);
-    try t.setAttribute(.{ .unset = {} });
-
-    t.carriageReturn();
-    try t.linefeed();
-    try t.linefeed();
-    try t.printString("Runtime: ");
-    try t.setAttribute(.{ .@"8_fg" = .red });
-    try t.printString(runtime_str);
-    try t.setAttribute(.{ .unset = {} });
-
-    // We don't print this on macOS because the exit code is always 0
-    // due to the way we launch the process.
-    if (comptime !builtin.target.os.tag.isDarwin()) {
-        const exit_code_str = try std.fmt.allocPrint(alloc, "{d}", .{exit_code});
-        t.carriageReturn();
-        try t.linefeed();
-        try t.printString("Exit Code: ");
-        try t.setAttribute(.{ .@"8_fg" = .red });
-        try t.printString(exit_code_str);
-        try t.setAttribute(.{ .unset = {} });
-    }
-
-    t.carriageReturn();
-    try t.linefeed();
-    try t.linefeed();
-    try t.printString("Press any key to close the window.");
-
-    // Hide the cursor
-    t.modes.set(.cursor_visible, false);
-}
-
 /// This outputs an error message when exec failed and we are the
 /// child process. This returns so the caller should probably exit
 /// after calling this.
@@ -386,63 +308,13 @@ fn processExitCommon(td: *termio.Termio.ThreadData, exit_code: u32) void {
         .{ exit_code, runtime_ms orelse 0 },
     );
 
-    // If our runtime was below some threshold then we assume that this
-    // was an abnormal exit and we show an error message.
-    if (runtime_ms) |runtime| runtime: {
-        // On macOS, our exit code detection doesn't work, possibly
-        // because of our `login` wrapper. More investigation required.
-        if (comptime !builtin.target.os.tag.isDarwin()) {
-            // If our exit code is zero, then the command was successful
-            // and we don't ever consider it abnormal.
-            if (exit_code == 0) break :runtime;
-        }
-
-        // Our runtime always has to be under the threshold to be
-        // considered abnormal. This is because a user can always
-        // manually do something like `exit 1` in their shell to
-        // force the exit code to be non-zero. We only want to detect
-        // abnormal exits that happen so quickly the user can't react.
-        if (runtime > execdata.abnormal_runtime_threshold_ms) break :runtime;
-        log.warn("abnormal process exit detected, showing error message", .{});
-
-        // Notify our main writer thread which has access to more
-        // information so it can show a better error message.
-        td.mailbox.send(.{
-            .child_exited_abnormally = .{
-                .exit_code = exit_code,
-                .runtime_ms = runtime,
-            },
-        }, null);
-        td.mailbox.notify();
-
-        return;
-    }
-
-    // We output a message so that the user knows whats going on and
-    // doesn't think their terminal just froze. We show this unconditionally
-    // on close even if `wait_after_command` is false and the surface closes
-    // immediately because if a user does an `undo` to restore a closed
-    // surface then they will see this message and know the process has
-    // completed.
-    terminal: {
-        td.renderer_state.mutex.lock();
-        defer td.renderer_state.mutex.unlock();
-        const t = td.renderer_state.terminal;
-        t.carriageReturn();
-        t.linefeed() catch break :terminal;
-        t.printString("Process exited. Press any key to close the terminal.") catch
-            break :terminal;
-        t.modes.set(.cursor_visible, false);
-    }
-
-    // If we're purposely waiting then we just return since the process
-    // exited flag is set to true. This allows the terminal window to remain
-    // open.
-    if (execdata.wait_after_command) return;
-
-    // Notify our surface we want to close
+    // We always notify the surface immediately that the child has
+    // exited and some metadata about the exit.
     _ = td.surface_mailbox.push(.{
-        .child_exited = {},
+        .child_exited = .{
+            .exit_code = exit_code,
+            .runtime_ms = runtime_ms orelse 0,
+        },
     }, .{ .forever = {} });
 }
 
@@ -563,14 +435,8 @@ pub fn queueWrite(
     _ = self;
     const exec = &td.backend.exec;
 
-    // If our process is exited then we send our surface a message
-    // about it but we don't queue any more writes.
-    if (exec.exited) {
-        _ = td.surface_mailbox.push(.{
-            .child_exited = {},
-        }, .{ .forever = {} });
-        return;
-    }
+    // If our process is exited then we don't send any more writes.
+    if (exec.exited) return;
 
     // We go through and chunk the data if necessary to fit into
     // our cached buffers that we can queue to the stream.
@@ -658,17 +524,6 @@ pub const ThreadData = struct {
     start: std.time.Instant,
     exited: bool = false,
 
-    /// The number of milliseconds below which we consider a process
-    /// exit to be abnormal. This is used to show an error message
-    /// when the process exits too quickly.
-    abnormal_runtime_threshold_ms: u32,
-
-    /// If true, do not immediately send a child exited message to the
-    /// surface to close the surface when the command exits. If this is
-    /// false we'll show a process exited message and wait for user input
-    /// to close the surface.
-    wait_after_command: bool,
-
     /// The data stream is the main IO for the pty.
     write_stream: xev.Stream,
 

src/termio/Termio.zig

@@ -168,8 +168,6 @@ pub const DerivedConfig = struct {
     foreground: configpkg.Config.Color,
     background: configpkg.Config.Color,
     osc_color_report_format: configpkg.Config.OSCColorReportFormat,
-    abnormal_runtime_threshold_ms: u32,
-    wait_after_command: bool,
     enquiry_response: []const u8,
 
     pub fn init(
@@ -190,8 +188,6 @@ pub const DerivedConfig = struct {
             .foreground = config.foreground,
             .background = config.background,
             .osc_color_report_format = config.@"osc-color-report-format",
-            .abnormal_runtime_threshold_ms = config.@"abnormal-command-exit-runtime",
-            .wait_after_command = config.@"wait-after-command",
             .enquiry_response = try alloc.dupe(u8, config.@"enquiry-response"),
 
             // This has to be last so that we copy AFTER the arena allocations
@@ -660,15 +656,6 @@ pub fn jumpToPrompt(self: *Termio, delta: isize) !void {
     try self.renderer_wakeup.notify();
 }
 
-/// Called when the child process exited abnormally but before
-/// the surface is notified.
-pub fn childExitedAbnormally(self: *Termio, exit_code: u32, runtime_ms: u64) !void {
-    self.renderer_state.mutex.lock();
-    defer self.renderer_state.mutex.unlock();
-    const t = self.renderer_state.terminal;
-    try self.backend.childExitedAbnormally(self.alloc, t, exit_code, runtime_ms);
-}
-
 /// Called when focus is gained or lost (when focus events are enabled)
 pub fn focusGained(self: *Termio, td: *ThreadData, focused: bool) !void {
     self.renderer_state.mutex.lock();

src/termio/Thread.zig

@@ -311,7 +311,6 @@ fn drainMailbox(
             .jump_to_prompt => |v| try io.jumpToPrompt(v),
             .start_synchronized_output => self.startSynchronizedOutput(cb),
             .linefeed_mode => |v| self.flags.linefeed_mode = v,
-            .child_exited_abnormally => |v| try io.childExitedAbnormally(v.exit_code, v.runtime_ms),
             .focused => |v| try io.focusGained(data, v),
             .write_small => |v| try io.queueWrite(
                 data,

src/termio/backend.zig

@@ -122,11 +122,7 @@ pub const ThreadData = union(Kind) {
     }
 
     pub fn changeConfig(self: *ThreadData, config: *termio.DerivedConfig) void {
-        switch (self.*) {
-            .exec => |*exec| {
-                exec.abnormal_runtime_threshold_ms = config.abnormal_runtime_threshold_ms;
-                exec.wait_after_command = config.wait_after_command;
-            },
-        }
+        _ = self;
+        _ = config;
     }
 };

src/termio/message.zig

@@ -1,6 +1,7 @@
 const std = @import("std");
 const assert = std.debug.assert;
 const Allocator = std.mem.Allocator;
+const apprt = @import("../apprt.zig");
 const renderer = @import("../renderer.zig");
 const terminal = @import("../terminal/main.zig");
 const termio = @import("../termio.zig");
@@ -58,15 +59,6 @@ pub const Message = union(enum) {
     /// Enable or disable linefeed mode (mode 20).
     linefeed_mode: bool,
 
-    /// The child exited abnormally. The termio state is marked
-    /// as process exited but the surface hasn't been notified to
-    /// close because termio can use this to update the terminal
-    /// with an error message.
-    child_exited_abnormally: struct {
-        exit_code: u32,
-        runtime_ms: u64,
-    },
-
     /// The surface gained or lost focus.
     focused: bool,