aaudio: fix hang in client when audioserver dies
Fix timeout detection so that callback thread can die.
Prevent AAudioBinderClient singleton from getting deleted,
which caused a subsequent lock on a dead object to hang.
Bug: 64988439
Test: "write_sine -m2 -pl" and "adb shell killall audioserver"
Change-Id: I044bce385b66e69007d1997f051c9d6c042b7871
diff --git a/media/libaaudio/src/binding/AAudioBinderClient.cpp b/media/libaaudio/src/binding/AAudioBinderClient.cpp
index a268e49..07ee2de 100644
--- a/media/libaaudio/src/binding/AAudioBinderClient.cpp
+++ b/media/libaaudio/src/binding/AAudioBinderClient.cpp
@@ -45,6 +45,7 @@
using android::IInterface;
using android::IAAudioService;
using android::Mutex;
+using android::ProcessState;
using android::sp;
using android::wp;
@@ -52,15 +53,19 @@
ANDROID_SINGLETON_STATIC_INSTANCE(AAudioBinderClient);
+// If we don't keep a strong pointer here then this singleton can get deleted!
+android::sp<AAudioBinderClient> gKeepBinderClient;
+
AAudioBinderClient::AAudioBinderClient()
: AAudioServiceInterface()
, Singleton<AAudioBinderClient>() {
-
+ gKeepBinderClient = this; // so this singleton won't get deleted
mAAudioClient = new AAudioClient(this);
- ALOGV("AAudioBinderClient() created mAAudioClient = %p", mAAudioClient.get());
+ ALOGV("AAudioBinderClient() this = %p, created mAAudioClient = %p", this, mAAudioClient.get());
}
AAudioBinderClient::~AAudioBinderClient() {
+ ALOGV("AAudioBinderClient()::~AAudioBinderClient() destroying %p", this);
Mutex::Autolock _l(mServiceLock);
if (mAAudioService != 0) {
IInterface::asBinder(mAAudioService)->unlinkToDeath(mAAudioClient);
@@ -75,19 +80,19 @@
bool needToRegister = false;
{
Mutex::Autolock _l(mServiceLock);
- if (mAAudioService == 0) {
+ if (mAAudioService.get() == nullptr) {
sp<IBinder> binder;
sp<IServiceManager> sm = defaultServiceManager();
// Try several times to get the service.
int retries = 4;
do {
binder = sm->getService(String16(AAUDIO_SERVICE_NAME)); // This will wait a while.
- if (binder != 0) {
+ if (binder.get() != nullptr) {
break;
}
} while (retries-- > 0);
- if (binder != 0) {
+ if (binder.get() != nullptr) {
// Ask for notification if the service dies.
status_t status = binder->linkToDeath(mAAudioClient);
// TODO review what we should do if this fails
@@ -98,7 +103,7 @@
mAAudioService = interface_cast<IAAudioService>(binder);
needToRegister = true;
// Make sure callbacks can be received by mAAudioClient
- android::ProcessState::self()->startThreadPool();
+ ProcessState::self()->startThreadPool();
} else {
ALOGE("AAudioBinderClient could not connect to %s", AAUDIO_SERVICE_NAME);
}
@@ -106,7 +111,7 @@
aaudioService = mAAudioService;
}
// Do this outside the mutex lock.
- if (needToRegister && aaudioService != 0) { // new client?
+ if (needToRegister && aaudioService.get() != nullptr) { // new client?
aaudioService->registerClient(mAAudioClient);
}
return aaudioService;
@@ -117,7 +122,6 @@
mAAudioService.clear(); // force a reconnect
}
-
/**
* @param request info needed to create the stream
* @param configuration contains information about the created stream
@@ -128,14 +132,12 @@
aaudio_handle_t stream;
for (int i = 0; i < 2; i++) {
const sp<IAAudioService> &service = getAAudioService();
- if (service == 0) {
- return AAUDIO_ERROR_NO_SERVICE;
- }
+ if (service.get() == nullptr) return AAUDIO_ERROR_NO_SERVICE;
stream = service->openStream(request, configurationOutput);
if (stream == AAUDIO_ERROR_NO_SERVICE) {
- ALOGE("AAudioBinderClient: lost connection to AAudioService.");
+ ALOGE("AAudioBinderClient::openStream lost connection to AAudioService.");
dropAAudioService(); // force a reconnect
} else {
break;
@@ -145,8 +147,8 @@
}
aaudio_result_t AAudioBinderClient::closeStream(aaudio_handle_t streamHandle) {
- const sp<IAAudioService> &service = getAAudioService();
- if (service == 0) return AAUDIO_ERROR_NO_SERVICE;
+ const sp<IAAudioService> service = getAAudioService();
+ if (service.get() == nullptr) return AAUDIO_ERROR_NO_SERVICE;
return service->closeStream(streamHandle);
}
@@ -155,32 +157,32 @@
*/
aaudio_result_t AAudioBinderClient::getStreamDescription(aaudio_handle_t streamHandle,
AudioEndpointParcelable &parcelable) {
- const sp<IAAudioService> &service = getAAudioService();
- if (service == 0) return AAUDIO_ERROR_NO_SERVICE;
+ const sp<IAAudioService> service = getAAudioService();
+ if (service.get() == nullptr) return AAUDIO_ERROR_NO_SERVICE;
return service->getStreamDescription(streamHandle, parcelable);
}
aaudio_result_t AAudioBinderClient::startStream(aaudio_handle_t streamHandle) {
- const sp<IAAudioService> &service = getAAudioService();
- if (service == 0) return AAUDIO_ERROR_NO_SERVICE;
+ const sp<IAAudioService> service = getAAudioService();
+ if (service.get() == nullptr) return AAUDIO_ERROR_NO_SERVICE;
return service->startStream(streamHandle);
}
aaudio_result_t AAudioBinderClient::pauseStream(aaudio_handle_t streamHandle) {
- const sp<IAAudioService> &service = getAAudioService();
- if (service == 0) return AAUDIO_ERROR_NO_SERVICE;
+ const sp<IAAudioService> service = getAAudioService();
+ if (service.get() == nullptr) return AAUDIO_ERROR_NO_SERVICE;
return service->pauseStream(streamHandle);
}
aaudio_result_t AAudioBinderClient::stopStream(aaudio_handle_t streamHandle) {
- const sp<IAAudioService> &service = getAAudioService();
- if (service == 0) return AAUDIO_ERROR_NO_SERVICE;
+ const sp<IAAudioService> service = getAAudioService();
+ if (service.get() == nullptr) return AAUDIO_ERROR_NO_SERVICE;
return service->stopStream(streamHandle);
}
aaudio_result_t AAudioBinderClient::flushStream(aaudio_handle_t streamHandle) {
- const sp<IAAudioService> &service = getAAudioService();
- if (service == 0) return AAUDIO_ERROR_NO_SERVICE;
+ const sp<IAAudioService> service = getAAudioService();
+ if (service.get() == nullptr) return AAUDIO_ERROR_NO_SERVICE;
return service->flushStream(streamHandle);
}
@@ -190,8 +192,8 @@
aaudio_result_t AAudioBinderClient::registerAudioThread(aaudio_handle_t streamHandle,
pid_t clientThreadId,
int64_t periodNanoseconds) {
- const sp<IAAudioService> &service = getAAudioService();
- if (service == 0) return AAUDIO_ERROR_NO_SERVICE;
+ const sp<IAAudioService> service = getAAudioService();
+ if (service.get() == nullptr) return AAUDIO_ERROR_NO_SERVICE;
return service->registerAudioThread(streamHandle,
clientThreadId,
periodNanoseconds);
@@ -199,8 +201,8 @@
aaudio_result_t AAudioBinderClient::unregisterAudioThread(aaudio_handle_t streamHandle,
pid_t clientThreadId) {
- const sp<IAAudioService> &service = getAAudioService();
- if (service == 0) return AAUDIO_ERROR_NO_SERVICE;
+ const sp<IAAudioService> service = getAAudioService();
+ if (service.get() == nullptr) return AAUDIO_ERROR_NO_SERVICE;
return service->unregisterAudioThread(streamHandle,
clientThreadId);
}
diff --git a/media/libaaudio/src/binding/AAudioBinderClient.h b/media/libaaudio/src/binding/AAudioBinderClient.h
index 89ae85c..f9da8b4 100644
--- a/media/libaaudio/src/binding/AAudioBinderClient.h
+++ b/media/libaaudio/src/binding/AAudioBinderClient.h
@@ -118,13 +118,13 @@
{
public:
AAudioClient(android::wp<AAudioBinderClient> aaudioBinderClient)
- : mBinderClient(aaudioBinderClient) {
+ : mBinderClient(aaudioBinderClient) {
}
// implement DeathRecipient
virtual void binderDied(const android::wp<android::IBinder>& who __unused) {
android::sp<AAudioBinderClient> client = mBinderClient.promote();
- if (client != 0) {
+ if (client.get() != nullptr) {
client->dropAAudioService();
}
ALOGW("AAudio service binderDied()!");
@@ -133,7 +133,7 @@
// implement BnAAudioClient
void onStreamChange(aaudio_handle_t handle, int32_t opcode, int32_t value) {
android::sp<AAudioBinderClient> client = mBinderClient.promote();
- if (client != 0) {
+ if (client.get() != nullptr) {
client->onStreamChange(handle, opcode, value);
}
}
@@ -141,10 +141,11 @@
android::wp<AAudioBinderClient> mBinderClient;
};
+private:
- android::Mutex mServiceLock;
+ android::Mutex mServiceLock;
android::sp<android::IAAudioService> mAAudioService;
- android::sp<AAudioClient> mAAudioClient;
+ android::sp<AAudioClient> mAAudioClient;
};
diff --git a/media/libaaudio/src/client/AudioStreamInternal.cpp b/media/libaaudio/src/client/AudioStreamInternal.cpp
index 036d931..7f2e495 100644
--- a/media/libaaudio/src/client/AudioStreamInternal.cpp
+++ b/media/libaaudio/src/client/AudioStreamInternal.cpp
@@ -554,12 +554,19 @@
wakeTimeNanos += mWakeupDelayNanos;
}
+ currentTimeNanos = AudioClock::getNanoseconds();
+ int64_t earliestWakeTime = currentTimeNanos + mMinimumSleepNanos;
+ // Guarantee a minimum sleep time.
+ if (wakeTimeNanos < earliestWakeTime) {
+ wakeTimeNanos = earliestWakeTime;
+ }
+
if (wakeTimeNanos > deadlineNanos) {
// If we time out, just return the framesWritten so far.
// TODO remove after we fix the deadline bug
ALOGW("AudioStreamInternal::processData(): entered at %lld nanos, currently %lld",
(long long) entryTimeNanos, (long long) currentTimeNanos);
- ALOGW("AudioStreamInternal::processData(): timed out after %lld nanos",
+ ALOGW("AudioStreamInternal::processData(): TIMEOUT after %lld nanos",
(long long) timeoutNanoseconds);
ALOGW("AudioStreamInternal::processData(): wakeTime = %lld, deadline = %lld nanos",
(long long) wakeTimeNanos, (long long) deadlineNanos);
@@ -570,13 +577,6 @@
break;
}
- currentTimeNanos = AudioClock::getNanoseconds();
- int64_t earliestWakeTime = currentTimeNanos + mMinimumSleepNanos;
- // Guarantee a minimum sleep time.
- if (wakeTimeNanos < earliestWakeTime) {
- wakeTimeNanos = earliestWakeTime;
- }
-
if (ATRACE_ENABLED()) {
int32_t fullFrames = mAudioEndpoint.getFullFramesAvailable();
ATRACE_INT(fifoName, fullFrames);