diff --git a/WordPress/Classes/Utility/Media/MediaExporter.swift b/WordPress/Classes/Utility/Media/MediaExporter.swift index 305ffa755230..2736051ea2f7 100644 --- a/WordPress/Classes/Utility/Media/MediaExporter.swift +++ b/WordPress/Classes/Utility/Media/MediaExporter.swift @@ -123,13 +123,49 @@ extension MediaExporter { } func export() async throws -> MediaExport { - try await withUnsafeThrowingContinuation { continuation in - export(onCompletion: { continuation.resume(returning: $0) }, - onError: { continuation.resume(throwing: $0) }) + try await withCheckedThrowingContinuation { continuation in + let once = ResumeOnce(continuation) + export(onCompletion: { once.resume(returning: $0) }, + onError: { once.resume(throwing: $0) }) } } } +/// A thread-safe wrapper that guarantees a `CheckedContinuation` is resumed at +/// most once, degrading any subsequent resume to a no-op. +/// +/// The callback-based media export APIs can, on a cancellation race, deliver a +/// second terminal callback (e.g. a stray `.cancelled` after a success). Bridging +/// them to `async` with a raw continuation would make that second resume undefined +/// behavior — a hard crash. Routing every resume through this type keeps the +/// bridge safe regardless of how many times the callback fires. +final class ResumeOnce { + private let lock = NSLock() + private var continuation: CheckedContinuation? + + init(_ continuation: CheckedContinuation) { + self.continuation = continuation + } + + func resume(returning value: T) { + take()?.resume(returning: value) + } + + func resume(throwing error: Error) { + take()?.resume(throwing: error) + } + + /// Atomically hands out the continuation exactly once, niling it so any + /// later call returns `nil`. + private func take() -> CheckedContinuation? { + lock.lock() + defer { lock.unlock() } + let continuation = self.continuation + self.continuation = nil + return continuation + } +} + /// Protocol of general options available for an export, typically corresponding to a user setting. /// protocol MediaExportingOptions { diff --git a/WordPress/Classes/Utility/Media/MediaThumbnailExporter.swift b/WordPress/Classes/Utility/Media/MediaThumbnailExporter.swift index c6157e33fe0c..df59f87b9dde 100644 --- a/WordPress/Classes/Utility/Media/MediaThumbnailExporter.swift +++ b/WordPress/Classes/Utility/Media/MediaThumbnailExporter.swift @@ -267,11 +267,12 @@ extension MediaThumbnailExporter { func exportThumbnail(forFileURL fileURL: URL) async throws -> (ThumbnailIdentifier, MediaExport) { let token = MediaExportCancelationToken() return try await withTaskCancellationHandler { - try await withUnsafeThrowingContinuation { continuation in + try await withCheckedThrowingContinuation { continuation in + let once = ResumeOnce(continuation) token.progress = exportThumbnail(forFile: fileURL, onCompletion: { - continuation.resume(returning: ($0, $1)) + once.resume(returning: ($0, $1)) }, onError: { - continuation.resume(throwing: $0) + once.resume(throwing: $0) }) } } onCancel: { @@ -282,11 +283,12 @@ extension MediaThumbnailExporter { func exportThumbnail(forVideoURL url: URL) async throws -> (ThumbnailIdentifier, MediaExport) { let token = MediaExportCancelationToken() return try await withTaskCancellationHandler { - try await withUnsafeThrowingContinuation { continuation in + try await withCheckedThrowingContinuation { continuation in + let once = ResumeOnce(continuation) token.progress = exportThumbnail(forVideoURL: url, onCompletion: { - continuation.resume(returning: ($0, $1)) + once.resume(returning: ($0, $1)) }, onError: { - continuation.resume(throwing: $0) + once.resume(throwing: $0) }) } } onCancel: { diff --git a/WordPress/Classes/Utility/Media/MediaVideoExporter.swift b/WordPress/Classes/Utility/Media/MediaVideoExporter.swift index d858ed05a160..0b73950c407e 100644 --- a/WordPress/Classes/Utility/Media/MediaVideoExporter.swift +++ b/WordPress/Classes/Utility/Media/MediaVideoExporter.swift @@ -221,10 +221,18 @@ class MediaVideoExporter: MediaExporter { exporter.options = imageOptions } exporter.mediaDirectoryType = self.mediaDirectoryType - let imageProgress = exporter.export( - onCompletion: onCompletion, - onError: onError) - progress.addChild(imageProgress, withPendingUnitCount: MediaExportProgressUnits.halfDone) + // The image export is synchronous and resumes the awaiting task via + // `onCompletion`/`onError`, so finish all `progress` bookkeeping *before* + // those callbacks fire. Mutating the `progress` object graph afterwards + // would race the resumed task, which can cancel `progress` from another + // thread (see the cancellation handler above). + exporter.export(onCompletion: { export in + progress.completedUnitCount = MediaExportProgressUnits.done + onCompletion(export) + }, onError: { error in + progress.completedUnitCount = MediaExportProgressUnits.done + onError(error) + }) }) return progress }