From b4d61a7dc84abc18eb295e6bb38ad7f6a59d025d Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Tue, 1 Sep 2026 22:45:14 -0600 Subject: [PATCH] Fix a crash generating thumbnails for self-hosted videos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The video-thumbnail async bridge used withUnsafeThrowingContinuation, so a double-resume (AVFoundation firing its image-generation completion handler a second time on teardown) was undefined behavior — an intermittent SIGSEGV — instead of a deterministic trap. It also mutated the Progress object graph after the awaiting task had already resumed and could be racing forward on another thread. Switch the three async bridges (MediaExporter.export(), MediaThumbnailExporter.exportThumbnail(forFileURL:/forVideoURL:)) to withCheckedThrowingContinuation routed through a resume-once guard so a stray second callback degrades to a no-op, and finalize the Progress before the resuming callback fires in MediaVideoExporter.exportPreviewImageForVideo. --- .../Classes/Utility/Media/MediaExporter.swift | 42 +++++++++++++++++-- .../Media/MediaThumbnailExporter.swift | 14 ++++--- .../Utility/Media/MediaVideoExporter.swift | 16 +++++-- 3 files changed, 59 insertions(+), 13 deletions(-) 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 }