Merge "libbinder: RPC clear behavior" am: 4d9b036fbb am: 4abc23f79d am: 9d71de850e am: 17b7dc0d7d am: 752da342f9

Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2154256

Change-Id: I2b64f6fa347dafb57b1a56ba2b02ddb485ba2127
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

2 files changed, 13 insertions, 22 deletions

diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 1c5654c597..d063001414 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -244,33 +244,31 @@ void RpcState::clear() {
                             "New state should be impossible after terminating!");
         return;
     }
+    mTerminated = true;
 
     if (SHOULD_LOG_RPC_DETAIL) {
         ALOGE("RpcState::clear()");
         dumpLocked();
     }
 
-    // if the destructor of a binder object makes another RPC call, then calling
-    // decStrong could deadlock. So, we must hold onto these binders until
-    // mNodeMutex is no longer taken.
-    std::vector<sp<IBinder>> tempHoldBinder;
-
-    mTerminated = true;
+    // invariants
     for (auto& [address, node] : mNodeForAddress) {
-        sp<IBinder> binder = node.binder.promote();
-        LOG_ALWAYS_FATAL_IF(binder == nullptr,
-                            "Binder expected to be owned with address: %" PRIu64 " %s", address,
-                            node.toString().c_str());
-
-        if (node.sentRef != nullptr) {
-            tempHoldBinder.push_back(node.sentRef);
+        bool guaranteedHaveBinder = node.timesSent > 0;
+        if (guaranteedHaveBinder) {
+            LOG_ALWAYS_FATAL_IF(node.sentRef == nullptr,
+                                "Binder expected to be owned with address: %" PRIu64 " %s", address,
+                                node.toString().c_str());
         }
     }
 
-    mNodeForAddress.clear();
+    // if the destructor of a binder object makes another RPC call, then calling
+    // decStrong could deadlock. So, we must hold onto these binders until
+    // mNodeMutex is no longer taken.
+    auto temp = std::move(mNodeForAddress);
+    mNodeForAddress.clear(); // RpcState isn't reusable, but for future/explicit
 
     _l.unlock();
-    tempHoldBinder.clear(); // explicit
+    temp.clear(); // explicit
 }
 
 void RpcState::dumpLocked() {
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index fca3c29ecd..8afb4031fb 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -1029,13 +1029,6 @@ TEST_P(BinderRpc, Callbacks) {
                 // since this session has an incoming connection w/ a threadpool, we
                 // need to manually shut it down
                 EXPECT_TRUE(proc.proc.sessions.at(0).session->shutdownAndWait(true));
-
-                proc.proc.host.setCustomExitStatusCheck([](int wstatus) {
-                    // Flaky. Sometimes gets SIGABRT.
-                    EXPECT_TRUE((WIFEXITED(wstatus) && WEXITSTATUS(wstatus) == 0) ||
-                                (WIFSIGNALED(wstatus) && WTERMSIG(wstatus) == SIGABRT))
-                            << "server process failed: " << WaitStatusToString(wstatus);
-                });
                 proc.expectAlreadyShutdown = true;
             }
         }