From 27e9f2b631fc27cd571faad4f2691fd5b455abbb Mon Sep 17 00:00:00 2001 From: abretonc7s Date: Fri, 21 Aug 2026 16:43:37 +0800 Subject: [PATCH 1/6] fix(sherpa-onnx.rn/android): let stopTts interrupt an in-flight generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit init, generate, stop and release share one single-threaded executor, so stop() queued behind the generation it was meant to interrupt. The isGenerating checks inside the callback loop could never fire during that generation, making stopTts() a no-op until generation had already finished. isGenerating is cleared on the calling thread now, before queueing. It is @Volatile, so the loop sees the write immediately. The AudioTrack teardown stays on the executor, because the generation loop writes to it from there and pausing it across threads is the hazard the serialization was protecting against. release() had the same defect and gets the same treatment. A/B on Pixel 6a via testTTSStopDuringPrefill(120), stop requested ~120ms in: main: generation 4957ms, 112610 samples in 1 callback -> stop ignored fixed: generation 826ms, 0 samples in 0 callbacks -> stop interrupted On main the "Stopping TTS generation" line lands 1.4s AFTER generation completes; with the fix generation aborts having emitted nothing. Worth recording: the log line alone appears in both, so presence proves nothing — the duration and sample count are what distinguish them. My first attempt at this A/B checked only for the line and would have passed either way. The app still SIGSEGVs afterwards delivering results to JS, 3.2s after the stop fires and entirely in Hermes frames. That is #436, unrelated to this change and present on main too. --- .../siteed/sherpaonnx/handlers/TtsHandler.kt | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt b/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt index 8e708d2f6..eee03d6c8 100644 --- a/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt +++ b/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt @@ -821,13 +821,21 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { * Stop TTS generation */ fun stop(promise: Promise) { + // Clear the flag on the CALLING thread, not the executor. init/generate/stop/release + // share one single-threaded executor, so queueing this behind an in-flight generate() + // meant it could not run until that generation had already finished — the + // isGenerating checks inside the callback loop could never fire during the generation + // they exist to interrupt, making stopTts() a no-op (#440). isGenerating is @Volatile, + // so the loop observes this write immediately. + isGenerating = false + executor.execute { try { Log.i(TAG, "Stopping TTS generation") - - // Set flag to stop callback-based generation - isGenerating = false - + + // Already cleared above; the executor body handles the AudioTrack, which must + // stay on this thread because the generation loop writes to it from here. + // Stop audio playback if (audioTrack?.playState == AudioTrack.PLAYSTATE_PLAYING) { audioTrack?.pause() @@ -856,13 +864,14 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { * Release TTS resources */ fun release(promise: Promise) { + // Same reason as stop(): clear the flag before queueing, so an in-flight generation + // stops feeding the executor rather than blocking this call behind itself (#440). + isGenerating = false + executor.execute { try { Log.i(TAG, "Releasing TTS resources") - // Set flag to stop any ongoing generation - isGenerating = false - // Release TTS resources releaseTtsResources() From acd0d4bb064529d528f962d157dd9d66feb608e7 Mon Sep 17 00:00:00 2001 From: abretonc7s Date: Fri, 21 Aug 2026 17:10:03 +0800 Subject: [PATCH 2/6] fix(sherpa-onnx.rn/android): make TTS cancellation survive queuing and the fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the boolean flag fix was too shallow. Two ways it still lost a cancellation, both confirmed in source. A queued generation swallowed the stop. generate() set isGenerating=true only inside its executor task, so a stop arriving after generate() was queued but before it ran cleared an already-false flag. The queued task then set it true and ran to completion ahead of the stop, which resolved as success having interrupted nothing. A boolean cannot express this: "already false" and "nothing to cancel" are the same value. Replaced with a monotonic request id. generate() claims an id on the calling thread before queueing; stop()/release() raise cancelledThrough to the newest claimed id, cancelling everything outstanding whether started or not. A generation started after the cancel keeps a higher id and is unaffected. The fallback undid the cancellation. generateWithCallback discards its return, so after an interrupt `audio` is null and control fell into the plain tts.generate() fallback, which ignores cancellation entirely. That restarted synthesis and resolved it as success — reversing the fix outright. It is also the path playAudio=false takes, which is the service default, so the common case was the broken one. Now checks the token before falling back. Also updated the prefill guard, whose comment called the check defensive on the grounds that stop() could not run mid-generation. That is no longer true — the fix is precisely that it can — so the check is load-bearing and now consults the token. And the testTTSStopDuringPrefill comment in agentic-bridge.ts still said Android could not interrupt at all; it now says what to look for, including that the "Stopping TTS generation" log line appears in both fixed and unfixed builds and only duration and sample count distinguish them. Kotlin compiles (:siteed_sherpa-onnx.rn:compileDebugKotlin BUILD SUCCESSFUL). Device validation pending. --- apps/sherpa-voice/src/agentic-bridge.ts | 17 ++-- .../siteed/sherpaonnx/handlers/TtsHandler.kt | 87 +++++++++++++++---- 2 files changed, 78 insertions(+), 26 deletions(-) diff --git a/apps/sherpa-voice/src/agentic-bridge.ts b/apps/sherpa-voice/src/agentic-bridge.ts index 0a81696c7..4f01360ef 100644 --- a/apps/sherpa-voice/src/agentic-bridge.ts +++ b/apps/sherpa-voice/src/agentic-bridge.ts @@ -2298,13 +2298,16 @@ if (__DEV__) { // Start playback then call stopTts() inside the prefill window. // - // NOTE: on Android this currently cannot interrupt generation — TtsHandler - // runs init/generate/stop/release on one single-thread executor, so stopTts() - // queues behind the in-flight generateTts() and only runs after it finishes. - // Logcat shows no "Stopping TTS generation" line until generation completes. - // Kept as a regression probe: if stop ever moves off that executor, this is - // the scenario to re-run. The bug would be a "playback started" line - // appearing AFTER "Stopping TTS generation". + // Android used to be unable to interrupt generation at all: TtsHandler runs + // init/generate/stop/release on one single-thread executor, so stopTts() queued + // behind the in-flight generateTts() and ran only after it finished. #440 moved + // cancellation onto the caller's thread, so this now does interrupt. + // + // What to look for: generation should end early and report far fewer samples than + // an uninterrupted run. A "playback started" line appearing AFTER "Stopping TTS + // generation" is the regression. The "Stopping TTS generation" log line alone + // proves nothing — it appears in both the fixed and unfixed builds; only the + // duration and sample count distinguish them. testTTSStopDuringPrefill: (stopAfterMs = 120, modelDir?: string) => { const op = 'ttsStopDuringPrefill' const BASE = MODELS_BASE diff --git a/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt b/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt index eee03d6c8..921bc2253 100644 --- a/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt +++ b/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt @@ -22,6 +22,38 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { // Read on the generation thread, written by stop()/release(). Volatile so the // write is visible if either ever runs off the shared single-thread executor. @Volatile private var isGenerating = false + + /** + * Monotonic id of the newest generation request (#440). + * + * A boolean alone loses a cancellation that arrives while generation is queued but not + * yet running: stop() clears an already-false flag, then the queued task sets it true + * and runs to completion, and stop() reports success having interrupted nothing. + * + * generate() claims an id before queueing. stop()/release() raise [cancelledThrough] to + * the newest claimed id, so every request outstanding at that moment is cancelled + * whether it had started or not. A request runs only while its own id is above the bar, + * so a generation started after the cancel is unaffected. + */ + private val requestCounter = java.util.concurrent.atomic.AtomicLong(0) + + /** Highest request id that has been cancelled. See [requestCounter]. */ + private val cancelledThrough = java.util.concurrent.atomic.AtomicLong(0) + + /** Whether [requestId] may keep generating, or has been cancelled out from under it. */ + private fun isActive(requestId: Long): Boolean = + requestId > cancelledThrough.get() + + /** Cancel every request claimed so far. Safe to call from any thread. */ + private fun cancelOutstandingRequests() { + val newest = requestCounter.get() + // Raise the bar rather than assigning: a concurrent cancel must not lower it. + while (true) { + val current = cancelledThrough.get() + if (current >= newest || cancelledThrough.compareAndSet(current, newest)) break + } + isGenerating = false + } private var audioTrack: AudioTrack? = null private var ttsModelConfig: OfflineTtsModelConfig? = null private var currentSampleRate: Int = 22050 // Default to 22050 Hz (common for speech) @@ -398,6 +430,10 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { Log.d(TAG, "Style parameters: lengthScale=$lengthScale, noiseScale=$noiseScale, noiseScaleW=$noiseScaleW") Log.d(TAG, "Using sample rate: $currentSampleRate Hz") + // Claimed here, not inside the executor: a cancel that arrives while this request is + // still queued must be able to see it (#440). + val requestId = requestCounter.incrementAndGet() + executor.execute { try { if (tts == null) { @@ -408,6 +444,12 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { throw Exception("TTS is already generating speech") } + if (!isActive(requestId)) { + // Cancelled while queued. Reject rather than resolve: nothing was + // produced, and reporting success here is the bug this guards. + throw Exception("TTS generation was cancelled before it started") + } + isGenerating = true // Apply parameters to the model config based on model type @@ -473,7 +515,7 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { // of invoke([F)Ljava/lang/Integer; in generateWithCallbackImpl. tts?.generateWithCallback(text, speakerId, speakingRate, object : Function1 { override fun invoke(samples: FloatArray): Int { - if (!isGenerating) { + if (!isActive(requestId) || !isGenerating) { Log.i(TAG, "TTS generation interrupted by stop request") return 0 // Stop generating } @@ -581,14 +623,13 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { // Short utterances can finish before reaching the prefill threshold. // - // The isGenerating check is defensive. stop() and release() clear the - // flag, but both run on the same single-thread executor as this - // generation, so today they queue behind it and cannot flip it here. - // If any of them ever moves off that executor, an unguarded start - // would play the buffered samples after the caller asked to stop, - // because stop() only pauses a track already in PLAYSTATE_PLAYING and - // a prefilling track is still STOPPED. - if (isGenerating && !playbackStarted && framesInCurrentTrack > 0) { + // The cancellation check is load-bearing, not defensive. stop() and + // release() now cancel on the caller's thread rather than queueing + // behind this generation (#440), so they can and do flip this mid-flight. + // Without the guard, a stop during prefill would still play the buffered + // samples: stop() only pauses a track already in PLAYSTATE_PLAYING, and a + // prefilling track is still STOPPED. + if (isActive(requestId) && isGenerating && !playbackStarted && framesInCurrentTrack > 0) { // The utterance ended below the prefill target, so the start // threshold will never be reached on its own. On API 31+ lower it // to what is actually buffered. Below API 31 the threshold is fixed @@ -651,6 +692,14 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { resultMap.putString("filePath", filePath) promise.resolve(resultMap) } else { + // A cancelled generation also lands here — generateWithCallback returns + // 0 and this path sees no audio — and tts.generate() cannot be + // interrupted, so falling through restarted synthesis and resolved it as + // success, undoing the stop entirely (#440). + if (!isActive(requestId)) { + throw Exception("TTS generation was cancelled") + } + // If no audio was generated, try again without callback Log.w(TAG, "No audio generated with callback method, trying again without callback") @@ -821,13 +870,13 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { * Stop TTS generation */ fun stop(promise: Promise) { - // Clear the flag on the CALLING thread, not the executor. init/generate/stop/release - // share one single-threaded executor, so queueing this behind an in-flight generate() - // meant it could not run until that generation had already finished — the - // isGenerating checks inside the callback loop could never fire during the generation - // they exist to interrupt, making stopTts() a no-op (#440). isGenerating is @Volatile, - // so the loop observes this write immediately. - isGenerating = false + // Cancel on the CALLING thread, not the executor. init/generate/stop/release share + // one single-threaded executor, so queueing this behind an in-flight generate() meant + // it could not run until that generation had already finished — the checks inside the + // callback loop could never fire during the generation they exist to interrupt, + // making stopTts() a no-op (#440). The fields are atomic/@Volatile, so the loop is + // guaranteed to see this write rather than a cached value. + cancelOutstandingRequests() executor.execute { try { @@ -864,9 +913,9 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { * Release TTS resources */ fun release(promise: Promise) { - // Same reason as stop(): clear the flag before queueing, so an in-flight generation - // stops feeding the executor rather than blocking this call behind itself (#440). - isGenerating = false + // Same reason as stop(): cancel before queueing, so an in-flight generation stops + // feeding the executor rather than blocking this call behind itself (#440). + cancelOutstandingRequests() executor.execute { try { From 6c556d31df5b8e933a1b7464c64b733f1237ea63 Mon Sep 17 00:00:00 2001 From: abretonc7s Date: Fri, 21 Aug 2026 17:41:54 +0800 Subject: [PATCH 3/6] fix(sherpa-onnx.rn/android): check cancellation after the blocking calls, not only before MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review found the token was consulted before every blocking call and after none of them, so a stop arriving during synthesis still reported success. `tts.generate()` cannot be interrupted at all, and it is what `playAudio=false` uses — the service default. The guard added last round sat before that call, so the entire synthesis window was unchecked: a stop during it fell straight through to `promise.resolve({success: true})`. The fallback path had the same shape. Both branches converge before the result is built, so one check there covers both. Playback could also start after a cancel. The callback checked cancellation on entry, but the chunk loop and the blocking `AudioTrack.write()` that follow can span a stop, and `stop()`'s pause/flush is queued behind this generation — so it cannot undo a `play()` that already ran. Both `play()` sites now re-check immediately before starting: the prefill-threshold one in the chunk loop, and the short-utterance tail. The CAS loop was confirmed correct by the reviewer's stress probe (100 rounds, 1000 concurrent raises), so it is unchanged. Kotlin compiles and the unit tests pass. Device validation still pending — the A/B that distinguishes a working stop from a no-op is duration and sample count, not the log line, which appears in both builds. --- .../siteed/sherpaonnx/handlers/TtsHandler.kt | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt b/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt index 921bc2253..280335dba 100644 --- a/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt +++ b/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt @@ -586,7 +586,12 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { totalSamplesWritten += written framesInCurrentTrack += written offset += written - if (PrefillPolicy.shouldStart( + // isActive first: the AudioTrack.write() above blocks, + // so a stop can land between the callback's entry check + // and here. stop()'s pause/flush is queued behind this + // generation and cannot undo a play() that already + // ran (#440). + if (isActive(requestId) && PrefillPolicy.shouldStart( playbackStarted, framesInCurrentTrack, currentSampleRate, @@ -660,9 +665,17 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { Log.d(TAG, "Padded short utterance with $padded silent frames to reach the pre-S start threshold") } } - audioTrack?.play() - playbackStarted = true - Log.d(TAG, "AudioTrack playback started after short utterance prefill: $framesInCurrentTrack samples") + // Re-checked immediately before play(): the padding writes above + // block, so a stop can land between the guard on this branch and + // here. stop()'s pause/flush is queued behind this generation, so + // audio started now would keep playing after the cancel (#440). + if (isActive(requestId)) { + audioTrack?.play() + playbackStarted = true + Log.d(TAG, "AudioTrack playback started after short utterance prefill: $framesInCurrentTrack samples") + } else { + Log.i(TAG, "Skipping playback start: generation was cancelled during prefill") + } } Log.d(TAG, "Completed callback generation. Total samples: $totalSamplesWritten in $totalCalls callback calls, head=${audioTrack?.playbackHeadPosition}, playState=${audioTrack?.playState}") @@ -673,6 +686,14 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { audio = generatedAudio?.samples } + // Both branches above can block for the whole synthesis — tts.generate() + // cannot be interrupted at all — so a stop that arrives during one of them + // is only observable here. Without this check the cancelled request still + // fell through to promise.resolve({success: true}) below (#440). + if (!isActive(requestId)) { + throw Exception("TTS generation was cancelled") + } + val endTime = System.currentTimeMillis() val duration = endTime - startTime Log.d(TAG, "Speech generation completed in ${duration}ms") From 8c507b2fca0db2066d433c892a54f6006daab512 Mon Sep 17 00:00:00 2001 From: abretonc7s Date: Sat, 22 Aug 2026 12:21:26 +0800 Subject: [PATCH 4/6] fix(sherpa-onnx.rn/android): cancel inside the chunk loop, stop conflating the flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from review, all real. The callback checked cancellation only on entry, so a stop landing mid-chunk kept issuing blocking writes and returned 1, telling the engine to carry on while stop()'s pause/flush sat queued behind the generation. It is now checked every chunk. The tts.generate() fallback checked the token before the call but not after. That call blocks for the whole synthesis and cannot be interrupted, so a stop during it still saved the file and resolved success. Both play() sites were check-then-act. A cancel landing between isActive() and play() started playback that stop() could not undo, its pause/flush being queued behind us. Each site now rechecks immediately after play() and pauses and flushes the track itself, since it is the only actor that can act before the executor drains. cancelOutstandingRequests snapshotted the counter and then cleared the shared isGenerating flag. A request claiming an id after that snapshot is not covered by the cancel, but its callback aborted on the global flag anyway. The flag is no longer touched there and no longer consulted as a cancellation signal — the request id already names exactly which requests are cancelled, and the generation that owns the flag clears it in its own finally. Device evidence. The reviewer noted my previous proof had zero callback calls, so it never exercised these windows. This run establishes a baseline first: uninterrupted 118784 samples, 1 callback call, playState 3 stopped at 1500ms 45056 samples, "interrupted mid-chunk by stop request", "Error generating speech: TTS generation was cancelled" 62% truncated, through the new in-loop check rather than the entry check, which is the path the earlier prefill-only test could not reach. Kotlin compiles and the 16 unit tests pass. --- .../siteed/sherpaonnx/handlers/TtsHandler.kt | 50 +++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt b/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt index 280335dba..d79fbb7fb 100644 --- a/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt +++ b/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt @@ -52,7 +52,11 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { val current = cancelledThrough.get() if (current >= newest || cancelledThrough.compareAndSet(current, newest)) break } - isGenerating = false + // isGenerating is deliberately NOT cleared here. It is a coarse "something is + // running" flag shared by every request, and clearing it aborted the callback of a + // request that claimed its id after the snapshot above — one this cancel does not + // cover. cancelledThrough already names exactly which requests are cancelled; the + // generation that owns the flag clears it in its own finally (#440). } private var audioTrack: AudioTrack? = null private var ttsModelConfig: OfflineTtsModelConfig? = null @@ -515,7 +519,7 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { // of invoke([F)Ljava/lang/Integer; in generateWithCallbackImpl. tts?.generateWithCallback(text, speakerId, speakingRate, object : Function1 { override fun invoke(samples: FloatArray): Int { - if (!isActive(requestId) || !isGenerating) { + if (!isActive(requestId)) { Log.i(TAG, "TTS generation interrupted by stop request") return 0 // Stop generating } @@ -562,6 +566,14 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { var offset = 0 while (offset < shortSamples.size) { + // Checked every chunk, not just on callback entry. A stop + // landing mid-chunk otherwise kept issuing blocking writes + // and returned 1, so the engine carried on generating + // while stop()'s pause/flush sat queued behind it (#440). + if (!isActive(requestId)) { + Log.i(TAG, "TTS generation interrupted mid-chunk by stop request") + return 0 + } val remainingSize = shortSamples.size - offset val currentChunkSize = kotlin.math.min(chunkSize, remainingSize) @@ -601,6 +613,21 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { ) { audioTrack?.play() playbackStarted = true + // Recheck after play(): a cancel landing between + // the guard above and here would otherwise start + // playback that stop() cannot undo, since its + // pause/flush is queued behind this generation. + // We are the only actor that can stop it now. + if (!isActive(requestId)) { + Log.i(TAG, "Cancelled during playback start; stopping the track") + try { + audioTrack?.pause() + audioTrack?.flush() + } catch (e: Exception) { + Log.w(TAG, "Failed to stop playback after cancel: ${e.message}") + } + return 0 + } // Head is expected to still be 0 here — output advances // asynchronously. The value logged at generation end is // the one that shows whether playback actually ran. @@ -634,7 +661,7 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { // Without the guard, a stop during prefill would still play the buffered // samples: stop() only pauses a track already in PLAYSTATE_PLAYING, and a // prefilling track is still STOPPED. - if (isActive(requestId) && isGenerating && !playbackStarted && framesInCurrentTrack > 0) { + if (isActive(requestId) && !playbackStarted && framesInCurrentTrack > 0) { // The utterance ended below the prefill target, so the start // threshold will never be reached on its own. On API 31+ lower it // to what is actually buffered. Below API 31 the threshold is fixed @@ -672,6 +699,16 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { if (isActive(requestId)) { audioTrack?.play() playbackStarted = true + // Same recheck as the chunk-loop play() above. + if (!isActive(requestId)) { + Log.i(TAG, "Cancelled during short-utterance playback start; stopping the track") + try { + audioTrack?.pause() + audioTrack?.flush() + } catch (e: Exception) { + Log.w(TAG, "Failed to stop playback after cancel: ${e.message}") + } + } Log.d(TAG, "AudioTrack playback started after short utterance prefill: $framesInCurrentTrack samples") } else { Log.i(TAG, "Skipping playback start: generation was cancelled during prefill") @@ -683,6 +720,13 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { // Generate without playback Log.d(TAG, "Using generate method without callback") val generatedAudio = tts?.generate(text, speakerId, speakingRate) + // tts.generate() blocks for the whole synthesis and cannot be + // interrupted, so a stop arriving during it is only observable here. + // Without this the cancelled request still saved a file and resolved + // success (#440). + if (!isActive(requestId)) { + throw Exception("TTS generation was cancelled") + } audio = generatedAudio?.samples } From 5732e87f086e8bd9b1d2d5ce11c814271b03440a Mon Sep 17 00:00:00 2001 From: abretonc7s Date: Sat, 22 Aug 2026 12:33:28 +0800 Subject: [PATCH 5/6] fix(sherpa-onnx.rn/android): guard the second fallback, make completion atomic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-4 review found three, including one I thought I had already fixed. There are two call sites for the blocking tts.generate(). I guarded one last round and never looked for another. The one I missed is the path that actually runs: the callback's return value is discarded, so audio is null and every uninterrupted playAudio=true request reaches it. A stop during that call still saved the WAV and resolved success. Both are guarded now. Cancellation and completion were not atomic. A recheck before the file write narrows the window but cannot close it — the write is synchronous, so a stop landing partway through left the request cancelled and resolved at once. Completion is now claimed through tryCompleteRequest under the same monitor cancelOutstandingRequests takes, so exactly one of the two wins. Both completion sites claim before writing. stop()'s queued cleanup paused and flushed the shared AudioTrack unconditionally, including for a generation that claimed its id after the cutoff and is therefore not cancelled by that stop. It now compares the counter against the cutoff it established and leaves a newer generation's track alone. Device-verified, fresh install (lastUpdateTime 12:31:03), both directions, because the completion claim sits in the success path too: uninterrupted 115456 samples, "Audio saved: true", no cancel stopped at 1500ms 45056 samples, "interrupted mid-chunk by stop request", "TTS generation was cancelled", no file written Kotlin compiles and the 16 unit tests pass. --- .../siteed/sherpaonnx/handlers/TtsHandler.kt | 70 +++++++++++++++++-- 1 file changed, 64 insertions(+), 6 deletions(-) diff --git a/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt b/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt index d79fbb7fb..13f049bd3 100644 --- a/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt +++ b/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt @@ -40,6 +40,31 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { /** Highest request id that has been cancelled. See [requestCounter]. */ private val cancelledThrough = java.util.concurrent.atomic.AtomicLong(0) + /** + * Request ids that have committed to resolving successfully. + * + * A recheck before writing the file narrows the cancel-versus-complete race but cannot + * close it: the WAV write is synchronous and a stop can land partway through, leaving + * the request cancelled and resolved at once. Claiming completion here is a single + * atomic step, so exactly one of the two wins (#440). + */ + private val completedRequests = + java.util.Collections.synchronizedSet(java.util.HashSet()) + + /** + * Claim completion for [requestId], or report that a cancel got there first. + * + * Returns false if the request was already cancelled, in which case the caller must + * not resolve. + */ + private fun tryCompleteRequest(requestId: Long): Boolean { + synchronized(completedRequests) { + if (!isActive(requestId)) return false + completedRequests.add(requestId) + return true + } + } + /** Whether [requestId] may keep generating, or has been cancelled out from under it. */ private fun isActive(requestId: Long): Boolean = requestId > cancelledThrough.get() @@ -47,10 +72,14 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { /** Cancel every request claimed so far. Safe to call from any thread. */ private fun cancelOutstandingRequests() { val newest = requestCounter.get() - // Raise the bar rather than assigning: a concurrent cancel must not lower it. - while (true) { - val current = cancelledThrough.get() - if (current >= newest || cancelledThrough.compareAndSet(current, newest)) break + // Under the same monitor as tryCompleteRequest, so cancel and complete cannot + // interleave: whichever takes it first decides that request's fate. + synchronized(completedRequests) { + // Raise the bar rather than assigning: a concurrent cancel must not lower it. + while (true) { + val current = cancelledThrough.get() + if (current >= newest || cancelledThrough.compareAndSet(current, newest)) break + } } // isGenerating is deliberately NOT cleared here. It is a coarse "something is // running" flag shared by every request, and clearing it aborted the callback of a @@ -748,6 +777,13 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { ?: "generated_audio_${System.currentTimeMillis()}" val filePath = "${reactContext.cacheDir.absolutePath}/$fileName.wav" + // Claim completion before writing. The write is synchronous, so a + // recheck alone leaves a window where a stop lands partway through and + // the request is both cancelled and resolved (#440). + if (!tryCompleteRequest(requestId)) { + throw Exception("TTS generation was cancelled") + } + // Use the correct sample rate from the model val saved = AudioUtils.saveAsWav(audio, currentSampleRate, filePath) Log.d(TAG, "Audio saved: $saved, file path: $filePath") @@ -770,6 +806,14 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { // Fallback: try again without callback val generatedAudio = tts?.generate(text, speakerId, speakingRate) + // Same recheck as the other fallback: this call blocks for the + // whole synthesis and cannot be interrupted, so a stop during it + // is only observable here. This is the common path, not the + // exceptional one — the callback's return is discarded above, so + // every uninterrupted playAudio=true request lands here (#440). + if (!isActive(requestId)) { + throw Exception("TTS generation was cancelled") + } audio = generatedAudio?.samples if (audio == null) { @@ -780,6 +824,13 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { ?: "generated_audio_${System.currentTimeMillis()}" val filePath = "${reactContext.cacheDir.absolutePath}/$fileName.wav" + // Claim completion before writing. The write is synchronous, so a + // recheck alone leaves a window where a stop lands partway through and + // the request is both cancelled and resolved (#440). + if (!tryCompleteRequest(requestId)) { + throw Exception("TTS generation was cancelled") + } + // Use the correct sample rate from the model val saved = AudioUtils.saveAsWav(audio, currentSampleRate, filePath) Log.d(TAG, "Audio saved: $saved, file path: $filePath") @@ -942,6 +993,11 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { // making stopTts() a no-op (#440). The fields are atomic/@Volatile, so the loop is // guaranteed to see this write rather than a cached value. cancelOutstandingRequests() + // The cutoff this stop established. A generation that claims a later id is not + // covered by it, so the queued cleanup below must not touch the shared AudioTrack + // on its behalf — that would contradict the promise that post-cutoff requests are + // unaffected (#440). + val cutoff = cancelledThrough.get() executor.execute { try { @@ -950,8 +1006,10 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { // Already cleared above; the executor body handles the AudioTrack, which must // stay on this thread because the generation loop writes to it from here. - // Stop audio playback - if (audioTrack?.playState == AudioTrack.PLAYSTATE_PLAYING) { + // Stop audio playback, unless a newer generation now owns the track. + if (requestCounter.get() > cutoff) { + Log.d(TAG, "Skipping AudioTrack cleanup: request ${requestCounter.get()} started after this stop") + } else if (audioTrack?.playState == AudioTrack.PLAYSTATE_PLAYING) { audioTrack?.pause() audioTrack?.flush() Log.d(TAG, "Paused and flushed AudioTrack") From 19649dd98ac3623c60b2827b61919f54d67075f1 Mon Sep 17 00:00:00 2001 From: abretonc7s Date: Sat, 22 Aug 2026 12:45:36 +0800 Subject: [PATCH 6/6] fix(sherpa-onnx.rn/android): track the AudioTrack owner, drop the growing set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-5 review found two. My stop cleanup compared requestCounter against the cutoff, which says a newer request exists — not that it owns the track. Call stopTts() and immediately queue a playAudio=false generate: the counter bumps while the cancelled request's track is still playing, cleanup skips it, and nothing else ever releases it, since the new request has no playback. The owner is now recorded when the track is built and cleared when it is released, and stop() compares that instead. completedRequests was only ever a monitor. Nothing read or removed its entries, so it grew by one boxed Long per successful synthesis. Replaced with a plain lock object, which gives the same mutual exclusion between cancel and complete. Device-verified, fresh install (lastUpdateTime 12:43:13), both directions: uninterrupted 105216 samples, "Audio saved: true" stopped at 1500ms 45056 samples, "interrupted mid-chunk by stop request", "TTS generation was cancelled", and "Paused and flushed AudioTrack" That last line is the point: the cancelled request owns the track, so cleanup runs. The skip path exists for a track a newer generation owns. Kotlin compiles and the 16 unit tests pass. --- .../siteed/sherpaonnx/handlers/TtsHandler.kt | 56 +++++++++++-------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt b/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt index 13f049bd3..5767300d4 100644 --- a/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt +++ b/packages/sherpa-onnx.rn/android/src/main/kotlin/net/siteed/sherpaonnx/handlers/TtsHandler.kt @@ -41,15 +41,21 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { private val cancelledThrough = java.util.concurrent.atomic.AtomicLong(0) /** - * Request ids that have committed to resolving successfully. + * Guards the cancel-versus-complete transition. * - * A recheck before writing the file narrows the cancel-versus-complete race but cannot - * close it: the WAV write is synchronous and a stop can land partway through, leaving - * the request cancelled and resolved at once. Claiming completion here is a single - * atomic step, so exactly one of the two wins (#440). + * A recheck before writing the file narrows that race but cannot close it: the WAV + * write is synchronous and a stop can land partway through, leaving the request + * cancelled and resolved at once. Holding this across both decisions makes exactly one + * of them win (#440). + * + * A plain lock rather than a set of completed ids: nothing ever read those entries, so + * the set only grew, one boxed Long per successful synthesis. */ - private val completedRequests = - java.util.Collections.synchronizedSet(java.util.HashSet()) + private val completionLock = Any() + + /** The request that owns [audioTrack], or 0 when nothing does. */ + @Volatile + private var audioTrackOwner: Long = 0 /** * Claim completion for [requestId], or report that a cancel got there first. @@ -57,13 +63,8 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { * Returns false if the request was already cancelled, in which case the caller must * not resolve. */ - private fun tryCompleteRequest(requestId: Long): Boolean { - synchronized(completedRequests) { - if (!isActive(requestId)) return false - completedRequests.add(requestId) - return true - } - } + private fun tryCompleteRequest(requestId: Long): Boolean = + synchronized(completionLock) { isActive(requestId) } /** Whether [requestId] may keep generating, or has been cancelled out from under it. */ private fun isActive(requestId: Long): Boolean = @@ -72,9 +73,9 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { /** Cancel every request claimed so far. Safe to call from any thread. */ private fun cancelOutstandingRequests() { val newest = requestCounter.get() - // Under the same monitor as tryCompleteRequest, so cancel and complete cannot + // Under the same lock as tryCompleteRequest, so cancel and complete cannot // interleave: whichever takes it first decides that request's fate. - synchronized(completedRequests) { + synchronized(completionLock) { // Raise the bar rather than assigning: a concurrent cancel must not lower it. while (true) { val current = cancelledThrough.get() @@ -516,7 +517,7 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { // Prefill the track before starting playback. Starting an empty stream // makes the first callback prone to an audible underrun. - initAudioTrack(currentSampleRate, startPlayback = false) + initAudioTrack(currentSampleRate, startPlayback = false, ownerRequestId = requestId) if (audioTrack?.state != AudioTrack.STATE_INITIALIZED) { Log.e(TAG, "Failed to initialize AudioTrack for playback!") @@ -564,7 +565,7 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { // prefill before starting. Carrying playbackStarted over // would start it immediately and let a remainder below the // threshold play as silence — or not at all. - initAudioTrack(currentSampleRate, startPlayback = false) + initAudioTrack(currentSampleRate, startPlayback = false, ownerRequestId = requestId) PrefillPolicy.onTrackReplaced().let { framesInCurrentTrack = it.framesInCurrentTrack playbackStarted = it.playbackStarted @@ -610,7 +611,7 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { if (audioTrack?.state != AudioTrack.STATE_INITIALIZED) { Log.w(TAG, "AudioTrack disabled during chunk write, reinitializing...") releaseAudioTrack() - initAudioTrack(currentSampleRate, startPlayback = false) + initAudioTrack(currentSampleRate, startPlayback = false, ownerRequestId = requestId) PrefillPolicy.onTrackReplaced().let { framesInCurrentTrack = it.framesInCurrentTrack playbackStarted = it.playbackStarted @@ -854,6 +855,9 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { * Release AudioTrack resources */ private fun releaseAudioTrack() { + // No owner once the track is gone; a stale id here would make stop() skip cleanup + // for a track that no longer exists. + audioTrackOwner = 0 try { // Release whenever the track is non-null, not only when it reached // STATE_INITIALIZED. A track whose build() succeeded but whose init @@ -876,7 +880,7 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { /** * Initialize audio track for playback */ - private fun initAudioTrack(sampleRate: Int, startPlayback: Boolean = true) { + private fun initAudioTrack(sampleRate: Int, startPlayback: Boolean = true, ownerRequestId: Long = 0) { try { // Release any existing AudioTrack first to prevent resource conflicts. // Same unconditional release as releaseAudioTrack(): an uninitialized @@ -907,6 +911,9 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { Log.d(TAG, "Creating AudioTrack with buffer size: $minBufferSize bytes, sample rate: $sherpaModelSampleRate Hz") // Create with explicit stream type for maximum compatibility + // Claim ownership before building: stop() uses this to tell a track that + // belongs to a newer generation from one left behind by a cancelled request. + audioTrackOwner = ownerRequestId audioTrack = AudioTrack.Builder() .setAudioAttributes( AudioAttributes.Builder() @@ -1007,8 +1014,13 @@ class TtsHandler(private val reactContext: ReactApplicationContext) { // stay on this thread because the generation loop writes to it from here. // Stop audio playback, unless a newer generation now owns the track. - if (requestCounter.get() > cutoff) { - Log.d(TAG, "Skipping AudioTrack cleanup: request ${requestCounter.get()} started after this stop") + // The owner, not the counter. A newer request existing does not mean it + // owns the track: stopTts() followed immediately by a playAudio=false + // generate bumps the counter while the cancelled request's track is still + // playing, and nothing else would ever release it (#440). + val owner = audioTrackOwner + if (owner > cutoff) { + Log.d(TAG, "Skipping AudioTrack cleanup: request $owner owns the track and started after this stop") } else if (audioTrack?.playState == AudioTrack.PLAYSTATE_PLAYING) { audioTrack?.pause() audioTrack?.flush()