From 2f9eb3cc8af6bf4aa9a6da01230725e3b643be29 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 21 Sep 2016 12:37:10 -0700 Subject: adb: kill adb_mutex_t, adb_cond_t. Now that we have support for std::mutex and std::condition_variable on Windows, remove our mutex compatibility layer in favor of the C++ one. Bug: http://b/31653591 Test: mma && $ANDROID_HOST_OUT/nativetest64/adb_test/adb_test && \ python test_adb.py && python test_device.py (also on Windows) Change-Id: I5b7ed9c45cc2a32edcf4e77b56dc28e441f15f34 --- adb_utils.cpp | 17 +++++------ client/main.cpp | 1 - mutex_list.h | 17 ----------- sysdeps.h | 46 ----------------------------- sysdeps_test.cpp | 11 ------- sysdeps_win32.cpp | 82 ++++++++++++++++++++++------------------------------ transport.cpp | 63 +++++++++++++++++++--------------------- transport_local.cpp | 46 ++++++++++++----------------- transport_test.cpp | 21 -------------- usb_linux_client.cpp | 67 ++++++++++++++++++------------------------ usb_windows.cpp | 48 ++++++++++++++---------------- 11 files changed, 142 insertions(+), 277 deletions(-) delete mode 100644 mutex_list.h diff --git a/adb_utils.cpp b/adb_utils.cpp index db39ef4..5a3b401 100644 --- a/adb_utils.cpp +++ b/adb_utils.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include @@ -47,8 +48,6 @@ #include #endif -ADB_MUTEX_DEFINE(basename_lock); -ADB_MUTEX_DEFINE(dirname_lock); #if defined(_WIN32) constexpr char kNullFileName[] = "NUL"; @@ -102,13 +101,15 @@ std::string escape_arg(const std::string& s) { } std::string adb_basename(const std::string& path) { + static std::mutex& basename_lock = *new std::mutex(); + // Copy path because basename may modify the string passed in. std::string result(path); // Use lock because basename() may write to a process global and return a // pointer to that. Note that this locking strategy only works if all other - // callers to dirname in the process also grab this same lock. - adb_mutex_lock(&basename_lock); + // callers to basename in the process also grab this same lock. + std::lock_guard lock(basename_lock); // Note that if std::string uses copy-on-write strings, &str[0] will cause // the copy to be made, so there is no chance of us accidentally writing to @@ -119,19 +120,19 @@ std::string adb_basename(const std::string& path) { // before leaving the lock. result.assign(name); - adb_mutex_unlock(&basename_lock); - return result; } std::string adb_dirname(const std::string& path) { + static std::mutex& dirname_lock = *new std::mutex(); + // Copy path because dirname may modify the string passed in. std::string result(path); // Use lock because dirname() may write to a process global and return a // pointer to that. Note that this locking strategy only works if all other // callers to dirname in the process also grab this same lock. - adb_mutex_lock(&dirname_lock); + std::lock_guard lock(dirname_lock); // Note that if std::string uses copy-on-write strings, &str[0] will cause // the copy to be made, so there is no chance of us accidentally writing to @@ -142,8 +143,6 @@ std::string adb_dirname(const std::string& path) { // before leaving the lock. result.assign(parent); - adb_mutex_unlock(&dirname_lock); - return result; } diff --git a/client/main.cpp b/client/main.cpp index 571c227..279bb70 100644 --- a/client/main.cpp +++ b/client/main.cpp @@ -170,7 +170,6 @@ int adb_server_main(int is_daemon, const std::string& socket_spec, int ack_reply } int main(int argc, char** argv) { - adb_sysdeps_init(); adb_trace_init(argv); return adb_commandline(argc - 1, const_cast(argv + 1)); } diff --git a/mutex_list.h b/mutex_list.h deleted file mode 100644 index 4a188ee..0000000 --- a/mutex_list.h +++ /dev/null @@ -1,17 +0,0 @@ -/* the list of mutexes used by adb */ -/* #ifndef __MUTEX_LIST_H - * Do not use an include-guard. This file is included once to declare the locks - * and once in win32 to actually do the runtime initialization. - */ -#ifndef ADB_MUTEX -#error ADB_MUTEX not defined when including this file -#endif -ADB_MUTEX(basename_lock) -ADB_MUTEX(dirname_lock) -ADB_MUTEX(transport_lock) -#if ADB_HOST -ADB_MUTEX(local_transports_lock) -#endif -ADB_MUTEX(usb_lock) - -#undef ADB_MUTEX diff --git a/sysdeps.h b/sysdeps.h index 8d99722..3ed589c 100644 --- a/sysdeps.h +++ b/sysdeps.h @@ -97,27 +97,6 @@ static __inline__ bool adb_is_separator(char c) { return c == '\\' || c == '/'; } -typedef CRITICAL_SECTION adb_mutex_t; - -#define ADB_MUTEX_DEFINE(x) adb_mutex_t x - -/* declare all mutexes */ -/* For win32, adb_sysdeps_init() will do the mutex runtime initialization. */ -#define ADB_MUTEX(x) extern adb_mutex_t x; -#include "mutex_list.h" - -extern void adb_sysdeps_init(void); - -static __inline__ void adb_mutex_lock( adb_mutex_t* lock ) -{ - EnterCriticalSection( lock ); -} - -static __inline__ void adb_mutex_unlock( adb_mutex_t* lock ) -{ - LeaveCriticalSection( lock ); -} - typedef void (*adb_thread_func_t)(void* arg); typedef HANDLE adb_thread_t; @@ -476,27 +455,6 @@ static __inline__ bool adb_is_separator(char c) { return c == '/'; } -typedef pthread_mutex_t adb_mutex_t; - -#define ADB_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER -#define adb_mutex_init pthread_mutex_init -#define adb_mutex_lock pthread_mutex_lock -#define adb_mutex_unlock pthread_mutex_unlock -#define adb_mutex_destroy pthread_mutex_destroy - -#define ADB_MUTEX_DEFINE(m) adb_mutex_t m = PTHREAD_MUTEX_INITIALIZER - -#define adb_cond_t pthread_cond_t -#define adb_cond_init pthread_cond_init -#define adb_cond_wait pthread_cond_wait -#define adb_cond_broadcast pthread_cond_broadcast -#define adb_cond_signal pthread_cond_signal -#define adb_cond_destroy pthread_cond_destroy - -/* declare all mutexes */ -#define ADB_MUTEX(x) extern adb_mutex_t x; -#include "mutex_list.h" - static __inline__ void close_on_exec(int fd) { fcntl( fd, F_SETFD, FD_CLOEXEC ); @@ -818,10 +776,6 @@ static __inline__ int adb_mkdir(const std::string& path, int mode) #undef mkdir #define mkdir ___xxx_mkdir -static __inline__ void adb_sysdeps_init(void) -{ -} - static __inline__ int adb_is_absolute_host_path(const char* path) { return path[0] == '/'; } diff --git a/sysdeps_test.cpp b/sysdeps_test.cpp index f871675..9f77942 100644 --- a/sysdeps_test.cpp +++ b/sysdeps_test.cpp @@ -269,17 +269,6 @@ TEST(sysdeps_mutex, mutex_smoke) { m.unlock(); } -// Our implementation on Windows aborts on double lock. -#if defined(_WIN32) -TEST(sysdeps_mutex, mutex_reentrant_lock) { - std::mutex &m = *new std::mutex(); - - m.lock(); - ASSERT_FALSE(m.try_lock()); - EXPECT_DEATH(m.lock(), "non-recursive mutex locked reentrantly"); -} -#endif - TEST(sysdeps_mutex, recursive_mutex_smoke) { static std::recursive_mutex &m = *new std::recursive_mutex(); diff --git a/sysdeps_win32.cpp b/sysdeps_win32.cpp index 5fda27b..4dd549d 100644 --- a/sysdeps_win32.cpp +++ b/sysdeps_win32.cpp @@ -27,6 +27,7 @@ #include #include +#include #include #include #include @@ -137,7 +138,7 @@ typedef struct FHRec_ #define WIN32_FH_BASE 2048 #define WIN32_MAX_FHS 2048 -static adb_mutex_t _win32_lock; +static std::mutex& _win32_lock = *new std::mutex(); static FHRec _win32_fhs[ WIN32_MAX_FHS ]; static int _win32_fh_next; // where to start search for free FHRec @@ -182,27 +183,24 @@ _fh_alloc( FHClass clazz ) { FH f = NULL; - adb_mutex_lock( &_win32_lock ); + std::lock_guard lock(_win32_lock); for (int i = _win32_fh_next; i < WIN32_MAX_FHS; ++i) { if (_win32_fhs[i].clazz == NULL) { f = &_win32_fhs[i]; _win32_fh_next = i + 1; - goto Exit; + f->clazz = clazz; + f->used = 1; + f->eof = 0; + f->name[0] = '\0'; + clazz->_fh_init(f); + return f; } } - D( "_fh_alloc: no more free file descriptors" ); - errno = EMFILE; // Too many open files -Exit: - if (f) { - f->clazz = clazz; - f->used = 1; - f->eof = 0; - f->name[0] = '\0'; - clazz->_fh_init(f); - } - adb_mutex_unlock( &_win32_lock ); - return f; + + D("_fh_alloc: no more free file descriptors"); + errno = EMFILE; // Too many open files + return nullptr; } @@ -211,7 +209,7 @@ _fh_close( FH f ) { // Use lock so that closing only happens once and so that _fh_alloc can't // allocate a FH that we're in the middle of closing. - adb_mutex_lock(&_win32_lock); + std::lock_guard lock(_win32_lock); int offset = f - _win32_fhs; if (_win32_fh_next > offset) { @@ -225,7 +223,6 @@ _fh_close( FH f ) f->used = 0; f->clazz = NULL; } - adb_mutex_unlock(&_win32_lock); return 0; } @@ -1234,17 +1231,6 @@ bool set_tcp_keepalive(int fd, int interval_sec) { return true; } -static adb_mutex_t g_console_output_buffer_lock; - -void -adb_sysdeps_init( void ) -{ -#define ADB_MUTEX(x) InitializeCriticalSection( & x ); -#include "mutex_list.h" - InitializeCriticalSection( &_win32_lock ); - InitializeCriticalSection( &g_console_output_buffer_lock ); -} - /**************************************************************************/ /**************************************************************************/ /***** *****/ @@ -2437,12 +2423,13 @@ size_t ParseCompleteUTF8(const char* const first, const char* const last, // Bytes that have not yet been output to the console because they are incomplete UTF-8 sequences. // Note that we use only one buffer even though stderr and stdout are logically separate streams. // This matches the behavior of Linux. -// Protected by g_console_output_buffer_lock. -static auto& g_console_output_buffer = *new std::vector(); // Internal helper function to write UTF-8 bytes to a console. Returns -1 on error. static int _console_write_utf8(const char* const buf, const size_t buf_size, FILE* stream, HANDLE console) { + static std::mutex& console_output_buffer_lock = *new std::mutex(); + static auto& console_output_buffer = *new std::vector(); + const int saved_errno = errno; std::vector combined_buffer; @@ -2450,24 +2437,25 @@ static int _console_write_utf8(const char* const buf, const size_t buf_size, FIL const char* utf8; size_t utf8_size; - adb_mutex_lock(&g_console_output_buffer_lock); - if (g_console_output_buffer.empty()) { - // If g_console_output_buffer doesn't have a buffered up incomplete UTF-8 sequence (the - // common case with plain ASCII), parse buf directly. - utf8 = buf; - utf8_size = internal::ParseCompleteUTF8(buf, buf + buf_size, &g_console_output_buffer); - } else { - // If g_console_output_buffer has a buffered up incomplete UTF-8 sequence, move it to - // combined_buffer (and effectively clear g_console_output_buffer) and append buf to - // combined_buffer, then parse it all together. - combined_buffer.swap(g_console_output_buffer); - combined_buffer.insert(combined_buffer.end(), buf, buf + buf_size); - - utf8 = combined_buffer.data(); - utf8_size = internal::ParseCompleteUTF8(utf8, utf8 + combined_buffer.size(), - &g_console_output_buffer); + { + std::lock_guard lock(console_output_buffer_lock); + if (console_output_buffer.empty()) { + // If console_output_buffer doesn't have a buffered up incomplete UTF-8 sequence (the + // common case with plain ASCII), parse buf directly. + utf8 = buf; + utf8_size = internal::ParseCompleteUTF8(buf, buf + buf_size, &console_output_buffer); + } else { + // If console_output_buffer has a buffered up incomplete UTF-8 sequence, move it to + // combined_buffer (and effectively clear console_output_buffer) and append buf to + // combined_buffer, then parse it all together. + combined_buffer.swap(console_output_buffer); + combined_buffer.insert(combined_buffer.end(), buf, buf + buf_size); + + utf8 = combined_buffer.data(); + utf8_size = internal::ParseCompleteUTF8(utf8, utf8 + combined_buffer.size(), + &console_output_buffer); + } } - adb_mutex_unlock(&g_console_output_buffer_lock); std::wstring utf16; diff --git a/transport.cpp b/transport.cpp index 3eaeb06..87712fc 100644 --- a/transport.cpp +++ b/transport.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include @@ -44,7 +45,7 @@ static void transport_unref(atransport *t); static auto& transport_list = *new std::list(); static auto& pending_list = *new std::list(); -ADB_MUTEX_DEFINE( transport_lock ); +static std::mutex& transport_lock = *new std::mutex(); const char* const kFeatureShell2 = "shell_v2"; const char* const kFeatureCmd = "cmd"; @@ -297,13 +298,12 @@ static void write_transport_thread(void* _t) { } void kick_transport(atransport* t) { - adb_mutex_lock(&transport_lock); + std::lock_guard lock(transport_lock); // As kick_transport() can be called from threads without guarantee that t is valid, // check if the transport is in transport_list first. if (std::find(transport_list.begin(), transport_list.end(), t) != transport_list.end()) { t->Kick(); } - adb_mutex_unlock(&transport_lock); } static int transport_registration_send = -1; @@ -333,7 +333,7 @@ device_tracker_remove( device_tracker* tracker ) device_tracker** pnode = &device_tracker_list; device_tracker* node = *pnode; - adb_mutex_lock( &transport_lock ); + std::lock_guard lock(transport_lock); while (node) { if (node == tracker) { *pnode = node->next; @@ -342,7 +342,6 @@ device_tracker_remove( device_tracker* tracker ) pnode = &node->next; node = *pnode; } - adb_mutex_unlock( &transport_lock ); } static void @@ -504,9 +503,10 @@ static void transport_registration_func(int _fd, unsigned ev, void *data) fdevent_remove(&(t->transport_fde)); adb_close(t->fd); - adb_mutex_lock(&transport_lock); - transport_list.remove(t); - adb_mutex_unlock(&transport_lock); + { + std::lock_guard lock(transport_lock); + transport_list.remove(t); + } if (t->product) free(t->product); @@ -555,10 +555,11 @@ static void transport_registration_func(int _fd, unsigned ev, void *data) } } - adb_mutex_lock(&transport_lock); - pending_list.remove(t); - transport_list.push_front(t); - adb_mutex_unlock(&transport_lock); + { + std::lock_guard lock(transport_lock); + pending_list.remove(t); + transport_list.push_front(t); + } update_transports(); } @@ -609,7 +610,8 @@ static void remove_transport(atransport *transport) static void transport_unref(atransport* t) { CHECK(t != nullptr); - adb_mutex_lock(&transport_lock); + + std::lock_guard lock(transport_lock); CHECK_GT(t->ref_count, 0u); t->ref_count--; if (t->ref_count == 0) { @@ -619,7 +621,6 @@ static void transport_unref(atransport* t) { } else { D("transport: %s unref (count=%zu)", t->serial, t->ref_count); } - adb_mutex_unlock(&transport_lock); } static int qual_match(const char *to_test, @@ -665,7 +666,7 @@ atransport* acquire_one_transport(TransportType type, const char* serial, *error_out = "no devices found"; } - adb_mutex_lock(&transport_lock); + std::unique_lock lock(transport_lock); for (const auto& t : transport_list) { if (t->connection_state == kCsNoPerm) { #if ADB_HOST @@ -713,7 +714,7 @@ atransport* acquire_one_transport(TransportType type, const char* serial, } } } - adb_mutex_unlock(&transport_lock); + lock.unlock(); // Don't return unauthorized devices; the caller can't do anything with them. if (result && result->connection_state == kCsUnauthorized) { @@ -914,21 +915,20 @@ static void append_transport(const atransport* t, std::string* result, std::string list_transports(bool long_listing) { std::string result; - adb_mutex_lock(&transport_lock); + + std::lock_guard lock(transport_lock); for (const auto& t : transport_list) { append_transport(t, &result, long_listing); } - adb_mutex_unlock(&transport_lock); return result; } /* hack for osx */ void close_usb_devices() { - adb_mutex_lock(&transport_lock); + std::lock_guard lock(transport_lock); for (const auto& t : transport_list) { t->Kick(); } - adb_mutex_unlock(&transport_lock); } #endif // ADB_HOST @@ -947,10 +947,9 @@ int register_socket_transport(int s, const char *serial, int port, int local) { return -1; } - adb_mutex_lock(&transport_lock); + std::unique_lock lock(transport_lock); for (const auto& transport : pending_list) { if (transport->serial && strcmp(serial, transport->serial) == 0) { - adb_mutex_unlock(&transport_lock); VLOG(TRANSPORT) << "socket transport " << transport->serial << " is already in pending_list and fails to register"; delete t; @@ -960,7 +959,6 @@ int register_socket_transport(int s, const char *serial, int port, int local) { for (const auto& transport : transport_list) { if (transport->serial && strcmp(serial, transport->serial) == 0) { - adb_mutex_unlock(&transport_lock); VLOG(TRANSPORT) << "socket transport " << transport->serial << " is already in transport_list and fails to register"; delete t; @@ -970,7 +968,8 @@ int register_socket_transport(int s, const char *serial, int port, int local) { pending_list.push_front(t); t->serial = strdup(serial); - adb_mutex_unlock(&transport_lock); + + lock.unlock(); register_transport(t); return 0; @@ -980,20 +979,19 @@ int register_socket_transport(int s, const char *serial, int port, int local) { atransport *find_transport(const char *serial) { atransport* result = nullptr; - adb_mutex_lock(&transport_lock); + std::lock_guard lock(transport_lock); for (auto& t : transport_list) { if (t->serial && strcmp(serial, t->serial) == 0) { result = t; break; } } - adb_mutex_unlock(&transport_lock); return result; } void kick_all_tcp_devices() { - adb_mutex_lock(&transport_lock); + std::lock_guard lock(transport_lock); for (auto& t : transport_list) { if (t->IsTcpDevice()) { // Kicking breaks the read_transport thread of this transport out of any read, then @@ -1003,7 +1001,6 @@ void kick_all_tcp_devices() { t->Kick(); } } - adb_mutex_unlock(&transport_lock); } #endif @@ -1023,20 +1020,20 @@ void register_usb_transport(usb_handle* usb, const char* serial, t->devpath = strdup(devpath); } - adb_mutex_lock(&transport_lock); - pending_list.push_front(t); - adb_mutex_unlock(&transport_lock); + { + std::lock_guard lock(transport_lock); + pending_list.push_front(t); + } register_transport(t); } // This should only be used for transports with connection_state == kCsNoPerm. void unregister_usb_transport(usb_handle *usb) { - adb_mutex_lock(&transport_lock); + std::lock_guard lock(transport_lock); transport_list.remove_if([usb](atransport* t) { return t->usb == usb && t->connection_state == kCsNoPerm; }); - adb_mutex_unlock(&transport_lock); } int check_header(apacket *p, atransport *t) diff --git a/transport_local.cpp b/transport_local.cpp index 89e950d..f895943 100644 --- a/transport_local.cpp +++ b/transport_local.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include @@ -47,7 +48,7 @@ // connected. #define ADB_LOCAL_TRANSPORT_MAX 16 -ADB_MUTEX_DEFINE(local_transports_lock); +static std::mutex& local_transports_lock = *new std::mutex(); /* we keep a list of opened transports. The atransport struct knows to which * local transport it is connected. The list is used to detect when we're @@ -384,14 +385,13 @@ static void remote_kick(atransport *t) #if ADB_HOST int nn; - adb_mutex_lock( &local_transports_lock ); + std::lock_guard lock(local_transports_lock); for (nn = 0; nn < ADB_LOCAL_TRANSPORT_MAX; nn++) { if (local_transports[nn] == t) { local_transports[nn] = NULL; break; } } - adb_mutex_unlock( &local_transports_lock ); #endif } @@ -435,9 +435,8 @@ static atransport* find_emulator_transport_by_adb_port_locked(int adb_port) atransport* find_emulator_transport_by_adb_port(int adb_port) { - adb_mutex_lock( &local_transports_lock ); + std::lock_guard lock(local_transports_lock); atransport* result = find_emulator_transport_by_adb_port_locked(adb_port); - adb_mutex_unlock( &local_transports_lock ); return result; } @@ -455,9 +454,8 @@ int get_available_local_transport_index_locked() int get_available_local_transport_index() { - adb_mutex_lock( &local_transports_lock ); + std::lock_guard lock(local_transports_lock); int result = get_available_local_transport_index_locked(); - adb_mutex_unlock( &local_transports_lock ); return result; } #endif @@ -477,26 +475,20 @@ int init_socket_transport(atransport *t, int s, int adb_port, int local) #if ADB_HOST if (local) { - adb_mutex_lock( &local_transports_lock ); - { - t->SetLocalPortForEmulator(adb_port); - atransport* existing_transport = - find_emulator_transport_by_adb_port_locked(adb_port); - int index = get_available_local_transport_index_locked(); - if (existing_transport != NULL) { - D("local transport for port %d already registered (%p)?", - adb_port, existing_transport); - fail = -1; - } else if (index < 0) { - // Too many emulators. - D("cannot register more emulators. Maximum is %d", - ADB_LOCAL_TRANSPORT_MAX); - fail = -1; - } else { - local_transports[index] = t; - } - } - adb_mutex_unlock( &local_transports_lock ); + std::lock_guard lock(local_transports_lock); + t->SetLocalPortForEmulator(adb_port); + atransport* existing_transport = find_emulator_transport_by_adb_port_locked(adb_port); + int index = get_available_local_transport_index_locked(); + if (existing_transport != NULL) { + D("local transport for port %d already registered (%p)?", adb_port, existing_transport); + fail = -1; + } else if (index < 0) { + // Too many emulators. + D("cannot register more emulators. Maximum is %d", ADB_LOCAL_TRANSPORT_MAX); + fail = -1; + } else { + local_transports[index] = t; + } } #endif return fail; diff --git a/transport_test.cpp b/transport_test.cpp index a6db07a..8b38e03 100644 --- a/transport_test.cpp +++ b/transport_test.cpp @@ -20,27 +20,6 @@ #include "adb.h" -class TransportSetup { -public: - TransportSetup() { -#ifdef _WIN32 - // Use extern instead of including sysdeps.h which brings in various macros - // that conflict with APIs used in this file. - extern void adb_sysdeps_init(void); - adb_sysdeps_init(); -#else - // adb_sysdeps_init() is an inline function that we cannot link against. -#endif - } -}; - -// Static initializer will call adb_sysdeps_init() before main() to initialize -// the transport mutex before it is used in the tests. Alternatives would be to -// use __attribute__((constructor)) here or to use that or a static initializer -// for adb_sysdeps_init() itself in sysdeps_win32.cpp (caveats of unclear -// init order), or to use a test fixture whose SetUp() could do the init once. -static TransportSetup g_TransportSetup; - TEST(transport, kick_transport) { atransport t; static size_t kick_count; diff --git a/usb_linux_client.cpp b/usb_linux_client.cpp index 0ba6b4b..1b05439 100644 --- a/usb_linux_client.cpp +++ b/usb_linux_client.cpp @@ -32,6 +32,8 @@ #include #include +#include +#include #include @@ -54,12 +56,14 @@ static int dummy_fd = -1; -struct usb_handle -{ - adb_cond_t notify; - adb_mutex_t lock; - bool open_new_connection; +struct usb_handle { + usb_handle() : kicked(false) { + } + + std::condition_variable notify; + std::mutex lock; std::atomic kicked; + bool open_new_connection = true; int (*write)(usb_handle *h, const void *data, int len); int (*read)(usb_handle *h, void *data, int len); @@ -67,12 +71,12 @@ struct usb_handle void (*close)(usb_handle *h); // Legacy f_adb - int fd; + int fd = -1; // FunctionFS - int control; - int bulk_out; /* "out" from the host's perspective => source for adbd */ - int bulk_in; /* "in" from the host's perspective => sink for adbd */ + int control = -1; + int bulk_out = -1; /* "out" from the host's perspective => source for adbd */ + int bulk_in = -1; /* "in" from the host's perspective => sink for adbd */ }; struct func_desc { @@ -248,12 +252,12 @@ static void usb_adb_open_thread(void* x) { while (true) { // wait until the USB device needs opening - adb_mutex_lock(&usb->lock); + std::unique_lock lock(usb->lock); while (!usb->open_new_connection) { - adb_cond_wait(&usb->notify, &usb->lock); + usb->notify.wait(lock); } usb->open_new_connection = false; - adb_mutex_unlock(&usb->lock); + lock.unlock(); D("[ usb_thread - opening device ]"); do { @@ -339,27 +343,20 @@ static void usb_adb_close(usb_handle *h) { h->kicked = false; adb_close(h->fd); // Notify usb_adb_open_thread to open a new connection. - adb_mutex_lock(&h->lock); + h->lock.lock(); h->open_new_connection = true; - adb_cond_signal(&h->notify); - adb_mutex_unlock(&h->lock); + h->lock.unlock(); + h->notify.notify_one(); } static void usb_adb_init() { - usb_handle* h = reinterpret_cast(calloc(1, sizeof(usb_handle))); - if (h == nullptr) fatal("couldn't allocate usb_handle"); + usb_handle* h = new usb_handle(); h->write = usb_adb_write; h->read = usb_adb_read; h->kick = usb_adb_kick; h->close = usb_adb_close; - h->kicked = false; - h->fd = -1; - - h->open_new_connection = true; - adb_cond_init(&h->notify, 0); - adb_mutex_init(&h->lock, 0); // Open the file /dev/android_adb_enable to trigger // the enabling of the adb USB function in the kernel. @@ -468,12 +465,12 @@ static void usb_ffs_open_thread(void* x) { while (true) { // wait until the USB device needs opening - adb_mutex_lock(&usb->lock); + std::unique_lock lock(usb->lock); while (!usb->open_new_connection) { - adb_cond_wait(&usb->notify, &usb->lock); + usb->notify.wait(lock); } usb->open_new_connection = false; - adb_mutex_unlock(&usb->lock); + lock.unlock(); while (true) { if (init_functionfs(usb)) { @@ -557,31 +554,22 @@ static void usb_ffs_close(usb_handle *h) { adb_close(h->bulk_out); adb_close(h->bulk_in); // Notify usb_adb_open_thread to open a new connection. - adb_mutex_lock(&h->lock); + h->lock.lock(); h->open_new_connection = true; - adb_cond_signal(&h->notify); - adb_mutex_unlock(&h->lock); + h->lock.unlock(); + h->notify.notify_one(); } static void usb_ffs_init() { D("[ usb_init - using FunctionFS ]"); - usb_handle* h = reinterpret_cast(calloc(1, sizeof(usb_handle))); - if (h == nullptr) fatal("couldn't allocate usb_handle"); + usb_handle* h = new usb_handle(); h->write = usb_ffs_write; h->read = usb_ffs_read; h->kick = usb_ffs_kick; h->close = usb_ffs_close; - h->kicked = false; - h->control = -1; - h->bulk_out = -1; - h->bulk_out = -1; - - h->open_new_connection = true; - adb_cond_init(&h->notify, 0); - adb_mutex_init(&h->lock, 0); D("[ usb_init - starting thread ]"); if (!adb_thread_create(usb_ffs_open_thread, h)) { @@ -608,6 +596,7 @@ int usb_read(usb_handle *h, void *data, int len) { return h->read(h, data, len); } + int usb_close(usb_handle *h) { h->close(h); diff --git a/usb_windows.cpp b/usb_windows.cpp index 8ecca37..4649454 100644 --- a/usb_windows.cpp +++ b/usb_windows.cpp @@ -19,13 +19,17 @@ #include "sysdeps.h" #include // winsock.h *must* be included before windows.h. -#include +#include +#include +#include + #include #include #include -#include -#include -#include + +#include + +#include #include @@ -73,7 +77,7 @@ static usb_handle handle_list = { }; /// Locker for the list of opened usb handles -ADB_MUTEX_DEFINE( usb_lock ); +static std::mutex& usb_lock = *new std::mutex(); /// Checks if there is opened usb handle in handle_list for this device. int known_device(const wchar_t* dev_name); @@ -141,9 +145,8 @@ int known_device(const wchar_t* dev_name) { int ret = 0; if (NULL != dev_name) { - adb_mutex_lock(&usb_lock); + std::lock_guard lock(usb_lock); ret = known_device_locked(dev_name); - adb_mutex_unlock(&usb_lock); } return ret; @@ -153,11 +156,10 @@ int register_new_device(usb_handle* handle) { if (NULL == handle) return 0; - adb_mutex_lock(&usb_lock); + std::lock_guard lock(usb_lock); // Check if device is already in the list if (known_device_locked(handle->interface_name)) { - adb_mutex_unlock(&usb_lock); return 0; } @@ -167,8 +169,6 @@ int register_new_device(usb_handle* handle) { handle->prev->next = handle; handle->next->prev = handle; - adb_mutex_unlock(&usb_lock); - return 1; } @@ -493,11 +493,8 @@ static void usb_kick_locked(usb_handle* handle) { void usb_kick(usb_handle* handle) { D("usb_kick"); if (NULL != handle) { - adb_mutex_lock(&usb_lock); - + std::lock_guard lock(usb_lock); usb_kick_locked(handle); - - adb_mutex_unlock(&usb_lock); } else { errno = EINVAL; } @@ -508,17 +505,17 @@ int usb_close(usb_handle* handle) { if (NULL != handle) { // Remove handle from the list - adb_mutex_lock(&usb_lock); - - if ((handle->next != handle) && (handle->prev != handle)) { - handle->next->prev = handle->prev; - handle->prev->next = handle->next; - handle->prev = handle; - handle->next = handle; + { + std::lock_guard lock(usb_lock); + + if ((handle->next != handle) && (handle->prev != handle)) { + handle->next->prev = handle->prev; + handle->prev->next = handle->next; + handle->prev = handle; + handle->next = handle; + } } - adb_mutex_unlock(&usb_lock); - // Cleanup handle usb_cleanup_handle(handle); free(handle); @@ -651,9 +648,8 @@ void find_devices() { static void kick_devices() { // Need to acquire lock to safely walk the list which might be modified // by another thread. - adb_mutex_lock(&usb_lock); + std::lock_guard lock(usb_lock); for (usb_handle* usb = handle_list.next; usb != &handle_list; usb = usb->next) { usb_kick_locked(usb); } - adb_mutex_unlock(&usb_lock); } -- cgit v1.2.3