oboe: prevent race condition in OpenSL close()

Do not unlock when stopping the stream. Call requestStop_l() or requestPause_l(). Fixes #1062
google · Nov 19, 2020 · 52e2163 · 52e2163
1 parent db0f680
commit 52e2163
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 13 deletions.
diff --git a/src/opensles/AudioInputStreamOpenSLES.cpp b/src/opensles/AudioInputStreamOpenSLES.cpp
@@ -216,19 +216,16 @@ Result AudioInputStreamOpenSLES::open() {
 
 Result AudioInputStreamOpenSLES::close() {
     LOGD("AudioInputStreamOpenSLES::%s()", __func__);
-    mLock.lock();
+    std::lock_guard<std::mutex> lock(mLock);
     Result result = Result::OK;
     if (getState() == StreamState::Closed){
         result = Result::ErrorClosed;
     } else {
-        mLock.unlock(); // avoid recursive lock
-        requestStop();
-        mLock.lock();
+        requestStop_l();
         // invalidate any interfaces
         mRecordInterface = nullptr;
         result = AudioStreamOpenSLES::close_l();
     }
-    mLock.unlock(); // avoid recursive lock
     return result;
 }
 
@@ -296,8 +293,12 @@ Result AudioInputStreamOpenSLES::requestFlush() {
 
 Result AudioInputStreamOpenSLES::requestStop() {
     LOGD("AudioInputStreamOpenSLES(): %s() called", __func__);
-
     std::lock_guard<std::mutex> lock(mLock);
+    return requestStop_l();
+}
+
+// Call under mLock
+Result AudioInputStreamOpenSLES::requestStop_l() {
     StreamState initialState = getState();
     switch (initialState) {
         case StreamState::Stopping:

diff --git a/src/opensles/AudioInputStreamOpenSLES.h b/src/opensles/AudioInputStreamOpenSLES.h
@@ -46,6 +46,7 @@ class AudioInputStreamOpenSLES : public AudioStreamOpenSLES {
     Result requestStop() override;
 
 protected:
+    Result requestStop_l();
 
     Result updateServiceFrameCounter() override;
 

diff --git a/src/opensles/AudioOutputStreamOpenSLES.cpp b/src/opensles/AudioOutputStreamOpenSLES.cpp
@@ -243,19 +243,17 @@ Result AudioOutputStreamOpenSLES::onAfterDestroy() {
 }
 
 Result AudioOutputStreamOpenSLES::close() {
-    mLock.lock();
+    LOGD("AudioOutputStreamOpenSLES::%s()", __func__);
+    std::lock_guard<std::mutex> lock(mLock);
     Result result = Result::OK;
     if (getState() == StreamState::Closed){
         result = Result::ErrorClosed;
     } else {
-        mLock.unlock(); // avoid recursive lock
-        requestPause();
-        mLock.lock();
+        requestPause_l();
         // invalidate any interfaces
         mPlayInterface = nullptr;
         result = AudioStreamOpenSLES::close_l();
     }
-    mLock.unlock(); // avoid recursive lock
     return result;
 }
 
@@ -317,8 +315,12 @@ Result AudioOutputStreamOpenSLES::requestStart() {
 
 Result AudioOutputStreamOpenSLES::requestPause() {
     LOGD("AudioOutputStreamOpenSLES(): %s() called", __func__);
-
     std::lock_guard<std::mutex> lock(mLock);
+    return requestPause_l();
+}
+
+// Call under mLock
+Result AudioOutputStreamOpenSLES::requestPause_l() {
     StreamState initialState = getState();
     switch (initialState) {
         case StreamState::Pausing:
@@ -374,8 +376,8 @@ Result AudioOutputStreamOpenSLES::requestFlush_l() {
 
 Result AudioOutputStreamOpenSLES::requestStop() {
     LOGD("AudioOutputStreamOpenSLES(): %s() called", __func__);
-
     std::lock_guard<std::mutex> lock(mLock);
+
     StreamState initialState = getState();
     switch (initialState) {
         case StreamState::Stopping:

diff --git a/src/opensles/AudioOutputStreamOpenSLES.h b/src/opensles/AudioOutputStreamOpenSLES.h
@@ -45,6 +45,7 @@ class AudioOutputStreamOpenSLES : public AudioStreamOpenSLES {
     Result requestStop() override;
 
 protected:
+    Result requestPause_l();
 
     void setFramesRead(int64_t framesRead);