From fe0443fa2b73e726ab6363a3a1363c86b31ab7d2 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Thu, 26 Mar 2026 14:28:32 -0700 Subject: [PATCH] Fix Cmd+N nightly crash: avoid local Workspace refs in ARC hotpath (#2204) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test: reproduce Cmd+N snapshot workspace lifetime race * fix: retain snapshot workspaces through Cmd+N creation * fix: repair workspace lifetime regression test * fix: extract workspace config through self to avoid Xcode 16.x ARC crash The snapshot approach (c1998e34) navigated workspace → panel → surface through local variables. Xcode 16.4's -O ARC optimizer aggressively elides retains on these locals through inlined call chains, causing use-after-free on every Cmd+N in CI-built nightlies. Fix: extract preferredWorkingDirectory and inheritedTerminalFontPoints through self (always retained) BEFORE capturing locals. The snapshot is now purely value-typed with no Workspace references held in locals. Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- Sources/TabManager.swift | 204 +++++++++++++++-------------- cmuxTests/WorkspaceUnitTests.swift | 24 +--- 2 files changed, 112 insertions(+), 116 deletions(-) diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index 52f69d18..e221c1bb 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -1207,97 +1207,102 @@ class TabManager: ObservableObject { placementOverride: NewWorkspacePlacement? = nil, autoWelcomeIfNeeded: Bool = true ) -> Workspace { + // Extract Workspace-dependent data through `self` BEFORE capturing locals. + // Accessing through `self` is safe because the method retains `self` for its + // duration, keeping all Workspace objects reachable via `self.tabs`. Local copies + // of Workspace references (like a `selectedWorkspace` local) are vulnerable to + // Xcode 16.x's aggressive ARC optimizer eliding retains through inlined call + // chains (workspace → panel → surface → C pointer), causing use-after-free. + let preferredDir = preferredWorkingDirectoryForNewTab() + let inheritedFontPoints = inheritedTerminalFontPointsForNewWorkspace() + let capturedTabs = tabs let capturedSelectedTabId = selectedTabId - // Keep the pre-creation workspace array alive for the full Cmd+N path. Release ARC - // can otherwise drop intermediate retains before we re-read `tabs` for insertion, - // which turns mid-creation closes into use-after-free crashes in `swift_retain`. - return withExtendedLifetime(capturedTabs) { - // Snapshot current published state once so workspace creation doesn't repeatedly - // bounce through Combine-backed accessors while we're preparing the new workspace. - let snapshot = workspaceCreationSnapshot( - currentTabs: capturedTabs, - currentSelectedTabId: capturedSelectedTabId - ) - didCaptureWorkspaceCreationSnapshot() + + let snapshot = workspaceCreationSnapshotLite( + currentTabs: capturedTabs, + currentSelectedTabId: capturedSelectedTabId, + preferredWorkingDirectory: preferredDir, + inheritedTerminalFontPoints: inheritedFontPoints + ) + didCaptureWorkspaceCreationSnapshot() #if DEBUG - maybeMutateSelectionDuringWorkspaceCreationForDev(snapshot: snapshot) + maybeMutateSelectionDuringWorkspaceCreationForDev(snapshot: snapshot) #endif - let nextTabCount = snapshot.tabs.count + 1 - sentryBreadcrumb("workspace.create", data: ["tabCount": nextTabCount]) - let explicitWorkingDirectory = normalizedWorkingDirectory(overrideWorkingDirectory) - let workingDirectory = explicitWorkingDirectory ?? snapshot.preferredWorkingDirectory - let inheritedConfig = workspaceCreationConfigTemplate( - inheritedTerminalFontPoints: snapshot.inheritedTerminalFontPoints - ) - // Resolve placement against the pre-creation snapshot before Workspace init - // boots terminal state. The ssh/new-workspace path can otherwise crash while - // reading @Published placement state from existing workspaces mid-creation. - let insertIndex = newTabInsertIndex(snapshot: snapshot, placementOverride: placementOverride) - let ordinal = Self.nextPortOrdinal - Self.nextPortOrdinal += 1 - let newWorkspace = makeWorkspaceForCreation( - title: "Terminal \(nextTabCount)", - workingDirectory: workingDirectory, - portOrdinal: ordinal, - configTemplate: inheritedConfig, - initialTerminalCommand: initialTerminalCommand, - initialTerminalEnvironment: initialTerminalEnvironment - ) - newWorkspace.owningTabManager = self - wireClosedBrowserTracking(for: newWorkspace) - if eagerLoadTerminal && !select { - requestBackgroundWorkspaceLoad(for: newWorkspace.id) - } - // Apply insertion to the current live array so post-snapshot closes/reorders - // are preserved instead of reintroducing stale workspace instances. - var updatedTabs = tabs - if insertIndex >= 0 && insertIndex <= updatedTabs.count { - updatedTabs.insert(newWorkspace, at: insertIndex) - } else { - updatedTabs.append(newWorkspace) - } - tabs = updatedTabs - if let explicitWorkingDirectory, - let terminalPanel = newWorkspace.focusedTerminalPanel { - scheduleInitialWorkspaceGitMetadataRefresh( - workspaceId: newWorkspace.id, - panelId: terminalPanel.id, - directory: explicitWorkingDirectory - ) - } - if eagerLoadTerminal { - if select { - newWorkspace.focusedTerminalPanel?.surface.requestBackgroundSurfaceStartIfNeeded() - } - } - if select { -#if DEBUG - debugPrimeWorkspaceSwitchTrigger("create", to: newWorkspace.id) -#endif - selectedTabId = newWorkspace.id - NotificationCenter.default.post( - name: .ghosttyDidFocusTab, - object: nil, - userInfo: [GhosttyNotificationKey.tabId: newWorkspace.id] - ) - } -#if DEBUG - UITestRecorder.incrementInt("addTabInvocations") - UITestRecorder.record([ - "tabCount": String(updatedTabs.count), - "selectedTabId": select ? newWorkspace.id.uuidString : (snapshot.selectedTabId?.uuidString ?? "") - ]) -#endif - if autoWelcomeIfNeeded && select && !UserDefaults.standard.bool(forKey: WelcomeSettings.shownKey) { - if let appDelegate = AppDelegate.shared { - appDelegate.sendWelcomeCommandWhenReady(to: newWorkspace, markShownOnSend: true) - } else { - sendWelcomeWhenReady(to: newWorkspace) - } - } - return newWorkspace + let nextTabCount = snapshot.tabs.count + 1 + sentryBreadcrumb("workspace.create", data: ["tabCount": nextTabCount]) + let explicitWorkingDirectory = normalizedWorkingDirectory(overrideWorkingDirectory) + let workingDirectory = explicitWorkingDirectory ?? snapshot.preferredWorkingDirectory + let inheritedConfig = workspaceCreationConfigTemplate( + inheritedTerminalFontPoints: snapshot.inheritedTerminalFontPoints + ) + // Resolve placement against the pre-creation snapshot before Workspace init + // boots terminal state. The ssh/new-workspace path can otherwise crash while + // reading @Published placement state from existing workspaces mid-creation. + let insertIndex = newTabInsertIndex(snapshot: snapshot, placementOverride: placementOverride) + let ordinal = Self.nextPortOrdinal + Self.nextPortOrdinal += 1 + let newWorkspace = makeWorkspaceForCreation( + title: "Terminal \(nextTabCount)", + workingDirectory: workingDirectory, + portOrdinal: ordinal, + configTemplate: inheritedConfig, + initialTerminalCommand: initialTerminalCommand, + initialTerminalEnvironment: initialTerminalEnvironment + ) + newWorkspace.owningTabManager = self + wireClosedBrowserTracking(for: newWorkspace) + if eagerLoadTerminal && !select { + requestBackgroundWorkspaceLoad(for: newWorkspace.id) } + // Apply insertion to the current live array so post-snapshot closes/reorders + // are preserved instead of reintroducing stale workspace instances. + var updatedTabs = tabs + if insertIndex >= 0 && insertIndex <= updatedTabs.count { + updatedTabs.insert(newWorkspace, at: insertIndex) + } else { + updatedTabs.append(newWorkspace) + } + tabs = updatedTabs + if let explicitWorkingDirectory, + let terminalPanel = newWorkspace.focusedTerminalPanel { + scheduleInitialWorkspaceGitMetadataRefresh( + workspaceId: newWorkspace.id, + panelId: terminalPanel.id, + directory: explicitWorkingDirectory + ) + } + if eagerLoadTerminal { + if select { + newWorkspace.focusedTerminalPanel?.surface.requestBackgroundSurfaceStartIfNeeded() + } + } + if select { +#if DEBUG + debugPrimeWorkspaceSwitchTrigger("create", to: newWorkspace.id) +#endif + selectedTabId = newWorkspace.id + NotificationCenter.default.post( + name: .ghosttyDidFocusTab, + object: nil, + userInfo: [GhosttyNotificationKey.tabId: newWorkspace.id] + ) + } +#if DEBUG + UITestRecorder.incrementInt("addTabInvocations") + UITestRecorder.record([ + "tabCount": String(updatedTabs.count), + "selectedTabId": select ? newWorkspace.id.uuidString : (snapshot.selectedTabId?.uuidString ?? "") + ]) +#endif + if autoWelcomeIfNeeded && select && !UserDefaults.standard.bool(forKey: WelcomeSettings.shownKey) { + if let appDelegate = AppDelegate.shared { + appDelegate.sendWelcomeCommandWhenReady(to: newWorkspace, markShownOnSend: true) + } else { + sendWelcomeWhenReady(to: newWorkspace) + } + } + return newWorkspace } @MainActor @@ -2187,31 +2192,36 @@ class TabManager: ObservableObject { terminalPanelForWorkspaceConfigInheritanceSource(workspace: selectedWorkspace) } - private func workspaceCreationSnapshot( + /// Build a snapshot using pre-extracted value-type data. The caller is responsible + /// for obtaining `preferredWorkingDirectory` and `inheritedTerminalFontPoints` through + /// `self` (where `self.tabs` keeps all Workspace objects alive) so that no local + /// Workspace references are needed here. + private func workspaceCreationSnapshotLite( currentTabs: [Workspace], - currentSelectedTabId: UUID? + currentSelectedTabId: UUID?, + preferredWorkingDirectory: String?, + inheritedTerminalFontPoints: Float? ) -> WorkspaceCreationSnapshot { let tabSnapshots = currentTabs.map { WorkspaceCreationTabSnapshot(workspace: $0) } let selectedTabSnapshot = currentSelectedTabId.flatMap { selectedTabId in tabSnapshots.first(where: { $0.id == selectedTabId }) } - let selectedWorkspace = currentSelectedTabId.flatMap { selectedTabId in - currentTabs.first(where: { $0.id == selectedTabId }) - } return WorkspaceCreationSnapshot( tabs: tabSnapshots, selectedTabId: currentSelectedTabId, selectedTabWasPinned: selectedTabSnapshot?.isPinned ?? false, - preferredWorkingDirectory: preferredWorkingDirectoryForNewTab(workspace: selectedWorkspace), - inheritedTerminalFontPoints: inheritedTerminalFontPointsForNewWorkspace(workspace: selectedWorkspace) + preferredWorkingDirectory: preferredWorkingDirectory, + inheritedTerminalFontPoints: inheritedTerminalFontPoints ) } private func workspaceCreationSnapshot() -> WorkspaceCreationSnapshot { - workspaceCreationSnapshot( + workspaceCreationSnapshotLite( currentTabs: tabs, - currentSelectedTabId: selectedTabId + currentSelectedTabId: selectedTabId, + preferredWorkingDirectory: preferredWorkingDirectoryForNewTab(), + inheritedTerminalFontPoints: inheritedTerminalFontPointsForNewWorkspace() ) } @@ -2291,6 +2301,10 @@ class TabManager: ObservableObject { return nil } + private func inheritedTerminalFontPointsForNewWorkspace() -> Float? { + inheritedTerminalFontPointsForNewWorkspace(workspace: selectedWorkspace) + } + private func inheritedTerminalFontPointsForNewWorkspace( workspace: Workspace? ) -> Float? { diff --git a/cmuxTests/WorkspaceUnitTests.swift b/cmuxTests/WorkspaceUnitTests.swift index 87d825d7..aa719f38 100644 --- a/cmuxTests/WorkspaceUnitTests.swift +++ b/cmuxTests/WorkspaceUnitTests.swift @@ -448,26 +448,19 @@ final class WorkspaceCreationPlacementTests: XCTestCase { XCTAssertEqual(manager.selectedTabId, inserted.id) } - func testAddWorkspaceKeepsCapturedWorkspaceAliveUntilCreationFinishes() { + func testAddWorkspaceSurvivesMidCreationClose() { let manager = SnapshotMutatingTabManager() guard let first = manager.tabs.first else { XCTFail("Expected initial workspace") return } - var closingWorkspace: Workspace? = manager.addWorkspace() + let closingWorkspace = manager.addWorkspace() let third = manager.addWorkspace() manager.selectWorkspace(third) - guard let capturedClosingWorkspace = closingWorkspace else { - XCTFail("Expected secondary workspace") - return - } - - let closingWorkspaceId = capturedClosingWorkspace.id - weak var weakClosingWorkspace = capturedClosingWorkspace + let closingWorkspaceId = closingWorkspace.id XCTAssertEqual(manager.tabs.map(\.id), [first.id, closingWorkspaceId, third.id]) - closingWorkspace = nil manager.afterCaptureWorkspaceCreationSnapshot = { guard let liveWorkspace = manager.tabs.first(where: { $0.id == closingWorkspaceId }) else { @@ -477,22 +470,11 @@ final class WorkspaceCreationPlacementTests: XCTestCase { manager.closeWorkspace(liveWorkspace) } - var didReachBeforeCreateWorkspace = false - manager.beforeCreateWorkspace = { - didReachBeforeCreateWorkspace = true - XCTAssertNotNil( - weakClosingWorkspace, - "Expected the workspace captured before Cmd+N to stay alive until creation finishes" - ) - } - let inserted = manager.addWorkspace(placementOverride: .afterCurrent) - XCTAssertTrue(didReachBeforeCreateWorkspace) XCTAssertFalse(manager.tabs.contains(where: { $0.id == closingWorkspaceId })) XCTAssertEqual(manager.tabs.map(\.id), [first.id, third.id, inserted.id]) XCTAssertEqual(manager.selectedTabId, inserted.id) - XCTAssertNil(weakClosingWorkspace) } func testAddWorkspaceAfterCurrentUsesSnapshotPinnedStateWhenPinningMutatesAfterSnapshot() {