dynamic services: fix extra sends

Before this change, hwservicemanager sometimes sent out client service
notifications for services which did not currently have known clients.
This is because HidlService always compared the number of clients
reported to the driver w/ '1', representing the service as held by
hwservicemanager. However, in tryUnregister calls and addClientCallback
calls, there are actually two known clients.

So, in this CL, the number of known clients is always piped through from
ServiceManager to HidlService.

Bug: 140310064
Test: hidl_lazy_test
Test: hidl_lazy_test (with hwservicemanager and the test modified to run
    ms rather than every 5s. This allows many more iterations to be
    tested at a time).
Test: hwservicemanager_test
Change-Id: I1d98aef29bf6ccfb5088f525c250874d3f2254f1

1 files changed, 12 insertions, 10 deletions

diff --git a/ServiceManager.cpp b/ServiceManager.cpp
index 116969e..0e45cc6 100644
--- a/ServiceManager.cpp
+++ b/ServiceManager.cpp
@@ -291,7 +291,7 @@ Return<sp<IBase>> ServiceManager::get(const hidl_string& hidlFqName,
     // time. This will run before anything else can modify the HidlService which is owned by this
     // object, so it will be in the same state that it was when this function returns.
     hardware::addPostCommandTask([hidlService] {
-        hidlService->handleClientCallbacks(false /* isCalledOnInterval */);
+        hidlService->handleClientCallbacks(false /* isCalledOnInterval */, 1 /*knownClientCount*/);
     });
 
     return service;
@@ -597,7 +597,10 @@ Return<bool> ServiceManager::registerClientCallback(const hidl_string& hidlFqNam
         return false;
     }
 
-    registered->addClientCallback(cb);
+    // knownClientCount
+    // - one from binder transaction (base here)
+    // - one from hwservicemanager
+    registered->addClientCallback(cb, 2 /*knownClientCount*/);
 
     return true;
 }
@@ -620,7 +623,8 @@ Return<bool> ServiceManager::unregisterClientCallback(const sp<IBase>& server,
 
 void ServiceManager::handleClientCallbacks() {
     forEachServiceEntry([&] (HidlService *service) {
-        service->handleClientCallbacks(true /* isCalledOnInterval */);
+        // hwservicemanager will hold one reference, so knownClientCount is 1.
+        service->handleClientCallbacks(true /* isCalledOnInterval */, 1 /*knownClientCount*/);
         return true;  // continue
     });
 }
@@ -683,14 +687,12 @@ Return<bool> ServiceManager::tryUnregister(const hidl_string& hidlFqName,
         return false;
     }
 
-    int clients = registered->forceHandleClientCallbacks(false /* isCalledOnInterval */);
+    // knownClientCount
+    // - one from binder transaction (base here)
+    // - one from hwservicemanager
+    bool clients = registered->forceHandleClientCallbacks(false /*isCalledOnInterval*/, 2 /*knownClientCount*/);
 
-    // clients < 0: feature not implemented or other error. Assume clients.
-    // Otherwise:
-    // - kernel driver will hold onto one refcount (during this transaction)
-    // - hwservicemanager has a refcount (guaranteed by this transaction)
-    // So, if clients > 2, then at least one other service on the system must hold a refcount.
-    if (clients < 0 || clients > 2) {
+    if (clients) {
         // client callbacks are either disabled or there are other clients
         LOG(INFO) << "Tried to unregister for " << fqName << "/" << name
             << " but there are clients: " << clients;