commit 98d6d9227f6599a5aa7b023217c02a01cc4536cb
author Phil Burk <philburk@google.com> Thu Jul 06 11:52:45 2017 -0700
committer Phil Burk <philburk@google.com> Thu Jul 06 16:41:07 2017 -0700
tree 847f63fabe692cdcc73dcdda8c681cd9f4087e97
parent 014a56b57af9e4c744d1ac6f5a95eb1ac7e3f445

aaudio: stop calling virtual methods from destructor

Was calling close(), which is abstract and virtual.
This may have been related to some audioserver crashes.
Also cleaned up some strong pointer handling.

Bug: 63390734
Bug: 63353455
Test: run CTS nativemedia/aaudio many times
Change-Id: Ib95aed60a64771b64395c67f0921c67146f9d10f

services/oboeservice/AAudioServiceStreamBase.cpp

diff --git a/services/oboeservice/AAudioServiceStreamBase.cpp b/services/oboeservice/AAudioServiceStreamBase.cpp
index c88038f..2e20287 100644
--- a/services/oboeservice/AAudioServiceStreamBase.cpp
+++ b/services/oboeservice/AAudioServiceStreamBase.cpp
@@ -41,8 +41,11 @@
 }
 
 AAudioServiceStreamBase::~AAudioServiceStreamBase() {
-    close();
-    ALOGD("AAudioServiceStreamBase::~AAudioServiceStreamBase() destroyed %p", this);
+    ALOGD("AAudioServiceStreamBase::~AAudioServiceStreamBase() destroying %p", this);
+    // If the stream is deleted without closing then audio resources will leak.
+    // Not being closed here would indicate an internal error. So we want to find this ASAP.
+    LOG_ALWAYS_FATAL_IF(mState != AAUDIO_STREAM_STATE_CLOSED,
+                        "service stream not closed, state = %d", mState);
 }
 
 std::string AAudioServiceStreamBase::dump() const {
@@ -71,11 +74,13 @@
 }
 
 aaudio_result_t AAudioServiceStreamBase::close() {
-    stop();
-    std::lock_guard<std::mutex> lock(mLockUpMessageQueue);
-    delete mUpMessageQueue;
-    mUpMessageQueue = nullptr;
-
+    if (mState != AAUDIO_STREAM_STATE_CLOSED) {
+        stopTimestampThread();
+        std::lock_guard<std::mutex> lock(mLockUpMessageQueue);
+        delete mUpMessageQueue;
+        mUpMessageQueue = nullptr;
+        mState = AAUDIO_STREAM_STATE_CLOSED;
+    }
     return AAUDIO_OK;
 }
 
@@ -106,9 +111,8 @@
     aaudio_result_t result = AAUDIO_OK;
     if (isRunning()) {
         // TODO wait for data to be played out
-        sendCurrentTimestamp();
-        mThreadEnabled.store(false);
-        result = mAAudioThread.stop();
+        sendCurrentTimestamp(); // warning - this calls a virtual function
+        result = stopTimestampThread();
         if (result != AAUDIO_OK) {
             disconnect();
             return result;
@@ -119,6 +123,15 @@
     return result;
 }
 
+aaudio_result_t AAudioServiceStreamBase::stopTimestampThread() {
+    aaudio_result_t result = AAUDIO_OK;
+    // clear flag that tells thread to loop
+    if (mThreadEnabled.exchange(false)) {
+        result = mAAudioThread.stop();
+    }
+    return result;
+}
+
 aaudio_result_t AAudioServiceStreamBase::flush() {
     sendServiceEvent(AAUDIO_SERVICE_EVENT_FLUSHED);
     mState = AAUDIO_STREAM_STATE_FLUSHED;
@@ -141,6 +154,7 @@
             nextTime = timestampScheduler.nextAbsoluteTime();
         } else  {
             // Sleep until it is time to send the next timestamp.
+            // TODO Wait for a signal with a timeout so that we can stop more quickly.
             AudioClock::sleepUntilNanoTime(nextTime);
         }
     }