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

Commit c742a7f1 authored by David Pursell's avatar David Pursell
Browse files

fastboot: add Socket timeout detection. 

UDP fastboot will require re-transmission in the case of datagrams
getting lost. This CL adds Socket functionality to easily distinguish
between a normal timeout and a socket failure.

I also found some Windows docs that indicate sockets may become
invalid after a call to recv() times out. This has never occurred in
my testing, but to be safe this switches the timeout implementation
to use select() instead of SO_RCVTIMEO.

Bug: http://b/26154914
Change-Id: Id7b598f8aea5df1a3676d24702b489042d5f9e3a
parent aae1eb2c

fastboot/socket.cpp

+36 −14
Original line number Diff line number Diff line
@@ -48,18 +48,6 @@ int Socket::Close() { 
    return ret;

}



bool Socket::SetReceiveTimeout(int timeout_ms) {

    if (timeout_ms != receive_timeout_ms_) {

        if (socket_set_receive_timeout(sock_, timeout_ms) == 0) {

            receive_timeout_ms_ = timeout_ms;

            return true;

        }

        return false;

    }



    return true;

}



ssize_t Socket::ReceiveAll(void* data, size_t length, int timeout_ms) {

    size_t total = 0;
@@ -82,6 +70,40 @@ int Socket::GetLocalPort() { 
    return socket_get_local_port(sock_);

}



// According to Windows setsockopt() documentation, if a Windows socket times out during send() or

// recv() the state is indeterminate and should not be used. Our UDP protocol relies on being able

// to re-send after a timeout, so we must use select() rather than SO_RCVTIMEO.

// See https://msdn.microsoft.com/en-us/library/windows/desktop/ms740476(v=vs.85).aspx.

bool Socket::WaitForRecv(int timeout_ms) {

    receive_timed_out_ = false;



    // In our usage |timeout_ms| <= 0 means block forever, so just return true immediately and let

    // the subsequent recv() do the blocking.

    if (timeout_ms <= 0) {

        return true;

    }



    // select() doesn't always check this case and will block for |timeout_ms| if we let it.

    if (sock_ == INVALID_SOCKET) {

        return false;

    }



    fd_set read_set;

    FD_ZERO(&read_set);

    FD_SET(sock_, &read_set);



    timeval timeout;

    timeout.tv_sec = timeout_ms / 1000;

    timeout.tv_usec = (timeout_ms % 1000) * 1000;



    int result = TEMP_FAILURE_RETRY(select(sock_ + 1, &read_set, nullptr, nullptr, &timeout));



    if (result == 0) {

        receive_timed_out_ = true;

    }

    return result == 1;

}



// Implements the Socket interface for UDP.

class UdpSocket : public Socket {

  public:
@@ -127,7 +149,7 @@ bool UdpSocket::Send(std::vector<cutils_socket_buffer_t> buffers) { 
}



ssize_t UdpSocket::Receive(void* data, size_t length, int timeout_ms) {

    if (!SetReceiveTimeout(timeout_ms)) {

    if (!WaitForRecv(timeout_ms)) {

        return -1;

    }
@@ -206,7 +228,7 @@ bool TcpSocket::Send(std::vector<cutils_socket_buffer_t> buffers) { 
}



ssize_t TcpSocket::Receive(void* data, size_t length, int timeout_ms) {

    if (!SetReceiveTimeout(timeout_ms)) {

    if (!WaitForRecv(timeout_ms)) {

        return -1;

    }

fastboot/socket.h

+11 −6
Original line number Diff line number Diff line
@@ -81,13 +81,17 @@ class Socket { 
    virtual bool Send(std::vector<cutils_socket_buffer_t> buffers) = 0;



    // Waits up to |timeout_ms| to receive up to |length| bytes of data. |timout_ms| of 0 will

    // block forever. Returns the number of bytes received or -1 on error/timeout. On timeout

    // errno will be set to EAGAIN or EWOULDBLOCK.

    // block forever. Returns the number of bytes received or -1 on error/timeout; see

    // ReceiveTimedOut() to distinguish between the two.

    virtual ssize_t Receive(void* data, size_t length, int timeout_ms) = 0;



    // Calls Receive() until exactly |length| bytes have been received or an error occurs.

    virtual ssize_t ReceiveAll(void* data, size_t length, int timeout_ms);



    // Returns true if the last Receive() call timed out normally and can be retried; fatal errors

    // or successful reads will return false.

    bool ReceiveTimedOut() { return receive_timed_out_; }



    // Closes the socket. Returns 0 on success, -1 on error.

    virtual int Close();
@@ -102,10 +106,13 @@ class Socket { 
    // Protected constructor to force factory function use.

    Socket(cutils_socket_t sock);



    // Update the socket receive timeout if necessary.

    bool SetReceiveTimeout(int timeout_ms);

    // Blocks up to |timeout_ms| until a read is possible on |sock_|, and sets |receive_timed_out_|

    // as appropriate to help distinguish between normal timeouts and fatal errors. Returns true if

    // a subsequent recv() on |sock_| will complete without blocking or if |timeout_ms| <= 0.

    bool WaitForRecv(int timeout_ms);



    cutils_socket_t sock_ = INVALID_SOCKET;

    bool receive_timed_out_ = false;



    // Non-class functions we want to override during tests to verify functionality. Implementation

    // should call this rather than using socket_send_buffers() directly.
@@ -113,8 +120,6 @@ class Socket { 
            socket_send_buffers_function_ = &socket_send_buffers;



  private:

    int receive_timeout_ms_ = 0;



    FRIEND_TEST(SocketTest, TestTcpSendBuffers);

    FRIEND_TEST(SocketTest, TestUdpSendBuffers);

fastboot/socket_mock.cpp

+24 −14
Original line number Diff line number Diff line
@@ -55,7 +55,7 @@ bool SocketMock::Send(const void* data, size_t length) { 
        return false;

    }



    bool return_value = events_.front().return_value;

    bool return_value = events_.front().status;

    events_.pop();

    return return_value;

}
@@ -76,21 +76,28 @@ ssize_t SocketMock::Receive(void* data, size_t length, int /*timeout_ms*/) { 
        return -1;

    }



    if (events_.front().type != EventType::kReceive) {

    const Event& event = events_.front();

    if (event.type != EventType::kReceive) {

        ADD_FAILURE() << "Receive() was called out-of-order";

        return -1;

    }



    if (events_.front().return_value > static_cast<ssize_t>(length)) {

        ADD_FAILURE() << "Receive(): not enough bytes (" << length << ") for "

                      << events_.front().message;

    const std::string& message = event.message;

    if (message.length() > length) {

        ADD_FAILURE() << "Receive(): not enough bytes (" << length << ") for " << message;

        return -1;

    }



    ssize_t return_value = events_.front().return_value;

    if (return_value > 0) {

        memcpy(data, events_.front().message.data(), return_value);

    receive_timed_out_ = event.status;

    ssize_t return_value = message.length();



    // Empty message indicates failure.

    if (message.empty()) {

        return_value = -1;

    } else {

        memcpy(data, message.data(), message.length());

    }



    events_.pop();

    return return_value;

}
@@ -124,18 +131,21 @@ void SocketMock::ExpectSendFailure(std::string message) { 
}



void SocketMock::AddReceive(std::string message) {

    ssize_t return_value = message.length();

    events_.push(Event(EventType::kReceive, std::move(message), return_value, nullptr));

    events_.push(Event(EventType::kReceive, std::move(message), false, nullptr));

}



void SocketMock::AddReceiveTimeout() {

    events_.push(Event(EventType::kReceive, "", true, nullptr));

}



void SocketMock::AddReceiveFailure() {

    events_.push(Event(EventType::kReceive, "", -1, nullptr));

    events_.push(Event(EventType::kReceive, "", false, nullptr));

}



void SocketMock::AddAccept(std::unique_ptr<Socket> sock) {

    events_.push(Event(EventType::kAccept, "", 0, std::move(sock)));

    events_.push(Event(EventType::kAccept, "", false, std::move(sock)));

}



SocketMock::Event::Event(EventType _type, std::string _message, ssize_t _return_value,

SocketMock::Event::Event(EventType _type, std::string _message, ssize_t _status,

                         std::unique_ptr<Socket> _sock)

        : type(_type), message(_message), return_value(_return_value), sock(std::move(_sock)) {}
 
        : type(_type), message(_message), status(_status), sock(std::move(_sock)) {}

fastboot/socket_mock.h

+6 −3
Original line number Diff line number Diff line
@@ -71,7 +71,10 @@ class SocketMock : public Socket { 
    // Adds data to provide for Receive().

    void AddReceive(std::string message);



    // Adds a Receive() failure.

    // Adds a Receive() timeout after which ReceiveTimedOut() will return true.

    void AddReceiveTimeout();



    // Adds a Receive() failure after which ReceiveTimedOut() will return false.

    void AddReceiveFailure();



    // Adds a Socket to return from Accept().
@@ -81,12 +84,12 @@ class SocketMock : public Socket { 
    enum class EventType { kSend, kReceive, kAccept };



    struct Event {

        Event(EventType _type, std::string _message, ssize_t _return_value,

        Event(EventType _type, std::string _message, ssize_t _status,

              std::unique_ptr<Socket> _sock);



        EventType type;

        std::string message;

        ssize_t return_value;

        bool status;  // Return value for Send() or timeout status for Receive().

        std::unique_ptr<Socket> sock;

    };

fastboot/socket_test.cpp

+51 −1
Original line number Diff line number Diff line
@@ -28,7 +28,8 @@ 
#include <gtest/gtest-spi.h>

#include <gtest/gtest.h>



enum { kTestTimeoutMs = 3000 };

static constexpr int kShortTimeoutMs = 10;

static constexpr int kTestTimeoutMs = 3000;



// Creates connected sockets |server| and |client|. Returns true on success.

bool MakeConnectedSockets(Socket::Protocol protocol, std::unique_ptr<Socket>* server,
@@ -87,6 +88,50 @@ TEST(SocketTest, TestSendAndReceive) { 
    }

}



TEST(SocketTest, TestReceiveTimeout) {

    std::unique_ptr<Socket> server, client;

    char buffer[16];



    for (Socket::Protocol protocol : {Socket::Protocol::kUdp, Socket::Protocol::kTcp}) {

        ASSERT_TRUE(MakeConnectedSockets(protocol, &server, &client));



        EXPECT_EQ(-1, server->Receive(buffer, sizeof(buffer), kShortTimeoutMs));

        EXPECT_TRUE(server->ReceiveTimedOut());



        EXPECT_EQ(-1, client->Receive(buffer, sizeof(buffer), kShortTimeoutMs));

        EXPECT_TRUE(client->ReceiveTimedOut());

    }



    // UDP will wait for timeout if the other side closes.

    ASSERT_TRUE(MakeConnectedSockets(Socket::Protocol::kUdp, &server, &client));

    EXPECT_EQ(0, server->Close());

    EXPECT_EQ(-1, client->Receive(buffer, sizeof(buffer), kShortTimeoutMs));

    EXPECT_TRUE(client->ReceiveTimedOut());

}



TEST(SocketTest, TestReceiveFailure) {

    std::unique_ptr<Socket> server, client;

    char buffer[16];



    for (Socket::Protocol protocol : {Socket::Protocol::kUdp, Socket::Protocol::kTcp}) {

        ASSERT_TRUE(MakeConnectedSockets(protocol, &server, &client));



        EXPECT_EQ(0, server->Close());

        EXPECT_EQ(-1, server->Receive(buffer, sizeof(buffer), kTestTimeoutMs));

        EXPECT_FALSE(server->ReceiveTimedOut());



        EXPECT_EQ(0, client->Close());

        EXPECT_EQ(-1, client->Receive(buffer, sizeof(buffer), kTestTimeoutMs));

        EXPECT_FALSE(client->ReceiveTimedOut());

    }



    // TCP knows right away when the other side closes and returns 0 to indicate EOF.

    ASSERT_TRUE(MakeConnectedSockets(Socket::Protocol::kTcp, &server, &client));

    EXPECT_EQ(0, server->Close());

    EXPECT_EQ(0, client->Receive(buffer, sizeof(buffer), kTestTimeoutMs));

    EXPECT_FALSE(client->ReceiveTimedOut());

}



// Tests sending and receiving large packets.

TEST(SocketTest, TestLargePackets) {

    std::string message(1024, '\0');
@@ -290,6 +335,11 @@ TEST(SocketMockTest, TestReceiveFailure) { 


    mock->AddReceiveFailure();

    EXPECT_FALSE(ReceiveString(mock, "foo"));

    EXPECT_FALSE(mock->ReceiveTimedOut());



    mock->AddReceiveTimeout();

    EXPECT_FALSE(ReceiveString(mock, "foo"));

    EXPECT_TRUE(mock->ReceiveTimedOut());



    mock->AddReceive("foo");

    mock->AddReceiveFailure();