aaudio: check for callback thread outside the lock
Calling AAudioStream_requestStop() from a callback is illegal.
But it should just return and error and not hang.
The hang can occur if AAudioStream_close() is called at the same time
from another thread.
Bug: 184370309
Test: adb shell test_disconnect_race
Change-Id: I5e0752611d4d3d3dd994a10e82a339fa39e65d42
diff --git a/media/libaaudio/src/client/AudioStreamInternalCapture.cpp b/media/libaaudio/src/client/AudioStreamInternalCapture.cpp
index 5d311fc..1bbe443 100644
--- a/media/libaaudio/src/client/AudioStreamInternalCapture.cpp
+++ b/media/libaaudio/src/client/AudioStreamInternalCapture.cpp
@@ -268,7 +268,7 @@
if (callbackResult == AAUDIO_CALLBACK_RESULT_STOP) {
ALOGD("%s(): callback returned AAUDIO_CALLBACK_RESULT_STOP", __func__);
- result = systemStopFromCallback();
+ result = systemStopInternal();
break;
}
}
diff --git a/media/libaaudio/src/client/AudioStreamInternalPlay.cpp b/media/libaaudio/src/client/AudioStreamInternalPlay.cpp
index b81e5e4..3f17e6b 100644
--- a/media/libaaudio/src/client/AudioStreamInternalPlay.cpp
+++ b/media/libaaudio/src/client/AudioStreamInternalPlay.cpp
@@ -301,7 +301,7 @@
}
} else if (callbackResult == AAUDIO_CALLBACK_RESULT_STOP) {
ALOGD("%s(): callback returned AAUDIO_CALLBACK_RESULT_STOP", __func__);
- result = systemStopFromCallback();
+ result = systemStopInternal();
break;
}
}
diff --git a/media/libaaudio/src/core/AudioStream.cpp b/media/libaaudio/src/core/AudioStream.cpp
index 53523c5..e8f71be 100644
--- a/media/libaaudio/src/core/AudioStream.cpp
+++ b/media/libaaudio/src/core/AudioStream.cpp
@@ -143,13 +143,13 @@
}
aaudio_result_t AudioStream::systemStart() {
- std::lock_guard<std::mutex> lock(mStreamLock);
-
if (collidesWithCallback()) {
ALOGE("%s cannot be called from a callback!", __func__);
return AAUDIO_ERROR_INVALID_STATE;
}
+ std::lock_guard<std::mutex> lock(mStreamLock);
+
switch (getState()) {
// Is this a good time to start?
case AAUDIO_STREAM_STATE_OPEN:
@@ -187,7 +187,6 @@
}
aaudio_result_t AudioStream::systemPause() {
- std::lock_guard<std::mutex> lock(mStreamLock);
if (!isPauseSupported()) {
return AAUDIO_ERROR_UNIMPLEMENTED;
@@ -198,6 +197,7 @@
return AAUDIO_ERROR_INVALID_STATE;
}
+ std::lock_guard<std::mutex> lock(mStreamLock);
switch (getState()) {
// Proceed with pausing.
case AAUDIO_STREAM_STATE_STARTING:
@@ -242,12 +242,12 @@
return AAUDIO_ERROR_UNIMPLEMENTED;
}
- std::lock_guard<std::mutex> lock(mStreamLock);
if (collidesWithCallback()) {
ALOGE("stream cannot be flushed from a callback!");
return AAUDIO_ERROR_INVALID_STATE;
}
+ std::lock_guard<std::mutex> lock(mStreamLock);
aaudio_result_t result = AAudio_isFlushAllowed(getState());
if (result != AAUDIO_OK) {
return result;
@@ -256,7 +256,7 @@
return requestFlush_l();
}
-aaudio_result_t AudioStream::systemStopFromCallback() {
+aaudio_result_t AudioStream::systemStopInternal() {
std::lock_guard<std::mutex> lock(mStreamLock);
aaudio_result_t result = safeStop_l();
if (result == AAUDIO_OK) {
@@ -267,17 +267,12 @@
}
aaudio_result_t AudioStream::systemStopFromApp() {
- std::lock_guard<std::mutex> lock(mStreamLock);
+ // This check can and should be done outside the lock.
if (collidesWithCallback()) {
ALOGE("stream cannot be stopped by calling from a callback!");
return AAUDIO_ERROR_INVALID_STATE;
}
- aaudio_result_t result = safeStop_l();
- if (result == AAUDIO_OK) {
- // We only call this for logging in "dumpsys audio". So ignore return code.
- (void) mPlayerBase->stopWithStatus();
- }
- return result;
+ return systemStopInternal();
}
aaudio_result_t AudioStream::safeStop_l() {
@@ -316,12 +311,12 @@
}
aaudio_result_t AudioStream::safeRelease() {
- // This may get temporarily unlocked in the MMAP 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;
}
+ // This may get temporarily unlocked in the MMAP release() when joining callback threads.
+ std::lock_guard<std::mutex> lock(mStreamLock);
if (getState() == AAUDIO_STREAM_STATE_CLOSING) { // already released?
return AAUDIO_OK;
}
@@ -329,17 +324,14 @@
}
aaudio_result_t AudioStream::safeReleaseClose() {
- // This get temporarily unlocked in the MMAP 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;
}
- releaseCloseFinal_l();
- return AAUDIO_OK;
+ return safeReleaseCloseInternal();
}
-aaudio_result_t AudioStream::safeReleaseCloseFromCallback() {
+aaudio_result_t AudioStream::safeReleaseCloseInternal() {
// This get temporarily unlocked in the MMAP release() when joining callback threads.
std::lock_guard<std::mutex> lock(mStreamLock);
releaseCloseFinal_l();
diff --git a/media/libaaudio/src/core/AudioStream.h b/media/libaaudio/src/core/AudioStream.h
index 333e665..abf62f3 100644
--- a/media/libaaudio/src/core/AudioStream.h
+++ b/media/libaaudio/src/core/AudioStream.h
@@ -408,7 +408,7 @@
/**
* This is called internally when an app callback returns AAUDIO_CALLBACK_RESULT_STOP.
*/
- aaudio_result_t systemStopFromCallback();
+ aaudio_result_t systemStopInternal();
/**
* Safely RELEASE a stream after taking mStreamLock and checking
@@ -424,7 +424,7 @@
*/
aaudio_result_t safeReleaseClose();
- aaudio_result_t safeReleaseCloseFromCallback();
+ aaudio_result_t safeReleaseCloseInternal();
protected:
diff --git a/media/libaaudio/src/legacy/AudioStreamLegacy.cpp b/media/libaaudio/src/legacy/AudioStreamLegacy.cpp
index fdaa2ab..60eb73a 100644
--- a/media/libaaudio/src/legacy/AudioStreamLegacy.cpp
+++ b/media/libaaudio/src/legacy/AudioStreamLegacy.cpp
@@ -124,7 +124,7 @@
__func__, callbackResult);
}
audioBuffer->size = 0;
- systemStopFromCallback();
+ systemStopInternal();
// Disable the callback just in case the system keeps trying to call us.
mCallbackEnabled.store(false);
}
diff --git a/services/oboeservice/AAudioServiceEndpointShared.cpp b/services/oboeservice/AAudioServiceEndpointShared.cpp
index 501e8c0..0d453cf 100644
--- a/services/oboeservice/AAudioServiceEndpointShared.cpp
+++ b/services/oboeservice/AAudioServiceEndpointShared.cpp
@@ -111,7 +111,7 @@
if (!endpoint->isConnected()) {
ALOGD("%s() call safeReleaseCloseFromCallback()", __func__);
// Release and close under a lock with no check for callback collisions.
- endpoint->getStreamInternal()->safeReleaseCloseFromCallback();
+ endpoint->getStreamInternal()->safeReleaseCloseInternal();
}
return result;