1 files changed, 156 insertions, 45 deletions

diff --git a/transport/HidlLazyUtils.cpp b/transport/HidlLazyUtils.cpp
index 08ed676..c5f8c74 100644
--- a/transport/HidlLazyUtils.cpp
+++ b/transport/HidlLazyUtils.cpp
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#define LOG_TAG "HidlLazyUtils"
+
 #include <hidl/HidlLazyUtils.h>
 #include <hidl/HidlTransportSupport.h>
 
@@ -29,55 +31,85 @@ namespace details {
 using ::android::hidl::base::V1_0::IBase;
 
 class ClientCounterCallback : public ::android::hidl::manager::V1_2::IClientCallback {
-   public:
-    ClientCounterCallback() : mNumConnectedServices(0) {}
+  public:
+    ClientCounterCallback() {}
 
     bool addRegisteredService(const sp<IBase>& service, const std::string& name);
 
-   protected:
+    bool tryUnregisterLocked();
+
+    void reRegisterLocked();
+
+    void setActiveServicesCallback(const std::function<bool(bool)>& activeServicesCallback);
+
+  protected:
     Return<void> onClients(const sp<IBase>& service, bool clients) override;
 
-   private:
+  private:
+    struct Service {
+        sp<IBase> service;
+        std::string name;
+        bool clients = false;
+        // Used to keep track of unregistered services to allow re-registry
+        bool registered = true;
+    };
+
+    /**
+     * Looks up service that is guaranteed to be registered (service from
+     * onClients).
+     */
+    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();
 
     /**
-     * Counter of the number of services that currently have at least one client.
+     * For below.
      */
-    size_t mNumConnectedServices;
+    std::mutex mMutex;
 
-    struct Service {
-        sp<IBase> service;
-        std::string name;
-    };
     /**
      * Number of services that have been registered.
      */
     std::vector<Service> mRegisteredServices;
+
+    /**
+     * Callback for reporting the number of services with clients.
+     */
+    std::function<bool(bool)> mActiveServicesCallback;
+
+    /**
+     * Previous value passed to the active services callback.
+     */
+    std::optional<bool> mPreviousHasClients;
 };
 
 class LazyServiceRegistrarImpl {
-   public:
+  public:
     LazyServiceRegistrarImpl() : mClientCallback(new ClientCounterCallback) {}
 
     status_t registerService(const sp<::android::hidl::base::V1_0::IBase>& service,
                              const std::string& name);
+    bool tryUnregister();
+    void reRegister();
+    void setActiveServicesCallback(const std::function<bool(bool)>& activeServicesCallback);
 
-   private:
+  private:
     sp<ClientCounterCallback> mClientCallback;
 };
 
 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});
@@ -86,7 +118,19 @@ bool ClientCounterCallback::addRegisteredService(const sp<IBase>& service,
     return success;
 }
 
-bool ClientCounterCallback::registerService(const sp<IBase>& service, const std::string& name) {
+ClientCounterCallback::Service& ClientCounterCallback::assertRegisteredServiceLocked(
+        const sp<IBase>& service) {
+    for (Service& registered : mRegisteredServices) {
+        if (registered.service != service) continue;
+        return registered;
+    }
+    LOG(FATAL) << "Got callback on service " << getDescriptor(service.get())
+               << " which we did not register.";
+    __builtin_unreachable();
+}
+
+bool ClientCounterCallback::registerServiceLocked(const sp<IBase>& service,
+                                                  const std::string& name) {
     auto manager = hardware::defaultServiceManager1_2();
 
     const std::string descriptor = getDescriptor(service.get());
@@ -108,62 +152,99 @@ 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) {
-    if (clients) {
-        mNumConnectedServices++;
-    } else {
-        mNumConnectedServices--;
+    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
+                   << " but hwservicemanager has notified has clients: " << clients;
+    }
+    registered.clients = clients;
+
+    size_t numWithClients = 0;
+    for (const Service& registered : mRegisteredServices) {
+        if (registered.clients) numWithClients++;
     }
 
-    LOG(INFO) << "Process has " << mNumConnectedServices << " (of " << mRegisteredServices.size()
+    LOG(INFO) << "Process has " << numWithClients << " (of " << mRegisteredServices.size()
               << " available) client(s) in use after notification " << getDescriptor(service.get())
-              << " has clients: " << clients;
+              << "/" << registered.name << " has clients: " << clients;
+
+    bool handledInCallback = false;
+    if (mActiveServicesCallback != nullptr) {
+        bool hasClients = numWithClients != 0;
+        if (hasClients != mPreviousHasClients) {
+            handledInCallback = mActiveServicesCallback(hasClients);
+            mPreviousHasClients = hasClients;
+        }
+    }
 
-    if (mNumConnectedServices == 0) {
-        tryShutdown();
+    // If there is no callback defined or the callback did not handle this
+    // client count change event, try to shutdown the process if its services
+    // have no clients.
+    if (!handledInCallback && numWithClients == 0) {
+        tryShutdownLocked();
     }
 
     return Status::ok();
 }
 
-void ClientCounterCallback::tryShutdown() {
-    LOG(INFO) << "Trying to exit HAL. No clients in use for any service in process.";
-
+bool ClientCounterCallback::tryUnregisterLocked() {
     auto manager = hardware::defaultServiceManager1_2();
 
-    auto unRegisterIt = mRegisteredServices.begin();
-    for (; unRegisterIt != mRegisteredServices.end(); ++unRegisterIt) {
-        auto& entry = (*unRegisterIt);
-
+    for (Service& entry : mRegisteredServices) {
         const std::string descriptor = getDescriptor(entry.service.get());
         bool success = manager->tryUnregister(descriptor, entry.name, entry.service);
 
         if (!success) {
             LOG(INFO) << "Failed to unregister HAL " << descriptor << "/" << entry.name;
-            break;
+            return false;
         }
-    }
 
-    if (unRegisterIt == mRegisteredServices.end()) {
-        LOG(INFO) << "Unregistered all clients and exiting";
-        exit(EXIT_SUCCESS);
+        // Mark the entry unregistered, but do not remove it (may still be re-registered)
+        entry.registered = false;
     }
 
-    for (auto reRegisterIt = mRegisteredServices.begin(); reRegisterIt != unRegisterIt;
-         reRegisterIt++) {
-        auto& entry = (*reRegisterIt);
+    return true;
+}
+
+void ClientCounterCallback::reRegisterLocked() {
+    for (Service& entry : mRegisteredServices) {
+        // re-register entry if not already registered
+        if (entry.registered) {
+            continue;
+        }
 
-        // re-register entry
-        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());
         }
+
+        entry.registered = true;
+    }
+}
+
+void ClientCounterCallback::tryShutdownLocked() {
+    LOG(INFO) << "Trying to exit HAL. No clients in use for any service in process.";
+
+    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 tryUnregisterLocked().
+    reRegisterLocked();
+}
+
+void ClientCounterCallback::setActiveServicesCallback(
+        const std::function<bool(bool)>& activeServicesCallback) {
+    std::lock_guard<std::mutex> lock(mMutex);
+
+    mActiveServicesCallback = activeServicesCallback;
 }
 
 status_t LazyServiceRegistrarImpl::registerService(
@@ -175,6 +256,23 @@ status_t LazyServiceRegistrarImpl::registerService(
     return ::android::OK;
 }
 
+bool LazyServiceRegistrarImpl::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() {
+    // see comments in header, this should only be called from the active
+    // services callback, see also b/191781736
+    mClientCallback->reRegisterLocked();
+}
+
+void LazyServiceRegistrarImpl::setActiveServicesCallback(
+        const std::function<bool(bool)>& activeServicesCallback) {
+    mClientCallback->setActiveServicesCallback(activeServicesCallback);
+}
+
 }  // namespace details
 
 LazyServiceRegistrar::LazyServiceRegistrar() {
@@ -191,5 +289,18 @@ status_t LazyServiceRegistrar::registerService(
     return mImpl->registerService(service, name);
 }
 
+bool LazyServiceRegistrar::tryUnregister() {
+    return mImpl->tryUnregister();
+}
+
+void LazyServiceRegistrar::reRegister() {
+    mImpl->reRegister();
+}
+
+void LazyServiceRegistrar::setActiveServicesCallback(
+        const std::function<bool(bool)>& activeServicesCallback) {
+    mImpl->setActiveServicesCallback(activeServicesCallback);
+}
+
 }  // namespace hardware
 }  // namespace android