From 46de1d7f03b7debfd14ccf77a6e63fa2e66f9f04 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 12 Apr 2017 13:57:06 -0700 Subject: [PATCH 1/3] adb: don't try to resolve 'localhost' Misconfigured systems can have localhost pointing to an address that isn't 127.0.0.1 or ::1. adb is the only caller of the libcutils socket_loopback functions, so move them into adb and switch the implementations over to using INADDR_LOOPBACK and in6addr_loopback, instead of resolving 'localhost' when connecting. Bug: http://b/37282612 Test: `killall adb; adb shell` Test: `killall adb; ip addr del 127.0.0.1/8 dev lo; adb shell` Change-Id: I01c1885f1d9757ad0f7b353dd04b4d1f057741c8 --- adb/Android.mk | 3 + adb/sysdeps.h | 14 +-- adb/sysdeps/network.h | 22 ++++ adb/sysdeps/posix/network.cpp | 127 ++++++++++++++++++++++++ libcutils/Android.bp | 1 - libcutils/include/cutils/sockets.h | 2 - libcutils/socket_loopback_server_unix.c | 88 ---------------- 7 files changed, 153 insertions(+), 104 deletions(-) create mode 100644 adb/sysdeps/network.h create mode 100644 adb/sysdeps/posix/network.cpp delete mode 100644 libcutils/socket_loopback_server_unix.c diff --git a/adb/Android.mk b/adb/Android.mk index e84120542..94ccc08ee 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -81,12 +81,14 @@ LIBADB_windows_CFLAGS := \ LIBADB_darwin_SRC_FILES := \ sysdeps_unix.cpp \ + sysdeps/posix/network.cpp \ client/usb_dispatch.cpp \ client/usb_libusb.cpp \ client/usb_osx.cpp \ LIBADB_linux_SRC_FILES := \ sysdeps_unix.cpp \ + sysdeps/posix/network.cpp \ client/usb_dispatch.cpp \ client/usb_libusb.cpp \ client/usb_linux.cpp \ @@ -123,6 +125,7 @@ LOCAL_SRC_FILES := \ $(LIBADB_SRC_FILES) \ adbd_auth.cpp \ jdwp_service.cpp \ + sysdeps/posix/network.cpp \ LOCAL_SANITIZE := $(adb_target_sanitize) diff --git a/adb/sysdeps.h b/adb/sysdeps.h index f95a8556f..5cd0cd1ac 100644 --- a/adb/sysdeps.h +++ b/adb/sysdeps.h @@ -35,6 +35,7 @@ #include #include "sysdeps/errno.h" +#include "sysdeps/network.h" #include "sysdeps/stat.h" /* @@ -248,8 +249,6 @@ extern int unix_open(const char* path, int options, ...); int unix_isatty(int fd); #define isatty ___xxx_isatty -int network_loopback_client(int port, int type, std::string* error); -int network_loopback_server(int port, int type, std::string* error); int network_inaddr_any_server(int port, int type, std::string* error); inline int network_local_client(const char* name, int namespace_id, int type, std::string* error) { @@ -587,17 +586,6 @@ inline int _fd_set_error_str(int fd, std::string* error) { return fd; } -inline int network_loopback_client(int port, int type, std::string* error) { - return _fd_set_error_str(socket_network_client("localhost", port, type), error); -} - -inline int network_loopback_server(int port, int type, std::string* error) { - int fd = socket_loopback_server(port, type); - if (fd < 0 && errno == EAFNOSUPPORT) - return _fd_set_error_str(socket_loopback_server6(port, type), error); - return _fd_set_error_str(fd, error); -} - inline int network_inaddr_any_server(int port, int type, std::string* error) { return _fd_set_error_str(socket_inaddr_any_server(port, type), error); } diff --git a/adb/sysdeps/network.h b/adb/sysdeps/network.h new file mode 100644 index 000000000..83ce37112 --- /dev/null +++ b/adb/sysdeps/network.h @@ -0,0 +1,22 @@ +#pragma once + +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +int network_loopback_client(int port, int type, std::string* error); +int network_loopback_server(int port, int type, std::string* error); diff --git a/adb/sysdeps/posix/network.cpp b/adb/sysdeps/posix/network.cpp new file mode 100644 index 000000000..45da5af4a --- /dev/null +++ b/adb/sysdeps/posix/network.cpp @@ -0,0 +1,127 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "sysdeps/network.h" + +#include +#include +#include + +#include + +#include "adb_unique_fd.h" + +static void set_error(std::string* error) { + if (error) { + *error = strerror(errno); + } +} + +static sockaddr* loopback_addr4(sockaddr_storage* addr, socklen_t* addrlen, int port) { + struct sockaddr_in* addr4 = reinterpret_cast(addr); + *addrlen = sizeof(*addr4); + + addr4->sin_family = AF_INET; + addr4->sin_addr.s_addr = htonl(INADDR_LOOPBACK); + addr4->sin_port = htons(port); + return reinterpret_cast(addr); +} + +static sockaddr* loopback_addr6(sockaddr_storage* addr, socklen_t* addrlen, int port) { + struct sockaddr_in6* addr6 = reinterpret_cast(addr); + *addrlen = sizeof(*addr6); + + addr6->sin6_family = AF_INET6; + addr6->sin6_addr = in6addr_loopback; + addr6->sin6_port = htons(port); + return reinterpret_cast(addr); +} + +static int _network_loopback_client(bool ipv6, int port, int type, std::string* error) { + unique_fd s(socket(ipv6 ? AF_INET6 : AF_INET, type, 0)); + if (s == -1) { + set_error(error); + return -1; + } + + struct sockaddr_storage addr_storage = {}; + socklen_t addrlen = sizeof(addr_storage); + sockaddr* addr = (ipv6 ? loopback_addr6 : loopback_addr4)(&addr_storage, &addrlen, 0); + + if (bind(s.get(), addr, addrlen) != 0) { + set_error(error); + return -1; + } + + addr = (ipv6 ? loopback_addr6 : loopback_addr4)(&addr_storage, &addrlen, port); + + if (connect(s.get(), addr, addrlen) != 0) { + set_error(error); + return -1; + } + + return s.release(); +} + +int network_loopback_client(int port, int type, std::string* error) { + // Try IPv4 first, use IPv6 as a fallback. + int rc = _network_loopback_client(false, port, type, error); + if (rc == -1) { + return _network_loopback_client(true, port, type, error); + } + return rc; +} + +static int _network_loopback_server(bool ipv6, int port, int type, std::string* error) { + unique_fd s(socket(ipv6 ? AF_INET6 : AF_INET, type, 0)); + if (s == -1) { + set_error(error); + return -1; + } + + int n = 1; + setsockopt(s.get(), SOL_SOCKET, SO_REUSEADDR, &n, sizeof(n)); + + struct sockaddr_storage addr_storage = {}; + socklen_t addrlen = sizeof(addr_storage); + sockaddr* addr = (ipv6 ? loopback_addr6 : loopback_addr4)(&addr_storage, &addrlen, port); + + if (bind(s, addr, addrlen) != 0) { + set_error(error); + return -1; + } + + if (type == SOCK_STREAM || type == SOCK_SEQPACKET) { + // Arbitrarily selected value, ported from libcutils. + if (listen(s, 4) != 0) { + set_error(error); + return -1; + } + } + + return s.release(); +} + +int network_loopback_server(int port, int type, std::string* error) { + int rc = _network_loopback_server(false, port, type, error); + + // Only attempt to listen on IPv6 if IPv4 is unavailable. + // We don't want to start an IPv6 server if there's already an IPv4 one running. + if (rc == -1 && (errno == EADDRNOTAVAIL || errno == EAFNOSUPPORT)) { + return _network_loopback_server(true, port, type, error); + } + return rc; +} diff --git a/libcutils/Android.bp b/libcutils/Android.bp index f668f18e5..bb82f4da3 100644 --- a/libcutils/Android.bp +++ b/libcutils/Android.bp @@ -24,7 +24,6 @@ libcutils_nonwindows_sources = [ "socket_inaddr_any_server_unix.c", "socket_local_client_unix.c", "socket_local_server_unix.c", - "socket_loopback_server_unix.c", "socket_network_client_unix.c", "sockets_unix.cpp", "str_parms.c", diff --git a/libcutils/include/cutils/sockets.h b/libcutils/include/cutils/sockets.h index d724dd6a5..b24468bf4 100644 --- a/libcutils/include/cutils/sockets.h +++ b/libcutils/include/cutils/sockets.h @@ -88,8 +88,6 @@ int android_get_control_socket(const char* name); cutils_socket_t socket_network_client(const char* host, int port, int type); int socket_network_client_timeout(const char* host, int port, int type, int timeout, int* getaddrinfo_error); -int socket_loopback_server(int port, int type); -int socket_loopback_server6(int port, int type); int socket_local_server(const char* name, int namespaceId, int type); int socket_local_server_bind(int s, const char* name, int namespaceId); int socket_local_client_connect(int fd, const char *name, int namespaceId, diff --git a/libcutils/socket_loopback_server_unix.c b/libcutils/socket_loopback_server_unix.c deleted file mode 100644 index 7b92fd62d..000000000 --- a/libcutils/socket_loopback_server_unix.c +++ /dev/null @@ -1,88 +0,0 @@ -/* -** Copyright 2006, The Android Open Source Project -** -** Licensed under the Apache License, Version 2.0 (the "License"); -** you may not use this file except in compliance with the License. -** You may obtain a copy of the License at -** -** http://www.apache.org/licenses/LICENSE-2.0 -** -** Unless required by applicable law or agreed to in writing, software -** distributed under the License is distributed on an "AS IS" BASIS, -** WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -** See the License for the specific language governing permissions and -** limitations under the License. -*/ - -#include -#include -#include -#include -#include - -#define LISTEN_BACKLOG 4 - -#if !defined(_WIN32) -#include -#include -#include -#include -#endif - -#include - -static int _socket_loopback_server(int family, int type, struct sockaddr * addr, size_t size) -{ - int s, n; - - s = socket(family, type, 0); - if(s < 0) - return -1; - - n = 1; - setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *) &n, sizeof(n)); - - if(bind(s, addr, size) < 0) { - close(s); - return -1; - } - - if (type == SOCK_STREAM) { - int ret; - - ret = listen(s, LISTEN_BACKLOG); - - if (ret < 0) { - close(s); - return -1; - } - } - - return s; -} - -/* open listen() port on loopback IPv6 interface */ -int socket_loopback_server6(int port, int type) -{ - struct sockaddr_in6 addr; - - memset(&addr, 0, sizeof(addr)); - addr.sin6_family = AF_INET6; - addr.sin6_port = htons(port); - addr.sin6_addr = in6addr_loopback; - - return _socket_loopback_server(AF_INET6, type, (struct sockaddr *) &addr, sizeof(addr)); -} - -/* open listen() port on loopback interface */ -int socket_loopback_server(int port, int type) -{ - struct sockaddr_in addr; - - memset(&addr, 0, sizeof(addr)); - addr.sin_family = AF_INET; - addr.sin_port = htons(port); - addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); - - return _socket_loopback_server(AF_INET, type, (struct sockaddr *) &addr, sizeof(addr)); -} From e1dacfc1b65b04958924d1f982263d4aa71b0b94 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 12 Apr 2017 17:00:49 -0700 Subject: [PATCH 2/3] adb: kill adb_thread_{create, join, detach, exit}. We have std::thread now, so we can delete this cruft. Test: python test_device.py Test: adb_test Test: wine adb_test.exe Test: /data/nativetest/adbd_test/adbd_test Change-Id: Ie1c1792547b20dec45e2a62ce6515fcb981c3ef8 --- adb/client/usb_linux.cpp | 6 +- adb/client/usb_osx.cpp | 6 +- adb/client/usb_windows.cpp | 14 ++--- adb/commandline.cpp | 9 +-- adb/daemon/mdns.cpp | 10 ++-- adb/daemon/usb.cpp | 4 +- adb/fdevent_test.cpp | 16 +++--- adb/fdevent_test.h | 6 +- adb/services.cpp | 10 +--- adb/shell_service.cpp | 8 +-- adb/socket_test.cpp | 30 +++------- adb/sysdeps.h | 115 ------------------------------------- adb/sysdeps_test.cpp | 66 ++++----------------- adb/transport.cpp | 10 +--- adb/transport_local.cpp | 16 ++---- 15 files changed, 62 insertions(+), 264 deletions(-) diff --git a/adb/client/usb_linux.cpp b/adb/client/usb_linux.cpp index 13b7674fc..3a45dbd71 100644 --- a/adb/client/usb_linux.cpp +++ b/adb/client/usb_linux.cpp @@ -574,7 +574,7 @@ static void register_device(const char* dev_name, const char* dev_path, register_usb_transport(done_usb, serial.c_str(), dev_path, done_usb->writeable); } -static void device_poll_thread(void*) { +static void device_poll_thread() { adb_thread_setname("device poll"); D("Created device thread"); while (true) { @@ -593,8 +593,6 @@ void usb_init() { actions.sa_handler = [](int) {}; sigaction(SIGALRM, &actions, nullptr); - if (!adb_thread_create(device_poll_thread, nullptr)) { - fatal_errno("cannot create device_poll thread"); - } + std::thread(device_poll_thread).detach(); } } // namespace native diff --git a/adb/client/usb_osx.cpp b/adb/client/usb_osx.cpp index d4fc7c0eb..8713b2c4b 100644 --- a/adb/client/usb_osx.cpp +++ b/adb/client/usb_osx.cpp @@ -405,7 +405,7 @@ err_get_num_ep: std::mutex& operate_device_lock = *new std::mutex(); -static void RunLoopThread(void* unused) { +static void RunLoopThread() { adb_thread_setname("RunLoop"); VLOG(USB) << "RunLoopThread started"; @@ -436,9 +436,7 @@ void usb_init() { usb_inited_flag = false; - if (!adb_thread_create(RunLoopThread, nullptr)) { - fatal_errno("cannot create RunLoop thread"); - } + std::thread(RunLoopThread).detach(); // Wait for initialization to finish while (!usb_inited_flag) { diff --git a/adb/client/usb_windows.cpp b/adb/client/usb_windows.cpp index 640e91ec3..9e00a5d6c 100644 --- a/adb/client/usb_windows.cpp +++ b/adb/client/usb_windows.cpp @@ -103,7 +103,7 @@ static void kick_devices(); /// Entry point for thread that polls (every second) for new usb interfaces. /// This routine calls find_devices in infinite loop. -static void device_poll_thread(void*); +static void device_poll_thread(); /// Initializes this module void usb_init(); @@ -174,7 +174,7 @@ int register_new_device(usb_handle* handle) { return 1; } -void device_poll_thread(void*) { +void device_poll_thread() { adb_thread_setname("Device Poll"); D("Created device thread"); @@ -203,7 +203,7 @@ static LRESULT CALLBACK _power_window_proc(HWND hwnd, UINT uMsg, WPARAM wParam, return DefWindowProcW(hwnd, uMsg, wParam, lParam); } -static void _power_notification_thread(void*) { +static void _power_notification_thread() { // This uses a thread with its own window message pump to get power // notifications. If adb runs from a non-interactive service account, this // might not work (not sure). If that happens to not work, we could use @@ -258,12 +258,8 @@ static void _power_notification_thread(void*) { } void usb_init() { - if (!adb_thread_create(device_poll_thread, nullptr)) { - fatal_errno("cannot create device poll thread"); - } - if (!adb_thread_create(_power_notification_thread, nullptr)) { - fatal_errno("cannot create power notification thread"); - } + std::thread(device_poll_thread).detach(); + std::thread(_power_notification_thread).detach(); } usb_handle* do_usb_open(const wchar_t* interface_name) { diff --git a/adb/commandline.cpp b/adb/commandline.cpp index 4979eef5c..730dc7246 100644 --- a/adb/commandline.cpp +++ b/adb/commandline.cpp @@ -663,13 +663,8 @@ static int RemoteShell(bool use_shell_protocol, const std::string& type_arg, #endif // TODO: combine read_and_dump with stdin_read_thread to make life simpler? - int exit_code = 1; - if (!adb_thread_create(stdin_read_thread_loop, args)) { - PLOG(ERROR) << "error starting stdin read thread"; - delete args; - } else { - exit_code = read_and_dump(fd, use_shell_protocol); - } + std::thread(stdin_read_thread_loop, args).detach(); + int exit_code = read_and_dump(fd, use_shell_protocol); // TODO: properly exit stdin_read_thread_loop and close |fd|. diff --git a/adb/daemon/mdns.cpp b/adb/daemon/mdns.cpp index 781114351..849378fef 100644 --- a/adb/daemon/mdns.cpp +++ b/adb/daemon/mdns.cpp @@ -17,12 +17,14 @@ #include "adb_mdns.h" #include "sysdeps.h" -#include #include #include -#include #include +#include +#include +#include + #include #include @@ -58,7 +60,7 @@ static void mdns_callback(DNSServiceRef /*ref*/, } } -static void setup_mdns_thread(void* /* unused */) { +static void setup_mdns_thread() { start_mdns(); std::lock_guard lock(mdns_lock); @@ -88,7 +90,7 @@ static void teardown_mdns() { void setup_mdns(int port_in) { port = port_in; - adb_thread_create(setup_mdns_thread, nullptr, nullptr); + std::thread(setup_mdns_thread).detach(); // TODO: Make this more robust against a hard kill. atexit(teardown_mdns); diff --git a/adb/daemon/usb.cpp b/adb/daemon/usb.cpp index 92e9039a7..7e46b0265 100644 --- a/adb/daemon/usb.cpp +++ b/adb/daemon/usb.cpp @@ -451,9 +451,7 @@ static void usb_ffs_init() { h->close = usb_ffs_close; D("[ usb_init - starting thread ]"); - if (!adb_thread_create(usb_ffs_open_thread, h)) { - fatal_errno("[ cannot create usb thread ]\n"); - } + std::thread(usb_ffs_open_thread, h).detach(); } void usb_init() { diff --git a/adb/fdevent_test.cpp b/adb/fdevent_test.cpp index c933ed528..bdb973a01 100644 --- a/adb/fdevent_test.cpp +++ b/adb/fdevent_test.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "adb_io.h" @@ -77,9 +78,9 @@ struct ThreadArg { }; TEST_F(FdeventTest, fdevent_terminate) { - adb_thread_t thread; PrepareThread(); - ASSERT_TRUE(adb_thread_create([](void*) { fdevent_loop(); }, nullptr, &thread)); + + std::thread thread(fdevent_loop); TerminateThread(thread); } @@ -112,7 +113,6 @@ TEST_F(FdeventTest, smoke) { int fd_pair2[2]; ASSERT_EQ(0, adb_socketpair(fd_pair1)); ASSERT_EQ(0, adb_socketpair(fd_pair2)); - adb_thread_t thread; ThreadArg thread_arg; thread_arg.first_read_fd = fd_pair1[0]; thread_arg.last_write_fd = fd_pair2[1]; @@ -121,8 +121,7 @@ TEST_F(FdeventTest, smoke) { int reader = fd_pair2[0]; PrepareThread(); - ASSERT_TRUE(adb_thread_create(reinterpret_cast(FdEventThreadFunc), &thread_arg, - &thread)); + std::thread thread(FdEventThreadFunc, &thread_arg); for (size_t i = 0; i < MESSAGE_LOOP_COUNT; ++i) { std::string read_buffer = MESSAGE; @@ -152,7 +151,7 @@ static void InvalidFdEventCallback(int fd, unsigned events, void* userdata) { } } -static void InvalidFdThreadFunc(void*) { +static void InvalidFdThreadFunc() { const int INVALID_READ_FD = std::numeric_limits::max() - 1; size_t happened_event_count = 0; InvalidFdArg read_arg; @@ -171,7 +170,6 @@ static void InvalidFdThreadFunc(void*) { } TEST_F(FdeventTest, invalid_fd) { - adb_thread_t thread; - ASSERT_TRUE(adb_thread_create(InvalidFdThreadFunc, nullptr, &thread)); - ASSERT_TRUE(adb_thread_join(thread)); + std::thread thread(InvalidFdThreadFunc); + thread.join(); } diff --git a/adb/fdevent_test.h b/adb/fdevent_test.h index ef65b747f..f4215ae19 100644 --- a/adb/fdevent_test.h +++ b/adb/fdevent_test.h @@ -16,6 +16,8 @@ #include +#include + #include "socket.h" #include "sysdeps.h" @@ -59,10 +61,10 @@ class FdeventTest : public ::testing::Test { #endif } - void TerminateThread(adb_thread_t thread) { + void TerminateThread(std::thread& thread) { fdevent_terminate_loop(); ASSERT_TRUE(WriteFdExactly(dummy, "", 1)); - ASSERT_TRUE(adb_thread_join(thread)); + thread.join(); ASSERT_EQ(0, adb_close(dummy)); } }; diff --git a/adb/services.cpp b/adb/services.cpp index 47f0a03d7..f764c52cc 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -31,6 +31,8 @@ #include #endif +#include + #include #include #include @@ -259,13 +261,7 @@ static int create_service_thread(void (*func)(int, void *), void *cookie) sti->cookie = cookie; sti->fd = s[1]; - if (!adb_thread_create(service_bootstrap_func, sti)) { - free(sti); - adb_close(s[0]); - adb_close(s[1]); - printf("cannot create service thread\n"); - return -1; - } + std::thread(service_bootstrap_func, sti).detach(); D("service thread started, %d:%d",s[0], s[1]); return s[0]; diff --git a/adb/shell_service.cpp b/adb/shell_service.cpp index d4f334baf..ee821f834 100644 --- a/adb/shell_service.cpp +++ b/adb/shell_service.cpp @@ -90,6 +90,7 @@ #include #include +#include #include #include @@ -392,12 +393,7 @@ bool Subprocess::ForkAndExec(std::string* error) { bool Subprocess::StartThread(std::unique_ptr subprocess, std::string* error) { Subprocess* raw = subprocess.release(); - if (!adb_thread_create(ThreadHandler, raw)) { - *error = - android::base::StringPrintf("failed to create subprocess thread: %s", strerror(errno)); - kill(raw->pid_, SIGKILL); - return false; - } + std::thread(ThreadHandler, raw).detach(); return true; } diff --git a/adb/socket_test.cpp b/adb/socket_test.cpp index f56f7f790..f7c66dbac 100644 --- a/adb/socket_test.cpp +++ b/adb/socket_test.cpp @@ -42,10 +42,6 @@ struct ThreadArg { class LocalSocketTest : public FdeventTest {}; -static void FdEventThreadFunc(void*) { - fdevent_loop(); -} - constexpr auto SLEEP_FOR_FDEVENT = 100ms; TEST_F(LocalSocketTest, smoke) { @@ -88,8 +84,7 @@ TEST_F(LocalSocketTest, smoke) { connect(prev_tail, end); PrepareThread(); - adb_thread_t thread; - ASSERT_TRUE(adb_thread_create(FdEventThreadFunc, nullptr, &thread)); + std::thread thread(fdevent_loop); for (size_t i = 0; i < MESSAGE_LOOP_COUNT; ++i) { std::string read_buffer = MESSAGE; @@ -152,9 +147,7 @@ TEST_F(LocalSocketTest, close_socket_with_packet) { arg.cause_close_fd = cause_close_fd[1]; PrepareThread(); - adb_thread_t thread; - ASSERT_TRUE(adb_thread_create(reinterpret_cast(CloseWithPacketThreadFunc), - &arg, &thread)); + std::thread thread(CloseWithPacketThreadFunc, &arg); // Wait until the fdevent_loop() starts. std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); ASSERT_EQ(0, adb_close(cause_close_fd[0])); @@ -177,9 +170,7 @@ TEST_F(LocalSocketTest, read_from_closing_socket) { arg.cause_close_fd = cause_close_fd[1]; PrepareThread(); - adb_thread_t thread; - ASSERT_TRUE(adb_thread_create(reinterpret_cast(CloseWithPacketThreadFunc), - &arg, &thread)); + std::thread thread(CloseWithPacketThreadFunc, &arg); // Wait until the fdevent_loop() starts. std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); ASSERT_EQ(0, adb_close(cause_close_fd[0])); @@ -211,10 +202,7 @@ TEST_F(LocalSocketTest, write_error_when_having_packets) { arg.cause_close_fd = cause_close_fd[1]; PrepareThread(); - adb_thread_t thread; - ASSERT_TRUE(adb_thread_create(reinterpret_cast(CloseWithPacketThreadFunc), - &arg, &thread)); - + std::thread thread(CloseWithPacketThreadFunc, &arg); // Wait until the fdevent_loop() starts. std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); EXPECT_EQ(2u + GetAdditionalLocalSocketCount(), fdevent_installed_count()); @@ -252,9 +240,7 @@ TEST_F(LocalSocketTest, close_socket_in_CLOSE_WAIT_state) { int listen_fd = network_inaddr_any_server(5038, SOCK_STREAM, &error); ASSERT_GE(listen_fd, 0); - adb_thread_t client_thread; - ASSERT_TRUE(adb_thread_create(reinterpret_cast(ClientThreadFunc), nullptr, - &client_thread)); + std::thread client_thread(ClientThreadFunc); int accept_fd = adb_socket_accept(listen_fd, nullptr, nullptr); ASSERT_GE(accept_fd, 0); @@ -262,16 +248,14 @@ TEST_F(LocalSocketTest, close_socket_in_CLOSE_WAIT_state) { arg.socket_fd = accept_fd; PrepareThread(); - adb_thread_t thread; - ASSERT_TRUE(adb_thread_create(reinterpret_cast(CloseRdHupSocketThreadFunc), - &arg, &thread)); + std::thread thread(CloseRdHupSocketThreadFunc, &arg); // Wait until the fdevent_loop() starts. std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); EXPECT_EQ(1u + GetAdditionalLocalSocketCount(), fdevent_installed_count()); // Wait until the client closes its socket. - ASSERT_TRUE(adb_thread_join(client_thread)); + client_thread.join(); std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); ASSERT_EQ(GetAdditionalLocalSocketCount(), fdevent_installed_count()); diff --git a/adb/sysdeps.h b/adb/sysdeps.h index 5cd0cd1ac..49c784779 100644 --- a/adb/sysdeps.h +++ b/adb/sysdeps.h @@ -99,64 +99,6 @@ static __inline__ bool adb_is_separator(char c) { return c == '\\' || c == '/'; } -typedef void (*adb_thread_func_t)(void* arg); -typedef HANDLE adb_thread_t; - -struct adb_winthread_args { - adb_thread_func_t func; - void* arg; -}; - -static unsigned __stdcall adb_winthread_wrapper(void* heap_args) { - // Move the arguments from the heap onto the thread's stack. - adb_winthread_args thread_args = *static_cast(heap_args); - delete static_cast(heap_args); - thread_args.func(thread_args.arg); - return 0; -} - -static __inline__ bool adb_thread_create(adb_thread_func_t func, void* arg, - adb_thread_t* thread = nullptr) { - adb_winthread_args* args = new adb_winthread_args{.func = func, .arg = arg}; - uintptr_t handle = _beginthreadex(nullptr, 0, adb_winthread_wrapper, args, 0, nullptr); - if (handle != static_cast(0)) { - if (thread) { - *thread = reinterpret_cast(handle); - } else { - CloseHandle(thread); - } - return true; - } - return false; -} - -static __inline__ bool adb_thread_join(adb_thread_t thread) { - switch (WaitForSingleObject(thread, INFINITE)) { - case WAIT_OBJECT_0: - CloseHandle(thread); - return true; - - case WAIT_FAILED: - fprintf(stderr, "adb_thread_join failed: %s\n", - android::base::SystemErrorCodeToString(GetLastError()).c_str()); - break; - - default: - abort(); - } - - return false; -} - -static __inline__ bool adb_thread_detach(adb_thread_t thread) { - CloseHandle(thread); - return true; -} - -static __inline__ void __attribute__((noreturn)) adb_thread_exit() { - _endthreadex(0); -} - static __inline__ int adb_thread_setname(const std::string& name) { // TODO: See https://msdn.microsoft.com/en-us/library/xcb2z8hs.aspx for how to set // the thread name in Windows. Unfortunately, it only works during debugging, but @@ -164,14 +106,6 @@ static __inline__ int adb_thread_setname(const std::string& name) { return 0; } -static __inline__ adb_thread_t adb_thread_self() { - return GetCurrentThread(); -} - -static __inline__ bool adb_thread_equal(adb_thread_t lhs, adb_thread_t rhs) { - return GetThreadId(lhs) == GetThreadId(rhs); -} - static __inline__ unsigned long adb_thread_id() { return GetCurrentThreadId(); @@ -644,55 +578,6 @@ inline int adb_socket_get_local_port(int fd) { #define unix_write adb_write #define unix_close adb_close -// Win32 is limited to DWORDs for thread return values; limit the POSIX systems to this as well to -// ensure compatibility. -typedef void (*adb_thread_func_t)(void* arg); -typedef pthread_t adb_thread_t; - -struct adb_pthread_args { - adb_thread_func_t func; - void* arg; -}; - -static void* adb_pthread_wrapper(void* heap_args) { - // Move the arguments from the heap onto the thread's stack. - adb_pthread_args thread_args = *reinterpret_cast(heap_args); - delete static_cast(heap_args); - thread_args.func(thread_args.arg); - return nullptr; -} - -static __inline__ bool adb_thread_create(adb_thread_func_t start, void* arg, - adb_thread_t* thread = nullptr) { - pthread_t temp; - pthread_attr_t attr; - pthread_attr_init(&attr); - pthread_attr_setdetachstate(&attr, thread ? PTHREAD_CREATE_JOINABLE : PTHREAD_CREATE_DETACHED); - auto* pthread_args = new adb_pthread_args{.func = start, .arg = arg}; - errno = pthread_create(&temp, &attr, adb_pthread_wrapper, pthread_args); - if (errno == 0) { - if (thread) { - *thread = temp; - } - return true; - } - return false; -} - -static __inline__ bool adb_thread_join(adb_thread_t thread) { - errno = pthread_join(thread, nullptr); - return errno == 0; -} - -static __inline__ bool adb_thread_detach(adb_thread_t thread) { - errno = pthread_detach(thread); - return errno == 0; -} - -static __inline__ void __attribute__((noreturn)) adb_thread_exit() { - pthread_exit(nullptr); -} - static __inline__ int adb_thread_setname(const std::string& name) { #ifdef __APPLE__ return pthread_setname_np(name.c_str()); diff --git a/adb/sysdeps_test.cpp b/adb/sysdeps_test.cpp index 9007e75f3..edb0fb3d1 100644 --- a/adb/sysdeps_test.cpp +++ b/adb/sysdeps_test.cpp @@ -25,54 +25,6 @@ #include "sysdeps.h" #include "sysdeps/chrono.h" -static void increment_atomic_int(void* c) { - std::this_thread::sleep_for(1s); - reinterpret_cast*>(c)->fetch_add(1); -} - -TEST(sysdeps_thread, smoke) { - std::atomic counter(0); - - for (int i = 0; i < 100; ++i) { - ASSERT_TRUE(adb_thread_create(increment_atomic_int, &counter)); - } - - std::this_thread::sleep_for(2s); - ASSERT_EQ(100, counter.load()); -} - -TEST(sysdeps_thread, join) { - std::atomic counter(0); - std::vector threads(500); - for (size_t i = 0; i < threads.size(); ++i) { - ASSERT_TRUE(adb_thread_create(increment_atomic_int, &counter, &threads[i])); - } - - int current = counter.load(); - ASSERT_GE(current, 0); - // Make sure that adb_thread_create actually creates threads, and doesn't do something silly - // like synchronously run the function passed in. The sleep in increment_atomic_int should be - // enough to keep this from being flakey. - ASSERT_LT(current, 500); - - for (const auto& thread : threads) { - ASSERT_TRUE(adb_thread_join(thread)); - } - - ASSERT_EQ(500, counter.load()); -} - -TEST(sysdeps_thread, exit) { - adb_thread_t thread; - ASSERT_TRUE(adb_thread_create( - [](void*) { - adb_thread_exit(); - for (;;) continue; - }, - nullptr, &thread)); - ASSERT_TRUE(adb_thread_join(thread)); -} - TEST(sysdeps_socketpair, smoke) { int fds[2]; ASSERT_EQ(0, adb_socketpair(fds)) << strerror(errno); @@ -254,13 +206,13 @@ TEST(sysdeps_mutex, mutex_smoke) { static std::mutex &m = *new std::mutex(); m.lock(); ASSERT_FALSE(m.try_lock()); - adb_thread_create([](void*) { + std::thread thread([]() { ASSERT_FALSE(m.try_lock()); m.lock(); finished.store(true); std::this_thread::sleep_for(200ms); m.unlock(); - }, nullptr); + }); ASSERT_FALSE(finished.load()); std::this_thread::sleep_for(100ms); @@ -270,6 +222,8 @@ TEST(sysdeps_mutex, mutex_smoke) { m.lock(); ASSERT_TRUE(finished.load()); m.unlock(); + + thread.join(); } TEST(sysdeps_mutex, recursive_mutex_smoke) { @@ -279,12 +233,12 @@ TEST(sysdeps_mutex, recursive_mutex_smoke) { ASSERT_TRUE(m.try_lock()); m.unlock(); - adb_thread_create([](void*) { + std::thread thread([]() { ASSERT_FALSE(m.try_lock()); m.lock(); std::this_thread::sleep_for(500ms); m.unlock(); - }, nullptr); + }); std::this_thread::sleep_for(100ms); m.unlock(); @@ -292,6 +246,8 @@ TEST(sysdeps_mutex, recursive_mutex_smoke) { ASSERT_FALSE(m.try_lock()); m.lock(); m.unlock(); + + thread.join(); } TEST(sysdeps_condition_variable, smoke) { @@ -300,14 +256,16 @@ TEST(sysdeps_condition_variable, smoke) { static volatile bool flag = false; std::unique_lock lock(m); - adb_thread_create([](void*) { + std::thread thread([]() { m.lock(); flag = true; cond.notify_one(); m.unlock(); - }, nullptr); + }); while (!flag) { cond.wait(lock); } + + thread.join(); } diff --git a/adb/transport.cpp b/adb/transport.cpp index c951f5b29..4686841ec 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -509,13 +510,8 @@ static void transport_registration_func(int _fd, unsigned ev, void* data) { fdevent_set(&(t->transport_fde), FDE_READ); - if (!adb_thread_create(write_transport_thread, t)) { - fatal_errno("cannot create write_transport thread"); - } - - if (!adb_thread_create(read_transport_thread, t)) { - fatal_errno("cannot create read_transport thread"); - } + std::thread(write_transport_thread, t).detach(); + std::thread(read_transport_thread, t).detach(); } { diff --git a/adb/transport_local.cpp b/adb/transport_local.cpp index 4198a5247..408f51fa1 100644 --- a/adb/transport_local.cpp +++ b/adb/transport_local.cpp @@ -199,7 +199,7 @@ static std::vector& retry_ports = *new std::vector; std::mutex &retry_ports_lock = *new std::mutex; std::condition_variable &retry_ports_cond = *new std::condition_variable; -static void client_socket_thread(void* x) { +static void client_socket_thread(int) { adb_thread_setname("client_socket_thread"); D("transport: client_socket_thread() starting"); PollAllLocalPortsForEmulator(); @@ -244,9 +244,8 @@ static void client_socket_thread(void* x) { #else // ADB_HOST -static void server_socket_thread(void* arg) { +static void server_socket_thread(int port) { int serverfd, fd; - int port = (int) (uintptr_t) arg; adb_thread_setname("server socket"); D("transport: server_socket_thread() starting"); @@ -325,7 +324,7 @@ static void server_socket_thread(void* arg) { * the transport registration is completed. That's why we need to send the * 'start' request after the transport is registered. */ -static void qemu_socket_thread(void* arg) { +static void qemu_socket_thread(int port) { /* 'accept' request to the adb QEMUD service. */ static const char _accept_req[] = "accept"; /* 'start' request to the adb QEMUD service. */ @@ -333,7 +332,6 @@ static void qemu_socket_thread(void* arg) { /* 'ok' reply from the adb QEMUD service. */ static const char _ok_resp[] = "ok"; - const int port = (int) (uintptr_t) arg; int fd; char tmp[256]; char con_name[32]; @@ -350,7 +348,7 @@ static void qemu_socket_thread(void* arg) { /* This could be an older version of the emulator, that doesn't * implement adb QEMUD service. Fall back to the old TCP way. */ D("adb service is not available. Falling back to TCP socket."); - adb_thread_create(server_socket_thread, arg); + std::thread(server_socket_thread, port).detach(); return; } @@ -394,7 +392,7 @@ static void qemu_socket_thread(void* arg) { void local_init(int port) { - adb_thread_func_t func; + void (*func)(int); const char* debug_name = ""; #if ADB_HOST @@ -414,9 +412,7 @@ void local_init(int port) #endif // !ADB_HOST D("transport: local %s init", debug_name); - if (!adb_thread_create(func, (void *) (uintptr_t) port)) { - fatal_errno("cannot create local socket %s thread", debug_name); - } + std::thread(func, port).detach(); } static void remote_kick(atransport *t) From 47330e21db24b2bce3e238a596904e17d178a78d Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 12 Apr 2017 17:04:41 -0700 Subject: [PATCH 3/3] adb: statically link libbase into the tests. Make it easier to run the Windows tests with wine. Test: adb_test Test: wine adb_test.exe Change-Id: I243c3adf4f6519dd7a38666ab42b384858351e21 --- adb/Android.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adb/Android.mk b/adb/Android.mk index 94ccc08ee..d17b063f0 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -220,9 +220,9 @@ LOCAL_SRC_FILES_linux := $(LIBADB_TEST_linux_SRCS) LOCAL_SRC_FILES_darwin := $(LIBADB_TEST_darwin_SRCS) LOCAL_SRC_FILES_windows := $(LIBADB_TEST_windows_SRCS) LOCAL_SANITIZE := $(adb_host_sanitize) -LOCAL_SHARED_LIBRARIES := libbase LOCAL_STATIC_LIBRARIES := \ libadb \ + libbase \ libcrypto_utils \ libcrypto \ libcutils \