fix: reject stale ghostty surface userdata pointers before ARC retain

Sources/GhosttyApp.swift

@@ -66,6 +66,81 @@ final class GhosttySurfaceCallbackContext {
     }
 }
 
+/// Registry of currently-live `GhosttySurfaceCallbackContext` userdata pointers.
+///
+/// Ghostty can invoke callbacks carrying a surface's `ghostty_surface_userdata` pointer
+/// well after the surface itself has been torn down -- e.g. a `.scrollbar` surface message
+/// enqueued by the renderer thread (ghostty/src/renderer/generic.zig:1477-1483) and later
+/// drained on main via `App.tick` -> `drainMailbox` -> `Surface.updateScrollbar` ->
+/// `performAction`, or a clipboard read/confirm callback invoked synchronously off-main
+/// (see `runtimeReadClipboardCallback` and `confirm_read_clipboard_cb` below). Resolving
+/// `Unmanaged.fromOpaque(_:).takeUnretainedValue()` on a pointer whose backing object was
+/// already released is undefined behavior: it crashed as an `objc_retain` `EXC_BAD_ACCESS`
+/// in `GhosttySurfaceCallbackContext.tabId.getter` in 6 identical CI minidumps (and once as
+/// `doesNotRecognizeSelector`, when the freed page had been reused by a live object of a
+/// different class).
+///
+/// `read_clipboard_cb`/`confirm_read_clipboard_cb` resolve this off the ghostty IO thread
+/// (see the "off-main" comment on `runtimeReadClipboardCallback` below, and
+/// `GhosttySurfaceCallbackContext`'s own doc comment) -- so a main-actor-only registry is
+/// NOT sound here. This uses a lock instead. `register`/`release`/`resolve` all take the
+/// same lock, and `resolve` copies out the needed value fields (`surfaceId`/`tabId`) into a
+/// plain struct while still holding it, so no caller anywhere ever sees a strong/unretained
+/// reference to the live class outside that critical section -- a concurrent `release` can
+/// never race a concurrent `resolve` into observing a pointer as "live" and then reading a
+/// freed object.
+enum GhosttySurfaceUserdataRegistry {
+    private static let lock = NSLock()
+    private static var livePointers: Set<UnsafeMutableRawPointer> = []
+
+    /// Call once, right after creating the context, before handing its pointer to ghostty
+    /// (`ghostty_surface_new`/`ghostty_surface_config_s.userdata`).
+    static func register(_ pointer: UnsafeMutableRawPointer) {
+        lock.lock()
+        livePointers.insert(pointer)
+        lock.unlock()
+    }
+
+    /// Unregisters `unmanaged`'s pointer and releases it, atomically with respect to
+    /// `resolve(from:)`. Use this instead of calling `.release()` directly at every
+    /// teardown site, so a concurrent resolver can never observe a pointer as live after
+    /// it's been freed.
+    static func release(_ unmanaged: Unmanaged<GhosttySurfaceCallbackContext>?) {
+        guard let unmanaged else { return }
+        let pointer = unmanaged.toOpaque()
+        lock.lock()
+        livePointers.remove(pointer)
+        lock.unlock()
+        unmanaged.release()
+    }
+
+    /// Resolves `pointer` to a value snapshot of its `GhosttySurfaceCallbackContext`, or
+    /// `nil` if `pointer` is nil or stale (already torn down). Safe to call from any
+    /// thread.
+    static func resolve(from pointer: UnsafeMutableRawPointer?) -> GhosttySurfaceCallbackSnapshot? {
+        guard let pointer else { return nil }
+        lock.lock()
+        defer { lock.unlock() }
+        guard livePointers.contains(pointer) else {
+#if DEBUG
+            dlog("surface.userdata stale pointer rejected ptr=\(pointer)")
+#endif
+            return nil
+        }
+        let context = Unmanaged<GhosttySurfaceCallbackContext>.fromOpaque(pointer).takeUnretainedValue()
+        return GhosttySurfaceCallbackSnapshot(surfaceId: context.surfaceId, tabId: context.tabId)
+    }
+}
+
+/// Value snapshot of a `GhosttySurfaceCallbackContext`, taken atomically with the liveness
+/// check in `GhosttySurfaceUserdataRegistry.resolve(from:)`. Callers never see the live
+/// class reference itself, so there's no way to retain (or read a property of) a
+/// `GhosttySurfaceCallbackContext` from outside that registry's lock.
+struct GhosttySurfaceCallbackSnapshot {
+    let surfaceId: UUID
+    let tabId: UUID?
+}
+
 // Minimal Ghostty wrapper for terminal rendering
 // This uses libghostty (GhosttyKit.xcframework) for actual terminal emulation
 
@@ -1486,9 +1561,8 @@ class GhosttyApp {
         }
     }
 
-    private static func callbackContext(from userdata: UnsafeMutableRawPointer?) -> GhosttySurfaceCallbackContext? {
-        guard let userdata else { return nil }
-        return Unmanaged<GhosttySurfaceCallbackContext>.fromOpaque(userdata).takeUnretainedValue()
+    private static func callbackContext(from userdata: UnsafeMutableRawPointer?) -> GhosttySurfaceCallbackSnapshot? {
+        GhosttySurfaceUserdataRegistry.resolve(from: userdata)
     }
 
     // MARK: - Main-thread-only live object resolution
@@ -2234,4 +2308,16 @@ class GhosttyApp {
             "\(timestamp) seq=\(sequence) t+\(String(format: "%.3f", uptimeMs))ms thread=\(threadLabel) frame60=\(frame60) frame120=\(frame120) cmux bg: \(message)\n"
         backgroundLogWriter.append(line)
     }
+
+#if DEBUG
+    /// Test-only seam exposing the private `callbackContext(from:)` resolution (which
+    /// wraps `GhosttySurfaceUserdataRegistry.resolve(from:)`) so tests can verify a stale
+    /// userdata pointer resolves to `nil` instead of dereferencing freed memory. Widened
+    /// from `private` to an internal, DEBUG-only static func rather than exposing the
+    /// registry or the callback context type directly, matching the DEBUG-only test
+    /// scaffolding pattern used elsewhere (e.g. `TerminalSurface.replaceSurfaceWithFreedPointerForTesting`).
+    static func debugCallbackContextResolves(from userdata: UnsafeMutableRawPointer?) -> Bool {
+        callbackContext(from: userdata) != nil
+    }
+#endif
 }

Sources/TerminalSurface.swift

@@ -653,7 +653,7 @@ final class TerminalSurface: Identifiable, ObservableObject {
                 "registryOwner=\(registeredOwnerToken)"
             )
 #endif
-            callbackContext?.release()
+            GhosttySurfaceUserdataRegistry.release(callbackContext)
             return nil
         }
         return surface
@@ -903,15 +903,15 @@ final class TerminalSurface: Identifiable, ObservableObject {
                 "workspace=\(tabId.uuidString.prefix(5))"
             )
 #endif
-            callbackContext?.release()
+            GhosttySurfaceUserdataRegistry.release(callbackContext)
             tapContext?.release()
             return
         }
 
 #if DEBUG
         if runtimeSurfaceFreedOutOfBandForTesting {
             runtimeSurfaceFreedOutOfBandForTesting = false
-            callbackContext?.release()
+            GhosttySurfaceUserdataRegistry.release(callbackContext)
             tapContext?.release()
             return
         }
@@ -933,7 +933,7 @@ final class TerminalSurface: Identifiable, ObservableObject {
             // it's torn down.
             SessionWALStore.shared.unregister(surface: surfaceToFree, surfaceId: surfaceIdForTap, deleteDirectory: true)
             ghostty_surface_free(surfaceToFree)
-            callbackContext?.release()
+            GhosttySurfaceUserdataRegistry.release(callbackContext)
             tapContext?.release()
 #if DEBUG
             dlog(
@@ -1156,8 +1156,9 @@ final class TerminalSurface: Identifiable, ObservableObject {
             nsview: Unmanaged.passUnretained(view).toOpaque()
         ))
         let callbackContext = Unmanaged.passRetained(GhosttySurfaceCallbackContext(surfaceId: id, tabId: tabId))
+        GhosttySurfaceUserdataRegistry.register(callbackContext.toOpaque())
         surfaceConfig.userdata = callbackContext.toOpaque()
-        surfaceCallbackContext?.release()
+        GhosttySurfaceUserdataRegistry.release(surfaceCallbackContext)
         surfaceCallbackContext = callbackContext
         surfaceConfig.scale_factor = scaleFactors.layer
         surfaceConfig.context = surfaceContext
@@ -1418,7 +1419,7 @@ final class TerminalSurface: Identifiable, ObservableObject {
             pendingReviveSeed = nil
             pendingReviveWinchPGID = nil
             pendingReviveWinchChildPID = nil
-            surfaceCallbackContext?.release()
+            GhosttySurfaceUserdataRegistry.release(surfaceCallbackContext)
             surfaceCallbackContext = nil
             print("Failed to create ghostty surface")
             #if DEBUG
@@ -2363,6 +2364,15 @@ final class TerminalSurface: Identifiable, ObservableObject {
         needsConfirmCloseOverrideForTesting = value
     }
 
+    /// Test-only seam exposing the raw `ghostty_surface_userdata` pointer this surface
+    /// currently owns, so a test can capture it before teardown and then attempt to
+    /// resolve it afterward (`GhosttyApp.debugCallbackContextResolves(from:)`) to prove a
+    /// stale pointer is rejected instead of dereferencing freed memory.
+    @MainActor
+    func debugCallbackUserdataPointer() -> UnsafeMutableRawPointer? {
+        surfaceCallbackContext?.toOpaque()
+    }
+
     /// Test-only helper to deterministically simulate a released runtime surface.
     @MainActor
     func releaseSurfaceForTesting() {
@@ -2372,7 +2382,7 @@ final class TerminalSurface: Identifiable, ObservableObject {
         outputTapContext = nil
 
         guard let surfaceToFree = surface else {
-            callbackContext?.release()
+            GhosttySurfaceUserdataRegistry.release(callbackContext)
             tapContext?.release()
             return
         }
@@ -2382,7 +2392,7 @@ final class TerminalSurface: Identifiable, ObservableObject {
         // Test-only teardown, not a real close: keep the WAL directory.
         SessionWALStore.shared.unregister(surface: surfaceToFree, surfaceId: id.uuidString)
         ghostty_surface_free(surfaceToFree)
-        callbackContext?.release()
+        GhosttySurfaceUserdataRegistry.release(callbackContext)
         tapContext?.release()
     }
 
@@ -2398,7 +2408,7 @@ final class TerminalSurface: Identifiable, ObservableObject {
         outputTapContext = nil
 
         guard let surfaceToFree = surface else {
-            callbackContext?.release()
+            GhosttySurfaceUserdataRegistry.release(callbackContext)
             tapContext?.release()
             return
         }
@@ -2408,7 +2418,7 @@ final class TerminalSurface: Identifiable, ObservableObject {
         SessionWALStore.shared.unregister(surface: surfaceToFree, surfaceId: id.uuidString)
         ghostty_surface_free(surfaceToFree)
         runtimeSurfaceFreedOutOfBandForTesting = true
-        callbackContext?.release()
+        GhosttySurfaceUserdataRegistry.release(callbackContext)
         tapContext?.release()
     }
 #endif

programaTests/WorkspaceUnitTests.swift

@@ -2184,6 +2184,50 @@ final class WorkspaceSplitWorkingDirectoryTests: XCTestCase {
         XCTAssertNil(sourcePanel.surface.surface, "Expected stale surface pointer to be quarantined")
 #else
         throw XCTSkip("Debug-only regression test")
+#endif
+    }
+
+    /// Regression test for the occluded-render re-land crash: 6 identical CI minidumps
+    /// symbolicated to `objc_retain` `EXC_BAD_ACCESS` in
+    /// `GhosttySurfaceCallbackContext.tabId.getter`, reached from
+    /// `GhosttyApp.callbackContext(from:)` resolving a `ghostty_surface_userdata` pointer
+    /// whose backing context had already been released by teardown. This exercises the
+    /// exact stale-pointer shape: capture the live userdata pointer, tear the surface down
+    /// through the real teardown path, then resolve that same pointer and assert it comes
+    /// back `nil` instead of dereferencing freed memory.
+    func testStaleSurfaceUserdataResolvesToNilInsteadOfCrashing() throws {
+#if DEBUG
+        let workspace = Workspace()
+        guard let sourcePanelId = workspace.focusedPanelId,
+              let sourcePanel = workspace.terminalPanel(for: sourcePanelId) else {
+            XCTFail("Expected focused terminal panel")
+            return
+        }
+
+        let window = try hostTerminalPanelInWindow(sourcePanel)
+        defer { window.orderOut(nil) }
+
+        guard let userdata = sourcePanel.surface.debugCallbackUserdataPointer() else {
+            XCTFail("Expected a live callback-context userdata pointer before teardown")
+            return
+        }
+
+        XCTAssertTrue(
+            GhosttyApp.debugCallbackContextResolves(from: userdata),
+            "Expected the live userdata pointer to resolve before teardown"
+        )
+
+        // Tear down through the real teardown path: this is the same release site
+        // (performSurfaceTeardown/liveSurfaceForGhosttyAccess's sibling) that frees the
+        // callback context in production.
+        sourcePanel.surface.replaceSurfaceWithFreedPointerForTesting()
+
+        XCTAssertFalse(
+            GhosttyApp.debugCallbackContextResolves(from: userdata),
+            "Expected a stale userdata pointer to resolve to nil instead of dereferencing freed memory"
+        )
+#else
+        throw XCTSkip("Debug-only regression test")
 #endif
     }
 }