Skip to content

Fix Cmd+N nightly crash: avoid local Workspace refs in ARC hotpath #2204

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
austinywang merged 5 commits into from
Mar 26, 2026
Merged

Fix Cmd+N nightly crash: avoid local Workspace refs in ARC hotpath #2204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9cd142a
test: reproduce Cmd+N snapshot workspace lifetime race
austinywang Mar 26, 2026
09872b6
fix: retain snapshot workspaces through Cmd+N creation
austinywang Mar 26, 2026
8085719
fix: repair workspace lifetime regression test
austinywang Mar 26, 2026
c5837db
fix: extract workspace config through self to avoid Xcode 16.x ARC crash
austinywang Mar 26, 2026
71d89b2
Merge origin/main into issue-2180-cmd-n-retain-crash
austinywang Mar 26, 2026
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
190 changes: 102 additions & 88 deletions Sources/TabManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
)
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 eagerLoadTerminal {
if select {
newWorkspace.focusedTerminalPanel?.surface.requestBackgroundSurfaceStartIfNeeded()
}
}
if select {
#if DEBUG
debugPrimeWorkspaceSwitchTrigger("create", to: newWorkspace.id)
debugPrimeWorkspaceSwitchTrigger("create", to: newWorkspace.id)
#endif
selectedTabId = newWorkspace.id
NotificationCenter.default.post(
name: .ghosttyDidFocusTab,
object: nil,
userInfo: [GhosttyNotificationKey.tabId: newWorkspace.id]
)
}
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 ?? "")
])
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)
}
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
}
return newWorkspace
}

@MainActor
Expand Down Expand Up @@ -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()
)
}

Expand Down Expand Up @@ -2291,6 +2301,10 @@ class TabManager: ObservableObject {
return nil
}

private func inheritedTerminalFontPointsForNewWorkspace() -> Float? {
inheritedTerminalFontPointsForNewWorkspace(workspace: selectedWorkspace)
}

private func inheritedTerminalFontPointsForNewWorkspace(
workspace: Workspace?
) -> Float? {
Expand Down
24 changes: 3 additions & 21 deletions cmuxTests/WorkspaceUnitTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
Comment on lines +451 to +478

@greptile-apps greptile-apps Bot Mar 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Regression test commit policy: test and fix landed in same commit

Per the CLAUDE.md regression test policy, new tests for a bug fix should be committed separately before the fix so that CI goes red first (proving the test catches the bug), then green after the fix lands.

This new test (testAddWorkspaceSurvivesMidCreationClose) and the production fix were committed together as c5837dbd. If the test actually fails on the unfixed code, it should have been split into two commits to demonstrate that on CI.

That said, because the underlying bug is an ARC optimizer use-after-free that manifests only in Release/CI builds (not easily provable in unit tests), and this test appears to exercise the behavioral outcome of a mid-creation close of a non-selected workspace (rather than the crash path directly), it may well pass on the unfixed code — in which case there is nothing to "prove red" and the single-commit structure is fine. If this test doesn't actually fail on the pre-fix code, a short comment on the test explaining that would be helpful context for future readers.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


func testAddWorkspaceAfterCurrentUsesSnapshotPinnedStateWhenPinningMutatesAfterSnapshot() {
Expand Down
Loading