From 678ba43bc33d2233cd8552c6a3713f64b1859e9c Mon Sep 17 00:00:00 2001 From: Kedar Fitwe <43736999+KD6763@users.noreply.github.com> Date: Wed, 8 Jul 2026 14:30:49 -0500 Subject: [PATCH 1/4] android: stop and detach the AudioRecord before joining the reader thread stopRecordingIfNeededImpl joined the reader thread while holding audioRecordStateLock and never stopped the AudioRecord first. Two consequences: - The reader acquires that lock at the top of every loop iteration, so a stop racing a loop boundary deadlocks against the join until its 2s timeout: the thread waits for the lock while the join waits for the thread. Observed in production as a ~2s stall on every capture stop cycle under churn. - A reader stuck inside a blocking AudioRecord.read() (stalled capture HAL, e.g. Bluetooth SCO route churn) has nothing to unblock it, so the join times out structurally. Stop the record inside the locked phase (a blocking read() returns promptly once the record leaves the recording state), detach it from the shared field so a concurrent init/start builds fresh state instead of touching a record the stopping thread may still be reading, and perform the join outside the lock. The stop entry points clear their request flags under the lock and run the join outside it; a concurrent stop that loses the race now returns cleanly instead of tripping the audioThread assert. --- .../org/webrtc/audio/WebRtcAudioRecord.java | 59 +++++++++++++++---- 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java b/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java index d8c76783ee..ed26bd227a 100644 --- a/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java +++ b/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java @@ -663,11 +663,11 @@ public boolean stopRecordingIfNeeded() { Logging.d(TAG, "stopRecordingIfNeeded"); synchronized(audioRecordStateLock) { clientCalledStartRecording.set(false); - if(audioThread != null) { - return stopRecordingIfNeededImpl(); + if (audioThread == null) { + return true; } } - return true; + return stopRecordingIfNeededImpl(); } @CalledByNative @@ -676,19 +676,25 @@ private boolean stopRecording() { synchronized(audioRecordStateLock) { nativeCalledStartRecording.set(false); nativeCalledInitRecording.set(false); - return stopRecordingIfNeededImpl(); } + return stopRecordingIfNeededImpl(); } private boolean stopRecordingIfNeededImpl() { + final AudioRecordThread stoppingThread; + final @Nullable AudioRecord stoppingRecord; synchronized(audioRecordStateLock) { if(clientCalledStartRecording.get() || nativeCalledStartRecording.get()) { // Someone has still requested recording, ignore stop request. return true; } + if (audioThread == null) { + // Already stopped by a concurrent caller. + return true; + } + Logging.d(TAG, "stopping recording"); - assertTrue(audioThread != null); if (future != null) { if (!future.isDone()) { // Might be needed if the client calls startRecording(), stopRecording() back-to-back. @@ -696,15 +702,44 @@ private boolean stopRecordingIfNeededImpl() { } future = null; } - audioThread.stopThread(); - if (!ThreadUtils.joinUninterruptibly(audioThread, AUDIO_RECORD_THREAD_JOIN_TIMEOUT_MS)) { - Logging.e(TAG, "Join of AudioRecordJavaThread timed out"); - WebRtcAudioUtils.logAudioState(TAG, context, audioManager); - } + stoppingThread = audioThread; audioThread = null; - releaseAudioResources(); - return true; + stoppingThread.stopThread(); + + // Detach the AudioRecord from the shared state before waiting for the + // thread, so a subsequent init/start builds fresh state instead of + // touching a record the stopping thread may still be reading. + stoppingRecord = audioRecord; + audioRecord = null; + effects.release(); + audioSourceMatchesRecordingSessionRef.set(null); + + // Stop the record before joining the thread. A blocking + // AudioRecord.read() only returns promptly once the record leaves the + // recording state, so without this the join below times out whenever + // the reader is stuck inside read(). + if (stoppingRecord != null) { + try { + stoppingRecord.stop(); + } catch (IllegalStateException e) { + Logging.e(TAG, "AudioRecord.stop failed: " + e.getMessage()); + } + } + } + + // Join outside audioRecordStateLock. The audio thread acquires that lock + // at the top of every loop iteration, so joining while holding it turns + // any stop that races a loop boundary into a guaranteed join timeout: + // the thread waits for the lock while the join waits for the thread. + if (!ThreadUtils.joinUninterruptibly(stoppingThread, AUDIO_RECORD_THREAD_JOIN_TIMEOUT_MS)) { + Logging.e(TAG, "Join of AudioRecordJavaThread timed out"); + WebRtcAudioUtils.logAudioState(TAG, context, audioManager); } + + if (stoppingRecord != null) { + stoppingRecord.release(); + } + return true; } @TargetApi(Build.VERSION_CODES.M) From 75a947f2a4159061f79eeaef01e4d9decab76c04 Mon Sep 17 00:00:00 2001 From: Kedar Fitwe <43736999+KD6763@users.noreply.github.com> Date: Wed, 8 Jul 2026 14:30:51 -0500 Subject: [PATCH 2/4] android: never release the AudioRecord under a live reader thread When the stop path's join timed out it released the AudioRecord anyway, while the abandoned reader thread could still be inside AudioRecord.read() on that record. Releasing a record under an in-flight read corrupts the platform client proxy accounting and aborts the process: releaseBuffer: mUnreleased out of range, !(stepCount:480 <= mUnreleased:0 <= mFrameCount:3840) On join timeout, transfer release responsibility to the reader through an atomic handoff slot (orphanedRecord + cleanupDone): the reader releases the record when it finally exits, and if it has already passed its cleanup point during the handoff the stop path reclaims and releases it. Exactly one side performs the release in every interleaving. --- .../org/webrtc/audio/WebRtcAudioRecord.java | 48 ++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java b/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java index ed26bd227a..a01f916efc 100644 --- a/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java +++ b/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java @@ -128,6 +128,11 @@ class WebRtcAudioRecord { */ private class AudioRecordThread extends Thread { private volatile boolean keepAlive = true; + // Handoff slot for an AudioRecord whose stop path gave up waiting for + // this thread (see stopRecordingIfNeededImpl). Exactly one side wins the + // getAndSet and performs the release. + private final AtomicReference orphanedRecord = new AtomicReference<>(); + private final AtomicBoolean cleanupDone = new AtomicBoolean(false); public AudioRecordThread(String name) { super(name); @@ -255,15 +260,41 @@ public void run() { } catch (IllegalStateException e) { Logging.e(TAG, "AudioRecord.stop failed: " + e.getMessage()); } + cleanupDone.set(true); + AudioRecord orphan = orphanedRecord.getAndSet(null); + if (orphan != null) { + // The stop path timed out joining this thread and transferred release + // responsibility here: releasing while this thread might still have + // been inside AudioRecord.read() would corrupt the platform client + // proxy state and abort the process. + Logging.w(TAG, "Releasing orphaned AudioRecord after delayed thread exit"); + try { + orphan.stop(); + } catch (IllegalStateException e) { + // Already stopped by the stop path. + } + orphan.release(); + } doAudioRecordStateCallback(AUDIO_RECORD_STOP); } - // Stops the inner thread loop and also calls AudioRecord.stop(). - // Does not block the calling thread. + // Stops the inner thread loop. Does not block the calling thread. public void stopThread() { Logging.d(TAG, "stopThread"); keepAlive = false; } + + // Called by the stop path when its join timed out. Returns null when this + // thread will release the record on exit, or the record itself when this + // thread has already finished cleanup and the caller must release it. + // Exactly one side observes the record, in every interleaving. + public @Nullable AudioRecord transferRecordOwnership(AudioRecord record) { + orphanedRecord.set(record); + if (cleanupDone.get()) { + return orphanedRecord.getAndSet(null); + } + return null; + } } @CalledByNative @@ -734,6 +765,19 @@ private boolean stopRecordingIfNeededImpl() { if (!ThreadUtils.joinUninterruptibly(stoppingThread, AUDIO_RECORD_THREAD_JOIN_TIMEOUT_MS)) { Logging.e(TAG, "Join of AudioRecordJavaThread timed out"); WebRtcAudioUtils.logAudioState(TAG, context, audioManager); + // The thread may still be inside AudioRecord.read(). Releasing the + // record here corrupts the platform AudioRecord client proxy and + // aborts the process (releaseBuffer: mUnreleased out of range), so + // hand release responsibility to the thread instead. + if (stoppingRecord != null) { + AudioRecord unclaimed = stoppingThread.transferRecordOwnership(stoppingRecord); + if (unclaimed != null) { + // The thread finished its cleanup during the handoff; no reader can + // be inside read() anymore, so releasing here is safe. + unclaimed.release(); + } + } + return true; } if (stoppingRecord != null) { From ea4c4244e80e99d5aa69b1fed086daf3ec36a6ca Mon Sep 17 00:00:00 2001 From: Kedar Fitwe <43736999+KD6763@users.noreply.github.com> Date: Wed, 8 Jul 2026 14:30:53 -0500 Subject: [PATCH 3/4] android: prevent a stopping reader from touching successor recording state With the join no longer serializing everything under audioRecordStateLock, a reader that is being stopped could observe state belonging to the next recording session and corrupt it: - The loop-top snapshot re-reads the shared audioRecord field each iteration, so a reader that passed the while(keepAlive) check could adopt a successor record installed during the stop's join window, read it concurrently with the new reader, and stop it on exit. keepAlive is now re-checked against the snapshot (the stop path publishes keepAlive=false before detaching, under the same lock), and exit cleanup stops only the record this thread actually read (activeRecord), never the shared field. - The startup preamble read the shared field twice without the lock and asserted on its recording state, which now races the stop's detach+stop: taken under the lock with a keepAlive guard. - The mid-loop re-init leg could re-create a record after a stop had already finished, leaving a record behind that no stop path would ever release; the re-init, restart, and state verification now run in one critical section gated on a keepAlive re-check, and a record stopped by shutdown is no longer misreported as a start failure (which permanently disabled useAudioRecord). - A read unblocked by the stop path's own AudioRecord.stop() exits through the loop condition instead of logging a spurious read error. --- .../org/webrtc/audio/WebRtcAudioRecord.java | 113 ++++++++++++++---- 1 file changed, 89 insertions(+), 24 deletions(-) diff --git a/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java b/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java index a01f916efc..ae33a33b90 100644 --- a/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java +++ b/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java @@ -128,6 +128,12 @@ class WebRtcAudioRecord { */ private class AudioRecordThread extends Thread { private volatile boolean keepAlive = true; + // The AudioRecord this thread most recently read from. Written only by + // this thread while running, read by it again during exit cleanup. The + // shared WebRtcAudioRecord.this.audioRecord field may already point at a + // newer record by the time this thread exits, so exit cleanup must not + // touch the shared field. + private @Nullable AudioRecord activeRecord; // Handoff slot for an AudioRecord whose stop path gave up waiting for // this thread (see stopRecordingIfNeededImpl). Exactly one side wins the // getAndSet and performs the release. @@ -142,8 +148,14 @@ public AudioRecordThread(String name) { public void run() { Process.setThreadPriority(Process.THREAD_PRIORITY_URGENT_AUDIO); Logging.d(TAG, "AudioRecordThread" + WebRtcAudioUtils.getThreadInfo()); - if (audioRecord != null) { - assertTrue(audioRecord.getRecordingState() == AudioRecord.RECORDSTATE_RECORDING); + // Snapshot and assert under the lock so a concurrent stop (which sets + // keepAlive=false, detaches the record, and stops it under the same + // lock) cannot interleave between the null check and the state read. + synchronized (audioRecordStateLock) { + AudioRecord startupRecord = WebRtcAudioRecord.this.audioRecord; + if (keepAlive && startupRecord != null) { + assertTrue(startupRecord.getRecordingState() == AudioRecord.RECORDSTATE_RECORDING); + } } // Audio recording has started and the client is informed about it. @@ -162,31 +174,67 @@ public void run() { audioRecord = WebRtcAudioRecord.this.audioRecord; shouldReportData = nativeCalledInitRecording.get(); } - - if (audioRecord == null && useAudioRecord) { - boolean result = initAudioRecord(); + // Re-check after the snapshot: a stop may have completed while this + // thread waited at the monitor, and a subsequent start may already + // have installed a NEW record into the shared field. The stop path + // publishes keepAlive=false before detaching, under the same lock, so + // a snapshot that can observe a successor record always observes + // keepAlive == false. Exit without touching the successor. + if (!keepAlive) { + break; + } + + // Do not re-create the AudioRecord once stopThread() has been called + // (re-checked under the lock inside the branch): a dying thread that + // re-inits would leave behind a record that no stop path will ever + // release. + if (audioRecord == null && useAudioRecord && keepAlive) { + boolean result; + synchronized (audioRecordStateLock) { + // A stop may have completed while this thread was blocked on the + // monitor; the keepAlive read above is stale in that case. + result = keepAlive && initAudioRecord(); + } if (!result) { - // Failed audio record init, don't try again. - useAudioRecord = false; + if (keepAlive) { + // Failed audio record init, don't try again. + useAudioRecord = false; + } } else { + boolean startFailed = false; + boolean stateMismatch = false; synchronized (audioRecordStateLock) { - audioRecord = WebRtcAudioRecord.this.audioRecord; + // Re-snapshot, start, and verify in one critical section, gated + // on keepAlive: a stop that completed in between must not let a + // dying reader adopt (and later stop) a successor record, and a + // record stopped by shutdown is not a start failure. + audioRecord = keepAlive ? WebRtcAudioRecord.this.audioRecord : null; + if (audioRecord != null) { + try { + audioRecord.startRecording(); + } catch (IllegalStateException e) { + startFailed = true; + reportWebRtcAudioRecordStartError(AudioRecordStartErrorCode.AUDIO_RECORD_START_EXCEPTION, + "AudioRecord.startRecording failed: " + e.getMessage()); + } + if (!startFailed + && audioRecord.getRecordingState() != AudioRecord.RECORDSTATE_RECORDING) { + stateMismatch = true; + reportWebRtcAudioRecordStartError(AudioRecordStartErrorCode.AUDIO_RECORD_START_STATE_MISMATCH, + "AudioRecord.startRecording failed - incorrect state: " + + audioRecord.getRecordingState()); + } + } } - - assertTrue(audioRecord != null); - try { - audioRecord.startRecording(); - } catch (IllegalStateException e) { - reportWebRtcAudioRecordStartError(AudioRecordStartErrorCode.AUDIO_RECORD_START_EXCEPTION, - "AudioRecord.startRecording failed: " + e.getMessage()); - audioRecord = null; - useAudioRecord = false; + if (audioRecord == null) { + // A concurrent stop either detached the fresh record before this + // thread could start it or finished while this thread waited at + // the monitor; that stop owns the record's release. Exit via the + // loop condition. + continue; } - if (useAudioRecord && audioRecord.getRecordingState() != AudioRecord.RECORDSTATE_RECORDING) { - reportWebRtcAudioRecordStartError(AudioRecordStartErrorCode.AUDIO_RECORD_START_STATE_MISMATCH, - "AudioRecord.startRecording failed - incorrect state: " - + audioRecord.getRecordingState()); + if (startFailed || stateMismatch) { audioRecord = null; useAudioRecord = false; } @@ -194,10 +242,19 @@ public void run() { } if (audioRecord != null && !useAudioRecord) { + synchronized (audioRecordStateLock) { + // Release through the shared path only while this record is still + // the shared one; a concurrent stop may have detached it already + // and taken over its release. + if (WebRtcAudioRecord.this.audioRecord == audioRecord) { + releaseAudioResources(); + } + } audioRecord = null; - releaseAudioResources(); } + activeRecord = audioRecord; + int bytesRead = 0; if (audioRecord != null) { bytesRead = audioRecord.read(byteBuffer, byteBuffer.capacity()); @@ -216,6 +273,11 @@ public void run() { } } } else { + if (!keepAlive) { + // The stop path stopped this record to unblock the read; exit + // through the loop condition without reporting an error. + continue; + } String errorMessage = "AudioRecord.read failed: " + bytesRead; Logging.e(TAG, errorMessage); @@ -253,9 +315,12 @@ public void run() { } } + // Stop only the record this thread was reading. The shared field may + // already point at a newer AudioRecord created by a subsequent start; + // stopping that one would corrupt the new reader's in-flight read. try { - if (audioRecord != null) { - audioRecord.stop(); + if (activeRecord != null) { + activeRecord.stop(); } } catch (IllegalStateException e) { Logging.e(TAG, "AudioRecord.stop failed: " + e.getMessage()); From 5571f522f4e8f5a706bcdc70df230fb3d0646af3 Mon Sep 17 00:00:00 2001 From: Kedar Fitwe Date: Mon, 3 Aug 2026 21:59:35 -0500 Subject: [PATCH 4/4] android: add Robolectric coverage for the AudioRecord stop-path race Covers the stop-path contract fixed in the stop/join rework: a stop must never release the AudioRecord under a reader still inside a blocking read(); a join timeout hands release ownership to the reader (orphan handoff, released exactly once); a leaked reader confines exit cleanup to the record it was reading and never touches a successor session's record; shutdown races never misreport a start failure or permanently disable useAudioRecord; and concurrent stop callers both succeed. The AudioRecord is a Mockito fake with a deterministic state machine whose read() blocks like a stalled capture HAL; record and buffer are injected by reflection because initRecordingImpl needs a direct ByteBuffer with a backing array, which the test JVM does not provide. --- sdk/android/BUILD.gn | 1 + .../audio/WebRtcAudioRecordStopRaceTest.java | 426 ++++++++++++++++++ 2 files changed, 427 insertions(+) create mode 100644 sdk/android/tests/src/org/webrtc/audio/WebRtcAudioRecordStopRaceTest.java diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn index b77b2efec2..c031f331ad 100644 --- a/sdk/android/BUILD.gn +++ b/sdk/android/BUILD.gn @@ -1912,6 +1912,7 @@ if (is_android) { "tests/src/org/webrtc/RenderSynchronizerTest.java", "tests/src/org/webrtc/ScalingSettingsTest.java", "tests/src/org/webrtc/audio/LowLatencyAudioBufferManagerTest.java", + "tests/src/org/webrtc/audio/WebRtcAudioRecordStopRaceTest.java", ] deps = [ diff --git a/sdk/android/tests/src/org/webrtc/audio/WebRtcAudioRecordStopRaceTest.java b/sdk/android/tests/src/org/webrtc/audio/WebRtcAudioRecordStopRaceTest.java new file mode 100644 index 0000000000..019972bbb1 --- /dev/null +++ b/sdk/android/tests/src/org/webrtc/audio/WebRtcAudioRecordStopRaceTest.java @@ -0,0 +1,426 @@ +/* + * Copyright 2026 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +package org.webrtc.audio; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.content.Context; +import android.media.AudioManager; +import android.media.AudioRecord; +import android.os.Build; +import androidx.test.runner.AndroidJUnit4; +import java.lang.reflect.Field; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; + +/** + * Tests for the WebRtcAudioRecord stop-path contract: a stop must never release the AudioRecord + * while the reader thread may still be inside a blocking AudioRecord.read() — on a device that + * corrupts the platform AudioRecord client proxy and aborts the process ("releaseBuffer: + * mUnreleased out of range"). A stop whose reader join times out hands release ownership to the + * reader instead; a leaked reader must confine its exit cleanup to the record it was reading and + * never touch a successor session's record; and a shutdown-stopped record must not be misreported + * as a start failure, which would permanently disable useAudioRecord and silence later sessions. + * + *

The AudioRecord is a Mockito fake with a deterministic state machine whose read() blocks like + * a stalled capture HAL: a STALLED read returns only once the record leaves the recording state + * (the platform behavior for a blocking read), while a WEDGED read ignores stop() entirely and + * returns only when explicitly unstuck, forcing the stop path's join to time out. The record and + * capture buffer are injected by reflection because initRecordingImpl needs a direct ByteBuffer + * with a backing array, which exists on ART but not on the test JVM; the start/stop machinery + * under test runs unmodified. + */ +@RunWith(AndroidJUnit4.class) +@Config(manifest = Config.NONE, sdk = Build.VERSION_CODES.O) +public class WebRtcAudioRecordStopRaceTest { + private static final int CAPTURE_BUFFER_BYTES = 960; + private static final long AWAIT_TIMEOUT_MS = 5000; + private static final long DRAIN_TIMEOUT_MS = 3000; + private static final int SAMPLE_RATE = 48000; + private static final int CHANNELS = 1; + + /** Deterministic AudioRecord fake whose read() blocks like a stalled capture HAL. */ + private static class FakeRecord { + /** read() returns once the record leaves the recording state, like a blocking HAL read. */ + static final int STALLED = 0; + /** read() ignores stop() and returns only when unstuck; forces the stop-path join timeout. */ + static final int WEDGED = 1; + + final AudioRecord record = mock(AudioRecord.class); + final AtomicBoolean recording = new AtomicBoolean(false); + final AtomicBoolean released = new AtomicBoolean(false); + final AtomicInteger releaseCount = new AtomicInteger(0); + final AtomicInteger readsStarted = new AtomicInteger(0); + final AtomicBoolean inRead = new AtomicBoolean(false); + final AtomicBoolean releasedDuringRead = new AtomicBoolean(false); + final AtomicBoolean stoppedCleanly = new AtomicBoolean(false); + final CountDownLatch unstick = new CountDownLatch(1); + + FakeRecord(int readBehavior) { + when(record.getState()) + .thenAnswer(invocation + -> released.get() ? AudioRecord.STATE_UNINITIALIZED : AudioRecord.STATE_INITIALIZED); + when(record.getRecordingState()) + .thenAnswer(invocation + -> recording.get() ? AudioRecord.RECORDSTATE_RECORDING + : AudioRecord.RECORDSTATE_STOPPED); + doAnswer(invocation -> { + recording.set(true); + return null; + }).when(record).startRecording(); + doAnswer(invocation -> { + recording.set(false); + return null; + }).when(record).stop(); + doAnswer(invocation -> { + recording.set(false); + released.set(true); + releaseCount.incrementAndGet(); + if (inRead.get()) { + // On a device this is the releaseBuffer abort: release() landed while a reader was + // still inside read(). + releasedDuringRead.set(true); + } + return null; + }).when(record).release(); + when(record.read(any(ByteBuffer.class), anyInt())).thenAnswer(invocation -> { + readsStarted.incrementAndGet(); + inRead.set(true); + try { + if (readBehavior == WEDGED) { + unstick.await(); + return 0; + } + while (recording.get() && !released.get()) { + Thread.sleep(1); + } + if (!released.get()) { + stoppedCleanly.set(true); + } + return 0; + } finally { + inRead.set(false); + } + }); + } + } + + private ScheduledExecutorService scheduler; + private final List fakes = new ArrayList<>(); + + @Before + public void setUp() { + scheduler = Executors.newSingleThreadScheduledExecutor(); + } + + @After + public void tearDown() throws InterruptedException { + // A failed assertion mid-test must not leak a blocked reader into the next test: unblock + // every fake read and drain before shutting down. + for (FakeRecord fake : fakes) { + fake.recording.set(false); + fake.unstick.countDown(); + } + long deadline = System.currentTimeMillis() + DRAIN_TIMEOUT_MS; + while (readerThreadCount() > 0 && System.currentTimeMillis() < deadline) { + Thread.sleep(10); + } + scheduler.shutdownNow(); + } + + @Test + public void stopDuringStalledRead_neverReleasesRecordUnderTheReader() throws Exception { + FakeRecord fake = newFake(FakeRecord.STALLED); + WebRtcAudioRecord webRtcAudioRecord = createWebRtcAudioRecord(null); + injectRecordingState(webRtcAudioRecord, fake.record); + assertTrue(webRtcAudioRecord.startRecordingIfNeeded()); + awaitTrue("reader never entered read()", () -> fake.readsStarted.get() > 0); + + long stopStartedAtMs = System.currentTimeMillis(); + assertTrue(webRtcAudioRecord.stopRecordingIfNeeded()); + long stopDurationMs = System.currentTimeMillis() - stopStartedAtMs; + + awaitTrue("reader thread leaked past stop", () -> readerThreadCount() == 0); + assertFalse("AudioRecord.release() was invoked while a read was in flight — on a device this" + + " is the releaseBuffer abort", + fake.releasedDuringRead.get()); + // The stop must unblock the stalled read via AudioRecord.stop() instead of burning the 2s + // join timeout and abandoning the thread. + assertTrue("stopRecordingIfNeeded took " + stopDurationMs + + "ms — it hit the join timeout instead of stopping the record to unblock the read", + stopDurationMs < 1500); + assertTrue("read did not observe a clean stop", fake.stoppedCleanly.get()); + assertEquals("record must be released exactly once", 1, fake.releaseCount.get()); + } + + @Test + public void stopThenImmediateRestart_neverLeaksReadersOrReleasesUnderRead() throws Exception { + WebRtcAudioRecord webRtcAudioRecord = createWebRtcAudioRecord(null); + for (int cycle = 0; cycle < 5; cycle++) { + FakeRecord fake = newFake(FakeRecord.STALLED); + injectRecordingState(webRtcAudioRecord, fake.record); + assertTrue("start cycle=" + cycle, webRtcAudioRecord.startRecordingIfNeeded()); + Thread.sleep(30); + assertTrue("stop cycle=" + cycle, webRtcAudioRecord.stopRecordingIfNeeded()); + awaitTrue("leaked reader threads after cycle=" + cycle, () -> readerThreadCount() == 0); + } + for (FakeRecord fake : fakes) { + assertFalse("release-during-read violation", fake.releasedDuringRead.get()); + assertEquals("record must be released exactly once", 1, fake.releaseCount.get()); + } + } + + /** + * The join-timeout branch: when not even a stopped record unblocks the read (wedged HAL), the + * stop path must return without releasing the record under the in-flight read — release + * ownership transfers to the reader, which releases the orphan on exit. + */ + @Test + public void joinTimeout_handsRecordToReaderInsteadOfReleasingUnderIt() throws Exception { + FakeRecord fake = newFake(FakeRecord.WEDGED); + WebRtcAudioRecord webRtcAudioRecord = createWebRtcAudioRecord(null); + injectRecordingState(webRtcAudioRecord, fake.record); + assertTrue(webRtcAudioRecord.startRecordingIfNeeded()); + awaitTrue("reader never entered read()", () -> fake.readsStarted.get() > 0); + + long stopStartedAtMs = System.currentTimeMillis(); + assertTrue(webRtcAudioRecord.stopRecordingIfNeeded()); + long stopDurationMs = System.currentTimeMillis() - stopStartedAtMs; + + // A wedged read cannot be unblocked, so this stop legitimately burns the full join timeout; + // the duration proves the timeout branch actually ran. + assertTrue("stop returned in " + stopDurationMs + + "ms — the wedged read should have forced the join timeout", + stopDurationMs >= 1900); + assertTrue("reader should still be wedged inside read()", readerThreadCount() > 0); + assertFalse("record was released under the wedged read", fake.releasedDuringRead.get()); + assertEquals("record must stay unreleased while the reader may still be inside read()", 0, + fake.releaseCount.get()); + + fake.unstick.countDown(); + awaitTrue("reader did not exit after the read unwedged", () -> readerThreadCount() == 0); + assertFalse(fake.releasedDuringRead.get()); + assertEquals( + "reader must release the orphaned record exactly once on exit", 1, fake.releaseCount.get()); + } + + /** + * A leaked (join-timed-out) reader overlapping a successor session must confine its exit + * cleanup to the record it was reading: the successor's record keeps recording, untouched. + */ + @Test + public void leakedReaderExit_neverTouchesSuccessorRecord() throws Exception { + FakeRecord wedged = newFake(FakeRecord.WEDGED); + FakeRecord successor = newFake(FakeRecord.STALLED); + WebRtcAudioRecord webRtcAudioRecord = createWebRtcAudioRecord(null); + injectRecordingState(webRtcAudioRecord, wedged.record); + assertTrue(webRtcAudioRecord.startRecordingIfNeeded()); + awaitTrue("first reader never entered read()", () -> wedged.readsStarted.get() > 0); + + // The wedged read forces the join timeout; the reader leaks by design. + assertTrue(webRtcAudioRecord.stopRecordingIfNeeded()); + assertEquals("expected exactly the wedged reader alive", 1, readerThreadCount()); + + // A successor session starts while the old reader is still inside read(). + injectRecordingState(webRtcAudioRecord, successor.record); + assertTrue(webRtcAudioRecord.startRecordingIfNeeded()); + awaitTrue("successor reader never entered read()", () -> successor.readsStarted.get() > 0); + assertEquals(2, readerThreadCount()); + + wedged.unstick.countDown(); + awaitTrue("wedged reader did not exit", () -> readerThreadCount() == 1); + assertEquals("old reader must release its orphaned record exactly once", 1, + wedged.releaseCount.get()); + verify(successor.record, never()).stop(); + assertEquals("old reader must not stop the successor record", + AudioRecord.RECORDSTATE_RECORDING, successor.record.getRecordingState()); + assertEquals("old reader must not release the successor record", 0, + successor.releaseCount.get()); + assertFalse(successor.releasedDuringRead.get()); + + assertTrue(webRtcAudioRecord.stopRecordingIfNeeded()); + awaitTrue("successor reader did not exit on clean stop", () -> readerThreadCount() == 0); + assertFalse(successor.releasedDuringRead.get()); + assertTrue("successor read did not observe a clean stop", successor.stoppedCleanly.get()); + assertEquals(1, successor.releaseCount.get()); + } + + /** + * Startup-window races: a stop landing immediately after start must not crash the reader + * preamble, must not report a start/read error for a record the shutdown itself stopped, and + * must leave capture usable — misreporting shutdown as a start failure permanently disabled + * useAudioRecord, silencing every later session. + */ + @Test + public void immediateStopAfterStart_neverMisreportsErrorOrDisablesCapture() throws Exception { + List errors = new CopyOnWriteArrayList<>(); + JavaAudioDeviceModule.AudioRecordErrorCallback errorCallback = + new JavaAudioDeviceModule.AudioRecordErrorCallback() { + @Override + public void onWebRtcAudioRecordInitError(String errorMessage) { + errors.add("init: " + errorMessage); + } + + @Override + public void onWebRtcAudioRecordStartError( + JavaAudioDeviceModule.AudioRecordStartErrorCode errorCode, String errorMessage) { + errors.add("start " + errorCode + ": " + errorMessage); + } + + @Override + public void onWebRtcAudioRecordError(String errorMessage) { + errors.add("runtime: " + errorMessage); + } + }; + + WebRtcAudioRecord webRtcAudioRecord = createWebRtcAudioRecord(errorCallback); + for (int cycle = 0; cycle < 20; cycle++) { + FakeRecord fake = newFake(FakeRecord.STALLED); + injectRecordingState(webRtcAudioRecord, fake.record); + assertTrue("start cycle=" + cycle, webRtcAudioRecord.startRecordingIfNeeded()); + assertTrue("stop cycle=" + cycle, webRtcAudioRecord.stopRecordingIfNeeded()); + awaitTrue("reader leaked after racing cycle=" + cycle, () -> readerThreadCount() == 0); + } + + // Capture must still work after every racing shutdown. + FakeRecord finalFake = newFake(FakeRecord.STALLED); + injectRecordingState(webRtcAudioRecord, finalFake.record); + assertTrue(webRtcAudioRecord.startRecordingIfNeeded()); + awaitTrue("capture no longer reads after racing stop cycles — useAudioRecord was disabled", + () -> finalFake.readsStarted.get() > 0); + assertTrue(webRtcAudioRecord.stopRecordingIfNeeded()); + awaitTrue("reader leaked after final cycle", () -> readerThreadCount() == 0); + + for (FakeRecord fake : fakes) { + assertFalse("release-during-read violation", fake.releasedDuringRead.get()); + } + assertTrue("errors reported for clean shutdown races: " + errors, errors.isEmpty()); + } + + /** + * stopRecordingIfNeededImpl runs outside audioRecordStateLock, making concurrent stop callers + * a legal interleaving: both must succeed, with one clean release. + */ + @Test + public void concurrentStops_bothSucceedWithSingleCleanRelease() throws Exception { + for (int cycle = 0; cycle < 10; cycle++) { + FakeRecord fake = newFake(FakeRecord.STALLED); + WebRtcAudioRecord webRtcAudioRecord = createWebRtcAudioRecord(null); + injectRecordingState(webRtcAudioRecord, fake.record); + assertTrue("start cycle=" + cycle, webRtcAudioRecord.startRecordingIfNeeded()); + awaitTrue("cycle=" + cycle + " reader never entered read()", + () -> fake.readsStarted.get() > 0); + + CyclicBarrier barrier = new CyclicBarrier(2); + Boolean[] results = new Boolean[2]; + List failures = new CopyOnWriteArrayList<>(); + Thread[] stoppers = new Thread[2]; + for (int i = 0; i < 2; i++) { + final int index = i; + stoppers[i] = new Thread(() -> { + try { + barrier.await(); + results[index] = webRtcAudioRecord.stopRecordingIfNeeded(); + } catch (Throwable t) { + failures.add(t); + } + }); + stoppers[i].start(); + } + for (Thread stopper : stoppers) { + stopper.join(5000); + assertFalse("cycle=" + cycle + " stopper thread did not finish", stopper.isAlive()); + } + assertTrue("cycle=" + cycle + " stop callers threw: " + failures, failures.isEmpty()); + assertEquals("cycle=" + cycle + " first stop must succeed", Boolean.TRUE, results[0]); + assertEquals("cycle=" + cycle + " second stop must succeed", Boolean.TRUE, results[1]); + awaitTrue("cycle=" + cycle + " reader leaked", () -> readerThreadCount() == 0); + assertFalse("cycle=" + cycle + " release-during-read", fake.releasedDuringRead.get()); + assertEquals("cycle=" + cycle + " record must be released exactly once", 1, + fake.releaseCount.get()); + } + } + + private FakeRecord newFake(int readBehavior) { + FakeRecord fake = new FakeRecord(readBehavior); + fakes.add(fake); + return fake; + } + + private WebRtcAudioRecord createWebRtcAudioRecord( + JavaAudioDeviceModule.AudioRecordErrorCallback errorCallback) { + Context context = RuntimeEnvironment.getApplication(); + AudioManager audioManager = (AudioManager) context.getSystemService(Context.AUDIO_SERVICE); + return new WebRtcAudioRecord(context, scheduler, audioManager, + WebRtcAudioRecord.DEFAULT_AUDIO_SOURCE, WebRtcAudioRecord.DEFAULT_AUDIO_FORMAT, + errorCallback, null /* stateCallback */, null /* audioSamplesReadyCallback */, + null /* audioBufferCallback */, false /* isAcousticEchoCancelerSupported */, + false /* isNoiseSuppressorSupported */, SAMPLE_RATE, CHANNELS); + } + + private static void injectRecordingState(WebRtcAudioRecord webRtcAudioRecord, AudioRecord record) + throws Exception { + ByteBuffer byteBuffer = ByteBuffer.allocate(CAPTURE_BUFFER_BYTES); + setField(webRtcAudioRecord, "audioRecord", record); + setField(webRtcAudioRecord, "byteBuffer", byteBuffer); + setField(webRtcAudioRecord, "emptyBytes", new byte[byteBuffer.capacity()]); + } + + private static void setField(WebRtcAudioRecord instance, String name, Object value) + throws Exception { + Field field = WebRtcAudioRecord.class.getDeclaredField(name); + field.setAccessible(true); + field.set(instance, value); + } + + private static int readerThreadCount() { + int count = 0; + for (Thread thread : Thread.getAllStackTraces().keySet()) { + if (thread.isAlive() && "AudioRecordJavaThread".equals(thread.getName())) { + count++; + } + } + return count; + } + + private static void awaitTrue(String message, Supplier condition) + throws InterruptedException { + long deadline = System.currentTimeMillis() + AWAIT_TIMEOUT_MS; + while (!condition.get() && System.currentTimeMillis() < deadline) { + Thread.sleep(5); + } + assertTrue(message, condition.get()); + } +}