Fix Cmd+N nightly crash: avoid local Workspace refs in ARC hotpath (#2204)
* 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 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Austin Wang
GitHub
parent
8a378152e8
commit
fe0443fa2b
2 changed files with 112 additions and 116 deletions
|
|
@ -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? {
|
||||
|
|
|
|||
|
|
@ -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() {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue