am ffa866f5: Merge "adb: detect sockets in CLOSE_WAIT state to prevent socket leak on linux."

* commit 'ffa866f5caefb13412f1814d0de06cda385feaa4':
  adb: detect sockets in CLOSE_WAIT state to prevent socket leak on linux.

adb/adb.cpp

@@ -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

@@ -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

@@ -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,41 +254,56 @@ TEST_F(LocalSocketTest, write_error_when_having_packets) {
     ASSERT_EQ(0, pthread_join(thread, nullptr));
 }
 
-struct CloseNoEventsArg {
-    int socket_fd;
+#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) {
-    asocket* s = create_local_socket(arg->socket_fd);
-    ASSERT_TRUE(s != nullptr);
+static void CloseRdHupSocketThreadFunc(CloseRdHupSocketArg* arg) {
+  asocket* s = create_local_socket(arg->socket_fd);
+  ASSERT_TRUE(s != nullptr);
 
-    InstallDummySocket();
-    fdevent_loop();
+  InstallDummySocket();
+  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));
+// 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));
 
-    CloseNoEventsArg arg;
-    arg.socket_fd = socket_fd[1];
-    pthread_t thread;
-    ASSERT_EQ(0, pthread_create(&thread, nullptr,
-                                reinterpret_cast<void* (*)(void*)>(CloseNoEventsThreadFunc),
-                                &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);
-
-    ASSERT_EQ(0, pthread_kill(thread, SIGUSR1));
-    ASSERT_EQ(0, pthread_join(thread, 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*)>(CloseRdHupSocketThreadFunc),
+                              &arg));
+  // Wait until the fdevent_loop() starts.
+  sleep(1);
+  ASSERT_EQ(2u, fdevent_installed_count());
+  // 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__)