diff --git a/shared/apple/AtmosProbe/AtmosProbePlugin.swift b/shared/apple/AtmosProbe/AtmosProbePlugin.swift index 575bf503..967904c2 100644 --- a/shared/apple/AtmosProbe/AtmosProbePlugin.swift +++ b/shared/apple/AtmosProbe/AtmosProbePlugin.swift @@ -642,6 +642,9 @@ final class AsbarProbe: NSObject { private var download: URLSessionDownloadTask? private var statusObserved = false private var cancelled = false + // Guarded by `lock`. Consumed exactly once — by the start path that reaches a + // verdict, by `cancel()`, or by `deinit`, whichever happens first. + private var startCompletion: ((String?) -> Void)? // Guarded by `lock`; read from the method-channel thread. private var enqueuedSamples = 0 @@ -663,11 +666,19 @@ final class AsbarProbe: NSObject { /// `completion` reports only whether the run could be started. func start(completion: @escaping (String?) -> Void) { + lock.lock() + startCompletion = completion + let aborted = cancelled + lock.unlock() + if aborted { + finishStart("cancelled") + return + } + if source.isFileURL { queue.async { [weak self] in guard let self else { return } - let error = self.beginReading(from: self.source) - DispatchQueue.main.async { completion(error) } + self.finishStart(self.beginReading(from: self.source)) } return } @@ -675,11 +686,11 @@ final class AsbarProbe: NSObject { // AVAssetReader needs a seekable local asset; stage the source first. setPhase("downloading") let task = URLSession.shared.downloadTask(with: source) { [weak self] url, _, error in - guard let self else { return } + guard let self else { return } // covered by `deinit` guard let url else { let message = error?.localizedDescription ?? "download failed" self.fail(message) - DispatchQueue.main.async { completion(message) } + self.finishStart(message) return } // The temporary file is removed as soon as this handler returns. @@ -691,7 +702,7 @@ final class AsbarProbe: NSObject { } catch { let message = "failed to stage asset: \(error.localizedDescription)" self.fail(message) - DispatchQueue.main.async { completion(message) } + self.finishStart(message) return } self.lock.lock() @@ -700,11 +711,11 @@ final class AsbarProbe: NSObject { self.lock.unlock() if aborted { try? FileManager.default.removeItem(at: staged) + self.finishStart("cancelled") // no-op if `cancel()` already fired return } self.queue.async { - let failureMessage = self.beginReading(from: staged) - DispatchQueue.main.async { completion(failureMessage) } + self.finishStart(self.beginReading(from: staged)) } } lock.lock() @@ -722,6 +733,7 @@ final class AsbarProbe: NSObject { temporaryFile = nil lock.unlock() task?.cancel() + finishStart("cancelled") queue.sync { if statusObserved, let renderer { @@ -745,6 +757,13 @@ final class AsbarProbe: NSObject { setPhase("cancelled") } + deinit { + // `stopPlayback()` drops the only strong reference (`asbar = nil`), so the + // probe can die with a download still in flight; the handler's + // `guard let self` would then swallow the verdict for good. + finishStart("cancelled") + } + /// Runs on `queue`. Returns a message on failure. private func beginReading(from url: URL) -> String? { lock.lock() @@ -1050,6 +1069,19 @@ final class AsbarProbe: NSObject { // MARK: - State + /// Delivers the start verdict at most once, always on the main queue. + /// Never call while holding `lock` (NSLock is not recursive). + private func finishStart(_ message: String?) { + lock.lock() + let completion = startCompletion + startCompletion = nil + lock.unlock() + guard let completion else { return } + // Captures only `completion` and `message` — never `self`, so this is safe + // to call from `deinit`. + DispatchQueue.main.async { completion(message) } + } + private func setPhase(_ value: String) { lock.lock() phase = value diff --git a/tvos/Runner.xcodeproj/project.pbxproj b/tvos/Runner.xcodeproj/project.pbxproj index 3ed55c85..721b04ef 100644 --- a/tvos/Runner.xcodeproj/project.pbxproj +++ b/tvos/Runner.xcodeproj/project.pbxproj @@ -11,6 +11,7 @@ 1C6B234E223CDFFE5BF3FC0B /* TVServices.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 25B7925EFD8A2C1C7EB667D5 /* TVServices.framework */; }; 28DB4404B17342F46BC2B0A1 /* TopShelfProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2F829B3F190657106F66379 /* TopShelfProvider.swift */; }; 2A7C0B1D9E5F4A6381027C12 /* MpvPlayerContractTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2A7C0B1D9E5F4A6381027C11 /* MpvPlayerContractTests.swift */; }; + A7A0C001A7A0C001A7A0C002 /* AtmosProbeContractTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A7A0C001A7A0C001A7A0C001 /* AtmosProbeContractTests.swift */; }; 35DB0C8FEF635A3BCA0B722A /* PackageInfoPlusPlugin.swift in Sources */ = {isa = PBXBuildFile; fileRef = F9426EFA282CDA8E0E98EEE9 /* PackageInfoPlusPlugin.swift */; }; 3B3967161E833CAA004F5970 /* AppFrameworkInfo.plist in Resources */ = {isa = PBXBuildFile; fileRef = 3B3967151E833CAA004F5970 /* AppFrameworkInfo.plist */; }; 5C71F5F7B33075F2B007825B /* MpvPlayerPluginShared.swift in Sources */ = {isa = PBXBuildFile; fileRef = 73645904F226A24585A092CE /* MpvPlayerPluginShared.swift */; }; @@ -121,6 +122,7 @@ 9D7A998830EDC8F77BF521D1 /* MpvPlayerPlugin.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = MpvPlayerPlugin.swift; path = ../ios/Runner/MpvPlayer/MpvPlayerPlugin.swift; sourceTree = ""; }; A12B8610AE5D580077264851 /* MpvPlayerCore.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = MpvPlayerCore.swift; path = ../ios/Runner/MpvPlayer/MpvPlayerCore.swift; sourceTree = ""; }; 2A7C0B1D9E5F4A6381027C11 /* MpvPlayerContractTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = MpvPlayerContractTests.swift; sourceTree = ""; }; + A7A0C001A7A0C001A7A0C001 /* AtmosProbeContractTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = AtmosProbeContractTests.swift; sourceTree = ""; }; A2484B9C94406BF0A99EB64A /* Pods-RunnerTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-RunnerTests.debug.xcconfig"; path = "Target Support Files/Pods-RunnerTests/Pods-RunnerTests.debug.xcconfig"; sourceTree = ""; }; A2635E12EB9322B151EE5127 /* MpvPipController.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = MpvPipController.swift; path = ../ios/Runner/MpvPlayer/MpvPipController.swift; sourceTree = ""; }; A5E1F001234567890ABCDE01 /* SystemShelfPluginTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = SystemShelfPluginTests.swift; sourceTree = ""; }; @@ -341,6 +343,7 @@ children = ( 4F1000014F1000014F100001 /* FlutterNativeTextInputTests.mm */, 2A7C0B1D9E5F4A6381027C11 /* MpvPlayerContractTests.swift */, + A7A0C001A7A0C001A7A0C001 /* AtmosProbeContractTests.swift */, FE6C8125B201A8BC3261AE2B /* TvosEventDeliveryCoordinatorTests.swift */, 934BC4E316D2AC788C954766 /* ConnectivityPlusPluginTests.swift */, A5E1F001234567890ABCDE01 /* SystemShelfPluginTests.swift */, @@ -595,6 +598,7 @@ files = ( 4F1000024F1000024F100002 /* FlutterNativeTextInputTests.mm in Sources */, 2A7C0B1D9E5F4A6381027C12 /* MpvPlayerContractTests.swift in Sources */, + A7A0C001A7A0C001A7A0C002 /* AtmosProbeContractTests.swift in Sources */, D2004D7BB4A40340AB7A01E0 /* TvosEventDeliveryCoordinatorTests.swift in Sources */, 65AC2C222043B3E6723E2076 /* ConnectivityPlusPluginTests.swift in Sources */, A5E1F001234567890ABCDE02 /* SystemShelfPluginTests.swift in Sources */, diff --git a/tvos/RunnerTests/AtmosProbeContractTests.swift b/tvos/RunnerTests/AtmosProbeContractTests.swift new file mode 100644 index 00000000..a455723b --- /dev/null +++ b/tvos/RunnerTests/AtmosProbeContractTests.swift @@ -0,0 +1,61 @@ +import Foundation +import XCTest + +@testable import Runner + +final class AtmosProbeContractTests: XCTestCase { + func testCancelCompletesStartExactlyOnce() { + let completionDelivered = expectation(description: "start completion delivered") + let duplicateCompletion = expectation(description: "start completion delivered twice") + duplicateCompletion.isInverted = true + var invocationCount = 0 + var probe: AsbarProbe? = makeProbe() + + probe?.start { message in + invocationCount += 1 + if invocationCount == 1 { + XCTAssertEqual(message, "cancelled") + completionDelivered.fulfill() + } else { + duplicateCompletion.fulfill() + } + } + probe?.cancel() + probe = nil + + wait(for: [completionDelivered], timeout: 1) + wait(for: [duplicateCompletion], timeout: 1) + XCTAssertEqual(invocationCount, 1) + } + + func testDeinitCompletesStartExactlyOnce() { + let completionDelivered = expectation(description: "start completion delivered") + let duplicateCompletion = expectation(description: "start completion delivered twice") + duplicateCompletion.isInverted = true + var invocationCount = 0 + var probe: AsbarProbe? = makeProbe() + + probe?.start { message in + invocationCount += 1 + if invocationCount == 1 { + XCTAssertEqual(message, "cancelled") + completionDelivered.fulfill() + } else { + duplicateCompletion.fulfill() + } + } + probe = nil + + wait(for: [completionDelivered], timeout: 1) + wait(for: [duplicateCompletion], timeout: 1) + XCTAssertEqual(invocationCount, 1) + } + + private func makeProbe() -> AsbarProbe { + AsbarProbe( + source: URL(string: "http://127.0.0.1:1/probe.eac3")!, + regenerateFormatDescription: false, + sessionMode: .moviePlayback + ) + } +}