libbinder: RPC process oneway w/ 'tail call'

When draining oneway commands (which must be serialized), we do a
recursive call to process a transaction. However, this wouldn't even be
considered to be a tailcall because of the complex destructors which
need to run. So, instead we work around this w/ goto to the beginning of
the function.

The alternative here (to a 'goto') to consider is creating a more
complex return type to processTransactInternal which would convince
processTransact to re-issue the command. Though, this would be a
somewhat larger refactor.

Fixes: 190638569
Test: binderRpcTest (OnewayStressTest repeatedly on device doesn't fail
    for several minutes - failed without this)
Change-Id: I9fbc75941452348e498849d5d59130487ef6cc44

1 files changed, 12 insertions, 7 deletions

diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 6899981e83..62eb58adba 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -565,7 +565,7 @@ status_t RpcState::processTransact(const base::unique_fd& fd, const sp<RpcSessio
         status != OK)
         return status;
 
-    return processTransactInternal(fd, session, std::move(transactionData), nullptr /*targetRef*/);
+    return processTransactInternal(fd, session, std::move(transactionData));
 }
 
 static void do_nothing_to_transact_data(Parcel* p, const uint8_t* data, size_t dataSize,
@@ -578,7 +578,13 @@ static void do_nothing_to_transact_data(Parcel* p, const uint8_t* data, size_t d
 }
 
 status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<RpcSession>& session,
-                                           CommandData transactionData, sp<IBinder>&& targetRef) {
+                                           CommandData transactionData) {
+    // for 'recursive' calls to this, we have already read and processed the
+    // binder from the transaction data and taken reference counts into account,
+    // so it is cached here.
+    sp<IBinder> targetRef;
+processTransactInternalTailCall:
+
     if (transactionData.size() < sizeof(RpcWireTransaction)) {
         ALOGE("Expecting %zu but got %zu bytes for RpcWireTransaction. Terminating!",
               sizeof(RpcWireTransaction), transactionData.size());
@@ -751,13 +757,12 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<R
                 // - gotta go fast
                 auto& todo = const_cast<BinderNode::AsyncTodo&>(it->second.asyncTodo.top());
 
-                CommandData nextData = std::move(todo.data);
-                sp<IBinder> nextRef = std::move(todo.ref);
+                // reset up arguments
+                transactionData = std::move(todo.data);
+                targetRef = std::move(todo.ref);
 
                 it->second.asyncTodo.pop();
-                _l.unlock();
-                return processTransactInternal(fd, session, std::move(nextData),
-                                               std::move(nextRef));
+                goto processTransactInternalTailCall;
             }
         }
         return OK;