summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--libs/binder/Binder.cpp71
-rw-r--r--libs/binder/include/binder/Binder.h2
-rw-r--r--libs/binder/include/binder/IBinder.h4
-rw-r--r--libs/binder/tests/Android.bp2
-rw-r--r--libs/binder/tests/binderLibTest.cpp68
5 files changed, 101 insertions, 46 deletions
diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp
index 19a22a51b2..d811b17e7e 100644
--- a/libs/binder/Binder.cpp
+++ b/libs/binder/Binder.cpp
@@ -17,6 +17,7 @@
#include <binder/Binder.h>
#include <atomic>
+#include <set>
#include <android-base/unique_fd.h>
#include <binder/BpBinder.h>
@@ -180,6 +181,38 @@ status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd,
// ---------------------------------------------------------------------------
+class BBinder::RpcServerLink : public IBinder::DeathRecipient {
+public:
+ // On binder died, calls RpcServer::shutdown on @a rpcServer, and removes itself from @a binder.
+ RpcServerLink(const sp<RpcServer>& rpcServer, const sp<IBinder>& keepAliveBinder,
+ const wp<BBinder>& binder)
+ : mRpcServer(rpcServer), mKeepAliveBinder(keepAliveBinder), mBinder(binder) {}
+ void binderDied(const wp<IBinder>&) override {
+ LOG_RPC_DETAIL("RpcServerLink: binder died, shutting down RpcServer");
+ if (mRpcServer == nullptr) {
+ ALOGW("RpcServerLink: Unable to shut down RpcServer because it does not exist.");
+ } else {
+ ALOGW_IF(!mRpcServer->shutdown(),
+ "RpcServerLink: RpcServer did not shut down properly. Not started?");
+ }
+ mRpcServer.clear();
+
+ auto promoted = mBinder.promote();
+ if (promoted == nullptr) {
+ ALOGW("RpcServerLink: Unable to remove link from parent binder object because parent "
+ "binder object is gone.");
+ } else {
+ promoted->removeRpcServerLink(sp<RpcServerLink>::fromExisting(this));
+ }
+ mBinder.clear();
+ }
+
+private:
+ sp<RpcServer> mRpcServer;
+ sp<IBinder> mKeepAliveBinder; // hold to avoid automatically unlinking
+ wp<BBinder> mBinder;
+};
+
class BBinder::Extras
{
public:
@@ -192,7 +225,7 @@ public:
// for below objects
Mutex mLock;
- sp<RpcServer> mRpcServer;
+ std::set<sp<RpcServerLink>> mRpcServerLinks;
BpBinder::ObjectManager mObjects;
};
@@ -489,24 +522,38 @@ status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd,
return INVALID_OPERATION;
}
+ // Weak ref to avoid circular dependency:
+ // BBinder -> RpcServerLink ----> RpcServer -X-> BBinder
+ // `-X-> BBinder
+ auto weakThis = wp<BBinder>::fromExisting(this);
+
Extras* e = getOrCreateExtras();
AutoMutex _l(e->mLock);
- if (e->mRpcServer != nullptr) {
- ALOGE("%s: Already have RPC client", __PRETTY_FUNCTION__);
- return ALREADY_EXISTS;
+ auto rpcServer = RpcServer::make();
+ LOG_ALWAYS_FATAL_IF(rpcServer == nullptr, "RpcServer::make returns null");
+ rpcServer->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
+ auto link = sp<RpcServerLink>::make(rpcServer, keepAliveBinder, weakThis);
+ if (auto status = keepAliveBinder->linkToDeath(link, nullptr, 0); status != OK) {
+ ALOGE("%s: keepAliveBinder->linkToDeath returns %s", __PRETTY_FUNCTION__,
+ statusToString(status).c_str());
+ return status;
}
- e->mRpcServer = RpcServer::make();
- LOG_ALWAYS_FATAL_IF(e->mRpcServer == nullptr, "RpcServer::make returns null");
- e->mRpcServer->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
- // Weak ref to avoid circular dependency: BBinder -> RpcServer -X-> BBinder
- e->mRpcServer->setRootObjectWeak(wp<BBinder>::fromExisting(this));
- e->mRpcServer->setupExternalServer(std::move(socketFd));
- e->mRpcServer->setMaxThreads(binderThreadPoolMaxCount);
- e->mRpcServer->start();
+ rpcServer->setRootObjectWeak(weakThis);
+ rpcServer->setupExternalServer(std::move(socketFd));
+ rpcServer->setMaxThreads(binderThreadPoolMaxCount);
+ rpcServer->start();
+ e->mRpcServerLinks.emplace(link);
LOG_RPC_DETAIL("%s(fd=%d) successful", __PRETTY_FUNCTION__, socketFdForPrint);
return OK;
}
+void BBinder::removeRpcServerLink(const sp<RpcServerLink>& link) {
+ Extras* e = mExtras.load(std::memory_order_acquire);
+ if (!e) return;
+ AutoMutex _l(e->mLock);
+ (void)e->mRpcServerLinks.erase(link);
+}
+
BBinder::~BBinder()
{
Extras* e = mExtras.load(std::memory_order_relaxed);
diff --git a/libs/binder/include/binder/Binder.h b/libs/binder/include/binder/Binder.h
index 0cc0ef1e23..472e546944 100644
--- a/libs/binder/include/binder/Binder.h
+++ b/libs/binder/include/binder/Binder.h
@@ -117,11 +117,13 @@ private:
BBinder(const BBinder& o);
BBinder& operator=(const BBinder& o);
+ class RpcServerLink;
class Extras;
Extras* getOrCreateExtras();
[[nodiscard]] status_t setRpcClientDebug(const Parcel& data);
+ void removeRpcServerLink(const sp<RpcServerLink>& link);
std::atomic<Extras*> mExtras;
diff --git a/libs/binder/include/binder/IBinder.h b/libs/binder/include/binder/IBinder.h
index 122c595078..f9cdac7de9 100644
--- a/libs/binder/include/binder/IBinder.h
+++ b/libs/binder/include/binder/IBinder.h
@@ -162,8 +162,8 @@ public:
* 2. spawns 1 new thread that calls RpcServer::join()
* - join() spawns some number of threads that accept() connections; see RpcServer
*
- * setRpcClientDebug() may only be called once.
- * TODO(b/182914638): If allow to shut down the client, addRpcClient can be called repeatedly.
+ * setRpcClientDebug() may be called multiple times. Each call will add a new RpcServer
+ * and opens up a TCP port.
*
* 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
diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
index c7c899fcd8..326d9fd58f 100644
--- a/libs/binder/tests/Android.bp
+++ b/libs/binder/tests/Android.bp
@@ -62,6 +62,7 @@ cc_test {
shared_libs: [
"libbase",
"libbinder",
+ "liblog",
"libutils",
],
static_libs: [
@@ -104,6 +105,7 @@ cc_test {
shared_libs: [
"libbase",
"libbinder",
+ "liblog",
"libutils",
],
static_libs: [
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index abbb16e205..1ecb0b7b58 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -15,14 +15,13 @@
*/
#include <errno.h>
-#include <fcntl.h>
-#include <fstream>
#include <poll.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <chrono>
+#include <fstream>
#include <thread>
#include <gmock/gmock.h>
@@ -31,6 +30,7 @@
#include <android-base/properties.h>
#include <android-base/result-gmock.h>
#include <android-base/result.h>
+#include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <binder/Binder.h>
#include <binder/BpBinder.h>
@@ -55,6 +55,7 @@ using namespace android;
using namespace std::string_literals;
using namespace std::chrono_literals;
using android::base::testing::HasValue;
+using android::base::testing::Ok;
using testing::ExplainMatchResult;
using testing::Not;
using testing::WithParamInterface;
@@ -1200,7 +1201,35 @@ public:
}
};
-class BinderLibRpcTest : public BinderLibRpcTestBase, public WithParamInterface<bool> {
+class BinderLibRpcTest : public BinderLibRpcTestBase {};
+
+TEST_F(BinderLibRpcTest, SetRpcClientDebug) {
+ auto binder = addServer();
+ ASSERT_TRUE(binder != nullptr);
+ auto [socket, port] = CreateSocket();
+ ASSERT_TRUE(socket.ok());
+ EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), sp<BBinder>::make()), StatusEq(OK));
+}
+
+// Tests for multiple RpcServer's on the same binder object.
+TEST_F(BinderLibRpcTest, SetRpcClientDebugTwice) {
+ auto binder = addServer();
+ ASSERT_TRUE(binder != nullptr);
+
+ auto [socket1, port1] = CreateSocket();
+ ASSERT_TRUE(socket1.ok());
+ auto keepAliveBinder1 = sp<BBinder>::make();
+ EXPECT_THAT(binder->setRpcClientDebug(std::move(socket1), keepAliveBinder1), StatusEq(OK));
+
+ auto [socket2, port2] = CreateSocket();
+ ASSERT_TRUE(socket2.ok());
+ auto keepAliveBinder2 = sp<BBinder>::make();
+ EXPECT_THAT(binder->setRpcClientDebug(std::move(socket2), keepAliveBinder2), StatusEq(OK));
+}
+
+// Negative tests for RPC APIs on IBinder. Call should fail in the same way on both remote and
+// local binders.
+class BinderLibRpcTestP : public BinderLibRpcTestBase, public WithParamInterface<bool> {
public:
sp<IBinder> GetService() {
return GetParam() ? sp<IBinder>(addServer()) : sp<IBinder>(sp<BBinder>::make());
@@ -1210,47 +1239,22 @@ public:
}
};
-TEST_P(BinderLibRpcTest, SetRpcClientDebug) {
- auto binder = GetService();
- ASSERT_TRUE(binder != nullptr);
- auto [socket, port] = CreateSocket();
- ASSERT_TRUE(socket.ok());
- EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), sp<BBinder>::make()), StatusEq(OK));
-}
-
-TEST_P(BinderLibRpcTest, SetRpcClientDebugNoFd) {
+TEST_P(BinderLibRpcTestP, SetRpcClientDebugNoFd) {
auto binder = GetService();
ASSERT_TRUE(binder != nullptr);
EXPECT_THAT(binder->setRpcClientDebug(android::base::unique_fd(), sp<BBinder>::make()),
StatusEq(BAD_VALUE));
}
-TEST_P(BinderLibRpcTest, SetRpcClientDebugNoKeepAliveBinder) {
+TEST_P(BinderLibRpcTestP, 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) {
- auto binder = GetService();
- ASSERT_TRUE(binder != nullptr);
-
- auto [socket1, port1] = CreateSocket();
- ASSERT_TRUE(socket1.ok());
- auto keepAliveBinder1 = sp<BBinder>::make();
- EXPECT_THAT(binder->setRpcClientDebug(std::move(socket1), keepAliveBinder1), StatusEq(OK));
-
- auto [socket2, port2] = CreateSocket();
- ASSERT_TRUE(socket2.ok());
- auto keepAliveBinder2 = sp<BBinder>::make();
- EXPECT_THAT(binder->setRpcClientDebug(std::move(socket2), keepAliveBinder2),
- StatusEq(ALREADY_EXISTS));
-}
-
-INSTANTIATE_TEST_CASE_P(BinderLibTest, BinderLibRpcTest, testing::Bool(),
- BinderLibRpcTest::ParamToString);
+INSTANTIATE_TEST_CASE_P(BinderLibTest, BinderLibRpcTestP, testing::Bool(),
+ BinderLibRpcTestP::ParamToString);
class BinderLibTestService;
class BinderLibRpcClientTest : public BinderLibRpcTestBase,
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2026-01-06 01:09:50 +0000