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

11 files changed, 200 insertions, 20 deletions

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");