Snap for 7513903 from 390007d9905d45b3f4a941b26127c13f690a9e18 to sc-v2-release
Change-Id: I60074b8cd3fdd75cd560e4d7f66606b085e7825b
diff --git a/transport/HidlLazyUtils.cpp b/transport/HidlLazyUtils.cpp
index be7470f..c5f8c74 100644
--- a/transport/HidlLazyUtils.cpp
+++ b/transport/HidlLazyUtils.cpp
@@ -36,9 +36,9 @@
bool addRegisteredService(const sp<IBase>& service, const std::string& name);
- bool tryUnregister();
+ bool tryUnregisterLocked();
- void reRegister();
+ void reRegisterLocked();
void setActiveServicesCallback(const std::function<bool(bool)>& activeServicesCallback);
@@ -58,18 +58,23 @@
* Looks up service that is guaranteed to be registered (service from
* onClients).
*/
- Service& assertRegisteredService(const sp<IBase>& service);
+ Service& assertRegisteredServiceLocked(const sp<IBase>& service);
/**
* Registers or re-registers services. Returns whether successful.
*/
- bool registerService(const sp<IBase>& service, const std::string& name);
+ bool registerServiceLocked(const sp<IBase>& service, const std::string& name);
/**
* Unregisters all services that we can. If we can't unregister all, re-register other
* services.
*/
- void tryShutdown();
+ void tryShutdownLocked();
+
+ /**
+ * For below.
+ */
+ std::mutex mMutex;
/**
* Number of services that have been registered.
@@ -103,7 +108,8 @@
bool ClientCounterCallback::addRegisteredService(const sp<IBase>& service,
const std::string& name) {
- bool success = registerService(service, name);
+ std::lock_guard<std::mutex> lock(mMutex);
+ bool success = registerServiceLocked(service, name);
if (success) {
mRegisteredServices.push_back({service, name});
@@ -112,7 +118,7 @@
return success;
}
-ClientCounterCallback::Service& ClientCounterCallback::assertRegisteredService(
+ClientCounterCallback::Service& ClientCounterCallback::assertRegisteredServiceLocked(
const sp<IBase>& service) {
for (Service& registered : mRegisteredServices) {
if (registered.service != service) continue;
@@ -123,7 +129,8 @@
__builtin_unreachable();
}
-bool ClientCounterCallback::registerService(const sp<IBase>& service, const std::string& name) {
+bool ClientCounterCallback::registerServiceLocked(const sp<IBase>& service,
+ const std::string& name) {
auto manager = hardware::defaultServiceManager1_2();
const std::string descriptor = getDescriptor(service.get());
@@ -145,13 +152,10 @@
return true;
}
-/**
- * onClients is oneway, so no need to worry about multi-threading. Note that this means multiple
- * invocations could occur on different threads however.
- */
Return<void> ClientCounterCallback::onClients(const sp<::android::hidl::base::V1_0::IBase>& service,
bool clients) {
- Service& registered = assertRegisteredService(service);
+ std::lock_guard<std::mutex> lock(mMutex);
+ Service& registered = assertRegisteredServiceLocked(service);
if (registered.clients == clients) {
LOG(FATAL) << "Process already thought " << getDescriptor(service.get()) << "/"
<< registered.name << " had clients: " << registered.clients
@@ -181,13 +185,13 @@
// client count change event, try to shutdown the process if its services
// have no clients.
if (!handledInCallback && numWithClients == 0) {
- tryShutdown();
+ tryShutdownLocked();
}
return Status::ok();
}
-bool ClientCounterCallback::tryUnregister() {
+bool ClientCounterCallback::tryUnregisterLocked() {
auto manager = hardware::defaultServiceManager1_2();
for (Service& entry : mRegisteredServices) {
@@ -206,14 +210,14 @@
return true;
}
-void ClientCounterCallback::reRegister() {
+void ClientCounterCallback::reRegisterLocked() {
for (Service& entry : mRegisteredServices) {
// re-register entry if not already registered
if (entry.registered) {
continue;
}
- if (!registerService(entry.service, entry.name)) {
+ if (!registerServiceLocked(entry.service, entry.name)) {
// Must restart. Otherwise, clients will never be able to get ahold of this service.
LOG(FATAL) << "Bad state: could not re-register " << getDescriptor(entry.service.get());
}
@@ -222,22 +226,24 @@
}
}
-void ClientCounterCallback::tryShutdown() {
+void ClientCounterCallback::tryShutdownLocked() {
LOG(INFO) << "Trying to exit HAL. No clients in use for any service in process.";
- if (tryUnregister()) {
+ if (tryUnregisterLocked()) {
LOG(INFO) << "Unregistered all clients and exiting";
exit(EXIT_SUCCESS);
}
// At this point, we failed to unregister some of the services, leaving the
// server in an inconsistent state. Re-register all services that were
- // unregistered by tryUnregister().
- reRegister();
+ // unregistered by tryUnregisterLocked().
+ reRegisterLocked();
}
void ClientCounterCallback::setActiveServicesCallback(
const std::function<bool(bool)>& activeServicesCallback) {
+ std::lock_guard<std::mutex> lock(mMutex);
+
mActiveServicesCallback = activeServicesCallback;
}
@@ -251,11 +257,15 @@
}
bool LazyServiceRegistrarImpl::tryUnregister() {
- return mClientCallback->tryUnregister();
+ // see comments in header, this should only be called from the active
+ // services callback, see also b/191781736
+ return mClientCallback->tryUnregisterLocked();
}
void LazyServiceRegistrarImpl::reRegister() {
- mClientCallback->reRegister();
+ // see comments in header, this should only be called from the active
+ // services callback, see also b/191781736
+ mClientCallback->reRegisterLocked();
}
void LazyServiceRegistrarImpl::setActiveServicesCallback(
diff --git a/transport/include/hidl/HidlLazyUtils.h b/transport/include/hidl/HidlLazyUtils.h
index 427611b..376b6ab 100644
--- a/transport/include/hidl/HidlLazyUtils.h
+++ b/transport/include/hidl/HidlLazyUtils.h
@@ -65,7 +65,8 @@
/**
* Try to unregister all services previously registered with 'registerService'.
- * Returns 'true' if successful.
+ * Returns 'true' if successful. This should only be called within
+ * the callback registered by setActiveServicesCallback.
*/
bool tryUnregister();