marketing-shibata50/cmux
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix use-after-free in ghostty_surface_refresh after sleep/wake (#432) (#619)

Browse source 
Add nil guard in forceRefresh() to prevent dereferencing freed surface
pointer. Split else-if chains in Workspace.swift so
requestBackgroundSurfaceStartIfNeeded() runs if surface is freed during
the refresh call. Add regression test exercising the crash path.
This commit is contained in:
Lawrence Chen 2026-02-27 01:44:02 -08:00 • committed by GitHub
parent 9ae737026d
commit dca8992901
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 80 additions and 16 deletions
Download patch file Download diff file Expand all files Collapse all files

46
Sources/GhosttyTerminalView.swift
View file

@ -2007,16 +2007,16 @@ final class TerminalSurface: Identifiable, ObservableObject {
/// Force a full size recalculation and surface redraw.
func forceRefresh() {
let hasSurface = surface != nil
let viewState: String
if let view = attachedView {
let inWindow = view.window != nil
let bounds = view.bounds
let metalOK = (view.layer as? CAMetalLayer) != nil
viewState = "inWindow=\(inWindow) bounds=\(bounds) metalOK=\(metalOK) hasSurface=\(hasSurface)"
} else {
viewState = "NO_ATTACHED_VIEW hasSurface=\(hasSurface)"
}
let hasSurface = surface != nil
let viewState: String
if let view = attachedView {
let inWindow = view.window != nil
let bounds = view.bounds
let metalOK = (view.layer as? CAMetalLayer) != nil
viewState = "inWindow=\(inWindow) bounds=\(bounds) metalOK=\(metalOK) hasSurface=\(hasSurface)"
} else {
viewState = "NO_ATTACHED_VIEW hasSurface=\(hasSurface)"
}
#if DEBUG
let ts = ISO8601DateFormatter().string(from: Date())
let line = "[\(ts)] forceRefresh: \(id) \(viewState)\n"
@ -2028,13 +2028,14 @@ final class TerminalSurface: Identifiable, ObservableObject {
} else {
FileManager.default.createFile(atPath: logPath, contents: line.data(using: .utf8))
}
#endif
#endif
guard let view = attachedView,
view.window != nil,
view.bounds.width > 0,
view.bounds.height > 0 else {
return
}
guard let currentSurface = self.surface else { return }
// Re-read self.surface before each ghostty call to guard against the surface
// being freed during wake-from-sleep geometry reconciliation (issue #432).
@ -2044,10 +2045,9 @@ final class TerminalSurface: Identifiable, ObservableObject {
// Reassert display id on topology churn (split close/reparent) before forcing a refresh.
// This avoids a first-run stuck-vsync state where Ghostty believes vsync is active
// but callbacks have not resumed for the current display.
if let surface = self.surface,
let displayID = (view.window?.screen ?? NSScreen.main)?.displayID,
if let displayID = (view.window?.screen ?? NSScreen.main)?.displayID,
displayID != 0 {
ghostty_surface_set_display_id(surface, displayID)
ghostty_surface_set_display_id(currentSurface, displayID)
}
view.forceRefreshSurface()
@ -2176,6 +2176,24 @@ final class TerminalSurface: Identifiable, ObservableObject {
return ghostty_surface_has_selection(surface)
}
#if DEBUG
/// Test-only helper to deterministically simulate a released runtime surface.
@MainActor
func releaseSurfaceForTesting() {
let callbackContext = surfaceCallbackContext
surfaceCallbackContext = nil
guard let surfaceToFree = surface else {
callbackContext?.release()
return
}
surface = nil
ghostty_surface_free(surfaceToFree)
callbackContext?.release()
}
#endif
deinit {
let callbackContext = surfaceCallbackContext
surfaceCallbackContext = nil

7
Sources/Workspace.swift
View file

@ -3083,8 +3083,10 @@ final class Workspace: Identifiable, ObservableObject {
// layout and view lifecycle changes that free surfaces (#432).
if terminalPanel.surface.surface != nil {
terminalPanel.surface.forceRefresh()
} else if isAttached && hasUsableBounds {
}
if terminalPanel.surface.surface == nil, isAttached && hasUsableBounds {
terminalPanel.surface.requestBackgroundSurfaceStartIfNeeded()
needsFollowUpPass = true
}
}
@ -3139,7 +3141,8 @@ final class Workspace: Identifiable, ObservableObject {
panel.hostedView.reconcileGeometryNow()
if panel.surface.surface != nil {
panel.surface.forceRefresh()
} else {
}
if panel.surface.surface == nil {
panel.surface.requestBackgroundSurfaceStartIfNeeded()
}
}

43
cmuxTests/CmuxWebViewKeyEquivalentTests.swift
View file

@ -6761,6 +6761,49 @@ final class GhosttySurfaceOverlayTests: XCTestCase {
XCTAssertFalse(hostedView.debugHasSearchOverlay())
}
func testForceRefreshNoopsAfterSurfaceReleaseDuringGeometryReconcile() throws {
#if DEBUG
let window = NSWindow(
contentRect: NSRect(x: 0, y: 0, width: 420, height: 280),
styleMask: [.titled, .closable],
backing: .buffered,
defer: false
)
defer { window.orderOut(nil) }
guard let contentView = window.contentView else {
XCTFail("Expected content view")
return
}
let surface = TerminalSurface(
tabId: UUID(),
context: GHOSTTY_SURFACE_CONTEXT_SPLIT,
configTemplate: nil,
workingDirectory: nil
)
let hostedView = surface.hostedView
hostedView.frame = contentView.bounds
hostedView.autoresizingMask = [.width, .height]
contentView.addSubview(hostedView)
window.makeKeyAndOrderFront(nil)
window.displayIfNeeded()
contentView.layoutSubtreeIfNeeded()
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
hostedView.reconcileGeometryNow()
surface.releaseSurfaceForTesting()
XCTAssertNil(surface.surface, "Surface should be nil after test release helper")
hostedView.reconcileGeometryNow()
surface.forceRefresh()
XCTAssertNil(surface.surface, "Force refresh should no-op when runtime surface is nil")
#else
throw XCTSkip("Debug-only regression test")
#endif
}
func testSearchOverlayMountDoesNotRetainTerminalSurface() {
weak var weakSurface: TerminalSurface?