commit 2b91790273db550a793bff9aea2dc999ce78e44e
author Martijn Coenen <maco@google.com> Wed Feb 14 08:10:11 2018 +0000
committer android-build-merger <android-build-merger@google.com> Wed Feb 14 08:10:11 2018 +0000
tree 3a578f6145f1f6557d83fdaf047c12538bd9827f
parent 2dfea68f476c5e6e49c5747e32c1ebcb7417c9e1
parent 356717a127eaf0c5fe9faa63f2a6c7e433a6a476

Unregister for notifications when we're done waiting. am: 3fa15c2679 am: 097f8a1672
am: 356717a127

Change-Id: I5e576a1ff4f4966a577ef98129fabf45b636da3b

transport/ServiceManagement.cpp

diff --git a/transport/ServiceManagement.cpp b/transport/ServiceManagement.cpp
index 4bdeb6e..e3c19e5 100644
--- a/transport/ServiceManagement.cpp
+++ b/transport/ServiceManagement.cpp
@@ -511,12 +511,9 @@
     }
 
     ~Waiter() {
-        if (mRegisteredForNotifications) {
-            if (!mSm->unregisterForNotifications(mInterfaceName, mInstanceName, this).
-                    withDefault(false)) {
-                LOG(ERROR) << "Could not unregister service notification for "
-                    << mInterfaceName << "/" << mInstanceName << ".";
-            }
+        if (!mDoneCalled) {
+            LOG(FATAL)
+                << "Waiter still registered for notifications, call done() before dropping ref!";
         }
     }
 
@@ -567,7 +564,23 @@
         std::unique_lock<std::mutex> lock(mMutex);
         mRegistered = false;
     }
-private:
+
+    // done() must be called before dropping the last strong ref to the Waiter, to make
+    // sure we can properly unregister with hwservicemanager.
+    void done() {
+        if (mRegisteredForNotifications) {
+            if (!mSm->unregisterForNotifications(mInterfaceName, mInstanceName, this)
+                     .withDefault(false)) {
+                LOG(ERROR) << "Could not unregister service notification for " << mInterfaceName
+                           << "/" << mInstanceName << ".";
+            } else {
+                mRegisteredForNotifications = false;
+            }
+        }
+        mDoneCalled = true;
+    }
+
+   private:
     const std::string mInterfaceName;
     const std::string mInstanceName;
     const sp<IServiceManager1_1>& mSm;
@@ -575,12 +588,14 @@
     std::condition_variable mCondition;
     bool mRegistered = false;
     bool mRegisteredForNotifications = false;
+    bool mDoneCalled = false;
 };
 
 void waitForHwService(
         const std::string &interface, const std::string &instanceName) {
     sp<Waiter> waiter = new Waiter(interface, instanceName, defaultServiceManager1_1());
     waiter->wait();
+    waiter->done();
 }
 
 // Prints relevant error/warning messages for error return values from
@@ -670,6 +685,7 @@
                 details::canCastInterface(base.get(), descriptor.c_str(), true /* emitError */);
 
             if (canCastRet.isOk() && canCastRet) {
+                waiter->done();
                 return base; // still needs to be wrapped by Bp class.
             }
 
@@ -683,6 +699,8 @@
         waiter->wait();
     }
 
+    waiter->done();
+
     if (getStub || vintfPassthru || vintfLegacy) {
         const sp<IServiceManager> pm = getPassthroughServiceManager();
         if (pm != nullptr) {