Add RpcServer::setRootObjectWeak(wp)
If the root binder object also holds a strong reference
to the RpcServer, to avoid circular dependency, the RpcServer
must hold a weak pointer of the RpcServer instead.
Test: binderRpcTest
Change-Id: I0d0c1f96a6a6d5339e2699f38edfaeb7ec2fd7f9
diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp
index 3a98cad..17c8efd 100644
--- a/libs/binder/RpcServer.cpp
+++ b/libs/binder/RpcServer.cpp
@@ -107,12 +107,21 @@
void RpcServer::setRootObject(const sp<IBinder>& binder) {
std::lock_guard<std::mutex> _l(mLock);
- mRootObject = binder;
+ mRootObjectWeak = mRootObject = binder;
+}
+
+void RpcServer::setRootObjectWeak(const wp<IBinder>& binder) {
+ std::lock_guard<std::mutex> _l(mLock);
+ mRootObject.clear();
+ mRootObjectWeak = binder;
}
sp<IBinder> RpcServer::getRootObject() {
std::lock_guard<std::mutex> _l(mLock);
- return mRootObject;
+ bool hasWeak = mRootObjectWeak.unsafe_get();
+ sp<IBinder> ret = mRootObjectWeak.promote();
+ ALOGW_IF(hasWeak && ret == nullptr, "RpcServer root object is freed, returning nullptr");
+ return ret;
}
void RpcServer::join() {
diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h
index b1e5519..7d0c198 100644
--- a/libs/binder/include/binder/RpcServer.h
+++ b/libs/binder/include/binder/RpcServer.h
@@ -90,8 +90,14 @@
/**
* The root object can be retrieved by any client, without any
* authentication. TODO(b/183988761)
+ *
+ * Holds a strong reference to the root object.
*/
void setRootObject(const sp<IBinder>& binder);
+ /**
+ * Holds a weak reference to the root object.
+ */
+ void setRootObjectWeak(const wp<IBinder>& binder);
sp<IBinder> getRootObject();
/**
@@ -127,6 +133,7 @@
std::mutex mLock; // for below
std::map<std::thread::id, std::thread> mConnectingThreads;
sp<IBinder> mRootObject;
+ wp<IBinder> mRootObjectWeak;
std::map<int32_t, sp<RpcSession>> mSessions;
int32_t mSessionIdCounter = 0;
};
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index b3ce744..260be57 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -929,6 +929,34 @@
}),
PrintSocketType);
+class BinderRpcServerRootObject : public ::testing::TestWithParam<std::tuple<bool, bool>> {};
+
+TEST_P(BinderRpcServerRootObject, WeakRootObject) {
+ using SetFn = std::function<void(RpcServer*, sp<IBinder>)>;
+ auto setRootObject = [](bool isStrong) -> SetFn {
+ return isStrong ? SetFn(&RpcServer::setRootObject) : SetFn(&RpcServer::setRootObjectWeak);
+ };
+
+ auto server = RpcServer::make();
+ auto [isStrong1, isStrong2] = GetParam();
+ auto binder1 = sp<BBinder>::make();
+ IBinder* binderRaw1 = binder1.get();
+ setRootObject(isStrong1)(server.get(), binder1);
+ EXPECT_EQ(binderRaw1, server->getRootObject());
+ binder1.clear();
+ EXPECT_EQ((isStrong1 ? binderRaw1 : nullptr), server->getRootObject());
+
+ auto binder2 = sp<BBinder>::make();
+ IBinder* binderRaw2 = binder2.get();
+ setRootObject(isStrong2)(server.get(), binder2);
+ EXPECT_EQ(binderRaw2, server->getRootObject());
+ binder2.clear();
+ EXPECT_EQ((isStrong2 ? binderRaw2 : nullptr), server->getRootObject());
+}
+
+INSTANTIATE_TEST_CASE_P(BinderRpc, BinderRpcServerRootObject,
+ ::testing::Combine(::testing::Bool(), ::testing::Bool()));
+
} // namespace android
int main(int argc, char** argv) {