fix(darwin): never start an AVAssetExportSession that was already cancelled - #191
Merged
Conversation
… rename `ChromaKeyConfig.backgroundImageData: Data?` became `backgroundImage: EncodedImage?` in 2.11.2, when a file-backed image started travelling as a path instead of bytes. The macOS test target kept calling the old label, so it no longer compiled — and since one broken file fails the whole target, *no* Swift test has run on macOS since.
…celled `AVAssetExportSession.export(to:as:)` assigns `outputURL`/`outputFileType` before it starts, and AVFoundation answers that assignment on a session which already left `.unknown` with an Objective-C `NSInternalInconsistencyException`. Swift cannot catch that from an async context, so it takes the host app down — 12 crashes across 2 users in 4 days on iOS/iPadOS 26.x, always within seconds of an editor being opened and dismissed again. The window was wide: a cancel arriving while the export waited for its `ExportGate` slot reached `cancelExport()` on a session that had never run. `.cancelled` counts as "started", so the queued job crashed the moment the gate let it through. The same applies to any cancel landing before `handle.attach(export:)` — a sticky `canceled` flag then poisoned every session the job attached afterwards. Two changes close it, and both are needed: - `RenderJobHandle` no longer force-cancels a session on attach. A start is claimed through `beginExport()`, which refuses it outright once the job is cancelled, and `cancel()` only reaches a session that both claimed its start and has actually left `.unknown` — that status change is exactly the point where AVFoundation's own assignment is through, which closes the remaining sub-microsecond window between the check and the call. A session attached later starts out unclaimed again (split halves, passthrough → full render). - `ExportSessionGuard` re-reads `status` immediately before every start and throws instead: `CancellationError` for a cancelled job (a normal outcome, mapped to `RenderCanceledException` on the Dart side) and an ordinary error for a session that already ran. All five `export(to:as:)` call sites and the legacy `exportAsynchronously` branches go through it, so the render, split, clip-transition and HDR-transcode paths are covered alike. Resolving the destination centrally also removes four `export.outputURL!` force-unwraps. Also: the pre-iOS-18 render branch now reports a cancelled export as a `CancellationError` instead of a generic failure, matching what the split path already did. Tests, both verified non-vacuous by neutering the fix and re-running: - `ExportSessionGuardTests` (iOS + macOS) pin the handle contract — a cancel before the start leaves the session untouched, a cancelled job is refused, a spent session is an error rather than a crash. Without the fix the test process aborts inside `export(to:as:isolation:)` with the reported stack. - Two integration tests sweep a cancel across the native setup phase for render and split. Without the fix the app dies partway through the sweep and takes the remaining tests with it; the crash reports carry the same `objc_exception_throw` stack as production. Verified: full macOS integration suite (22/23 files, the one failure — `error_handling_test` "timestamp beyond duration" — reproduces unchanged on `stable`), 356 Dart unit tests, both Swift test targets, analyze and format.
The start guard only covered `RenderJobHandle.cancel()`. `ExportWatchdog`'s stall and timeout hooks still called `cancelExport()` unconditionally, so the same uncatchable Objective-C exception stayed reachable through a fired bound. And a session the guard skips because it is still `.unknown` was left running on the pre-iOS 18 / pre-macOS 15 `exportAsynchronously` path, where nothing else stops it: it kept encoding past the `ExportGate` slot it no longer held. `ExportSessionGuard.forceCancel` is now the single route to `cancelExport()` and `ExportSessionDriver` the single route into a start. The driver claims the start before it creates the progress observer — a refused start no longer leaves one streaming `states()` for a job that already failed — and force-cancels the session as it unwinds, which is what stops the export the watchdog had to skip. Render, both split halves, the transition pre-render and the HDR pre-transcode each carried their own copy of that dance; the transition copy waited on a continuation that ignored cancellation entirely. Cancellation also stopped being swallowed. `transcodeClipsIfNeeded` throws instead of quietly falling back to the HEVC 10-bit source the pre-transcode exists to avoid, `preRenderTransitions` stops instead of rewriting the clip list into hard cuts, and the render path maps a bare `CancellationError` to CANCELED as the split path already did. The guard cases now live once in example/shared_tests, compiled into both RunnerTests targets, and cover the re-attach invariant (a session attached after a claimed one starts out unclaimed) and `forceCancel` refusing an unstarted session. The split cancel sweep gets its own tighter delays — a passthrough split finishes in milliseconds, so the render sweep's 150-400ms points cancelled nothing — and cleans up through addTearDown, so a failed expectation no longer leaks the halves it wrote.
Plain `xcodebuild` on a fresh checkout, or after `flutter clean`, fails to resolve FlutterGeneratedPluginSwiftPackage instead of running a single test. The package is generated at Flutter's default minimum OS (iOS 13.0 / macOS 10.15) and only `flutter build` raises it to the Xcode project's deployment target, while file_picker needs iOS 14, pro_image_editor macOS 11 and pro_video_editor itself macOS 12. The script puts a `flutter build --config-only` in front, which writes the deployment target without paying for a compile, and documents why the step is not decoration.
…solved Moving the transition pre-render inside the `do` block put a throwing statement ahead of `outputURL = resolveOutputURL(...)`, and `finalize` reads that `URL!` into a `[URL]` literal. A render cancelled during the pre-render stage therefore unwound straight into a force-unwrap of nil — trading the Objective-C exception this branch removes for a Swift one, on the very path the changelog advertises as fixed. `finalize` now reads the path as the optional it is. The sweep that covers this renders two clips joined by a dissolve, which is what puts a job in that stage at all; without the fix it dies partway through, the same way the existing sweeps do without theirs. Also hoists the cancel-then-assert pair the sweeps share into two helpers, and uses them in the older cancellation test that spelled the same idiom out inline.
Two exports leave their output where nothing can reach it afterwards. A transition blend is tracked for cleanup only once it is handed back, so an export that stops mid-write leaves a partial clip in the temporary directory that `finalize` never learns about. This became reachable in this branch: the blend used to wait on a continuation immune to cancellation, so it always ran to completion. A split half writes to a path the caller chose, and a truncated file there is indistinguishable from a finished one for anyone who just checks that it exists. Both now delete their own partial output before rethrowing. The passthrough fallback is unaffected — it re-creates the file it retries.
`ExportSessionGuard` documents `forceCancel` as the one place that calls `cancelExport()`, but `ExtractAudio` still called it directly on a session published to its cancel closure from the main queue while the worker thread had yet to start it — the cancel-before-start shape this branch exists to close. It goes through the guard now, and the start re-checks the flag so a cancelled extraction stops instead of encoding a result the completion handler throws away. `MergeAudio.transcode` still wrapped `exportAsynchronously` in a checked continuation, which is exactly what was deleted from `ClipTransitionRenderer` here for being immune to cancellation: a cancelled merge kept encoding its container to the end. It runs through `ExportSessionDriver` now, which also means it can report a `CancellationError` — so the five error mappings, which had drifted into four different spellings of the same policy, share one helper. Stop-motion gets the same treatment: it throws `CancellationError` from `Task.checkCancellation` and mapped it to RENDER_ERROR.
…leave `exportAsynchronously` publishes its status change on its own queue, so the read immediately after it can still see `.unknown` — which matched neither `.waiting` nor `.exporting` and skipped the poll loop outright. The guard below then reported a healthy export as "failed with status 0", and since that path never cancels, the encoder it abandoned kept running past the gate slot the job no longer held. Waiting for the terminal statuses instead cannot exit early. The progress observer on the modern path is now awaited after being cancelled, not just cancelled: it retains the session, its composition and the custom compositor, and the caller releases its gate slot the moment the driver throws. Also scopes the header's "every export" claim to what is now true.
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
1 participant
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.
Fixes #189.
The crash
AVAssetExportSession.export(to:as:)assignsoutputURL/outputFileTypebefore it starts. On a session that already left.unknown, AVFoundation answers that with an Objective-CNSInternalInconsistencyException— uncatchable from Swift async code, so the host app dies. Reported as 12 crashes across 2 users in 4 days on iOS/iPadOS 26.x, always within seconds of an editor being opened and dismissed.The window was wide. A cancel arriving while the export waited for its
ExportGateslot reachedcancelExport()on a session that had never run;.cancelledcounts as "started", so the queued job crashed the moment the gate let it through. Any cancel landing beforehandle.attach(export:)did the same, because the stickycanceledflag then poisoned every session the job attached afterwards.The fix
RenderJobHandleno longer force-cancels a session on attach. The start is claimed viabeginExport(), which refuses it once the job is cancelled, andcancel()only reaches a session that claimed its start and has left.unknown— that status change is precisely where AVFoundation's own assignment is through, which also closes the sub-microsecond window between check and call. A session attached later starts out unclaimed again (split halves, passthrough → full render).ExportSessionGuardre-readsstatusimmediately before every start and throws instead:CancellationErrorfor a cancelled job (a normal outcome, surfaced asRenderCanceledException), an ordinary error for a session that already ran. All fiveexport(to:as:)call sites and the legacyexportAsynchronouslybranches go through it — render, split, clip transition and HDR transcode alike. Resolving the destination centrally also removes fourexport.outputURL!force-unwraps.CancellationErrorinstead of a generic failure, matching the split path.Tests
Both layers verified non-vacuous by neutering the fix and re-running:
ExportSessionGuardTests(iOS + macOS) pin the handle contract: a cancel before the start leaves the session untouched, a cancelled job is refused, a spent session is an error rather than a crash. Without the fix the test process aborts insideexport(to:as:isolation:)with the stack from the issue.Two integration tests sweep a cancel across the native setup phase, for render and for split. Without the fix the app dies partway through the sweep and takes the remaining tests with it:
The crash reports carry the production stack:
objc_exception_throw→AVAssetExportSession.export(to:as:isolation:)→RenderVideo.monitorExportProgress,Abort trap: 6.Verification
error_handling_test→ "timestamp beyond duration is handled", reproduces identically on unmodifiedstable(agetThumbnailsedge case on theAVAssetImageGeneratorpath, unrelated to this change).flutter test: 356/356.flutter analyzeanddart format: clean.The first commit is a prerequisite, not part of the fix: the macOS
RunnerTeststarget stopped compiling whenChromaKeyConfig.backgroundImageDatabecamebackgroundImagein 2.11.2, so no Swift test had run on macOS since.