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

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

libbinder: oneway calls delay refcounting 

For oneway calls, we might be processing many at the same time. We can
avoid sending any decref calls until we finish processing all oneway
transactions. So, for instance, if we send 100 transactions, and they
can all be processed at the same time, we'll only write one response to
update the refcount in the caller process.

Note: this was written to try to fix the deadlock in "libbinder: RPC avoid
poll", but it does not. Still, I think it's a good improvement.

Bug: 182940634
Test: binderRpcTest
Change-Id: Ieb5374127cc79222b09f5efb12436211f609c9bb
parent fd1e8a03

libs/binder/Parcel.cpp

+3 −0
Original line number Diff line number Diff line
@@ -291,6 +291,9 @@ status_t Parcel::unflattenBinder(sp<IBinder>* out) const 
            if (status_t status = mSession->state()->onBinderEntering(mSession, addr, &binder);

                status != OK)

                return status;

            if (status_t status = mSession->state()->flushExcessBinderRefs(mSession, addr, binder);

                status != OK)

                return status;

        }



        return finishUnflattenBinder(binder, out);

libs/binder/RpcState.cpp

+45 −15
Original line number Diff line number Diff line
@@ -152,7 +152,7 @@ status_t RpcState::onBinderEntering(const sp<RpcSession>& session, uint64_t addr 
        return BAD_VALUE;

    }



    std::unique_lock<std::mutex> _l(mNodeMutex);

    std::lock_guard<std::mutex> _l(mNodeMutex);

    if (mTerminated) return DEAD_OBJECT;



    if (auto it = mNodeForAddress.find(address); it != mNodeForAddress.end()) {
@@ -160,20 +160,7 @@ status_t RpcState::onBinderEntering(const sp<RpcSession>& session, uint64_t addr 


        // implicitly have strong RPC refcount, since we received this binder

        it->second.timesRecd++;



        _l.unlock();



        // 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 = (*out)->localBinder() ? 0 : 1;



        // 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.

        return session->sendDecStrongToTarget(address, targetRecd);

        return OK;

    }



    // we don't know about this binder, so the other side of the connection
@@ -194,6 +181,36 @@ status_t RpcState::onBinderEntering(const sp<RpcSession>& session, uint64_t addr 
    return OK;

}



status_t RpcState::flushExcessBinderRefs(const sp<RpcSession>& session, uint64_t address,

                                         const sp<IBinder>& binder) {

    std::unique_lock<std::mutex> _l(mNodeMutex);

    if (mTerminated) return DEAD_OBJECT;



    auto it = mNodeForAddress.find(address);



    LOG_ALWAYS_FATAL_IF(it == mNodeForAddress.end(), "Can't be deleted while we hold sp<>");

    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;



    // 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) {

        _l.unlock();



        return session->sendDecStrongToTarget(address, targetRecd);

    }



    return OK;

}



size_t RpcState::countBinders() {

    std::lock_guard<std::mutex> _l(mNodeMutex);

    return mNodeForAddress.size();
@@ -841,6 +858,12 @@ processTransactInternalTailCall: 
        }

    }



    // Binder refs are flushed for oneway calls only after all calls which are

    // built up are executed. Otherwise, they fill up the binder buffer.

    if (addr != 0 && replyStatus == OK && !oneway) {

        replyStatus = flushExcessBinderRefs(session, addr, target);

    }



    if (oneway) {

        if (replyStatus != OK) {

            ALOGW("Oneway call failed with error: %d", replyStatus);
@@ -889,6 +912,13 @@ processTransactInternalTailCall: 
                goto processTransactInternalTailCall;

            }

        }



        // done processing all the async commands on this binder that we can, so

        // write decstrongs on the binder

        if (addr != 0 && replyStatus == OK) {

            return flushExcessBinderRefs(session, addr, target);

        }



        return OK;

    }

libs/binder/RpcState.h

+7 −0
Original line number Diff line number Diff line
@@ -129,6 +129,13 @@ public: 
     */

    [[nodiscard]] status_t onBinderEntering(const sp<RpcSession>& session, uint64_t address,

                                            sp<IBinder>* out);

    /**

     * Called on incoming binders to update refcounting information. This should

     * only be called when it is done as part of making progress on a

     * transaction.

     */

    [[nodiscard]] status_t flushExcessBinderRefs(const sp<RpcSession>& session, uint64_t address,

                                                 const sp<IBinder>& binder);



    size_t countBinders();

    void dump();