Migrate the core Workflow runtime to Swift Concurrency#394
Draft
blakemcanally wants to merge 19 commits into
Draft
Migrate the core Workflow runtime to Swift Concurrency#394blakemcanally wants to merge 19 commits into
blakemcanally wants to merge 19 commits into
Conversation
Replaces the DispatchQueue.workflowExecution convention (dispatchPrecondition checks, dispatch-async deferral) with compiler-enforced @mainactor isolation and main-actor Tasks. Sink stays nonisolated; runtime entry goes through MainActor.assumeIsolated, preserving the crash-on-misuse contract. Co-Authored-By: Claude Fable 5 <[email protected]>
render/makeInitialState/workflowDidChange/apply are now @mainactor requirements, and RenderContext is a @mainactor type. Conforming types inherit isolation via protocol-conformance inference, so existing conformances continue to compile unchanged. Co-Authored-By: Claude Fable 5 <[email protected]>
Co-Authored-By: Claude Fable 5 <[email protected]>
Streams created after finish() are immediately finished and empty, matching the documented contract. Co-Authored-By: Claude Fable 5 <[email protected]>
Natively buffered streams (not Publisher.values, which drops events under backpressure). renderings replays the current value and conflates to the newest; outputs is unbounded and lossless. Co-Authored-By: Claude Fable 5 <[email protected]>
Co-Authored-By: Claude Fable 5 <[email protected]>
Sample conformances with witnesses declared in separate extensions do not infer isolation from the Workflow protocol and need explicit annotations after the runtime's @mainactor migration. Co-Authored-By: Claude Fable 5 <[email protected]>
The Tests/ directories in each partner module were annotated during the @mainactor migration, but the parallel TestingTests/ directories (covering each module's -Testing helper library) were missed, leaving XCTestCase classes that call main actor-isolated renderTester()/ send(action:) APIs from a synchronous nonisolated context. This blocked a full tuist test --path Samples UnitTests run. Co-Authored-By: Claude Fable 5 <[email protected]>
Co-Authored-By: Claude Fable 5 <[email protected]>
Bumps swift-tools-version to 6.0. Other targets remain in v5 mode with targeted strict concurrency. Co-Authored-By: Claude Fable 5 <[email protected]>
Co-Authored-By: Claude Fable 5 <[email protected]>
…tion note) Softens an overstated FIFO-ordering claim in the sink-event deferral comment, and documents why WorkflowCombine's mapOutput spells @sendable explicitly (implicit for @mainactor function types under the core module's Swift 6 mode, explicit under this module's Swift 5 mode — same type either way). Co-Authored-By: Claude Fable 5 <[email protected]>
Swift 6.3.2's performance inliner crashes (infinite recursion in its layout-constraint compatibility check) when optimizing the deallocating deinit of a generic class that declares an isolated deinit. Replace the isolated deinits on WorkflowNode and WorkflowHost with plain deinits that assume main-actor isolation, matching the runtime's guarantee that these references are released on the main actor. Co-Authored-By: Claude Fable 5 <[email protected]>
The isolated-deinit replacement (85bf6fc) ran finalization inline via MainActor.assumeIsolated. That subtly changed runtime behavior: an isolated deinit enqueues its body onto the actor whenever the last release happens outside a main-actor task context (e.g. plain main-thread code), so observer callbacks and subject completions never interleaved with the main-actor operation that released the reference. Running them inline reordered those side effects relative to in-flight UI work. Store the finalization as a main-actor closure created at init (such closures are implicitly Sendable in Swift 6 mode) and have the plain deinit schedule it unconditionally. This is deterministic — unlike isolated deinit, which is inline-or-enqueued depending on the releasing context — avoids the Swift 6.3.2 optimizer crash, and removes two MainActor.assumeIsolated uses. The "Alive" signpost interval is now anchored to a SignpostRef owned by the node, since the node itself is gone by the time the scheduled finalization emits the end of the interval. Co-Authored-By: Claude Fable 5 <[email protected]>
An isolated deinit defers releasing the object's stored properties along with its body, so a workflow tree released mid-render never tore down children, side-effect lifetimes, or event pipes while the render pass was still running. Scheduling only the finalization side effects left the deallocation cascade inline at the release point, where it can still interleave with in-flight main-actor work and perturb layout. Capture the node's subtree manager (and the host's root node) in the scheduled finalization closure so the entire teardown happens in the finalization job, matching isolated-deinit ordering deterministically. Co-Authored-By: Claude Fable 5 <[email protected]>
Deferring the host's whole tree through the finalization task holds the previous tree (including in-flight view state) alive across whatever the releasing code does next, which is its own source of interleaving. The node-level deferral is what protects an in-progress render pass from mid-pass subtree teardown; the host's root-node release can stay at the release point. Co-Authored-By: Claude Fable 5 <[email protected]>
Matches the dispatch semantics of `isolated deinit` (SE-0371) instead of unconditionally enqueueing: a release from a main-actor task context finalizes inline, so callers that drop the last reference can observe teardown side effects (observer callbacks, side-effect terminations, subject completions) synchronously. Releases outside a task context still enqueue onto the main actor so finalization can't interleave with whatever main-actor operation released the reference. The inline path also restores single-job cascade teardown: once an enqueued finalization runs (inside a main-actor task), the child deinits it triggers take the inline path, so an entire subtree tears down within one main-actor job instead of one job per tree level with arbitrary main-queue work interleaved between levels. Co-Authored-By: Claude Fable 5 <[email protected]>
Task-context detection was the wrong discriminator: synchronous code (e.g. sync XCTest methods) that drops the last reference to a workflow host has no current task, yet legitimately expects teardown side effects — side-effect terminations, observer callbacks — to be observable when the drop returns, exactly as a plain inline deinit behaves. The actual hazard is a release performed while the runtime is mid-operation: tearing down inline there interleaves observer callbacks, subject completions, and subtree teardown with the in-flight render pass or action cascade. So mark the runtime's main-actor operations with a re-entrancy counter (WorkflowRuntimeActivity) at every entry point — EventPipe.handle covers all event cascades; the host marks its initial render, update(workflow:), and output handling — and finalize inline only when on the main thread with the runtime quiescent, enqueueing onto the main actor otherwise. An enqueued finalization runs with the runtime quiescent, so the child deinits it triggers finalize inline: an entire subtree still tears down within one main-actor job rather than one job per tree level. Co-Authored-By: Claude Fable 5 <[email protected]>
Deferring a mid-operation finalization to a Task pushed teardown to a later main-queue drain: unrelated main-queue work could interleave with it, and the extra runloop hop shifted the timing that snapshot tests (text-caret blink phase) and leak detectors observe. Queue the deferred finalizations on WorkflowRuntimeActivity instead and run them when the outermost operation exits — teardown still can't interleave with the in-flight render pass or action cascade, but it completes within the same runloop callout, matching pre-deferral timing at runloop granularity. Off-main-thread releases still hop onto the main actor. Co-Authored-By: Claude Fable 5 <[email protected]>
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
Migrates the core
Workflowmodule to Swift Concurrency: the runtime is now@MainActor-isolated, the core module compiles in Swift 6 language mode, and the host gains nativeAsyncStreamobservation and anasyncside-effect API. The change is deliberately additive — Combine publishers, theLifetime-based side-effect API, and partner modules (WorkflowReactiveSwift, WorkflowRxSwift, WorkflowCombine) keep their existing surfaces.This follows up on #392 (core off ReactiveSwift): that PR removed the reactive dependency; this one replaces the remaining pre-concurrency machinery (the
DispatchQueue.workflowExecutionconvention anddispatchPreconditionchecks) with compiler-enforced isolation.What changed
1.
@MainActorruntime isolationWorkflowHost,WorkflowNode,SubtreeManager,RenderContext, and theWorkflow/WorkflowActionprotocol requirements are now@MainActor. Conforming types inherit isolation automatically through the conformance — most workflow code compiles unchanged. Code that touched the runtime from a non-main context, which previously crashed at runtime viadispatchPrecondition, now fails to compile instead.2. Native async observation of a host
WorkflowHostingController.outputsmirrors the host. These are built on a small internal multicaster rather than bridgingrenderingPublisher.values, becausePublisher.valuessilently drops events under backpressure (FB13156448) — a lossy bridge is not an acceptable foundation for output events.3. Structured side effects
The work runs in a node-owned
Taskand is cancelled cooperatively when the side-effect's key disappears — the async analogue of the existingLifetime-based overload, which is unchanged.4. Swift 6 language mode (core only)
The package manifest moves to swift-tools 6.0. Only the core
Workflowtarget compiles in.v6mode; every other target stays in Swift 5 mode withStrictConcurrency=targeted. No@unchecked Sendable,nonisolated(unsafe), or@preconcurrency importanywhere in core.Source compatibility
Intent: everything compiles as before except code that was already violating the main-thread contract at runtime.
Accepted source breaks (compile-time errors replacing runtime
dispatchPreconditioncrashes):Sinkis nowSendableandSink.sendis@MainActor, along with the closures it wraps (AnyWorkflow.mapOutput,AnyWorkflowAction's closure initializers andinit(_ base:),StateMutationSink.send,WorkflowLogging.enabled/config). Passingsink.sendor{ sink.send(...) }as a UI callback compiles exactly as before (Swift 6 function-conversion rules); only callingsendfrom a nonisolated synchronous context breaks — which previously crashed.WorkflowTesting'srenderTester(),render(...), andsend(action:)are@MainActor— consumer test suites will typically need@MainActoron test classes/methods. This repo's own suites (30+ files) show the shape of the change: annotations only, no logic edits.@MainActor(48 sample files in this PR demonstrate the sweep; it's mechanical).Behavioral notes:
WorkflowHost/WorkflowNodeuseisolated deinit: dropping the last reference off the main actor now defers deallocation (and publisher completion / stream finish) to the main actor instead of tearing down runtime state on the releasing thread. This serializes deinit-time subject completion with accessors — previously a latent race.Taskinstead ofDispatchQueue.main.async. Delivery stays on the main actor; relative ordering with other main-queue work is best-effort per Task scheduling semantics rather than strict queue FIFO. All reentrancy suites pass under both runtime configurations.Open questions for reviewers
Sink.sendisolation. The alternative — keepingsendnonisolated with aMainActor.assumeIsolatedcrash contract — is not implementable under v6 without@uncheckedescapes (region isolation rejects capturing the non-sendingvalue into a@MainActorclosure), and would breakSinkcrossing into@Sendableworker Tasks. We believe the isolated design is the only sound one, but it's the most visible API change here.renderingsrequiresRendering: Sendable. Forced byAsyncStream.Continuation.yieldtakingsending Element. Typical non-SendableScreenrenderings can't use the stream and should keep usingrenderingPublisher(main-actor-confined). Is the honest-but-narrower API acceptable, or should we hold it until renderings can beSendable-audited?Runtime.ConfigurationandWorkflowLogging.ConfiggainedSendable, andWorkflowCombine.Publisher.running(in:key:)/mapOutputgained@MainActor— additive isolation attributes required by the v6 core; flagged for completeness.runSideEffectTask inherits render-time priority, whileWorkflowConcurrency'sWorkerWorkflowuses.high; worth harmonizing in a follow-up?Testing
ConcurrencyTests,WorkflowHost_EventEmissionTests,SinkEventHandlersuites) under both runtime configurations — the highest-risk area for the dispatch→Task change.AsyncMulticasterTests(6),WorkflowHostAsyncTests(4),AsyncSideEffectTests(2), plus a hosting-controlleroutputstest (verified red→green on-simulator).WorkflowTesting-Testson the iOS 26.5 simulator) fails identically at the base commit — pre-existing IssueReporting crash, not introduced here.main: zero public symbols removed; all signature changes are isolation/Sendableannotations (enumerated above).Follow-ups (not in this PR)
DispatchQueue.workflowExecutionand theLifetime-based side-effect API once adoption settles.renderingPublisher/outputPublisherbehind WorkflowCombine at the next major.🤖 Generated with Claude Code