commit e96ed0ed360cddfc3edb0cad48e85120f80674ba
author Steven Moreland <smoreland@google.com> Mon Sep 27 17:43:53 2021 -0700
committer Steven Moreland <smoreland@google.com> Tue Sep 28 01:09:42 2021 +0000
tree 32ee11feec52e9972eea87d27a268c98b2093c37
parent 43921d5d92d6e27bf1ec00de062746f032268717

libbinder: RPC delay bp refcounts

For proxy binders, we don't need to be sending reference count
information. We can drop all RPC refs when the BpBinder is deleted.

Bug: 182940634
Test: binderRpcTest
Change-Id: I9b37495aceeea70fbe12b6cc0c90b903e5f5439d

libs/binder/RpcState.cpp

diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index a3079fe..82efe6d 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -183,6 +183,10 @@
 
 status_t RpcState::flushExcessBinderRefs(const sp<RpcSession>& session, uint64_t address,
                                          const sp<IBinder>& binder) {
+    // We can flush all references when the binder is destroyed. No need to send
+    // extra reference counting packets now.
+    if (binder->remoteBinder()) return OK;
+
     std::unique_lock<std::mutex> _l(mNodeMutex);
     if (mTerminated) return DEAD_OBJECT;
 
@@ -192,20 +196,19 @@
     LOG_ALWAYS_FATAL_IF(it->second.binder != binder,
                         "Caller of flushExcessBinderRefs using inconsistent arguments");
 
-    // if this is a local binder, then we want to get rid of all refcounts
-    // (tell the other process it can drop the binder when it wants to - we
-    // have a local sp<>, so we will drop it when we want to as well). if
-    // this is a remote binder, then we need to hold onto one refcount until
-    // it is dropped in BpBinder::onLastStrongRef
-    size_t targetRecd = binder->localBinder() ? 0 : 1;
+    LOG_ALWAYS_FATAL_IF(it->second.timesSent <= 0, "Local binder must have been sent %p",
+                        binder.get());
 
-    // We have timesRecd RPC refcounts, but we only need to hold on to one
-    // when we keep the object. All additional dec strongs are sent
-    // immediately, we wait to send the last one in BpBinder::onLastDecStrong.
-    if (it->second.timesRecd != targetRecd) {
+    // For a local binder, we only need to know that we sent it. Now that we
+    // have an sp<> for this call, we don't need anything more. If the other
+    // process is done with this binder, it needs to know we received the
+    // refcount associated with this call, so we can acknowledge that we
+    // received it. Once (or if) it has no other refcounts, it would reply with
+    // its own decStrong so that it could be removed from this session.
+    if (it->second.timesRecd != 0) {
         _l.unlock();
 
-        return session->sendDecStrongToTarget(address, targetRecd);
+        return session->sendDecStrongToTarget(address, 0);
     }
 
     return OK;