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

Commit 9b927e2e authored by Devin Moore's avatar Devin Moore
Browse files

Remove TransferDeathRecipients when ABpBinder is deleted 

Store a weak reference on the ABpBinder to the death recipients it's
linked to so it can initiate a clean up when being destructed.

Test what happens when a binder is deleted.
Test what happens when a binder is deleted from the death recipient when
the binder dies.

Ignore-AOSP-First: b/319210610

Test: atest libbinder_ndk_unit_test
Bug: 325612851
Change-Id: Ifc532b64d8ca0fb586f7fbadd0956e103ded94fc
parent 77add0b0

libs/binder/ndk/ibinder.cpp

+35 −3
Original line number Diff line number Diff line
@@ -258,11 +258,24 @@ status_t ABBinder::onTransact(transaction_code_t code, const Parcel& data, Parce 
    }

}



void ABBinder::addDeathRecipient(const ::android::sp<AIBinder_DeathRecipient>& /* recipient */,

                                 void* /* cookie */) {

    LOG_ALWAYS_FATAL("Should not reach this. Can't linkToDeath local binders.");

}



ABpBinder::ABpBinder(const ::android::sp<::android::IBinder>& binder)

    : AIBinder(nullptr /*clazz*/), mRemote(binder) {

    LOG_ALWAYS_FATAL_IF(binder == nullptr, "binder == nullptr");

}

ABpBinder::~ABpBinder() {}



ABpBinder::~ABpBinder() {

    for (auto& recip : mDeathRecipients) {

        sp<AIBinder_DeathRecipient> strongRecip = recip.recipient.promote();

        if (strongRecip) {

            strongRecip->pruneThisTransferEntry(getBinder(), recip.cookie);

        }

    }

}



sp<AIBinder> ABpBinder::lookupOrCreateFromBinder(const ::android::sp<::android::IBinder>& binder) {

    if (binder == nullptr) {
@@ -301,6 +314,11 @@ sp<AIBinder> ABpBinder::lookupOrCreateFromBinder(const ::android::sp<::android:: 
    return ret;

}



void ABpBinder::addDeathRecipient(const ::android::sp<AIBinder_DeathRecipient>& recipient,

                                  void* cookie) {

    mDeathRecipients.emplace_back(recipient, cookie);

}



struct AIBinder_Weak {

    wp<AIBinder> binder;

};
@@ -426,6 +444,17 @@ AIBinder_DeathRecipient::AIBinder_DeathRecipient(AIBinder_DeathRecipient_onBinde 
    LOG_ALWAYS_FATAL_IF(onDied == nullptr, "onDied == nullptr");

}



void AIBinder_DeathRecipient::pruneThisTransferEntry(const sp<IBinder>& who, void* cookie) {

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

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

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

                                              auto tdrWho = tdr->getWho();

                                              return tdrWho != nullptr && tdrWho.promote() == who &&

                                                     cookie == tdr->getCookie();

                                          }),

                           mDeathRecipients.end());

}



void AIBinder_DeathRecipient::pruneDeadTransferEntriesLocked() {

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

                                          [](const sp<TransferDeathRecipient>& tdr) {
@@ -554,8 +583,11 @@ binder_status_t AIBinder_linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* 
        return STATUS_UNEXPECTED_NULL;

    }



    // returns binder_status_t

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

    binder_status_t ret = recipient->linkToDeath(binder->getBinder(), cookie);

    if (ret == STATUS_OK) {

        binder->addDeathRecipient(recipient, cookie);

    }

    return ret;

}



binder_status_t AIBinder_unlinkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,

libs/binder/ndk/ibinder_internal.h

+12 −0
Original line number Diff line number Diff line
@@ -51,6 +51,8 @@ struct AIBinder : public virtual ::android::RefBase { 
        ::android::sp<::android::IBinder> binder = const_cast<AIBinder*>(this)->getBinder();

        return binder->remoteBinder() != nullptr;

    }

    virtual void addDeathRecipient(const ::android::sp<AIBinder_DeathRecipient>& recipient,

                                   void* cookie) = 0;



   private:

    // AIBinder instance is instance of this class for a local object. In order to transact on a
@@ -78,6 +80,8 @@ struct ABBinder : public AIBinder, public ::android::BBinder { 
    ::android::status_t dump(int fd, const ::android::Vector<::android::String16>& args) override;

    ::android::status_t onTransact(uint32_t code, const ::android::Parcel& data,

                                   ::android::Parcel* reply, binder_flags_t flags) override;

    void addDeathRecipient(const ::android::sp<AIBinder_DeathRecipient>& /* recipient */,

                           void* /* cookie */) override;



   private:

    ABBinder(const AIBinder_Class* clazz, void* userData);
@@ -106,12 +110,19 @@ struct ABpBinder : public AIBinder { 


    bool isServiceFuzzing() const { return mServiceFuzzing; }

    void setServiceFuzzing() { mServiceFuzzing = true; }

    void addDeathRecipient(const ::android::sp<AIBinder_DeathRecipient>& recipient,

                           void* cookie) override;



   private:

    friend android::sp<ABpBinder>;

    explicit ABpBinder(const ::android::sp<::android::IBinder>& binder);

    ::android::sp<::android::IBinder> mRemote;

    bool mServiceFuzzing = false;

    struct DeathRecipientInfo {

        android::wp<AIBinder_DeathRecipient> recipient;

        void* cookie;

    };

    std::vector<DeathRecipientInfo> mDeathRecipients;

};



struct AIBinder_Class {
@@ -183,6 +194,7 @@ struct AIBinder_DeathRecipient : ::android::RefBase { 
    binder_status_t linkToDeath(const ::android::sp<::android::IBinder>&, void* cookie);

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

    void setOnUnlinked(AIBinder_DeathRecipient_onBinderUnlinked onUnlinked);

    void pruneThisTransferEntry(const ::android::sp<::android::IBinder>&, void* cookie);



   private:

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

libs/binder/ndk/tests/iface.cpp

+1 −0
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ using ::android::wp; 


const char* IFoo::kSomeInstanceName = "libbinder_ndk-test-IFoo";

const char* IFoo::kInstanceNameToDieFor = "libbinder_ndk-test-IFoo-to-die";

const char* IFoo::kInstanceNameToDieFor2 = "libbinder_ndk-test-IFoo-to-die2";

const char* IFoo::kIFooDescriptor = "my-special-IFoo-class";



struct IFoo_Class_Data {

libs/binder/ndk/tests/include/iface/iface.h

+1 −0
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ class IFoo : public virtual ::android::RefBase { 
   public:

    static const char* kSomeInstanceName;

    static const char* kInstanceNameToDieFor;

    static const char* kInstanceNameToDieFor2;

    static const char* kIFooDescriptor;



    static AIBinder_Class* kClass;

libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp

+123 −1
Original line number Diff line number Diff line
@@ -536,6 +536,7 @@ TEST(NdkBinder, DeathRecipient) { 
    bool deathReceived = false;



    std::function<void(void)> onDeath = [&] {

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

        std::cerr << "Binder died (as requested)." << std::endl;

        deathReceived = true;

        deathCv.notify_one();
@@ -547,6 +548,7 @@ TEST(NdkBinder, DeathRecipient) { 
    bool wasDeathReceivedFirst = false;



    std::function<void(void)> onUnlink = [&] {

        std::unique_lock<std::mutex> lockUnlink(unlinkMutex);

        std::cerr << "Binder unlinked (as requested)." << std::endl;

        wasDeathReceivedFirst = deathReceived;

        unlinkReceived = true;
@@ -560,7 +562,6 @@ TEST(NdkBinder, DeathRecipient) { 


    EXPECT_EQ(STATUS_OK, AIBinder_linkToDeath(binder, recipient, static_cast<void*>(cookie)));



    // the binder driver should return this if the service dies during the transaction

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



    foo = nullptr;
@@ -579,6 +580,123 @@ TEST(NdkBinder, DeathRecipient) { 
    binder = nullptr;

}



TEST(NdkBinder, DeathRecipientDropBinderNoDeath) {

    using namespace std::chrono_literals;



    std::mutex deathMutex;

    std::condition_variable deathCv;

    bool deathReceived = false;



    std::function<void(void)> onDeath = [&] {

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

        std::cerr << "Binder died (as requested)." << std::endl;

        deathReceived = true;

        deathCv.notify_one();

    };



    std::mutex unlinkMutex;

    std::condition_variable unlinkCv;

    bool unlinkReceived = false;

    bool wasDeathReceivedFirst = false;



    std::function<void(void)> onUnlink = [&] {

        std::unique_lock<std::mutex> lockUnlink(unlinkMutex);

        std::cerr << "Binder unlinked (as requested)." << std::endl;

        wasDeathReceivedFirst = deathReceived;

        unlinkReceived = true;

        unlinkCv.notify_one();

    };



    // keep the death recipient around

    ndk::ScopedAIBinder_DeathRecipient recipient(AIBinder_DeathRecipient_new(LambdaOnDeath));

    AIBinder_DeathRecipient_setOnUnlinked(recipient.get(), LambdaOnUnlink);



    {

        AIBinder* binder;

        sp<IFoo> foo = IFoo::getService(IFoo::kInstanceNameToDieFor2, &binder);

        ASSERT_NE(nullptr, foo.get());

        ASSERT_NE(nullptr, binder);



        DeathRecipientCookie* cookie = new DeathRecipientCookie{&onDeath, &onUnlink};



        EXPECT_EQ(STATUS_OK,

                  AIBinder_linkToDeath(binder, recipient.get(), static_cast<void*>(cookie)));

        // let the sp<IFoo> and AIBinder fall out of scope

        AIBinder_decStrong(binder);

        binder = nullptr;

    }



    {

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

        EXPECT_FALSE(deathCv.wait_for(lockDeath, 100ms, [&] { return deathReceived; }));

        EXPECT_FALSE(deathReceived);

    }



    {

        std::unique_lock<std::mutex> lockUnlink(unlinkMutex);

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

        EXPECT_TRUE(unlinkReceived);

        EXPECT_FALSE(wasDeathReceivedFirst);

    }

}



TEST(NdkBinder, DeathRecipientDropBinderOnDied) {

    using namespace std::chrono_literals;



    std::mutex deathMutex;

    std::condition_variable deathCv;

    bool deathReceived = false;



    sp<IFoo> foo;

    AIBinder* binder;

    std::function<void(void)> onDeath = [&] {

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

        std::cerr << "Binder died (as requested)." << std::endl;

        deathReceived = true;

        AIBinder_decStrong(binder);

        binder = nullptr;

        deathCv.notify_one();

    };



    std::mutex unlinkMutex;

    std::condition_variable unlinkCv;

    bool unlinkReceived = false;

    bool wasDeathReceivedFirst = false;



    std::function<void(void)> onUnlink = [&] {

        std::unique_lock<std::mutex> lockUnlink(unlinkMutex);

        std::cerr << "Binder unlinked (as requested)." << std::endl;

        wasDeathReceivedFirst = deathReceived;

        unlinkReceived = true;

        unlinkCv.notify_one();

    };



    ndk::ScopedAIBinder_DeathRecipient recipient(AIBinder_DeathRecipient_new(LambdaOnDeath));

    AIBinder_DeathRecipient_setOnUnlinked(recipient.get(), LambdaOnUnlink);



    foo = IFoo::getService(IFoo::kInstanceNameToDieFor2, &binder);

    ASSERT_NE(nullptr, foo.get());

    ASSERT_NE(nullptr, binder);



    DeathRecipientCookie* cookie = new DeathRecipientCookie{&onDeath, &onUnlink};

    EXPECT_EQ(STATUS_OK, AIBinder_linkToDeath(binder, recipient.get(), static_cast<void*>(cookie)));



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



    {

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

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

        EXPECT_TRUE(deathReceived);

    }



    {

        std::unique_lock<std::mutex> lockUnlink(unlinkMutex);

        EXPECT_TRUE(deathCv.wait_for(lockUnlink, 100ms, [&] { return unlinkReceived; }));

        EXPECT_TRUE(unlinkReceived);

        EXPECT_TRUE(wasDeathReceivedFirst);

    }

}



TEST(NdkBinder, RetrieveNonNdkService) {

#pragma clang diagnostic push

#pragma clang diagnostic ignored "-Wdeprecated-declarations"
@@ -956,6 +1074,10 @@ int main(int argc, char* argv[]) { 
        prctl(PR_SET_PDEATHSIG, SIGHUP);

        return manualThreadPoolService(IFoo::kInstanceNameToDieFor);

    }

    if (fork() == 0) {

        prctl(PR_SET_PDEATHSIG, SIGHUP);

        return manualThreadPoolService(IFoo::kInstanceNameToDieFor2);

    }

    if (fork() == 0) {

        prctl(PR_SET_PDEATHSIG, SIGHUP);

        return manualPollingService(IFoo::kSomeInstanceName);