diff options
author | 2024-11-29 10:40:41 +0000 | |
---|---|---|
committer | 2024-12-03 17:46:33 +0000 | |
commit | 5e1b7e1b0a23a47ab76ed022284fd40b67ebe503 (patch) | |
tree | d3a7bb860c95427b32ea175c35ea235847925d94 | |
parent | dc207544897a30fe412ffcc0deab07dd974358d1 (diff) |
Remove static list in libbinder for caching
This adds a new flag to identify Lazy services.
This flag is set when addService is called from
LazyServiceRegistrar.
This flag is then used by libbinder in a client.
When getService is called, libbinder decides if
the binder should be cached or not using this flag.
Doc: go/libbinder-cache-v2
Flag: RELEASE_LIBBINDER_REMOVE_STATIC_LIST
Test: atest binderCacheUnitTest
Bug: 333854840
Change-Id: I5fff0140f635dddb4f5a6d618f4e9abace6feb54
-rw-r--r-- | cmds/servicemanager/ServiceManager.cpp | 21 | ||||
-rw-r--r-- | cmds/servicemanager/ServiceManager.h | 2 | ||||
-rw-r--r-- | cmds/servicemanager/test_sm.cpp | 19 | ||||
-rw-r--r-- | libs/binder/Android.bp | 24 | ||||
-rw-r--r-- | libs/binder/BackendUnifiedServiceManager.cpp | 51 | ||||
-rw-r--r-- | libs/binder/BackendUnifiedServiceManager.h | 3 | ||||
-rw-r--r-- | libs/binder/IServiceManager.cpp | 5 | ||||
-rw-r--r-- | libs/binder/LazyServiceRegistrar.cpp | 7 | ||||
-rw-r--r-- | libs/binder/aidl/android/os/IServiceManager.aidl | 5 | ||||
-rw-r--r-- | libs/binder/aidl/android/os/Service.aidl | 4 | ||||
-rw-r--r-- | libs/binder/aidl/android/os/ServiceWithMetadata.aidl | 32 | ||||
-rw-r--r-- | libs/binder/ndk/include_platform/android/binder_manager.h | 1 | ||||
-rw-r--r-- | libs/binder/tests/Android.bp | 1 | ||||
-rw-r--r-- | libs/binder/tests/binderCacheUnitTest.cpp | 87 |
14 files changed, 226 insertions, 36 deletions
diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp index fa7cb64f3a..38a125bb54 100644 --- a/cmds/servicemanager/ServiceManager.cpp +++ b/cmds/servicemanager/ServiceManager.cpp @@ -396,7 +396,7 @@ Status ServiceManager::getService(const std::string& name, sp<IBinder>* outBinde SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); - *outBinder = tryGetBinder(name, true); + *outBinder = tryGetBinder(name, true).service; // returns ok regardless of result for legacy reasons return Status::ok(); } @@ -430,13 +430,15 @@ os::Service ServiceManager::tryGetService(const std::string& name, bool startIfN return os::Service::make<os::Service::Tag::accessor>(nullptr); } return os::Service::make<os::Service::Tag::accessor>( - tryGetBinder(*accessorName, startIfNotFound)); + tryGetBinder(*accessorName, startIfNotFound).service); } else { - return os::Service::make<os::Service::Tag::binder>(tryGetBinder(name, startIfNotFound)); + return os::Service::make<os::Service::Tag::serviceWithMetadata>( + tryGetBinder(name, startIfNotFound)); } } -sp<IBinder> ServiceManager::tryGetBinder(const std::string& name, bool startIfNotFound) { +os::ServiceWithMetadata ServiceManager::tryGetBinder(const std::string& name, + bool startIfNotFound) { SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); @@ -450,13 +452,13 @@ sp<IBinder> ServiceManager::tryGetBinder(const std::string& name, bool startIfNo if (!service->allowIsolated && is_multiuser_uid_isolated(ctx.uid)) { LOG(WARNING) << "Isolated app with UID " << ctx.uid << " requested '" << name << "', but the service is not allowed for isolated apps."; - return nullptr; + return os::ServiceWithMetadata(); } out = service->binder; } if (!mAccess->canFind(ctx, name)) { - return nullptr; + return os::ServiceWithMetadata(); } if (!out && startIfNotFound) { @@ -473,8 +475,11 @@ sp<IBinder> ServiceManager::tryGetBinder(const std::string& name, bool startIfNo CHECK(handleServiceClientCallback(2 /* sm + transaction */, name, false)); service->guaranteeClient = true; } - - return out; + os::ServiceWithMetadata serviceWithMetadata = os::ServiceWithMetadata(); + serviceWithMetadata.service = out; + serviceWithMetadata.isLazyService = + service ? service->dumpPriority & FLAG_IS_LAZY_SERVICE : false; + return serviceWithMetadata; } bool isValidServiceName(const std::string& name) { diff --git a/cmds/servicemanager/ServiceManager.h b/cmds/servicemanager/ServiceManager.h index c92141b393..964abee6af 100644 --- a/cmds/servicemanager/ServiceManager.h +++ b/cmds/servicemanager/ServiceManager.h @@ -114,7 +114,7 @@ private: void removeClientCallback(const wp<IBinder>& who, ClientCallbackMap::iterator* it); os::Service tryGetService(const std::string& name, bool startIfNotFound); - sp<IBinder> tryGetBinder(const std::string& name, bool startIfNotFound); + os::ServiceWithMetadata tryGetBinder(const std::string& name, bool startIfNotFound); binder::Status canAddService(const Access::CallingContext& ctx, const std::string& name, std::optional<std::string>* accessor); binder::Status canFindService(const Access::CallingContext& ctx, const std::string& name, diff --git a/cmds/servicemanager/test_sm.cpp b/cmds/servicemanager/test_sm.cpp index 95f459f7f0..e620770e18 100644 --- a/cmds/servicemanager/test_sm.cpp +++ b/cmds/servicemanager/test_sm.cpp @@ -92,6 +92,11 @@ TEST(AddService, HappyHappy) { auto sm = getPermissiveServiceManager(); EXPECT_TRUE(sm->addService("foo", getBinder(), false /*allowIsolated*/, IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk()); + + EXPECT_TRUE(sm->addService("lazyfoo", getBinder(), false /*allowIsolated*/, + IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT | + IServiceManager::FLAG_IS_LAZY_SERVICE) + .isOk()); } TEST(AddService, EmptyNameDisallowed) { @@ -156,7 +161,7 @@ TEST(AddService, OverwriteExistingService) { Service outA; EXPECT_TRUE(sm->getService2("foo", &outA).isOk()); - EXPECT_EQ(serviceA, outA.get<Service::Tag::binder>()); + EXPECT_EQ(serviceA, outA.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinderA; EXPECT_TRUE(sm->getService("foo", &outBinderA).isOk()); EXPECT_EQ(serviceA, outBinderA); @@ -168,7 +173,7 @@ TEST(AddService, OverwriteExistingService) { Service outB; EXPECT_TRUE(sm->getService2("foo", &outB).isOk()); - EXPECT_EQ(serviceB, outB.get<Service::Tag::binder>()); + EXPECT_EQ(serviceB, outB.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinderB; EXPECT_TRUE(sm->getService("foo", &outBinderB).isOk()); EXPECT_EQ(serviceB, outBinderB); @@ -195,7 +200,7 @@ TEST(GetService, HappyHappy) { Service out; EXPECT_TRUE(sm->getService2("foo", &out).isOk()); - EXPECT_EQ(service, out.get<Service::Tag::binder>()); + EXPECT_EQ(service, out.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinder; EXPECT_TRUE(sm->getService("foo", &outBinder).isOk()); EXPECT_EQ(service, outBinder); @@ -206,7 +211,7 @@ TEST(GetService, NonExistant) { Service out; EXPECT_TRUE(sm->getService2("foo", &out).isOk()); - EXPECT_EQ(nullptr, out.get<Service::Tag::binder>()); + EXPECT_EQ(nullptr, out.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinder; EXPECT_TRUE(sm->getService("foo", &outBinder).isOk()); EXPECT_EQ(nullptr, outBinder); @@ -227,7 +232,7 @@ TEST(GetService, NoPermissionsForGettingService) { Service out; // returns nullptr but has OK status for legacy compatibility EXPECT_TRUE(sm->getService2("foo", &out).isOk()); - EXPECT_EQ(nullptr, out.get<Service::Tag::binder>()); + EXPECT_EQ(nullptr, out.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinder; EXPECT_TRUE(sm->getService("foo", &outBinder).isOk()); EXPECT_EQ(nullptr, outBinder); @@ -257,7 +262,7 @@ TEST(GetService, AllowedFromIsolated) { Service out; EXPECT_TRUE(sm->getService2("foo", &out).isOk()); - EXPECT_EQ(service, out.get<Service::Tag::binder>()); + EXPECT_EQ(service, out.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinder; EXPECT_TRUE(sm->getService("foo", &outBinder).isOk()); EXPECT_EQ(service, outBinder); @@ -289,7 +294,7 @@ TEST(GetService, NotAllowedFromIsolated) { Service out; // returns nullptr but has OK status for legacy compatibility EXPECT_TRUE(sm->getService2("foo", &out).isOk()); - EXPECT_EQ(nullptr, out.get<Service::Tag::binder>()); + EXPECT_EQ(nullptr, out.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinder; EXPECT_TRUE(sm->getService("foo", &outBinder).isOk()); EXPECT_EQ(nullptr, outBinder); diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp index e10e4ddc23..aac369ddd1 100644 --- a/libs/binder/Android.bp +++ b/libs/binder/Android.bp @@ -456,6 +456,28 @@ cc_library_shared { } soong_config_module_type { + name: "libbinder_remove_cache_static_list_config", + module_type: "cc_defaults", + config_namespace: "libbinder", + bool_variables: ["release_libbinder_remove_cache_static_list"], + properties: [ + "cflags", + ], +} + +libbinder_remove_cache_static_list_config { + name: "libbinder_remove_cache_static_list_flag", + soong_config_variables: { + release_libbinder_remove_cache_static_list: { + cflags: ["-DLIBBINDER_REMOVE_CACHE_STATIC_LIST"], + conditions_default: { + cflags: ["-DNO_LIBBINDER_REMOVE_CACHE_STATIC_LIST"], + }, + }, + }, +} + +soong_config_module_type { name: "libbinder_client_cache_config", module_type: "cc_defaults", config_namespace: "libbinder", @@ -504,6 +526,7 @@ cc_defaults { defaults: [ "libbinder_client_cache_flag", "libbinder_addservice_cache_flag", + "libbinder_remove_cache_static_list_flag", ], srcs: [ "BufferedTextOutput.cpp", @@ -826,6 +849,7 @@ filegroup { "aidl/android/os/IServiceCallback.aidl", "aidl/android/os/IServiceManager.aidl", "aidl/android/os/Service.aidl", + "aidl/android/os/ServiceWithMetadata.aidl", "aidl/android/os/ServiceDebugInfo.aidl", ], path: "aidl", diff --git a/libs/binder/BackendUnifiedServiceManager.cpp b/libs/binder/BackendUnifiedServiceManager.cpp index 123dfd2c05..03d687a9cc 100644 --- a/libs/binder/BackendUnifiedServiceManager.cpp +++ b/libs/binder/BackendUnifiedServiceManager.cpp @@ -16,6 +16,7 @@ #include "BackendUnifiedServiceManager.h" #include <android/os/IAccessor.h> +#include <android/os/IServiceManager.h> #include <binder/RpcSession.h> #if defined(__BIONIC__) && !defined(__ANDROID_VNDK__) @@ -36,6 +37,12 @@ constexpr bool kUseCacheInAddService = true; constexpr bool kUseCacheInAddService = false; #endif +#ifdef LIBBINDER_REMOVE_CACHE_STATIC_LIST +constexpr bool kRemoveStaticList = true; +#else +constexpr bool kRemoveStaticList = false; +#endif + using AidlServiceManager = android::os::IServiceManager; using android::os::IAccessor; using binder::Status; @@ -110,6 +117,13 @@ static const char* kStaticCachableList[] = { // go/keep-sorted end }; +os::ServiceWithMetadata createServiceWithMetadata(const sp<IBinder>& service, bool isLazyService) { + os::ServiceWithMetadata out = os::ServiceWithMetadata(); + out.service = service; + out.isLazyService = isLazyService; + return out; +} + bool BinderCacheWithInvalidation::isClientSideCachingEnabled(const std::string& serviceName) { sp<ProcessState> self = ProcessState::selfOrNull(); if (!self || self->getThreadPoolMaxTotalThreadCount() <= 0) { @@ -132,15 +146,21 @@ Status BackendUnifiedServiceManager::updateCache(const std::string& serviceName, return Status::ok(); } - if (service.getTag() == os::Service::Tag::binder) { - return updateCache(serviceName, service.get<os::Service::Tag::binder>()); + if (service.getTag() == os::Service::Tag::serviceWithMetadata) { + auto serviceWithMetadata = service.get<os::Service::Tag::serviceWithMetadata>(); + return updateCache(serviceName, serviceWithMetadata.service, + serviceWithMetadata.isLazyService); } return Status::ok(); } Status BackendUnifiedServiceManager::updateCache(const std::string& serviceName, - const sp<IBinder>& binder) { + const sp<IBinder>& binder, bool isServiceLazy) { std::string traceStr; + // Don't cache if service is lazy + if (kRemoveStaticList && isServiceLazy) { + return Status::ok(); + } if (atrace_is_tag_enabled(ATRACE_TAG_AIDL)) { traceStr = "BinderCacheWithInvalidation::updateCache : " + serviceName; } @@ -153,7 +173,9 @@ Status BackendUnifiedServiceManager::updateCache(const std::string& serviceName, binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL, "BinderCacheWithInvalidation::updateCache failed: " "isBinderAlive_false"); - } else if (mCacheForGetService->isClientSideCachingEnabled(serviceName)) { + } + // If we reach here with kRemoveStaticList=true then we know service isn't lazy + else if (kRemoveStaticList || mCacheForGetService->isClientSideCachingEnabled(serviceName)) { binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL, "BinderCacheWithInvalidation::updateCache successful"); return mCacheForGetService->setItem(serviceName, binder); @@ -173,7 +195,7 @@ bool BackendUnifiedServiceManager::returnIfCached(const std::string& serviceName 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); + *_out = createServiceWithMetadata(item, false); return true; } return false; @@ -188,7 +210,7 @@ Status BackendUnifiedServiceManager::getService(const ::std::string& name, sp<IBinder>* _aidl_return) { os::Service service; Status status = getService2(name, &service); - *_aidl_return = service.get<os::Service::Tag::binder>(); + *_aidl_return = service.get<os::Service::Tag::serviceWithMetadata>().service; return status; } @@ -227,14 +249,16 @@ Status BackendUnifiedServiceManager::checkService(const ::std::string& name, os: Status BackendUnifiedServiceManager::toBinderService(const ::std::string& name, const os::Service& in, os::Service* _out) { switch (in.getTag()) { - case os::Service::Tag::binder: { - if (in.get<os::Service::Tag::binder>() == nullptr) { + case os::Service::Tag::serviceWithMetadata: { + auto serviceWithMetadata = in.get<os::Service::Tag::serviceWithMetadata>(); + if (serviceWithMetadata.service == nullptr) { // failed to find a service. Check to see if we have any local // injected Accessors for this service. os::Service accessor; Status status = getInjectedAccessor(name, &accessor); if (!status.isOk()) { - *_out = os::Service::make<os::Service::Tag::binder>(nullptr); + *_out = os::Service::make<os::Service::Tag::serviceWithMetadata>( + createServiceWithMetadata(nullptr, false)); return status; } if (accessor.getTag() == os::Service::Tag::accessor && @@ -255,7 +279,8 @@ Status BackendUnifiedServiceManager::toBinderService(const ::std::string& name, sp<IAccessor> accessor = interface_cast<IAccessor>(accessorBinder); if (accessor == nullptr) { ALOGE("Service#accessor doesn't have accessor. VM is maybe starting..."); - *_out = os::Service::make<os::Service::Tag::binder>(nullptr); + *_out = os::Service::make<os::Service::Tag::serviceWithMetadata>( + createServiceWithMetadata(nullptr, false)); return Status::ok(); } auto request = [=] { @@ -276,7 +301,8 @@ Status BackendUnifiedServiceManager::toBinderService(const ::std::string& name, return Status::fromStatusT(status); } session->setSessionSpecificRoot(accessorBinder); - *_out = os::Service::make<os::Service::Tag::binder>(session->getRootObject()); + *_out = os::Service::make<os::Service::Tag::serviceWithMetadata>( + createServiceWithMetadata(session->getRootObject(), false)); return Status::ok(); } default: { @@ -291,7 +317,8 @@ Status BackendUnifiedServiceManager::addService(const ::std::string& name, Status status = mTheRealServiceManager->addService(name, service, allowIsolated, dumpPriority); // mEnableAddServiceCache is true by default. if (kUseCacheInAddService && mEnableAddServiceCache && status.isOk()) { - return updateCache(name, service); + return updateCache(name, service, + dumpPriority & android::os::IServiceManager::FLAG_IS_LAZY_SERVICE); } return status; } diff --git a/libs/binder/BackendUnifiedServiceManager.h b/libs/binder/BackendUnifiedServiceManager.h index b46bf194a1..6a0d06a079 100644 --- a/libs/binder/BackendUnifiedServiceManager.h +++ b/libs/binder/BackendUnifiedServiceManager.h @@ -159,7 +159,8 @@ private: 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); - binder::Status updateCache(const std::string& serviceName, const sp<IBinder>& binder); + binder::Status updateCache(const std::string& serviceName, const sp<IBinder>& binder, + bool isLazyService); bool returnIfCached(const std::string& serviceName, os::Service* _out); }; diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp index 61866d70d3..53435c357b 100644 --- a/libs/binder/IServiceManager.cpp +++ b/libs/binder/IServiceManager.cpp @@ -152,7 +152,8 @@ protected: virtual Status realGetService(const std::string& name, sp<IBinder>* _aidl_return) { Service service; Status status = mUnifiedServiceManager->getService2(name, &service); - *_aidl_return = service.get<Service::Tag::binder>(); + auto serviceWithMetadata = service.get<Service::Tag::serviceWithMetadata>(); + *_aidl_return = serviceWithMetadata.service; return status; } }; @@ -607,7 +608,7 @@ sp<IBinder> CppBackendShim::checkService(const String16& name) const { if (!mUnifiedServiceManager->checkService(String8(name).c_str(), &ret).isOk()) { return nullptr; } - return ret.get<Service::Tag::binder>(); + return ret.get<Service::Tag::serviceWithMetadata>().service; } status_t CppBackendShim::addService(const String16& name, const sp<IBinder>& service, diff --git a/libs/binder/LazyServiceRegistrar.cpp b/libs/binder/LazyServiceRegistrar.cpp index 0f0af0b142..27cfe0cf21 100644 --- a/libs/binder/LazyServiceRegistrar.cpp +++ b/libs/binder/LazyServiceRegistrar.cpp @@ -19,7 +19,6 @@ #include <android/os/BnClientCallback.h> #include <android/os/IServiceManager.h> #include <binder/IPCThreadState.h> -#include <binder/IServiceManager.h> #include <binder/LazyServiceRegistrar.h> namespace android { @@ -134,6 +133,12 @@ bool ClientCounterCallbackImpl::registerServiceLocked(const sp<IBinder>& service std::string regStr = (reRegister) ? "Re-registering" : "Registering"; ALOGI("%s service %s", regStr.c_str(), name.c_str()); + if (dumpFlags & android::os::IServiceManager::FLAG_IS_LAZY_SERVICE) { + ALOGW("FLAG_IS_LAZY_SERVICE flag already set. This should only be set by " + "ClientCounterCallbackImpl in LazyServiceRegistrar"); + } + dumpFlags |= android::os::IServiceManager::FLAG_IS_LAZY_SERVICE; + if (Status status = manager->addService(name.c_str(), service, allowIsolated, dumpFlags); !status.isOk()) { ALOGE("Failed to register service %s (%s)", name.c_str(), status.toString8().c_str()); diff --git a/libs/binder/aidl/android/os/IServiceManager.aidl b/libs/binder/aidl/android/os/IServiceManager.aidl index 1d1f84fc01..69edef86aa 100644 --- a/libs/binder/aidl/android/os/IServiceManager.aidl +++ b/libs/binder/aidl/android/os/IServiceManager.aidl @@ -49,6 +49,8 @@ interface IServiceManager { DUMP_FLAG_PRIORITY_CRITICAL | DUMP_FLAG_PRIORITY_HIGH | DUMP_FLAG_PRIORITY_NORMAL | DUMP_FLAG_PRIORITY_DEFAULT; + const int FLAG_IS_LAZY_SERVICE = 1 << 30; + /* Allows services to dump sections in protobuf format. */ const int DUMP_FLAG_PROTO = 1 << 4; @@ -61,7 +63,8 @@ interface IServiceManager { * * Returns null if the service does not exist. * - * @deprecated TODO(b/355394904): Use getService2 instead. + * @deprecated TODO(b/355394904): Use getService2 instead. This does not return metadata + * that is included in ServiceWithMetadata */ @UnsupportedAppUsage @nullable IBinder getService(@utf8InCpp String name); diff --git a/libs/binder/aidl/android/os/Service.aidl b/libs/binder/aidl/android/os/Service.aidl index 4c5210911c..3bc658833b 100644 --- a/libs/binder/aidl/android/os/Service.aidl +++ b/libs/binder/aidl/android/os/Service.aidl @@ -16,6 +16,8 @@ package android.os; +import android.os.ServiceWithMetadata; + /** * Service is a union of different service types that can be returned * by the internal {@link ServiceManager#getService(name)} API. @@ -23,6 +25,6 @@ package android.os; * @hide */ union Service { - @nullable IBinder binder; + ServiceWithMetadata serviceWithMetadata; @nullable IBinder accessor; }
\ No newline at end of file diff --git a/libs/binder/aidl/android/os/ServiceWithMetadata.aidl b/libs/binder/aidl/android/os/ServiceWithMetadata.aidl new file mode 100644 index 0000000000..96f76ffe78 --- /dev/null +++ b/libs/binder/aidl/android/os/ServiceWithMetadata.aidl @@ -0,0 +1,32 @@ +/* + * 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. + */ + +package android.os; + +/** + * Service binder with metadata. + * @hide + */ +parcelable ServiceWithMetadata { + /** + * IBinder to service + */ + @nullable IBinder service; + /** + * boolean if the IBinder can be cached by client. + */ + boolean isLazyService; +} diff --git a/libs/binder/ndk/include_platform/android/binder_manager.h b/libs/binder/ndk/include_platform/android/binder_manager.h index cc4943b9c3..f5df8d5c2f 100644 --- a/libs/binder/ndk/include_platform/android/binder_manager.h +++ b/libs/binder/ndk/include_platform/android/binder_manager.h @@ -34,6 +34,7 @@ enum AServiceManager_AddServiceFlag : uint32_t { ADD_SERVICE_DUMP_FLAG_PRIORITY_HIGH = 1 << 2, ADD_SERVICE_DUMP_FLAG_PRIORITY_NORMAL = 1 << 3, ADD_SERVICE_DUMP_FLAG_PRIORITY_DEFAULT = 1 << 4, + // All other bits are reserved for internal usage }; /** diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp index 642fbf3b56..c21d7c61cd 100644 --- a/libs/binder/tests/Android.bp +++ b/libs/binder/tests/Android.bp @@ -73,6 +73,7 @@ cc_test { defaults: [ "libbinder_client_cache_flag", "libbinder_addservice_cache_flag", + "libbinder_remove_cache_static_list_flag", ], test_suites: ["general-tests"], require_root: true, diff --git a/libs/binder/tests/binderCacheUnitTest.cpp b/libs/binder/tests/binderCacheUnitTest.cpp index 3195c55b0b..19395c2dcd 100644 --- a/libs/binder/tests/binderCacheUnitTest.cpp +++ b/libs/binder/tests/binderCacheUnitTest.cpp @@ -40,6 +40,12 @@ constexpr bool kUseCacheInAddService = true; constexpr bool kUseCacheInAddService = false; #endif +#ifdef LIBBINDER_REMOVE_CACHE_STATIC_LIST +constexpr bool kRemoveStaticList = true; +#else +constexpr bool kRemoveStaticList = false; +#endif + // A service name which is in the static list of cachable services const String16 kCachedServiceName = String16("isub"); @@ -69,8 +75,10 @@ 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); + os::ServiceWithMetadata serviceWithMetadata = os::ServiceWithMetadata(); + serviceWithMetadata.service = innerSm.getService(String16(name.c_str())); + serviceWithMetadata.isLazyService = false; + *_out = os::Service::make<os::Service::Tag::serviceWithMetadata>(serviceWithMetadata); return binder::Status::ok(); } @@ -85,6 +93,75 @@ public: FakeServiceManager innerSm; }; +// Returns services with isLazyService flag as true. +class MockAidlServiceManager2 : public os::IServiceManagerDefault { +public: + MockAidlServiceManager2() : innerSm() {} + + binder::Status checkService(const ::std::string& name, os::Service* _out) override { + os::ServiceWithMetadata serviceWithMetadata = os::ServiceWithMetadata(); + serviceWithMetadata.service = innerSm.getService(String16(name.c_str())); + serviceWithMetadata.isLazyService = true; + *_out = os::Service::make<os::Service::Tag::serviceWithMetadata>(serviceWithMetadata); + 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)); + } + + void clearServices() { innerSm.clear(); } + + FakeServiceManager innerSm; +}; + +class LibbinderCacheRemoveStaticList : public ::testing::Test { +protected: + void SetUp() override { + fakeServiceManager = sp<MockAidlServiceManager2>::make(); + mServiceManager = getServiceManagerShimFromAidlServiceManagerForTests(fakeServiceManager); + mServiceManager->enableAddServiceCache(true); + } + void TearDown() override {} + +public: + void cacheAddServiceAndConfirmCacheMiss(const sp<IBinder>& binder1) { + // Add a service. This shouldn't cache it. + EXPECT_EQ(OK, + mServiceManager->addService(kCachedServiceName, binder1, + /*allowIsolated = */ false, + android::os::IServiceManager::FLAG_IS_LAZY_SERVICE)); + // Try to populate cache. Cache shouldn't be updated. + EXPECT_EQ(binder1, mServiceManager->checkService(kCachedServiceName)); + fakeServiceManager->clearServices(); + EXPECT_EQ(nullptr, mServiceManager->checkService(kCachedServiceName)); + } + + sp<MockAidlServiceManager2> fakeServiceManager; + sp<android::IServiceManager> mServiceManager; +}; + +TEST_F(LibbinderCacheRemoveStaticList, AddLocalServiceAndConfirmCacheMiss) { + if (!kRemoveStaticList) { + GTEST_SKIP() << "Skipping as feature is not enabled"; + return; + } + sp<IBinder> binder1 = sp<BBinder>::make(); + cacheAddServiceAndConfirmCacheMiss(binder1); +} + +TEST_F(LibbinderCacheRemoveStaticList, AddRemoteServiceAndConfirmCacheMiss) { + if (!kRemoveStaticList) { + GTEST_SKIP() << "Skipping as feature is not enabled"; + return; + } + sp<IBinder> binder1 = defaultServiceManager()->checkService(kServerName); + ASSERT_NE(binder1, nullptr); + cacheAddServiceAndConfirmCacheMiss(binder1); +} + class LibbinderCacheAddServiceTest : public ::testing::Test { protected: void SetUp() override { @@ -231,7 +308,13 @@ TEST_F(LibbinderCacheTest, NullBinderNotCached) { EXPECT_EQ(binder2, result); } +// TODO(b/333854840): Remove this test removing the static list TEST_F(LibbinderCacheTest, DoNotCacheServiceNotInList) { + if (kRemoveStaticList) { + GTEST_SKIP() << "Skipping test as static list is disabled"; + return; + } + sp<IBinder> binder1 = sp<BBinder>::make(); sp<IBinder> binder2 = sp<BBinder>::make(); String16 serviceName = String16("NewLibbinderCacheTest"); |