Skip to content

Commit 2b61344

Browse files
author
Alice Wang
committed
[native] Restore ServiceManager#checkService() to return IBinder
This fixes crashes in 3p libraries. A new API ServiceManager#checkService2() has been introduced to work with the Service enum type. Bug: 387175643 Test: atest servicemanager_test Change-Id: I647f4a11469717c54111afab562a0be2d5260044
1 parent 1656f6b commit 2b61344

9 files changed

Lines changed: 51 additions & 11 deletions

File tree

cmds/servicemanager/ServiceManager.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,16 @@ Status ServiceManager::getService2(const std::string& name, os::Service* outServ
410410
return Status::ok();
411411
}
412412

413-
Status ServiceManager::checkService(const std::string& name, os::Service* outService) {
413+
Status ServiceManager::checkService(const std::string& name, sp<IBinder>* outBinder) {
414+
SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS(
415+
PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str())));
416+
417+
*outBinder = tryGetBinder(name, false).service;
418+
// returns ok regardless of result for legacy reasons
419+
return Status::ok();
420+
}
421+
422+
Status ServiceManager::checkService2(const std::string& name, os::Service* outService) {
414423
SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS(
415424
PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str())));
416425

cmds/servicemanager/ServiceManager.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ class ServiceManager : public os::BnServiceManager, public IBinder::DeathRecipie
4646
// getService will try to start any services it cannot find
4747
binder::Status getService(const std::string& name, sp<IBinder>* outBinder) override;
4848
binder::Status getService2(const std::string& name, os::Service* outService) override;
49-
binder::Status checkService(const std::string& name, os::Service* outService) override;
49+
binder::Status checkService(const std::string& name, sp<IBinder>* outBinder) override;
50+
binder::Status checkService2(const std::string& name, os::Service* outService) override;
5051
binder::Status addService(const std::string& name, const sp<IBinder>& binder,
5152
bool allowIsolated, int32_t dumpPriority) override;
5253
binder::Status listServices(int32_t dumpPriority, std::vector<std::string>* outList) override;

cmds/servicemanager/test_sm.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,11 @@ TEST(GetService, HappyHappy) {
204204
sp<IBinder> outBinder;
205205
EXPECT_TRUE(sm->getService("foo", &outBinder).isOk());
206206
EXPECT_EQ(service, outBinder);
207+
208+
EXPECT_TRUE(sm->checkService2("foo", &out).isOk());
209+
EXPECT_EQ(service, out.get<Service::Tag::serviceWithMetadata>().service);
210+
EXPECT_TRUE(sm->checkService("foo", &outBinder).isOk());
211+
EXPECT_EQ(service, outBinder);
207212
}
208213

209214
TEST(GetService, NonExistant) {

libs/binder/BackendUnifiedServiceManager.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,15 +238,25 @@ Status BackendUnifiedServiceManager::getService2(const ::std::string& name, os::
238238
return status;
239239
}
240240

241-
Status BackendUnifiedServiceManager::checkService(const ::std::string& name, os::Service* _out) {
241+
Status BackendUnifiedServiceManager::checkService(const ::std::string& name,
242+
sp<IBinder>* _aidl_return) {
243+
os::Service service;
244+
Status status = checkService2(name, &service);
245+
if (status.isOk()) {
246+
*_aidl_return = service.get<os::Service::Tag::serviceWithMetadata>().service;
247+
}
248+
return status;
249+
}
250+
251+
Status BackendUnifiedServiceManager::checkService2(const ::std::string& name, os::Service* _out) {
242252
os::Service service;
243253
if (returnIfCached(name, _out)) {
244254
return Status::ok();
245255
}
246256

247257
Status status = Status::ok();
248258
if (mTheRealServiceManager) {
249-
status = mTheRealServiceManager->checkService(name, &service);
259+
status = mTheRealServiceManager->checkService2(name, &service);
250260
}
251261
if (status.isOk()) {
252262
status = toBinderService(name, service, _out);

libs/binder/BackendUnifiedServiceManager.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,8 @@ class BackendUnifiedServiceManager : public android::os::BnServiceManager {
122122

123123
binder::Status getService(const ::std::string& name, sp<IBinder>* _aidl_return) override;
124124
binder::Status getService2(const ::std::string& name, os::Service* out) override;
125-
binder::Status checkService(const ::std::string& name, os::Service* out) override;
125+
binder::Status checkService(const ::std::string& name, sp<IBinder>* _aidl_return) override;
126+
binder::Status checkService2(const ::std::string& name, os::Service* out) override;
126127
binder::Status addService(const ::std::string& name, const sp<IBinder>& service,
127128
bool allowIsolated, int32_t dumpPriority) override;
128129
binder::Status listServices(int32_t dumpPriority,

libs/binder/IServiceManager.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ sp<IBinder> CppBackendShim::getService(const String16& name) const {
624624

625625
sp<IBinder> CppBackendShim::checkService(const String16& name) const {
626626
Service ret;
627-
if (!mUnifiedServiceManager->checkService(String8(name).c_str(), &ret).isOk()) {
627+
if (!mUnifiedServiceManager->checkService2(String8(name).c_str(), &ret).isOk()) {
628628
return nullptr;
629629
}
630630
return ret.get<Service::Tag::serviceWithMetadata>().service;

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,22 @@ interface IServiceManager {
8181
*/
8282
Service getService2(@utf8InCpp String name);
8383

84+
/**
85+
* Retrieve an existing service called @a name from the service
86+
* manager. Non-blocking. Returns null if the service does not exist.
87+
*
88+
* @deprecated TODO(b/355394904): Use checkService2 instead. This does not
89+
* return metadata that is included in ServiceWithMetadata
90+
*/
91+
@UnsupportedAppUsage
92+
@nullable IBinder checkService(@utf8InCpp String name);
93+
8494
/**
8595
* Retrieve an existing service called @a name from the service
8696
* manager. Non-blocking. Returns null if the service does not
8797
* exist.
8898
*/
89-
@UnsupportedAppUsage
90-
Service checkService(@utf8InCpp String name);
99+
Service checkService2(@utf8InCpp String name);
91100

92101
/**
93102
* Place a new @a service called @a name into the service

libs/binder/servicedispatcher.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,12 @@ class ServiceManagerProxyToNative : public android::os::BnServiceManager {
127127
// We can't send BpBinder for regular binder over RPC.
128128
return android::binder::Status::fromStatusT(android::INVALID_OPERATION);
129129
}
130-
android::binder::Status checkService(const std::string&, android::os::Service*) override {
130+
android::binder::Status checkService(const std::string&,
131+
android::sp<android::IBinder>*) override {
132+
// We can't send BpBinder for regular binder over RPC.
133+
return android::binder::Status::fromStatusT(android::INVALID_OPERATION);
134+
}
135+
android::binder::Status checkService2(const std::string&, android::os::Service*) override {
131136
// We can't send BpBinder for regular binder over RPC.
132137
return android::binder::Status::fromStatusT(android::INVALID_OPERATION);
133138
}

libs/binder/tests/binderCacheUnitTest.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ class MockAidlServiceManager : public os::IServiceManagerDefault {
7474
public:
7575
MockAidlServiceManager() : innerSm() {}
7676

77-
binder::Status checkService(const ::std::string& name, os::Service* _out) override {
77+
binder::Status checkService2(const ::std::string& name, os::Service* _out) override {
7878
os::ServiceWithMetadata serviceWithMetadata = os::ServiceWithMetadata();
7979
serviceWithMetadata.service = innerSm.getService(String16(name.c_str()));
8080
serviceWithMetadata.isLazyService = false;
@@ -98,7 +98,7 @@ class MockAidlServiceManager2 : public os::IServiceManagerDefault {
9898
public:
9999
MockAidlServiceManager2() : innerSm() {}
100100

101-
binder::Status checkService(const ::std::string& name, os::Service* _out) override {
101+
binder::Status checkService2(const ::std::string& name, os::Service* _out) override {
102102
os::ServiceWithMetadata serviceWithMetadata = os::ServiceWithMetadata();
103103
serviceWithMetadata.service = innerSm.getService(String16(name.c_str()));
104104
serviceWithMetadata.isLazyService = true;

0 commit comments

Comments
 (0)