diff options
author | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2021-07-02 01:12:03 +0000 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2021-07-02 01:12:03 +0000 |
commit | a7124a45ee45f4b437c1ed004120094b39b8298c (patch) | |
tree | 8a5d133f1c9c6dc833bfde097a8a033bc25835e0 | |
parent | af8c307c85de93cd095f2063781eadf088b7dbb7 (diff) | |
parent | 390007d9905d45b3f4a941b26127c13f690a9e18 (diff) | |
download | libhidl-android12L-d2-release.tar.gz |
Snap for 7513903 from 390007d9905d45b3f4a941b26127c13f690a9e18 to sc-d2-releaseandroid-12.1.0_r26android-12.1.0_r25android-12.1.0_r24android-12.1.0_r23android-12.1.0_r18android-12.1.0_r17android-12.1.0_r16android-12.1.0_r15android-12.1.0_r14android-12.1.0_r13android-12.1.0_r12android12L-d2-s8-releaseandroid12L-d2-s7-releaseandroid12L-d2-s6-releaseandroid12L-d2-s5-releaseandroid12L-d2-s4-releaseandroid12L-d2-s3-releaseandroid12L-d2-s2-releaseandroid12L-d2-s1-releaseandroid12L-d2-release
Change-Id: I389119c7a200e92ca53473ddcec5fad79f1aa1b9
-rw-r--r-- | transport/HidlLazyUtils.cpp | 56 | ||||
-rw-r--r-- | transport/include/hidl/HidlLazyUtils.h | 3 |
2 files changed, 35 insertions, 24 deletions
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 @@ class ClientCounterCallback : public ::android::hidl::manager::V1_2::IClientCall 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 @@ class ClientCounterCallback : public ::android::hidl::manager::V1_2::IClientCall * 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 @@ class LazyServiceRegistrarImpl { 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 @@ bool ClientCounterCallback::addRegisteredService(const sp<IBase>& service, 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 @@ ClientCounterCallback::Service& ClientCounterCallback::assertRegisteredService( __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 @@ bool ClientCounterCallback::registerService(const sp<IBase>& service, const std: 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 @@ Return<void> ClientCounterCallback::onClients(const sp<::android::hidl::base::V1 // 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 @@ bool ClientCounterCallback::tryUnregister() { 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::reRegister() { } } -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 @@ status_t LazyServiceRegistrarImpl::registerService( } 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 @@ class LazyServiceRegistrar { /** * 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(); |