mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-08-24 16:11:43 +00:00
macos: avoid IOSurface leak on automated surface creation
Fixes #13444 A close while AppKit temporarily cleared/changed a surface's window would leak the surface in the controller's pslit tree. This retained surface kept a bunch of resources around, particularly large IOSurfaces. This seems to only be reproducible under scripted load: rapid terminal creation/destruction so that destruction happens just while there is a nil window on a surface view. Track surface ownership in a weak controller map updated alongside the split tree, with validated fallbacks for existing attachment state. Resolve scripted and App Intent operations through that ownership, and route non-confirming root closes directly through the immediate tab or window close path so teardown always reaches the renderer.
This commit is contained in:
@@ -27,7 +27,7 @@ struct CloseTerminalIntent: AppIntent {
|
||||
throw GhosttyIntentError.surfaceNotFound
|
||||
}
|
||||
|
||||
guard let controller = surfaceView.window?.windowController as? BaseTerminalController else {
|
||||
guard let controller = BaseTerminalController.controller(owning: surfaceView) else {
|
||||
return .result()
|
||||
}
|
||||
|
||||
|
||||
@@ -27,7 +27,7 @@ struct FocusTerminalIntent: AppIntent {
|
||||
throw GhosttyIntentError.surfaceNotFound
|
||||
}
|
||||
|
||||
guard let controller = surfaceView.window?.windowController as? BaseTerminalController else {
|
||||
guard let controller = BaseTerminalController.controller(owning: surfaceView) else {
|
||||
return .result()
|
||||
}
|
||||
|
||||
|
||||
@@ -112,7 +112,7 @@ final class ScriptTerminal: NSObject {
|
||||
baseConfig = nil
|
||||
}
|
||||
|
||||
guard let controller = surfaceView.window?.windowController as? BaseTerminalController else {
|
||||
guard let controller = BaseTerminalController.controller(owning: surfaceView) else {
|
||||
command.scriptErrorNumber = errAEEventFailed
|
||||
command.scriptErrorString = "Terminal is not in a splittable window."
|
||||
return nil
|
||||
@@ -142,7 +142,7 @@ final class ScriptTerminal: NSObject {
|
||||
return nil
|
||||
}
|
||||
|
||||
guard let controller = surfaceView.window?.windowController as? BaseTerminalController else {
|
||||
guard let controller = BaseTerminalController.controller(owning: surfaceView) else {
|
||||
command.scriptErrorNumber = errAEEventFailed
|
||||
command.scriptErrorString = "Terminal is not in a window."
|
||||
return nil
|
||||
@@ -163,7 +163,7 @@ final class ScriptTerminal: NSObject {
|
||||
return nil
|
||||
}
|
||||
|
||||
guard let controller = surfaceView.window?.windowController as? BaseTerminalController else {
|
||||
guard let controller = BaseTerminalController.controller(owning: surfaceView) else {
|
||||
command.scriptErrorNumber = errAEEventFailed
|
||||
command.scriptErrorString = "Terminal is not in a window."
|
||||
return nil
|
||||
|
||||
@@ -32,6 +32,11 @@ class BaseTerminalController: NSWindowController,
|
||||
TerminalViewModel,
|
||||
ClipboardConfirmationViewDelegate,
|
||||
FullscreenDelegate {
|
||||
/// Weak surface-to-controller ownership independent of AppKit's transient
|
||||
/// view and window attachment state.
|
||||
private static let surfaceControllers =
|
||||
NSMapTable<Ghostty.SurfaceView, BaseTerminalController>.weakToWeakObjects()
|
||||
|
||||
/// The app instance that this terminal view will represent.
|
||||
let ghostty: Ghostty.App
|
||||
|
||||
@@ -42,7 +47,10 @@ class BaseTerminalController: NSWindowController,
|
||||
|
||||
/// The tree of splits within this terminal window.
|
||||
@Published var surfaceTree: SplitTree<Ghostty.SurfaceView> = .init() {
|
||||
didSet { surfaceTreeDidChange(from: oldValue, to: surfaceTree) }
|
||||
didSet {
|
||||
Self.updateSurfaceControllers(self, from: oldValue, to: surfaceTree)
|
||||
surfaceTreeDidChange(from: oldValue, to: surfaceTree)
|
||||
}
|
||||
}
|
||||
|
||||
/// This can be set to show/hide the command palette.
|
||||
@@ -140,6 +148,7 @@ class BaseTerminalController: NSWindowController,
|
||||
// Initialize our initial surface.
|
||||
guard let ghostty_app = ghostty.app else { preconditionFailure("app must be loaded") }
|
||||
self.surfaceTree = tree ?? .init(view: Ghostty.SurfaceView(ghostty_app, baseConfig: base))
|
||||
Self.updateSurfaceControllers(self, from: .init(), to: surfaceTree)
|
||||
|
||||
// Setup our bell state for the window
|
||||
setupBellNotificationPublisher()
|
||||
@@ -231,6 +240,44 @@ class BaseTerminalController: NSWindowController,
|
||||
|
||||
// MARK: Methods
|
||||
|
||||
/// Finds the controller whose split tree owns the given surface.
|
||||
///
|
||||
/// A surface's `window` can briefly be nil or point at its previous window
|
||||
/// while AppKit is attaching or moving a native tab. Callers performing
|
||||
/// lifecycle operations must use tree ownership rather than that transient
|
||||
/// view relationship.
|
||||
static func controller(owning surface: Ghostty.SurfaceView) -> BaseTerminalController? {
|
||||
if let controller = surfaceControllers.object(forKey: surface),
|
||||
controller.surfaceTree.contains(surface) {
|
||||
return controller
|
||||
}
|
||||
|
||||
if let controller = surface.window?.windowController as? BaseTerminalController,
|
||||
controller.surfaceTree.contains(surface) {
|
||||
return controller
|
||||
}
|
||||
|
||||
return NSApp.windows
|
||||
.compactMap { $0.windowController as? BaseTerminalController }
|
||||
.first { $0.surfaceTree.contains(surface) }
|
||||
}
|
||||
|
||||
private static func updateSurfaceControllers(
|
||||
_ controller: BaseTerminalController,
|
||||
from oldTree: SplitTree<Ghostty.SurfaceView>,
|
||||
to newTree: SplitTree<Ghostty.SurfaceView>
|
||||
) {
|
||||
for surface in oldTree where !newTree.contains(surface) {
|
||||
if surfaceControllers.object(forKey: surface) === controller {
|
||||
surfaceControllers.removeObject(forKey: surface)
|
||||
}
|
||||
}
|
||||
|
||||
for surface in newTree {
|
||||
surfaceControllers.setObject(controller, forKey: surface)
|
||||
}
|
||||
}
|
||||
|
||||
/// Create a new split.
|
||||
@discardableResult
|
||||
func newSplit(
|
||||
|
||||
@@ -667,12 +667,20 @@ class TerminalController: BaseTerminalController, TabGroupCloseCoordinator.Contr
|
||||
|
||||
// More than 1 window means we have tabs and we're closing a tab
|
||||
if window?.tabGroup?.windows.count ?? 0 > 1 {
|
||||
closeTab(nil)
|
||||
if withConfirmation {
|
||||
closeTab(nil)
|
||||
} else {
|
||||
closeTabImmediately()
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
// 1 window, closing the window
|
||||
closeWindow(nil)
|
||||
if withConfirmation {
|
||||
closeWindow(nil)
|
||||
} else {
|
||||
closeWindowImmediately()
|
||||
}
|
||||
}
|
||||
|
||||
func closeTabImmediately(registerRedo: Bool = true) {
|
||||
|
||||
Reference in New Issue
Block a user