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
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 6899981..62eb58a 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -565,7 +565,7 @@
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 @@
}
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 @@
// - 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;