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

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

libbinder: RPC use DEAD_OBJECT over -ECANCELED 

When we shutdown a connection with FdTrigger, we return -ECANCELED.
However, the next call to that FD will return with -EPIPE
(DEAD_OBJECT). It's not really important: if your call happens to get
interrupted, you'll get -ECANCELED or if your call happened to be
scheduled slightly later, you'd get DEAD_OBJECT. Actually, we don't care
to distinguish between these two cases because existing clients of
libbinder only have and consider DEAD_OBJECT now. Whether the server is
shutting down right now, or you make the call a tiny bit later, we want
to return the exact same error code in order to be consistent w/ the
behavior of libbinder when it uses the binder driver.

This was causing a flake in a later CL (fixing the 'Die' race), when
rpcSend failing causes the server to shutdown.

Bug: 200167417
Test: binderRpcTest 'Callbacks' case repeated 100s of times
Change-Id: Id2977f05eb249691326955e6f2424d4e5e08b417
parent 963b7bba

libs/binder/FdTrigger.cpp

+1 −1
Original line number Diff line number Diff line
@@ -53,7 +53,7 @@ status_t FdTrigger::triggerablePoll(base::borrowed_fd fd, int16_t event) { 
            continue;

        }

        if (pfd[1].revents & POLLHUP) {

            return -ECANCELED;

            return DEAD_OBJECT;

        }

        return pfd[0].revents & event ? OK : DEAD_OBJECT;

    }

libs/binder/RpcTransportTls.cpp

+3 −3
Original line number Diff line number Diff line
@@ -241,7 +241,7 @@ private: 
    status_t handlePoll(int event, android::base::borrowed_fd fd, FdTrigger* fdTrigger,

                        const char* fnString) {

        status_t ret = fdTrigger->triggerablePoll(fd, event);

        if (ret != OK && ret != DEAD_OBJECT && ret != -ECANCELED) {

        if (ret != OK && ret != DEAD_OBJECT) {

            ALOGE("triggerablePoll error while poll()-ing after %s(): %s", fnString,

                  statusToString(ret).c_str());

        }
@@ -348,7 +348,7 @@ status_t RpcTransportTls::interruptableWriteFully(FdTrigger* fdTrigger, const vo 


    // Before doing any I/O, check trigger once. This ensures the trigger is checked at least

    // once. The trigger is also checked via triggerablePoll() after every SSL_write().

    if (fdTrigger->isTriggered()) return -ECANCELED;

    if (fdTrigger->isTriggered()) return DEAD_OBJECT;



    while (buffer < end) {

        size_t todo = std::min<size_t>(end - buffer, std::numeric_limits<int>::max());
@@ -379,7 +379,7 @@ status_t RpcTransportTls::interruptableReadFully(FdTrigger* fdTrigger, void* dat 


    // Before doing any I/O, check trigger once. This ensures the trigger is checked at least

    // once. The trigger is also checked via triggerablePoll() after every SSL_write().

    if (fdTrigger->isTriggered()) return -ECANCELED;

    if (fdTrigger->isTriggered()) return DEAD_OBJECT;



    while (buffer < end) {

        size_t todo = std::min<size_t>(end - buffer, std::numeric_limits<int>::max());

libs/binder/tests/binderRpcTest.cpp

+3 −3
Original line number Diff line number Diff line
@@ -1794,9 +1794,9 @@ TEST_P(RpcTransportTest, Trigger) { 
        }



        status = serverTransport->interruptableWriteFully(fdTrigger, msg2.data(), msg2.size());

        if (status != -ECANCELED)

        if (status != DEAD_OBJECT)

            return AssertionFailure() << "When FdTrigger is shut down, interruptableWriteFully "

                                         "should return -ECANCELLED, but it is "

                                         "should return DEAD_OBJECT, but it is "

                                      << statusToString(status);

        return AssertionSuccess();

    };
@@ -1830,7 +1830,7 @@ TEST_P(RpcTransportTest, Trigger) { 
    }

    writeCv.notify_all();

    // After this line, server thread unblocks and attempts to write the second message, but

    // shutdown is triggered, so write should failed with -ECANCELLED. See |serverPostConnect|.

    // shutdown is triggered, so write should failed with DEAD_OBJECT. See |serverPostConnect|.

    // On the client side, second read fails with DEAD_OBJECT

    ASSERT_FALSE(client.readMessage(msg2));

}