Fix crash when creating a new workspace via snapshot-based state
Snapshot TabManager's published state (tabs, selectedTabId) at the start of workspace creation so mutations don't bounce through Combine-backed accessors mid-creation. Prevents crashes when adding a workspace via Cmd+N, the new-workspace button, or any other creation path that triggers re-entrant publishes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
e52959a528
commit
c1998e347c
2 changed files with 141 additions and 20 deletions
|
|
@ -754,6 +754,15 @@ class TabManager: ObservableObject {
|
|||
private var pendingWorkspaceUnfocusTarget: (tabId: UUID, panelId: UUID)?
|
||||
private var sidebarSelectedWorkspaceIds: Set<UUID> = []
|
||||
var confirmCloseHandler: ((String, String, Bool) -> Bool)?
|
||||
private struct WorkspaceCreationSnapshot {
|
||||
let tabs: [Workspace]
|
||||
let selectedTabId: UUID?
|
||||
|
||||
var selectedWorkspace: Workspace? {
|
||||
guard let selectedTabId else { return nil }
|
||||
return tabs.first(where: { $0.id == selectedTabId })
|
||||
}
|
||||
}
|
||||
#if DEBUG
|
||||
private var debugWorkspaceSwitchCounter: UInt64 = 0
|
||||
private var debugWorkspaceSwitchId: UInt64 = 0
|
||||
|
|
@ -927,26 +936,32 @@ class TabManager: ObservableObject {
|
|||
placementOverride: NewWorkspacePlacement? = nil,
|
||||
autoWelcomeIfNeeded: Bool = true
|
||||
) -> Workspace {
|
||||
sentryBreadcrumb("workspace.create", data: ["tabCount": tabs.count + 1])
|
||||
// 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()
|
||||
let nextTabCount = snapshot.tabs.count + 1
|
||||
sentryBreadcrumb("workspace.create", data: ["tabCount": nextTabCount])
|
||||
let explicitWorkingDirectory = normalizedWorkingDirectory(overrideWorkingDirectory)
|
||||
let workingDirectory = explicitWorkingDirectory ?? preferredWorkingDirectoryForNewTab()
|
||||
let inheritedConfig = inheritedTerminalConfigForNewWorkspace()
|
||||
let workingDirectory = explicitWorkingDirectory ?? preferredWorkingDirectoryForNewTab(snapshot: snapshot)
|
||||
let inheritedConfig = inheritedTerminalConfigForNewWorkspace(snapshot: snapshot)
|
||||
let ordinal = Self.nextPortOrdinal
|
||||
Self.nextPortOrdinal += 1
|
||||
let newWorkspace = Workspace(
|
||||
title: "Terminal \(tabs.count + 1)",
|
||||
title: "Terminal \(nextTabCount)",
|
||||
workingDirectory: workingDirectory,
|
||||
portOrdinal: ordinal,
|
||||
configTemplate: inheritedConfig
|
||||
)
|
||||
newWorkspace.owningTabManager = self
|
||||
wireClosedBrowserTracking(for: newWorkspace)
|
||||
let insertIndex = newTabInsertIndex(placementOverride: placementOverride)
|
||||
if insertIndex >= 0 && insertIndex <= tabs.count {
|
||||
tabs.insert(newWorkspace, at: insertIndex)
|
||||
let insertIndex = newTabInsertIndex(snapshot: snapshot, placementOverride: placementOverride)
|
||||
var updatedTabs = snapshot.tabs
|
||||
if insertIndex >= 0 && insertIndex <= updatedTabs.count {
|
||||
updatedTabs.insert(newWorkspace, at: insertIndex)
|
||||
} else {
|
||||
tabs.append(newWorkspace)
|
||||
updatedTabs.append(newWorkspace)
|
||||
}
|
||||
tabs = updatedTabs
|
||||
if let explicitWorkingDirectory,
|
||||
let terminalPanel = newWorkspace.focusedTerminalPanel {
|
||||
scheduleInitialWorkspaceGitMetadataRefresh(
|
||||
|
|
@ -973,8 +988,8 @@ class TabManager: ObservableObject {
|
|||
#if DEBUG
|
||||
UITestRecorder.incrementInt("addTabInvocations")
|
||||
UITestRecorder.record([
|
||||
"tabCount": String(tabs.count),
|
||||
"selectedTabId": select ? newWorkspace.id.uuidString : (selectedTabId?.uuidString ?? "")
|
||||
"tabCount": String(updatedTabs.count),
|
||||
"selectedTabId": select ? newWorkspace.id.uuidString : (snapshot.selectedTabId?.uuidString ?? "")
|
||||
])
|
||||
#endif
|
||||
if autoWelcomeIfNeeded && select && !UserDefaults.standard.bool(forKey: WelcomeSettings.shownKey) {
|
||||
|
|
@ -1202,7 +1217,20 @@ class TabManager: ObservableObject {
|
|||
}
|
||||
|
||||
func terminalPanelForWorkspaceConfigInheritanceSource() -> TerminalPanel? {
|
||||
guard let workspace = selectedWorkspace else { return nil }
|
||||
terminalPanelForWorkspaceConfigInheritanceSource(snapshot: workspaceCreationSnapshot())
|
||||
}
|
||||
|
||||
private func workspaceCreationSnapshot() -> WorkspaceCreationSnapshot {
|
||||
WorkspaceCreationSnapshot(
|
||||
tabs: tabs,
|
||||
selectedTabId: selectedTabId
|
||||
)
|
||||
}
|
||||
|
||||
private func terminalPanelForWorkspaceConfigInheritanceSource(
|
||||
snapshot: WorkspaceCreationSnapshot
|
||||
) -> TerminalPanel? {
|
||||
guard let workspace = snapshot.selectedWorkspace else { return nil }
|
||||
if let focusedTerminal = workspace.focusedTerminalPanel {
|
||||
return focusedTerminal
|
||||
}
|
||||
|
|
@ -1217,13 +1245,19 @@ class TabManager: ObservableObject {
|
|||
}
|
||||
|
||||
private func inheritedTerminalConfigForNewWorkspace() -> ghostty_surface_config_s? {
|
||||
if let sourceSurface = terminalPanelForWorkspaceConfigInheritanceSource()?.surface.surface {
|
||||
inheritedTerminalConfigForNewWorkspace(snapshot: workspaceCreationSnapshot())
|
||||
}
|
||||
|
||||
private func inheritedTerminalConfigForNewWorkspace(
|
||||
snapshot: WorkspaceCreationSnapshot
|
||||
) -> ghostty_surface_config_s? {
|
||||
if let sourceSurface = terminalPanelForWorkspaceConfigInheritanceSource(snapshot: snapshot)?.surface.surface {
|
||||
return cmuxInheritedSurfaceConfig(
|
||||
sourceSurface: sourceSurface,
|
||||
context: GHOSTTY_SURFACE_CONTEXT_TAB
|
||||
)
|
||||
}
|
||||
if let fallbackFontPoints = selectedWorkspace?.lastRememberedTerminalFontPointsForConfigInheritance() {
|
||||
if let fallbackFontPoints = snapshot.selectedWorkspace?.lastRememberedTerminalFontPointsForConfigInheritance() {
|
||||
var config = ghostty_surface_config_new()
|
||||
config.font_size = fallbackFontPoints
|
||||
return config
|
||||
|
|
@ -1239,24 +1273,36 @@ class TabManager: ObservableObject {
|
|||
}
|
||||
|
||||
private func newTabInsertIndex(placementOverride: NewWorkspacePlacement? = nil) -> Int {
|
||||
newTabInsertIndex(snapshot: workspaceCreationSnapshot(), placementOverride: placementOverride)
|
||||
}
|
||||
|
||||
private func newTabInsertIndex(
|
||||
snapshot: WorkspaceCreationSnapshot,
|
||||
placementOverride: NewWorkspacePlacement? = nil
|
||||
) -> Int {
|
||||
let placement = placementOverride ?? WorkspacePlacementSettings.current()
|
||||
let pinnedCount = tabs.filter { $0.isPinned }.count
|
||||
let selectedIndex = selectedTabId.flatMap { tabId in
|
||||
tabs.firstIndex(where: { $0.id == tabId })
|
||||
let pinnedCount = snapshot.tabs.filter { $0.isPinned }.count
|
||||
let selectedIndex = snapshot.selectedTabId.flatMap { tabId in
|
||||
snapshot.tabs.firstIndex(where: { $0.id == tabId })
|
||||
}
|
||||
let selectedIsPinned = selectedIndex.map { tabs[$0].isPinned } ?? false
|
||||
let selectedIsPinned = selectedIndex.map { snapshot.tabs[$0].isPinned } ?? false
|
||||
return WorkspacePlacementSettings.insertionIndex(
|
||||
placement: placement,
|
||||
selectedIndex: selectedIndex,
|
||||
selectedIsPinned: selectedIsPinned,
|
||||
pinnedCount: pinnedCount,
|
||||
totalCount: tabs.count
|
||||
totalCount: snapshot.tabs.count
|
||||
)
|
||||
}
|
||||
|
||||
private func preferredWorkingDirectoryForNewTab() -> String? {
|
||||
guard let selectedTabId,
|
||||
let tab = tabs.first(where: { $0.id == selectedTabId }) else {
|
||||
preferredWorkingDirectoryForNewTab(snapshot: workspaceCreationSnapshot())
|
||||
}
|
||||
|
||||
private func preferredWorkingDirectoryForNewTab(
|
||||
snapshot: WorkspaceCreationSnapshot
|
||||
) -> String? {
|
||||
guard let tab = snapshot.selectedWorkspace else {
|
||||
return nil
|
||||
}
|
||||
let focusedDirectory = tab.focusedPanelId
|
||||
|
|
|
|||
|
|
@ -277,6 +277,71 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
|
|||
XCTAssertNil(appDelegate.tabManagerFor(windowId: orphanWindowId), "Orphaned context should be pruned after failed resolution")
|
||||
}
|
||||
|
||||
func testCustomCmdTNewWorkspacePrunesOrphanedContextWithoutLiveWindow() {
|
||||
guard let appDelegate = AppDelegate.shared else {
|
||||
XCTFail("Expected AppDelegate.shared")
|
||||
return
|
||||
}
|
||||
|
||||
let existingWindowIds = mainWindowIds()
|
||||
let orphanWindowId = UUID()
|
||||
let orphanManager = TabManager()
|
||||
let orphanSidebarState = SidebarState()
|
||||
let orphanSidebarSelectionState = SidebarSelectionState()
|
||||
|
||||
autoreleasepool {
|
||||
var orphanWindow: NSWindow? = NSWindow(
|
||||
contentRect: NSRect(x: 0, y: 0, width: 320, height: 240),
|
||||
styleMask: [.titled, .closable, .resizable],
|
||||
backing: .buffered,
|
||||
defer: false
|
||||
)
|
||||
orphanWindow?.identifier = NSUserInterfaceItemIdentifier("cmux.main.\(orphanWindowId.uuidString)")
|
||||
appDelegate.registerMainWindow(
|
||||
orphanWindow!,
|
||||
windowId: orphanWindowId,
|
||||
tabManager: orphanManager,
|
||||
sidebarState: orphanSidebarState,
|
||||
sidebarSelectionState: orphanSidebarSelectionState
|
||||
)
|
||||
orphanWindow = nil
|
||||
}
|
||||
|
||||
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))
|
||||
|
||||
XCTAssertNil(appDelegate.mainWindow(for: orphanWindowId), "Test precondition: orphaned context should not have a live window")
|
||||
|
||||
let orphanCount = orphanManager.tabs.count
|
||||
let remappedCmdT = StoredShortcut(key: "t", command: true, shift: false, option: false, control: false)
|
||||
|
||||
withTemporaryShortcut(action: .newTab, shortcut: remappedCmdT) {
|
||||
guard let event = makeKeyDownEvent(
|
||||
key: "t",
|
||||
modifiers: [.command],
|
||||
keyCode: 17, // kVK_ANSI_T
|
||||
windowNumber: 0
|
||||
) else {
|
||||
XCTFail("Failed to construct remapped Cmd+T event")
|
||||
return
|
||||
}
|
||||
|
||||
#if DEBUG
|
||||
XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event))
|
||||
#else
|
||||
XCTFail("debugHandleCustomShortcut is only available in DEBUG")
|
||||
#endif
|
||||
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))
|
||||
}
|
||||
|
||||
XCTAssertEqual(orphanManager.tabs.count, orphanCount, "Orphaned manager must not receive a new workspace from remapped Cmd+T")
|
||||
XCTAssertNil(appDelegate.tabManagerFor(windowId: orphanWindowId), "Remapped Cmd+T should prune the orphaned context after failed resolution")
|
||||
|
||||
let createdWindowIds = mainWindowIds().subtracting(existingWindowIds)
|
||||
for windowId in createdWindowIds {
|
||||
closeWindow(withId: windowId)
|
||||
}
|
||||
}
|
||||
|
||||
func testCmdDigitRoutesToEventWindowWhenActiveManagerIsStale() {
|
||||
guard let appDelegate = AppDelegate.shared else {
|
||||
XCTFail("Expected AppDelegate.shared")
|
||||
|
|
@ -2378,6 +2443,16 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
|
|||
return NSApp.windows.first(where: { $0.identifier?.rawValue == identifier })
|
||||
}
|
||||
|
||||
private func mainWindowIds() -> Set<UUID> {
|
||||
Set(NSApp.windows.compactMap { window in
|
||||
guard let raw = window.identifier?.rawValue,
|
||||
raw.hasPrefix("cmux.main.") else {
|
||||
return nil
|
||||
}
|
||||
return UUID(uuidString: String(raw.dropFirst("cmux.main.".count)))
|
||||
})
|
||||
}
|
||||
|
||||
private func closeWindow(withId windowId: UUID) {
|
||||
guard let window = window(withId: windowId) else { return }
|
||||
window.performClose(nil)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue