Skip to content

Commit 186bccd

Browse files
Treehugger Robotandroid-build-merge-worker-robot
Treehugger Robot
authored and
android-build-merge-worker-robot
committed
Merge "Remove static list in libbinder for caching" into main am: 6dc6700
Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/3380036 Change-Id: Ic2e273b0123034f30e1f97f0e8eca9abd1c35501 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
2 parents a5d4f31 + 6dc6700 commit 186bccd

14 files changed

Lines changed: 226 additions & 36 deletions

‎cmds/servicemanager/ServiceManager.cpp‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ Status ServiceManager::getService(const std::string& name, sp<IBinder>* outBinde
396396
SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS(
397397
PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str())));
398398

399-
*outBinder = tryGetBinder(name, true);
399+
*outBinder = tryGetBinder(name, true).service;
400400
// returns ok regardless of result for legacy reasons
401401
return Status::ok();
402402
}
@@ -430,13 +430,15 @@ os::Service ServiceManager::tryGetService(const std::string& name, bool startIfN
430430
return os::Service::make<os::Service::Tag::accessor>(nullptr);
431431
}
432432
return os::Service::make<os::Service::Tag::accessor>(
433-
tryGetBinder(*accessorName, startIfNotFound));
433+
tryGetBinder(*accessorName, startIfNotFound).service);
434434
} else {
435-
return os::Service::make<os::Service::Tag::binder>(tryGetBinder(name, startIfNotFound));
435+
return os::Service::make<os::Service::Tag::serviceWithMetadata>(
436+
tryGetBinder(name, startIfNotFound));
436437
}
437438
}
438439

439-
sp<IBinder> ServiceManager::tryGetBinder(const std::string& name, bool startIfNotFound) {
440+
os::ServiceWithMetadata ServiceManager::tryGetBinder(const std::string& name,
441+
bool startIfNotFound) {
440442
SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS(
441443
PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str())));
442444

@@ -450,13 +452,13 @@ sp<IBinder> ServiceManager::tryGetBinder(const std::string& name, bool startIfNo
450452
if (!service->allowIsolated && is_multiuser_uid_isolated(ctx.uid)) {
451453
LOG(WARNING) << "Isolated app with UID " << ctx.uid << " requested '" << name
452454
<< "', but the service is not allowed for isolated apps.";
453-
return nullptr;
455+
return os::ServiceWithMetadata();
454456
}
455457
out = service->binder;
456458
}
457459

458460
if (!mAccess->canFind(ctx, name)) {
459-
return nullptr;
461+
return os::ServiceWithMetadata();
460462
}
461463

462464
if (!out && startIfNotFound) {
@@ -473,8 +475,11 @@ sp<IBinder> ServiceManager::tryGetBinder(const std::string& name, bool startIfNo
473475
CHECK(handleServiceClientCallback(2 /* sm + transaction */, name, false));
474476
service->guaranteeClient = true;
475477
}
476-
477-
return out;
478+
os::ServiceWithMetadata serviceWithMetadata = os::ServiceWithMetadata();
479+
serviceWithMetadata.service = out;
480+
serviceWithMetadata.isLazyService =
481+
service ? service->dumpPriority & FLAG_IS_LAZY_SERVICE : false;
482+
return serviceWithMetadata;
478483
}
479484

480485
bool isValidServiceName(const std::string& name) {

‎cmds/servicemanager/ServiceManager.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ class ServiceManager : public os::BnServiceManager, public IBinder::DeathRecipie
114114
void removeClientCallback(const wp<IBinder>& who, ClientCallbackMap::iterator* it);
115115

116116
os::Service tryGetService(const std::string& name, bool startIfNotFound);
117-
sp<IBinder> tryGetBinder(const std::string& name, bool startIfNotFound);
117+
os::ServiceWithMetadata tryGetBinder(const std::string& name, bool startIfNotFound);
118118
binder::Status canAddService(const Access::CallingContext& ctx, const std::string& name,
119119
std::optional<std::string>* accessor);
120120
binder::Status canFindService(const Access::CallingContext& ctx, const std::string& name,

‎cmds/servicemanager/test_sm.cpp‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ TEST(AddService, HappyHappy) {
9292
auto sm = getPermissiveServiceManager();
9393
EXPECT_TRUE(sm->addService("foo", getBinder(), false /*allowIsolated*/,
9494
IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
95+
96+
EXPECT_TRUE(sm->addService("lazyfoo", getBinder(), false /*allowIsolated*/,
97+
IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT |
98+
IServiceManager::FLAG_IS_LAZY_SERVICE)
99+
.isOk());
95100
}
96101

97102
TEST(AddService, EmptyNameDisallowed) {
@@ -156,7 +161,7 @@ TEST(AddService, OverwriteExistingService) {
156161

157162
Service outA;
158163
EXPECT_TRUE(sm->getService2("foo", &outA).isOk());
159-
EXPECT_EQ(serviceA, outA.get<Service::Tag::binder>());
164+
EXPECT_EQ(serviceA, outA.get<Service::Tag::serviceWithMetadata>().service);
160165
sp<IBinder> outBinderA;
161166
EXPECT_TRUE(sm->getService("foo", &outBinderA).isOk());
162167
EXPECT_EQ(serviceA, outBinderA);
@@ -168,7 +173,7 @@ TEST(AddService, OverwriteExistingService) {
168173

169174
Service outB;
170175
EXPECT_TRUE(sm->getService2("foo", &outB).isOk());
171-
EXPECT_EQ(serviceB, outB.get<Service::Tag::binder>());
176+
EXPECT_EQ(serviceB, outB.get<Service::Tag::serviceWithMetadata>().service);
172177
sp<IBinder> outBinderB;
173178
EXPECT_TRUE(sm->getService("foo", &outBinderB).isOk());
174179
EXPECT_EQ(serviceB, outBinderB);
@@ -195,7 +200,7 @@ TEST(GetService, HappyHappy) {
195200

196201
Service out;
197202
EXPECT_TRUE(sm->getService2("foo", &out).isOk());
198-
EXPECT_EQ(service, out.get<Service::Tag::binder>());
203+
EXPECT_EQ(service, out.get<Service::Tag::serviceWithMetadata>().service);
199204
sp<IBinder> outBinder;
200205
EXPECT_TRUE(sm->getService("foo", &outBinder).isOk());
201206
EXPECT_EQ(service, outBinder);
@@ -206,7 +211,7 @@ TEST(GetService, NonExistant) {
206211

207212
Service out;
208213
EXPECT_TRUE(sm->getService2("foo", &out).isOk());
209-
EXPECT_EQ(nullptr, out.get<Service::Tag::binder>());
214+
EXPECT_EQ(nullptr, out.get<Service::Tag::serviceWithMetadata>().service);
210215
sp<IBinder> outBinder;
211216
EXPECT_TRUE(sm->getService("foo", &outBinder).isOk());
212217
EXPECT_EQ(nullptr, outBinder);
@@ -227,7 +232,7 @@ TEST(GetService, NoPermissionsForGettingService) {
227232
Service out;
228233
// returns nullptr but has OK status for legacy compatibility
229234
EXPECT_TRUE(sm->getService2("foo", &out).isOk());
230-
EXPECT_EQ(nullptr, out.get<Service::Tag::binder>());
235+
EXPECT_EQ(nullptr, out.get<Service::Tag::serviceWithMetadata>().service);
231236
sp<IBinder> outBinder;
232237
EXPECT_TRUE(sm->getService("foo", &outBinder).isOk());
233238
EXPECT_EQ(nullptr, outBinder);
@@ -257,7 +262,7 @@ TEST(GetService, AllowedFromIsolated) {
257262

258263
Service out;
259264
EXPECT_TRUE(sm->getService2("foo", &out).isOk());
260-
EXPECT_EQ(service, out.get<Service::Tag::binder>());
265+
EXPECT_EQ(service, out.get<Service::Tag::serviceWithMetadata>().service);
261266
sp<IBinder> outBinder;
262267
EXPECT_TRUE(sm->getService("foo", &outBinder).isOk());
263268
EXPECT_EQ(service, outBinder);
@@ -289,7 +294,7 @@ TEST(GetService, NotAllowedFromIsolated) {
289294
Service out;
290295
// returns nullptr but has OK status for legacy compatibility
291296
EXPECT_TRUE(sm->getService2("foo", &out).isOk());
292-
EXPECT_EQ(nullptr, out.get<Service::Tag::binder>());
297+
EXPECT_EQ(nullptr, out.get<Service::Tag::serviceWithMetadata>().service);
293298
sp<IBinder> outBinder;
294299
EXPECT_TRUE(sm->getService("foo", &outBinder).isOk());
295300
EXPECT_EQ(nullptr, outBinder);

‎libs/binder/Android.bp‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,28 @@ cc_library_shared {
455455
],
456456
}
457457

458+
soong_config_module_type {
459+
name: "libbinder_remove_cache_static_list_config",
460+
module_type: "cc_defaults",
461+
config_namespace: "libbinder",
462+
bool_variables: ["release_libbinder_remove_cache_static_list"],
463+
properties: [
464+
"cflags",
465+
],
466+
}
467+
468+
libbinder_remove_cache_static_list_config {
469+
name: "libbinder_remove_cache_static_list_flag",
470+
soong_config_variables: {
471+
release_libbinder_remove_cache_static_list: {
472+
cflags: ["-DLIBBINDER_REMOVE_CACHE_STATIC_LIST"],
473+
conditions_default: {
474+
cflags: ["-DNO_LIBBINDER_REMOVE_CACHE_STATIC_LIST"],
475+
},
476+
},
477+
},
478+
}
479+
458480
soong_config_module_type {
459481
name: "libbinder_client_cache_config",
460482
module_type: "cc_defaults",
@@ -504,6 +526,7 @@ cc_defaults {
504526
defaults: [
505527
"libbinder_client_cache_flag",
506528
"libbinder_addservice_cache_flag",
529+
"libbinder_remove_cache_static_list_flag",
507530
],
508531
srcs: [
509532
"BufferedTextOutput.cpp",
@@ -826,6 +849,7 @@ filegroup {
826849
"aidl/android/os/IServiceCallback.aidl",
827850
"aidl/android/os/IServiceManager.aidl",
828851
"aidl/android/os/Service.aidl",
852+
"aidl/android/os/ServiceWithMetadata.aidl",
829853
"aidl/android/os/ServiceDebugInfo.aidl",
830854
],
831855
path: "aidl",

‎libs/binder/BackendUnifiedServiceManager.cpp‎

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "BackendUnifiedServiceManager.h"
1717

1818
#include <android/os/IAccessor.h>
19+
#include <android/os/IServiceManager.h>
1920
#include <binder/RpcSession.h>
2021

2122
#if defined(__BIONIC__) && !defined(__ANDROID_VNDK__)
@@ -36,6 +37,12 @@ constexpr bool kUseCacheInAddService = true;
3637
constexpr bool kUseCacheInAddService = false;
3738
#endif
3839

40+
#ifdef LIBBINDER_REMOVE_CACHE_STATIC_LIST
41+
constexpr bool kRemoveStaticList = true;
42+
#else
43+
constexpr bool kRemoveStaticList = false;
44+
#endif
45+
3946
using AidlServiceManager = android::os::IServiceManager;
4047
using android::os::IAccessor;
4148
using binder::Status;
@@ -110,6 +117,13 @@ static const char* kStaticCachableList[] = {
110117
// go/keep-sorted end
111118
};
112119

120+
os::ServiceWithMetadata createServiceWithMetadata(const sp<IBinder>& service, bool isLazyService) {
121+
os::ServiceWithMetadata out = os::ServiceWithMetadata();
122+
out.service = service;
123+
out.isLazyService = isLazyService;
124+
return out;
125+
}
126+
113127
bool BinderCacheWithInvalidation::isClientSideCachingEnabled(const std::string& serviceName) {
114128
sp<ProcessState> self = ProcessState::selfOrNull();
115129
if (!self || self->getThreadPoolMaxTotalThreadCount() <= 0) {
@@ -132,15 +146,21 @@ Status BackendUnifiedServiceManager::updateCache(const std::string& serviceName,
132146
return Status::ok();
133147
}
134148

135-
if (service.getTag() == os::Service::Tag::binder) {
136-
return updateCache(serviceName, service.get<os::Service::Tag::binder>());
149+
if (service.getTag() == os::Service::Tag::serviceWithMetadata) {
150+
auto serviceWithMetadata = service.get<os::Service::Tag::serviceWithMetadata>();
151+
return updateCache(serviceName, serviceWithMetadata.service,
152+
serviceWithMetadata.isLazyService);
137153
}
138154
return Status::ok();
139155
}
140156

141157
Status BackendUnifiedServiceManager::updateCache(const std::string& serviceName,
142-
const sp<IBinder>& binder) {
158+
const sp<IBinder>& binder, bool isServiceLazy) {
143159
std::string traceStr;
160+
// Don't cache if service is lazy
161+
if (kRemoveStaticList && isServiceLazy) {
162+
return Status::ok();
163+
}
144164
if (atrace_is_tag_enabled(ATRACE_TAG_AIDL)) {
145165
traceStr = "BinderCacheWithInvalidation::updateCache : " + serviceName;
146166
}
@@ -153,7 +173,9 @@ Status BackendUnifiedServiceManager::updateCache(const std::string& serviceName,
153173
binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
154174
"BinderCacheWithInvalidation::updateCache failed: "
155175
"isBinderAlive_false");
156-
} else if (mCacheForGetService->isClientSideCachingEnabled(serviceName)) {
176+
}
177+
// If we reach here with kRemoveStaticList=true then we know service isn't lazy
178+
else if (kRemoveStaticList || mCacheForGetService->isClientSideCachingEnabled(serviceName)) {
157179
binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
158180
"BinderCacheWithInvalidation::updateCache successful");
159181
return mCacheForGetService->setItem(serviceName, binder);
@@ -173,7 +195,7 @@ bool BackendUnifiedServiceManager::returnIfCached(const std::string& serviceName
173195
sp<IBinder> item = mCacheForGetService->getItem(serviceName);
174196
// TODO(b/363177618): Enable caching for binders which are always null.
175197
if (item != nullptr && item->isBinderAlive()) {
176-
*_out = os::Service::make<os::Service::Tag::binder>(item);
198+
*_out = createServiceWithMetadata(item, false);
177199
return true;
178200
}
179201
return false;
@@ -188,7 +210,7 @@ Status BackendUnifiedServiceManager::getService(const ::std::string& name,
188210
sp<IBinder>* _aidl_return) {
189211
os::Service service;
190212
Status status = getService2(name, &service);
191-
*_aidl_return = service.get<os::Service::Tag::binder>();
213+
*_aidl_return = service.get<os::Service::Tag::serviceWithMetadata>().service;
192214
return status;
193215
}
194216

@@ -227,14 +249,16 @@ Status BackendUnifiedServiceManager::checkService(const ::std::string& name, os:
227249
Status BackendUnifiedServiceManager::toBinderService(const ::std::string& name,
228250
const os::Service& in, os::Service* _out) {
229251
switch (in.getTag()) {
230-
case os::Service::Tag::binder: {
231-
if (in.get<os::Service::Tag::binder>() == nullptr) {
252+
case os::Service::Tag::serviceWithMetadata: {
253+
auto serviceWithMetadata = in.get<os::Service::Tag::serviceWithMetadata>();
254+
if (serviceWithMetadata.service == nullptr) {
232255
// failed to find a service. Check to see if we have any local
233256
// injected Accessors for this service.
234257
os::Service accessor;
235258
Status status = getInjectedAccessor(name, &accessor);
236259
if (!status.isOk()) {
237-
*_out = os::Service::make<os::Service::Tag::binder>(nullptr);
260+
*_out = os::Service::make<os::Service::Tag::serviceWithMetadata>(
261+
createServiceWithMetadata(nullptr, false));
238262
return status;
239263
}
240264
if (accessor.getTag() == os::Service::Tag::accessor &&
@@ -255,7 +279,8 @@ Status BackendUnifiedServiceManager::toBinderService(const ::std::string& name,
255279
sp<IAccessor> accessor = interface_cast<IAccessor>(accessorBinder);
256280
if (accessor == nullptr) {
257281
ALOGE("Service#accessor doesn't have accessor. VM is maybe starting...");
258-
*_out = os::Service::make<os::Service::Tag::binder>(nullptr);
282+
*_out = os::Service::make<os::Service::Tag::serviceWithMetadata>(
283+
createServiceWithMetadata(nullptr, false));
259284
return Status::ok();
260285
}
261286
auto request = [=] {
@@ -276,7 +301,8 @@ Status BackendUnifiedServiceManager::toBinderService(const ::std::string& name,
276301
return Status::fromStatusT(status);
277302
}
278303
session->setSessionSpecificRoot(accessorBinder);
279-
*_out = os::Service::make<os::Service::Tag::binder>(session->getRootObject());
304+
*_out = os::Service::make<os::Service::Tag::serviceWithMetadata>(
305+
createServiceWithMetadata(session->getRootObject(), false));
280306
return Status::ok();
281307
}
282308
default: {
@@ -291,7 +317,8 @@ Status BackendUnifiedServiceManager::addService(const ::std::string& name,
291317
Status status = mTheRealServiceManager->addService(name, service, allowIsolated, dumpPriority);
292318
// mEnableAddServiceCache is true by default.
293319
if (kUseCacheInAddService && mEnableAddServiceCache && status.isOk()) {
294-
return updateCache(name, service);
320+
return updateCache(name, service,
321+
dumpPriority & android::os::IServiceManager::FLAG_IS_LAZY_SERVICE);
295322
}
296323
return status;
297324
}

‎libs/binder/BackendUnifiedServiceManager.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,8 @@ class BackendUnifiedServiceManager : public android::os::BnServiceManager {
159159
binder::Status toBinderService(const ::std::string& name, const os::Service& in,
160160
os::Service* _out);
161161
binder::Status updateCache(const std::string& serviceName, const os::Service& service);
162-
binder::Status updateCache(const std::string& serviceName, const sp<IBinder>& binder);
162+
binder::Status updateCache(const std::string& serviceName, const sp<IBinder>& binder,
163+
bool isLazyService);
163164
bool returnIfCached(const std::string& serviceName, os::Service* _out);
164165
};
165166

‎libs/binder/IServiceManager.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,8 @@ class CppBackendShim : public IServiceManager {
152152
virtual Status realGetService(const std::string& name, sp<IBinder>* _aidl_return) {
153153
Service service;
154154
Status status = mUnifiedServiceManager->getService2(name, &service);
155-
*_aidl_return = service.get<Service::Tag::binder>();
155+
auto serviceWithMetadata = service.get<Service::Tag::serviceWithMetadata>();
156+
*_aidl_return = serviceWithMetadata.service;
156157
return status;
157158
}
158159
};
@@ -607,7 +608,7 @@ sp<IBinder> CppBackendShim::checkService(const String16& name) const {
607608
if (!mUnifiedServiceManager->checkService(String8(name).c_str(), &ret).isOk()) {
608609
return nullptr;
609610
}
610-
return ret.get<Service::Tag::binder>();
611+
return ret.get<Service::Tag::serviceWithMetadata>().service;
611612
}
612613

613614
status_t CppBackendShim::addService(const String16& name, const sp<IBinder>& service,

‎libs/binder/LazyServiceRegistrar.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
#include <android/os/BnClientCallback.h>
2020
#include <android/os/IServiceManager.h>
2121
#include <binder/IPCThreadState.h>
22-
#include <binder/IServiceManager.h>
2322
#include <binder/LazyServiceRegistrar.h>
2423

2524
namespace android {
@@ -134,6 +133,12 @@ bool ClientCounterCallbackImpl::registerServiceLocked(const sp<IBinder>& service
134133
std::string regStr = (reRegister) ? "Re-registering" : "Registering";
135134
ALOGI("%s service %s", regStr.c_str(), name.c_str());
136135

136+
if (dumpFlags & android::os::IServiceManager::FLAG_IS_LAZY_SERVICE) {
137+
ALOGW("FLAG_IS_LAZY_SERVICE flag already set. This should only be set by "
138+
"ClientCounterCallbackImpl in LazyServiceRegistrar");
139+
}
140+
dumpFlags |= android::os::IServiceManager::FLAG_IS_LAZY_SERVICE;
141+
137142
if (Status status = manager->addService(name.c_str(), service, allowIsolated, dumpFlags);
138143
!status.isOk()) {
139144
ALOGE("Failed to register service %s (%s)", name.c_str(), status.toString8().c_str());

‎libs/binder/aidl/android/os/IServiceManager.aidl‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ interface IServiceManager {
4949
DUMP_FLAG_PRIORITY_CRITICAL | DUMP_FLAG_PRIORITY_HIGH
5050
| DUMP_FLAG_PRIORITY_NORMAL | DUMP_FLAG_PRIORITY_DEFAULT;
5151

52+
const int FLAG_IS_LAZY_SERVICE = 1 << 30;
53+
5254
/* Allows services to dump sections in protobuf format. */
5355
const int DUMP_FLAG_PROTO = 1 << 4;
5456

@@ -61,7 +63,8 @@ interface IServiceManager {
6163
*
6264
* Returns null if the service does not exist.
6365
*
64-
* @deprecated TODO(b/355394904): Use getService2 instead.
66+
* @deprecated TODO(b/355394904): Use getService2 instead. This does not return metadata
67+
* that is included in ServiceWithMetadata
6568
*/
6669
@UnsupportedAppUsage
6770
@nullable IBinder getService(@utf8InCpp String name);

0 commit comments

Comments
 (0)