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

Commit aa77e22d authored by Yabin Cui's avatar Yabin Cui
Browse files

adb: detect sockets in CLOSE_WAIT state to prevent socket leak on linux. 

It is possible that the adb server on host has many sockets in
CLOSE_WAIT state. To prevent socket leak, always enable POLLRDHUP
in fdevent.cpp to detect sockets in CLOSE_WAIT state.

Update LocalSocketTest unit tests:
Change half_close_with_packet to read_from_closing_socket, as reading
from a SHUT_WR socket is not needed in adb.
Change close_with_no_events_installed to close_socket_in_CLOSE_WAIT_state,
as the latter is more close to the real situation in use.

Bug: 23314034

Change-Id: Ice4f4036624e5584eab6ba5848e7f169c92f037f
parent d490db98

adb/adb.cpp

+5 −0
Original line number Diff line number Diff line
@@ -395,6 +395,11 @@ void handle_packet(apacket *p, atransport *t) 
                    D("Invalid A_OKAY(%d,%d), expected A_OKAY(%d,%d) on transport %s",

                      p->msg.arg0, p->msg.arg1, s->peer->id, p->msg.arg1, t->serial);

                }

            } else {

                // When receiving A_OKAY from device for A_OPEN request, the host server may

                // have closed the local socket because of client disconnection. Then we need

                // to send A_CLSE back to device to close the service on device.

                send_close(p->msg.arg1, p->msg.arg0, t);

            }

        }

        break;

adb/fdevent.cpp

+11 −0
Original line number Diff line number Diff line
@@ -58,6 +58,12 @@ struct PollNode { 
  PollNode(fdevent* fde) : fde(fde) {

      memset(&pollfd, 0, sizeof(pollfd));

      pollfd.fd = fde->fd;



#if defined(__linux__)

      // Always enable POLLRDHUP, so the host server can take action when some clients disconnect.

      // Then we can avoid leaving many sockets in CLOSE_WAIT state. See http://b/23314034.

      pollfd.events = POLLRDHUP;

#endif

  }

};
@@ -234,6 +240,11 @@ static void fdevent_process() { 
            // be detected at that point.

            events |= FDE_READ | FDE_ERROR;

        }

#if defined(__linux__)

        if (pollfd.revents & POLLRDHUP) {

            events |= FDE_READ | FDE_ERROR;

        }

#endif

        if (events != 0) {

            auto it = g_poll_node_map.find(pollfd.fd);

            CHECK(it != g_poll_node_map.end());

adb/socket_test.cpp

+48 −39
Original line number Diff line number Diff line
@@ -168,7 +168,7 @@ static void CloseWithPacketThreadFunc(CloseWithPacketArg* arg) { 
// The socket is closing but having some packets, so it is not closed. Then

// some write error happens in the socket's file handler, e.g., the file

// handler is closed.

TEST_F(LocalSocketTest, close_with_packet) {

TEST_F(LocalSocketTest, close_socket_with_packet) {

    int socket_fd[2];

    ASSERT_EQ(0, adb_socketpair(socket_fd));

    int cause_close_fd[2];
@@ -193,13 +193,8 @@ TEST_F(LocalSocketTest, close_with_packet) { 
    ASSERT_EQ(0, pthread_join(thread, nullptr));

}



#undef shutdown



// This test checks if we can read packets from a closing local socket.

// The socket's file handler may be non readable if the other side has

// called shutdown(SHUT_WR). But we should always write packets

// successfully to the other side.

TEST_F(LocalSocketTest, half_close_with_packet) {

TEST_F(LocalSocketTest, read_from_closing_socket) {

    int socket_fd[2];

    ASSERT_EQ(0, adb_socketpair(socket_fd));

    int cause_close_fd[2];
@@ -217,7 +212,6 @@ TEST_F(LocalSocketTest, half_close_with_packet) { 
    ASSERT_EQ(0, adb_close(cause_close_fd[0]));

    sleep(1);

    ASSERT_EQ(2u, fdevent_installed_count());

    ASSERT_EQ(0, shutdown(socket_fd[0], SHUT_WR));



    // Verify if we can read successfully.

    std::vector<char> buf(arg.bytes_written);
@@ -260,11 +254,21 @@ TEST_F(LocalSocketTest, write_error_when_having_packets) { 
    ASSERT_EQ(0, pthread_join(thread, nullptr));

}



struct CloseNoEventsArg {

#if defined(__linux__)



static void ClientThreadFunc() {

    std::string error;

    int fd = network_loopback_client(5038, SOCK_STREAM, &error);

    ASSERT_GE(fd, 0) << error;

    sleep(2);

    ASSERT_EQ(0, adb_close(fd));

}



struct CloseRdHupSocketArg {

  int socket_fd;

};



static void CloseNoEventsThreadFunc(CloseNoEventsArg* arg) {

static void CloseRdHupSocketThreadFunc(CloseRdHupSocketArg* arg) {

  asocket* s = create_local_socket(arg->socket_fd);

  ASSERT_TRUE(s != nullptr);
@@ -272,29 +276,34 @@ static void CloseNoEventsThreadFunc(CloseNoEventsArg* arg) { 
  fdevent_loop();

}



// This test checks when a local socket doesn't enable FDE_READ/FDE_WRITE/FDE_ERROR, it

// can still be closed when some error happens on its file handler.

// This test successes on linux but fails on mac because of different implementation of

// poll(). I think the function tested here is useful to make adb server more stable on

// linux.

TEST_F(LocalSocketTest, close_with_no_events_installed) {

    int socket_fd[2];

    ASSERT_EQ(0, adb_socketpair(socket_fd));



    CloseNoEventsArg arg;

    arg.socket_fd = socket_fd[1];

// This test checks if we can close sockets in CLOSE_WAIT state.

TEST_F(LocalSocketTest, close_socket_in_CLOSE_WAIT_state) {

  std::string error;

  int listen_fd = network_inaddr_any_server(5038, SOCK_STREAM, &error);

  ASSERT_GE(listen_fd, 0);

  pthread_t client_thread;

  ASSERT_EQ(0, pthread_create(&client_thread, nullptr,

                              reinterpret_cast<void* (*)(void*)>(ClientThreadFunc), nullptr));



  struct sockaddr addr;

  socklen_t alen;

  alen = sizeof(addr);

  int accept_fd = adb_socket_accept(listen_fd, &addr, &alen);

  ASSERT_GE(accept_fd, 0);

  CloseRdHupSocketArg arg;

  arg.socket_fd = accept_fd;

  pthread_t thread;

  ASSERT_EQ(0, pthread_create(&thread, nullptr,

                                reinterpret_cast<void* (*)(void*)>(CloseNoEventsThreadFunc),

                              reinterpret_cast<void* (*)(void*)>(CloseRdHupSocketThreadFunc),

                              &arg));

  // Wait until the fdevent_loop() starts.

  sleep(1);

  ASSERT_EQ(2u, fdevent_installed_count());

    ASSERT_EQ(0, adb_close(socket_fd[0]));



    // Wait until the socket is closed.

    sleep(1);



  // Wait until the client closes its socket.

  ASSERT_EQ(0, pthread_join(client_thread, nullptr));

  sleep(2);

  ASSERT_EQ(0, pthread_kill(thread, SIGUSR1));

  ASSERT_EQ(0, pthread_join(thread, nullptr));

}



#endif  // defined(__linux__)