fix(android): reply exactly once to preparePlayer (upstream #488) - #1
Open
yagoquesadafloriach wants to merge 11 commits into
Open
fix(android): reply exactly once to preparePlayer (upstream #488)#1yagoquesadafloriach wants to merge 11 commits into
yagoquesadafloriach wants to merge 11 commits into
Conversation
preparePlayer captured the one-shot MethodChannel.Result inside a Player.Listener that stays attached for the player's whole lifetime. The success path was guarded by isPlayerPrepared; the error path was guarded by nothing, so a playback failure after a successful prepare called result.error on a Result that had already been answered. Flutter's DartMessenger enforces one reply per message with an atomic flag and throws IllegalStateException "Reply already submitted" on the second — on the main thread, inside a listener callback, so the process dies. - Add hasReplied, reset at the top of preparePlayer, and guard both result.success(true) and result.error(...) with it. Once isPlayerPrepared is true, onPlayerError no longer touches result at all. - Tear the previous player down at the top of preparePlayer via the new no-arg stop(). The old listener still captures the previous, already-answered Result; leaving it attached let the old player reply to it. This also resets isPlayerPrepared, so re-preparing the same key can reply again instead of leaving the Dart future pending forever (the "dead play button"). - release() now detaches the listener and nulls the player. A listener left bound to a released player can still receive queued events, which is a second route to a late reply. - stopAllPlayers replied once per player via stop(result) and then once more after the loop: a second, independent route to the same fatal on the same channel. It now uses the no-arg stop() and replies exactly once. - Post-prepare failures can no longer be returned through the spent Result, so surface them on a new onPlayerError channel event, exposed in Dart as PlayerController.onPlayerError, and move the player to stopped. Upstream SimformSolutionsPvtLtd#495 fires onDidFinishPlayingAudio with finishType 2 here, which Dart turns into a completion event — a broken file would read as "finished normally". Port of upstream PR SimformSolutionsPvtLtd#495 (Closes SimformSolutionsPvtLtd#488) onto af6bc5a. Refs: SimformSolutionsPvtLtd#495 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WaveformExtractor replied to its Result from four places with no coordination between them: the plugin's onProgress == 1.0F callback, MediaCodec.onError, the input-buffer failure path, and the pre-decode catch. Any two of them firing for one extractWaveformData call throws "Reply already submitted". This is the variant reported in SimformSolutionsPvtLtd#375, which the preparePlayer guard does not cover. - Route every reply through submitWaveformData()/submitError(), which claim isReplySubmitted under a lock so the first reply wins and later ones are no-ops. - Post replies and onCurrentExtractedWaveformData on the main handler. Codec callbacks arrive on a codec-owned thread and Flutter channels are main-thread only, so these were already being invoked off the platform thread. - Add the @volatile released flag and take the lock in the codec callbacks, so a callback that lands during teardown bails out instead of touching a freed codec or extractor. stop() claims the codec under the lock and releases outside it — MediaCodec.stop()/release() drains in-flight callbacks that contend for the same lock — and off the callback thread, wrapping each call so an already-released codec cannot crash teardown. - Reply at EOF as well. Previously a file whose sample count never reached the expected point count left the Dart future pending forever. - The plugin called stop() on the *new* extractor right after startDecode(). That was harmless only because stop() early-returned on a `started` flag that was never set to true, so the codec was never released at all. It now tears down the previous extractor before replacing it, matching upstream main. Port of upstream PR SimformSolutionsPvtLtd#495 onto af6bc5a. Refs: SimformSolutionsPvtLtd#495 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MediaExtractor.setDataSource() and getTrackFormat() block synchronously, and for a large or unreadable file they can stall for seconds. startDecode() runs from the platform method-channel handler, so that stall was on the Android main thread: UI freeze, and an ANR if it ran long enough. Move the setup onto a decode thread and interrupt it from stop(). The reply and teardown paths were already made thread-safe in the previous commit, which is what makes this safe to move off the platform thread. Port of upstream PR SimformSolutionsPvtLtd#495 onto af6bc5a. Refs: SimformSolutionsPvtLtd#495 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tLtd#495 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The extraction callback resolved its target with extractors[playerKey] at the moment it fired. Since decode now runs on its own thread, a late callback from a superseded extractor resolved to whichever extractor currently occupies the key and replied to that one's Result — answering the wrong call and leaving the superseded call pending forever. Close over the extractor instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Upstream already fixed every defect this addressed, so carrying a hand-rolled version on a 1.2.0 base duplicated merged work and added maintenance surface for a code path this app never reaches (`shouldExtractWaveform: false` at both call sites). - SimformSolutionsPvtLtd#375, the extractor's double-reply, was closed by SimformSolutionsPvtLtd#409 in 1.3.0; `main` carries an `isReplySubmitted` guard. - Cancelling a superseded extraction was fixed by SimformSolutionsPvtLtd#414. - Codec-teardown crashes were fixed by SimformSolutionsPvtLtd#431. Reverts dd40b59, a09b3dd and 9bb09db. `WaveformExtractor.kt` is now byte-identical to upstream 1.2.0. The `stopAllPlayers` reply fix from 32617c6 is untouched — that one is a genuine second route to SimformSolutionsPvtLtd#488. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Backport of upstream SimformSolutionsPvtLtd#420, merged after 1.2.0 and so missing from this base. `setFinishMode` never replied on success, so a caller awaiting `controller.setFinishMode(...)` waited forever. The plugin's dispatch for it had the same defect from a different angle: a nested `?.let` fell through silently when the key was null or no player was registered. Same one-shot-Result defect family as SimformSolutionsPvtLtd#488, and live for any caller that awaits it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Records why this fork exists at all: SimformSolutionsPvtLtd#488 is open, SimformSolutionsPvtLtd#495 is open, and upstream main (2.0.2) still carries the bare `result.error(...)` in `onPlayerError`, so upgrading does not resolve the crash. Also documents what was deliberately dropped and why, so nobody reimplements the extractor work that SimformSolutionsPvtLtd#409/SimformSolutionsPvtLtd#414/SimformSolutionsPvtLtd#431 already merged. Bumps the version to 1.2.0-lh1 so the lockfile and build output identify the fork rather than reporting plain 1.2.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…— REVERT BEFORE MERGE The natural trigger for SimformSolutionsPvtLtd#488 (ExoPlayer failing *after* prepare already answered its Result) cannot be driven from the UI, so a clean run proves nothing on its own. This adds a synthetic late-error injection plus a switch that restores the pre-fix reply path, so the crash can be reproduced on demand as a negative control. Drives the two-phase harness in the app repo at scripts/test-reply-already-submitted.sh. Verified on a OnePlus A6003 (arm64-v8a, API 30): the control run reproduces the exact Crashlytics 10820c7c stack (DartMessenger$Reply.reply -> MethodChannel$IncomingMethodCallHandler$1.error -> onPlayerError), and the guarded run survives with zero fatals and delivers a PlayerError. This commit is scaffolding, not product code. Revert it before merging. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ORE MERGE Lets the app-side ExoPlayer-code-to-exception mapping be exercised on a real device instead of only in unit tests. Second half of the scaffold; revert alongside a522a07 before merging. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reverts a522a07 and dacad52. The scaffold existed to reproduce SimformSolutionsPvtLtd#488 on demand and to exercise the app-side error-code mapping on a real device. Both are done, so the injection, the guard bypass and their logging come out. AudioPlayer.kt is restored to exactly the state the v1.2.0-lh1 tag points at, so the app's pin resolves to an identical tree and does not move. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
yagoquesadafloriach
marked this pull request as ready for review
August 12, 2026 12:55
yagoquesadafloriach
requested review from
raulmabe-labhouse and
sergi-labhouse
August 12, 2026 12:55
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.
What
Kills the Android
IllegalStateException: Reply already submittedcrash frompreparePlayer, and adds aPlayerController.onPlayerErrorstream for playback failures that arrive too late to be thrown.Why
nº3 Android crash in Summary AI: 618 fatal events / 282 users over 30 days, present since app 2.5.2 (Crashlytics
10820c7c).preparePlayerhands its one-shotMethodChannel.Resultto an ExoPlayer listener that outlives the call. It repliessuccessonSTATE_READY, then replies again witherroron any lateronPlayerError. Second reply, main thread, fatal.Upstream #488 has been open since April and its fix #495 since June. Upstream
main(2.0.2) still contains the bareresult.error(...), so upgrading doesn't fix this — hence the fork.How
hasRepliedguards both reply sites, and the previous player is torn down before it's replaced so a detached listener can't answer a spent result.onPlayerErrorchannel event and move the player to stopped. Two things fix: 🐛 Prevent crashes and ANR on network loss during playback SimformSolutionsPvtLtd/audio_waveforms#495 does that we deliberately skipped: a network retry loop (we play local files, and a truncated recording matches itsERROR_CODE_IO_UNSPECIFIEDpredicate — 5 × 3s of silence before anything is reported), and reportingfinishType: 2, which Dart turns into a completion event so a broken file reads as "finished normally".setFinishModenever replied on success, soawait controller.setFinishMode(...)hung forever. Same one-shot-result defect family.upstream-1.2.0, which is upstreamaf6bc5a8verbatim — the ref audio_app pins. Not this fork'smain, which sits ~9 months of upstream history behind and would bury the diff.Risks
WaveformExtractor.ktis untouched and byte-identical to upstream 1.2.0. An earlier revision of this branch rewrote it; that was reverted in3604ce5once it turned out upstream had already fixed all of it (#409, #414, #431). Take those by upgrading to 2.0.x, not by reimplementing on an old base.stopAllPlayersorphans its players, andMediaCodec.setCallbackwith a null handler dispatches on the main thread. Both also exist in 2.0.2 and neither is reachable from audio_app, so fixing them would mean diverging for no gain.