Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ada72bd2 authored by Steven Moreland's avatar Steven Moreland
Browse files

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
parent 7f26fabe

libs/binder/RpcState.cpp

+12 −7
Original line number Diff line number Diff line
@@ -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;

libs/binder/RpcState.h

+1 −2
Original line number Diff line number Diff line
@@ -144,8 +144,7 @@ private: 
                                           const RpcWireHeader& command);

    [[nodiscard]] status_t processTransactInternal(const base::unique_fd& fd,

                                                   const sp<RpcSession>& session,

                                                   CommandData transactionData,

                                                   sp<IBinder>&& targetRef);

                                                   CommandData transactionData);

    [[nodiscard]] status_t processDecStrong(const base::unique_fd& fd,

                                            const sp<RpcSession>& session,

                                            const RpcWireHeader& command);