diff options
author | 2024-06-25 14:38:42 +0000 | |
---|---|---|
committer | 2024-09-05 01:11:48 +0000 | |
commit | b6ed0eb38fd1ea22116b08eea8ee0fa418540965 (patch) | |
tree | 2e301342848ed34589b7401933705867bedd4fd8 | |
parent | d53d0744fcd1cf7354941ad231bb633fee308066 (diff) |
Add a cache to getService with invalidation
Test: atest binderCacheUnitTest
Bug: 333854840
Flag: LIBBINDER_CLIENT_CACHE
Change-Id: I1b4e8482817c422850aed7359beaf7174b55445e
-rw-r--r-- | libs/binder/BackendUnifiedServiceManager.cpp | 120 | ||||
-rw-r--r-- | libs/binder/BackendUnifiedServiceManager.h | 81 | ||||
-rw-r--r-- | libs/binder/IServiceManager.cpp | 6 | ||||
-rw-r--r-- | libs/binder/TEST_MAPPING | 3 | ||||
-rw-r--r-- | libs/binder/include/binder/IServiceManagerUnitTestHelper.h | 29 | ||||
-rw-r--r-- | libs/binder/tests/Android.bp | 24 | ||||
-rw-r--r-- | libs/binder/tests/binderCacheUnitTest.cpp | 214 |
7 files changed, 474 insertions, 3 deletions
diff --git a/libs/binder/BackendUnifiedServiceManager.cpp b/libs/binder/BackendUnifiedServiceManager.cpp index fee36da911..5680798d0d 100644 --- 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,10 +144,17 @@ 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; } @@ -55,9 +162,16 @@ binder::Status BackendUnifiedServiceManager::getService2(const ::std::string& na 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 index 387eb35346..47b2ec9f2c 100644 --- 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 @@ public: } 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 index cba21b2094..8a90ce2cab 100644 --- 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 index 125617363a..95a5da20ae 100644 --- 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 new file mode 100644 index 0000000000..ff25163ddc --- /dev/null +++ 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
\ No newline at end of file diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp index 1e463a45cc..0e653af707 100644 --- 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 new file mode 100644 index 0000000000..92dab19a63 --- /dev/null +++ b/libs/binder/tests/binderCacheUnitTest.cpp @@ -0,0 +1,214 @@ +/* + * 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. + */ +#include <gtest/gtest.h> + +#include <android-base/logging.h> +#include <android/os/IServiceManager.h> +#include <binder/IBinder.h> +#include <binder/IPCThreadState.h> +#include <binder/IServiceManager.h> +#include <binder/IServiceManagerUnitTestHelper.h> +#include "fakeservicemanager/FakeServiceManager.h" + +#include <sys/prctl.h> +#include <thread> + +using namespace android; + +#ifdef LIBBINDER_CLIENT_CACHE +constexpr bool kUseLibbinderCache = true; +#else +constexpr bool kUseLibbinderCache = false; +#endif + +// A service name which is in the static list of cachable services +const String16 kCachedServiceName = String16("isub"); + +#define EXPECT_OK(status) \ + do { \ + binder::Status stat = (status); \ + EXPECT_TRUE(stat.isOk()) << stat; \ + } while (false) + +const String16 kServerName = String16("binderCacheUnitTest"); + +class FooBar : public BBinder { +public: + status_t onTransact(uint32_t, const Parcel&, Parcel*, uint32_t) { + // exit the server + std::thread([] { exit(EXIT_FAILURE); }).detach(); + return OK; + } + void killServer(sp<IBinder> binder) { + Parcel data, reply; + binder->transact(0, data, &reply, 0); + } +}; + +class MockAidlServiceManager : public os::IServiceManagerDefault { +public: + MockAidlServiceManager() : innerSm() {} + + binder::Status checkService(const ::std::string& name, os::Service* _out) override { + sp<IBinder> binder = innerSm.getService(String16(name.c_str())); + *_out = os::Service::make<os::Service::Tag::binder>(binder); + return binder::Status::ok(); + } + + binder::Status addService(const std::string& name, const sp<IBinder>& service, + bool allowIsolated, int32_t dumpPriority) override { + return binder::Status::fromStatusT( + innerSm.addService(String16(name.c_str()), service, allowIsolated, dumpPriority)); + } + + FakeServiceManager innerSm; +}; + +class LibbinderCacheTest : public ::testing::Test { +protected: + void SetUp() override { + sp<MockAidlServiceManager> sm = sp<MockAidlServiceManager>::make(); + mServiceManager = getServiceManagerShimFromAidlServiceManagerForTests(sm); + } + + void TearDown() override {} + +public: + void cacheAndConfirmCacheHit(const sp<IBinder>& binder1, const sp<IBinder>& binder2) { + // Add a service + EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder1)); + // Get the service. This caches it. + sp<IBinder> result = mServiceManager->checkService(kCachedServiceName); + ASSERT_EQ(binder1, result); + + // Add the different binder and replace the service. + // The cache should still hold the original binder. + EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder2)); + + result = mServiceManager->checkService(kCachedServiceName); + if (kUseLibbinderCache) { + // If cache is enabled, we should get the binder to Service Manager. + EXPECT_EQ(binder1, result); + } else { + // If cache is disabled, then we should get the newer binder + EXPECT_EQ(binder2, result); + } + } + + sp<android::IServiceManager> mServiceManager; +}; + +TEST_F(LibbinderCacheTest, AddLocalServiceAndConfirmCacheHit) { + sp<IBinder> binder1 = sp<BBinder>::make(); + sp<IBinder> binder2 = sp<BBinder>::make(); + + cacheAndConfirmCacheHit(binder1, binder2); +} + +TEST_F(LibbinderCacheTest, AddRemoteServiceAndConfirmCacheHit) { + sp<IBinder> binder1 = defaultServiceManager()->checkService(kServerName); + ASSERT_NE(binder1, nullptr); + sp<IBinder> binder2 = IInterface::asBinder(mServiceManager); + + cacheAndConfirmCacheHit(binder1, binder2); +} + +TEST_F(LibbinderCacheTest, RemoveFromCacheOnServerDeath) { + sp<IBinder> binder1 = defaultServiceManager()->checkService(kServerName); + FooBar foo = FooBar(); + + EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder1)); + + // Check Service, this caches the binder + sp<IBinder> result = mServiceManager->checkService(kCachedServiceName); + ASSERT_EQ(binder1, result); + + // Kill the server, this should remove from cache. + foo.killServer(binder1); + pid_t pid; + ASSERT_EQ(OK, binder1->getDebugPid(&pid)); + system(("kill -9 " + std::to_string(pid)).c_str()); + + sp<IBinder> binder2 = sp<BBinder>::make(); + + // Add new service with the same name. + // This will replace the service in FakeServiceManager. + EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder2)); + + // Confirm that new service is returned instead of old. + sp<IBinder> result2 = mServiceManager->checkService(kCachedServiceName); + ASSERT_EQ(binder2, result2); +} + +TEST_F(LibbinderCacheTest, NullBinderNotCached) { + sp<IBinder> binder1 = nullptr; + sp<IBinder> binder2 = sp<BBinder>::make(); + + // Check for a cacheble service which isn't registered. + // FakeServiceManager should return nullptr. + // This shouldn't be cached. + sp<IBinder> result = mServiceManager->checkService(kCachedServiceName); + ASSERT_EQ(binder1, result); + + // Add the same service + EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder2)); + + // This should return the newly added service. + result = mServiceManager->checkService(kCachedServiceName); + EXPECT_EQ(binder2, result); +} + +TEST_F(LibbinderCacheTest, DoNotCacheServiceNotInList) { + sp<IBinder> binder1 = sp<BBinder>::make(); + sp<IBinder> binder2 = sp<BBinder>::make(); + String16 serviceName = String16("NewLibbinderCacheTest"); + // Add a service + EXPECT_EQ(OK, mServiceManager->addService(serviceName, binder1)); + // Get the service. This shouldn't caches it. + sp<IBinder> result = mServiceManager->checkService(serviceName); + ASSERT_EQ(binder1, result); + + // Add the different binder and replace the service. + EXPECT_EQ(OK, mServiceManager->addService(serviceName, binder2)); + + // Confirm that we get the new service + result = mServiceManager->checkService(serviceName); + EXPECT_EQ(binder2, result); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + + if (fork() == 0) { + prctl(PR_SET_PDEATHSIG, SIGHUP); + + // Start a FooBar service and add it to the servicemanager. + sp<IBinder> server = new FooBar(); + defaultServiceManager()->addService(kServerName, server); + + IPCThreadState::self()->joinThreadPool(true); + exit(1); // should not reach + } + + status_t err = ProcessState::self()->setThreadPoolMaxThreadCount(3); + ProcessState::self()->startThreadPool(); + CHECK_EQ(ProcessState::self()->isThreadPoolStarted(), true); + CHECK_GT(ProcessState::self()->getThreadPoolMaxTotalThreadCount(), 0); + + auto binder = defaultServiceManager()->waitForService(kServerName); + CHECK_NE(nullptr, binder.get()); + return RUN_ALL_TESTS(); +} |