From 2202044af4a38cb414eab3e41c326bde655f618d Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Fri, 27 Feb 2026 01:42:17 -0800 Subject: [PATCH] Fix drag-handle crash on launch from stale foreign-window events (#490) (#620) Add window-identity check to windowDragHandleShouldCaptureHit so stale leftMouseDown events from other apps (Finder, Dock) during launch don't trigger the SwiftUI hierarchy walk while initial layout is mutating. Add NSLock to breadcrumb limiter for thread safety. Update existing tests to pass eventWindow for window-attached drag handles. --- Sources/WindowDragHandleView.swift | 63 +++++++++++----- cmuxTests/CmuxWebViewKeyEquivalentTests.swift | 75 ++++++++++++++++--- 2 files changed, 112 insertions(+), 26 deletions(-) diff --git a/Sources/WindowDragHandleView.swift b/Sources/WindowDragHandleView.swift index 4238c04a..d8d23f7c 100644 --- a/Sources/WindowDragHandleView.swift +++ b/Sources/WindowDragHandleView.swift @@ -11,9 +11,13 @@ private func windowDragHandleEventTypeDescription(_ eventType: NSEvent.EventType } private enum WindowDragHandleBreadcrumbLimiter { + private static let lock = NSLock() private static var lastEmissionByKey: [String: CFAbsoluteTime] = [:] static func shouldEmit(key: String, minInterval: CFTimeInterval) -> Bool { + lock.lock() + defer { lock.unlock() } + let now = CFAbsoluteTimeGetCurrent() if let previous = lastEmissionByKey[key], (now - previous) < minInterval { return false @@ -55,17 +59,27 @@ private func windowDragHandleEmitBreadcrumb( sentryBreadcrumb(message, category: "titlebar.drag", data: data) } -private func windowDragHandleShouldDeferHitCapture(for eventType: NSEvent.EventType?) -> Bool { - switch eventType { - case .leftMouseDown?: +private func windowDragHandleShouldResolveActiveHitCapture( + for eventType: NSEvent.EventType?, + eventWindow: NSWindow?, + dragHandleWindow: NSWindow? +) -> Bool { + // We only need active hit resolution for titlebar mouse-down handling. + // During launch, NSApp.currentEvent can transiently point at a stale + // leftMouseDown from outside this window (for example Finder/Dock + // activation). Treat those as passive events so we never walk SwiftUI/ + // AppKit hierarchy while initial layout is mutating it. + guard eventType == .leftMouseDown else { return false - default: - // Only left-mouse-down needs the full view-hierarchy walk. - // All other events (mouseMoved, cursorUpdate, activation, nil, …) - // bail out immediately so we never re-enter SwiftUI views during - // a layout pass — which causes exclusive-access crashes (#490). + } + guard let dragHandleWindow else { + // Test-only views may not be attached to a window. return true } + guard let eventWindow else { + return false + } + return eventWindow === dragHandleWindow } /// Runs the same action macOS titlebars use for double-click: @@ -210,19 +224,22 @@ func windowDragHandleShouldTreatTopHitAsPassiveHost(_ view: NSView) -> Bool { func windowDragHandleShouldCaptureHit( _ point: NSPoint, in dragHandleView: NSView, - eventType: NSEvent.EventType? = NSApp.currentEvent?.type + eventType: NSEvent.EventType? = NSApp.currentEvent?.type, + eventWindow: NSWindow? = NSApp.currentEvent?.window ) -> Bool { + let dragHandleWindow = dragHandleView.window + // Suppression recovery runs first so stale depth is cleared even for // passive events — the associated-object reads/writes here are pure ObjC // runtime calls and cannot trigger Swift exclusive-access violations. - if isWindowDragSuppressed(window: dragHandleView.window) { + if isWindowDragSuppressed(window: dragHandleWindow) { // Recover from stale suppression if a prior interaction missed cleanup. // We only keep suppression active while the left mouse button is down. if (NSEvent.pressedMouseButtons & 0x1) == 0 { - let clearedDepth = clearWindowDragSuppression(window: dragHandleView.window) + let clearedDepth = clearWindowDragSuppression(window: dragHandleWindow) windowDragHandleEmitBreadcrumb( "titlebar.dragHandle.suppression.recovered", - window: dragHandleView.window, + window: dragHandleWindow, eventType: eventType, point: point, minInterval: 20, @@ -237,7 +254,7 @@ func windowDragHandleShouldCaptureHit( #endif } else { #if DEBUG - let depth = windowDragSuppressionDepth(window: dragHandleView.window) + let depth = windowDragSuppressionDepth(window: dragHandleWindow) dlog( "titlebar.dragHandle.hitTest capture=false reason=suppressed depth=\(depth) point=\(windowDragHandleFormatPoint(point))" ) @@ -248,11 +265,17 @@ func windowDragHandleShouldCaptureHit( // Bail out before the view-hierarchy walk so we never re-enter SwiftUI // views during a layout pass — which causes exclusive-access crashes (#490). - if windowDragHandleShouldDeferHitCapture(for: eventType) { + if !windowDragHandleShouldResolveActiveHitCapture( + for: eventType, + eventWindow: eventWindow, + dragHandleWindow: dragHandleWindow + ) { #if DEBUG let eventTypeDescription = eventType.map { String(describing: $0) } ?? "nil" + let eventWindowNumber = eventWindow?.windowNumber ?? -1 + let dragWindowNumber = dragHandleWindow?.windowNumber ?? -1 dlog( - "titlebar.dragHandle.hitTest capture=false reason=passiveEvent eventType=\(eventTypeDescription) point=\(windowDragHandleFormatPoint(point))" + "titlebar.dragHandle.hitTest capture=false reason=passiveEvent eventType=\(eventTypeDescription) eventWindow=\(eventWindowNumber) dragWindow=\(dragWindowNumber) point=\(windowDragHandleFormatPoint(point))" ) #endif return false @@ -300,7 +323,7 @@ func windowDragHandleShouldCaptureHit( #endif windowDragHandleEmitBreadcrumb( "titlebar.dragHandle.hitTest.blockedBySiblingHit", - window: dragHandleView.window, + window: dragHandleWindow, eventType: eventType, point: point, minInterval: 8, @@ -335,7 +358,13 @@ struct WindowDragHandleView: NSViewRepresentable { override var mouseDownCanMoveWindow: Bool { false } override func hitTest(_ point: NSPoint) -> NSView? { - let shouldCapture = windowDragHandleShouldCaptureHit(point, in: self) + let currentEvent = NSApp.currentEvent + let shouldCapture = windowDragHandleShouldCaptureHit( + point, + in: self, + eventType: currentEvent?.type, + eventWindow: currentEvent?.window + ) #if DEBUG dlog( "titlebar.dragHandle.hitTestResult capture=\(shouldCapture) point=\(windowDragHandleFormatPoint(point)) window=\(window != nil)" diff --git a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift index 36f0ec07..bb1c2f9b 100644 --- a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift +++ b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift @@ -6133,7 +6133,7 @@ final class WindowDragHandleHitTests: XCTestCase { private final class ReentrantDragHandleView: NSView { override func hitTest(_ point: NSPoint) -> NSView? { - let shouldCapture = windowDragHandleShouldCaptureHit(point, in: self, eventType: .leftMouseDown) + let shouldCapture = windowDragHandleShouldCaptureHit(point, in: self, eventType: .leftMouseDown, eventWindow: self.window) return shouldCapture ? self : nil } } @@ -6196,6 +6196,67 @@ final class WindowDragHandleHitTests: XCTestCase { XCTAssertTrue(windowDragHandleShouldCaptureHit(point, in: dragHandle, eventType: .leftMouseDown)) } + func testDragHandleSkipsForeignLeftMouseDownDuringLaunch() { + let point = NSPoint(x: 180, y: 18) + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 220, height: 36), + styleMask: [.titled, .closable], + backing: .buffered, + defer: false + ) + defer { window.orderOut(nil) } + guard let contentView = window.contentView else { + XCTFail("Expected content view") + return + } + + let container = NSView(frame: contentView.bounds) + container.autoresizingMask = [.width, .height] + contentView.addSubview(container) + + let dragHandle = NSView(frame: container.bounds) + dragHandle.autoresizingMask = [.width, .height] + container.addSubview(dragHandle) + + let foreignWindow = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 220, height: 36), + styleMask: [.titled], + backing: .buffered, + defer: false + ) + defer { foreignWindow.orderOut(nil) } + + XCTAssertFalse( + windowDragHandleShouldCaptureHit( + point, + in: dragHandle, + eventType: .leftMouseDown, + eventWindow: nil + ), + "Launch activation events without a matching window should not trigger drag-handle hierarchy walk" + ) + + XCTAssertFalse( + windowDragHandleShouldCaptureHit( + point, + in: dragHandle, + eventType: .leftMouseDown, + eventWindow: foreignWindow + ), + "Left mouse-down events for a different window should be treated as passive" + ) + + XCTAssertTrue( + windowDragHandleShouldCaptureHit( + point, + in: dragHandle, + eventType: .leftMouseDown, + eventWindow: window + ), + "Left mouse-down events for this window should still capture empty titlebar space" + ) + } + func testPassiveHostingTopHitClassification() { XCTAssertTrue(windowDragHandleShouldTreatTopHitAsPassiveHost(HostContainerView(frame: .zero))) XCTAssertFalse(windowDragHandleShouldTreatTopHitAsPassiveHost(NSButton(frame: .zero))) @@ -6271,7 +6332,7 @@ final class WindowDragHandleHitTests: XCTestCase { nestedContainer.addSubview(nestedDragHandle) XCTAssertFalse( - windowDragHandleShouldCaptureHit(point, in: nestedDragHandle, eventType: .leftMouseDown), + windowDragHandleShouldCaptureHit(point, in: nestedDragHandle, eventType: .leftMouseDown, eventWindow: nestedWindow), "Nested window drag handle should be blocked by top-hit titlebar container" ) @@ -6279,11 +6340,11 @@ final class WindowDragHandleHitTests: XCTestCase { let probe = PassThroughProbeView(frame: outerContainer.bounds) probe.autoresizingMask = [.width, .height] probe.onHitTest = { - nestedCaptureResult = windowDragHandleShouldCaptureHit(point, in: nestedDragHandle, eventType: .leftMouseDown) + nestedCaptureResult = windowDragHandleShouldCaptureHit(point, in: nestedDragHandle, eventType: .leftMouseDown, eventWindow: nestedWindow) } outerContainer.addSubview(probe) - _ = windowDragHandleShouldCaptureHit(point, in: outerDragHandle, eventType: .leftMouseDown) + _ = windowDragHandleShouldCaptureHit(point, in: outerDragHandle, eventType: .leftMouseDown, eventWindow: outerWindow) XCTAssertEqual( nestedCaptureResult, @@ -6330,7 +6391,7 @@ final class WindowDragHandleHitTests: XCTestCase { container.addSubview(dragHandle) XCTAssertTrue( - windowDragHandleShouldCaptureHit(point, in: dragHandle, eventType: .leftMouseDown), + windowDragHandleShouldCaptureHit(point, in: dragHandle, eventType: .leftMouseDown, eventWindow: window), "Reentrant same-window top-hit resolution should not trigger exclusivity crashes" ) } @@ -7775,7 +7836,6 @@ final class GhosttyTerminalViewVisibilityPolicyTests: XCTestCase { func testImmediateStateUpdateAllowedWhenHostNotInWindow() { XCTAssertTrue( GhosttyTerminalView.shouldApplyImmediateHostedStateUpdate( - hostWindowAttached: false, hostedViewHasSuperview: true, isBoundToCurrentHost: false ) @@ -7785,7 +7845,6 @@ final class GhosttyTerminalViewVisibilityPolicyTests: XCTestCase { func testImmediateStateUpdateAllowedWhenBoundToCurrentHost() { XCTAssertTrue( GhosttyTerminalView.shouldApplyImmediateHostedStateUpdate( - hostWindowAttached: true, hostedViewHasSuperview: true, isBoundToCurrentHost: true ) @@ -7795,7 +7854,6 @@ final class GhosttyTerminalViewVisibilityPolicyTests: XCTestCase { func testImmediateStateUpdateSkippedForStaleHostBoundElsewhere() { XCTAssertFalse( GhosttyTerminalView.shouldApplyImmediateHostedStateUpdate( - hostWindowAttached: true, hostedViewHasSuperview: true, isBoundToCurrentHost: false ) @@ -7805,7 +7863,6 @@ final class GhosttyTerminalViewVisibilityPolicyTests: XCTestCase { func testImmediateStateUpdateAllowedWhenUnboundAndNotAttachedAnywhere() { XCTAssertTrue( GhosttyTerminalView.shouldApplyImmediateHostedStateUpdate( - hostWindowAttached: true, hostedViewHasSuperview: false, isBoundToCurrentHost: false )