From 451506ab5ad582f939465cf69e762edfd0248cff Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Sun, 30 Nov 2025 10:55:14 -0500 Subject: [PATCH] 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.