Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4ffb0c7f authored by Yifan Hong's avatar Yifan Hong
Browse files

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
parent bdc80541

libs/binder/RpcServer.cpp

+11 −2
Original line number Diff line number Diff line
@@ -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() {

libs/binder/include/binder/RpcServer.h

+7 −0
Original line number Diff line number Diff line
@@ -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;

};

libs/binder/tests/binderRpcTest.cpp

+28 −0
Original line number Diff line number Diff line
@@ -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) {