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

Commit a7e51752 authored by Steven Moreland's avatar Steven Moreland Committed by Gerrit Code Review
Browse files

Merge changes Ieb537412,I2339a8e0 

* changes:
  libbinder: oneway calls delay refcounting
  libbinder: dec refs include count
parents 13ba6f3c d8083310

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

+5 −3
Original line number Diff line number Diff line
@@ -217,15 +217,17 @@ status_t RpcSession::transact(const sp<IBinder>& binder, uint32_t code, const Pa 
}



status_t RpcSession::sendDecStrong(const BpBinder* binder) {

    return sendDecStrong(binder->getPrivateAccessor().rpcAddress());

    // target is 0 because this is used to free BpBinder objects

    return sendDecStrongToTarget(binder->getPrivateAccessor().rpcAddress(), 0 /*target*/);

}



status_t RpcSession::sendDecStrong(uint64_t address) {

status_t RpcSession::sendDecStrongToTarget(uint64_t address, size_t target) {

    ExclusiveConnection connection;

    status_t status = ExclusiveConnection::find(sp<RpcSession>::fromExisting(this),

                                                ConnectionUse::CLIENT_REFCOUNT, &connection);

    if (status != OK) return status;

    return state()->sendDecStrong(connection.get(), sp<RpcSession>::fromExisting(this), address);

    return state()->sendDecStrongToTarget(connection.get(), sp<RpcSession>::fromExisting(this),

                                          address, target);

}



status_t RpcSession::readId() {

libs/binder/RpcState.cpp

+77 −26
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,13 +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();



        // 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->sendDecStrong(address);

        return OK;

    }



    // we don't know about this binder, so the other side of the connection
@@ -187,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();
@@ -571,32 +595,43 @@ status_t RpcState::waitForReply(const sp<RpcSession::RpcConnection>& connection, 
    return OK;

}



status_t RpcState::sendDecStrong(const sp<RpcSession::RpcConnection>& connection,

                                 const sp<RpcSession>& session, uint64_t addr) {

status_t RpcState::sendDecStrongToTarget(const sp<RpcSession::RpcConnection>& connection,

                                         const sp<RpcSession>& session, uint64_t addr,

                                         size_t target) {

    RpcDecStrong body = {

            .address = RpcWireAddress::fromRaw(addr),

    };



    {

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

        if (mTerminated) return DEAD_OBJECT; // avoid fatal only, otherwise races

        auto it = mNodeForAddress.find(addr);

        LOG_ALWAYS_FATAL_IF(it == mNodeForAddress.end(),

                            "Sending dec strong on unknown address %" PRIu64, addr);

        LOG_ALWAYS_FATAL_IF(it->second.timesRecd <= 0, "Bad dec strong %" PRIu64, addr);



        it->second.timesRecd--;

        LOG_ALWAYS_FATAL_IF(it->second.timesRecd < target, "Can't dec count of %zu to %zu.",

                            it->second.timesRecd, target);



        // typically this happens when multiple threads send dec refs at the

        // same time - the transactions will get combined automatically

        if (it->second.timesRecd == target) return OK;



        body.amount = it->second.timesRecd - target;

        it->second.timesRecd = target;



        LOG_ALWAYS_FATAL_IF(nullptr != tryEraseNode(it),

                            "Bad state. RpcState shouldn't own received binder");

    }



    RpcWireHeader cmd = {

            .command = RPC_COMMAND_DEC_STRONG,

            .bodySize = sizeof(RpcWireAddress),

            .bodySize = sizeof(RpcDecStrong),

    };

    if (status_t status = rpcSend(connection, session, "dec ref header", &cmd, sizeof(cmd));

        status != OK)

        return status;

    if (status_t status = rpcSend(connection, session, "dec ref body", &addr, sizeof(addr));

        status != OK)

        return status;

    return OK;



    return rpcSend(connection, session, "dec ref body", &body, sizeof(body));

}



status_t RpcState::getAndExecuteCommand(const sp<RpcSession::RpcConnection>& connection,
@@ -827,6 +862,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);
@@ -875,6 +916,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;

    }
@@ -916,16 +964,15 @@ status_t RpcState::processDecStrong(const sp<RpcSession::RpcConnection>& connect 
        status != OK)

        return status;



    if (command.bodySize != sizeof(RpcWireAddress)) {

        ALOGE("Expecting %zu but got %" PRId32 " bytes for RpcWireAddress. Terminating!",

              sizeof(RpcWireAddress), command.bodySize);

    if (command.bodySize != sizeof(RpcDecStrong)) {

        ALOGE("Expecting %zu but got %" PRId32 " bytes for RpcDecStrong. Terminating!",

              sizeof(RpcDecStrong), command.bodySize);

        (void)session->shutdownAndWait(false);

        return BAD_VALUE;

    }

    RpcWireAddress* address = reinterpret_cast<RpcWireAddress*>(commandData.data());



    uint64_t addr = RpcWireAddress::toRaw(*address);

    RpcDecStrong* body = reinterpret_cast<RpcDecStrong*>(commandData.data());



    uint64_t addr = RpcWireAddress::toRaw(body->address);

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

    auto it = mNodeForAddress.find(addr);

    if (it == mNodeForAddress.end()) {
@@ -943,15 +990,19 @@ status_t RpcState::processDecStrong(const sp<RpcSession::RpcConnection>& connect 
        return BAD_VALUE;

    }



    if (it->second.timesSent == 0) {

        ALOGE("No record of sending binder, but requested decStrong: %" PRIu64, addr);

    if (it->second.timesSent < body->amount) {

        ALOGE("Record of sending binder %zu times, but requested decStrong for %" PRIu64 " of %u",

              it->second.timesSent, addr, body->amount);

        return OK;

    }



    LOG_ALWAYS_FATAL_IF(it->second.sentRef == nullptr, "Inconsistent state, lost ref for %" PRIu64,

                        addr);



    it->second.timesSent--;

    LOG_RPC_DETAIL("Processing dec strong of %" PRIu64 " by %u from %zu", addr, body->amount,

                   it->second.timesSent);



    it->second.timesSent -= body->amount;

    sp<IBinder> tempHold = tryEraseNode(it);

    _l.unlock();

    tempHold = nullptr; // destructor may make binder calls on this session

libs/binder/RpcState.h

+30 −2
Original line number Diff line number Diff line
@@ -82,8 +82,29 @@ public: 
                                           uint64_t address, uint32_t code, const Parcel& data,

                                           const sp<RpcSession>& session, Parcel* reply,

                                           uint32_t flags);

    [[nodiscard]] status_t sendDecStrong(const sp<RpcSession::RpcConnection>& connection,

                                         const sp<RpcSession>& session, uint64_t address);



    /**

     * The ownership model here carries an implicit strong refcount whenever a

     * binder is sent across processes. Since we have a local strong count in

     * sp<> over these objects, we only ever need to keep one of these. So,

     * typically we tell the remote process that we drop all the implicit dec

     * strongs, and we hold onto the last one. 'target' here is the target

     * timesRecd (the number of remaining reference counts) we wish to keep.

     * Typically this should be '0' or '1'. The target is used instead of an

     * explicit decrement count in order to allow multiple threads to lower the

     * number of counts simultaneously. Since we only lower the count to 0 when

     * a binder is deleted, targets of '1' should only be sent when the caller

     * owns a local strong reference to the binder. Larger targets may be used

     * for testing, and to make the function generic, but generally this should

     * be avoided because it would be hard to guarantee another thread doesn't

     * lower the number of held refcounts to '1'. Note also, these refcounts

     * must be sent actively. If they are sent when binders are deleted, this

     * can cause leaks, since even remote binders carry an implicit strong ref

     * when they are sent to another process.

     */

    [[nodiscard]] status_t sendDecStrongToTarget(const sp<RpcSession::RpcConnection>& connection,

                                                 const sp<RpcSession>& session, uint64_t address,

                                                 size_t target);



    enum class CommandType {

        ANY,
@@ -108,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();

libs/binder/RpcWireFormat.h

+8 −1
Original line number Diff line number Diff line
@@ -85,7 +85,7 @@ enum : uint32_t { 
     */

    RPC_COMMAND_REPLY,

    /**

     * follows is RpcWireAddress

     * follows is RpcDecStrong

     *

     * note - this in the protocol directly instead of as a 'special

     * transaction' in order to keep it as lightweight as possible (we don't
@@ -117,6 +117,13 @@ struct RpcWireHeader { 
};

static_assert(sizeof(RpcWireHeader) == 16);



struct RpcDecStrong {

    RpcWireAddress address;

    uint32_t amount;

    uint32_t reserved;

};

static_assert(sizeof(RpcDecStrong) == 16);



struct RpcWireTransaction {

    RpcWireAddress address;

    uint32_t code;