diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn index b77b2efec20..c031f331ad3 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/src/java/org/webrtc/audio/WebRtcAudioRecord.java b/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java index d8c76783eed..ae33a33b90e 100644 --- a/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java +++ b/sdk/android/src/java/org/webrtc/audio/WebRtcAudioRecord.java @@ -128,6 +128,17 @@ 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. + private final AtomicReference orphanedRecord = new AtomicReference<>(); + private final AtomicBoolean cleanupDone = new AtomicBoolean(false); public AudioRecordThread(String name) { super(name); @@ -137,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. @@ -157,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; } @@ -189,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()); @@ -211,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); @@ -248,22 +315,51 @@ 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()); } + 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 @@ -663,11 +759,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 +772,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 +798,57 @@ 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(); + 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); + // 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) { + stoppingRecord.release(); + } + return true; } @TargetApi(Build.VERSION_CODES.M) 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 00000000000..019972bbb19 --- /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()); + } +}