Add keepAliveBinder argument to setRpcClientDebug.
When the given binder object dies, the corresponding
RpcServer is also shut down.
This CL does not make use of the argument other than
null checks. See follow-up CL for implementation.
Test: binderLibTest
Bug: 182914638
Change-Id: Id787e70f885b165db703e7bb6fa01e6bec271fe6
diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp
index 194be21..19a22a5 100644
--- a/libs/binder/Binder.cpp
+++ b/libs/binder/Binder.cpp
@@ -150,7 +150,8 @@
return OK;
}
-status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd) {
+status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd,
+ const sp<IBinder>& keepAliveBinder) {
if constexpr (!kEnableRpcDevServers) {
ALOGW("setRpcClientDebug disallowed because RPC is not enabled");
return INVALID_OPERATION;
@@ -158,7 +159,7 @@
BBinder* local = this->localBinder();
if (local != nullptr) {
- return local->BBinder::setRpcClientDebug(std::move(socketFd));
+ return local->BBinder::setRpcClientDebug(std::move(socketFd), keepAliveBinder);
}
BpBinder* proxy = this->remoteBinder();
@@ -173,6 +174,7 @@
status = data.writeFileDescriptor(socketFd.release(), true /* own */);
if (status != OK) return status;
}
+ if (status = data.writeStrongBinder(keepAliveBinder); status != OK) return status;
return transact(SET_RPC_CLIENT_TRANSACTION, data, &reply);
}
@@ -453,11 +455,14 @@
if (hasSocketFd) {
if (status = data.readUniqueFileDescriptor(&clientFd); status != OK) return status;
}
+ sp<IBinder> keepAliveBinder;
+ if (status = data.readNullableStrongBinder(&keepAliveBinder); status != OK) return status;
- return setRpcClientDebug(std::move(clientFd));
+ return setRpcClientDebug(std::move(clientFd), keepAliveBinder);
}
-status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd) {
+status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd,
+ const sp<IBinder>& keepAliveBinder) {
if constexpr (!kEnableRpcDevServers) {
ALOGW("%s: disallowed because RPC is not enabled", __PRETTY_FUNCTION__);
return INVALID_OPERATION;
@@ -471,6 +476,11 @@
return BAD_VALUE;
}
+ if (keepAliveBinder == nullptr) {
+ ALOGE("%s: No keepAliveBinder provided.", __PRETTY_FUNCTION__);
+ return UNEXPECTED_NULL;
+ }
+
size_t binderThreadPoolMaxCount = ProcessState::self()->getThreadPoolMaxThreadCount();
if (binderThreadPoolMaxCount <= 1) {
ALOGE("%s: ProcessState thread pool max count is %zu. RPC is disabled for this service "
diff --git a/libs/binder/include/binder/Binder.h b/libs/binder/include/binder/Binder.h
index 53885ff..0cc0ef1 100644
--- a/libs/binder/include/binder/Binder.h
+++ b/libs/binder/include/binder/Binder.h
@@ -101,7 +101,8 @@
// to another process.
void setParceled();
- [[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd clientFd);
+ [[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd clientFd,
+ const sp<IBinder>& keepAliveBinder);
protected:
virtual ~BBinder();
diff --git a/libs/binder/include/binder/IBinder.h b/libs/binder/include/binder/IBinder.h
index 06cb21d..122c595 100644
--- a/libs/binder/include/binder/IBinder.h
+++ b/libs/binder/include/binder/IBinder.h
@@ -168,8 +168,11 @@
* Note: A thread is spawned for each accept()'ed fd, which may call into functions of the
* interface freely. See RpcServer::join(). To avoid such race conditions, implement the service
* functions with multithreading support.
+ *
+ * On death of @a keepAliveBinder, the RpcServer shuts down.
*/
- [[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd socketFd);
+ [[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd socketFd,
+ const sp<IBinder>& keepAliveBinder);
// NOLINTNEXTLINE(google-default-arguments)
virtual status_t transact( uint32_t code,
diff --git a/libs/binder/servicedispatcher.cpp b/libs/binder/servicedispatcher.cpp
index a2fa842..0c677a8 100644
--- a/libs/binder/servicedispatcher.cpp
+++ b/libs/binder/servicedispatcher.cpp
@@ -26,9 +26,11 @@
#include <binder/IServiceManager.h>
#include <binder/RpcServer.h>
+using android::BBinder;
using android::defaultServiceManager;
using android::OK;
using android::RpcServer;
+using android::sp;
using android::statusToString;
using android::String16;
using android::base::Basename;
@@ -74,7 +76,8 @@
return EX_SOFTWARE;
}
auto socket = rpcServer->releaseServer();
- auto status = binder->setRpcClientDebug(std::move(socket));
+ auto keepAliveBinder = sp<BBinder>::make();
+ auto status = binder->setRpcClientDebug(std::move(socket), keepAliveBinder);
if (status != OK) {
LOG(ERROR) << "setRpcClientDebug failed with " << statusToString(status);
return EX_SOFTWARE;
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 9370f52..abbb16e 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -1215,13 +1215,22 @@
ASSERT_TRUE(binder != nullptr);
auto [socket, port] = CreateSocket();
ASSERT_TRUE(socket.ok());
- EXPECT_THAT(binder->setRpcClientDebug(std::move(socket)), StatusEq(OK));
+ EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), sp<BBinder>::make()), StatusEq(OK));
}
TEST_P(BinderLibRpcTest, SetRpcClientDebugNoFd) {
auto binder = GetService();
ASSERT_TRUE(binder != nullptr);
- EXPECT_THAT(binder->setRpcClientDebug(android::base::unique_fd()), StatusEq(BAD_VALUE));
+ EXPECT_THAT(binder->setRpcClientDebug(android::base::unique_fd(), sp<BBinder>::make()),
+ StatusEq(BAD_VALUE));
+}
+
+TEST_P(BinderLibRpcTest, SetRpcClientDebugNoKeepAliveBinder) {
+ auto binder = GetService();
+ ASSERT_TRUE(binder != nullptr);
+ auto [socket, port] = CreateSocket();
+ ASSERT_TRUE(socket.ok());
+ EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), nullptr), StatusEq(UNEXPECTED_NULL));
}
TEST_P(BinderLibRpcTest, SetRpcClientDebugTwice) {
@@ -1230,11 +1239,14 @@
auto [socket1, port1] = CreateSocket();
ASSERT_TRUE(socket1.ok());
- EXPECT_THAT(binder->setRpcClientDebug(std::move(socket1)), StatusEq(OK));
+ auto keepAliveBinder1 = sp<BBinder>::make();
+ EXPECT_THAT(binder->setRpcClientDebug(std::move(socket1), keepAliveBinder1), StatusEq(OK));
auto [socket2, port2] = CreateSocket();
ASSERT_TRUE(socket2.ok());
- EXPECT_THAT(binder->setRpcClientDebug(std::move(socket2)), StatusEq(ALREADY_EXISTS));
+ auto keepAliveBinder2 = sp<BBinder>::make();
+ EXPECT_THAT(binder->setRpcClientDebug(std::move(socket2), keepAliveBinder2),
+ StatusEq(ALREADY_EXISTS));
}
INSTANTIATE_TEST_CASE_P(BinderLibTest, BinderLibRpcTest, testing::Bool(),
diff --git a/libs/binder/tests/unit_fuzzers/BinderFuzzFunctions.h b/libs/binder/tests/unit_fuzzers/BinderFuzzFunctions.h
index 1484fec..8d2b714 100644
--- a/libs/binder/tests/unit_fuzzers/BinderFuzzFunctions.h
+++ b/libs/binder/tests/unit_fuzzers/BinderFuzzFunctions.h
@@ -74,7 +74,8 @@
bbinder->getDebugPid();
},
[](FuzzedDataProvider*, const sp<BBinder>& bbinder) -> void {
- (void)bbinder->setRpcClientDebug(android::base::unique_fd());
+ (void)bbinder->setRpcClientDebug(android::base::unique_fd(),
+ sp<BBinder>::make());
}};
} // namespace android