diff options
author | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2021-07-01 01:10:21 +0000 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2021-07-01 01:10:21 +0000 |
commit | ef7cca7d56ab4763602de2185926ccff406a900a (patch) | |
tree | 8a5d133f1c9c6dc833bfde097a8a033bc25835e0 | |
parent | 6b96981b6ff1b180dbd8f3f52e7a5880fd8cf0be (diff) | |
parent | 8578d50d847b92c8de63d513ddf17557773a4751 (diff) | |
download | libhidl-android-12.0.0_r10.tar.gz |
Snap for 7510676 from 8578d50d847b92c8de63d513ddf17557773a4751 to sc-releaseandroid-vts-12.0_r9android-vts-12.0_r8android-vts-12.0_r7android-vts-12.0_r6android-vts-12.0_r5android-vts-12.0_r4android-vts-12.0_r3android-vts-12.0_r2android-vts-12.0_r12android-vts-12.0_r11android-vts-12.0_r10android-vts-12.0_r1android-security-12.0.0_r60android-security-12.0.0_r59android-security-12.0.0_r58android-security-12.0.0_r57android-security-12.0.0_r56android-security-12.0.0_r55android-security-12.0.0_r54android-security-12.0.0_r53android-security-12.0.0_r52android-security-12.0.0_r51android-security-12.0.0_r50android-security-12.0.0_r49android-security-12.0.0_r48android-security-12.0.0_r47android-security-12.0.0_r46android-security-12.0.0_r45android-security-12.0.0_r44android-security-12.0.0_r43android-security-12.0.0_r42android-security-12.0.0_r41android-security-12.0.0_r40android-security-12.0.0_r39android-security-12.0.0_r38android-security-12.0.0_r37android-security-12.0.0_r36android-security-12.0.0_r35android-security-12.0.0_r34android-platform-12.0.0_r9android-platform-12.0.0_r8android-platform-12.0.0_r7android-platform-12.0.0_r6android-platform-12.0.0_r5android-platform-12.0.0_r4android-platform-12.0.0_r31android-platform-12.0.0_r30android-platform-12.0.0_r3android-platform-12.0.0_r29android-platform-12.0.0_r28android-platform-12.0.0_r27android-platform-12.0.0_r26android-platform-12.0.0_r25android-platform-12.0.0_r24android-platform-12.0.0_r23android-platform-12.0.0_r22android-platform-12.0.0_r21android-platform-12.0.0_r20android-platform-12.0.0_r2android-platform-12.0.0_r19android-platform-12.0.0_r18android-platform-12.0.0_r17android-platform-12.0.0_r16android-platform-12.0.0_r15android-platform-12.0.0_r14android-platform-12.0.0_r13android-platform-12.0.0_r12android-platform-12.0.0_r11android-platform-12.0.0_r10android-platform-12.0.0_r1android-cts-12.0_r9android-cts-12.0_r8android-cts-12.0_r7android-cts-12.0_r6android-cts-12.0_r5android-cts-12.0_r4android-cts-12.0_r3android-cts-12.0_r2android-cts-12.0_r12android-cts-12.0_r11android-cts-12.0_r10android-cts-12.0_r1android-12.0.0_r9android-12.0.0_r8android-12.0.0_r34android-12.0.0_r33android-12.0.0_r31android-12.0.0_r30android-12.0.0_r3android-12.0.0_r25android-12.0.0_r2android-12.0.0_r11android-12.0.0_r10android-12.0.0_r1android12-tests-releaseandroid12-security-releaseandroid12-s5-releaseandroid12-s4-releaseandroid12-s3-releaseandroid12-s2-releaseandroid12-s1-releaseandroid12-releaseandroid12-platform-release
Change-Id: I0d3b3d5fe722075087c8c4d9b5a3f8ac598a4b45
-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(); |