Merge "Add RpcServer::setRootObjectWeak(wp)" am: 5b7b63796e

Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/1700336

Change-Id: I588665563a6b14b66cd023b61c4f61a193e31ea4

3 files changed, 46 insertions, 2 deletions

diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp
index 3a98cadb94..17c8efdc41 100644
--- a/libs/binder/RpcServer.cpp
+++ b/libs/binder/RpcServer.cpp
@@ -107,12 +107,21 @@ size_t RpcServer::getMaxThreads() {
 
 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 b1e5519cff..7d0c198596 100644
--- a/libs/binder/include/binder/RpcServer.h
+++ b/libs/binder/include/binder/RpcServer.h
@@ -90,8 +90,14 @@ public:
     /**
      * 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 @@ private:
     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 b3ce7447a9..260be570f9 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -929,6 +929,34 @@ INSTANTIATE_TEST_CASE_P(PerSocket, BinderRpc,
                         }),
                         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) {