[video_player] Fix crash regression with async calls on iOS/macOS (#13383) Because Pigeon closures have references to the bridge object that serves as the API handler, it's not safe for it to have an `unowned` reference to the player instance that it delegates to. This changes it to a weak reference, and uses dummy return values as necessary for cases where the player has already been destroyed by the time the async call reaches the bridge. Also includes a unit test simulating that situation, which crashes without the fix. Fixes https://github.com/flutter/flutter/issues/195056 ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
diff --git a/packages/video_player/video_player_avfoundation/CHANGELOG.md b/packages/video_player/video_player_avfoundation/CHANGELOG.md index 14f566c..39d5de0 100644 --- a/packages/video_player/video_player_avfoundation/CHANGELOG.md +++ b/packages/video_player/video_player_avfoundation/CHANGELOG.md
@@ -1,3 +1,7 @@ +## 2.12.3 + +* Fixes a potential crash when a player is disposed while an async method is in progress. + ## 2.12.2 * Converts native player implementation to Swift.
diff --git a/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.swift b/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.swift index 1e2d543..7a33edf 100644 --- a/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.swift +++ b/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.swift
@@ -935,6 +935,41 @@ #expect(!frameDurationIsValid(CMTime.indefinite)) } + @Test func apiBridgeHandlesPlayerDestruction() async throws { + weak var weakPlayer: VideoPlayer? + var apiBridge: VideoPlayerInstanceBridge? + + let videoPlayerPlugin = try createInitializedPlugin(avFactory: StubAVFactory()) + + let identifier = try await videoPlayerPlugin.createPlatformViewPlayer( + options: CreationOptions(uri: mp4TestURI, httpHeaders: [:])) + + // Simulation a temporary lifetime extension of the bridge, which can happen if an + // async Pigeon closure is executing when `dispose()` is called. + autoreleasepool { + let player = videoPlayerPlugin.playersByIdentifier[identifier] + weakPlayer = player + apiBridge = player?.hostApiBridge + + player?.dispose() + } + + // Wait for the weak pointer to be invalidated, indicating that the player has been deallocated. + let checkInterval = 0.1 + let maxTries = Int64(30 / checkInterval) + for _ in 1...maxTries { + if weakPlayer == nil { + break + } + try await Task.sleep(nanoseconds: UInt64(checkInterval * 1_000_000_000)) + } + + #expect(weakPlayer == nil) + #expect(apiBridge != nil) + // Calling the bridge should return a placeholder value rather than crash. + #expect(try apiBridge?.getPosition() == 0) + } + // MARK: - Helper Methods /// Creates a plugin with the given dependencies, and default stubs for any that aren't provided,
diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/VideoPlayer.swift b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/VideoPlayer.swift index cd2a963..b4dd396 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/VideoPlayer.swift +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/VideoPlayer.swift
@@ -25,7 +25,7 @@ // is fixed, by making VideoPlayerInstanceApi a @MainActor protocol and returning to // having VideoPlayer conform to it. class VideoPlayerInstanceBridge: VideoPlayerInstanceApi { - private unowned let player: VideoPlayer + private weak var player: VideoPlayer? init(player: VideoPlayer) { self.player = player @@ -33,65 +33,65 @@ func setLooping(_ looping: Bool) throws { MainActor.assumeIsolated { - player.setLooping(looping) + player?.setLooping(looping) } } func setVolume(_ volume: Double) throws { MainActor.assumeIsolated { - player.setVolume(volume) + player?.setVolume(volume) } } func setPlaybackSpeed(_ speed: Double) throws { MainActor.assumeIsolated { - player.setPlaybackSpeed(speed) + player?.setPlaybackSpeed(speed) } } func play() throws { MainActor.assumeIsolated { - player.play() + player?.play() } } func getPosition() throws -> Int64 { MainActor.assumeIsolated { - return player.getPosition() + return player?.getPosition() ?? 0 } } func seek(to position: Int64) async throws { - try await player.seek(to: position) + try await player?.seek(to: position) } func pause() throws { MainActor.assumeIsolated { - player.pause() + player?.pause() } } func dispose() throws { MainActor.assumeIsolated { - player.dispose() + player?.dispose() } } func getAudioTracks() async throws -> [MediaSelectionAudioTrackData] { - return try await player.getAudioTracks() + return try await player?.getAudioTracks() ?? [] } func selectAudioTrack(atIndex trackIndex: Int64) async throws { - return try await player.selectAudioTrack(atIndex: trackIndex) + try await player?.selectAudioTrack(atIndex: trackIndex) } func getVideoTracks() async throws -> NativeVideoTrackData { - return try await player.getVideoTracks() + return try await player?.getVideoTracks() ?? NativeVideoTrackData() } func selectVideoTrack(withBitrate bitrate: Int64) throws { MainActor.assumeIsolated { - player.selectVideoTrack(withBitrate: bitrate) + player?.selectVideoTrack(withBitrate: bitrate) } } @@ -99,7 +99,7 @@ throws { MainActor.assumeIsolated { - player.setPreventsDisplaySleepDuringVideoPlayback(preventsDisplaySleepDuringVideoPlayback) + player?.setPreventsDisplaySleepDuringVideoPlayback(preventsDisplaySleepDuringVideoPlayback) } }
diff --git a/packages/video_player/video_player_avfoundation/pubspec.yaml b/packages/video_player/video_player_avfoundation/pubspec.yaml index d49c799..593326f 100644 --- a/packages/video_player/video_player_avfoundation/pubspec.yaml +++ b/packages/video_player/video_player_avfoundation/pubspec.yaml
@@ -2,7 +2,7 @@ description: iOS and macOS implementation of the video_player plugin. repository: https://github.com/flutter/packages/tree/main/packages/video_player/video_player_avfoundation issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22 -version: 2.12.2 +version: 2.12.3 environment: sdk: ^3.13.0