From 451506ab5ad582f939465cf69e762edfd0248cff Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Sun, 30 Nov 2025 10:55:14 -0500 Subject: [PATCH 1/7] macos: graceful child process termination Our previous application termination path (e.g. Cmd-Q) ultimately led to Surface.deinit, which uses a detached Task to free its Ghostty surface. That didn't give the child processes (e.g. bash) a chance to receive a SIGHUP before the app completely exits, potentially resulting in list shell history and un-run SIGHUP handlers. This change implements a more graceful termination sequence that synchronously frees all of our surfaces (via ghostty_surface_free) and then delays application termination by a short amount (currently 100ms) to give the child processes some time to perform their cleanup work. All of the existing close/undo/termination behaviors remain unchanged (e.g. confirmation, quit-on-last-window-closed, etc.). Verified using: #!/bin/bash trap 'echo "SIGHUP at $(date)" >> /tmp/ghostty-sigup.log' SIGHUP echo "Waiting for SIGHUP... " read -p "Press Cmd+Q to quit Ghostty..." --- macos/Sources/App/macOS/AppDelegate.swift | 38 ++++++++++++++----- .../Terminal/BaseTerminalController.swift | 14 +++++++ .../Terminal/TerminalController.swift | 7 ++++ 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index a971df9ba..a3bc7110f 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -362,14 +362,33 @@ class AppDelegate: NSObject, return derivedConfig.shouldQuitAfterLastWindowClosed } + /// Initiates graceful application termination. + private func terminateGracefully() -> NSApplication.TerminateReply { + /// Free our surfaces synchronously, ensuring SIGHUP signals are sent to all + /// child processes (e.g., bash) so they can clean up before the app exits. + TerminalController.freeAllSurfaces() + + if !quickController.surfaceTree.isEmpty { + quickController.freeSurfaces() + } + + // Schedule termination after a brief delay to allow child processes + // to handle SIGHUP and complete cleanup (e.g., bash saving history). + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + NSApp.reply(toApplicationShouldTerminate: true) + } + + return .terminateLater + } + func applicationShouldTerminate(_ sender: NSApplication) -> NSApplication.TerminateReply { let windows = NSApplication.shared.windows - if windows.isEmpty { return .terminateNow } + if windows.isEmpty { return terminateGracefully() } // If we've already accepted to install an update, then we don't need to // confirm quit. The user is already expecting the update to happen. if updateController.isInstalling { - return .terminateNow + return terminateGracefully() } // This probably isn't fully safe. The isEmpty check above is aspirational, it doesn't @@ -380,7 +399,7 @@ class AppDelegate: NSObject, // here because I don't want to remove it in a patch release cycle but we should // target removing it soon. if (windows.allSatisfy { !$0.isVisible }) { - return .terminateNow + return terminateGracefully() } // If the user is shutting down, restarting, or logging out, we don't confirm quit. @@ -393,8 +412,7 @@ class AppDelegate: NSObject, if let why = event.attributeDescriptor(forKeyword: keyword) { switch why.typeCodeValue { case kAEShutDown, kAERestart, kAEReallyLogOut: - return .terminateNow - + return terminateGracefully() default: break } @@ -402,7 +420,7 @@ class AppDelegate: NSObject, } // If our app says we don't need to confirm, we can exit now. - if !ghostty.needsConfirmQuit { return .terminateNow } + if !ghostty.needsConfirmQuit { return terminateGracefully() } return terminate() } @@ -1301,7 +1319,7 @@ extension AppDelegate { .filter { !$0.windowCanBeClosedWithoutConfirmation() } guard !controllersNeedConfirmation.isEmpty else { - return .terminateNow + return terminateGracefully() } if controllersNeedConfirmation.count == 1 { @@ -1313,7 +1331,7 @@ extension AppDelegate { ) if [.OK, .alertFirstButtonReturn].contains(response) { - await NSApp.reply(toApplicationShouldTerminate: true) + await MainActor.run { _ = self.terminateGracefully() } } else { await NSApp.reply(toApplicationShouldTerminate: false) } @@ -1334,7 +1352,7 @@ extension AppDelegate { reviewWindows(controllersNeedConfirmation) return .terminateLater case .alertSecondButtonReturn: - return .terminateNow + return terminateGracefully() default: return .terminateCancel } @@ -1360,7 +1378,7 @@ extension AppDelegate { return } } - await NSApp.reply(toApplicationShouldTerminate: true) + await MainActor.run { _ = self.terminateGracefully() } } } } diff --git a/macos/Sources/Features/Terminal/BaseTerminalController.swift b/macos/Sources/Features/Terminal/BaseTerminalController.swift index 9de22b3d9..b41c5c881 100644 --- a/macos/Sources/Features/Terminal/BaseTerminalController.swift +++ b/macos/Sources/Features/Terminal/BaseTerminalController.swift @@ -438,6 +438,20 @@ class BaseTerminalController: NSWindowController, } } + /// Frees all surfaces in this controller synchronously. + /// + /// This is used during app termination to ensure SIGHUP signals are sent + /// to child processes before the app exits. Unlike the async Surface.deinit + /// approach, this calls ghostty_surface_free synchronously. + func freeSurfaces() { + for surfaceView in surfaceTree { + if let surface = surfaceView.surface { + ghostty_surface_free(surface) + } + } + surfaceTree = .init() + } + // MARK: Split Tree Management /// Find the next surface to focus when a node is being closed. diff --git a/macos/Sources/Features/Terminal/TerminalController.swift b/macos/Sources/Features/Terminal/TerminalController.swift index bcccef8f9..dc47783d0 100644 --- a/macos/Sources/Features/Terminal/TerminalController.swift +++ b/macos/Sources/Features/Terminal/TerminalController.swift @@ -977,6 +977,13 @@ class TerminalController: BaseTerminalController, TabGroupCloseCoordinator.Contr undoManager?.endUndoGrouping() } + /// Frees all surfaces from all terminal controllers synchronously. + /// + /// This is used during termination to gracefully destroy our surfaces. + static internal func freeAllSurfaces() { + all.forEach { $0.freeSurfaces() } + } + // MARK: Undo/Redo /// The state that we require to recreate a TerminalController from an undo. From 1a5504592d7a7c9fd72659d3e97ba64ddfae6953 Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Sun, 30 Nov 2025 16:33:17 -0500 Subject: [PATCH 2/7] core: add ghostty_surface_process_stop() This exported function allows the app to stop a surface's child process. ghostty_surface_process_exited() can be used to check whether it's done stopping. --- include/ghostty.h | 1 + src/apprt/embedded.zig | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/include/ghostty.h b/include/ghostty.h index fbfe3ee2c..271991129 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -1107,6 +1107,7 @@ GHOSTTY_API ghostty_app_t ghostty_surface_app(ghostty_surface_t); GHOSTTY_API ghostty_surface_config_s ghostty_surface_inherited_config(ghostty_surface_t, ghostty_surface_context_e); GHOSTTY_API void ghostty_surface_update_config(ghostty_surface_t, ghostty_config_t); GHOSTTY_API bool ghostty_surface_needs_confirm_quit(ghostty_surface_t); +GHOSTTY_API bool ghostty_surface_process_stop(ghostty_surface_t); GHOSTTY_API bool ghostty_surface_process_exited(ghostty_surface_t); GHOSTTY_API void ghostty_surface_refresh(ghostty_surface_t); GHOSTTY_API void ghostty_surface_draw(ghostty_surface_t); diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 7310159cc..cb0d2c0d4 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -1600,6 +1600,13 @@ pub const CAPI = struct { return surface.core_surface.child_exited; } + /// Stops the child process by sending it the kill command (SIGHUP). + export fn ghostty_surface_process_stop(surface: *Surface) void { + switch (surface.core_surface.io.backend) { + .exec => |*exec| exec.subprocess.stop(), + } + } + /// Returns true if the surface has a selection. export fn ghostty_surface_has_selection(surface: *Surface) bool { return surface.core_surface.hasSelection(); From 93af8c895510037c1af3860c784de5c6581fd7f8 Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Sun, 30 Nov 2025 16:36:17 -0500 Subject: [PATCH 3/7] macos: revise graceful termination approach Freeing the surfaces directly was a flawed approach because its a fundamental structure within the application. Instead, we now gather all of the surfaces and stop their processes. The surfaces remain alive which allows us to poll them and delay application termination (up to a time limit) until they've exited. --- macos/Sources/App/macOS/AppDelegate.swift | 35 +++++++++++++------ .../Terminal/BaseTerminalController.swift | 14 -------- .../Terminal/TerminalController.swift | 7 ---- .../Surface View/SurfaceView_AppKit.swift | 6 ++++ 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index a3bc7110f..39f1b6872 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -363,20 +363,35 @@ class AppDelegate: NSObject, } /// Initiates graceful application termination. + /// + /// We signal all child processes to stop and then wait for them to exit + /// (or for our timeout to expire) before terminating the application. private func terminateGracefully() -> NSApplication.TerminateReply { - /// Free our surfaces synchronously, ensuring SIGHUP signals are sent to all - /// child processes (e.g., bash) so they can clean up before the app exits. - TerminalController.freeAllSurfaces() + let surfaces = TerminalController.all.flatMap { $0.surfaceTree } + quickController.surfaceTree + surfaces.forEach { $0.stopProcess() } - if !quickController.surfaceTree.isEmpty { - quickController.freeSurfaces() + let deadline = DispatchTime.now() + .milliseconds(500) + let pollInterval: DispatchTimeInterval = .milliseconds(50) + + func waitForProcesses() { + if surfaces.allSatisfy({ $0.processExited }) { + NSApp.reply(toApplicationShouldTerminate: true) + return + } + + if DispatchTime.now() >= deadline { + Ghostty.logger.info("child process deadline exceeded; terminating immediately") + NSApp.reply(toApplicationShouldTerminate: true) + return + } + + DispatchQueue.main.asyncAfter(deadline: .now() + pollInterval) { + waitForProcesses() + } } - // Schedule termination after a brief delay to allow child processes - // to handle SIGHUP and complete cleanup (e.g., bash saving history). - DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { - NSApp.reply(toApplicationShouldTerminate: true) - } + Ghostty.logger.debug("waiting for child processes to exit") + waitForProcesses() return .terminateLater } diff --git a/macos/Sources/Features/Terminal/BaseTerminalController.swift b/macos/Sources/Features/Terminal/BaseTerminalController.swift index b41c5c881..9de22b3d9 100644 --- a/macos/Sources/Features/Terminal/BaseTerminalController.swift +++ b/macos/Sources/Features/Terminal/BaseTerminalController.swift @@ -438,20 +438,6 @@ class BaseTerminalController: NSWindowController, } } - /// Frees all surfaces in this controller synchronously. - /// - /// This is used during app termination to ensure SIGHUP signals are sent - /// to child processes before the app exits. Unlike the async Surface.deinit - /// approach, this calls ghostty_surface_free synchronously. - func freeSurfaces() { - for surfaceView in surfaceTree { - if let surface = surfaceView.surface { - ghostty_surface_free(surface) - } - } - surfaceTree = .init() - } - // MARK: Split Tree Management /// Find the next surface to focus when a node is being closed. diff --git a/macos/Sources/Features/Terminal/TerminalController.swift b/macos/Sources/Features/Terminal/TerminalController.swift index dc47783d0..bcccef8f9 100644 --- a/macos/Sources/Features/Terminal/TerminalController.swift +++ b/macos/Sources/Features/Terminal/TerminalController.swift @@ -977,13 +977,6 @@ class TerminalController: BaseTerminalController, TabGroupCloseCoordinator.Contr undoManager?.endUndoGrouping() } - /// Frees all surfaces from all terminal controllers synchronously. - /// - /// This is used during termination to gracefully destroy our surfaces. - static internal func freeAllSurfaces() { - all.forEach { $0.freeSurfaces() } - } - // MARK: Undo/Redo /// The state that we require to recreate a TerminalController from an undo. diff --git a/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift index 887482b30..59993581f 100644 --- a/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift @@ -145,6 +145,12 @@ extension Ghostty { return ghostty_surface_process_exited(surface) } + // Stops the child process. + func stopProcess() { + guard let surface = self.surface else { return } + ghostty_surface_process_stop(surface) + } + // Returns the inspector instance for this surface, or nil if the // surface has been closed or no inspector is active. var inspector: Ghostty.Inspector? { From 9fe900aa017768234d5bfd163bee999e7359a669 Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Fri, 12 Dec 2025 09:57:04 -0500 Subject: [PATCH 4/7] core: add Surface.stopProcess This keeps the IO backend interaction inside of the owning Surface code. --- src/Surface.zig | 8 ++++++++ src/apprt/embedded.zig | 4 +--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index 5d16f3326..3f582ea8b 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -851,6 +851,14 @@ pub fn close(self: *Surface) void { self.rt_surface.close(self.needsConfirmQuit()); } +/// Stop the child process by sending it the kill command (SIGHUP). +/// child_exited will be set once the child process has stopped. +pub fn stopProcess(self: *Surface) void { + switch (self.io.backend) { + .exec => |*exec| exec.subprocess.stop(), + } +} + /// Returns a mailbox that can be used to send messages to this surface. inline fn surfaceMailbox(self: *Surface) Mailbox { return .{ diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index cb0d2c0d4..c569f93e0 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -1602,9 +1602,7 @@ pub const CAPI = struct { /// Stops the child process by sending it the kill command (SIGHUP). export fn ghostty_surface_process_stop(surface: *Surface) void { - switch (surface.core_surface.io.backend) { - .exec => |*exec| exec.subprocess.stop(), - } + surface.core_surface.stopProcess(); } /// Returns true if the surface has a selection. From 03ca0d215f04eaadd409cb119f09abfcfbb019e5 Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Tue, 23 Dec 2025 09:24:01 -0500 Subject: [PATCH 5/7] macos: use quickTerminalControllerState --- macos/Sources/App/macOS/AppDelegate.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index 39f1b6872..017a23420 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -367,7 +367,10 @@ class AppDelegate: NSObject, /// We signal all child processes to stop and then wait for them to exit /// (or for our timeout to expire) before terminating the application. private func terminateGracefully() -> NSApplication.TerminateReply { - let surfaces = TerminalController.all.flatMap { $0.surfaceTree } + quickController.surfaceTree + var surfaces = TerminalController.all.flatMap { $0.surfaceTree } + if case .initialized(let controller) = quickTerminalControllerState { + surfaces += controller.surfaceTree + } surfaces.forEach { $0.stopProcess() } let deadline = DispatchTime.now() + .milliseconds(500) From eaa30182113483299dcf2ef2cb62ec546a790f41 Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Tue, 23 Dec 2025 09:26:06 -0500 Subject: [PATCH 6/7] macos: parameterize the termination timeout --- macos/Sources/App/macOS/AppDelegate.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index 017a23420..f853e5166 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -366,15 +366,16 @@ class AppDelegate: NSObject, /// /// We signal all child processes to stop and then wait for them to exit /// (or for our timeout to expire) before terminating the application. - private func terminateGracefully() -> NSApplication.TerminateReply { + private func terminateGracefully( + timeout: DispatchTimeInterval = .milliseconds(500), + ) -> NSApplication.TerminateReply { var surfaces = TerminalController.all.flatMap { $0.surfaceTree } if case .initialized(let controller) = quickTerminalControllerState { surfaces += controller.surfaceTree } surfaces.forEach { $0.stopProcess() } - let deadline = DispatchTime.now() + .milliseconds(500) - let pollInterval: DispatchTimeInterval = .milliseconds(50) + let deadline = DispatchTime.now() + timeout func waitForProcesses() { if surfaces.allSatisfy({ $0.processExited }) { @@ -388,7 +389,7 @@ class AppDelegate: NSObject, return } - DispatchQueue.main.asyncAfter(deadline: .now() + pollInterval) { + DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(50)) { waitForProcesses() } } From 0bf1f75be97ea5ee37bda5dd71f86b565c7aa277 Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Sun, 17 May 2026 11:02:07 -0400 Subject: [PATCH 7/7] core: add ghostty_app_quit for batched graceful termination Replaces the per-surface stop+poll dance with a single libghostty entry point that hangs up all child processes in parallel and waits (up to a caller-provided timeout) for them to exit. The per-surface Subprocess.stop now bounds its own wait time, too, so a stuck child can't stall application teardown. --- include/ghostty.h | 2 +- macos/Sources/App/macOS/AppDelegate.swift | 45 ++----- .../Surface View/SurfaceView_AppKit.swift | 6 - src/App.zig | 34 ++++++ src/Surface.zig | 9 +- src/apprt/embedded.zig | 14 ++- src/termio/Exec.zig | 115 +++++++++++++----- 7 files changed, 150 insertions(+), 75 deletions(-) diff --git a/include/ghostty.h b/include/ghostty.h index 271991129..9332def7e 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -1094,6 +1094,7 @@ GHOSTTY_API void ghostty_app_keyboard_changed(ghostty_app_t); GHOSTTY_API void ghostty_app_open_config(ghostty_app_t); GHOSTTY_API void ghostty_app_update_config(ghostty_app_t, ghostty_config_t); GHOSTTY_API bool ghostty_app_needs_confirm_quit(ghostty_app_t); +GHOSTTY_API void ghostty_app_quit(ghostty_app_t, uint64_t); GHOSTTY_API bool ghostty_app_has_global_keybinds(ghostty_app_t); GHOSTTY_API void ghostty_app_set_color_scheme(ghostty_app_t, ghostty_color_scheme_e); @@ -1107,7 +1108,6 @@ GHOSTTY_API ghostty_app_t ghostty_surface_app(ghostty_surface_t); GHOSTTY_API ghostty_surface_config_s ghostty_surface_inherited_config(ghostty_surface_t, ghostty_surface_context_e); GHOSTTY_API void ghostty_surface_update_config(ghostty_surface_t, ghostty_config_t); GHOSTTY_API bool ghostty_surface_needs_confirm_quit(ghostty_surface_t); -GHOSTTY_API bool ghostty_surface_process_stop(ghostty_surface_t); GHOSTTY_API bool ghostty_surface_process_exited(ghostty_surface_t); GHOSTTY_API void ghostty_surface_refresh(ghostty_surface_t); GHOSTTY_API void ghostty_surface_draw(ghostty_surface_t); diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index f853e5166..78a059e12 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -362,42 +362,21 @@ class AppDelegate: NSObject, return derivedConfig.shouldQuitAfterLastWindowClosed } - /// Initiates graceful application termination. + /// Initiates graceful application termination by giving every child + /// process a chance to exit before the app terminates. /// - /// We signal all child processes to stop and then wait for them to exit - /// (or for our timeout to expire) before terminating the application. + /// The wait happens inside libghostty: it sends SIGHUP to all processes + /// in parallel, then waits up to `timeout` (total) for them to exit. + /// This blocks the main thread, which is acceptable here because we've + /// already committed to terminating. private func terminateGracefully( - timeout: DispatchTimeInterval = .milliseconds(500), + timeout: TimeInterval = 0.5 ) -> NSApplication.TerminateReply { - var surfaces = TerminalController.all.flatMap { $0.surfaceTree } - if case .initialized(let controller) = quickTerminalControllerState { - surfaces += controller.surfaceTree + if let app = ghostty.app { + Ghostty.logger.debug("waiting for child processes to exit") + ghostty_app_quit(app, UInt64(max(0, timeout * 1000))) } - surfaces.forEach { $0.stopProcess() } - - let deadline = DispatchTime.now() + timeout - - func waitForProcesses() { - if surfaces.allSatisfy({ $0.processExited }) { - NSApp.reply(toApplicationShouldTerminate: true) - return - } - - if DispatchTime.now() >= deadline { - Ghostty.logger.info("child process deadline exceeded; terminating immediately") - NSApp.reply(toApplicationShouldTerminate: true) - return - } - - DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(50)) { - waitForProcesses() - } - } - - Ghostty.logger.debug("waiting for child processes to exit") - waitForProcesses() - - return .terminateLater + return .terminateNow } func applicationShouldTerminate(_ sender: NSApplication) -> NSApplication.TerminateReply { @@ -1351,6 +1330,7 @@ extension AppDelegate { if [.OK, .alertFirstButtonReturn].contains(response) { await MainActor.run { _ = self.terminateGracefully() } + await NSApp.reply(toApplicationShouldTerminate: true) } else { await NSApp.reply(toApplicationShouldTerminate: false) } @@ -1398,6 +1378,7 @@ extension AppDelegate { } } await MainActor.run { _ = self.terminateGracefully() } + await NSApp.reply(toApplicationShouldTerminate: true) } } } diff --git a/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift index 59993581f..887482b30 100644 --- a/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift @@ -145,12 +145,6 @@ extension Ghostty { return ghostty_surface_process_exited(surface) } - // Stops the child process. - func stopProcess() { - guard let surface = self.surface else { return } - ghostty_surface_process_stop(surface) - } - // Returns the inspector instance for this surface, or nil if the // surface has been closed or no inspector is active. var inspector: Ghostty.Inspector? { diff --git a/src/App.zig b/src/App.zig index 93ee7dea1..3200baa96 100644 --- a/src/App.zig +++ b/src/App.zig @@ -102,6 +102,40 @@ pub fn init( }; } +/// Send SIGHUP to every surface's child process and wait up to timeout_ms +/// (total, across all surfaces) for them to exit. Intended to be called +/// once during app shutdown so that children get a chance to clean up +/// (e.g., shells writing history) before the apprt tears down each +/// surface. Surfaces and resources are not freed here; that happens via +/// the normal apprt deinit path afterwards. +/// +/// We drain the mailbox during the wait so that child_exited messages +/// posted by IO threads are observed; otherwise we'd block the same +/// main thread that the message pump runs on. +pub fn quit(self: *App, rt_app: *apprt.App, timeout_ms: u64) void { + // Hang up all children in parallel so they shut down concurrently + // rather than one-at-a-time during sequential surface deinits. + for (self.surfaces.items) |surface| surface.core().hangupProcess(); + + const deadline: i64 = std.time.milliTimestamp() +| @as(i64, @intCast(timeout_ms)); + while (std.time.milliTimestamp() < deadline) { + self.drainMailbox(rt_app) catch |err| + log.warn("error draining mailbox during quit err={}", .{err}); + + var all_exited = true; + for (self.surfaces.items) |surface| { + if (!surface.core().child_exited) { + all_exited = false; + break; + } + } + if (all_exited) return; + std.Thread.sleep(10 * std.time.ns_per_ms); + } + + log.warn("not all child processes exited within {}ms", .{timeout_ms}); +} + pub fn deinit(self: *App) void { // Clean up all our surfaces for (self.surfaces.items) |surface| surface.deinit(); diff --git a/src/Surface.zig b/src/Surface.zig index 3f582ea8b..8486d9ebb 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -851,11 +851,14 @@ pub fn close(self: *Surface) void { self.rt_surface.close(self.needsConfirmQuit()); } -/// Stop the child process by sending it the kill command (SIGHUP). +/// Send SIGHUP to the child process without waiting for it to exit. /// child_exited will be set once the child process has stopped. -pub fn stopProcess(self: *Surface) void { +/// +/// Used during app shutdown to hang up all children in parallel before +/// any individual surface starts its (bounded) wait in deinit. +pub fn hangupProcess(self: *Surface) void { switch (self.io.backend) { - .exec => |*exec| exec.subprocess.stop(), + .exec => |*exec| exec.subprocess.hangup(), } } diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index c569f93e0..59d0fddb8 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -1442,6 +1442,15 @@ pub const CAPI = struct { core_app.destroy(); } + /// Begin a graceful application shutdown by sending SIGHUP to every + /// surface's child process and waiting up to timeout_ms (total, across + /// all children) for them to exit. Intended to be called once during + /// app termination so children can clean up (e.g., shells writing + /// history) before the apprt tears down each surface. + export fn ghostty_app_quit(v: *App, timeout_ms: u64) void { + v.core_app.quit(v, timeout_ms); + } + /// Update the focused state of the app. export fn ghostty_app_set_focus( app: *App, @@ -1600,11 +1609,6 @@ pub const CAPI = struct { return surface.core_surface.child_exited; } - /// Stops the child process by sending it the kill command (SIGHUP). - export fn ghostty_surface_process_stop(surface: *Surface) void { - surface.core_surface.stopProcess(); - } - /// Returns true if the surface has a selection. export fn ghostty_surface_has_selection(surface: *Surface) bool { return surface.core_surface.hasSelection(); diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 87d47807c..26f58570a 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -34,6 +34,13 @@ const log = std.log.scoped(.io_exec); /// The termios poll rate in milliseconds. const TERMIOS_POLL_MS = 200; +/// Default deadline for waiting on the child to exit after SIGHUP, in +/// milliseconds. SIGHUP delivery is effectively instant and well-behaved +/// shells exit within microseconds; the deadline only matters when a child +/// is hung (e.g., in trapped SIGHUP and refusing to exit) and we don't +/// want that to stall application shutdown. +const stop_deadline_ms: u64 = 500; + /// If we build with flatpak support then we have to keep track of /// a potential execution on the host. const FlatpakHostCommand = if (!build_config.flatpak) struct { @@ -1053,7 +1060,7 @@ const Subprocess = struct { else => return err, } }; - errdefer killCommand(&cmd) catch |err| { + errdefer killCommand(&cmd, stop_deadline_ms) catch |err| { log.warn("error killing command during cleanup err={}", .{err}); }; log.info("started subcommand path={s} pid={?}", .{ self.args[0], cmd.pid }); @@ -1086,17 +1093,51 @@ const Subprocess = struct { self.process = null; } - /// Stop the subprocess. This is safe to call anytime. This will wait - /// for the subprocess to register that it has been signalled, but not - /// for it to terminate, so it will not block. + /// Send SIGHUP to the subprocess without waiting for it to exit. + /// + /// Calling this before stop() lets multiple subprocesses begin shutting + /// down in parallel rather than serially: hangup all, then deinit each + /// (which only has to wait for exit, not also for signal delivery). + /// + /// This is safe to call multiple times and will do nothing if the + /// process has already been stopped. + pub fn hangup(self: *Subprocess) void { + switch (self.process orelse return) { + .fork_exec => |*cmd| { + if (cmd.pid) |pid| switch (builtin.os.tag) { + .windows => { + if (windows.kernel32.TerminateProcess(pid, 0) == 0) { + log.err( + "error terminating command: {}", + .{windows.kernel32.GetLastError()}, + ); + } + }, + + else => _ = hangupPid(pid) catch |err| + log.err("error sending SIGHUP to command: {}", .{err}), + }; + }, + + .flatpak => |*cmd| if (comptime build_config.flatpak) { + killCommandFlatpak(cmd) catch |err| + log.err("error sending SIGHUP to command: {}", .{err}); + }, + } + } + + /// Stop the subprocess. This is safe to call anytime. Sends SIGHUP and + /// waits up to stop_deadline_ms for the child to exit before returning. + /// Calling hangup() first is harmless and lets the child start exiting + /// sooner, which shortens this wait in practice. /// This does not close the pty. pub fn stop(self: *Subprocess) void { switch (self.process orelse return) { .fork_exec => |*cmd| { // Note: this will also wait for the command to exit, so // DO NOT call cmd.wait - killCommand(cmd) catch |err| - log.err("error sending SIGHUP to command, may hang: {}", .{err}); + killCommand(cmd, stop_deadline_ms) catch |err| + log.err("error stopping command: {}", .{err}); }, .flatpak => |*cmd| if (comptime build_config.flatpak) { @@ -1134,9 +1175,10 @@ const Subprocess = struct { } /// Kill the underlying subprocess. This sends a SIGHUP to the child - /// process. This also waits for the command to exit and will return the - /// exit code. - fn killCommand(command: *Command) !void { + /// process and then waits up to deadline_ms for it to exit. Returns + /// regardless of whether the child has actually exited once the + /// deadline has passed; a stuck child shouldn't stall teardown. + fn killCommand(command: *Command, deadline_ms: u64) !void { if (command.pid) |pid| { switch (builtin.os.tag) { .windows => { @@ -1147,36 +1189,47 @@ const Subprocess = struct { _ = try command.wait(false); }, - else => try killPid(pid), + else => try killPid(pid, deadline_ms), } } } - fn killPid(pid: c.pid_t) !void { - const pgid = getpgid(pid) orelse return; + /// Send SIGHUP to a process group. Returns true on success, false if + /// the process group could not be found (already exited). + fn hangupPid(pid: c.pid_t) !bool { + const pgid = getpgid(pid) orelse return false; + + switch (posix.errno(c.killpg(pgid, c.SIGHUP))) { + .SUCCESS => log.debug("process group killed pgid={}", .{pgid}), + else => |err| { + // killpg returns EPERM on Darwin even when delivery succeeds + // (see https://openradar.appspot.com/radar?id=4970011239673856). + if ((comptime builtin.target.os.tag.isDarwin()) and + err == .PERM) + { + log.debug("killpg failed with EPERM, expected on Darwin and ignoring", .{}); + return true; + } + + log.warn("error killing process group pgid={} err={}", .{ pgid, err }); + return error.KillFailed; + }, + } + return true; + } + + fn killPid(pid: c.pid_t, deadline_ms: u64) !void { + const deadline: i64 = std.time.milliTimestamp() +| @as(i64, @intCast(deadline_ms)); // It is possible to send a killpg between the time that // our child process calls setsid but before or simultaneous // to calling execve. In this case, the direct child dies // but grandchildren survive. To work around this, we loop // and repeatedly kill the process group until all - // descendents are well and truly dead. We will not rest - // until the entire family tree is obliterated. + // descendents are well and truly dead, or until our deadline + // expires. while (true) { - switch (posix.errno(c.killpg(pgid, c.SIGHUP))) { - .SUCCESS => log.debug("process group killed pgid={}", .{pgid}), - else => |err| killpg: { - if ((comptime builtin.target.os.tag.isDarwin()) and - err == .PERM) - { - log.debug("killpg failed with EPERM, expected on Darwin and ignoring", .{}); - break :killpg; - } - - log.warn("error killing process group pgid={} err={}", .{ pgid, err }); - return error.KillFailed; - }, - } + if (!try hangupPid(pid)) return; // See Command.zig wait for why we specify WNOHANG. // The gist is that it lets us detect when children @@ -1185,6 +1238,12 @@ const Subprocess = struct { const res = posix.waitpid(pid, std.c.W.NOHANG); log.debug("waitpid result={}", .{res.pid}); if (res.pid != 0) break; + + if (std.time.milliTimestamp() >= deadline) { + log.warn("child pid={} did not exit within {}ms", .{ pid, deadline_ms }); + break; + } + std.Thread.sleep(10 * std.time.ns_per_ms); } }