Merge "Add a cache to getService with invalidation" into main

Treehugger Robot · Gerrit Code Review · commit 69333d682bdc · 2024-09-05T02:44:39.000Z
diff --git a/libs/binder/BackendUnifiedServiceManager.cpp b/libs/binder/BackendUnifiedServiceManager.cpp
@@ -24,11 +24,111 @@
 
 namespace android {
 
+#ifdef LIBBINDER_CLIENT_CACHE
+constexpr bool kUseCache = true;
+#else
+constexpr bool kUseCache = false;
+#endif
+
 using AidlServiceManager = android::os::IServiceManager;
 using IAccessor = android::os::IAccessor;
 
+static const char* kStaticCachableList[] = {
+        "activity",
+        "android.hardware.thermal.IThermal/default",
+        "android.hardware.power.IPower/default",
+        "android.frameworks.stats.IStats/default",
+        "android.system.suspend.ISystemSuspend/default",
+        "appops",
+        "audio",
+        "batterystats",
+        "carrier_config",
+        "connectivity",
+        "content_capture",
+        "device_policy",
+        "display",
+        "dropbox",
+        "econtroller",
+        "isub",
+        "legacy_permission",
+        "location",
+        "media.extractor",
+        "media.metrics",
+        "media.player",
+        "media.resource_manager",
+        "netd_listener",
+        "netstats",
+        "network_management",
+        "nfc",
+        "package_native",
+        "performance_hint",
+        "permission",
+        "permissionmgr",
+        "permission_checker",
+        "phone",
+        "platform_compat",
+        "power",
+        "role",
+        "sensorservice",
+        "statscompanion",
+        "telephony.registry",
+        "thermalservice",
+        "time_detector",
+        "trust",
+        "uimode",
+        "virtualdevice",
+        "virtualdevice_native",
+        "webviewupdate",
+};
+
+bool BinderCacheWithInvalidation::isClientSideCachingEnabled(const std::string& serviceName) {
+    if (ProcessState::self()->getThreadPoolMaxTotalThreadCount() <= 0) {
+        ALOGW("Thread Pool max thread count is 0. Cannot cache binder as linkToDeath cannot be "
+              "implemented. serviceName: %s",
+              serviceName.c_str());
+        return false;
+    }
+    for (const char* name : kStaticCachableList) {
+        if (name == serviceName) {
+            return true;
+        }
+    }
+    return false;
+}
+
+binder::Status BackendUnifiedServiceManager::updateCache(const std::string& serviceName,
+                                                         const os::Service& service) {
+    if (!kUseCache) {
+        return binder::Status::ok();
+    }
+    if (service.getTag() == os::Service::Tag::binder) {
+        sp<IBinder> binder = service.get<os::Service::Tag::binder>();
+        if (binder && mCacheForGetService->isClientSideCachingEnabled(serviceName) &&
+            binder->isBinderAlive()) {
+            return mCacheForGetService->setItem(serviceName, binder);
+        }
+    }
+    return binder::Status::ok();
+}
+
+bool BackendUnifiedServiceManager::returnIfCached(const std::string& serviceName,
+                                                  os::Service* _out) {
+    if (!kUseCache) {
+        return false;
+    }
+    sp<IBinder> item = mCacheForGetService->getItem(serviceName);
+    // TODO(b/363177618): Enable caching for binders which are always null.
+    if (item != nullptr && item->isBinderAlive()) {
+        *_out = os::Service::make<os::Service::Tag::binder>(item);
+        return true;
+    }
+    return false;
+}
+
 BackendUnifiedServiceManager::BackendUnifiedServiceManager(const sp<AidlServiceManager>& impl)
-      : mTheRealServiceManager(impl) {}
+      : mTheRealServiceManager(impl) {
+    mCacheForGetService = std::make_shared<BinderCacheWithInvalidation>();
+}
 
 sp<AidlServiceManager> BackendUnifiedServiceManager::getImpl() {
     return mTheRealServiceManager;
@@ -44,20 +144,34 @@ binder::Status BackendUnifiedServiceManager::getService(const ::std::string& nam
 
 binder::Status BackendUnifiedServiceManager::getService2(const ::std::string& name,
                                                          os::Service* _out) {
+    if (returnIfCached(name, _out)) {
+        return binder::Status::ok();
+    }
     os::Service service;
     binder::Status status = mTheRealServiceManager->getService2(name, &service);
+
     if (status.isOk()) {
-        return toBinderService(name, service, _out);
+        status = toBinderService(name, service, _out);
+        if (status.isOk()) {
+            return updateCache(name, service);
+        }
     }
     return status;
 }
 
 binder::Status BackendUnifiedServiceManager::checkService(const ::std::string& name,
                                                           os::Service* _out) {
     os::Service service;
+    if (returnIfCached(name, _out)) {
+        return binder::Status::ok();
+    }
+
     binder::Status status = mTheRealServiceManager->checkService(name, &service);
     if (status.isOk()) {
-        return toBinderService(name, service, _out);
+        status = toBinderService(name, service, _out);
+        if (status.isOk()) {
+            return updateCache(name, service);
+        }
     }
     return status;
 }
diff --git a/libs/binder/BackendUnifiedServiceManager.h b/libs/binder/BackendUnifiedServiceManager.h
@@ -18,9 +18,87 @@
 #include <android/os/BnServiceManager.h>
 #include <android/os/IServiceManager.h>
 #include <binder/IPCThreadState.h>
+#include <map>
+#include <memory>
 
 namespace android {
 
+class BinderCacheWithInvalidation
+      : public std::enable_shared_from_this<BinderCacheWithInvalidation> {
+    class BinderInvalidation : public IBinder::DeathRecipient {
+    public:
+        BinderInvalidation(std::weak_ptr<BinderCacheWithInvalidation> cache, const std::string& key)
+              : mCache(cache), mKey(key) {}
+
+        void binderDied(const wp<IBinder>& who) override {
+            sp<IBinder> binder = who.promote();
+            if (std::shared_ptr<BinderCacheWithInvalidation> cache = mCache.lock()) {
+                cache->removeItem(mKey, binder);
+            } else {
+                ALOGI("Binder Cache pointer expired: %s", mKey.c_str());
+            }
+        }
+
+    private:
+        std::weak_ptr<BinderCacheWithInvalidation> mCache;
+        std::string mKey;
+    };
+    struct Entry {
+        sp<IBinder> service;
+        sp<BinderInvalidation> deathRecipient;
+    };
+
+public:
+    sp<IBinder> getItem(const std::string& key) const {
+        std::lock_guard<std::mutex> lock(mCacheMutex);
+
+        if (auto it = mCache.find(key); it != mCache.end()) {
+            return it->second.service;
+        }
+        return nullptr;
+    }
+
+    bool removeItem(const std::string& key, const sp<IBinder>& who) {
+        std::lock_guard<std::mutex> lock(mCacheMutex);
+        if (auto it = mCache.find(key); it != mCache.end()) {
+            if (it->second.service == who) {
+                status_t result = who->unlinkToDeath(it->second.deathRecipient);
+                if (result != DEAD_OBJECT) {
+                    ALOGW("Unlinking to dead binder resulted in: %d", result);
+                }
+                mCache.erase(key);
+                return true;
+            }
+        }
+        return false;
+    }
+
+    binder::Status setItem(const std::string& key, const sp<IBinder>& item) {
+        sp<BinderInvalidation> deathRecipient =
+                sp<BinderInvalidation>::make(shared_from_this(), key);
+
+        // linkToDeath if binder is a remote binder.
+        if (item->localBinder() == nullptr) {
+            status_t status = item->linkToDeath(deathRecipient);
+            if (status != android::OK) {
+                ALOGE("Failed to linkToDeath binder for service %s. Error: %d", key.c_str(),
+                      status);
+                return binder::Status::fromStatusT(status);
+            }
+        }
+        std::lock_guard<std::mutex> lock(mCacheMutex);
+        Entry entry = {.service = item, .deathRecipient = deathRecipient};
+        mCache[key] = entry;
+        return binder::Status::ok();
+    }
+
+    bool isClientSideCachingEnabled(const std::string& serviceName);
+
+private:
+    std::map<std::string, Entry> mCache;
+    mutable std::mutex mCacheMutex;
+};
+
 class BackendUnifiedServiceManager : public android::os::BnServiceManager {
 public:
     explicit BackendUnifiedServiceManager(const sp<os::IServiceManager>& impl);
@@ -58,9 +136,12 @@ class BackendUnifiedServiceManager : public android::os::BnServiceManager {
     }
 
 private:
+    std::shared_ptr<BinderCacheWithInvalidation> mCacheForGetService;
     sp<os::IServiceManager> mTheRealServiceManager;
     binder::Status toBinderService(const ::std::string& name, const os::Service& in,
                                    os::Service* _out);
+    binder::Status updateCache(const std::string& serviceName, const os::Service& service);
+    bool returnIfCached(const std::string& serviceName, os::Service* _out);
 };
 
 sp<BackendUnifiedServiceManager> getBackendUnifiedServiceManager();
diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp
@@ -18,6 +18,7 @@
 #define LOG_TAG "ServiceManagerCppClient"
 
 #include <binder/IServiceManager.h>
+#include <binder/IServiceManagerUnitTestHelper.h>
 #include "BackendUnifiedServiceManager.h"
 
 #include <inttypes.h>
@@ -311,6 +312,11 @@ void setDefaultServiceManager(const sp<IServiceManager>& sm) {
     }
 }
 
+sp<IServiceManager> getServiceManagerShimFromAidlServiceManagerForTests(
+        const sp<AidlServiceManager>& sm) {
+    return sp<CppBackendShim>::make(sp<BackendUnifiedServiceManager>::make(sm));
+}
+
 std::weak_ptr<AccessorProvider> addAccessorProvider(RpcAccessorProvider&& providerCallback) {
     std::lock_guard<std::mutex> lock(gAccessorProvidersMutex);
     std::shared_ptr<AccessorProvider> provider =
diff --git a/libs/binder/TEST_MAPPING b/libs/binder/TEST_MAPPING
@@ -133,6 +133,9 @@
     {
       "name": "binder_sdk_test",
       "host": true
+    },
+    {
+      "name": "binderCacheUnitTest"
     }
   ],
   "imports": [
diff --git a/libs/binder/include/binder/IServiceManagerUnitTestHelper.h b/libs/binder/include/binder/IServiceManagerUnitTestHelper.h
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <android/os/IServiceManager.h>
+#include "IServiceManager.h"
+namespace android {
+
+/**
+ * Encapsulate an AidlServiceManager in a CppBackendShim. Only used for testing.
+ */
+LIBBINDER_EXPORTED sp<IServiceManager> getServiceManagerShimFromAidlServiceManagerForTests(
+        const sp<os::IServiceManager>& sm);
+
+} // namespace android
diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
@@ -51,6 +51,30 @@ cc_test {
     ],
 }
 
+cc_test {
+    name: "binderCacheUnitTest",
+    target: {
+        darwin: {
+            enabled: false,
+        },
+    },
+    srcs: [
+        "binderCacheUnitTest.cpp",
+    ],
+    shared_libs: [
+        "liblog",
+        "libbinder",
+        "libcutils",
+        "libutils",
+    ],
+    static_libs: [
+        "libfakeservicemanager",
+    ],
+    defaults: ["libbinder_client_cache_flag"],
+    test_suites: ["general-tests"],
+    require_root: true,
+}
+
 // unit test only, which can run on host and doesn't use /dev/binder
 cc_test {
     name: "binderUnitTest",
diff --git a/libs/binder/tests/binderCacheUnitTest.cpp b/libs/binder/tests/binderCacheUnitTest.cpp

Original file line number	Diff line number	Diff line change
`@@ -133,6 +133,9 @@`
`133`	`133`	`{`
`134`	`134`	`"name": "binder_sdk_test",`
`135`	`135`	`"host": true`
	`136`	`+ },`
	`137`	`+ {`
	`138`	`+ "name": "binderCacheUnitTest"`
`136`	`139`	`}`
`137`	`140`	`],`
`138`	`141`	`"imports": [`