aaudio: handle shared disconnect before start
Unplugging or plugging in a headset while an AAudio stream
is open causes a DISCONNECT. If a disconnect occurs between
open and start on a shared MMAP stream then it leaves an orphan
MMAP port.
This resource leak can prevent MMAP from working until the
device is rebooted.
This was caused by the mMapStream getting deleted
and preventing stopClient() from working.
The fix involves using a mutex to protect mMapStream.
Also we check to see if the endpoint was disconnected
during startClient() and stop the client port if so.
Bug: 344011310
Test: see bug for repro steps
Change-Id: I7aadf1f2dbc7886deb2cb59faf72598614ad4258
diff --git a/media/libaaudio/src/client/AudioStreamInternal.cpp b/media/libaaudio/src/client/AudioStreamInternal.cpp
index b2e93f0..fa3f5a0 100644
--- a/media/libaaudio/src/client/AudioStreamInternal.cpp
+++ b/media/libaaudio/src/client/AudioStreamInternal.cpp
@@ -620,17 +620,19 @@
audio_port_handle_t *portHandle) {
ALOGV("%s() called", __func__);
if (getServiceHandle() == AAUDIO_HANDLE_INVALID) {
+ ALOGE("%s() getServiceHandle() is invalid", __func__);
return AAUDIO_ERROR_INVALID_STATE;
}
aaudio_result_t result = mServiceInterface.startClient(mServiceStreamHandleInfo,
client, attr, portHandle);
- ALOGV("%s(%d) returning %d", __func__, *portHandle, result);
+ ALOGV("%s(), got %d, returning %d", __func__, *portHandle, result);
return result;
}
aaudio_result_t AudioStreamInternal::stopClient(audio_port_handle_t portHandle) {
ALOGV("%s(%d) called", __func__, portHandle);
if (getServiceHandle() == AAUDIO_HANDLE_INVALID) {
+ ALOGE("%s(%d) getServiceHandle() is invalid", __func__, portHandle);
return AAUDIO_ERROR_INVALID_STATE;
}
aaudio_result_t result = mServiceInterface.stopClient(mServiceStreamHandleInfo, portHandle);
diff --git a/services/oboeservice/AAudioServiceEndpointMMAP.cpp b/services/oboeservice/AAudioServiceEndpointMMAP.cpp
index 5b4fca9..d663f37 100644
--- a/services/oboeservice/AAudioServiceEndpointMMAP.cpp
+++ b/services/oboeservice/AAudioServiceEndpointMMAP.cpp
@@ -205,6 +205,8 @@
"sample_rate=%u, channel_mask=%#x, device=%d",
__func__, config->format, config->sample_rate,
config->channel_mask, deviceId);
+
+ const std::lock_guard<std::mutex> lock(mMmapStreamLock);
const status_t status = MmapStreamInterface::openMmapStream(streamDirection,
&attributes,
config,
@@ -246,7 +248,7 @@
__func__, config->format, getDeviceId(), getSessionId());
// Create MMAP/NOIRQ buffer.
- result = createMmapBuffer();
+ result = createMmapBuffer_l();
if (result != AAUDIO_OK) {
goto error;
}
@@ -283,7 +285,7 @@
return result;
error:
- close();
+ close_l();
// restore original requests
setDeviceId(mRequestedDeviceId);
setSessionId(requestedSessionId);
@@ -291,13 +293,28 @@
}
void AAudioServiceEndpointMMAP::close() {
- if (mMmapStream != nullptr) {
- // Needs to be explicitly cleared or CTS will fail but it is not clear why.
- mMmapStream.clear();
+ bool closedIt = false;
+ {
+ const std::lock_guard<std::mutex> lock(mMmapStreamLock);
+ closedIt = close_l();
+ }
+ if (closedIt) {
+ // TODO Why is this needed?
AudioClock::sleepForNanos(100 * AAUDIO_NANOS_PER_MILLISECOND);
}
}
+bool AAudioServiceEndpointMMAP::close_l() { // requires mMmapStreamLock
+ bool closedIt = false;
+ if (mMmapStream != nullptr) {
+ // Needs to be explicitly cleared or CTS will fail but it is not clear why.
+ ALOGD("%s() clear mMmapStream", __func__);
+ mMmapStream.clear();
+ closedIt = true;
+ }
+ return closedIt;
+}
+
aaudio_result_t AAudioServiceEndpointMMAP::startStream(sp<AAudioServiceStreamBase> stream,
audio_port_handle_t *clientHandle __unused) {
// Start the client on behalf of the AAudio service.
@@ -318,7 +335,7 @@
}
aaudio_result_t AAudioServiceEndpointMMAP::stopStream(sp<AAudioServiceStreamBase> /*stream*/,
- audio_port_handle_t /*clientHandle*/) {
+ audio_port_handle_t clientHandle) {
mFramesTransferred.reset32();
// Round 64-bit counter up to a multiple of the buffer capacity.
@@ -328,36 +345,68 @@
mFramesTransferred.roundUp64(getBufferCapacity());
// Use the port handle that was provided by openMmapStream().
- ALOGV("%s() mPortHandle = %d", __func__, mPortHandle);
- return stopClient(mPortHandle);
+ aaudio_result_t result = stopClient(mPortHandle);
+ ALOGD("%s(%d): called stopClient(%d=mPortHandle), returning %d", __func__,
+ (int)clientHandle, mPortHandle, result);
+ return result;
}
aaudio_result_t AAudioServiceEndpointMMAP::startClient(const android::AudioClient& client,
const audio_attributes_t *attr,
- audio_port_handle_t *clientHandle) {
- return mMmapStream == nullptr
- ? AAUDIO_ERROR_NULL
- : AAudioConvert_androidToAAudioResult(mMmapStream->start(client, attr, clientHandle));
+ audio_port_handle_t *portHandlePtr) {
+ const std::lock_guard<std::mutex> lock(mMmapStreamLock);
+ if (mMmapStream == nullptr) {
+ ALOGW("%s(): called after mMmapStream set to NULL", __func__);
+ return AAUDIO_ERROR_NULL;
+ } else if (!isConnected()) {
+ ALOGD("%s(): MMAP stream was disconnected", __func__);
+ return AAUDIO_ERROR_DISCONNECTED;
+ } else {
+ aaudio_result_t result = AAudioConvert_androidToAAudioResult(
+ mMmapStream->start(client, attr, portHandlePtr));
+ if (!isConnected() && (portHandlePtr != nullptr)) {
+ ALOGD("%s(): MMAP stream DISCONNECTED after starting port %d, will stop it",
+ __func__, *portHandlePtr);
+ mMmapStream->stop(*portHandlePtr);
+ *portHandlePtr = AUDIO_PORT_HANDLE_NONE;
+ result = AAUDIO_ERROR_DISCONNECTED;
+ }
+ ALOGD("%s(): returning port %d, result %d", __func__,
+ (portHandlePtr == nullptr) ? -1 : *portHandlePtr, result);
+ return result;
+ }
}
-aaudio_result_t AAudioServiceEndpointMMAP::stopClient(audio_port_handle_t clientHandle) {
- return mMmapStream == nullptr
- ? AAUDIO_ERROR_NULL
- : AAudioConvert_androidToAAudioResult(mMmapStream->stop(clientHandle));
+aaudio_result_t AAudioServiceEndpointMMAP::stopClient(audio_port_handle_t portHandle) {
+ const std::lock_guard<std::mutex> lock(mMmapStreamLock);
+ if (mMmapStream == nullptr) {
+ ALOGE("%s(%d): called after mMmapStream set to NULL", __func__, (int)portHandle);
+ return AAUDIO_ERROR_NULL;
+ } else {
+ aaudio_result_t result = AAudioConvert_androidToAAudioResult(
+ mMmapStream->stop(portHandle));
+ ALOGD("%s(%d): returning %d", __func__, (int)portHandle, result);
+ return result;
+ }
}
aaudio_result_t AAudioServiceEndpointMMAP::standby() {
- return mMmapStream == nullptr
- ? AAUDIO_ERROR_NULL
- : AAudioConvert_androidToAAudioResult(mMmapStream->standby());
+ const std::lock_guard<std::mutex> lock(mMmapStreamLock);
+ if (mMmapStream == nullptr) {
+ ALOGW("%s(): called after mMmapStream set to NULL", __func__);
+ return AAUDIO_ERROR_NULL;
+ } else {
+ return AAudioConvert_androidToAAudioResult(mMmapStream->standby());
+ }
}
aaudio_result_t AAudioServiceEndpointMMAP::exitStandby(AudioEndpointParcelable* parcelable) {
+ const std::lock_guard<std::mutex> lock(mMmapStreamLock);
if (mMmapStream == nullptr) {
return AAUDIO_ERROR_NULL;
}
mAudioDataWrapper->reset();
- const aaudio_result_t result = createMmapBuffer();
+ const aaudio_result_t result = createMmapBuffer_l();
if (result == AAUDIO_OK) {
getDownDataDescription(parcelable);
}
@@ -367,10 +416,12 @@
// Get free-running DSP or DMA hardware position from the HAL.
aaudio_result_t AAudioServiceEndpointMMAP::getFreeRunningPosition(int64_t *positionFrames,
int64_t *timeNanos) {
- struct audio_mmap_position position;
+ const std::lock_guard<std::mutex> lock(mMmapStreamLock);
if (mMmapStream == nullptr) {
+ ALOGW("%s(): called after mMmapStream set to NULL", __func__);
return AAUDIO_ERROR_NULL;
}
+ struct audio_mmap_position position;
const status_t status = mMmapStream->getMmapPosition(&position);
ALOGV("%s() status= %d, pos = %d, nanos = %lld\n",
__func__, status, position.position_frames, (long long) position.time_nanoseconds);
@@ -475,9 +526,14 @@
aaudio_result_t AAudioServiceEndpointMMAP::getExternalPosition(uint64_t *positionFrames,
int64_t *timeNanos)
{
+ const std::lock_guard<std::mutex> lock(mMmapStreamLock);
if (mHalExternalPositionStatus != AAUDIO_OK) {
return mHalExternalPositionStatus;
}
+ if (mMmapStream == nullptr) {
+ ALOGW("%s(): called after mMmapStream set to NULL", __func__);
+ return AAUDIO_ERROR_NULL;
+ }
uint64_t tempPositionFrames;
int64_t tempTimeNanos;
const status_t status = mMmapStream->getExternalPosition(&tempPositionFrames, &tempTimeNanos);
@@ -552,13 +608,20 @@
return mHalExternalPositionStatus;
}
-aaudio_result_t AAudioServiceEndpointMMAP::createMmapBuffer()
+// mMmapStreamLock should be held when calling this function.
+aaudio_result_t AAudioServiceEndpointMMAP::createMmapBuffer_l()
{
memset(&mMmapBufferinfo, 0, sizeof(struct audio_mmap_buffer_info));
int32_t minSizeFrames = getBufferCapacity();
if (minSizeFrames <= 0) { // zero will get rejected
minSizeFrames = AAUDIO_BUFFER_CAPACITY_MIN;
}
+
+ if (mMmapStream == nullptr) {
+ ALOGW("%s(): called after mMmapStream set to NULL", __func__);
+ return AAUDIO_ERROR_NULL;
+ }
+
const status_t status = mMmapStream->createMmapBuffer(minSizeFrames, &mMmapBufferinfo);
const bool isBufferShareable = mMmapBufferinfo.flags & AUDIO_MMAP_APPLICATION_SHAREABLE;
if (status != OK) {
@@ -598,6 +661,7 @@
// Call to HAL to make sure the transport FD was able to be closed by binder.
// This is a tricky workaround for a problem in Binder.
// TODO:[b/192048842] When that problem is fixed we may be able to remove or change this code.
+ ALOGD("%s() - call getMmapPosition() as a hack to clear FD stuck in Binder", __func__);
struct audio_mmap_position position;
mMmapStream->getMmapPosition(&position);
@@ -613,11 +677,14 @@
}
void AAudioServiceEndpointMMAP::reportData() {
+ const std::lock_guard<std::mutex> lock(mMmapStreamLock);
+
if (mMmapStream == nullptr) {
// This must not happen
ALOGE("%s() invalid state, mmap stream is not initialized", __func__);
return;
}
+
auto fifo = mAudioDataWrapper->getFifoBuffer();
if (fifo == nullptr) {
ALOGE("%s() fifo buffer is not initialized, cannot report data", __func__);
diff --git a/services/oboeservice/AAudioServiceEndpointMMAP.h b/services/oboeservice/AAudioServiceEndpointMMAP.h
index eaa578c..962d390 100644
--- a/services/oboeservice/AAudioServiceEndpointMMAP.h
+++ b/services/oboeservice/AAudioServiceEndpointMMAP.h
@@ -50,7 +50,7 @@
aaudio_result_t open(const aaudio::AAudioStreamRequest &request) override;
- void close() override;
+ void close() override EXCLUDES(mMmapStreamLock);
aaudio_result_t startStream(android::sp<AAudioServiceStreamBase> stream,
audio_port_handle_t *clientHandle) override;
@@ -60,15 +60,19 @@
aaudio_result_t startClient(const android::AudioClient& client,
const audio_attributes_t *attr,
- audio_port_handle_t *clientHandle) override;
+ audio_port_handle_t *clientHandle) override
+ EXCLUDES(mMmapStreamLock);
- aaudio_result_t stopClient(audio_port_handle_t clientHandle) override;
+ aaudio_result_t stopClient(audio_port_handle_t clientHandle) override
+ EXCLUDES(mMmapStreamLock);
- aaudio_result_t standby() override;
+ aaudio_result_t standby() override EXCLUDES(mMmapStreamLock);
- aaudio_result_t exitStandby(AudioEndpointParcelable* parcelable) override;
+ aaudio_result_t exitStandby(AudioEndpointParcelable* parcelable) override
+ EXCLUDES(mMmapStreamLock);
- aaudio_result_t getFreeRunningPosition(int64_t *positionFrames, int64_t *timeNanos) override;
+ aaudio_result_t getFreeRunningPosition(int64_t *positionFrames, int64_t *timeNanos) override
+ EXCLUDES(mMmapStreamLock);
aaudio_result_t getTimestamp(int64_t *positionFrames, int64_t *timeNanos) override;
@@ -88,22 +92,31 @@
return mHardwareTimeOffsetNanos;
}
- aaudio_result_t getExternalPosition(uint64_t *positionFrames, int64_t *timeNanos);
+ aaudio_result_t getExternalPosition(uint64_t *positionFrames, int64_t *timeNanos)
+ EXCLUDES(mMmapStreamLock);
- int64_t nextDataReportTime();
+ int64_t nextDataReportTime() EXCLUDES(mMmapStreamLock);
- void reportData();
+ void reportData() EXCLUDES(mMmapStreamLock);
private:
- aaudio_result_t openWithConfig(audio_config_base_t* config);
+ /**
+ *
+ * @return true if mMapStream was cleared
+ */
+ bool close_l() REQUIRES(mMmapStreamLock);
- aaudio_result_t createMmapBuffer();
+ aaudio_result_t openWithConfig(audio_config_base_t* config) EXCLUDES(mMmapStreamLock);
+
+ aaudio_result_t createMmapBuffer_l() REQUIRES(mMmapStreamLock);
MonotonicCounter mFramesTransferred;
// Interface to the AudioFlinger MMAP support.
- android::sp<android::MmapStreamInterface> mMmapStream;
+ mutable std::mutex mMmapStreamLock;
+ android::sp<android::MmapStreamInterface> mMmapStream GUARDED_BY(mMmapStreamLock);
+
struct audio_mmap_buffer_info mMmapBufferinfo;
// There is only one port associated with an MMAP endpoint.
diff --git a/services/oboeservice/AAudioServiceStreamBase.cpp b/services/oboeservice/AAudioServiceStreamBase.cpp
index 20a5ea8..78cf706 100644
--- a/services/oboeservice/AAudioServiceStreamBase.cpp
+++ b/services/oboeservice/AAudioServiceStreamBase.cpp
@@ -227,6 +227,10 @@
ALOGE("%s() has no endpoint", __func__);
return AAUDIO_ERROR_INVALID_STATE;
}
+ if (!endpoint->isConnected()) {
+ ALOGE("%s() endpoint was already disconnected", __func__);
+ return AAUDIO_ERROR_DISCONNECTED;
+ }
return endpoint->startStream(this, &mClientHandle);
}
@@ -334,6 +338,7 @@
aaudio_result_t AAudioServiceStreamBase::stop_l() {
aaudio_result_t result = AAUDIO_OK;
if (!isRunning()) {
+ ALOGW("%s() stream not running, returning early", __func__);
return result;
}
const int64_t beginNs = AudioClock::getNanoseconds();
diff --git a/services/oboeservice/AAudioServiceStreamMMAP.cpp b/services/oboeservice/AAudioServiceStreamMMAP.cpp
index fc53949..5203e50 100644
--- a/services/oboeservice/AAudioServiceStreamMMAP.cpp
+++ b/services/oboeservice/AAudioServiceStreamMMAP.cpp
@@ -150,9 +150,9 @@
aaudio_result_t AAudioServiceStreamMMAP::startClient(const android::AudioClient& client,
const audio_attributes_t *attr,
- audio_port_handle_t *clientHandle) {
+ audio_port_handle_t *portHandlePtr) {
if (com::android::media::aaudio::start_stop_client_from_command_thread()) {
- return sendStartClientCommand(client, attr, clientHandle);
+ return sendStartClientCommand(client, attr, portHandlePtr);
} else {
sp<AAudioServiceEndpoint> endpoint = mServiceEndpointWeak.promote();
if (endpoint == nullptr) {
@@ -160,7 +160,9 @@
return AAUDIO_ERROR_INVALID_STATE;
}
// Start the client on behalf of the application. Generate a new porthandle.
- aaudio_result_t result = endpoint->startClient(client, attr, clientHandle);
+ aaudio_result_t result = endpoint->startClient(client, attr, portHandlePtr);
+ ALOGD("%s() flag off, got port %d", __func__,
+ ((portHandlePtr == nullptr) ? -1 : *portHandlePtr));
return result;
}
}