aaudio: add AAudioStream_release()

Split close into close and release phases.
Release() will release hardware and service resources.
Close() will delete the stream object.
This allows us to defer the deletion and avoid race conditions.

The CLOSING state is used to indicate that a stream has been released.

Added some test cases to test_various.cpp.
A lone call to close() will automatically call release() so we should
have good code coverage for testing.

Bug: 136288001
Test: adb shell test_various
Test: atest CtsNativeMediaAAudioTestCases
Change-Id: Ia648838f30c521ba101f55259dbcd4594c1263cd

media/libaaudio/include/aaudio/AAudio.h

@@ -1021,10 +1021,26 @@ AAUDIO_API aaudio_result_t  AAudioStreamBuilder_delete(AAudioStreamBuilder* buil
 // Stream Control
 // ============================================================
 
+#if __ANDROID_API__ >= 30
 /**
- * Free the resources associated with a stream created by AAudioStreamBuilder_openStream()
+ * Free the audio resources associated with a stream created by
+ * AAudioStreamBuilder_openStream().
+ * AAudioStream_close() should be called at some point after calling
+ * this function.
  *
- * Available since API level 26.
+ * After this call, the stream will be in {@link #AAUDIO_STREAM_STATE_CLOSING}
+ *
+ * @param stream reference provided by AAudioStreamBuilder_openStream()
+ * @return {@link #AAUDIO_OK} or a negative error.
+ */
+AAUDIO_API aaudio_result_t  AAudioStream_release(AAudioStream* stream) __INTRODUCED_IN(30);
+#endif // __ANDROID_API__
+
+/**
+ * Delete the internal data structures associated with the stream created
+ * by AAudioStreamBuilder_openStream().
+ *
+ * If AAudioStream_release() has not been called then it will be called automatically.
  *
  * @param stream reference provided by AAudioStreamBuilder_openStream()
  * @return {@link #AAUDIO_OK} or a negative error.

media/libaaudio/src/client/AudioStreamInternal.cpp

@@ -258,18 +258,17 @@ aaudio_result_t AudioStreamInternal::open(const AudioStreamBuilder &builder) {
     return result;
 
 error:
-    close();
+    releaseCloseFinal();
     return result;
 }
 
 // This must be called under mStreamLock.
-aaudio_result_t AudioStreamInternal::close() {
+aaudio_result_t AudioStreamInternal::release_l() {
     aaudio_result_t result = AAUDIO_OK;
     ALOGV("%s(): mServiceStreamHandle = 0x%08X", __func__, mServiceStreamHandle);
     if (mServiceStreamHandle != AAUDIO_HANDLE_INVALID) {
-        // Don't close a stream while it is running.
         aaudio_stream_state_t currentState = getState();
-        // Don't close a stream while it is running. Stop it first.
+        // 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) {
@@ -282,10 +281,8 @@ aaudio_result_t AudioStreamInternal::close() {
         mServiceInterface.closeStream(serviceStreamHandle);
         delete[] mCallbackBuffer;
         mCallbackBuffer = nullptr;
-
-        setState(AAUDIO_STREAM_STATE_CLOSED);
         result = mEndPointParcelable.close();
-        aaudio_result_t result2 = AudioStream::close();
+        aaudio_result_t result2 = AudioStream::release_l();
         return (result != AAUDIO_OK) ? result : result2;
     } else {
         return AAUDIO_ERROR_INVALID_HANDLE;

media/libaaudio/src/client/AudioStreamInternal.h

@@ -58,7 +58,7 @@ public:
 
     aaudio_result_t open(const AudioStreamBuilder &builder) override;
 
-    aaudio_result_t close() override;
+    aaudio_result_t release_l() override;
 
     aaudio_result_t setBufferSize(int32_t requestedFrames) override;
 

media/libaaudio/src/client/AudioStreamInternalPlay.cpp

@@ -49,7 +49,7 @@ aaudio_result_t AudioStreamInternalPlay::open(const AudioStreamBuilder &builder)
                              getDeviceChannelCount());
 
         if (result != AAUDIO_OK) {
-            close();
+            releaseCloseFinal();
         }
         // Sample rate is constrained to common values by now and should not overflow.
         int32_t numFrames = kRampMSec * getSampleRate() / AAUDIO_MILLIS_PER_SECOND;

media/libaaudio/src/core/AAudioAudio.cpp

@@ -25,7 +25,6 @@
 
 #include <aaudio/AAudio.h>
 #include <aaudio/AAudioTesting.h>
-
 #include "AudioClock.h"
 #include "AudioGlobal.h"
 #include "AudioStreamBuilder.h"
@@ -231,21 +230,42 @@ AAUDIO_API aaudio_result_t  AAudioStreamBuilder_delete(AAudioStreamBuilder* buil
     return AAUDIO_ERROR_NULL;
 }
 
-AAUDIO_API aaudio_result_t  AAudioStream_close(AAudioStream* stream)
-{
+AAUDIO_API aaudio_result_t  AAudioStream_release(AAudioStream* stream) {
     aaudio_result_t result = AAUDIO_ERROR_NULL;
-    AudioStream *audioStream = convertAAudioStreamToAudioStream(stream);
+    AudioStream* audioStream = convertAAudioStreamToAudioStream(stream);
+    if (audioStream != nullptr) {
+        aaudio_stream_id_t id = audioStream->getId();
+        ALOGD("%s(s#%u) called ---------------", __func__, id);
+        result = audioStream->safeRelease();
+        // safeRelease() will only fail if called illegally, for example, from a callback.
+        // That would result in the release of an active stream, which would cause a crash.
+        if (result != AAUDIO_OK) {
+            ALOGW("%s(s#%u) failed. Release it from another thread.",
+                  __func__, id);
+        }
+        ALOGD("%s(s#%u) returned %d %s ---------", __func__,
+                id, result, AAudio_convertResultToText(result));
+    }
+    return result;
+}
+
+AAUDIO_API aaudio_result_t  AAudioStream_close(AAudioStream* stream) {
+    aaudio_result_t result = AAUDIO_ERROR_NULL;
+    AudioStream* audioStream = convertAAudioStreamToAudioStream(stream);
     if (audioStream != nullptr) {
         aaudio_stream_id_t id = audioStream->getId();
         ALOGD("%s(s#%u) called ---------------", __func__, id);
-        result = audioStream->safeClose();
-        // Close will only fail if called illegally, for example, from a callback.
+        result = audioStream->safeRelease();
+        // safeRelease will only fail if called illegally, for example, from a callback.
         // That would result in deleting an active stream, which would cause a crash.
-        if (result == AAUDIO_OK) {
+        if (result != AAUDIO_OK) {
+            ALOGW("%s(s#%u) failed. Close it from another thread.",
+                  __func__, id);
+        } else {
             audioStream->unregisterPlayerBase();
+             // Mark CLOSED to keep destructors from asserting.
+            audioStream->closeFinal();
             delete audioStream;
-        } else {
-            ALOGW("%s attempt to close failed. Close it from another thread.", __func__);
         }
         ALOGD("%s(s#%u) returned %d ---------", __func__, id, result);
     }

media/libaaudio/src/core/AudioGlobal.cpp

@@ -87,9 +87,9 @@ const char* AudioGlobal_convertStreamStateToText(aaudio_stream_state_t state) {
         AAUDIO_CASE_ENUM(AAUDIO_STREAM_STATE_FLUSHED);
         AAUDIO_CASE_ENUM(AAUDIO_STREAM_STATE_STOPPING);
         AAUDIO_CASE_ENUM(AAUDIO_STREAM_STATE_STOPPED);
-        AAUDIO_CASE_ENUM(AAUDIO_STREAM_STATE_DISCONNECTED);
         AAUDIO_CASE_ENUM(AAUDIO_STREAM_STATE_CLOSING);
         AAUDIO_CASE_ENUM(AAUDIO_STREAM_STATE_CLOSED);
+        AAUDIO_CASE_ENUM(AAUDIO_STREAM_STATE_DISCONNECTED);
     }
     return "Unrecognized AAudio state.";
 }

media/libaaudio/src/core/AudioStream.cpp

@@ -249,25 +249,29 @@ aaudio_result_t AudioStream::safeStop() {
     return requestStop();
 }
 
-aaudio_result_t AudioStream::safeClose() {
-    // This get temporarily unlocked in the close when joining callback threads.
+aaudio_result_t AudioStream::safeRelease() {
+    // This get temporarily unlocked in the release() when joining callback threads.
     std::lock_guard<std::mutex> lock(mStreamLock);
     if (collidesWithCallback()) {
         ALOGE("%s cannot be called from a callback!", __func__);
         return AAUDIO_ERROR_INVALID_STATE;
     }
-    return close();
+    if (getState() == AAUDIO_STREAM_STATE_CLOSING) {
+        return AAUDIO_OK;
+    }
+    return release_l();
 }
 
 void AudioStream::setState(aaudio_stream_state_t state) {
-    ALOGV("%s(%d) from %d to %d", __func__, getId(), mState, state);
+    ALOGD("%s(s#%d) from %d to %d", __func__, getId(), mState, state);
     // CLOSED is a final state
     if (mState == AAUDIO_STREAM_STATE_CLOSED) {
         ALOGE("%s(%d) tried to set to %d but already CLOSED", __func__, getId(), state);
 
-    // Once DISCONNECTED, we can only move to CLOSED state.
+    // Once DISCONNECTED, we can only move to CLOSING or CLOSED state.
     } else if (mState == AAUDIO_STREAM_STATE_DISCONNECTED
-               && state != AAUDIO_STREAM_STATE_CLOSED) {
+               && !(state == AAUDIO_STREAM_STATE_CLOSING
+                   || state == AAUDIO_STREAM_STATE_CLOSED)) {
         ALOGE("%s(%d) tried to set to %d but already DISCONNECTED", __func__, getId(), state);
 
     } else {

media/libaaudio/src/core/AudioStream.h

@@ -115,13 +115,32 @@ public:
     virtual aaudio_result_t open(const AudioStreamBuilder& builder);
 
     /**
-     * Close the stream and deallocate any resources from the open() call.
-     * It is safe to call close() multiple times.
+     * Free any hardware or system resources from the open() call.
+     * It is safe to call release_l() multiple times.
      */
-    virtual aaudio_result_t close() {
+    virtual aaudio_result_t release_l() {
+        setState(AAUDIO_STREAM_STATE_CLOSING);
         return AAUDIO_OK;
     }
 
+    aaudio_result_t closeFinal() {
+        // State is checked by destructor.
+        setState(AAUDIO_STREAM_STATE_CLOSED);
+        return AAUDIO_OK;
+    }
+
+    /**
+     * Release then close the stream.
+     * @return AAUDIO_OK or negative error.
+     */
+    aaudio_result_t releaseCloseFinal() {
+        aaudio_result_t result = release_l(); // TODO review locking
+        if (result == AAUDIO_OK) {
+          result = closeFinal();
+        }
+        return result;
+    }
+
     // This is only used to identify a stream in the logs without
     // revealing any pointers.
     aaudio_stream_id_t getId() {
@@ -373,7 +392,7 @@ public:
      */
     aaudio_result_t systemStopFromCallback();
 
-    aaudio_result_t safeClose();
+    aaudio_result_t safeRelease();
 
 protected:
 

media/libaaudio/src/legacy/AudioStreamRecord.cpp

@@ -182,7 +182,7 @@ aaudio_result_t AudioStreamRecord::open(const AudioStreamBuilder& builder)
         // Did we get a valid track?
         status_t status = mAudioRecord->initCheck();
         if (status != OK) {
-            close();
+            releaseCloseFinal();
             ALOGE("open(), initCheck() returned %d", status);
             return AAudioConvert_androidToAAudioResult(status);
         }
@@ -278,16 +278,17 @@ aaudio_result_t AudioStreamRecord::open(const AudioStreamBuilder& builder)
     return AAUDIO_OK;
 }
 
-aaudio_result_t AudioStreamRecord::close()
-{
-    // TODO add close() or release() to AudioRecord API then call it from here
-    if (getState() != AAUDIO_STREAM_STATE_CLOSED) {
+aaudio_result_t AudioStreamRecord::release_l() {
+    // TODO add close() or release() to AudioFlinger's AudioRecord API.
+    //  Then call it from here
+    if (getState() != AAUDIO_STREAM_STATE_CLOSING) {
         mAudioRecord->removeAudioDeviceCallback(mDeviceCallback);
         mAudioRecord.clear();
-        setState(AAUDIO_STREAM_STATE_CLOSED);
+        mFixedBlockWriter.close();
+        return AudioStream::release_l();
+    } else {
+        return AAUDIO_OK; // already released
     }
-    mFixedBlockWriter.close();
-    return AudioStream::close();
 }
 
 const void * AudioStreamRecord::maybeConvertDeviceData(const void *audioData, int32_t numFrames) {

media/libaaudio/src/legacy/AudioStreamRecord.h

@@ -38,7 +38,7 @@ public:
     virtual ~AudioStreamRecord();
 
     aaudio_result_t open(const AudioStreamBuilder & builder) override;
-    aaudio_result_t close() override;
+    aaudio_result_t release_l() override;
 
     aaudio_result_t requestStart() override;
     aaudio_result_t requestStop() override;

media/libaaudio/src/legacy/AudioStreamTrack.cpp

@@ -176,7 +176,7 @@ aaudio_result_t AudioStreamTrack::open(const AudioStreamBuilder& builder)
     // Did we get a valid track?
     status_t status = mAudioTrack->initCheck();
     if (status != NO_ERROR) {
-        close();
+        releaseCloseFinal();
         ALOGE("open(), initCheck() returned %d", status);
         return AAudioConvert_androidToAAudioResult(status);
     }
@@ -239,14 +239,18 @@ aaudio_result_t AudioStreamTrack::open(const AudioStreamBuilder& builder)
     return AAUDIO_OK;
 }
 
-aaudio_result_t AudioStreamTrack::close()
-{
-    if (getState() != AAUDIO_STREAM_STATE_CLOSED) {
+aaudio_result_t AudioStreamTrack::release_l() {
+    if (getState() != AAUDIO_STREAM_STATE_CLOSING) {
         mAudioTrack->removeAudioDeviceCallback(mDeviceCallback);
-        setState(AAUDIO_STREAM_STATE_CLOSED);
+        // TODO Investigate why clear() causes a hang in test_various.cpp
+        // if I call close() from a data callback.
+        // But the same thing in AudioRecord is OK!
+        // mAudioTrack.clear();
+        mFixedBlockReader.close();
+        return AudioStream::release_l();
+    } else {
+        return AAUDIO_OK; // already released
     }
-    mFixedBlockReader.close();
-    return AAUDIO_OK;
 }
 
 void AudioStreamTrack::processCallback(int event, void *info) {

media/libaaudio/src/legacy/AudioStreamTrack.h

@@ -41,7 +41,7 @@ public:
 
 
     aaudio_result_t open(const AudioStreamBuilder & builder) override;
-    aaudio_result_t close() override;
+    aaudio_result_t release_l() override;
 
     aaudio_result_t requestStart() override;
     aaudio_result_t requestPause() override;

media/libaaudio/src/libaaudio.map.txt

@@ -58,6 +58,7 @@ LIBAAUDIO {
     AAudioStream_getTimestamp;
     AAudioStream_isMMapUsed;
     AAudioStream_isPrivacySensitive;   # introduced=30
+    AAudioStream_release;        # introduced=30
   local:
     *;
 };

media/libaaudio/tests/test_various.cpp

@@ -40,10 +40,62 @@ aaudio_data_callback_result_t NoopDataCallbackProc(
     return AAUDIO_CALLBACK_RESULT_CONTINUE;
 }
 
-// Test AAudioStream_setBufferSizeInFrames()
-
 constexpr int64_t NANOS_PER_MILLISECOND = 1000 * 1000;
 
+void checkReleaseThenClose(aaudio_performance_mode_t perfMode,
+        aaudio_sharing_mode_t sharingMode) {
+    AAudioStreamBuilder* aaudioBuilder = nullptr;
+    AAudioStream* aaudioStream = nullptr;
+
+    // Use an AAudioStreamBuilder to contain requested parameters.
+    ASSERT_EQ(AAUDIO_OK, AAudio_createStreamBuilder(&aaudioBuilder));
+
+    // Request stream properties.
+    AAudioStreamBuilder_setDataCallback(aaudioBuilder,
+                                        NoopDataCallbackProc,
+                                        nullptr);
+    AAudioStreamBuilder_setPerformanceMode(aaudioBuilder, perfMode);
+    AAudioStreamBuilder_setSharingMode(aaudioBuilder, sharingMode);
+
+    // Create an AAudioStream using the Builder.
+    ASSERT_EQ(AAUDIO_OK,
+              AAudioStreamBuilder_openStream(aaudioBuilder, &aaudioStream));
+    AAudioStreamBuilder_delete(aaudioBuilder);
+
+    ASSERT_EQ(AAUDIO_OK, AAudioStream_requestStart(aaudioStream));
+
+    sleep(1);
+
+    EXPECT_EQ(AAUDIO_OK, AAudioStream_requestStop(aaudioStream));
+
+    EXPECT_EQ(AAUDIO_OK, AAudioStream_release(aaudioStream));
+    aaudio_stream_state_t state = AAudioStream_getState(aaudioStream);
+    EXPECT_EQ(AAUDIO_STREAM_STATE_CLOSING, state);
+
+    // We should be able to call this again without crashing.
+    EXPECT_EQ(AAUDIO_OK, AAudioStream_release(aaudioStream));
+    state = AAudioStream_getState(aaudioStream);
+    EXPECT_EQ(AAUDIO_STREAM_STATE_CLOSING, state);
+
+    EXPECT_EQ(AAUDIO_OK, AAudioStream_close(aaudioStream));
+}
+
+TEST(test_various, aaudio_release_close_none) {
+    checkReleaseThenClose(AAUDIO_PERFORMANCE_MODE_NONE,
+            AAUDIO_SHARING_MODE_SHARED);
+    // No EXCLUSIVE streams with MODE_NONE.
+}
+
+TEST(test_various, aaudio_release_close_low_shared) {
+    checkReleaseThenClose(AAUDIO_PERFORMANCE_MODE_LOW_LATENCY,
+            AAUDIO_SHARING_MODE_SHARED);
+}
+
+TEST(test_various, aaudio_release_close_low_exclusive) {
+    checkReleaseThenClose(AAUDIO_PERFORMANCE_MODE_LOW_LATENCY,
+            AAUDIO_SHARING_MODE_EXCLUSIVE);
+}
+
 enum FunctionToCall {
     CALL_START, CALL_STOP, CALL_PAUSE, CALL_FLUSH
 };

services/oboeservice/AAudioEndpointManager.cpp

@@ -284,7 +284,7 @@ void AAudioEndpointManager::closeSharedEndpoint(sp<AAudioServiceEndpoint> servic
 
         serviceEndpoint->close();
         mSharedCloseCount++;
-        ALOGV("%s() %p for device %d",
+        ALOGV("%s(%p) closed for device %d",
               __func__, serviceEndpoint.get(), serviceEndpoint->getDeviceId());
     }
 }

services/oboeservice/AAudioServiceEndpointShared.cpp

@@ -85,7 +85,7 @@ aaudio_result_t AAudioServiceEndpointShared::open(const aaudio::AAudioStreamRequ
 }
 
 aaudio_result_t AAudioServiceEndpointShared::close() {
-    return getStreamInternal()->close();
+    return getStreamInternal()->releaseCloseFinal();
 }
 
 // Glue between C and C++ callbacks.

services/oboeservice/AAudioServiceStreamBase.cpp

@@ -56,7 +56,8 @@ AAudioServiceStreamBase::~AAudioServiceStreamBase() {
     LOG_ALWAYS_FATAL_IF(!(getState() == AAUDIO_STREAM_STATE_CLOSED
                         || getState() == AAUDIO_STREAM_STATE_UNINITIALIZED
                         || getState() == AAUDIO_STREAM_STATE_DISCONNECTED),
-                        "service stream still open, state = %d", getState());
+                        "service stream %p still open, state = %d",
+                        this, getState());
 }
 
 std::string AAudioServiceStreamBase::dumpHeader() {
@@ -126,13 +127,13 @@ error:
 }
 
 aaudio_result_t AAudioServiceStreamBase::close() {
-    aaudio_result_t result = AAUDIO_OK;
     if (getState() == AAUDIO_STREAM_STATE_CLOSED) {
         return AAUDIO_OK;
     }
 
     stop();
 
+    aaudio_result_t result = AAUDIO_OK;
     sp<AAudioServiceEndpoint> endpoint = mServiceEndpointWeak.promote();
     if (endpoint == nullptr) {
         result = AAUDIO_ERROR_INVALID_STATE;

services/oboeservice/AAudioServiceStreamMMAP.cpp

@@ -49,16 +49,6 @@ AAudioServiceStreamMMAP::AAudioServiceStreamMMAP(android::AAudioService &aAudioS
         , mInService(inService) {
 }
 
-aaudio_result_t AAudioServiceStreamMMAP::close() {
-    if (getState() == AAUDIO_STREAM_STATE_CLOSED) {
-        return AAUDIO_OK;
-    }
-
-    stop();
-
-    return AAudioServiceStreamBase::close();
-}
-
 // Open stream on HAL and pass information about the shared memory buffer back to the client.
 aaudio_result_t AAudioServiceStreamMMAP::open(const aaudio::AAudioStreamRequest &request) {
 

services/oboeservice/AAudioServiceStreamMMAP.h

@@ -67,8 +67,6 @@ public:
 
     aaudio_result_t stopClient(audio_port_handle_t clientHandle) override;
 
-    aaudio_result_t close() override;
-
     const char *getTypeText() const override { return "MMAP"; }
 
 protected: