commit c7c5db80460b4a6d19371165941cf1caa345bc7c
author Steven Moreland <smoreland@google.com> Thu Mar 21 16:01:13 2019 -0700
committer android-build-merger <android-build-merger@google.com> Thu Mar 21 16:01:13 2019 -0700
tree b10e44bb55e43be04d1af35be8807a7dd7b7a621
parent 3ddeb74685e1f7e1be89e75d39b24b4be8936ec5
parent 62729eea877bdce41dc4341072422664b95c3f26

Log for duplicate service registration. am: d90d9f2bc9 am: 2137299e95
am: 62729eea87

Change-Id: I60240ae17be65c5126412039442404ea71455964

ServiceManager.cpp

diff --git a/ServiceManager.cpp b/ServiceManager.cpp
index f76afcd..c274d0e 100644
--- a/ServiceManager.cpp
+++ b/ServiceManager.cpp
@@ -314,7 +314,7 @@
     return addSuccess;
 }
 
-bool ServiceManager::addImpl(const hidl_string& name,
+bool ServiceManager::addImpl(const std::string& name,
                              const sp<IBase>& service,
                              const hidl_vec<hidl_string>& interfaceChain,
                              const AccessControl::CallingContext& callingContext) {
@@ -332,16 +332,36 @@
         }
     }
 
+    // Detect duplicate registration
+    if (interfaceChain.size() > 1) {
+        // second to last entry should be the highest base class other than IBase.
+        const std::string baseFqName = interfaceChain[interfaceChain.size() - 2];
+        const HidlService *hidlService = lookup(baseFqName, name);
+        if (hidlService != nullptr && hidlService->getService() != nullptr) {
+            // This shouldn't occur during normal operation. Here are some cases where
+            // it might get hit:
+            // - bad configuration (service installed on device multiple times)
+            // - race between death notification and a new service being registered
+            //     (previous logs should indicate a separate problem)
+            const std::string childFqName = interfaceChain[0];
+            pid_t newServicePid = IPCThreadState::self()->getCallingPid();
+            pid_t oldServicePid = hidlService->getDebugPid();
+            LOG(WARNING) << "Detected instance of " << childFqName << " (pid: " << newServicePid
+                    << ") registering over instance of or with base of " << baseFqName << " (pid: "
+                    << oldServicePid << ").";
+        }
+    }
+
+    // Unregister superclass if subclass is registered over it
     {
         // For IBar extends IFoo if IFoo/default is being registered, remove
         // IBar/default. This makes sure the following two things are equivalent
         // 1). IBar::castFrom(IFoo::getService(X))
         // 2). IBar::getService(X)
         // assuming that IBar is declared in the device manifest and there
-        // is also not an IBaz extends IFoo.
+        // is also not an IBaz extends IFoo and there is no race.
         const std::string childFqName = interfaceChain[0];
-        const PackageInterfaceMap &ifaceMap = mServiceMap[childFqName];
-        const HidlService *hidlService = ifaceMap.lookup(name);
+        const HidlService *hidlService = lookup(childFqName, name);
         if (hidlService != nullptr) {
             const sp<IBase> remove = hidlService->getService();
 

ServiceManager.h

diff --git a/ServiceManager.h b/ServiceManager.h
index 8f0c83a..e70110c 100644
--- a/ServiceManager.h
+++ b/ServiceManager.h
@@ -88,7 +88,7 @@
 
     virtual void serviceDied(uint64_t cookie, const wp<IBase>& who);
 private:
-    bool addImpl(const hidl_string& name,
+    bool addImpl(const std::string& name,
                  const sp<IBase>& service,
                  const hidl_vec<hidl_string>& interfaceChain,
                  const AccessControl::CallingContext& callingContext);