fix: Android recorderController.stop() never completes#486
Conversation
Prevent encoder hang and ensure proper cleanup when stopping recordings. Add queueEosBuffer helper and proactively queue EOS in signalToStop if an input buffer is available, run MediaCodec callback on the handler, move completionCallback to finally, call release() after sending results or on error, and post result.success on the main looper for thread-safety.
Post the EOS check to the handler thread so EOS queuing runs on the same thread as onInputBufferAvailable, preventing race conditions when an input buffer is already available. Add an early return if the encoder is stopped. Move handlerThread.quitSafely() into the finally block and remove join() to avoid deadlocking when stopEncoder is invoked from callbacks; ensure completionCallback is invoked before quitting the thread.
|
Same issue! |
|
This does not work for me it actually crashes the app when I call stop(). |
| // Quit the handler thread after invoking the callback. | ||
| // Don't call join() -- stopEncoder() is called from callbacks | ||
| // running on this same thread, so joining would deadlock. | ||
| handlerThread.quitSafely() |
There was a problem hiding this comment.
🟡 mediaCodec.release() runs before quitSafely(), but quitSafely() still drains messages already queued on this handler before the thread quits. If an onInputBufferAvailable / feedEncoder message is already pending, it runs after release and calls getInputBuffer() on a released codec → IllegalStateException on the handler thread (uncaught — feedEncoder() has no try/catch). Consider handler.removeCallbacksAndMessages(null) before releasing the codec, or guarding the codec calls in feedEncoder. Low-probability, but the unguarded path exists.
| // both threads try to queue EOS with the same buffer index. | ||
| handler.post { | ||
| if (isEncoderStopped) return@post | ||
| if (currentInputBufferIndex >= 0 && inputQueue.isEmpty()) { |
There was a problem hiding this comment.
🟡 currentInputBufferIndex, isEncodingComplete and totalBytesEncoded (declared at lines 58–76) are accessed from three threads: the recording thread (queueInputBuffer / feedEncoder), this handler thread, and the caller thread of signalToStop. The handler.post here adds a happens-before barrier for this path (good), but onInputBufferAvailable reads isEncodingComplete directly from the framework with no barrier, and queueInputBuffer reads currentInputBufferIndex on the recording thread unsynchronized. Recommend marking these fields @Volatile to make cross-thread visibility explicit.
| hashMap[Constants.resultFilePath] = recorderSettings?.path | ||
| hashMap[Constants.resultDuration] = duration | ||
| result.success(hashMap) | ||
| Handler(Looper.getMainLooper()).post { |
There was a problem hiding this comment.
🟢 Minor: sendBytesToFlutter runs on every audio chunk (hot path), and both it and sendRecordingResult allocate a new Handler per call. Hoist a single private val mainHandler = Handler(Looper.getMainLooper()) and reuse it.
| // Post the EOS check to the handler thread so it runs on the same | ||
| // thread as onInputBufferAvailable, avoiding race conditions where | ||
| // both threads try to queue EOS with the same buffer index. | ||
| handler.post { |
There was a problem hiding this comment.
🟢 Flagging a dependency (no change strictly required): EOS is only queued here when currentInputBufferIndex >= 0 && inputQueue.isEmpty(). Otherwise it relies on a later onInputBufferAvailable to drain the queue and queue EOS. In the normal stop flow that holds (AudioRecord is stopped, no new input, queue drains). But if an input buffer is never returned, stopEncoder → completionCallback never fires and the Flutter stop() future hangs again — the original bug. Worth a comment/assertion documenting the assumption.
|
Thanks for the fix, @rubenvde! This solves the stop()-never-completing hang nicely. I've left a few comments on the diff, a couple worth addressing before merge, plus two minor cleanups. Could you take a look and resolve them? Happy to discuss any of them. Thanks again! |
Hi @yuanhoujun! Thanks for chiming in. Just a heads-up: this PR is about the recorder- |
5 participants
Description
Fixes a hang where
await recorderController.stop()on Android never completes, especially for recordings >30 seconds. This was introduced in the 2.0.x rewrite that replacedMediaRecorderwithAudioRecord+MediaCodec.Root causes fixed:
Deadlock in
stopEncoder()—stopEncoder()is called fromonOutputBufferAvailableon the handler thread, but it calledhandlerThread.join(), waiting for itself to finish. Removed thejoin()anduse
quitSafely()only.Race condition in
signalToStop()— EOS buffer queuing now runs on the handler thread viahandler.post {}, avoiding races withonInputBufferAvailablethat caused "MediaCodec discarded an unknownbuffer" errors.
release()called before async completion — For the non-WAV path,release()was called synchronously aftersignalToStop(), before the completion callback fired. Movedrelease()into the completioncallback.
result.success()called from wrong thread —sendRecordingResult()now posts to the main looper, since Flutter'sMethodChannel.Resultmust be called on the platform thread.completionCallbackswallowed on error — MovedcompletionCallback?.invoke()to afinallyblock instopEncoder()so the Flutter result is always resolved.Missing handler in
setCallback()—mediaCodec.setCallback()now passes thehandlerso callbacks run on the intendedHandlerThread.Checklist
fix:,feat:,docs:etc).docsand added dartdoc comments with///.examplesordocs.There's no test infrastructure in the project and the bug is a native Android threading issue that's difficult to unit test from Dart
Breaking Change?
Related Issues
#470