Conversation
Previously `next(_:)` was @MainActor-isolated, which trapped at runtime
when called from a Combine sink that resumed on the cooperative thread
pool — typical pattern: a view-model awaits an async use case that
bridges back via `Future { promise in Task { promise(.success(value)) }
}` (e.g. Zesame's CombineWrapper), which does not preserve the caller's
MainActor isolation. The `.sink { navigator.next(.x) }` then ran off-main
and the runtime isolation check (`_swift_task_checkIsolatedSwift`) trapped
with EXC_BREAKPOINT.
Fix: mark `next(_:)` `nonisolated`. When called on the main thread it
fast-paths through `MainActor.assumeIsolated` (no Task hop). When called
off-main it dispatches the subject send to `Task { @mainactor in … }`, so
coordinator subscribers always receive on the main actor regardless of
which scheduler the upstream pipeline used.
The class becomes `@unchecked Sendable` so the off-main hop can capture
self across the actor boundary. Safe because `navigationSubject` is the
only mutable state and every write goes through `next(_:)` which now
guarantees the main-actor hop. Remove `@unchecked` once Combine's
`PassthroughSubject` gains native `Sendable`.
Adds a `where NavigationStep: Sendable` constraint on the `next(_:)`
extension because the step value is captured across the actor hop. All
nav-step types in current consumers (Zhip, the SignUpDemo example) are
already Sendable, so this is a non-breaking addition for them.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a Swift Concurrency runtime isolation trap when Navigator.next(_:) is invoked from Combine pipelines that may resume off-main (e.g., after bridging async work back into Future), by making next(_:) callable from any context and hopping to the main actor internally.
Changes:
- Mark
Navigatoras@unchecked Sendableand document the intended safety invariants. - Change
next(_:)tononisolatedand ensure the underlyingPassthroughSubject.sendexecutes on the main actor (fast-path on main, otherwise hop viaTask { @MainActor in … }). - Constrain the
next(_:)extension toNavigationStep: Sendableso the step can be captured across the actor hop.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+61
to
+66
| /// `@unchecked Sendable` because `PassthroughSubject` itself does not yet | ||
| /// conform to `Sendable` (Apple's Combine has not been audited for Swift 6 | ||
| /// strict concurrency). The unchecked claim is safe here because every | ||
| /// mutation of `navigationSubject` is funnelled through ``next(_:)``, which | ||
| /// hops to the main actor before sending; the lazy `navigation` accessor is | ||
| /// `@MainActor`-isolated. Remove `@unchecked` when Combine's |
| @@ -73,13 +83,22 @@ public final class Navigator<NavigationStep> { | |||
| public init() {} | |||
| } | |||
|
|
|||
Copilot review flagged the previous wording "every mutation of navigationSubject is funnelled through next(_:)" as an overclaim: PassthroughSubject also mutates internal state during subscription, cancellation, and demand changes, not just `send`. Reworded to be precise — `next(_:)` covers every send, the @mainactor `navigation` accessor covers every read, and Combine's own subscription bookkeeping is documented thread-safe (so a sink torn down off-main via AnyCancellable.deinit doesn't race our sends). The other review comment (next(_:) Sendable constraint is source- breaking) doesn't apply: the package is at 0.1.x where source breaks are intentional, and every current consumer (Zhip, SignUpDemo) already declares Sendable on their step enums. Already covered in the PR body. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sajjon
added a commit
to Sajjon/Zhip
that referenced
this pull request
May 3, 2026
Picks up the Navigator.next isolation fix (Sajjon/NanoViewController#3): `next(_:)` is now nonisolated and hops to the main actor internally, so view-models can safely emit navigation steps from Combine sinks that resume on the cooperative pool — unblocking the Zesame-CombineWrapper crash documented in commit f8ff88e. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.
Summary
Fix a runtime trap in
Navigator.next(_:)when called from a Combine sink that resumed on the cooperative thread pool.Repro
A view-model awaits an async use case that bridges back to Combine via
Future { promise in Task { promise(.success(value)) } }(e.g. Zesame'sCombineWrapper). TheFuturedoesn't preserve the caller's@MainActorisolation, so the downstream.sink { navigator.next(.x) }runs on the cooperative pool. Withnext(_:)annotated@MainActor, the runtime isolation check (_swift_task_checkIsolatedSwift) trapped withEXC_BREAKPOINT.Fix
Mark
next(_:)nonisolatedand bridge to main inside:MainActor.assumeIsolated— no Task hop, no allocation.sendtoTask { @MainActor in … }, so coordinator subscribers always receive on the main actor regardless of which scheduler the upstream pipeline used.Type changes
Navigator<NavigationStep>becomes@unchecked Sendable. Safe in this codebase:navigationSubjectis the only mutable state and every write goes throughnext(_:)which guarantees the main-actor hop. Documented at the type declaration with a follow-up note: drop the@uncheckedonce Combine'sPassthroughSubjectgains nativeSendableconformance.next(_:)extension picks up awhere NavigationStep: Sendableconstraint because the step value is captured across the actor hop. All current consumers (Zhip,SignUpDemo) already declareSendableon their step enums, so this is non-breaking.Test plan
xcodebuild build -scheme NanoViewController-Package -destination 'iPhone 17, iOS 26.1'—BUILD SUCCEEDEDjust test— all 75 tests pass under Swift 6 language modeswiftformat --lint+swiftlint --strict— clean (pre-commit hooks ran)Future { Task { promise(.success) } }→.sink { navigator.next(.x) }) no longer crashes🤖 Generated with Claude Code