Isolate XPCHelperClient to the main actor - #1495
Merged
Alexander5015 merged 1 commit intoAug 29, 2026
Merged
Conversation
Dbhardwaj99
marked this pull request as draft
August 28, 2026 05:29
Dbhardwaj99
marked this pull request as ready for review
August 28, 2026 05:41
Member
|
fixed in #1496 |
Alexander5015
force-pushed
the
chore/xpc-client-concurrency
branch
from
August 29, 2026 16:04
8b0833d to
6fb85db
Compare
XPCHelperClient held mutable connection state on a non-isolated class while every method hopped through `await MainActor.run` to reach it. That sent both `self` and the resulting `RemoteXPCService` across an isolation boundary on each call, which accounted for 56 of the strict-concurrency diagnostics: 19 `sending 'self'`, 19 non-Sendable captures of `self` in `@Sendable` closures, 15 `RemoteXPCService` Sendable violations, and the rest. Isolating the class to the main actor removes the boundary rather than annotating each crossing. The hops disappear, so `self` and the service never leave the actor, and the type becomes implicitly Sendable. - `ensureRemoteService()` is now called directly instead of via `MainActor.run` - `startMonitoringAccessibilityAuthorization` uses `Task` rather than `Task.detached`, so it stays on the actor - `shared` and `init` stay `nonisolated`, which is safe now that the type is Sendable, preserving the existing call contract for non-isolated callers - `requestAccessibilityAuthorization()` stays `nonisolated` since it is fire-and-forget and its work hops onto the actor internally - the singleton's `deinit` is removed and `init` made private; a process-lifetime singleton never deinits, and teardown is already explicit in `applicationWillTerminate` Relates to TheBoredTeam#1054.
Alexander5015
force-pushed
the
chore/xpc-client-concurrency
branch
from
August 29, 2026 16:43
6fb85db to
693a8d0
Compare
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second step of the Swift 6 migration tracked in #1054, following #1430.
The problem
XPCHelperClientkept its connection state (remoteService,connection,lunarListener,monitoringTask) as mutable properties on a non-isolated class, while every method reached thatstate through
await MainActor.run { ensureRemoteService() }and then used the returned serviceoutside the actor.
That sent two things across an isolation boundary on every call, which is where 56 of the
project's strict-concurrency diagnostics came from:
sending 'self' risks causing data racescapture of 'self' with non-Sendable type 'XPCHelperClient' in a '@Sendable' closuretype 'RemoteXPCService<any BoringNotchXPCHelperProtocol>' does not conform to 'Sendable'passing closure as a 'sending' parameterstatic property 'shared' is not concurrency-safeThe change
Isolating the class to the main actor removes the boundary instead of annotating each crossing.
The
MainActor.runhops go away, so neitherselfnor the service ever leaves the actor, and thetype becomes implicitly
Sendable.ensureRemoteService()is called directly rather than throughMainActor.runstartMonitoringAccessibilityAuthorizationusesTaskinstead ofTask.detachedso it stayson the actor
sharedandinitremainnonisolated, which is safe now that the type isSendable. Thiskeeps the existing call contract, so non-isolated callers such as
MediaKeyInterceptorcompileunchanged
requestAccessibilityAuthorization()remainsnonisolatedbecause it is fire and forget, andits work hops onto the actor internally
deinitis removed andinitmade private. A main-actor class has a non-isolateddeinitthat cannot touch isolated stored properties, and for a process-lifetime singleton it was
unreachable regardless. Teardown is already explicit in
applicationWillTerminate, which callsstopMonitoringAccessibilityAuthorization()directlyNet effect on the file is 43 insertions and 80 deletions.
Results
Project-wide diagnostics that the compiler marks as
this is an error in the Swift 6 language mode, measured on clean Debug builds before and after:XPCHelperClient.swiftcomponents/OSD/Managers/XPC/BrightnessManager.swiftcomponents/OSD/Managers/LunarManager.swiftBrightnessManagerimproves without being touched, because it calls into this client.LunarManagergains one:sending value of non-Sendable type 'LunarEventListener'. Now thatstartLunarEventStream(listener:)is main-actor isolated, passing a non-Sendable listener into itfrom a detached task is a crossing. That is a pre-existing problem in
LunarManagerwhich thischange surfaces rather than causes, and that file is already on the list for its own pass.
One practical note for planning: per-file counts shift as these land, so the remaining split is
worth re-measuring before each PR rather than working from the original table in #1054.
Testing
Tested on a locally signed build, since the ad-hoc signed helper in a stock Debug build can't be
granted Accessibility. That signing change isn't part of this PR.
Accessibility prompt works, brightness, volume and keyboard backlight all show in the notch and
take effect, toggling Replace System OSD off and on restarts the monitor fine, and quitting from
the menu exits cleanly. Didn't test the Lunar paths since I don't have Lunar installed.
Relates to #1054. Further steps remain.