commit 26485229358cb240dd2f2f8bf69e13a0885eb822
author Devin Moore <devinmoore@google.com> Thu Aug 06 13:58:29 2020 -0700
committer Steven Moreland <smoreland@google.com> Mon Aug 10 16:48:49 2020 +0000
tree 951c7eb25ce22a3dd37d26dc16dcfb3ac18a0b2a
parent 2147dc738bcbe00d82c3296f577687576d605b15

servicemanager: Overwrite old service when a new one is registered

We want to overwrite an old service with the same name as a new service
that is being added. The old one will be cleaned up as servicemanager
loses the reference to it.

Test: atest servicemanager_test && additional tests outlined in Bug
Bug: 162553096
Merged-In: I8835d9f7505367d83a0f29f1d928a07b3d57e56a
Change-Id: I8835d9f7505367d83a0f29f1d928a07b3d57e56a

cmds/servicemanager/ServiceManager.cpp

diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp
index 1f9892a..e80c321 100644
--- a/cmds/servicemanager/ServiceManager.cpp
+++ b/cmds/servicemanager/ServiceManager.cpp
@@ -213,17 +213,18 @@
         return Status::fromExceptionCode(Status::EX_ILLEGAL_STATE);
     }
 
-    auto entry = mNameToService.emplace(name, Service {
+    // Overwrite the old service if it exists
+    mNameToService[name] = Service {
         .binder = binder,
         .allowIsolated = allowIsolated,
         .dumpPriority = dumpPriority,
         .debugPid = ctx.debugPid,
-    });
+    };
 
     auto it = mNameToRegistrationCallback.find(name);
     if (it != mNameToRegistrationCallback.end()) {
         for (const sp<IServiceCallback>& cb : it->second) {
-            entry.first->second.guaranteeClient = true;
+            mNameToService[name].guaranteeClient = true;
             // permission checked in registerForNotifications
             cb->onRegistration(name, binder);
         }

cmds/servicemanager/test_sm.cpp

diff --git a/cmds/servicemanager/test_sm.cpp b/cmds/servicemanager/test_sm.cpp
index 25245be..fb9f9df 100644
--- a/cmds/servicemanager/test_sm.cpp
+++ b/cmds/servicemanager/test_sm.cpp
@@ -135,6 +135,26 @@
         IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
 }
 
+TEST(AddService, OverwriteExistingService) {
+    auto sm = getPermissiveServiceManager();
+    sp<IBinder> serviceA = getBinder();
+    EXPECT_TRUE(sm->addService("foo", serviceA, false /*allowIsolated*/,
+        IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
+
+    sp<IBinder> outA;
+    EXPECT_TRUE(sm->getService("foo", &outA).isOk());
+    EXPECT_EQ(serviceA, outA);
+
+    // serviceA should be overwritten by serviceB
+    sp<IBinder> serviceB = getBinder();
+    EXPECT_TRUE(sm->addService("foo", serviceB, false /*allowIsolated*/,
+        IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
+
+    sp<IBinder> outB;
+    EXPECT_TRUE(sm->getService("foo", &outB).isOk());
+    EXPECT_EQ(serviceB, outB);
+}
+
 TEST(AddService, NoPermissions) {
     std::unique_ptr<MockAccess> access = std::make_unique<NiceMock<MockAccess>>();