From 74395b3facab0995af157791e04a83623172aff6 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 9 May 2017 13:43:35 -0700 Subject: adb: move all cleanup to a function with defined ordering. We want to explicitly define the order in which we teardown adb, so move all of the at_quick_exits sprinkled throughout into one function containing all of the cleanup functions. Bug: http://b/37104408 Test: adb kill-server; adb start-server Change-Id: I394f5782eb147e394d4b87df1ba364c061de4b90 --- adb_listeners.cpp | 33 ++++++++++++++++++++++++++------- adb_listeners.h | 2 ++ client/main.cpp | 15 +++++++++++++-- client/usb_dispatch.cpp | 6 ++++++ client/usb_libusb.cpp | 20 +++++++++++++------- client/usb_windows.cpp | 2 ++ transport.cpp | 17 ++++++++--------- transport.h | 1 + usb.h | 1 + 9 files changed, 72 insertions(+), 25 deletions(-) diff --git a/adb_listeners.cpp b/adb_listeners.cpp index 18b1492..30cb29b 100644 --- a/adb_listeners.cpp +++ b/adb_listeners.cpp @@ -19,8 +19,12 @@ #include #include +#include +#include + #include #include +#include #include #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> 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 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 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 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 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 lock(listener_list_mutex); + auto pred = [](const std::unique_ptr& 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 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); } diff --git a/adb_listeners.h b/adb_listeners.h index 8eba00a..70a2ee1 100644 --- a/adb_listeners.h +++ b/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 */ diff --git a/client/main.cpp b/client/main.cpp index fe5099c..bd9ad01 100644 --- a/client/main.cpp +++ b/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(); diff --git a/client/usb_dispatch.cpp b/client/usb_dispatch.cpp index 710a3ce..c4eed78 100644 --- a/client/usb_dispatch.cpp +++ b/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(h), data, len) diff --git a/client/usb_libusb.cpp b/client/usb_libusb.cpp index 5a65865..18a8ff2 100644 --- a/client/usb_libusb.cpp +++ b/client/usb_libusb.cpp @@ -415,15 +415,21 @@ void usb_init() { // Spawn a thread to do device enumeration. // TODO: Use libusb_hotplug_* instead? + std::unique_lock lock(device_poll_mutex); device_poll_thread = new std::thread(poll_for_devices); - android::base::at_quick_exit([]() { - { - std::unique_lock lock(device_poll_mutex); - terminate_device_poll_thread = true; +} + +void usb_cleanup() { + { + std::unique_lock lock(device_poll_mutex); + terminate_device_poll_thread = true; + + if (!device_poll_thread) { + return; } - device_poll_cv.notify_all(); - device_poll_thread->join(); - }); + } + device_poll_cv.notify_all(); + device_poll_thread->join(); } // Dispatch a libusb transfer, unlock |device_lock|, and then wait for the result. diff --git a/client/usb_windows.cpp b/client/usb_windows.cpp index ec55b0e..1620e6e 100644 --- a/client/usb_windows.cpp +++ b/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; diff --git a/transport.cpp b/transport.cpp index 20de642..13cfaa1 100644 --- a/transport.cpp +++ b/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 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 lock(transport_lock); + for (auto t : transport_list) { + t->Kick(); + } } /* the fdevent select pump is single threaded */ diff --git a/transport.h b/transport.h index e129355..006aaf4 100644 --- a/transport.h +++ b/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); diff --git a/usb.h b/usb.h index e867ec8..f428ede 100644 --- a/usb.h +++ b/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); \ -- cgit v1.2.3