commit dd582929677fab6ffdb52f36cd1e16855bb7321a
author Phil Burk <philburk@google.com> Thu Oct 15 20:29:51 2020 +0000
committer Phil Burk <philburk@google.com> Wed Oct 21 00:37:34 2020 +0000
tree 76c8557c340edeacb098665d331ba7a7f6f398e2
parent 9bcf4989bf3e09167503e94571b15626e56f4453

aaudio: cleanup thread handling

Fix problem when an attempt is made to join the callback from the
callback. It used to leave mHasThread in the wrong state.
This was fixed in AAudioThread and in AudioStream.

Add "_l" suffix to functions that need to be locked.

Simplify the way that reference counted objects are passed to threads
using incStrong() and decStrong().

Bug: 171296283
Test: atest AAudioTestCases
Change-Id: I034049c4cb9021c6073fff441e49214ee898b804

media/libaaudio/src/client/AudioStreamInternal.cpp

diff --git a/media/libaaudio/src/client/AudioStreamInternal.cpp b/media/libaaudio/src/client/AudioStreamInternal.cpp
index 94f10e5..c2dcd35 100644
--- a/media/libaaudio/src/client/AudioStreamInternal.cpp
+++ b/media/libaaudio/src/client/AudioStreamInternal.cpp
@@ -75,6 +75,7 @@
 }
 
 AudioStreamInternal::~AudioStreamInternal() {
+    ALOGD("%s() %p called", __func__, this);
 }
 
 aaudio_result_t AudioStreamInternal::open(const AudioStreamBuilder &builder) {
@@ -270,21 +271,21 @@
     return result;
 
 error:
-    releaseCloseFinal();
+    safeReleaseClose();
     return result;
 }
 
 // This must be called under mStreamLock.
 aaudio_result_t AudioStreamInternal::release_l() {
     aaudio_result_t result = AAUDIO_OK;
-    ALOGV("%s(): mServiceStreamHandle = 0x%08X", __func__, mServiceStreamHandle);
+    ALOGD("%s(): mServiceStreamHandle = 0x%08X", __func__, mServiceStreamHandle);
     if (mServiceStreamHandle != AAUDIO_HANDLE_INVALID) {
         aaudio_stream_state_t currentState = getState();
         // Don't release a stream while it is running. Stop it first.
         // If DISCONNECTED then we should still try to stop in case the
         // error callback is still running.
         if (isActive() || currentState == AAUDIO_STREAM_STATE_DISCONNECTED) {
-            requestStop();
+            requestStop_l();
         }
 
         logReleaseBufferState();
@@ -330,7 +331,7 @@
  * The processing code will then save the current offset
  * between client and server and apply that to any position given to the app.
  */
-aaudio_result_t AudioStreamInternal::requestStart()
+aaudio_result_t AudioStreamInternal::requestStart_l()
 {
     int64_t startTime;
     if (mServiceStreamHandle == AAUDIO_HANDLE_INVALID) {
@@ -373,7 +374,7 @@
                               * AAUDIO_NANOS_PER_SECOND
                               / getSampleRate();
         mCallbackEnabled.store(true);
-        result = createThread(periodNanos, aaudio_callback_thread_proc, this);
+        result = createThread_l(periodNanos, aaudio_callback_thread_proc, this);
     }
     if (result != AAUDIO_OK) {
         setState(originalState);
@@ -399,26 +400,29 @@
 }
 
 // This must be called under mStreamLock.
-aaudio_result_t AudioStreamInternal::stopCallback()
+aaudio_result_t AudioStreamInternal::stopCallback_l()
 {
     if (isDataCallbackSet()
             && (isActive() || getState() == AAUDIO_STREAM_STATE_DISCONNECTED)) {
         mCallbackEnabled.store(false);
-        aaudio_result_t result = joinThread(NULL); // may temporarily unlock mStreamLock
+        aaudio_result_t result = joinThread_l(NULL); // may temporarily unlock mStreamLock
         if (result == AAUDIO_ERROR_INVALID_HANDLE) {
             ALOGD("%s() INVALID_HANDLE, stream was probably stolen", __func__);
             result = AAUDIO_OK;
         }
         return result;
     } else {
+        ALOGD("%s() skipped, isDataCallbackSet() = %d, isActive() = %d, getState()  = %d", __func__,
+            isDataCallbackSet(), isActive(), getState());
         return AAUDIO_OK;
     }
 }
 
 // This must be called under mStreamLock.
-aaudio_result_t AudioStreamInternal::requestStop() {
-    aaudio_result_t result = stopCallback();
+aaudio_result_t AudioStreamInternal::requestStop_l() {
+    aaudio_result_t result = stopCallback_l();
     if (result != AAUDIO_OK) {
+        ALOGW("%s() stop callback returned %d, returning early", __func__, result);
         return result;
     }
     // The stream may have been unlocked temporarily to let a callback finish
@@ -426,6 +430,7 @@
     // Check to make sure the stream still needs to be stopped.
     // See also AudioStream::safeStop().
     if (!(isActive() || getState() == AAUDIO_STREAM_STATE_DISCONNECTED)) {
+        ALOGD("%s() returning early, not active or disconnected", __func__);
         return AAUDIO_OK;
     }
 
@@ -805,11 +810,15 @@
     return mBufferCapacityInFrames;
 }
 
-// This must be called under mStreamLock.
 aaudio_result_t AudioStreamInternal::joinThread(void** returnArg) {
     return AudioStream::joinThread(returnArg, calculateReasonableTimeout(getFramesPerBurst()));
 }
 
+// This must be called under mStreamLock.
+aaudio_result_t AudioStreamInternal::joinThread_l(void** returnArg) {
+    return AudioStream::joinThread_l(returnArg, calculateReasonableTimeout(getFramesPerBurst()));
+}
+
 bool AudioStreamInternal::isClockModelInControl() const {
     return isActive() && mAudioEndpoint->isFreeRunning() && mClockModel.isRunning();
 }