commit 64127ca21d37ebf5d812cd026a1a8084a2fa39df
author Steven Moreland <smoreland@google.com> Wed Mar 13 09:25:44 2019 -0700
committer Steven Moreland <smoreland@google.com> Mon Mar 18 17:17:00 2019 -0700
tree 49430e44841f3cfd808aae7be5c181a2b3c326a5
parent 91e66f9ba18f3d053859c58d6af97cdd4f79579b

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

libs/binder/ndk/ibinder.cpp

diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index bdbc0d3..bd6886d 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -269,6 +269,18 @@
     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 @@
     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 @@
     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 @@
     }
 
     // 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 @@
     }
 
     // returns binder_status_t
-    return recipient->unlinkToDeath(binder, cookie);
+    return recipient->unlinkToDeath(binder->getBinder(), cookie);
 }
 
 uid_t AIBinder_getCallingUid() {
@@ -555,9 +577,15 @@
         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);
 }