From 5de798e9103c09ae0ae2052822211d36d1fd3b1f Mon Sep 17 00:00:00 2001
From: Jon Spivack <spivack@google.com>
Date: Fri, 27 Dec 2019 20:13:26 -0800
Subject: Dynamically stop services with multiple interfaces

This fixes a couple workflow bugs with dynamic services that had multiple interfaces in use at once. Attempting to stop one could affect the reference count of the other, and re-adding it after a failed removal yielded duplicate registrations.

Bug: 146903840
Test: aidl_lazy_test
Change-Id: I7ec80a280dabf7c576b7b00dff404a68c24ae5f1
---
 cmds/servicemanager/ServiceManager.cpp | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

(limited to 'cmds/servicemanager/ServiceManager.cpp')

diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp
index ae74ac3847..abe64365f3 100644
--- a/cmds/servicemanager/ServiceManager.cpp
+++ b/cmds/servicemanager/ServiceManager.cpp
@@ -423,11 +423,12 @@ ssize_t ServiceManager::Service::getNodeStrongRefCount() {
 
 void ServiceManager::handleClientCallbacks() {
     for (const auto& [name, service] : mNameToService) {
-        handleServiceClientCallback(name);
+        handleServiceClientCallback(name, true);
     }
 }
 
-ssize_t ServiceManager::handleServiceClientCallback(const std::string& serviceName) {
+ssize_t ServiceManager::handleServiceClientCallback(const std::string& serviceName,
+                                                    bool isCalledOnInterval) {
     auto serviceIt = mNameToService.find(serviceName);
     if (serviceIt == mNameToService.end() || mNameToClientCallback.count(serviceName) < 1) {
         return -1;
@@ -451,14 +452,17 @@ ssize_t ServiceManager::handleServiceClientCallback(const std::string& serviceNa
         service.guaranteeClient = false;
     }
 
-    if (hasClients && !service.hasClients) {
-        // client was retrieved in some other way
-        sendClientCallbackNotifications(serviceName, true);
-    }
+    // only send notifications if this was called via the interval checking workflow
+    if (isCalledOnInterval) {
+        if (hasClients && !service.hasClients) {
+            // client was retrieved in some other way
+            sendClientCallbackNotifications(serviceName, true);
+        }
 
-    // there are no more clients, but the callback has not been called yet
-    if (!hasClients && service.hasClients) {
-        sendClientCallbackNotifications(serviceName, false);
+        // there are no more clients, but the callback has not been called yet
+        if (!hasClients && service.hasClients) {
+            sendClientCallbackNotifications(serviceName, false);
+        }
     }
 
     return count;
@@ -518,7 +522,7 @@ Status ServiceManager::tryUnregisterService(const std::string& name, const sp<IB
         return Status::fromExceptionCode(Status::EX_ILLEGAL_STATE);
     }
 
-    int clients = handleServiceClientCallback(name);
+    int clients = handleServiceClientCallback(name, false);
 
     // clients < 0: feature not implemented or other error. Assume clients.
     // Otherwise:
@@ -527,7 +531,7 @@ Status ServiceManager::tryUnregisterService(const std::string& name, const sp<IB
     // So, if clients > 2, then at least one other service on the system must hold a refcount.
     if (clients < 0 || clients > 2) {
         // client callbacks are either disabled or there are other clients
-        LOG(INFO) << "Tried to unregister " << name << " but there are clients: " << clients;
+        LOG(INFO) << "Tried to unregister " << name << ", but there are clients: " << clients;
         return Status::fromExceptionCode(Status::EX_ILLEGAL_STATE);
     }
 
-- 
cgit v1.2.3-59-g8ed1b