libbinder: Add ClientCounterCallbackImpl to LazyServiceRegistrar

This extra layer of indirection below ClientCounterCallback fixes a shared pointer ownership issue between LazyServiceRegistrar and ServiceManager. It also allows for implementation changes (like this one) without changing headers and breaking VNDK.

Bug: 170212632
Test: Manual (Went through reproduction steps in bug on cf_x86_phone-userdebug)
Test: atest aidl_lazy_test
Change-Id: I4164a6d44e567c752726953e85aee0e91c6b525e
Merged-In: I4164a6d44e567c752726953e85aee0e91c6b525e

1 files changed, 35 insertions, 11 deletions

diff --git a/libs/binder/LazyServiceRegistrar.cpp b/libs/binder/LazyServiceRegistrar.cpp
index 6f49aa1607..f2c5139b56 100644
--- a/libs/binder/LazyServiceRegistrar.cpp
+++ b/libs/binder/LazyServiceRegistrar.cpp
@@ -29,16 +29,12 @@ namespace internal {
 
 using AidlServiceManager = android::os::IServiceManager;
 
-class ClientCounterCallback : public ::android::os::BnClientCallback {
+class ClientCounterCallbackImpl : public ::android::os::BnClientCallback {
 public:
-    ClientCounterCallback() : mNumConnectedServices(0), mForcePersist(false) {}
+    ClientCounterCallbackImpl() : mNumConnectedServices(0), mForcePersist(false) {}
 
     bool registerService(const sp<IBinder>& service, const std::string& name,
                          bool allowIsolated, int dumpFlags);
-
-    /**
-     * Set a flag to prevent services from automatically shutting down
-     */
     void forcePersist(bool persist);
 
 protected:
@@ -69,7 +65,23 @@ private:
     bool mForcePersist;
 };
 
-bool ClientCounterCallback::registerService(const sp<IBinder>& service, const std::string& name,
+class ClientCounterCallback {
+public:
+    ClientCounterCallback();
+
+    bool registerService(const sp<IBinder>& service, const std::string& name,
+                                            bool allowIsolated, int dumpFlags);
+
+    /**
+     * Set a flag to prevent services from automatically shutting down
+     */
+    void forcePersist(bool persist);
+
+private:
+    sp<ClientCounterCallbackImpl> mImpl;
+};
+
+bool ClientCounterCallbackImpl::registerService(const sp<IBinder>& service, const std::string& name,
                                             bool allowIsolated, int dumpFlags) {
     auto manager = interface_cast<AidlServiceManager>(asBinder(defaultServiceManager()));
 
@@ -95,7 +107,7 @@ bool ClientCounterCallback::registerService(const sp<IBinder>& service, const st
     return true;
 }
 
-void ClientCounterCallback::forcePersist(bool persist) {
+void ClientCounterCallbackImpl::forcePersist(bool persist) {
     mForcePersist = persist;
     if(!mForcePersist) {
         // Attempt a shutdown in case the number of clients hit 0 while the flag was on
@@ -107,7 +119,7 @@ void ClientCounterCallback::forcePersist(bool persist) {
  * onClients is oneway, so no need to worry about multi-threading. Note that this means multiple
  * invocations could occur on different threads however.
  */
-Status ClientCounterCallback::onClients(const sp<IBinder>& service, bool clients) {
+Status ClientCounterCallbackImpl::onClients(const sp<IBinder>& service, bool clients) {
     if (clients) {
         mNumConnectedServices++;
     } else {
@@ -122,7 +134,7 @@ Status ClientCounterCallback::onClients(const sp<IBinder>& service, bool clients
     return Status::ok();
 }
 
-void ClientCounterCallback::tryShutdown() {
+void ClientCounterCallbackImpl::tryShutdown() {
     if(mNumConnectedServices > 0) {
         // Should only shut down if there are no clients
         return;
@@ -143,7 +155,6 @@ void ClientCounterCallback::tryShutdown() {
 
         bool success = manager->tryUnregisterService(entry.first, entry.second.service).isOk();
 
-
         if (!success) {
             ALOGI("Failed to unregister service %s", entry.first.c_str());
             break;
@@ -168,6 +179,19 @@ void ClientCounterCallback::tryShutdown() {
     }
 }
 
+ClientCounterCallback::ClientCounterCallback() {
+      mImpl = new ClientCounterCallbackImpl();
+}
+
+bool ClientCounterCallback::registerService(const sp<IBinder>& service, const std::string& name,
+                                            bool allowIsolated, int dumpFlags) {
+    return mImpl->registerService(service, name, allowIsolated, dumpFlags);
+}
+
+void ClientCounterCallback::forcePersist(bool persist) {
+    mImpl->forcePersist(persist);
+}
+
 }  // namespace internal
 
 LazyServiceRegistrar::LazyServiceRegistrar() {