Release terminal IO callback userdata only after the native surface free

Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Lifecycle/TerminalSurfaceRuntimeTeardownCoordinator.swift

@@ -52,13 +52,64 @@ public actor TerminalSurfaceRuntimeTeardownCoordinator {
         freeSurface: @escaping @Sendable (ghostty_surface_t) -> Void = { surface in
             ghostty_surface_free(surface)
         }
+    ) {
+        enqueueRuntimeTeardown(
+            id: id,
+            workspaceId: workspaceId,
+            reason: reason,
+            surface: surface,
+            callbackContext: callbackContext,
+            manualIOContext: nil,
+            byteTeeLease: nil,
+            freeSurface: freeSurface
+        )
+    }
+
+    /// Queues a native-surface free that also transports the surface's other
+    /// retained callback userdata.
+    ///
+    /// `ghostty_surface_free` is the synchronization point that joins
+    /// ghostty's IO threads: the io-reader thread fires the PTY tee callback
+    /// and the io thread fires the MANUAL-mode `io_write_cb` right up until
+    /// the free returns. Transporting the manual IO context and the byte-tee
+    /// lease through the request keeps their userdata retained across that
+    /// window; the coordinator releases them only after the free completes,
+    /// so no in-flight callback can dereference freed userdata.
+    ///
+    /// - Parameters:
+    ///   - id: The owning surface id.
+    ///   - workspaceId: The owning workspace id.
+    ///   - reason: The teardown reason, for diagnostics.
+    ///   - surface: The native surface pointer, already removed from all
+    ///     main-thread owner state.
+    ///   - callbackContext: The retained callback context released on the
+    ///     main actor after the free completes.
+    ///   - manualIOContext: The retained MANUAL-mode `io_write_cb` userdata,
+    ///     released on the main actor after the free completes.
+    ///   - byteTeeLease: The retained PTY tee lease, released on the main
+    ///     actor after the free completes.
+    ///   - freeSurface: The free operation; defaults to
+    ///     `ghostty_surface_free`.
+    nonisolated func enqueueRuntimeTeardown(
+        id: UUID,
+        workspaceId: UUID,
+        reason: String,
+        surface: ghostty_surface_t,
+        callbackContext: Unmanaged<GhosttySurfaceCallbackContext>?,
+        manualIOContext: Unmanaged<TerminalManualIOWriteBox>?,
+        byteTeeLease: (any TerminalByteTeeLease)?,
+        freeSurface: @escaping @Sendable (ghostty_surface_t) -> Void = { surface in
+            ghostty_surface_free(surface)
+        }
     ) {
         let request = TerminalSurfaceRuntimeTeardownRequest(
             id: id,
             workspaceId: workspaceId,
             reason: reason,
             surface: surface,
             callbackContext: callbackContext,
+            manualIOContext: manualIOContext,
+            byteTeeLease: byteTeeLease,
             freeSurface: freeSurface
         )
         Task {
@@ -99,12 +150,18 @@ public actor TerminalSurfaceRuntimeTeardownCoordinator {
         )
 #endif
         request.freeSurface(request.surface)
-        if request.callbackContext != nil {
+        if request.callbackContext != nil || request.manualIOContext != nil || request.byteTeeLease != nil {
             // The request is the @unchecked Sendable transport for the
-            // Unmanaged context; release through the request so the @Sendable
+            // Unmanaged contexts; release through the request so the @Sendable
             // closure never captures the non-Sendable Unmanaged directly.
+            // Ordered after freeSurface: the native free joins ghostty's IO
+            // threads, so no tee/io_write callback can still hold this
+            // userdata. The byte-tee lease goes last so tests can use its
+            // release as the "all userdata released" beacon.
             await MainActor.run {
                 request.callbackContext?.release()
+                request.manualIOContext?.release()
+                request.byteTeeLease?.release()
             }
         }
 #if DEBUG

Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Lifecycle/TerminalSurfaceRuntimeTeardownRequest.swift

@@ -7,14 +7,23 @@ public import CmuxTerminalCore
 /// The native pointer has been removed from all main-thread owner state
 /// before this request is created; this wrapper only transports the one-shot
 /// free. It is `@unchecked Sendable` for exactly that reason: the surface
-/// pointer and `Unmanaged` callback context are exclusively owned by the
-/// request from creation until the coordinator consumes them.
+/// pointer, the `Unmanaged` callback contexts, and the byte-tee lease are
+/// exclusively owned by the request from creation until the coordinator
+/// consumes them.
+///
+/// The transported callback userdata (`callbackContext`, `manualIOContext`,
+/// `byteTeeLease`) is released only after `freeSurface` returns: the native
+/// free joins ghostty's IO threads (the io-reader thread that fires the PTY
+/// tee callback and the io thread that fires the MANUAL-mode `io_write_cb`),
+/// so a release ordered after the free can never race an in-flight callback.
 struct TerminalSurfaceRuntimeTeardownRequest: @unchecked Sendable {
     let id: UUID
     let workspaceId: UUID
     let reason: String
     let surface: ghostty_surface_t
     let callbackContext: Unmanaged<GhosttySurfaceCallbackContext>?
+    let manualIOContext: Unmanaged<TerminalManualIOWriteBox>?
+    let byteTeeLease: (any TerminalByteTeeLease)?
     let freeSurface: @Sendable (ghostty_surface_t) -> Void
 #if DEBUG
     let surfaceToken: String
@@ -27,13 +36,17 @@ struct TerminalSurfaceRuntimeTeardownRequest: @unchecked Sendable {
         reason: String,
         surface: ghostty_surface_t,
         callbackContext: Unmanaged<GhosttySurfaceCallbackContext>?,
+        manualIOContext: Unmanaged<TerminalManualIOWriteBox>?,
+        byteTeeLease: (any TerminalByteTeeLease)?,
         freeSurface: @escaping @Sendable (ghostty_surface_t) -> Void
     ) {
         self.id = id
         self.workspaceId = workspaceId
         self.reason = reason
         self.surface = surface
         self.callbackContext = callbackContext
+        self.manualIOContext = manualIOContext
+        self.byteTeeLease = byteTeeLease
         self.freeSurface = freeSurface
 #if DEBUG
         self.surfaceToken = String(id.uuidString.prefix(5))

Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+RuntimeLifecycle.swift

@@ -263,18 +263,19 @@ extension TerminalSurface {
 
 #if DEBUG
         if let freeSurface = Self.runtimeSurfaceFreeOverrideForTesting {
+            // Transport manualIOContext and teeLease through the request too:
+            // the coordinator releases all callback userdata only after the
+            // native free, which is what joins ghostty's IO threads.
             runtimeTeardown.enqueueRuntimeTeardown(
                 id: id,
                 workspaceId: tabId,
                 reason: "teardown",
                 surface: surfaceToFree,
                 callbackContext: callbackContext,
+                manualIOContext: manualIOContext,
+                byteTeeLease: teeLease,
                 freeSurface: freeSurface
             )
-            // The teardown coordinator releases callbackContext; manualIOContext
-            // and teeLease are not transported through the request, so release them here.
-            manualIOContext?.release()
-            teeLease?.release()
             return
         }
 #endif
@@ -332,18 +333,19 @@ extension TerminalSurface {
 
 #if DEBUG
         if let freeSurface = Self.runtimeSurfaceFreeOverrideForTesting {
+            // Transport manualIOContext and teeLease through the request too:
+            // the coordinator releases all callback userdata only after the
+            // native free, which is what joins ghostty's IO threads.
             runtimeTeardown.enqueueRuntimeTeardown(
                 id: id,
                 workspaceId: tabId,
                 reason: reason,
                 surface: surfaceToFree,
                 callbackContext: callbackContext,
+                manualIOContext: manualIOContext,
+                byteTeeLease: teeLease,
                 freeSurface: freeSurface
             )
-            // The teardown coordinator releases callbackContext; manualIOContext
-            // and teeLease are not transported through the request, so release them here.
-            manualIOContext?.release()
-            teeLease?.release()
             return
         }
 #endif

Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swift

@@ -179,8 +179,11 @@ public final class TerminalSurface: Identifiable, ObservableObject {
     /// For MANUAL-I/O remote tmux display surfaces: whether to suppress
     /// ghostty primary-screen reflow on resize.
     var manualIONoReflow = true
-    /// Retained userdata for the MANUAL-mode `io_write_cb`; released alongside
-    /// the surface.
+    /// Retained userdata for the MANUAL-mode `io_write_cb`. ghostty's io
+    /// thread can invoke the callback until `ghostty_surface_free` returns, so
+    /// every teardown path releases this strictly after the native free
+    /// (inline in the free task, or transported through the teardown
+    /// coordinator's request).
     var manualIOContext: Unmanaged<TerminalManualIOWriteBox>?
     /// Output delivered before the runtime surface exists. Flushed once the
     /// surface is created so background mirror output is not lost.
@@ -243,10 +246,16 @@ public final class TerminalSurface: Identifiable, ObservableObject {
     var claudeCommandShimPendingCreationSource: RuntimeSurfaceCreationSource?
     /// The retained byte-tee lease for the libghostty PTY tee callback (cmux
     /// fork extension). Installed in `createSurface` after
-    /// `ghostty_surface_new` succeeds; released alongside
-    /// `surfaceCallbackContext` whenever we tear down or rebuild the
-    /// surface. The Mac sync server reads the tee'd bytes to broadcast
-    /// raw PTY output to paired iPhones (`MobileTerminalByteTee`).
+    /// `ghostty_surface_new` succeeds. The Mac sync server reads the tee'd
+    /// bytes to broadcast raw PTY output to paired iPhones
+    /// (`MobileTerminalByteTee`).
+    ///
+    /// Lifetime: the tee callback fires on ghostty's io-reader thread for
+    /// every output chunk until `ghostty_surface_free` joins that thread, so
+    /// every teardown path releases the lease strictly after the native free
+    /// (inline in the free task, or transported through the teardown
+    /// coordinator's request). Releasing earlier is a use-after-free on the
+    /// io-reader thread.
     var mobileByteTeeLease: (any TerminalByteTeeLease)?
     /// The desired focus state for the Ghostty C surface. May be set before the
     /// C surface exists (e.g. during layout restoration); `createSurface`
@@ -562,19 +571,20 @@ public final class TerminalSurface: Identifiable, ObservableObject {
 #endif
 
         // Keep teardown asynchronous to avoid re-entrant close/deinit loops, but retain
-        // callback userdata until surface free completes so callbacks never dereference
-        // a deallocated view pointer.
+        // ALL callback userdata until the native free completes: ghostty's io-reader
+        // thread keeps firing the PTY tee callback (and the io thread the MANUAL-mode
+        // io_write_cb) until ghostty_surface_free joins those threads, so releasing
+        // manualIOContext or teeLease here would leave a use-after-free window until
+        // the coordinator's deferred free runs.
         runtimeTeardown.enqueueRuntimeTeardown(
             id: id,
             workspaceId: tabId,
             reason: "deinit",
             surface: surfaceToFree,
-            callbackContext: callbackContext
+            callbackContext: callbackContext,
+            manualIOContext: manualIOContext,
+            byteTeeLease: teeLease
         )
-        // The teardown coordinator releases callbackContext; manualIOContext and
-        // teeLease are not transported through the request, so release them here.
-        manualIOContext?.release()
-        teeLease?.release()
     }
 }
 

Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/RecordingTerminalByteTeeLease.swift

@@ -0,0 +1,15 @@
+@testable import CmuxTerminal
+
+/// A byte-tee lease that reports its release to a ``TeardownOrderRecorder``
+/// so lifetime tests can assert release ordering against the native free.
+final class RecordingTerminalByteTeeLease: TerminalByteTeeLease {
+    private let recorder: TeardownOrderRecorder
+
+    init(recorder: TeardownOrderRecorder) {
+        self.recorder = recorder
+    }
+
+    func release() {
+        recorder.record(.teeLeaseRelease)
+    }
+}

Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TeardownOrderRecorder.swift

@@ -0,0 +1,53 @@
+import Foundation
+
+/// Records the order of teardown events emitted by synchronous callbacks (an
+/// injected native free running on the teardown coordinator's worker, a byte-tee
+/// lease release) and lets tests await a target event count without polling.
+///
+/// @unchecked Sendable: all state is guarded by `lock`; the recording entry
+/// points are synchronous callbacks with no async context (the sanctioned lock
+/// carve-out for off-isolation compare-and-set).
+final class TeardownOrderRecorder: @unchecked Sendable {
+    enum Event: Equatable, Sendable {
+        case nativeFree
+        case teeLeaseRelease
+    }
+
+    private let lock = NSLock()
+    private var storedEvents: [Event] = []
+    private var waiters: [(count: Int, continuation: CheckedContinuation<Void, Never>)] = []
+
+    /// The events recorded so far, in order.
+    var events: [Event] {
+        lock.lock()
+        defer { lock.unlock() }
+        return storedEvents
+    }
+
+    /// Records an event and resumes any waiter whose target count is reached.
+    func record(_ event: Event) {
+        lock.lock()
+        storedEvents.append(event)
+        let count = storedEvents.count
+        let resumable = waiters.filter { $0.count <= count }.map(\.continuation)
+        waiters.removeAll { $0.count <= count }
+        lock.unlock()
+        for continuation in resumable {
+            continuation.resume()
+        }
+    }
+
+    /// Suspends until at least `count` events have been recorded.
+    func waitForEventCount(_ count: Int) async {
+        await withCheckedContinuation { continuation in
+            lock.lock()
+            if storedEvents.count >= count {
+                lock.unlock()
+                continuation.resume()
+                return
+            }
+            waiters.append((count, continuation))
+            lock.unlock()
+        }
+    }
+}