Merge changes I394f5782,I9475f5d0,I59707186

* changes:
  adb: move all cleanup to a function with defined ordering.
  adb: make `adb kill-server` wait for the server to die.
  adb: libusb: replace sleep with timed CV wait.

adb/adb.cpp

@@ -1054,15 +1054,12 @@ int handle_host_request(const char* service, TransportType type,
     if (strcmp(service, "kill") == 0) {
         fprintf(stderr, "adb server killed by remote request\n");
         fflush(stdout);
+
+        // Send a reply even though we don't read it anymore, so that old versions
+        // of adb that do read it don't spew error messages.
         SendOkay(reply_fd);
 
-        // On Windows, if the process exits with open sockets that
-        // shutdown(SD_SEND) has not been called on, TCP RST segments will be
-        // sent to the peers which will cause their next recv() to error-out
-        // with WSAECONNRESET. In the case of this code, that means the client
-        // may not read the OKAY sent above.
-        adb_shutdown(reply_fd);
-
+        // Rely on process exit to close the socket for us.
         android::base::quick_exit(0);
     }
 

adb/adb_client.cpp

@@ -123,7 +123,7 @@ bool adb_status(int fd, std::string* error) {
     return false;
 }
 
-int _adb_connect(const std::string& service, std::string* error) {
+static int _adb_connect(const std::string& service, std::string* error) {
     D("_adb_connect: %s", service.c_str());
     if (service.empty() || service.size() > MAX_PAYLOAD_V1) {
         *error = android::base::StringPrintf("bad service name length (%zd)",
@@ -158,6 +158,25 @@ int _adb_connect(const std::string& service, std::string* error) {
     return fd;
 }
 
+bool adb_kill_server() {
+    D("adb_kill_server");
+    std::string reason;
+    int fd = socket_spec_connect(__adb_server_socket_spec, &reason);
+    if (fd < 0) {
+        fprintf(stderr, "cannot connect to daemon at %s: %s\n", __adb_server_socket_spec,
+                reason.c_str());
+        return true;
+    }
+
+    if (!SendProtocolString(fd, "host:kill")) {
+        fprintf(stderr, "error: write failure during connection: %s\n", strerror(errno));
+        return false;
+    }
+
+    ReadOrderlyShutdown(fd);
+    return true;
+}
+
 int adb_connect(const std::string& service, std::string* error) {
     // first query the adb server's version
     int fd = _adb_connect("host:version", error);
@@ -214,18 +233,7 @@ int adb_connect(const std::string& service, std::string* error) {
         if (version != ADB_SERVER_VERSION) {
             fprintf(stderr, "adb server version (%d) doesn't match this client (%d); killing...\n",
                     version, ADB_SERVER_VERSION);
-            fd = _adb_connect("host:kill", error);
-            if (fd >= 0) {
-                ReadOrderlyShutdown(fd);
-                adb_close(fd);
-            } else {
-                // If we couldn't connect to the server or had some other error,
-                // report it, but still try to start the server.
-                fprintf(stderr, "error: %s\n", error->c_str());
-            }
-
-            /* XXX can we better detect its death? */
-            std::this_thread::sleep_for(2s);
+            adb_kill_server();
             goto start_server;
         }
     }

adb/adb_client.h

@@ -26,7 +26,9 @@
 // Connect to adb, connect to the named service, and return a valid fd for
 // interacting with that service upon success or a negative number on failure.
 int adb_connect(const std::string& service, std::string* _Nonnull error);
-int _adb_connect(const std::string& service, std::string* _Nonnull error);
+
+// Kill the currently running adb server, if it exists.
+bool adb_kill_server();
 
 // Connect to adb, connect to the named service, returns true if the connection
 // succeeded AND the service returned OKAY. Outputs any returned error otherwise.

adb/adb_listeners.cpp

@@ -19,8 +19,12 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+#include <algorithm>
+#include <list>
+
 #include <android-base/stringprintf.h>
 #include <android-base/strings.h>
+#include <android-base/thread_annotations.h>
 #include <cutils/sockets.h>
 
 #include "socket_spec.h"
@@ -64,8 +68,9 @@ alistener::~alistener() {
 
 // listener_list retains ownership of all created alistener objects. Removing an alistener from
 // this list will cause it to be deleted.
+static auto& listener_list_mutex = *new std::mutex();
 typedef std::list<std::unique_ptr<alistener>> ListenerList;
-static ListenerList& listener_list = *new ListenerList();
+static ListenerList& listener_list GUARDED_BY(listener_list_mutex) = *new ListenerList();
 
 static void ss_listener_event_func(int _fd, unsigned ev, void *_l) {
     if (ev & FDE_READ) {
@@ -108,7 +113,8 @@ static void listener_event_func(int _fd, unsigned ev, void* _l)
 }
 
 // Called as a transport disconnect function. |arg| is the raw alistener*.
-static void listener_disconnect(void* arg, atransport*) {
+static void listener_disconnect(void* arg, atransport*) EXCLUDES(listener_list_mutex) {
+    std::lock_guard<std::mutex> lock(listener_list_mutex);
     for (auto iter = listener_list.begin(); iter != listener_list.end(); ++iter) {
         if (iter->get() == arg) {
             (*iter)->transport = nullptr;
@@ -119,7 +125,8 @@ static void listener_disconnect(void* arg, atransport*) {
 }
 
 // Write the list of current listeners (network redirections) into a string.
-std::string format_listeners() {
+std::string format_listeners() EXCLUDES(listener_list_mutex) {
+    std::lock_guard<std::mutex> lock(listener_list_mutex);
     std::string result;
     for (auto& l : listener_list) {
         // Ignore special listeners like those for *smartsocket*
@@ -135,7 +142,9 @@ std::string format_listeners() {
     return result;
 }
 
-InstallStatus remove_listener(const char* local_name, atransport* transport) {
+InstallStatus remove_listener(const char* local_name, atransport* transport)
+    EXCLUDES(listener_list_mutex) {
+    std::lock_guard<std::mutex> lock(listener_list_mutex);
     for (auto iter = listener_list.begin(); iter != listener_list.end(); ++iter) {
         if (local_name == (*iter)->local_name) {
             listener_list.erase(iter);
@@ -145,7 +154,8 @@ InstallStatus remove_listener(const char* local_name, atransport* transport) {
     return INSTALL_STATUS_LISTENER_NOT_FOUND;
 }
 
-void remove_all_listeners() {
+void remove_all_listeners() EXCLUDES(listener_list_mutex) {
+    std::lock_guard<std::mutex> lock(listener_list_mutex);
     auto iter = listener_list.begin();
     while (iter != listener_list.end()) {
         // Never remove smart sockets.
@@ -157,9 +167,18 @@ void remove_all_listeners() {
     }
 }
 
+void close_smartsockets() EXCLUDES(listener_list_mutex) {
+    std::lock_guard<std::mutex> lock(listener_list_mutex);
+    auto pred = [](const std::unique_ptr<alistener>& listener) {
+        return listener->local_name == "*smartsocket*";
+    };
+    listener_list.erase(std::remove_if(listener_list.begin(), listener_list.end(), pred));
+}
+
 InstallStatus install_listener(const std::string& local_name, const char* connect_to,
                                atransport* transport, int no_rebind, int* resolved_tcp_port,
-                               std::string* error) {
+                               std::string* error) EXCLUDES(listener_list_mutex) {
+    std::lock_guard<std::mutex> lock(listener_list_mutex);
     for (auto& l : listener_list) {
         if (local_name == l->local_name) {
             // Can't repurpose a smartsocket.
@@ -212,7 +231,7 @@ InstallStatus install_listener(const std::string& local_name, const char* connec
 
     if (transport) {
         listener->disconnect.opaque = listener.get();
-        listener->disconnect.func   = listener_disconnect;
+        listener->disconnect.func = listener_disconnect;
         transport->AddDisconnect(&listener->disconnect);
     }
 

adb/adb_listeners.h

@@ -41,4 +41,6 @@ std::string format_listeners();
 InstallStatus remove_listener(const char* local_name, atransport* transport);
 void remove_all_listeners(void);
 
+void close_smartsockets();
+
 #endif /* __ADB_LISTENERS_H */

adb/client/main.cpp

@@ -92,6 +92,16 @@ static BOOL WINAPI ctrlc_handler(DWORD type) {
 }
 #endif
 
+void adb_server_cleanup() {
+    // Upon exit, we want to clean up in the following order:
+    //   1. close_smartsockets, so that we don't get any new clients
+    //   2. kick_all_transports, to avoid writing only part of a packet to a transport.
+    //   3. usb_cleanup, to tear down the USB stack.
+    close_smartsockets();
+    kick_all_transports();
+    usb_cleanup();
+}
+
 int adb_server_main(int is_daemon, const std::string& socket_spec, int ack_reply_fd) {
 #if defined(_WIN32)
     // adb start-server starts us up with stdout and stderr hooked up to
@@ -111,12 +121,13 @@ int adb_server_main(int is_daemon, const std::string& socket_spec, int ack_reply
     SetConsoleCtrlHandler(ctrlc_handler, TRUE);
 #else
     signal(SIGINT, [](int) {
-        android::base::quick_exit(0);
+        fdevent_run_on_main_thread([]() { android::base::quick_exit(0); });
     });
 #endif
 
-    init_transport_registration();
+    android::base::at_quick_exit(adb_server_cleanup);
 
+    init_transport_registration();
     init_mdns_transport_discovery();
 
     usb_init();

adb/client/usb_dispatch.cpp

@@ -27,6 +27,12 @@ void usb_init() {
     }
 }
 
+void usb_cleanup() {
+    if (should_use_libusb()) {
+        libusb::usb_cleanup();
+    }
+}
+
 int usb_write(usb_handle* h, const void* data, int len) {
     return should_use_libusb()
                ? libusb::usb_write(reinterpret_cast<libusb::usb_handle*>(h), data, len)

adb/client/usb_libusb.cpp

@@ -152,7 +152,9 @@ static auto& usb_handles = *new std::unordered_map<std::string, std::unique_ptr<
 static auto& usb_handles_mutex = *new std::mutex();
 
 static std::thread* device_poll_thread = nullptr;
-static std::atomic<bool> terminate_device_poll_thread(false);
+static bool terminate_device_poll_thread = false;
+static auto& device_poll_mutex = *new std::mutex();
+static auto& device_poll_cv = *new std::condition_variable();
 
 static std::string get_device_address(libusb_device* device) {
     return StringPrintf("usb:%d:%d", libusb_get_bus_number(device),
@@ -376,7 +378,7 @@ static void process_device(libusb_device* device) {
 static void poll_for_devices() {
     libusb_device** list;
     adb_thread_setname("device poll");
-    while (!terminate_device_poll_thread) {
+    while (true) {
         const ssize_t device_count = libusb_get_device_list(nullptr, &list);
 
         LOG(VERBOSE) << "found " << device_count << " attached devices";
@@ -389,7 +391,10 @@ static void poll_for_devices() {
 
         adb_notify_device_scan_complete();
 
-        std::this_thread::sleep_for(500ms);
+        std::unique_lock<std::mutex> lock(device_poll_mutex);
+        if (device_poll_cv.wait_for(lock, 500ms, []() { return terminate_device_poll_thread; })) {
+            return;
+        }
     }
 }
 
@@ -410,11 +415,21 @@ void usb_init() {
 
     // Spawn a thread to do device enumeration.
     // TODO: Use libusb_hotplug_* instead?
+    std::unique_lock<std::mutex> lock(device_poll_mutex);
     device_poll_thread = new std::thread(poll_for_devices);
-    android::base::at_quick_exit([]() {
+}
+
+void usb_cleanup() {
+    {
+        std::unique_lock<std::mutex> lock(device_poll_mutex);
         terminate_device_poll_thread = true;
-        device_poll_thread->join();
-    });
+
+        if (!device_poll_thread) {
+            return;
+        }
+    }
+    device_poll_cv.notify_all();
+    device_poll_thread->join();
 }
 
 // Dispatch a libusb transfer, unlock |device_lock|, and then wait for the result.

adb/client/usb_windows.cpp

@@ -265,6 +265,8 @@ void usb_init() {
   std::thread(_power_notification_thread).detach();
 }
 
+void usb_cleanup() {}
+
 usb_handle* do_usb_open(const wchar_t* interface_name) {
   unsigned long name_len = 0;
 

adb/commandline.cpp

@@ -1546,25 +1546,7 @@ int adb_commandline(int argc, const char** argv) {
         return 0;
     }
     else if (!strcmp(argv[0], "kill-server")) {
-        std::string error;
-        int fd = _adb_connect("host:kill", &error);
-        if (fd == -2) {
-            // Failed to make network connection to server. Don't output the
-            // network error since that is expected.
-            fprintf(stderr,"* server not running *\n");
-            // Successful exit code because the server is already "killed".
-            return 0;
-        } else if (fd == -1) {
-            // Some other error.
-            fprintf(stderr, "error: %s\n", error.c_str());
-            return 1;
-        } else {
-            // Successfully connected, kill command sent, okay status came back.
-            // Server should exit() in a moment, if not already.
-            ReadOrderlyShutdown(fd);
-            adb_close(fd);
-            return 0;
-        }
+        return adb_kill_server() ? 0 : 1;
     }
     else if (!strcmp(argv[0], "sideload")) {
         if (argc != 2) return syntax_error("sideload requires an argument");

adb/transport.cpp

@@ -568,15 +568,14 @@ void init_transport_registration(void) {
                     transport_registration_func, 0);
 
     fdevent_set(&transport_registration_fde, FDE_READ);
-#if ADB_HOST
-    android::base::at_quick_exit([]() {
-        // To avoid only writing part of a packet to a transport after exit, kick all transports.
-        std::lock_guard<std::mutex> lock(transport_lock);
-        for (auto t : transport_list) {
-            t->Kick();
-        }
-    });
-#endif
+}
+
+void kick_all_transports() {
+    // To avoid only writing part of a packet to a transport after exit, kick all transports.
+    std::lock_guard<std::mutex> lock(transport_lock);
+    for (auto t : transport_list) {
+        t->Kick();
+    }
 }
 
 /* the fdevent select pump is single threaded */

adb/transport.h

@@ -207,6 +207,7 @@ void init_mdns_transport_discovery(void);
 std::string list_transports(bool long_listing);
 atransport* find_transport(const char* serial);
 void kick_all_tcp_devices();
+void kick_all_transports();
 
 void register_usb_transport(usb_handle* h, const char* serial,
                             const char* devpath, unsigned writeable);

adb/usb.h

@@ -22,6 +22,7 @@
 
 #define ADB_USB_INTERFACE(handle_ref_type)                       \
     void usb_init();                                             \
+    void usb_cleanup();                                          \
     int usb_write(handle_ref_type h, const void* data, int len); \
     int usb_read(handle_ref_type h, void* data, int len);        \
     int usb_close(handle_ref_type h);                            \