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

Commit 64127ca2 authored by Steven Moreland's avatar Steven Moreland
Browse files

libbinder_ndk: unlinkToDeath dead proxy 

This CL automatically unlinks to death when a proxy dies (to clean
up additional data) and documents the behavior.

Bug: N/A
Test: ./runtests.sh
Change-Id: Ib092b25273a51f5afb12904bfe491927ad404f8d
parent 91e66f9b

libs/binder/include/binder/IBinder.h

+3 −0
Original line number Diff line number Diff line
@@ -143,6 +143,9 @@ public: 
     * dies.  The @a cookie is optional.  If non-NULL, you can

     * supply a NULL @a recipient, and the recipient previously

     * added with that cookie will be unlinked.

     *

     * If the binder is dead, this will return DEAD_OBJECT. Deleting

     * the object will also unlink all death recipients.

     */

    // NOLINTNEXTLINE(google-default-arguments)

    virtual status_t        unlinkToDeath(  const wp<DeathRecipient>& recipient,

libs/binder/ndk/ibinder.cpp

+38 −10
Original line number Diff line number Diff line
@@ -269,6 +269,18 @@ void AIBinder_DeathRecipient::TransferDeathRecipient::binderDied(const wp<IBinde 
    CHECK(who == mWho);



    mOnDied(mCookie);



    sp<AIBinder_DeathRecipient> recipient = mParentRecipient.promote();

    sp<IBinder> strongWho = who.promote();



    // otherwise this will be cleaned up later with pruneDeadTransferEntriesLocked

    if (recipient != nullptr && strongWho != nullptr) {

        status_t result = recipient->unlinkToDeath(strongWho, mCookie);

        if (result != ::android::DEAD_OBJECT) {

            LOG(WARNING) << "Unlinking to dead binder resulted in: " << result;

        }

    }



    mWho = nullptr;

}
@@ -277,24 +289,34 @@ AIBinder_DeathRecipient::AIBinder_DeathRecipient(AIBinder_DeathRecipient_onBinde 
    CHECK(onDied != nullptr);

}



binder_status_t AIBinder_DeathRecipient::linkToDeath(AIBinder* binder, void* cookie) {

void AIBinder_DeathRecipient::pruneDeadTransferEntriesLocked() {

    mDeathRecipients.erase(std::remove_if(mDeathRecipients.begin(), mDeathRecipients.end(),

                                          [](const sp<TransferDeathRecipient>& tdr) {

                                              return tdr->getWho() == nullptr;

                                          }),

                           mDeathRecipients.end());

}



binder_status_t AIBinder_DeathRecipient::linkToDeath(sp<IBinder> binder, void* cookie) {

    CHECK(binder != nullptr);



    std::lock_guard<std::mutex> l(mDeathRecipientsMutex);



    sp<TransferDeathRecipient> recipient =

            new TransferDeathRecipient(binder->getBinder(), cookie, mOnDied);

            new TransferDeathRecipient(binder, cookie, this, mOnDied);



    status_t status = binder->getBinder()->linkToDeath(recipient, cookie, 0 /*flags*/);

    status_t status = binder->linkToDeath(recipient, cookie, 0 /*flags*/);

    if (status != STATUS_OK) {

        return PruneStatusT(status);

    }



    mDeathRecipients.push_back(recipient);



    pruneDeadTransferEntriesLocked();

    return STATUS_OK;

}



binder_status_t AIBinder_DeathRecipient::unlinkToDeath(AIBinder* binder, void* cookie) {

binder_status_t AIBinder_DeathRecipient::unlinkToDeath(sp<IBinder> binder, void* cookie) {

    CHECK(binder != nullptr);



    std::lock_guard<std::mutex> l(mDeathRecipientsMutex);
@@ -302,10 +324,10 @@ binder_status_t AIBinder_DeathRecipient::unlinkToDeath(AIBinder* binder, void* c 
    for (auto it = mDeathRecipients.rbegin(); it != mDeathRecipients.rend(); ++it) {

        sp<TransferDeathRecipient> recipient = *it;



        if (recipient->getCookie() == cookie && recipient->getWho() == binder->getBinder()) {

        if (recipient->getCookie() == cookie && recipient->getWho() == binder) {

            mDeathRecipients.erase(it.base() - 1);



            status_t status = binder->getBinder()->unlinkToDeath(recipient, cookie, 0 /*flags*/);

            status_t status = binder->unlinkToDeath(recipient, cookie, 0 /*flags*/);

            if (status != ::android::OK) {

                LOG(ERROR) << __func__

                           << ": removed reference to death recipient but unlink failed.";
@@ -390,7 +412,7 @@ binder_status_t AIBinder_linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* 
    }



    // returns binder_status_t

    return recipient->linkToDeath(binder, cookie);

    return recipient->linkToDeath(binder->getBinder(), cookie);

}



binder_status_t AIBinder_unlinkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,
@@ -401,7 +423,7 @@ binder_status_t AIBinder_unlinkToDeath(AIBinder* binder, AIBinder_DeathRecipient 
    }



    // returns binder_status_t

    return recipient->unlinkToDeath(binder, cookie);

    return recipient->unlinkToDeath(binder->getBinder(), cookie);

}



uid_t AIBinder_getCallingUid() {
@@ -555,9 +577,15 @@ AIBinder_DeathRecipient* AIBinder_DeathRecipient_new( 
        LOG(ERROR) << __func__ << ": requires non-null onBinderDied parameter.";

        return nullptr;

    }

    return new AIBinder_DeathRecipient(onBinderDied);

    auto ret = new AIBinder_DeathRecipient(onBinderDied);

    ret->incStrong(nullptr);

    return ret;

}



void AIBinder_DeathRecipient_delete(AIBinder_DeathRecipient* recipient) {

    delete recipient;

    if (recipient == nullptr) {

        return;

    }



    recipient->decStrong(nullptr);

}

libs/binder/ndk/ibinder_internal.h

+15 −4
Original line number Diff line number Diff line
@@ -128,13 +128,14 @@ struct AIBinder_Class { 
//

// When the AIBinder_DeathRecipient is dropped, so are the actual underlying death recipients. When

// the IBinder dies, only a wp to it is kept.

struct AIBinder_DeathRecipient {

struct AIBinder_DeathRecipient : ::android::RefBase {

    // One of these is created for every linkToDeath. This is to be able to recover data when a

    // binderDied receipt only gives us information about the IBinder.

    struct TransferDeathRecipient : ::android::IBinder::DeathRecipient {

        TransferDeathRecipient(const ::android::wp<::android::IBinder>& who, void* cookie,

                               const ::android::wp<AIBinder_DeathRecipient>& parentRecipient,

                               const AIBinder_DeathRecipient_onBinderDied onDied)

            : mWho(who), mCookie(cookie), mOnDied(onDied) {}

            : mWho(who), mCookie(cookie), mParentRecipient(parentRecipient), mOnDied(onDied) {}



        void binderDied(const ::android::wp<::android::IBinder>& who) override;
@@ -144,14 +145,24 @@ struct AIBinder_DeathRecipient { 
       private:

        ::android::wp<::android::IBinder> mWho;

        void* mCookie;



        ::android::wp<AIBinder_DeathRecipient> mParentRecipient;



        // This is kept separately from AIBinder_DeathRecipient in case the death recipient is

        // deleted while the death notification is fired

        const AIBinder_DeathRecipient_onBinderDied mOnDied;

    };



    explicit AIBinder_DeathRecipient(AIBinder_DeathRecipient_onBinderDied onDied);

    binder_status_t linkToDeath(AIBinder* binder, void* cookie);

    binder_status_t unlinkToDeath(AIBinder* binder, void* cookie);

    binder_status_t linkToDeath(::android::sp<::android::IBinder>, void* cookie);

    binder_status_t unlinkToDeath(::android::sp<::android::IBinder> binder, void* cookie);



   private:

    // When the user of this API deletes a Bp object but not the death recipient, the

    // TransferDeathRecipient object can't be cleaned up. This is called whenever a new

    // TransferDeathRecipient is linked, and it ensures that mDeathRecipients can't grow unbounded.

    void pruneDeadTransferEntriesLocked();



    std::mutex mDeathRecipientsMutex;

    std::vector<::android::sp<TransferDeathRecipient>> mDeathRecipients;

    AIBinder_DeathRecipient_onBinderDied mOnDied;

libs/binder/ndk/include_ndk/android/binder_ibinder.h

+5 −0
Original line number Diff line number Diff line
@@ -301,6 +301,11 @@ binder_status_t AIBinder_linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* 
 * may return a binder transaction failure and in case the death recipient cannot be found, it

 * returns STATUS_NAME_NOT_FOUND.

 *

 * This only ever needs to be called when the AIBinder_DeathRecipient remains for use with other

 * AIBinder objects. If the death recipient is deleted, all binders will automatically be unlinked.

 * If the binder dies, it will automatically unlink. If the binder is deleted, it will be

 * automatically unlinked.

 *

 * \param binder the binder object to remove a previously linked death recipient from.

 * \param recipient the callback to remove.

 * \param cookie the cookie used to link to death.

libs/binder/ndk/test/main_client.cpp

+4 −1
Original line number Diff line number Diff line
@@ -86,12 +86,15 @@ TEST(NdkBinder, DeathRecipient) { 
    // the binder driver should return this if the service dies during the transaction

    EXPECT_EQ(STATUS_DEAD_OBJECT, foo->die());



    foo = nullptr;

    AIBinder_decStrong(binder);

    binder = nullptr;



    std::unique_lock<std::mutex> lock(deathMutex);

    EXPECT_TRUE(deathCv.wait_for(lock, 1s, [&] { return deathRecieved; }));

    EXPECT_TRUE(deathRecieved);



    AIBinder_DeathRecipient_delete(recipient);

    AIBinder_decStrong(binder);

}



TEST(NdkBinder, RetrieveNonNdkService) {