diff options
| -rw-r--r-- | libs/binder/include/binder/IBinder.h | 3 | ||||
| -rw-r--r-- | libs/binder/ndk/ibinder.cpp | 48 | ||||
| -rw-r--r-- | libs/binder/ndk/ibinder_internal.h | 19 | ||||
| -rw-r--r-- | libs/binder/ndk/include_ndk/android/binder_ibinder.h | 5 | ||||
| -rw-r--r-- | libs/binder/ndk/test/main_client.cpp | 5 |
5 files changed, 65 insertions, 15 deletions
diff --git a/libs/binder/include/binder/IBinder.h b/libs/binder/include/binder/IBinder.h index 14edcbe72a..06309637f7 100644 --- a/libs/binder/include/binder/IBinder.h +++ b/libs/binder/include/binder/IBinder.h @@ -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, diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp index bdbc0d3b8b..bd6886d1ee 100644 --- a/libs/binder/ndk/ibinder.cpp +++ b/libs/binder/ndk/ibinder.cpp @@ -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); } diff --git a/libs/binder/ndk/ibinder_internal.h b/libs/binder/ndk/ibinder_internal.h index 202d6d2e6a..5cb68c291b 100644 --- a/libs/binder/ndk/ibinder_internal.h +++ b/libs/binder/ndk/ibinder_internal.h @@ -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; diff --git a/libs/binder/ndk/include_ndk/android/binder_ibinder.h b/libs/binder/ndk/include_ndk/android/binder_ibinder.h index bddc10de42..80d12541be 100644 --- a/libs/binder/ndk/include_ndk/android/binder_ibinder.h +++ b/libs/binder/ndk/include_ndk/android/binder_ibinder.h @@ -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. diff --git a/libs/binder/ndk/test/main_client.cpp b/libs/binder/ndk/test/main_client.cpp index bff601ec26..8467734c75 100644 --- a/libs/binder/ndk/test/main_client.cpp +++ b/libs/binder/ndk/test/main_client.cpp @@ -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) { |