Skip to content

Avoid MainActor executor crashes in tab hit testing #190

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

Open
austinywang wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Avoid MainActor executor crashes in tab hit testing #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions Sources/Bonsplit/Internal/Views/TabBarItemGeometryRegistry.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ final class TabBarItemGeometryRegistry {
object: clipView,
queue: .main
) { [weak self] _ in
MainActor.assumeIsolated {
Task { @MainActor [weak self] in
self?.scrollBoundsDidChange()
}
}
Expand All @@ -120,7 +120,7 @@ final class TabBarItemGeometryRegistry {
object: scrollView,
queue: .main
) { [weak self] _ in
MainActor.assumeIsolated {
Task { @MainActor [weak self] in
self?.userWillScroll()
}
}
Comment on lines 113 to 126

@greptile-apps greptile-apps Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Async delivery changes willStartLiveScroll ordering guarantee

The willStartLiveScrollNotification is documented to fire synchronously before the first scroll delta is applied. Replacing assumeIsolated with Task { @MainActor } means userWillScroll() (which clears expectedProgrammaticOffset and pendingScrollIntent) now runs one run-loop turn after the notification, while boundsDidChangeNotification tasks from that same scroll gesture may already be enqueued. Although Task enqueues are FIFO on the MainActor and the willStartLiveScroll task should arrive first, any notification delivered before that task drains — e.g., a re-entrant layout triggered by AppKit — can create a task that observes stale expectedProgrammaticOffset and issues a spurious programmatic offset correction mid-gesture. The window is tiny in practice, but worth a comment explaining why ordering is still safe here.

Expand Down Expand Up @@ -266,7 +266,7 @@ final class TabBarItemGeometryRegistry {
object: documentView,
queue: .main
) { [weak self] _ in
MainActor.assumeIsolated {
Task { @MainActor [weak self] in
self?.documentGeometryDidChange()
}
}
Expand Down Expand Up @@ -413,7 +413,7 @@ struct TabItemHitRegionView: NSViewRepresentable {
}

final class RegionNSView: NSView, BonsplitTabItemHitRegionProviding {
nonisolated(unsafe) private var hitBounds: NSRect = .zero
private var hitBounds: NSRect = .zero
private var tabId: UUID?
private weak var geometryRegistry: TabBarItemGeometryRegistry?
private var containerFrameObserver: NSObjectProtocol?
Expand Down Expand Up @@ -481,7 +481,7 @@ struct TabItemHitRegionView: NSViewRepresentable {
notifyGeometryDidChange()
}

nonisolated func containsBonsplitTabItemHit(localPoint: NSPoint) -> Bool {
func containsBonsplitTabItemHit(localPoint: NSPoint) -> Bool {
hitBounds
.insetBy(
dx: -BonsplitTabItemHitTesting.horizontalSlop,
Expand Down Expand Up @@ -529,7 +529,7 @@ struct TabItemHitRegionView: NSViewRepresentable {
object: containerView,
queue: .main
) { [weak self] _ in
MainActor.assumeIsolated {
Task { @MainActor [weak self] in
self?.notifyGeometryDidChange()
}
}
Expand Down
18 changes: 9 additions & 9 deletions Sources/Bonsplit/Internal/Views/TabBarView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public enum BonsplitTabBarHitRegionRegistry {
}

public protocol BonsplitTabItemHitRegionProviding: AnyObject {
@MainActor
func containsBonsplitTabItemHit(localPoint: NSPoint) -> Bool
}

Expand Down Expand Up @@ -83,6 +84,7 @@ public enum BonsplitTabItemHitRegionRegistry {
return true
}

@MainActor
public static func containsWindowPoint(_ windowPoint: CGPoint, in window: NSWindow) -> Bool {
for view in snapshot() {
guard view.window === window,
Expand Down Expand Up @@ -2313,15 +2315,13 @@ private struct TabBarDragAndHoverView: NSViewRepresentable {
}
}

nonisolated func containsBonsplitTabItemHit(localPoint: NSPoint) -> Bool {
MainActor.assumeIsolated {
let frames = geometryRegistry?.frames(for: tabIds, in: self).values.map { $0 } ?? []
return BonsplitTabItemHitTesting.containsTabLaneHit(
localPoint: localPoint,
tabFrames: frames,
bounds: bounds
)
}
func containsBonsplitTabItemHit(localPoint: NSPoint) -> Bool {
let frames = geometryRegistry?.frames(for: tabIds, in: self).values.map { $0 } ?? []
return BonsplitTabItemHitTesting.containsTabLaneHit(
localPoint: localPoint,
tabFrames: frames,
bounds: bounds
)
}

override func updateTrackingAreas() {
Expand Down
Loading