Skip to content

fix: guard GhosttyNSView screen observers against stale delivery (mechanism 2, part 2) #259

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
arzafran merged 1 commit into from
Aug 5, 2026
Merged

fix: guard GhosttyNSView screen observers against stale delivery (mechanism 2, part 2) #259

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions Sources/GhosttyNSView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,16 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
var trackingArea: NSTrackingArea?
private var windowObserver: NSObjectProtocol?
private var screenParametersObserver: NSObjectProtocol?
// Teardown-race guard (ported from #258's GhosttySurfaceScrollView fix; this was
// the residual crasher on the reland gate — the `notification.object as? NSWindow`
// downcast in windowDidChangeScreen implicitly retaining a freed notification
// object). NotificationCenter's removeObserver does NOT cancel a block already
// handed off to the main queue — during rapid window create/close, a block
// enqueued against the *previous* window/registration batch can still run after
// viewDidMoveToWindow has already torn down and re-registered observers for a
// new window. Every block captures the generation live at its own registration
// and no-ops if the instance has since moved on to a newer generation.
private var windowObserverGeneration: UInt64 = 0
var lastScrollEventTime: CFTimeInterval = 0
private var visibleInUI: Bool = true
private var pendingSurfaceSize: CGSize?
Expand Down Expand Up @@ -386,6 +396,8 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
NotificationCenter.default.removeObserver(screenParametersObserver)
self.screenParametersObserver = nil
}
windowObserverGeneration &+= 1
let observerGeneration = windowObserverGeneration
// Balance the cursor stack if the view is removed while hover is active
if wordPathHoverActive {
wordPathHoverActive = false
Expand Down Expand Up @@ -418,7 +430,8 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
object: window,
queue: .main
) { [weak self] notification in
self?.windowDidChangeScreen(notification)
guard let self, self.windowObserverGeneration == observerGeneration else { return }
self.windowDidChangeScreen(notification)
}

// NSWindow.didChangeScreenNotification only fires when AppKit decides this
Expand All @@ -431,7 +444,8 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
object: nil,
queue: .main
) { [weak self] notification in
self?.applicationDidChangeScreenParameters(notification)
guard let self, self.windowObserverGeneration == observerGeneration else { return }
self.applicationDidChangeScreenParameters(notification)
}

if let surface = terminalSurface?.surface,
Expand Down Expand Up @@ -853,6 +867,7 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
if let screenParametersObserver {
NotificationCenter.default.removeObserver(screenParametersObserver)
}
windowObserverGeneration &+= 1
if let trackingArea {
removeTrackingArea(trackingArea)
}
Expand Down
Loading