From db262ffa62d789d4889ebe73abbb29fa0f373125 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 8 Feb 2016 11:22:50 -0800 Subject: [PATCH 01/20] adb: fix mkdirs test. The behavior of mkdirs was changed a while ago, without updating the test. Change-Id: I2aaa73818933b281e911c42a14e3c843d8bd972a (cherry picked from commit 1172b2ec71db29533231ddc4d979be98d7503f0b) --- adb/adb_utils_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adb/adb_utils_test.cpp b/adb/adb_utils_test.cpp index 794dce63a..dfe6f20c3 100644 --- a/adb/adb_utils_test.cpp +++ b/adb/adb_utils_test.cpp @@ -112,7 +112,7 @@ TEST(adb_utils, adb_dirname) { } void test_mkdirs(const std::string basepath) { - EXPECT_TRUE(mkdirs(basepath)); + EXPECT_TRUE(mkdirs(adb_dirname(basepath))); EXPECT_NE(-1, adb_creat(basepath.c_str(), 0600)); EXPECT_FALSE(mkdirs(basepath + "/subdir/")); } From 9ad736a637de57d56b682674346b279dff5fea90 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Thu, 4 Feb 2016 11:59:12 -0800 Subject: [PATCH 02/20] adb: make ctrl-c when spawning a daemon not kill the daemon. Previously, using ctrl-c in a command that needs to spawn a daemon because one isn't already available would kill the daemon along with the foreground process. Bug: http://b/26982628 Change-Id: I7fefc531c3e4895423e7b466322b5426d01dc9ef (cherry picked from commit b72b3f8c92a0b2d4377d5bec15eb45869b8711ac) --- adb/client/main.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/adb/client/main.cpp b/adb/client/main.cpp index b7b30c50c..6397c5239 100644 --- a/adb/client/main.cpp +++ b/adb/client/main.cpp @@ -21,6 +21,7 @@ #include #include #include +#include // We only build the affinity WAR code for Linux. #if defined(__linux__) @@ -125,6 +126,13 @@ int adb_server_main(int is_daemon, int server_port, int ack_reply_fd) { close_stdin(); setup_daemon_logging(); +#if !defined(_WIN32) + // Set the process group so that ctrl-c in the spawning process doesn't kill us. + // Do this here instead of after the fork so that a ctrl-c between the "starting server" and + // "done starting server" messages gets a chance to terminate the server. + setpgrp(); +#endif + // Any error output written to stderr now goes to adb.log. We could // keep around a copy of the stderr fd and use that to write any errors // encountered by the following code, but that is probably overkill. From 65f7080c6681aaf6e9bb60d6f62c26939e4f9901 Mon Sep 17 00:00:00 2001 From: Yabin Cui Date: Mon, 8 Feb 2016 22:36:42 -0800 Subject: [PATCH 03/20] adb: setsid() for adb host server. To create a daemon for adb host server, we should call setsid() for the daemon process. However, previously we call setsid() for the adb client process, which results in nothing but EPERM error. Bug: 26982628 Change-Id: I2763ae3d5a243706927d7ef6af5095138c0ce2d8 (cherry picked from commit 6bf323b97a11194d6186f8db2ee4a5eaca8d0141) --- adb/adb.cpp | 2 -- adb/client/main.cpp | 10 ++++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/adb/adb.cpp b/adb/adb.cpp index 58ccd0a70..cb54d04e0 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -883,8 +883,6 @@ int launch_server(int server_port) fprintf(stderr, "ADB server didn't ACK\n" ); return -1; } - - setsid(); } #endif /* !defined(_WIN32) */ return 0; diff --git a/adb/client/main.cpp b/adb/client/main.cpp index 6397c5239..27b7109e2 100644 --- a/adb/client/main.cpp +++ b/adb/client/main.cpp @@ -127,10 +127,12 @@ int adb_server_main(int is_daemon, int server_port, int ack_reply_fd) { setup_daemon_logging(); #if !defined(_WIN32) - // Set the process group so that ctrl-c in the spawning process doesn't kill us. - // Do this here instead of after the fork so that a ctrl-c between the "starting server" and - // "done starting server" messages gets a chance to terminate the server. - setpgrp(); + // Start a new session for the daemon. Do this here instead of after the fork so + // that a ctrl-c between the "starting server" and "done starting server" messages + // gets a chance to terminate the server. + if (setsid() == -1) { + fatal("setsid() failed: %s", strerror(errno)); + } #endif // Any error output written to stderr now goes to adb.log. We could From 32a2b60c4e2213b6303031768dc71c4fd6ceb4cf Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 9 Feb 2016 16:44:54 -0800 Subject: [PATCH 04/20] adb: allow wine's output for sysdeps_win32 strerror test. Change-Id: Ia5d04a2347df1bcd8c7efcc1da9cec9725007a58 (cherry picked from commit 22554c91676d8981fbfc8b4811d8be3b78a0b74a) --- adb/sysdeps_win32_test.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/adb/sysdeps_win32_test.cpp b/adb/sysdeps_win32_test.cpp index 8f610cfda..c3a3fd7c1 100755 --- a/adb/sysdeps_win32_test.cpp +++ b/adb/sysdeps_win32_test.cpp @@ -87,8 +87,12 @@ TEST(sysdeps_win32, adb_strerror) { TestAdbStrError(-1, "Unknown error"); // Test very big, positive unknown error. TestAdbStrError(1000000, "Unknown error"); + // Test success case. - TestAdbStrError(0, "No error"); + // Wine returns "Success" for strerror(0), Windows returns "No error", so accept both. + std::string success = adb_strerror(0); + EXPECT_TRUE(success == "Success" || success == "No error") << "strerror(0) = " << success; + // Test error that regular strerror() should have a string for. TestAdbStrError(EPERM, "Operation not permitted"); // Test error that regular strerror() doesn't have a string for, but that From d302a15a60feb453bb018da99b9abae8dab2ca01 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 9 Feb 2016 14:59:09 -0800 Subject: [PATCH 05/20] adb: sysdeps: add support for joining threads. Bug: http://b/27105824 Change-Id: I44e4edbb2a59565c35f1f3e6a6394ac258591f95 (cherry picked from commit 3b3e10d0465506abc7d9e07c0381396b3726f183) --- adb/Android.mk | 1 + adb/sysdeps.h | 83 ++++++++++++++++++++++++++++++++++++++------ adb/sysdeps_test.cpp | 59 +++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 10 deletions(-) create mode 100644 adb/sysdeps_test.cpp diff --git a/adb/Android.mk b/adb/Android.mk index d62922355..baa498530 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -58,6 +58,7 @@ LIBADB_SRC_FILES := \ LIBADB_TEST_SRCS := \ adb_io_test.cpp \ adb_utils_test.cpp \ + sysdeps_test.cpp \ transport_test.cpp \ LIBADB_CFLAGS := \ diff --git a/adb/sysdeps.h b/adb/sysdeps.h index 16796cd5c..761a4c799 100644 --- a/adb/sysdeps.h +++ b/adb/sysdeps.h @@ -30,6 +30,7 @@ #include // Include this before open/unlink are defined as macros below. +#include #include /* @@ -114,13 +115,57 @@ static __inline__ void adb_mutex_unlock( adb_mutex_t* lock ) LeaveCriticalSection( lock ); } -typedef void* (*adb_thread_func_t)(void* arg); +typedef void* (*adb_thread_func_t)(void* arg); +typedef HANDLE adb_thread_t; -typedef void (*win_thread_func_t)(void* arg); +struct win_thread_args { + adb_thread_func_t func; + void* arg; +}; -static __inline__ bool adb_thread_create(adb_thread_func_t func, void* arg) { - uintptr_t tid = _beginthread((win_thread_func_t)func, 0, arg); - return (tid != static_cast(-1L)); +static unsigned __stdcall win_thread_wrapper(void* args) { + win_thread_args thread_args = *static_cast(args); + delete static_cast(args); + void* result = thread_args.func(thread_args.arg); + return reinterpret_cast(result); +} + +static __inline__ bool adb_thread_create(adb_thread_func_t func, void* arg, + adb_thread_t* thread = nullptr) { + win_thread_args* args = new win_thread_args{.func = func, .arg = arg}; + uintptr_t handle = _beginthreadex(nullptr, 0, win_thread_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__ int adb_thread_setname(const std::string& name) { @@ -658,14 +703,32 @@ static __inline__ int adb_socket_accept(int serverfd, struct sockaddr* addr, typedef void* (*adb_thread_func_t)( void* arg ); -static __inline__ bool adb_thread_create(adb_thread_func_t start, void* arg) { +typedef pthread_t adb_thread_t; + +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, PTHREAD_CREATE_DETACHED); + pthread_attr_setdetachstate(&attr, thread ? PTHREAD_CREATE_JOINABLE : PTHREAD_CREATE_DETACHED); + errno = pthread_create(&temp, &attr, start, arg); + if (errno == 0) { + if (thread) { + *thread = temp; + } + return true; + } + return false; +} - pthread_t thread; - errno = pthread_create(&thread, &attr, start, arg); - return (errno == 0); +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__ int adb_thread_setname(const std::string& name) { diff --git a/adb/sysdeps_test.cpp b/adb/sysdeps_test.cpp new file mode 100644 index 000000000..24a0d6f06 --- /dev/null +++ b/adb/sysdeps_test.cpp @@ -0,0 +1,59 @@ +/* + * Copyright (C) 2065 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 "sysdeps.h" + +static void* increment_atomic_int(void* c) { + sleep(1); + reinterpret_cast*>(c)->fetch_add(1); + return nullptr; +} + +TEST(sysdeps_thread, smoke) { + std::atomic counter(0); + + for (int i = 0; i < 100; ++i) { + ASSERT_TRUE(adb_thread_create(increment_atomic_int, &counter)); + } + + sleep(2); + 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()); +} From a1071c6924654de89e077f50f47271bc3e08c569 Mon Sep 17 00:00:00 2001 From: Spencer Low Date: Wed, 10 Feb 2016 15:03:50 -0800 Subject: [PATCH 06/20] adb: mkdirs fixes Fix pathological case where the directory to be created can't be created because there is already a file there. This was previously returning success because the wrong var was passed to directory_exists(). Fix test to exercise this situation. Also clarify tests. Change-Id: I0dc0f14084e0eda4e1498874d4ab2a6445d322ac Signed-off-by: Spencer Low (cherry picked from commit 85c45bd5a185f09f24bb0d790b2038fe72b567a9) --- adb/adb_utils.cpp | 14 +++++++------- adb/adb_utils_test.cpp | 16 +++++++++++----- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp index 474d1b403..8a16e51c0 100644 --- a/adb/adb_utils.cpp +++ b/adb/adb_utils.cpp @@ -134,7 +134,7 @@ std::string adb_dirname(const std::string& path) { return result; } -// Given a relative or absolute filepath, create the parent directory hierarchy +// Given a relative or absolute filepath, create the directory hierarchy // as needed. Returns true if the hierarchy is/was setup. bool mkdirs(const std::string& path) { // TODO: all the callers do unlink && mkdirs && adb_creat --- @@ -157,12 +157,12 @@ bool mkdirs(const std::string& path) { return true; } + const std::string parent(adb_dirname(path)); + // If dirname returned the same path as what we passed in, don't go recursive. // This can happen on Windows when walking up the directory hierarchy and not // finding anything that already exists (unlike POSIX that will eventually // find . or /). - const std::string parent(adb_dirname(path)); - if (parent == path) { errno = ENOENT; return false; @@ -174,14 +174,14 @@ bool mkdirs(const std::string& path) { } // Now that the parent directory hierarchy of 'path' has been ensured, - // create parent itself. + // create path itself. if (adb_mkdir(path, 0775) == -1) { - // Can't just check for errno == EEXIST because it might be a file that - // exists. const int saved_errno = errno; - if (directory_exists(parent)) { + // If someone else created the directory, that is ok. + if (directory_exists(path)) { return true; } + // There might be a pre-existing file at 'path', or there might have been some other error. errno = saved_errno; return false; } diff --git a/adb/adb_utils_test.cpp b/adb/adb_utils_test.cpp index dfe6f20c3..f1ebaa130 100644 --- a/adb/adb_utils_test.cpp +++ b/adb/adb_utils_test.cpp @@ -112,20 +112,26 @@ TEST(adb_utils, adb_dirname) { } void test_mkdirs(const std::string basepath) { - EXPECT_TRUE(mkdirs(adb_dirname(basepath))); - EXPECT_NE(-1, adb_creat(basepath.c_str(), 0600)); - EXPECT_FALSE(mkdirs(basepath + "/subdir/")); + // Test creating a directory hierarchy. + EXPECT_TRUE(mkdirs(basepath)); + // Test finding an existing directory hierarchy. + EXPECT_TRUE(mkdirs(basepath)); + const std::string filepath = basepath + "/file"; + // Verify that the hierarchy was created by trying to create a file in it. + EXPECT_NE(-1, adb_creat(filepath.c_str(), 0600)); + // If a file exists where we want a directory, the operation should fail. + EXPECT_FALSE(mkdirs(filepath)); } TEST(adb_utils, mkdirs) { TemporaryDir td; // Absolute paths. - test_mkdirs(std::string(td.path) + "/dir/subdir/file"); + test_mkdirs(std::string(td.path) + "/dir/subdir"); // Relative paths. ASSERT_EQ(0, chdir(td.path)) << strerror(errno); - test_mkdirs(std::string("relative/subrel/file")); + test_mkdirs(std::string("relative/subrel")); } #if !defined(_WIN32) From 7e76c895443850c7da5a5e4f07a7a16fa483c5bd Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 12 Feb 2016 11:33:53 -0800 Subject: [PATCH 07/20] adb: redact reference to secret internal time machine. Change-Id: Ic6744cc7c858576d7e6172460b32902e007b6fd3 (cherry picked from commit 6b42a2bfd5fcfabd8e80fa062a35a7e584c510a7) --- adb/sysdeps_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adb/sysdeps_test.cpp b/adb/sysdeps_test.cpp index 24a0d6f06..daceb89b9 100644 --- a/adb/sysdeps_test.cpp +++ b/adb/sysdeps_test.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2065 The Android Open Source Project + * Copyright (C) 2016 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. From d9db09c3158d3da6aad34fbb926888ceafab3a55 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 12 Feb 2016 14:31:15 -0800 Subject: [PATCH 08/20] adb: make adb_thread_func_t return void, add adb_thread_exit. Windows restricts the return value of threads to 32-bits, even on 64-bit platforms. Since we don't actually return meaningful values from thread, resolve this inconsistency with POSIX by making adb's thread abstraction only take void functions. Change-Id: I5c23b4432314f13bf16d606fd5e6b6b7b6ef98b5 (cherry picked from commit b5fea14e13bb6e41b36f374c954dc55faeef4627) --- adb/commandline.cpp | 4 +--- adb/services.cpp | 4 +--- adb/shell_service.cpp | 6 ++--- adb/sysdeps.h | 48 ++++++++++++++++++++++++++++++---------- adb/sysdeps_test.cpp | 14 ++++++++++-- adb/transport.cpp | 8 ++----- adb/transport_local.cpp | 33 ++++++++++++--------------- adb/usb_linux.cpp | 3 +-- adb/usb_linux_client.cpp | 12 ++++------ adb/usb_osx.cpp | 5 +---- adb/usb_windows.cpp | 10 +++------ 11 files changed, 77 insertions(+), 70 deletions(-) diff --git a/adb/commandline.cpp b/adb/commandline.cpp index 85ab4d18a..8e76168cb 100644 --- a/adb/commandline.cpp +++ b/adb/commandline.cpp @@ -482,7 +482,7 @@ struct StdinReadArgs { // Loops to read from stdin and push the data to the given FD. // The argument should be a pointer to a StdinReadArgs object. This function // will take ownership of the object and delete it when finished. -static void* stdin_read_thread_loop(void* x) { +static void stdin_read_thread_loop(void* x) { std::unique_ptr args(reinterpret_cast(x)); #if !defined(_WIN32) @@ -586,8 +586,6 @@ static void* stdin_read_thread_loop(void* x) { } } } - - return nullptr; } // Returns a shell service string with the indicated arguments and command. diff --git a/adb/services.cpp b/adb/services.cpp index 9cbf78794..75cbe5dd1 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -57,13 +57,11 @@ struct stinfo { void *cookie; }; -void *service_bootstrap_func(void *x) -{ +static void service_bootstrap_func(void* x) { stinfo* sti = reinterpret_cast(x); adb_thread_setname(android::base::StringPrintf("service %d", sti->fd)); sti->func(sti->fd, sti->cookie); free(sti); - return 0; } #if !ADB_HOST diff --git a/adb/shell_service.cpp b/adb/shell_service.cpp index d080e09bf..f84447f5f 100644 --- a/adb/shell_service.cpp +++ b/adb/shell_service.cpp @@ -198,7 +198,7 @@ class Subprocess { // Opens the file at |pts_name|. int OpenPtyChildFd(const char* pts_name, ScopedFd* error_sfd); - static void* ThreadHandler(void* userdata); + static void ThreadHandler(void* userdata); void PassDataStreams(); void WaitForExit(); @@ -465,7 +465,7 @@ int Subprocess::OpenPtyChildFd(const char* pts_name, ScopedFd* error_sfd) { return child_fd; } -void* Subprocess::ThreadHandler(void* userdata) { +void Subprocess::ThreadHandler(void* userdata) { Subprocess* subprocess = reinterpret_cast(userdata); adb_thread_setname(android::base::StringPrintf( @@ -475,8 +475,6 @@ void* Subprocess::ThreadHandler(void* userdata) { D("deleting Subprocess for PID %d", subprocess->pid()); delete subprocess; - - return nullptr; } void Subprocess::PassDataStreams() { diff --git a/adb/sysdeps.h b/adb/sysdeps.h index 761a4c799..f9f6f6905 100644 --- a/adb/sysdeps.h +++ b/adb/sysdeps.h @@ -115,25 +115,26 @@ static __inline__ void adb_mutex_unlock( adb_mutex_t* lock ) LeaveCriticalSection( lock ); } -typedef void* (*adb_thread_func_t)(void* arg); +typedef void (*adb_thread_func_t)(void* arg); typedef HANDLE adb_thread_t; -struct win_thread_args { +struct adb_winthread_args { adb_thread_func_t func; void* arg; }; -static unsigned __stdcall win_thread_wrapper(void* args) { - win_thread_args thread_args = *static_cast(args); - delete static_cast(args); - void* result = thread_args.func(thread_args.arg); - return reinterpret_cast(result); +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) { - win_thread_args* args = new win_thread_args{.func = func, .arg = arg}; - uintptr_t handle = _beginthreadex(nullptr, 0, win_thread_wrapper, args, 0, 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); @@ -168,6 +169,10 @@ static __inline__ bool adb_thread_detach(adb_thread_t thread) { return true; } +static __inline__ void __attribute__((noreturn)) adb_thread_exit() { + ExitThread(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 @@ -701,17 +706,32 @@ static __inline__ int adb_socket_accept(int serverfd, struct sockaddr* addr, #define unix_write adb_write #define unix_close adb_close -typedef void* (*adb_thread_func_t)( void* arg ); - +// 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); - errno = pthread_create(&temp, &attr, start, arg); + 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; @@ -731,6 +751,10 @@ static __inline__ bool adb_thread_detach(adb_thread_t 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 daceb89b9..360eaa7f9 100644 --- a/adb/sysdeps_test.cpp +++ b/adb/sysdeps_test.cpp @@ -20,10 +20,9 @@ #include "sysdeps.h" -static void* increment_atomic_int(void* c) { +static void increment_atomic_int(void* c) { sleep(1); reinterpret_cast*>(c)->fetch_add(1); - return nullptr; } TEST(sysdeps_thread, smoke) { @@ -57,3 +56,14 @@ TEST(sysdeps_thread, join) { 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)); +} diff --git a/adb/transport.cpp b/adb/transport.cpp index f8c8c615f..8ca1e494a 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -190,8 +190,7 @@ void send_packet(apacket *p, atransport *t) // // read_transport thread reads data from a transport (representing a usb/tcp connection), // and makes the main thread call handle_packet(). -static void *read_transport_thread(void *_t) -{ +static void read_transport_thread(void* _t) { atransport *t = reinterpret_cast(_t); apacket *p; @@ -244,13 +243,11 @@ oops: D("%s: read_transport thread is exiting", t->serial); kick_transport(t); transport_unref(t); - return 0; } // write_transport thread gets packets sent by the main thread (through send_packet()), // and writes to a transport (representing a usb/tcp connection). -static void *write_transport_thread(void *_t) -{ +static void write_transport_thread(void* _t) { atransport *t = reinterpret_cast(_t); apacket *p; int active = 0; @@ -295,7 +292,6 @@ static void *write_transport_thread(void *_t) D("%s: write_transport thread is exiting, fd %d", t->serial, t->fd); kick_transport(t); transport_unref(t); - return 0; } static void kick_transport_locked(atransport* t) { diff --git a/adb/transport_local.cpp b/adb/transport_local.cpp index d2a375a24..e6e699b6d 100644 --- a/adb/transport_local.cpp +++ b/adb/transport_local.cpp @@ -121,8 +121,7 @@ int local_connect_arbitrary_ports(int console_port, int adb_port, std::string* e } #if ADB_HOST -static void *client_socket_thread(void *x) -{ +static void client_socket_thread(void* x) { adb_thread_setname("client_socket_thread"); D("transport: client_socket_thread() starting"); while (true) { @@ -135,13 +134,11 @@ static void *client_socket_thread(void *x) } sleep(1); } - return 0; } #else // ADB_HOST -static void *server_socket_thread(void * arg) -{ +static void server_socket_thread(void* arg) { int serverfd, fd; sockaddr_storage ss; sockaddr *addrp = reinterpret_cast(&ss); @@ -174,7 +171,6 @@ static void *server_socket_thread(void * arg) } } D("transport: server_socket_thread() exiting"); - return 0; } /* This is relevant only for ADB daemon running inside the emulator. */ @@ -220,14 +216,13 @@ 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) -{ -/* 'accept' request to the adb QEMUD service. */ -static const char _accept_req[] = "accept"; -/* 'start' request to the adb QEMUD service. */ -static const char _start_req[] = "start"; -/* 'ok' reply from the adb QEMUD service. */ -static const char _ok_resp[] = "ok"; +static void qemu_socket_thread(void* arg) { + /* 'accept' request to the adb QEMUD service. */ + static const char _accept_req[] = "accept"; + /* 'start' request to the adb QEMUD service. */ + static const char _start_req[] = "start"; + /* 'ok' reply from the adb QEMUD service. */ + static const char _ok_resp[] = "ok"; const int port = (int) (uintptr_t) arg; int res, fd; @@ -247,7 +242,7 @@ static const char _ok_resp[] = "ok"; * 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); - return 0; + return; } for(;;) { @@ -275,21 +270,21 @@ static const char _ok_resp[] = "ok"; fd = qemu_pipe_open(con_name); if (fd < 0) { D("adb service become unavailable."); - return 0; + return; } } else { D("Unable to send the '%s' request to ADB service.", _accept_req); - return 0; + return; } } D("transport: qemu_socket_thread() exiting"); - return 0; + return; } #endif // !ADB_HOST void local_init(int port) { - void* (*func)(void *); + adb_thread_func_t func; const char* debug_name = ""; #if ADB_HOST diff --git a/adb/usb_linux.cpp b/adb/usb_linux.cpp index ed5d2d67e..500898a70 100644 --- a/adb/usb_linux.cpp +++ b/adb/usb_linux.cpp @@ -571,7 +571,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* unused) { +static void device_poll_thread(void*) { adb_thread_setname("device poll"); D("Created device thread"); while (true) { @@ -580,7 +580,6 @@ static void* device_poll_thread(void* unused) { kick_disconnected_devices(); sleep(1); } - return nullptr; } void usb_init() { diff --git a/adb/usb_linux_client.cpp b/adb/usb_linux_client.cpp index a4f1a7054..c863ed205 100644 --- a/adb/usb_linux_client.cpp +++ b/adb/usb_linux_client.cpp @@ -232,10 +232,7 @@ static const struct { }, }; - - -static void *usb_adb_open_thread(void *x) -{ +static void usb_adb_open_thread(void* x) { struct usb_handle *usb = (struct usb_handle *)x; int fd; @@ -270,7 +267,7 @@ static void *usb_adb_open_thread(void *x) } // never gets here - return 0; + abort(); } static int usb_adb_write(usb_handle *h, const void *data, int len) @@ -434,8 +431,7 @@ err: return; } -static void *usb_ffs_open_thread(void *x) -{ +static void usb_ffs_open_thread(void* x) { struct usb_handle *usb = (struct usb_handle *)x; adb_thread_setname("usb ffs open"); @@ -462,7 +458,7 @@ static void *usb_ffs_open_thread(void *x) } // never gets here - return 0; + abort(); } static int usb_ffs_write(usb_handle* h, const void* data, int len) { diff --git a/adb/usb_osx.cpp b/adb/usb_osx.cpp index 148be1d78..54d4c6c12 100644 --- a/adb/usb_osx.cpp +++ b/adb/usb_osx.cpp @@ -400,9 +400,7 @@ err_get_num_ep: return NULL; } - -void* RunLoopThread(void* unused) -{ +static void RunLoopThread(void* unused) { adb_thread_setname("RunLoop"); InitUSB(); @@ -420,7 +418,6 @@ void* RunLoopThread(void* unused) IONotificationPortDestroy(notificationPort); LOG(DEBUG) << "RunLoopThread done"; - return NULL; } static void usb_cleanup() { diff --git a/adb/usb_windows.cpp b/adb/usb_windows.cpp index e79008f48..8ecca3724 100644 --- a/adb/usb_windows.cpp +++ b/adb/usb_windows.cpp @@ -97,7 +97,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. -void* device_poll_thread(void* unused); +static void device_poll_thread(void*); /// Initializes this module void usb_init(); @@ -172,7 +172,7 @@ int register_new_device(usb_handle* handle) { return 1; } -void* device_poll_thread(void* unused) { +void device_poll_thread(void*) { adb_thread_setname("Device Poll"); D("Created device thread"); @@ -180,8 +180,6 @@ void* device_poll_thread(void* unused) { find_devices(); adb_sleep_ms(1000); } - - return NULL; } static LRESULT CALLBACK _power_window_proc(HWND hwnd, UINT uMsg, WPARAM wParam, @@ -203,7 +201,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* unused) { +static void _power_notification_thread(void*) { // 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 @@ -255,8 +253,6 @@ static void* _power_notification_thread(void* unused) { // shutting down. Not a big deal since the whole process will be going away // soon anyway. D("Power notification thread exiting"); - - return NULL; } void usb_init() { From 2674cd922e439e1d9e1db80e2dae5638a25f0390 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 12 Feb 2016 15:45:04 -0800 Subject: [PATCH 09/20] adb: sysdeps_win32: actually change ExitThread to _endthreadex. Forgot to amend this into b5fea14e. Change-Id: Id04e639eb87043901681db789d7a7925300fa867 (cherry picked from commit d7b3749202df90b49d4ab554944e0dd3564fe35a) --- adb/sysdeps.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adb/sysdeps.h b/adb/sysdeps.h index f9f6f6905..3bae09ea1 100644 --- a/adb/sysdeps.h +++ b/adb/sysdeps.h @@ -170,7 +170,7 @@ static __inline__ bool adb_thread_detach(adb_thread_t thread) { } static __inline__ void __attribute__((noreturn)) adb_thread_exit() { - ExitThread(0); + _endthreadex(0); } static __inline__ int adb_thread_setname(const std::string& name) { From 8b642e42b4a30bb4ffef1427da3660241a13b046 Mon Sep 17 00:00:00 2001 From: Dimitry Ivanov Date: Fri, 12 Feb 2016 18:40:29 -0800 Subject: [PATCH 10/20] Add missing liblog dependency Bug: http://b/27171986 Change-Id: Ia7f03c72de9a8e5d61896bde6b4b1af396376f54 (cherry picked from commit 01f21da7329ed88bff94e82bab1a889e9244cad9) --- adb/Android.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adb/Android.mk b/adb/Android.mk index baa498530..b792bd44b 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -158,7 +158,7 @@ LOCAL_SRC_FILES := \ LOCAL_SANITIZE := $(adb_target_sanitize) LOCAL_STATIC_LIBRARIES := libadbd -LOCAL_SHARED_LIBRARIES := libbase libcutils +LOCAL_SHARED_LIBRARIES := liblog libbase libcutils include $(BUILD_NATIVE_TEST) # libdiagnose_usb From 0b57ef735a0ae56d35fd034a2892dd2f3a92788d Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Thu, 18 Feb 2016 14:52:07 -0800 Subject: [PATCH 11/20] adb: fix clang-format for access modifier dedent. It was previously -1 (the default from Google style, since it uses 2 space indentation), instead of -2. Change-Id: I1865505ce17a2cc13b85de58bda55c3b1dfcf08c (cherry picked from commit 5da522ec4553ff512f9b237152c92354343ae6db) --- adb/.clang-format | 1 + 1 file changed, 1 insertion(+) diff --git a/adb/.clang-format b/adb/.clang-format index 673753525..fc4eb1bc0 100644 --- a/adb/.clang-format +++ b/adb/.clang-format @@ -2,6 +2,7 @@ BasedOnStyle: Google AllowShortBlocksOnASingleLine: false AllowShortFunctionsOnASingleLine: false +AccessModifierOffset: -2 ColumnLimit: 100 CommentPragmas: NOLINT:.* DerivePointerAlignment: false From addab3d84dccd01cae9ca7b8a4b274b935f18fd4 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 16 Feb 2016 17:34:53 -0800 Subject: [PATCH 12/20] adb: don't emulate fdevent or socketpair on Windows. Change-Id: I16cf7d4427eb79f36db39e91f85402a268fa72f5 (cherry picked from commit 3777d2ecc05d397ca501f4ee296e4e66568bb1bd) --- adb/Android.mk | 3 +- adb/adb_utils.cpp | 1 + adb/fdevent.cpp | 19 +- adb/sysdeps.h | 49 +- adb/sysdeps_test.cpp | 103 +++ adb/sysdeps_win32.cpp | 1484 ++++------------------------------------- 6 files changed, 272 insertions(+), 1387 deletions(-) diff --git a/adb/Android.mk b/adb/Android.mk index b792bd44b..7835d9f3a 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -50,6 +50,7 @@ LIBADB_SRC_FILES := \ adb_listeners.cpp \ adb_trace.cpp \ adb_utils.cpp \ + fdevent.cpp \ sockets.cpp \ transport.cpp \ transport_local.cpp \ @@ -75,12 +76,10 @@ LIBADB_windows_CFLAGS := \ $(ADB_COMMON_windows_CFLAGS) \ LIBADB_darwin_SRC_FILES := \ - fdevent.cpp \ get_my_path_darwin.cpp \ usb_osx.cpp \ LIBADB_linux_SRC_FILES := \ - fdevent.cpp \ get_my_path_linux.cpp \ usb_linux.cpp \ diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp index 8a16e51c0..26e376ce6 100644 --- a/adb/adb_utils.cpp +++ b/adb/adb_utils.cpp @@ -213,6 +213,7 @@ std::string perror_str(const char* msg) { } #if !defined(_WIN32) +// Windows version provided in sysdeps_win32.cpp bool set_file_block_mode(int fd, bool block) { int flags = fcntl(fd, F_GETFL, 0); if (flags == -1) { diff --git a/adb/fdevent.cpp b/adb/fdevent.cpp index 386f22186..461736422 100644 --- a/adb/fdevent.cpp +++ b/adb/fdevent.cpp @@ -21,10 +21,8 @@ #include "fdevent.h" #include -#include #include #include -#include #include #include @@ -54,7 +52,7 @@ int SHELL_EXIT_NOTIFY_FD = -1; struct PollNode { fdevent* fde; - ::pollfd pollfd; + adb_pollfd pollfd; PollNode(fdevent* fde) : fde(fde) { memset(&pollfd, 0, sizeof(pollfd)); @@ -73,17 +71,17 @@ struct PollNode { static auto& g_poll_node_map = *new std::unordered_map(); static auto& g_pending_list = *new std::list(); static bool main_thread_valid; -static pthread_t main_thread; +static unsigned long main_thread_id; static void check_main_thread() { if (main_thread_valid) { - CHECK_NE(0, pthread_equal(main_thread, pthread_self())); + CHECK_EQ(main_thread_id, adb_thread_id()); } } static void set_main_thread() { main_thread_valid = true; - main_thread = pthread_self(); + main_thread_id = adb_thread_id(); } static std::string dump_fde(const fdevent* fde) { @@ -217,7 +215,7 @@ void fdevent_del(fdevent* fde, unsigned events) { fdevent_set(fde, (fde->state & FDE_EVENTMASK) & ~events); } -static std::string dump_pollfds(const std::vector& pollfds) { +static std::string dump_pollfds(const std::vector& pollfds) { std::string result; for (const auto& pollfd : pollfds) { std::string op; @@ -233,13 +231,13 @@ static std::string dump_pollfds(const std::vector& pollfds) { } static void fdevent_process() { - std::vector pollfds; + std::vector pollfds; for (const auto& pair : g_poll_node_map) { pollfds.push_back(pair.second.pollfd); } CHECK_GT(pollfds.size(), 0u); D("poll(), pollfds = %s", dump_pollfds(pollfds).c_str()); - int ret = TEMP_FAILURE_RETRY(poll(&pollfds[0], pollfds.size(), -1)); + int ret = adb_poll(&pollfds[0], pollfds.size(), -1); if (ret == -1) { PLOG(ERROR) << "poll(), ret = " << ret; return; @@ -289,6 +287,9 @@ static void fdevent_call_fdfunc(fdevent* fde) } #if !ADB_HOST + +#include + static void fdevent_subproc_event_func(int fd, unsigned ev, void* /* userdata */) { diff --git a/adb/sysdeps.h b/adb/sysdeps.h index 3bae09ea1..7af2979ec 100644 --- a/adb/sysdeps.h +++ b/adb/sysdeps.h @@ -180,6 +180,14 @@ 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(); @@ -263,24 +271,6 @@ int unix_isatty(int fd); /* normally provided by */ extern void* load_file(const char* pathname, unsigned* psize); -/* normally provided by "fdevent.h" */ - -#define FDE_READ 0x0001 -#define FDE_WRITE 0x0002 -#define FDE_ERROR 0x0004 -#define FDE_DONT_CLOSE 0x0080 - -typedef void (*fd_func)(int fd, unsigned events, void *userdata); - -fdevent *fdevent_create(int fd, fd_func func, void *arg); -void fdevent_destroy(fdevent *fde); -void fdevent_install(fdevent *fde, int fd, fd_func func, void *arg); -void fdevent_remove(fdevent *item); -void fdevent_set(fdevent *fde, unsigned events); -void fdevent_add(fdevent *fde, unsigned events); -void fdevent_del(fdevent *fde, unsigned events); -void fdevent_loop(); - static __inline__ void adb_sleep_ms( int mseconds ) { Sleep( mseconds ); @@ -304,6 +294,14 @@ extern int adb_setsockopt(int fd, int level, int optname, const void* optva extern int adb_socketpair( int sv[2] ); +struct adb_pollfd { + int fd; + short events; + short revents; +}; +extern int adb_poll(adb_pollfd* fds, size_t nfds, int timeout); +#define poll ___xxx_poll + static __inline__ int adb_is_absolute_host_path(const char* path) { return isalpha(path[0]) && path[1] == ':' && path[2] == '\\'; } @@ -456,14 +454,14 @@ size_t ParseCompleteUTF8(const char* first, const char* last, std::vector* #else /* !_WIN32 a.k.a. Unix */ -#include "fdevent.h" #include #include #include -#include -#include -#include #include +#include +#include +#include +#include #include #include @@ -803,6 +801,13 @@ static __inline__ int adb_socketpair( int sv[2] ) #undef socketpair #define socketpair ___xxx_socketpair +typedef struct pollfd adb_pollfd; +static __inline__ int adb_poll(adb_pollfd* fds, size_t nfds, int timeout) { + return TEMP_FAILURE_RETRY(poll(fds, nfds, timeout)); +} + +#define poll ___xxx_poll + static __inline__ void adb_sleep_ms( int mseconds ) { usleep( mseconds*1000 ); diff --git a/adb/sysdeps_test.cpp b/adb/sysdeps_test.cpp index 360eaa7f9..19856dcba 100644 --- a/adb/sysdeps_test.cpp +++ b/adb/sysdeps_test.cpp @@ -18,6 +18,7 @@ #include #include +#include "adb_io.h" #include "sysdeps.h" static void increment_atomic_int(void* c) { @@ -67,3 +68,105 @@ TEST(sysdeps_thread, exit) { nullptr, &thread)); ASSERT_TRUE(adb_thread_join(thread)); } + +TEST(sysdeps_socketpair, smoke) { + int fds[2]; + ASSERT_EQ(0, adb_socketpair(fds)) << strerror(errno); + ASSERT_TRUE(WriteFdExactly(fds[0], "foo", 4)); + ASSERT_TRUE(WriteFdExactly(fds[1], "bar", 4)); + + char buf[4]; + ASSERT_TRUE(ReadFdExactly(fds[1], buf, 4)); + ASSERT_STREQ(buf, "foo"); + ASSERT_TRUE(ReadFdExactly(fds[0], buf, 4)); + ASSERT_STREQ(buf, "bar"); + ASSERT_EQ(0, adb_close(fds[0])); + ASSERT_EQ(0, adb_close(fds[1])); +} + +class sysdeps_poll : public ::testing::Test { + protected: + int fds[2]; + void SetUp() override { + ASSERT_EQ(0, adb_socketpair(fds)) << strerror(errno); + } + + void TearDown() override { + ASSERT_EQ(0, adb_close(fds[0])); + ASSERT_EQ(0, adb_close(fds[1])); + } +}; + +TEST_F(sysdeps_poll, smoke) { + adb_pollfd pfd[2]; + pfd[0].fd = fds[0]; + pfd[0].events = POLLRDNORM; + pfd[1].fd = fds[1]; + pfd[1].events = POLLWRNORM; + + EXPECT_EQ(1, adb_poll(pfd, 2, 0)); + EXPECT_EQ(0, pfd[0].revents); + EXPECT_EQ(POLLWRNORM, pfd[1].revents); + + ASSERT_TRUE(WriteFdExactly(fds[1], "foo", 4)); + + // Wait for the socketpair to be flushed. + EXPECT_EQ(1, adb_poll(pfd, 1, 100)); + EXPECT_EQ(POLLRDNORM, pfd[0].revents); + + EXPECT_EQ(2, adb_poll(pfd, 2, 0)); + EXPECT_EQ(POLLRDNORM, pfd[0].revents); + EXPECT_EQ(POLLWRNORM, pfd[1].revents); +} + +TEST_F(sysdeps_poll, timeout) { + adb_pollfd pfd; + pfd.fd = fds[0]; + pfd.events = POLLRDNORM; + + EXPECT_EQ(0, adb_poll(&pfd, 1, 100)); + EXPECT_EQ(0, pfd.revents); + + ASSERT_TRUE(WriteFdExactly(fds[1], "foo", 4)); + + EXPECT_EQ(1, adb_poll(&pfd, 1, 100)); + EXPECT_EQ(POLLRDNORM, pfd.revents); +} + +TEST_F(sysdeps_poll, invalid_fd) { + adb_pollfd pfd[3]; + pfd[0].fd = fds[0]; + pfd[0].events = POLLRDNORM; + pfd[1].fd = INT_MAX; + pfd[1].events = POLLRDNORM; + pfd[2].fd = fds[1]; + pfd[2].events = POLLWRNORM; + + ASSERT_TRUE(WriteFdExactly(fds[1], "foo", 4)); + + // Wait for the socketpair to be flushed. + EXPECT_EQ(1, adb_poll(pfd, 1, 100)); + EXPECT_EQ(POLLRDNORM, pfd[0].revents); + + EXPECT_EQ(3, adb_poll(pfd, 3, 0)); + EXPECT_EQ(POLLRDNORM, pfd[0].revents); + EXPECT_EQ(POLLNVAL, pfd[1].revents); + EXPECT_EQ(POLLWRNORM, pfd[2].revents); +} + +TEST_F(sysdeps_poll, duplicate_fd) { + adb_pollfd pfd[2]; + pfd[0].fd = fds[0]; + pfd[0].events = POLLRDNORM; + pfd[1] = pfd[0]; + + EXPECT_EQ(0, adb_poll(pfd, 2, 0)); + EXPECT_EQ(0, pfd[0].revents); + EXPECT_EQ(0, pfd[1].revents); + + ASSERT_TRUE(WriteFdExactly(fds[1], "foo", 4)); + + EXPECT_EQ(2, adb_poll(pfd, 2, 100)); + EXPECT_EQ(POLLRDNORM, pfd[0].revents); + EXPECT_EQ(POLLRDNORM, pfd[1].revents); +} diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp index 0b0898186..0dbfb9847 100644 --- a/adb/sysdeps_win32.cpp +++ b/adb/sysdeps_win32.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include @@ -39,6 +40,7 @@ #include #include "adb.h" +#include "adb_utils.h" extern void fatal(const char *fmt, ...); @@ -54,7 +56,6 @@ typedef struct FHClassRec_ { int (*_fh_lseek)(FH, int, int); int (*_fh_read)(FH, void*, int); int (*_fh_write)(FH, const void*, int); - void (*_fh_hook)(FH, int, EventHook); } FHClassRec; static void _fh_file_init(FH); @@ -62,7 +63,6 @@ static int _fh_file_close(FH); static int _fh_file_lseek(FH, int, int); static int _fh_file_read(FH, void*, int); static int _fh_file_write(FH, const void*, int); -static void _fh_file_hook(FH, int, EventHook); static const FHClassRec _fh_file_class = { _fh_file_init, @@ -70,7 +70,6 @@ static const FHClassRec _fh_file_class = { _fh_file_lseek, _fh_file_read, _fh_file_write, - _fh_file_hook }; static void _fh_socket_init(FH); @@ -78,7 +77,6 @@ static int _fh_socket_close(FH); static int _fh_socket_lseek(FH, int, int); static int _fh_socket_read(FH, void*, int); static int _fh_socket_write(FH, const void*, int); -static void _fh_socket_hook(FH, int, EventHook); static const FHClassRec _fh_socket_class = { _fh_socket_init, @@ -86,7 +84,6 @@ static const FHClassRec _fh_socket_class = { _fh_socket_lseek, _fh_socket_read, _fh_socket_write, - _fh_socket_hook }; #define assert(cond) \ @@ -174,9 +171,6 @@ void *load_file(const char *fn, unsigned *_sz) /**************************************************************************/ /**************************************************************************/ -/* used to emulate unix-domain socket pairs */ -typedef struct SocketPairRec_* SocketPair; - typedef struct FHRec_ { FHClass clazz; @@ -185,10 +179,8 @@ typedef struct FHRec_ union { HANDLE handle; SOCKET socket; - SocketPair pair; } u; - HANDLE event; int mask; char name[32]; @@ -197,7 +189,6 @@ typedef struct FHRec_ #define fh_handle u.handle #define fh_socket u.socket -#define fh_pair u.pair #define WIN32_FH_BASE 100 @@ -672,19 +663,56 @@ static void _socket_set_errno( const DWORD err ) { } } -static void _fh_socket_init( FH f ) { - f->fh_socket = INVALID_SOCKET; - f->event = WSACreateEvent(); - if (f->event == WSA_INVALID_EVENT) { - D("WSACreateEvent failed: %s", - android::base::SystemErrorCodeToString(WSAGetLastError()).c_str()); - - // _event_socket_start assumes that this field is INVALID_HANDLE_VALUE - // on failure, instead of NULL which is what Windows really returns on - // error. It might be better to change all the other code to look for - // NULL, but that is a much riskier change. - f->event = INVALID_HANDLE_VALUE; +extern int adb_poll(adb_pollfd* fds, size_t nfds, int timeout) { + // WSAPoll doesn't handle invalid/non-socket handles, so we need to handle them ourselves. + int skipped = 0; + std::vector sockets; + std::vector original; + for (size_t i = 0; i < nfds; ++i) { + FH fh = _fh_from_int(fds[i].fd, __func__); + if (!fh || !fh->used || fh->clazz != &_fh_socket_class) { + D("adb_poll received bad FD %d", fds[i].fd); + fds[i].revents = POLLNVAL; + ++skipped; + } else { + WSAPOLLFD wsapollfd = { + .fd = fh->u.socket, + .events = static_cast(fds[i].events) + }; + sockets.push_back(wsapollfd); + original.push_back(&fds[i]); + } } + + if (sockets.empty()) { + return skipped; + } + + int result = WSAPoll(sockets.data(), sockets.size(), timeout); + if (result == SOCKET_ERROR) { + _socket_set_errno(WSAGetLastError()); + return -1; + } + + // Map the results back onto the original set. + for (size_t i = 0; i < sockets.size(); ++i) { + original[i]->revents = sockets[i].revents; + } + + // WSAPoll appears to return the number of unique FDs with avaiable events, instead of how many + // of the pollfd elements have a non-zero revents field, which is what it and poll are specified + // to do. Ignore its result and calculate the proper return value. + result = 0; + for (size_t i = 0; i < nfds; ++i) { + if (fds[i].revents != 0) { + ++result; + } + } + return result; +} + +static void _fh_socket_init(FH f) { + f->fh_socket = INVALID_SOCKET; f->mask = 0; } @@ -705,13 +733,6 @@ static int _fh_socket_close( FH f ) { } f->fh_socket = INVALID_SOCKET; } - if (f->event != NULL) { - if (!CloseHandle(f->event)) { - D("CloseHandle failed: %s", - android::base::SystemErrorCodeToString(GetLastError()).c_str()); - } - f->event = NULL; - } f->mask = 0; return 0; } @@ -1083,6 +1104,25 @@ int adb_setsockopt( int fd, int level, int optname, const void* optval, soc return result; } +int adb_getsockname(int fd, struct sockaddr* sockaddr, socklen_t* optlen) { + FH fh = _fh_from_int(fd, __func__); + + if (!fh || fh->clazz != &_fh_socket_class) { + D("adb_getsockname: invalid fd %d", fd); + errno = EBADF; + return -1; + } + + int result = getsockname(fh->fh_socket, sockaddr, optlen); + if (result == SOCKET_ERROR) { + const DWORD err = WSAGetLastError(); + D("adb_getsockname: setsockopt on fd %d failed: %s\n", fd, + android::base::SystemErrorCodeToString(err).c_str()); + _socket_set_errno(err); + result = -1; + } + return result; +} int adb_shutdown(int fd) { @@ -1105,1352 +1145,88 @@ int adb_shutdown(int fd) return 0; } -/**************************************************************************/ -/**************************************************************************/ -/***** *****/ -/***** emulated socketpairs *****/ -/***** *****/ -/**************************************************************************/ -/**************************************************************************/ +// Emulate socketpair(2) by binding and connecting to a socket. +int adb_socketpair(int sv[2]) { + int server = -1; + int client = -1; + int accepted = -1; + sockaddr_storage addr_storage; + socklen_t addr_len = sizeof(addr_storage); + sockaddr_in* addr = nullptr; + std::string error; -/* we implement socketpairs directly in use space for the following reasons: - * - it avoids copying data from/to the Nt kernel - * - it allows us to implement fdevent hooks easily and cheaply, something - * that is not possible with standard Win32 pipes !! - * - * basically, we use two circular buffers, each one corresponding to a given - * direction. - * - * each buffer is implemented as two regions: - * - * region A which is (a_start,a_end) - * region B which is (0, b_end) with b_end <= a_start - * - * an empty buffer has: a_start = a_end = b_end = 0 - * - * a_start is the pointer where we start reading data - * a_end is the pointer where we start writing data, unless it is BUFFER_SIZE, - * then you start writing at b_end - * - * the buffer is full when b_end == a_start && a_end == BUFFER_SIZE - * - * there is room when b_end < a_start || a_end < BUFER_SIZE - * - * when reading, a_start is incremented, it a_start meets a_end, then - * we do: a_start = 0, a_end = b_end, b_end = 0, and keep going on.. - */ - -#define BIP_BUFFER_SIZE 4096 - -#if 0 -#include -# define BIPD(x) D x -# define BIPDUMP bip_dump_hex - -static void bip_dump_hex( const unsigned char* ptr, size_t len ) -{ - int nn, len2 = len; - - if (len2 > 8) len2 = 8; - - for (nn = 0; nn < len2; nn++) - printf("%02x", ptr[nn]); - printf(" "); - - for (nn = 0; nn < len2; nn++) { - int c = ptr[nn]; - if (c < 32 || c > 127) - c = '.'; - printf("%c", c); - } - printf("\n"); - fflush(stdout); -} - -#else -# define BIPD(x) do {} while (0) -# define BIPDUMP(p,l) BIPD(p) -#endif - -typedef struct BipBufferRec_ -{ - int a_start; - int a_end; - int b_end; - int fdin; - int fdout; - int closed; - int can_write; /* boolean */ - HANDLE evt_write; /* event signaled when one can write to a buffer */ - int can_read; /* boolean */ - HANDLE evt_read; /* event signaled when one can read from a buffer */ - CRITICAL_SECTION lock; - unsigned char buff[ BIP_BUFFER_SIZE ]; - -} BipBufferRec, *BipBuffer; - -static void -bip_buffer_init( BipBuffer buffer ) -{ - D( "bit_buffer_init %p", buffer ); - buffer->a_start = 0; - buffer->a_end = 0; - buffer->b_end = 0; - buffer->can_write = 1; - buffer->can_read = 0; - buffer->fdin = 0; - buffer->fdout = 0; - buffer->closed = 0; - buffer->evt_write = CreateEvent( NULL, TRUE, TRUE, NULL ); - buffer->evt_read = CreateEvent( NULL, TRUE, FALSE, NULL ); - InitializeCriticalSection( &buffer->lock ); -} - -static void -bip_buffer_close( BipBuffer bip ) -{ - bip->closed = 1; - - if (!bip->can_read) { - SetEvent( bip->evt_read ); - } - if (!bip->can_write) { - SetEvent( bip->evt_write ); - } -} - -static void -bip_buffer_done( BipBuffer bip ) -{ - BIPD(( "bip_buffer_done: %d->%d", bip->fdin, bip->fdout )); - CloseHandle( bip->evt_read ); - CloseHandle( bip->evt_write ); - DeleteCriticalSection( &bip->lock ); -} - -static int -bip_buffer_write( BipBuffer bip, const void* src, int len ) -{ - int avail, count = 0; - - if (len <= 0) - return 0; - - BIPD(( "bip_buffer_write: enter %d->%d len %d", bip->fdin, bip->fdout, len )); - BIPDUMP( src, len ); - - if (bip->closed) { - errno = EPIPE; - return -1; + server = network_loopback_server(0, SOCK_STREAM, &error); + if (server < 0) { + D("adb_socketpair: failed to create server: %s", error.c_str()); + goto fail; } - EnterCriticalSection( &bip->lock ); - - while (!bip->can_write) { - int ret; - LeaveCriticalSection( &bip->lock ); - - if (bip->closed) { - errno = EPIPE; - return -1; - } - /* spinlocking here is probably unfair, but let's live with it */ - ret = WaitForSingleObject( bip->evt_write, INFINITE ); - if (ret != WAIT_OBJECT_0) { /* buffer probably closed */ - D( "bip_buffer_write: error %d->%d WaitForSingleObject returned %d, error %ld", bip->fdin, bip->fdout, ret, GetLastError() ); - return 0; - } - if (bip->closed) { - errno = EPIPE; - return -1; - } - EnterCriticalSection( &bip->lock ); + if (adb_getsockname(server, reinterpret_cast(&addr_storage), &addr_len) < 0) { + D("adb_socketpair: adb_getsockname failed: %s", strerror(errno)); + goto fail; } - BIPD(( "bip_buffer_write: exec %d->%d len %d", bip->fdin, bip->fdout, len )); - - avail = BIP_BUFFER_SIZE - bip->a_end; - if (avail > 0) - { - /* we can append to region A */ - if (avail > len) - avail = len; - - memcpy( bip->buff + bip->a_end, src, avail ); - src = (const char *)src + avail; - count += avail; - len -= avail; - - bip->a_end += avail; - if (bip->a_end == BIP_BUFFER_SIZE && bip->a_start == 0) { - bip->can_write = 0; - ResetEvent( bip->evt_write ); - goto Exit; - } + if (addr_storage.ss_family != AF_INET) { + D("adb_socketpair: unknown address family received: %d", addr_storage.ss_family); + errno = ECONNABORTED; + goto fail; } - if (len == 0) - goto Exit; - - avail = bip->a_start - bip->b_end; - assert( avail > 0 ); /* since can_write is TRUE */ - - if (avail > len) - avail = len; - - memcpy( bip->buff + bip->b_end, src, avail ); - count += avail; - bip->b_end += avail; - - if (bip->b_end == bip->a_start) { - bip->can_write = 0; - ResetEvent( bip->evt_write ); + addr = reinterpret_cast(&addr_storage); + D("adb_socketpair: bound on port %d", ntohs(addr->sin_port)); + client = network_loopback_client(ntohs(addr->sin_port), SOCK_STREAM, &error); + if (client < 0) { + D("adb_socketpair: failed to connect client: %s", error.c_str()); + goto fail; } -Exit: - assert( count > 0 ); - - if ( !bip->can_read ) { - bip->can_read = 1; - SetEvent( bip->evt_read ); - } - - BIPD(( "bip_buffer_write: exit %d->%d count %d (as=%d ae=%d be=%d cw=%d cr=%d", - bip->fdin, bip->fdout, count, bip->a_start, bip->a_end, bip->b_end, bip->can_write, bip->can_read )); - LeaveCriticalSection( &bip->lock ); - - return count; - } - -static int -bip_buffer_read( BipBuffer bip, void* dst, int len ) -{ - int avail, count = 0; - - if (len <= 0) - return 0; - - BIPD(( "bip_buffer_read: enter %d->%d len %d", bip->fdin, bip->fdout, len )); - - EnterCriticalSection( &bip->lock ); - while ( !bip->can_read ) - { -#if 0 - LeaveCriticalSection( &bip->lock ); - errno = EAGAIN; - return -1; -#else - int ret; - LeaveCriticalSection( &bip->lock ); - - if (bip->closed) { - errno = EPIPE; - return -1; - } - - ret = WaitForSingleObject( bip->evt_read, INFINITE ); - if (ret != WAIT_OBJECT_0) { /* probably closed buffer */ - D( "bip_buffer_read: error %d->%d WaitForSingleObject returned %d, error %ld", bip->fdin, bip->fdout, ret, GetLastError()); - return 0; - } - if (bip->closed) { - errno = EPIPE; - return -1; - } - EnterCriticalSection( &bip->lock ); -#endif - } - - BIPD(( "bip_buffer_read: exec %d->%d len %d", bip->fdin, bip->fdout, len )); - - avail = bip->a_end - bip->a_start; - assert( avail > 0 ); /* since can_read is TRUE */ - - if (avail > len) - avail = len; - - memcpy( dst, bip->buff + bip->a_start, avail ); - dst = (char *)dst + avail; - count += avail; - len -= avail; - - bip->a_start += avail; - if (bip->a_start < bip->a_end) - goto Exit; - - bip->a_start = 0; - bip->a_end = bip->b_end; - bip->b_end = 0; - - avail = bip->a_end; - if (avail > 0) { - if (avail > len) - avail = len; - memcpy( dst, bip->buff, avail ); - count += avail; - bip->a_start += avail; - - if ( bip->a_start < bip->a_end ) - goto Exit; - - bip->a_start = bip->a_end = 0; - } - - bip->can_read = 0; - ResetEvent( bip->evt_read ); - -Exit: - assert( count > 0 ); - - if (!bip->can_write ) { - bip->can_write = 1; - SetEvent( bip->evt_write ); - } - - BIPDUMP( (const unsigned char*)dst - count, count ); - BIPD(( "bip_buffer_read: exit %d->%d count %d (as=%d ae=%d be=%d cw=%d cr=%d", - bip->fdin, bip->fdout, count, bip->a_start, bip->a_end, bip->b_end, bip->can_write, bip->can_read )); - LeaveCriticalSection( &bip->lock ); - - return count; -} - -typedef struct SocketPairRec_ -{ - BipBufferRec a2b_bip; - BipBufferRec b2a_bip; - FH a_fd; - int used; - -} SocketPairRec; - -void _fh_socketpair_init( FH f ) -{ - f->fh_pair = NULL; -} - -static int -_fh_socketpair_close( FH f ) -{ - if ( f->fh_pair ) { - SocketPair pair = f->fh_pair; - - if ( f == pair->a_fd ) { - pair->a_fd = NULL; - } - - bip_buffer_close( &pair->b2a_bip ); - bip_buffer_close( &pair->a2b_bip ); - - if ( --pair->used == 0 ) { - bip_buffer_done( &pair->b2a_bip ); - bip_buffer_done( &pair->a2b_bip ); - free( pair ); - } - f->fh_pair = NULL; + accepted = adb_socket_accept(server, nullptr, nullptr); + if (accepted < 0) { + const DWORD err = WSAGetLastError(); + D("adb_socketpair: failed to accept: %s", + android::base::SystemErrorCodeToString(err).c_str()); + _socket_set_errno(err); + goto fail; } + adb_close(server); + sv[0] = client; + sv[1] = accepted; return 0; -} -static int -_fh_socketpair_lseek( FH f, int pos, int origin ) -{ - errno = ESPIPE; +fail: + if (server >= 0) { + adb_close(server); + } + if (client >= 0) { + adb_close(client); + } + if (accepted >= 0) { + adb_close(accepted); + } return -1; } -static int -_fh_socketpair_read( FH f, void* buf, int len ) -{ - SocketPair pair = f->fh_pair; - BipBuffer bip; +bool set_file_block_mode(int fd, bool block) { + FH fh = _fh_from_int(fd, __func__); - if (!pair) - return -1; - - if ( f == pair->a_fd ) - bip = &pair->b2a_bip; - else - bip = &pair->a2b_bip; - - return bip_buffer_read( bip, buf, len ); -} - -static int -_fh_socketpair_write( FH f, const void* buf, int len ) -{ - SocketPair pair = f->fh_pair; - BipBuffer bip; - - if (!pair) - return -1; - - if ( f == pair->a_fd ) - bip = &pair->a2b_bip; - else - bip = &pair->b2a_bip; - - return bip_buffer_write( bip, buf, len ); -} - - -static void _fh_socketpair_hook( FH f, int event, EventHook hook ); /* forward */ - -static const FHClassRec _fh_socketpair_class = -{ - _fh_socketpair_init, - _fh_socketpair_close, - _fh_socketpair_lseek, - _fh_socketpair_read, - _fh_socketpair_write, - _fh_socketpair_hook -}; - - -int adb_socketpair(int sv[2]) { - SocketPair pair; - - unique_fh fa(_fh_alloc(&_fh_socketpair_class)); - if (!fa) { - return -1; - } - unique_fh fb(_fh_alloc(&_fh_socketpair_class)); - if (!fb) { - return -1; + if (!fh || !fh->used) { + errno = EBADF; + return false; } - pair = reinterpret_cast(malloc(sizeof(*pair))); - if (pair == NULL) { - D("adb_socketpair: not enough memory to allocate pipes" ); - return -1; - } - - bip_buffer_init( &pair->a2b_bip ); - bip_buffer_init( &pair->b2a_bip ); - - fa->fh_pair = pair; - fb->fh_pair = pair; - pair->used = 2; - pair->a_fd = fa.get(); - - sv[0] = _fh_to_int(fa.get()); - sv[1] = _fh_to_int(fb.get()); - - pair->a2b_bip.fdin = sv[0]; - pair->a2b_bip.fdout = sv[1]; - pair->b2a_bip.fdin = sv[1]; - pair->b2a_bip.fdout = sv[0]; - - snprintf( fa->name, sizeof(fa->name), "%d(pair:%d)", sv[0], sv[1] ); - snprintf( fb->name, sizeof(fb->name), "%d(pair:%d)", sv[1], sv[0] ); - D( "adb_socketpair: returns (%d, %d)", sv[0], sv[1] ); - fa.release(); - fb.release(); - return 0; -} - -/**************************************************************************/ -/**************************************************************************/ -/***** *****/ -/***** fdevents emulation *****/ -/***** *****/ -/***** this is a very simple implementation, we rely on the fact *****/ -/***** that ADB doesn't use FDE_ERROR. *****/ -/***** *****/ -/**************************************************************************/ -/**************************************************************************/ - -#define FATAL(fmt, ...) fatal("%s: " fmt, __FUNCTION__, ##__VA_ARGS__) - -#if DEBUG -static void dump_fde(fdevent *fde, const char *info) -{ - fprintf(stderr,"FDE #%03d %c%c%c %s\n", fde->fd, - fde->state & FDE_READ ? 'R' : ' ', - fde->state & FDE_WRITE ? 'W' : ' ', - fde->state & FDE_ERROR ? 'E' : ' ', - info); -} -#else -#define dump_fde(fde, info) do { } while(0) -#endif - -#define FDE_EVENTMASK 0x00ff -#define FDE_STATEMASK 0xff00 - -#define FDE_ACTIVE 0x0100 -#define FDE_PENDING 0x0200 -#define FDE_CREATED 0x0400 - -static void fdevent_plist_enqueue(fdevent *node); -static void fdevent_plist_remove(fdevent *node); -static fdevent *fdevent_plist_dequeue(void); - -static fdevent list_pending = { - .next = &list_pending, - .prev = &list_pending, -}; - -static fdevent **fd_table = 0; -static int fd_table_max = 0; - -typedef struct EventLooperRec_* EventLooper; - -typedef struct EventHookRec_ -{ - EventHook next; - FH fh; - HANDLE h; - int wanted; /* wanted event flags */ - int ready; /* ready event flags */ - void* aux; - void (*prepare)( EventHook hook ); - int (*start) ( EventHook hook ); - void (*stop) ( EventHook hook ); - int (*check) ( EventHook hook ); - int (*peek) ( EventHook hook ); -} EventHookRec; - -static EventHook _free_hooks; - -static EventHook -event_hook_alloc(FH fh) { - EventHook hook = _free_hooks; - if (hook != NULL) { - _free_hooks = hook->next; + if (fh->clazz == &_fh_socket_class) { + u_long x = !block; + if (ioctlsocket(fh->u.socket, FIONBIO, &x) != 0) { + _socket_set_errno(WSAGetLastError()); + return false; + } + return true; } else { - hook = reinterpret_cast(malloc(sizeof(*hook))); - if (hook == NULL) - fatal( "could not allocate event hook\n" ); - } - hook->next = NULL; - hook->fh = fh; - hook->wanted = 0; - hook->ready = 0; - hook->h = INVALID_HANDLE_VALUE; - hook->aux = NULL; - - hook->prepare = NULL; - hook->start = NULL; - hook->stop = NULL; - hook->check = NULL; - hook->peek = NULL; - - return hook; -} - -static void -event_hook_free( EventHook hook ) -{ - hook->fh = NULL; - hook->wanted = 0; - hook->ready = 0; - hook->next = _free_hooks; - _free_hooks = hook; -} - - -static void -event_hook_signal( EventHook hook ) -{ - FH f = hook->fh; - int fd = _fh_to_int(f); - fdevent* fde = fd_table[ fd - WIN32_FH_BASE ]; - - if (fde != NULL && fde->fd == fd) { - if ((fde->state & FDE_PENDING) == 0) { - fde->state |= FDE_PENDING; - fdevent_plist_enqueue( fde ); - } - fde->events |= hook->wanted; + errno = ENOTSOCK; + return false; } } - -#define MAX_LOOPER_HANDLES WIN32_MAX_FHS - -typedef struct EventLooperRec_ -{ - EventHook hooks; - HANDLE htab[ MAX_LOOPER_HANDLES ]; - int htab_count; - -} EventLooperRec; - -static EventHook* -event_looper_find_p( EventLooper looper, FH fh ) -{ - EventHook *pnode = &looper->hooks; - EventHook node = *pnode; - for (;;) { - if ( node == NULL || node->fh == fh ) - break; - pnode = &node->next; - node = *pnode; - } - return pnode; -} - -static void -event_looper_hook( EventLooper looper, int fd, int events ) -{ - FH f = _fh_from_int(fd, __func__); - EventHook *pnode; - EventHook node; - - if (f == NULL) /* invalid arg */ { - D("event_looper_hook: invalid fd=%d", fd); - return; - } - - pnode = event_looper_find_p( looper, f ); - node = *pnode; - if ( node == NULL ) { - node = event_hook_alloc( f ); - node->next = *pnode; - *pnode = node; - } - - if ( (node->wanted & events) != events ) { - /* this should update start/stop/check/peek */ - D("event_looper_hook: call hook for %d (new=%x, old=%x)", - fd, node->wanted, events); - f->clazz->_fh_hook( f, events & ~node->wanted, node ); - node->wanted |= events; - } else { - D("event_looper_hook: ignoring events %x for %d wanted=%x)", - events, fd, node->wanted); - } -} - -static void -event_looper_unhook( EventLooper looper, int fd, int events ) -{ - FH fh = _fh_from_int(fd, __func__); - EventHook *pnode = event_looper_find_p( looper, fh ); - EventHook node = *pnode; - - if (node != NULL) { - int events2 = events & node->wanted; - if ( events2 == 0 ) { - D( "event_looper_unhook: events %x not registered for fd %d", events, fd ); - return; - } - node->wanted &= ~events2; - if (!node->wanted) { - *pnode = node->next; - event_hook_free( node ); - } - } -} - -/* - * A fixer for WaitForMultipleObjects on condition that there are more than 64 - * handles to wait on. - * - * In cetain cases DDMS may establish more than 64 connections with ADB. For - * instance, this may happen if there are more than 64 processes running on a - * device, or there are multiple devices connected (including the emulator) with - * the combined number of running processes greater than 64. In this case using - * WaitForMultipleObjects to wait on connection events simply wouldn't cut, - * because of the API limitations (64 handles max). So, we need to provide a way - * to scale WaitForMultipleObjects to accept an arbitrary number of handles. The - * easiest (and "Microsoft recommended") way to do that would be dividing the - * handle array into chunks with the chunk size less than 64, and fire up as many - * waiting threads as there are chunks. Then each thread would wait on a chunk of - * handles, and will report back to the caller which handle has been set. - * Here is the implementation of that algorithm. - */ - -/* Number of handles to wait on in each wating thread. */ -#define WAIT_ALL_CHUNK_SIZE 63 - -/* Descriptor for a wating thread */ -typedef struct WaitForAllParam { - /* A handle to an event to signal when waiting is over. This handle is shared - * accross all the waiting threads, so each waiting thread knows when any - * other thread has exited, so it can exit too. */ - HANDLE main_event; - /* Upon exit from a waiting thread contains the index of the handle that has - * been signaled. The index is an absolute index of the signaled handle in - * the original array. This pointer is shared accross all the waiting threads - * and it's not guaranteed (due to a race condition) that when all the - * waiting threads exit, the value contained here would indicate the first - * handle that was signaled. This is fine, because the caller cares only - * about any handle being signaled. It doesn't care about the order, nor - * about the whole list of handles that were signaled. */ - LONG volatile *signaled_index; - /* Array of handles to wait on in a waiting thread. */ - HANDLE* handles; - /* Number of handles in 'handles' array to wait on. */ - int handles_count; - /* Index inside the main array of the first handle in the 'handles' array. */ - int first_handle_index; - /* Waiting thread handle. */ - HANDLE thread; -} WaitForAllParam; - -/* Waiting thread routine. */ -static unsigned __stdcall -_in_waiter_thread(void* arg) -{ - HANDLE wait_on[WAIT_ALL_CHUNK_SIZE + 1]; - int res; - WaitForAllParam* const param = (WaitForAllParam*)arg; - - /* We have to wait on the main_event in order to be notified when any of the - * sibling threads is exiting. */ - wait_on[0] = param->main_event; - /* The rest of the handles go behind the main event handle. */ - memcpy(wait_on + 1, param->handles, param->handles_count * sizeof(HANDLE)); - - res = WaitForMultipleObjects(param->handles_count + 1, wait_on, FALSE, INFINITE); - if (res > 0 && res < (param->handles_count + 1)) { - /* One of the original handles got signaled. Save its absolute index into - * the output variable. */ - InterlockedCompareExchange(param->signaled_index, - res - 1L + param->first_handle_index, -1L); - } - - /* Notify the caller (and the siblings) that the wait is over. */ - SetEvent(param->main_event); - - _endthreadex(0); - return 0; -} - -/* WaitForMultipeObjects fixer routine. - * Param: - * handles Array of handles to wait on. - * handles_count Number of handles in the array. - * Return: - * (>= 0 && < handles_count) - Index of the signaled handle in the array, or - * WAIT_FAILED on an error. - */ -static int -_wait_for_all(HANDLE* handles, int handles_count) -{ - WaitForAllParam* threads; - HANDLE main_event; - int chunks, chunk, remains; - - /* This variable is going to be accessed by several threads at the same time, - * this is bound to fail randomly when the core is run on multi-core machines. - * To solve this, we need to do the following (1 _and_ 2): - * 1. Use the "volatile" qualifier to ensure the compiler doesn't optimize - * out the reads/writes in this function unexpectedly. - * 2. Ensure correct memory ordering. The "simple" way to do that is to wrap - * all accesses inside a critical section. But we can also use - * InterlockedCompareExchange() which always provide a full memory barrier - * on Win32. - */ - volatile LONG sig_index = -1; - - /* Calculate number of chunks, and allocate thread param array. */ - chunks = handles_count / WAIT_ALL_CHUNK_SIZE; - remains = handles_count % WAIT_ALL_CHUNK_SIZE; - threads = (WaitForAllParam*)malloc((chunks + (remains ? 1 : 0)) * - sizeof(WaitForAllParam)); - if (threads == NULL) { - D("Unable to allocate thread array for %d handles.", handles_count); - return (int)WAIT_FAILED; - } - - /* Create main event to wait on for all waiting threads. This is a "manualy - * reset" event that will remain set once it was set. */ - main_event = CreateEvent(NULL, TRUE, FALSE, NULL); - if (main_event == NULL) { - D("Unable to create main event. Error: %ld", GetLastError()); - free(threads); - return (int)WAIT_FAILED; - } - - /* - * Initialize waiting thread parameters. - */ - - for (chunk = 0; chunk < chunks; chunk++) { - threads[chunk].main_event = main_event; - threads[chunk].signaled_index = &sig_index; - threads[chunk].first_handle_index = WAIT_ALL_CHUNK_SIZE * chunk; - threads[chunk].handles = handles + threads[chunk].first_handle_index; - threads[chunk].handles_count = WAIT_ALL_CHUNK_SIZE; - } - if (remains) { - threads[chunk].main_event = main_event; - threads[chunk].signaled_index = &sig_index; - threads[chunk].first_handle_index = WAIT_ALL_CHUNK_SIZE * chunk; - threads[chunk].handles = handles + threads[chunk].first_handle_index; - threads[chunk].handles_count = remains; - chunks++; - } - - /* Start the waiting threads. */ - for (chunk = 0; chunk < chunks; chunk++) { - /* Note that using adb_thread_create is not appropriate here, since we - * need a handle to wait on for thread termination. */ - threads[chunk].thread = (HANDLE)_beginthreadex(NULL, 0, _in_waiter_thread, - &threads[chunk], 0, NULL); - if (threads[chunk].thread == NULL) { - /* Unable to create a waiter thread. Collapse. */ - D("Unable to create a waiting thread %d of %d. errno=%d", - chunk, chunks, errno); - chunks = chunk; - SetEvent(main_event); - break; - } - } - - /* Wait on any of the threads to get signaled. */ - WaitForSingleObject(main_event, INFINITE); - - /* Wait on all the waiting threads to exit. */ - for (chunk = 0; chunk < chunks; chunk++) { - WaitForSingleObject(threads[chunk].thread, INFINITE); - CloseHandle(threads[chunk].thread); - } - - CloseHandle(main_event); - free(threads); - - - const int ret = (int)InterlockedCompareExchange(&sig_index, -1, -1); - return (ret >= 0) ? ret : (int)WAIT_FAILED; -} - -static EventLooperRec win32_looper; - -static void fdevent_init(void) -{ - win32_looper.htab_count = 0; - win32_looper.hooks = NULL; -} - -static void fdevent_connect(fdevent *fde) -{ - EventLooper looper = &win32_looper; - int events = fde->state & FDE_EVENTMASK; - - if (events != 0) - event_looper_hook( looper, fde->fd, events ); -} - -static void fdevent_disconnect(fdevent *fde) -{ - EventLooper looper = &win32_looper; - int events = fde->state & FDE_EVENTMASK; - - if (events != 0) - event_looper_unhook( looper, fde->fd, events ); -} - -static void fdevent_update(fdevent *fde, unsigned events) -{ - EventLooper looper = &win32_looper; - unsigned events0 = fde->state & FDE_EVENTMASK; - - if (events != events0) { - int removes = events0 & ~events; - int adds = events & ~events0; - if (removes) { - D("fdevent_update: remove %x from %d", removes, fde->fd); - event_looper_unhook( looper, fde->fd, removes ); - } - if (adds) { - D("fdevent_update: add %x to %d", adds, fde->fd); - event_looper_hook ( looper, fde->fd, adds ); - } - } -} - -static void fdevent_process() -{ - EventLooper looper = &win32_looper; - EventHook hook; - int gotone = 0; - - /* if we have at least one ready hook, execute it/them */ - for (hook = looper->hooks; hook; hook = hook->next) { - hook->ready = 0; - if (hook->prepare) { - hook->prepare(hook); - if (hook->ready != 0) { - event_hook_signal( hook ); - gotone = 1; - } - } - } - - /* nothing's ready yet, so wait for something to happen */ - if (!gotone) - { - looper->htab_count = 0; - - for (hook = looper->hooks; hook; hook = hook->next) - { - if (hook->start && !hook->start(hook)) { - D( "fdevent_process: error when starting a hook" ); - return; - } - if (hook->h != INVALID_HANDLE_VALUE) { - int nn; - - for (nn = 0; nn < looper->htab_count; nn++) - { - if ( looper->htab[nn] == hook->h ) - goto DontAdd; - } - looper->htab[ looper->htab_count++ ] = hook->h; - DontAdd: - ; - } - } - - if (looper->htab_count == 0) { - D( "fdevent_process: nothing to wait for !!" ); - return; - } - - do - { - int wait_ret; - - D( "adb_win32: waiting for %d events", looper->htab_count ); - if (looper->htab_count > MAXIMUM_WAIT_OBJECTS) { - D("handle count %d exceeds MAXIMUM_WAIT_OBJECTS.", looper->htab_count); - wait_ret = _wait_for_all(looper->htab, looper->htab_count); - } else { - wait_ret = WaitForMultipleObjects( looper->htab_count, looper->htab, FALSE, INFINITE ); - } - if (wait_ret == (int)WAIT_FAILED) { - D( "adb_win32: wait failed, error %ld", GetLastError() ); - } else { - D( "adb_win32: got one (index %d)", wait_ret ); - - /* according to Cygwin, some objects like consoles wake up on "inappropriate" events - * like mouse movements. we need to filter these with the "check" function - */ - if ((unsigned)wait_ret < (unsigned)looper->htab_count) - { - for (hook = looper->hooks; hook; hook = hook->next) - { - if ( looper->htab[wait_ret] == hook->h && - (!hook->check || hook->check(hook)) ) - { - D( "adb_win32: signaling %s for %x", hook->fh->name, hook->ready ); - event_hook_signal( hook ); - gotone = 1; - break; - } - } - } - } - } - while (!gotone); - - for (hook = looper->hooks; hook; hook = hook->next) { - if (hook->stop) - hook->stop( hook ); - } - } - - for (hook = looper->hooks; hook; hook = hook->next) { - if (hook->peek && hook->peek(hook)) - event_hook_signal( hook ); - } -} - - -static void fdevent_register(fdevent *fde) -{ - int fd = fde->fd - WIN32_FH_BASE; - - if(fd < 0) { - FATAL("bogus negative fd (%d)\n", fde->fd); - } - - if(fd >= fd_table_max) { - int oldmax = fd_table_max; - if(fde->fd > 32000) { - FATAL("bogus huuuuge fd (%d)\n", fde->fd); - } - if(fd_table_max == 0) { - fdevent_init(); - fd_table_max = 256; - } - while(fd_table_max <= fd) { - fd_table_max *= 2; - } - fd_table = reinterpret_cast(realloc(fd_table, sizeof(fdevent*) * fd_table_max)); - if(fd_table == 0) { - FATAL("could not expand fd_table to %d entries\n", fd_table_max); - } - memset(fd_table + oldmax, 0, sizeof(int) * (fd_table_max - oldmax)); - } - - fd_table[fd] = fde; -} - -static void fdevent_unregister(fdevent *fde) -{ - int fd = fde->fd - WIN32_FH_BASE; - - if((fd < 0) || (fd >= fd_table_max)) { - FATAL("fd out of range (%d)\n", fde->fd); - } - - if(fd_table[fd] != fde) { - FATAL("fd_table out of sync"); - } - - fd_table[fd] = 0; - - if(!(fde->state & FDE_DONT_CLOSE)) { - dump_fde(fde, "close"); - adb_close(fde->fd); - } -} - -static void fdevent_plist_enqueue(fdevent *node) -{ - fdevent *list = &list_pending; - - node->next = list; - node->prev = list->prev; - node->prev->next = node; - list->prev = node; -} - -static void fdevent_plist_remove(fdevent *node) -{ - node->prev->next = node->next; - node->next->prev = node->prev; - node->next = 0; - node->prev = 0; -} - -static fdevent *fdevent_plist_dequeue(void) -{ - fdevent *list = &list_pending; - fdevent *node = list->next; - - if(node == list) return 0; - - list->next = node->next; - list->next->prev = list; - node->next = 0; - node->prev = 0; - - return node; -} - -fdevent *fdevent_create(int fd, fd_func func, void *arg) -{ - fdevent *fde = (fdevent*) malloc(sizeof(fdevent)); - if(fde == 0) return 0; - fdevent_install(fde, fd, func, arg); - fde->state |= FDE_CREATED; - return fde; -} - -void fdevent_destroy(fdevent *fde) -{ - if(fde == 0) return; - if(!(fde->state & FDE_CREATED)) { - FATAL("fde %p not created by fdevent_create()\n", fde); - } - fdevent_remove(fde); -} - -void fdevent_install(fdevent *fde, int fd, fd_func func, void *arg) -{ - memset(fde, 0, sizeof(fdevent)); - fde->state = FDE_ACTIVE; - fde->fd = fd; - fde->func = func; - fde->arg = arg; - - fdevent_register(fde); - dump_fde(fde, "connect"); - fdevent_connect(fde); - fde->state |= FDE_ACTIVE; -} - -void fdevent_remove(fdevent *fde) -{ - if(fde->state & FDE_PENDING) { - fdevent_plist_remove(fde); - } - - if(fde->state & FDE_ACTIVE) { - fdevent_disconnect(fde); - dump_fde(fde, "disconnect"); - fdevent_unregister(fde); - } - - fde->state = 0; - fde->events = 0; -} - - -void fdevent_set(fdevent *fde, unsigned events) -{ - events &= FDE_EVENTMASK; - - if((fde->state & FDE_EVENTMASK) == (int)events) return; - - if(fde->state & FDE_ACTIVE) { - fdevent_update(fde, events); - dump_fde(fde, "update"); - } - - fde->state = (fde->state & FDE_STATEMASK) | events; - - if(fde->state & FDE_PENDING) { - /* if we're pending, make sure - ** we don't signal an event that - ** is no longer wanted. - */ - fde->events &= (~events); - if(fde->events == 0) { - fdevent_plist_remove(fde); - fde->state &= (~FDE_PENDING); - } - } -} - -void fdevent_add(fdevent *fde, unsigned events) -{ - fdevent_set( - fde, (fde->state & FDE_EVENTMASK) | (events & FDE_EVENTMASK)); -} - -void fdevent_del(fdevent *fde, unsigned events) -{ - fdevent_set( - fde, (fde->state & FDE_EVENTMASK) & (~(events & FDE_EVENTMASK))); -} - -void fdevent_loop() -{ - fdevent *fde; - - for(;;) { -#if DEBUG - fprintf(stderr,"--- ---- waiting for events\n"); -#endif - fdevent_process(); - - while((fde = fdevent_plist_dequeue())) { - unsigned events = fde->events; - fde->events = 0; - fde->state &= (~FDE_PENDING); - dump_fde(fde, "callback"); - fde->func(fde->fd, events, fde->arg); - } - } -} - -/** FILE EVENT HOOKS - **/ - -static void _event_file_prepare( EventHook hook ) -{ - if (hook->wanted & (FDE_READ|FDE_WRITE)) { - /* we can always read/write */ - hook->ready |= hook->wanted & (FDE_READ|FDE_WRITE); - } -} - -static int _event_file_peek( EventHook hook ) -{ - return (hook->wanted & (FDE_READ|FDE_WRITE)); -} - -static void _fh_file_hook( FH f, int events, EventHook hook ) -{ - hook->h = f->fh_handle; - hook->prepare = _event_file_prepare; - hook->peek = _event_file_peek; -} - -/** SOCKET EVENT HOOKS - **/ - -static void _event_socket_verify( EventHook hook, WSANETWORKEVENTS* evts ) -{ - if ( evts->lNetworkEvents & (FD_READ|FD_ACCEPT|FD_CLOSE) ) { - if (hook->wanted & FDE_READ) - hook->ready |= FDE_READ; - if ((evts->iErrorCode[FD_READ] != 0) && hook->wanted & FDE_ERROR) - hook->ready |= FDE_ERROR; - } - if ( evts->lNetworkEvents & (FD_WRITE|FD_CONNECT|FD_CLOSE) ) { - if (hook->wanted & FDE_WRITE) - hook->ready |= FDE_WRITE; - if ((evts->iErrorCode[FD_WRITE] != 0) && hook->wanted & FDE_ERROR) - hook->ready |= FDE_ERROR; - } - if ( evts->lNetworkEvents & FD_OOB ) { - if (hook->wanted & FDE_ERROR) - hook->ready |= FDE_ERROR; - } -} - -static void _event_socket_prepare( EventHook hook ) -{ - WSANETWORKEVENTS evts; - - /* look if some of the events we want already happened ? */ - if (!WSAEnumNetworkEvents( hook->fh->fh_socket, NULL, &evts )) - _event_socket_verify( hook, &evts ); -} - -static int _socket_wanted_to_flags( int wanted ) -{ - int flags = 0; - if (wanted & FDE_READ) - flags |= FD_READ | FD_ACCEPT | FD_CLOSE; - - if (wanted & FDE_WRITE) - flags |= FD_WRITE | FD_CONNECT | FD_CLOSE; - - if (wanted & FDE_ERROR) - flags |= FD_OOB; - - return flags; -} - -static int _event_socket_start( EventHook hook ) -{ - /* create an event which we're going to wait for */ - FH fh = hook->fh; - long flags = _socket_wanted_to_flags( hook->wanted ); - - hook->h = fh->event; - if (hook->h == INVALID_HANDLE_VALUE) { - D( "_event_socket_start: no event for %s", fh->name ); - return 0; - } - - if ( flags != fh->mask ) { - D( "_event_socket_start: hooking %s for %x (flags %ld)", hook->fh->name, hook->wanted, flags ); - if ( WSAEventSelect( fh->fh_socket, hook->h, flags ) ) { - D( "_event_socket_start: WSAEventSelect() for %s failed, error %d", hook->fh->name, WSAGetLastError() ); - CloseHandle( hook->h ); - hook->h = INVALID_HANDLE_VALUE; - exit(1); - return 0; - } - fh->mask = flags; - } - return 1; -} - -static void _event_socket_stop( EventHook hook ) -{ - hook->h = INVALID_HANDLE_VALUE; -} - -static int _event_socket_check( EventHook hook ) -{ - int result = 0; - FH fh = hook->fh; - WSANETWORKEVENTS evts; - - if (!WSAEnumNetworkEvents( fh->fh_socket, hook->h, &evts ) ) { - _event_socket_verify( hook, &evts ); - result = (hook->ready != 0); - if (result) { - ResetEvent( hook->h ); - } - } - D( "_event_socket_check %s returns %d", fh->name, result ); - return result; -} - -static int _event_socket_peek( EventHook hook ) -{ - WSANETWORKEVENTS evts; - FH fh = hook->fh; - - /* look if some of the events we want already happened ? */ - if (!WSAEnumNetworkEvents( fh->fh_socket, NULL, &evts )) { - _event_socket_verify( hook, &evts ); - if (hook->ready) - ResetEvent( hook->h ); - } - - return hook->ready != 0; -} - - - -static void _fh_socket_hook( FH f, int events, EventHook hook ) -{ - hook->prepare = _event_socket_prepare; - hook->start = _event_socket_start; - hook->stop = _event_socket_stop; - hook->check = _event_socket_check; - hook->peek = _event_socket_peek; - - // TODO: check return value? - _event_socket_start( hook ); -} - -/** SOCKETPAIR EVENT HOOKS - **/ - -static void _event_socketpair_prepare( EventHook hook ) -{ - FH fh = hook->fh; - SocketPair pair = fh->fh_pair; - BipBuffer rbip = (pair->a_fd == fh) ? &pair->b2a_bip : &pair->a2b_bip; - BipBuffer wbip = (pair->a_fd == fh) ? &pair->a2b_bip : &pair->b2a_bip; - - if (hook->wanted & FDE_READ && rbip->can_read) - hook->ready |= FDE_READ; - - if (hook->wanted & FDE_WRITE && wbip->can_write) - hook->ready |= FDE_WRITE; - } - - static int _event_socketpair_start( EventHook hook ) - { - FH fh = hook->fh; - SocketPair pair = fh->fh_pair; - BipBuffer rbip = (pair->a_fd == fh) ? &pair->b2a_bip : &pair->a2b_bip; - BipBuffer wbip = (pair->a_fd == fh) ? &pair->a2b_bip : &pair->b2a_bip; - - if (hook->wanted == FDE_READ) - hook->h = rbip->evt_read; - - else if (hook->wanted == FDE_WRITE) - hook->h = wbip->evt_write; - - else { - D("_event_socketpair_start: can't handle FDE_READ+FDE_WRITE" ); - return 0; - } - D( "_event_socketpair_start: hook %s for %x wanted=%x", - hook->fh->name, _fh_to_int(fh), hook->wanted); - return 1; -} - -static int _event_socketpair_peek( EventHook hook ) -{ - _event_socketpair_prepare( hook ); - return hook->ready != 0; -} - -static void _fh_socketpair_hook( FH fh, int events, EventHook hook ) -{ - hook->prepare = _event_socketpair_prepare; - hook->start = _event_socketpair_start; - hook->peek = _event_socketpair_peek; -} - - static adb_mutex_t g_console_output_buffer_lock; void From 8443fd994a19b685d5f93b2a233b9a04170f9172 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 17 Feb 2016 16:45:39 -0800 Subject: [PATCH 13/20] adb: move win32 fd base to 2048, fix fd allocation. Windows has a maximum fd limit of 2048, so we can avoid collision with real file descriptors by starting from there. Also, fds would be previously be allocated by a linear walk from the last allocated FD, instead of the lowest available FD, as required by POSIX. Keep track of the lowest available file descriptor to make things feel more familiar. Change-Id: Id6ac1c54f4f7964a6cdfa8d3f4f96262e4881964 (cherry picked from commit b6232b96ddda96f3d72dd787fae0b0087ddbcab9) --- adb/sysdeps_win32.cpp | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp index 0dbfb9847..c36d77991 100644 --- a/adb/sysdeps_win32.cpp +++ b/adb/sysdeps_win32.cpp @@ -190,8 +190,7 @@ typedef struct FHRec_ #define fh_handle u.handle #define fh_socket u.socket -#define WIN32_FH_BASE 100 - +#define WIN32_FH_BASE 2048 #define WIN32_MAX_FHS 128 static adb_mutex_t _win32_lock; @@ -241,17 +240,10 @@ _fh_alloc( FHClass clazz ) adb_mutex_lock( &_win32_lock ); - // Search entire array, starting from _win32_fh_next. - for (int nn = 0; nn < WIN32_MAX_FHS; nn++) { - // Keep incrementing _win32_fh_next to avoid giving out an index that - // was recently closed, to try to avoid use-after-free. - const int index = _win32_fh_next++; - // Handle wrap-around of _win32_fh_next. - if (_win32_fh_next == WIN32_MAX_FHS) { - _win32_fh_next = 0; - } - if (_win32_fhs[index].clazz == NULL) { - f = &_win32_fhs[index]; + 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; } } @@ -276,6 +268,12 @@ _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); + + int offset = f - _win32_fhs; + if (_win32_fh_next > offset) { + _win32_fh_next = offset; + } + if (f->used) { f->clazz->_fh_close( f ); f->name[0] = '\0'; From c1d252bec233b422a445cf6b711b33d7f541e4ef Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Thu, 18 Feb 2016 13:43:55 -0800 Subject: [PATCH 14/20] adb: add fd exhaustion test, fix errno reporting in sysdeps_win32. Add a test for FD exhaustion, and fix cases where we weren't properly setting errno. Change-Id: I486055bb9ead31089ce76b210c11de9e973f3256 (cherry picked from commit 6487e74a5991263cda5e59dbd21710d2372b0fa1) --- adb/sysdeps_test.cpp | 20 ++++++ adb/sysdeps_win32.cpp | 149 +++++++++++++++++++++--------------------- 2 files changed, 96 insertions(+), 73 deletions(-) diff --git a/adb/sysdeps_test.cpp b/adb/sysdeps_test.cpp index 19856dcba..253d62fc2 100644 --- a/adb/sysdeps_test.cpp +++ b/adb/sysdeps_test.cpp @@ -84,6 +84,26 @@ TEST(sysdeps_socketpair, smoke) { ASSERT_EQ(0, adb_close(fds[1])); } +TEST(sysdeps_fd, exhaustion) { + std::vector fds; + int socketpair[2]; + + while (adb_socketpair(socketpair) == 0) { + fds.push_back(socketpair[0]); + fds.push_back(socketpair[1]); + } + + ASSERT_EQ(EMFILE, errno) << strerror(errno); + for (int fd : fds) { + ASSERT_EQ(0, adb_close(fd)); + } + ASSERT_EQ(0, adb_socketpair(socketpair)); + ASSERT_EQ(socketpair[0], fds[0]); + ASSERT_EQ(socketpair[1], fds[1]); + ASSERT_EQ(0, adb_close(socketpair[0])); + ASSERT_EQ(0, adb_close(socketpair[1])); +} + class sysdeps_poll : public ::testing::Test { protected: int fds[2]; diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp index c36d77991..7eae186c4 100644 --- a/adb/sysdeps_win32.cpp +++ b/adb/sysdeps_win32.cpp @@ -726,8 +726,9 @@ static int _fh_socket_close( FH f ) { #endif } if (closesocket(f->fh_socket) == SOCKET_ERROR) { - D("closesocket failed: %s", - android::base::SystemErrorCodeToString(WSAGetLastError()).c_str()); + // Don't set errno here, since adb_close will ignore it. + const DWORD err = WSAGetLastError(); + D("closesocket failed: %s", android::base::SystemErrorCodeToString(err).c_str()); } f->fh_socket = INVALID_SOCKET; } @@ -839,16 +840,15 @@ static int GetSocketProtocolFromSocketType(int type) { int network_loopback_client(int port, int type, std::string* error) { struct sockaddr_in addr; - SOCKET s; + SOCKET s; - unique_fh f(_fh_alloc(&_fh_socket_class)); + unique_fh f(_fh_alloc(&_fh_socket_class)); if (!f) { *error = strerror(errno); return -1; } - if (!_winsock_init) - _init_winsock(); + if (!_winsock_init) _init_winsock(); memset(&addr, 0, sizeof(addr)); addr.sin_family = AF_INET; @@ -856,30 +856,32 @@ int network_loopback_client(int port, int type, std::string* error) { addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); s = socket(AF_INET, type, GetSocketProtocolFromSocketType(type)); - if(s == INVALID_SOCKET) { + if (s == INVALID_SOCKET) { + const DWORD err = WSAGetLastError(); *error = android::base::StringPrintf("cannot create socket: %s", - android::base::SystemErrorCodeToString(WSAGetLastError()).c_str()); + android::base::SystemErrorCodeToString(err).c_str()); D("%s", error->c_str()); + _socket_set_errno(err); return -1; } f->fh_socket = s; - if(connect(s, (struct sockaddr *) &addr, sizeof(addr)) == SOCKET_ERROR) { + if (connect(s, (struct sockaddr*)&addr, sizeof(addr)) == SOCKET_ERROR) { // Save err just in case inet_ntoa() or ntohs() changes the last error. const DWORD err = WSAGetLastError(); *error = android::base::StringPrintf("cannot connect to %s:%u: %s", - inet_ntoa(addr.sin_addr), ntohs(addr.sin_port), - android::base::SystemErrorCodeToString(err).c_str()); - D("could not connect to %s:%d: %s", - type != SOCK_STREAM ? "udp" : "tcp", port, error->c_str()); + inet_ntoa(addr.sin_addr), ntohs(addr.sin_port), + android::base::SystemErrorCodeToString(err).c_str()); + D("could not connect to %s:%d: %s", type != SOCK_STREAM ? "udp" : "tcp", port, + error->c_str()); + _socket_set_errno(err); return -1; } const int fd = _fh_to_int(f.get()); - snprintf( f->name, sizeof(f->name), "%d(lo-client:%s%d)", fd, - type != SOCK_STREAM ? "udp:" : "", port ); - D( "port %d type %s => fd %d", port, type != SOCK_STREAM ? "udp" : "tcp", - fd ); + snprintf(f->name, sizeof(f->name), "%d(lo-client:%s%d)", fd, type != SOCK_STREAM ? "udp:" : "", + port); + D("port %d type %s => fd %d", port, type != SOCK_STREAM ? "udp" : "tcp", fd); f.release(); return fd; } @@ -887,20 +889,18 @@ int network_loopback_client(int port, int type, std::string* error) { #define LISTEN_BACKLOG 4 // interface_address is INADDR_LOOPBACK or INADDR_ANY. -static int _network_server(int port, int type, u_long interface_address, - std::string* error) { +static int _network_server(int port, int type, u_long interface_address, std::string* error) { struct sockaddr_in addr; - SOCKET s; - int n; + SOCKET s; + int n; - unique_fh f(_fh_alloc(&_fh_socket_class)); + unique_fh f(_fh_alloc(&_fh_socket_class)); if (!f) { *error = strerror(errno); return -1; } - if (!_winsock_init) - _init_winsock(); + if (!_winsock_init) _init_winsock(); memset(&addr, 0, sizeof(addr)); addr.sin_family = AF_INET; @@ -911,9 +911,11 @@ static int _network_server(int port, int type, u_long interface_address, // IPv4 and IPv6. s = socket(AF_INET, type, GetSocketProtocolFromSocketType(type)); if (s == INVALID_SOCKET) { + const DWORD err = WSAGetLastError(); *error = android::base::StringPrintf("cannot create socket: %s", - android::base::SystemErrorCodeToString(WSAGetLastError()).c_str()); + android::base::SystemErrorCodeToString(err).c_str()); D("%s", error->c_str()); + _socket_set_errno(err); return -1; } @@ -922,40 +924,41 @@ static int _network_server(int port, int type, u_long interface_address, // Note: SO_REUSEADDR on Windows allows multiple processes to bind to the // same port, so instead use SO_EXCLUSIVEADDRUSE. n = 1; - if (setsockopt(s, SOL_SOCKET, SO_EXCLUSIVEADDRUSE, (const char*)&n, - sizeof(n)) == SOCKET_ERROR) { - *error = android::base::StringPrintf( - "cannot set socket option SO_EXCLUSIVEADDRUSE: %s", - android::base::SystemErrorCodeToString(WSAGetLastError()).c_str()); + if (setsockopt(s, SOL_SOCKET, SO_EXCLUSIVEADDRUSE, (const char*)&n, sizeof(n)) == SOCKET_ERROR) { + const DWORD err = WSAGetLastError(); + *error = android::base::StringPrintf("cannot set socket option SO_EXCLUSIVEADDRUSE: %s", + android::base::SystemErrorCodeToString(err).c_str()); D("%s", error->c_str()); + _socket_set_errno(err); return -1; } - if (bind(s, (struct sockaddr *) &addr, sizeof(addr)) == SOCKET_ERROR) { + if (bind(s, (struct sockaddr*)&addr, sizeof(addr)) == SOCKET_ERROR) { // Save err just in case inet_ntoa() or ntohs() changes the last error. const DWORD err = WSAGetLastError(); - *error = android::base::StringPrintf("cannot bind to %s:%u: %s", - inet_ntoa(addr.sin_addr), ntohs(addr.sin_port), - android::base::SystemErrorCodeToString(err).c_str()); - D("could not bind to %s:%d: %s", - type != SOCK_STREAM ? "udp" : "tcp", port, error->c_str()); + *error = android::base::StringPrintf("cannot bind to %s:%u: %s", inet_ntoa(addr.sin_addr), + ntohs(addr.sin_port), + android::base::SystemErrorCodeToString(err).c_str()); + D("could not bind to %s:%d: %s", type != SOCK_STREAM ? "udp" : "tcp", port, error->c_str()); + _socket_set_errno(err); return -1; } if (type == SOCK_STREAM) { if (listen(s, LISTEN_BACKLOG) == SOCKET_ERROR) { - *error = android::base::StringPrintf("cannot listen on socket: %s", - android::base::SystemErrorCodeToString(WSAGetLastError()).c_str()); - D("could not listen on %s:%d: %s", - type != SOCK_STREAM ? "udp" : "tcp", port, error->c_str()); + const DWORD err = WSAGetLastError(); + *error = android::base::StringPrintf( + "cannot listen on socket: %s", android::base::SystemErrorCodeToString(err).c_str()); + D("could not listen on %s:%d: %s", type != SOCK_STREAM ? "udp" : "tcp", port, + error->c_str()); + _socket_set_errno(err); return -1; } } const int fd = _fh_to_int(f.get()); - snprintf( f->name, sizeof(f->name), "%d(%s-server:%s%d)", fd, - interface_address == INADDR_LOOPBACK ? "lo" : "any", - type != SOCK_STREAM ? "udp:" : "", port ); - D( "port %d type %s => fd %d", port, type != SOCK_STREAM ? "udp" : "tcp", - fd ); + snprintf(f->name, sizeof(f->name), "%d(%s-server:%s%d)", fd, + interface_address == INADDR_LOOPBACK ? "lo" : "any", type != SOCK_STREAM ? "udp:" : "", + port); + D("port %d type %s => fd %d", port, type != SOCK_STREAM ? "udp" : "tcp", fd); f.release(); return fd; } @@ -989,54 +992,57 @@ int network_connect(const std::string& host, int port, int type, int timeout, st struct addrinfo* addrinfo_ptr = nullptr; #if (NTDDI_VERSION >= NTDDI_WINXPSP2) || (_WIN32_WINNT >= _WIN32_WINNT_WS03) - // TODO: When the Android SDK tools increases the Windows system - // requirements >= WinXP SP2, switch to android::base::UTF8ToWide() + GetAddrInfoW(). +// TODO: When the Android SDK tools increases the Windows system +// requirements >= WinXP SP2, switch to android::base::UTF8ToWide() + GetAddrInfoW(). #else - // Otherwise, keep using getaddrinfo(), or do runtime API detection - // with GetProcAddress("GetAddrInfoW"). +// Otherwise, keep using getaddrinfo(), or do runtime API detection +// with GetProcAddress("GetAddrInfoW"). #endif if (getaddrinfo(host.c_str(), port_str, &hints, &addrinfo_ptr) != 0) { - *error = android::base::StringPrintf( - "cannot resolve host '%s' and port %s: %s", host.c_str(), - port_str, android::base::SystemErrorCodeToString(WSAGetLastError()).c_str()); + const DWORD err = WSAGetLastError(); + *error = android::base::StringPrintf("cannot resolve host '%s' and port %s: %s", + host.c_str(), port_str, + android::base::SystemErrorCodeToString(err).c_str()); + D("%s", error->c_str()); + _socket_set_errno(err); return -1; } - std::unique_ptr - addrinfo(addrinfo_ptr, freeaddrinfo); + std::unique_ptr addrinfo(addrinfo_ptr, freeaddrinfo); addrinfo_ptr = nullptr; // TODO: Try all the addresses if there's more than one? This just uses // the first. Or, could call WSAConnectByName() (Windows Vista and newer) // which tries all addresses, takes a timeout and more. - SOCKET s = socket(addrinfo->ai_family, addrinfo->ai_socktype, - addrinfo->ai_protocol); - if(s == INVALID_SOCKET) { + SOCKET s = socket(addrinfo->ai_family, addrinfo->ai_socktype, addrinfo->ai_protocol); + if (s == INVALID_SOCKET) { + const DWORD err = WSAGetLastError(); *error = android::base::StringPrintf("cannot create socket: %s", - android::base::SystemErrorCodeToString(WSAGetLastError()).c_str()); + android::base::SystemErrorCodeToString(err).c_str()); D("%s", error->c_str()); + _socket_set_errno(err); return -1; } f->fh_socket = s; // TODO: Implement timeouts for Windows. Seems like the default in theory // (according to http://serverfault.com/a/671453) and in practice is 21 sec. - if(connect(s, addrinfo->ai_addr, addrinfo->ai_addrlen) == SOCKET_ERROR) { + if (connect(s, addrinfo->ai_addr, addrinfo->ai_addrlen) == SOCKET_ERROR) { // TODO: Use WSAAddressToString or inet_ntop on address. - *error = android::base::StringPrintf("cannot connect to %s:%s: %s", - host.c_str(), port_str, - android::base::SystemErrorCodeToString(WSAGetLastError()).c_str()); - D("could not connect to %s:%s:%s: %s", - type != SOCK_STREAM ? "udp" : "tcp", host.c_str(), port_str, - error->c_str()); + const DWORD err = WSAGetLastError(); + *error = android::base::StringPrintf("cannot connect to %s:%s: %s", host.c_str(), port_str, + android::base::SystemErrorCodeToString(err).c_str()); + D("could not connect to %s:%s:%s: %s", type != SOCK_STREAM ? "udp" : "tcp", host.c_str(), + port_str, error->c_str()); + _socket_set_errno(err); return -1; } const int fd = _fh_to_int(f.get()); - snprintf( f->name, sizeof(f->name), "%d(net-client:%s%d)", fd, - type != SOCK_STREAM ? "udp:" : "", port ); - D( "host '%s' port %d type %s => fd %d", host.c_str(), port, - type != SOCK_STREAM ? "udp" : "tcp", fd ); + snprintf(f->name, sizeof(f->name), "%d(net-client:%s%d)", fd, type != SOCK_STREAM ? "udp:" : "", + port); + D("host '%s' port %d type %s => fd %d", host.c_str(), port, type != SOCK_STREAM ? "udp" : "tcp", + fd); f.release(); return fd; } @@ -1180,10 +1186,7 @@ int adb_socketpair(int sv[2]) { accepted = adb_socket_accept(server, nullptr, nullptr); if (accepted < 0) { - const DWORD err = WSAGetLastError(); - D("adb_socketpair: failed to accept: %s", - android::base::SystemErrorCodeToString(err).c_str()); - _socket_set_errno(err); + D("adb_socketpair: failed to accept: %s", strerror(errno)); goto fail; } adb_close(server); From b582fa3bd8223ba1cf5936ffcb10e078d4ff13e1 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 10 Feb 2016 14:49:00 -0800 Subject: [PATCH 15/20] adb: make fdevent_test, socket_test compile on Windows. Switch pthread_* to use the adb_thread_* abstractions to allow the fdevent and socket tests to compile on Win32. Bug: http://b/27105824 Change-Id: I6541bb1398780b999837e701837d7f86a5eee8ca (cherry picked from commit 022d447e9efcff59e22f0ab13764282116f235dd) --- adb/Android.mk | 10 +- adb/fdevent.cpp | 11 ++ adb/fdevent.h | 4 +- adb/fdevent_test.cpp | 63 ++++------- adb/fdevent_test.h | 58 ++++++++++ adb/socket_test.cpp | 263 +++++++++++++++++++------------------------ 6 files changed, 210 insertions(+), 199 deletions(-) create mode 100644 adb/fdevent_test.h diff --git a/adb/Android.mk b/adb/Android.mk index 7835d9f3a..e0997eafd 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -59,6 +59,8 @@ LIBADB_SRC_FILES := \ LIBADB_TEST_SRCS := \ adb_io_test.cpp \ adb_utils_test.cpp \ + fdevent_test.cpp \ + socket_test.cpp \ sysdeps_test.cpp \ transport_test.cpp \ @@ -87,14 +89,6 @@ LIBADB_windows_SRC_FILES := \ sysdeps_win32.cpp \ usb_windows.cpp \ -LIBADB_TEST_linux_SRCS := \ - fdevent_test.cpp \ - socket_test.cpp \ - -LIBADB_TEST_darwin_SRCS := \ - fdevent_test.cpp \ - socket_test.cpp \ - LIBADB_TEST_windows_SRCS := \ sysdeps_win32_test.cpp \ diff --git a/adb/fdevent.cpp b/adb/fdevent.cpp index 461736422..902548ec5 100644 --- a/adb/fdevent.cpp +++ b/adb/fdevent.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -70,6 +71,7 @@ struct PollNode { // That's why we don't need a lock for fdevent. static auto& g_poll_node_map = *new std::unordered_map(); static auto& g_pending_list = *new std::list(); +static std::atomic terminate_loop(false); static bool main_thread_valid; static unsigned long main_thread_id; @@ -364,6 +366,10 @@ void fdevent_loop() #endif // !ADB_HOST while (true) { + if (terminate_loop) { + return; + } + D("--- --- waiting for events"); fdevent_process(); @@ -376,6 +382,10 @@ void fdevent_loop() } } +void fdevent_terminate_loop() { + terminate_loop = true; +} + size_t fdevent_installed_count() { return g_poll_node_map.size(); } @@ -384,4 +394,5 @@ void fdevent_reset() { g_poll_node_map.clear(); g_pending_list.clear(); main_thread_valid = false; + terminate_loop = false; } diff --git a/adb/fdevent.h b/adb/fdevent.h index 657fde5e9..207f9b702 100644 --- a/adb/fdevent.h +++ b/adb/fdevent.h @@ -76,9 +76,9 @@ void fdevent_set_timeout(fdevent *fde, int64_t timeout_ms); */ void fdevent_loop(); -// For debugging only. +// The following functions are used only for tests. +void fdevent_terminate_loop(); size_t fdevent_installed_count(); -// For debugging only. void fdevent_reset(); #endif diff --git a/adb/fdevent_test.cpp b/adb/fdevent_test.cpp index 7fe3d37d2..c933ed528 100644 --- a/adb/fdevent_test.cpp +++ b/adb/fdevent_test.cpp @@ -18,15 +18,13 @@ #include -#include -#include - #include #include #include #include #include "adb_io.h" +#include "fdevent_test.h" class FdHandler { public: @@ -48,7 +46,7 @@ class FdHandler { if (events & FDE_READ) { ASSERT_EQ(fd, handler->read_fd_); char c; - ASSERT_EQ(1, read(fd, &c, 1)); + ASSERT_EQ(1, adb_read(fd, &c, 1)); handler->queue_.push(c); fdevent_add(&handler->write_fde_, FDE_WRITE); } @@ -57,7 +55,7 @@ class FdHandler { ASSERT_FALSE(handler->queue_.empty()); char c = handler->queue_.front(); handler->queue_.pop(); - ASSERT_EQ(1, write(fd, &c, 1)); + ASSERT_EQ(1, adb_write(fd, &c, 1)); if (handler->queue_.empty()) { fdevent_del(&handler->write_fde_, FDE_WRITE); } @@ -72,29 +70,19 @@ class FdHandler { std::queue queue_; }; -static void signal_handler(int) { - pthread_exit(nullptr); -} - -class FdeventTest : public ::testing::Test { - protected: - static void SetUpTestCase() { - ASSERT_NE(SIG_ERR, signal(SIGUSR1, signal_handler)); - ASSERT_NE(SIG_ERR, signal(SIGPIPE, SIG_IGN)); - } - - virtual void SetUp() { - fdevent_reset(); - ASSERT_EQ(0u, fdevent_installed_count()); - } -}; - struct ThreadArg { int first_read_fd; int last_write_fd; size_t middle_pipe_count; }; +TEST_F(FdeventTest, fdevent_terminate) { + adb_thread_t thread; + PrepareThread(); + ASSERT_TRUE(adb_thread_create([](void*) { fdevent_loop(); }, nullptr, &thread)); + TerminateThread(thread); +} + static void FdEventThreadFunc(ThreadArg* arg) { std::vector read_fds; std::vector write_fds; @@ -102,7 +90,7 @@ static void FdEventThreadFunc(ThreadArg* arg) { read_fds.push_back(arg->first_read_fd); for (size_t i = 0; i < arg->middle_pipe_count; ++i) { int fds[2]; - ASSERT_EQ(0, pipe(fds)); + ASSERT_EQ(0, adb_socketpair(fds)); read_fds.push_back(fds[0]); write_fds.push_back(fds[1]); } @@ -122,9 +110,9 @@ TEST_F(FdeventTest, smoke) { const std::string MESSAGE = "fdevent_test"; int fd_pair1[2]; int fd_pair2[2]; - ASSERT_EQ(0, pipe(fd_pair1)); - ASSERT_EQ(0, pipe(fd_pair2)); - pthread_t thread; + 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]; @@ -132,9 +120,9 @@ TEST_F(FdeventTest, smoke) { int writer = fd_pair1[1]; int reader = fd_pair2[0]; - ASSERT_EQ(0, pthread_create(&thread, nullptr, - reinterpret_cast(FdEventThreadFunc), - &thread_arg)); + PrepareThread(); + ASSERT_TRUE(adb_thread_create(reinterpret_cast(FdEventThreadFunc), &thread_arg, + &thread)); for (size_t i = 0; i < MESSAGE_LOOP_COUNT; ++i) { std::string read_buffer = MESSAGE; @@ -144,10 +132,9 @@ TEST_F(FdeventTest, smoke) { ASSERT_EQ(read_buffer, write_buffer); } - ASSERT_EQ(0, pthread_kill(thread, SIGUSR1)); - ASSERT_EQ(0, pthread_join(thread, nullptr)); - ASSERT_EQ(0, close(writer)); - ASSERT_EQ(0, close(reader)); + TerminateThread(thread); + ASSERT_EQ(0, adb_close(writer)); + ASSERT_EQ(0, adb_close(reader)); } struct InvalidFdArg { @@ -161,7 +148,7 @@ static void InvalidFdEventCallback(int fd, unsigned events, void* userdata) { ASSERT_EQ(arg->expected_events, events); fdevent_remove(&arg->fde); if (++*(arg->happened_event_count) == 2) { - pthread_exit(nullptr); + fdevent_terminate_loop(); } } @@ -184,9 +171,7 @@ static void InvalidFdThreadFunc(void*) { } TEST_F(FdeventTest, invalid_fd) { - pthread_t thread; - ASSERT_EQ(0, pthread_create(&thread, nullptr, - reinterpret_cast(InvalidFdThreadFunc), - nullptr)); - ASSERT_EQ(0, pthread_join(thread, nullptr)); + adb_thread_t thread; + ASSERT_TRUE(adb_thread_create(InvalidFdThreadFunc, nullptr, &thread)); + ASSERT_TRUE(adb_thread_join(thread)); } diff --git a/adb/fdevent_test.h b/adb/fdevent_test.h new file mode 100644 index 000000000..c853bceec --- /dev/null +++ b/adb/fdevent_test.h @@ -0,0 +1,58 @@ +/* + * Copyright (C) 2016 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 "socket.h" +#include "sysdeps.h" + +class FdeventTest : public ::testing::Test { + protected: + int dummy = -1; + + static void SetUpTestCase() { +#if !defined(_WIN32) + ASSERT_NE(SIG_ERR, signal(SIGPIPE, SIG_IGN)); +#endif + } + + void SetUp() override { + fdevent_reset(); + ASSERT_EQ(0u, fdevent_installed_count()); + } + + // Register a dummy socket used to wake up the fdevent loop to tell it to die. + void PrepareThread() { + int dummy_fds[2]; + if (adb_socketpair(dummy_fds) != 0) { + FAIL() << "failed to create socketpair: " << strerror(errno); + } + + asocket* dummy_socket = create_local_socket(dummy_fds[1]); + if (!dummy_socket) { + FAIL() << "failed to create local socket: " << strerror(errno); + } + dummy_socket->ready(dummy_socket); + dummy = dummy_fds[0]; + } + + void TerminateThread(adb_thread_t thread) { + fdevent_terminate_loop(); + ASSERT_TRUE(WriteFdExactly(dummy, "", 1)); + ASSERT_TRUE(adb_thread_join(thread)); + ASSERT_EQ(0, adb_close(dummy)); + } +}; diff --git a/adb/socket_test.cpp b/adb/socket_test.cpp index 03cab64a5..471ca09e4 100644 --- a/adb/socket_test.cpp +++ b/adb/socket_test.cpp @@ -18,119 +18,89 @@ #include +#include #include #include #include #include -#include -#include #include #include "adb.h" #include "adb_io.h" +#include "fdevent_test.h" #include "socket.h" #include "sysdeps.h" -static void signal_handler(int) { - ASSERT_EQ(1u, fdevent_installed_count()); - pthread_exit(nullptr); -} - -// On host, register a dummy socket, so fdevet_loop() will not abort when previously -// registered local sockets are all closed. On device, fdevent_subproc_setup() installs -// one fdevent which can be considered as dummy socket. -static void InstallDummySocket() { -#if ADB_HOST - int dummy_fds[2]; - ASSERT_EQ(0, pipe(dummy_fds)); - asocket* dummy_socket = create_local_socket(dummy_fds[0]); - ASSERT_TRUE(dummy_socket != nullptr); - dummy_socket->ready(dummy_socket); -#endif -} - struct ThreadArg { int first_read_fd; int last_write_fd; size_t middle_pipe_count; }; -static void FdEventThreadFunc(ThreadArg* arg) { - std::vector read_fds; - std::vector write_fds; +class LocalSocketTest : public FdeventTest {}; - read_fds.push_back(arg->first_read_fd); - for (size_t i = 0; i < arg->middle_pipe_count; ++i) { - int fds[2]; - ASSERT_EQ(0, adb_socketpair(fds)); - read_fds.push_back(fds[0]); - write_fds.push_back(fds[1]); - } - write_fds.push_back(arg->last_write_fd); - - for (size_t i = 0; i < read_fds.size(); ++i) { - asocket* reader = create_local_socket(read_fds[i]); - ASSERT_TRUE(reader != nullptr); - asocket* writer = create_local_socket(write_fds[i]); - ASSERT_TRUE(writer != nullptr); - reader->peer = writer; - writer->peer = reader; - reader->ready(reader); - } - - InstallDummySocket(); +static void FdEventThreadFunc(void*) { fdevent_loop(); } -class LocalSocketTest : public ::testing::Test { - protected: - static void SetUpTestCase() { - ASSERT_NE(SIG_ERR, signal(SIGUSR1, signal_handler)); - ASSERT_NE(SIG_ERR, signal(SIGPIPE, SIG_IGN)); - } - - virtual void SetUp() { - fdevent_reset(); - ASSERT_EQ(0u, fdevent_installed_count()); - } -}; - TEST_F(LocalSocketTest, smoke) { - const size_t PIPE_COUNT = 100; - const size_t MESSAGE_LOOP_COUNT = 100; + // Join two socketpairs with a chain of intermediate socketpairs. + int first[2]; + std::vector> intermediates; + int last[2]; + + constexpr size_t INTERMEDIATE_COUNT = 50; + constexpr size_t MESSAGE_LOOP_COUNT = 100; const std::string MESSAGE = "socket_test"; - int fd_pair1[2]; - int fd_pair2[2]; - ASSERT_EQ(0, adb_socketpair(fd_pair1)); - ASSERT_EQ(0, adb_socketpair(fd_pair2)); - pthread_t thread; - ThreadArg thread_arg; - thread_arg.first_read_fd = fd_pair1[0]; - thread_arg.last_write_fd = fd_pair2[1]; - thread_arg.middle_pipe_count = PIPE_COUNT; - int writer = fd_pair1[1]; - int reader = fd_pair2[0]; - ASSERT_EQ(0, pthread_create(&thread, nullptr, - reinterpret_cast(FdEventThreadFunc), - &thread_arg)); + intermediates.resize(INTERMEDIATE_COUNT); + ASSERT_EQ(0, adb_socketpair(first)) << strerror(errno); + ASSERT_EQ(0, adb_socketpair(last)) << strerror(errno); + asocket* prev_tail = create_local_socket(first[1]); + ASSERT_NE(nullptr, prev_tail); + + auto connect = [](asocket* tail, asocket* head) { + tail->peer = head; + head->peer = tail; + tail->ready(tail); + }; + + for (auto& intermediate : intermediates) { + ASSERT_EQ(0, adb_socketpair(intermediate.data())) << strerror(errno); + + asocket* head = create_local_socket(intermediate[0]); + ASSERT_NE(nullptr, head); + + asocket* tail = create_local_socket(intermediate[1]); + ASSERT_NE(nullptr, tail); + + connect(prev_tail, head); + prev_tail = tail; + } + + asocket* end = create_local_socket(last[0]); + ASSERT_NE(nullptr, end); + connect(prev_tail, end); + + PrepareThread(); + adb_thread_t thread; + ASSERT_TRUE(adb_thread_create(FdEventThreadFunc, nullptr, &thread)); - usleep(1000); for (size_t i = 0; i < MESSAGE_LOOP_COUNT; ++i) { std::string read_buffer = MESSAGE; std::string write_buffer(MESSAGE.size(), 'a'); - ASSERT_TRUE(WriteFdExactly(writer, read_buffer.c_str(), read_buffer.size())); - ASSERT_TRUE(ReadFdExactly(reader, &write_buffer[0], write_buffer.size())); + ASSERT_TRUE(WriteFdExactly(first[0], &read_buffer[0], read_buffer.size())); + ASSERT_TRUE(ReadFdExactly(last[1], &write_buffer[0], write_buffer.size())); ASSERT_EQ(read_buffer, write_buffer); } - ASSERT_EQ(0, adb_close(writer)); - ASSERT_EQ(0, adb_close(reader)); - // Wait until the local sockets are closed. - sleep(1); - ASSERT_EQ(0, pthread_kill(thread, SIGUSR1)); - ASSERT_EQ(0, pthread_join(thread, nullptr)); + ASSERT_EQ(0, adb_close(first[0])); + ASSERT_EQ(0, adb_close(last[1])); + + // Wait until the local sockets are closed. + adb_sleep_ms(100); + TerminateThread(thread); } struct CloseWithPacketArg { @@ -160,7 +130,6 @@ static void CloseWithPacketThreadFunc(CloseWithPacketArg* arg) { s->peer = cause_close_s; cause_close_s->ready(cause_close_s); - InstallDummySocket(); fdevent_loop(); } @@ -176,21 +145,19 @@ TEST_F(LocalSocketTest, close_socket_with_packet) { CloseWithPacketArg arg; arg.socket_fd = socket_fd[1]; arg.cause_close_fd = cause_close_fd[1]; - pthread_t thread; - ASSERT_EQ(0, pthread_create(&thread, nullptr, - reinterpret_cast(CloseWithPacketThreadFunc), - &arg)); - // Wait until the fdevent_loop() starts. - sleep(1); - ASSERT_EQ(0, adb_close(cause_close_fd[0])); - 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)); + PrepareThread(); + adb_thread_t thread; + ASSERT_TRUE(adb_thread_create(reinterpret_cast(CloseWithPacketThreadFunc), + &arg, &thread)); + // Wait until the fdevent_loop() starts. + adb_sleep_ms(100); + ASSERT_EQ(0, adb_close(cause_close_fd[0])); + adb_sleep_ms(100); + EXPECT_EQ(2u, fdevent_installed_count()); + ASSERT_EQ(0, adb_close(socket_fd[0])); + + TerminateThread(thread); } // This test checks if we can read packets from a closing local socket. @@ -203,26 +170,23 @@ TEST_F(LocalSocketTest, read_from_closing_socket) { arg.socket_fd = socket_fd[1]; arg.cause_close_fd = cause_close_fd[1]; - pthread_t thread; - ASSERT_EQ(0, pthread_create(&thread, nullptr, - reinterpret_cast(CloseWithPacketThreadFunc), - &arg)); + PrepareThread(); + adb_thread_t thread; + ASSERT_TRUE(adb_thread_create(reinterpret_cast(CloseWithPacketThreadFunc), + &arg, &thread)); // Wait until the fdevent_loop() starts. - sleep(1); + adb_sleep_ms(100); ASSERT_EQ(0, adb_close(cause_close_fd[0])); - sleep(1); - ASSERT_EQ(2u, fdevent_installed_count()); + adb_sleep_ms(100); + EXPECT_EQ(2u, fdevent_installed_count()); // Verify if we can read successfully. std::vector buf(arg.bytes_written); + ASSERT_NE(0u, arg.bytes_written); ASSERT_EQ(true, ReadFdExactly(socket_fd[0], buf.data(), buf.size())); 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)); + TerminateThread(thread); } // This test checks if we can close local socket in the following situation: @@ -238,20 +202,17 @@ TEST_F(LocalSocketTest, write_error_when_having_packets) { arg.socket_fd = socket_fd[1]; arg.cause_close_fd = cause_close_fd[1]; - pthread_t thread; - ASSERT_EQ(0, pthread_create(&thread, nullptr, - reinterpret_cast(CloseWithPacketThreadFunc), - &arg)); + PrepareThread(); + adb_thread_t thread; + ASSERT_TRUE(adb_thread_create(reinterpret_cast(CloseWithPacketThreadFunc), + &arg, &thread)); + // Wait until the fdevent_loop() starts. - sleep(1); - ASSERT_EQ(3u, fdevent_installed_count()); + adb_sleep_ms(100); + EXPECT_EQ(3u, 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)); + TerminateThread(thread); } #if defined(__linux__) @@ -260,50 +221,52 @@ static void ClientThreadFunc() { std::string error; int fd = network_loopback_client(5038, SOCK_STREAM, &error); ASSERT_GE(fd, 0) << error; - sleep(2); + adb_sleep_ms(200); ASSERT_EQ(0, adb_close(fd)); } struct CloseRdHupSocketArg { - int socket_fd; + int socket_fd; }; static void CloseRdHupSocketThreadFunc(CloseRdHupSocketArg* arg) { - asocket* s = create_local_socket(arg->socket_fd); - ASSERT_TRUE(s != nullptr); + asocket* s = create_local_socket(arg->socket_fd); + ASSERT_TRUE(s != nullptr); - InstallDummySocket(); - fdevent_loop(); + fdevent_loop(); } // 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(ClientThreadFunc), nullptr)); + std::string error; + int listen_fd = network_inaddr_any_server(5038, SOCK_STREAM, &error); + ASSERT_GE(listen_fd, 0); - 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(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)); + adb_thread_t client_thread; + ASSERT_TRUE(adb_thread_create(reinterpret_cast(ClientThreadFunc), nullptr, + &client_thread)); + + 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; + + PrepareThread(); + adb_thread_t thread; + ASSERT_TRUE(adb_thread_create(reinterpret_cast(CloseRdHupSocketThreadFunc), + &arg, &thread)); + + // Wait until the fdevent_loop() starts. + adb_sleep_ms(100); + EXPECT_EQ(2u, fdevent_installed_count()); + + // Wait until the client closes its socket. + ASSERT_TRUE(adb_thread_join(client_thread)); + + TerminateThread(thread); } #endif // defined(__linux__) From cd2039e484e470a463ccf903feed373d10ab86e0 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 19 Feb 2016 10:42:40 -0800 Subject: [PATCH 16/20] adb: detect when the client disconnects in wait-for-device. Avoid leaking a thread and its associated resources when a user cancels wait-for-device. Bug: http://b/26966721 Bug: https://code.google.com/p/android/issues/detail?id=199088 Change-Id: Idac80a24e9739ddd24e500fe14826a78f350c018 (cherry picked from commit 09855472f421dd249ec2721ad255ffc15acab2c1) --- adb/services.cpp | 14 ++++++++++++-- adb/sysdeps_test.cpp | 25 +++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/adb/services.cpp b/adb/services.cpp index 75cbe5dd1..2eef1c265 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -49,6 +49,7 @@ #include "remount_service.h" #include "services.h" #include "shell_service.h" +#include "sysdeps.h" #include "transport.h" struct stinfo { @@ -369,12 +370,21 @@ static void wait_for_state(int fd, void* data) { std::string error = "unknown error"; const char* serial = sinfo->serial.length() ? sinfo->serial.c_str() : NULL; atransport* t = acquire_one_transport(sinfo->transport_type, serial, &is_ambiguous, &error); - if (t != nullptr && t->connection_state == sinfo->state) { SendOkay(fd); break; } else if (!is_ambiguous) { - adb_sleep_ms(1000); + adb_pollfd pfd = {.fd = fd, .events = POLLIN }; + int rc = adb_poll(&pfd, 1, 1000); + if (rc < 0) { + SendFail(fd, error); + break; + } else if (rc > 0 && (pfd.revents & POLLHUP) != 0) { + // The other end of the socket is closed, probably because the other side was + // terminated, bail out. + break; + } + // Try again... } else { SendFail(fd, error); diff --git a/adb/sysdeps_test.cpp b/adb/sysdeps_test.cpp index 253d62fc2..3904cc0b5 100644 --- a/adb/sysdeps_test.cpp +++ b/adb/sysdeps_test.cpp @@ -112,8 +112,12 @@ class sysdeps_poll : public ::testing::Test { } void TearDown() override { - ASSERT_EQ(0, adb_close(fds[0])); - ASSERT_EQ(0, adb_close(fds[1])); + if (fds[0] >= 0) { + ASSERT_EQ(0, adb_close(fds[0])); + } + if (fds[1] >= 0) { + ASSERT_EQ(0, adb_close(fds[1])); + } } }; @@ -190,3 +194,20 @@ TEST_F(sysdeps_poll, duplicate_fd) { EXPECT_EQ(POLLRDNORM, pfd[0].revents); EXPECT_EQ(POLLRDNORM, pfd[1].revents); } + +TEST_F(sysdeps_poll, disconnect) { + adb_pollfd pfd; + pfd.fd = fds[0]; + pfd.events = POLLIN; + + EXPECT_EQ(0, adb_poll(&pfd, 1, 0)); + EXPECT_EQ(0, pfd.revents); + + EXPECT_EQ(0, adb_close(fds[1])); + fds[1] = -1; + + EXPECT_EQ(1, adb_poll(&pfd, 1, 100)); + + // Linux returns POLLIN | POLLHUP, Windows returns just POLLHUP. + EXPECT_EQ(POLLHUP, pfd.revents & POLLHUP); +} From 8d68591c46716a855cca4c8af6c36656310f55dc Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 19 Feb 2016 13:41:33 -0800 Subject: [PATCH 17/20] adb: change unsigned to uint32_t in sync struct definitions. Change-Id: I9757ab853cfad1a2e1393ef32bcab222ab84acef (cherry picked from commit 69469c4e9ffac450f6807a06ca35edcbf2071071) --- adb/file_sync_service.h | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/adb/file_sync_service.h b/adb/file_sync_service.h index 38382c1ed..460e9dc03 100644 --- a/adb/file_sync_service.h +++ b/adb/file_sync_service.h @@ -41,25 +41,25 @@ struct SyncRequest { union syncmsg { struct __attribute__((packed)) { - unsigned id; - unsigned mode; - unsigned size; - unsigned time; + uint32_t id; + uint32_t mode; + uint32_t size; + uint32_t time; } stat; struct __attribute__((packed)) { - unsigned id; - unsigned mode; - unsigned size; - unsigned time; - unsigned namelen; + uint32_t id; + uint32_t mode; + uint32_t size; + uint32_t time; + uint32_t namelen; } dent; struct __attribute__((packed)) { - unsigned id; - unsigned size; + uint32_t id; + uint32_t size; } data; struct __attribute__((packed)) { - unsigned id; - unsigned msglen; + uint32_t id; + uint32_t msglen; } status; }; From 4d74811cd422438855cac67702e9535c43d502fe Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 19 Feb 2016 14:33:14 -0800 Subject: [PATCH 18/20] adbd: restore the old error handling behavior. Restore the previous file sync error handling behavior of reporting failure, and then consuming packets from the other end until receiving a DONE packet. Bug: http://b/26816782 Change-Id: I9708f2a36c072547e191fa0b6b42dffc31f8a2f2 (cherry picked from commit 20a96c7d792cf55c76b6291b46480e584face9bf) --- adb/file_sync_service.cpp | 55 +++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/adb/file_sync_service.cpp b/adb/file_sync_service.cpp index 29c662929..926dbcfa9 100644 --- a/adb/file_sync_service.cpp +++ b/adb/file_sync_service.cpp @@ -183,8 +183,6 @@ static bool handle_send_file(int s, const char* path, uid_t uid, } while (true) { - unsigned int len; - if (!ReadFdExactly(s, &msg.data, sizeof(msg.data))) goto fail; if (msg.data.id != ID_DATA) { @@ -193,17 +191,17 @@ static bool handle_send_file(int s, const char* path, uid_t uid, break; } SendSyncFail(s, "invalid data message"); - goto fail; + goto abort; } - len = msg.data.size; - if (len > buffer.size()) { // TODO: resize buffer? + + if (msg.data.size > buffer.size()) { // TODO: resize buffer? SendSyncFail(s, "oversize data message"); - goto fail; + goto abort; } - if (!ReadFdExactly(s, &buffer[0], len)) goto fail; + if (!ReadFdExactly(s, &buffer[0], msg.data.size)) goto abort; - if (!WriteFdExactly(fd, &buffer[0], len)) { + if (!WriteFdExactly(fd, &buffer[0], msg.data.size)) { SendSyncFailErrno(s, "write failed"); goto fail; } @@ -221,6 +219,35 @@ static bool handle_send_file(int s, const char* path, uid_t uid, return WriteFdExactly(s, &msg.status, sizeof(msg.status)); fail: + // If there's a problem on the device, we'll send an ID_FAIL message and + // close the socket. Unfortunately the kernel will sometimes throw that + // data away if the other end keeps writing without reading (which is + // the case with old versions of adb). To maintain compatibility, keep + // reading and throwing away ID_DATA packets until the other side notices + // that we've reported an error. + while (true) { + if (!ReadFdExactly(s, &msg.data, sizeof(msg.data))) goto fail; + + if (msg.data.id == ID_DONE) { + goto abort; + } else if (msg.data.id != ID_DATA) { + char id[5]; + memcpy(id, &msg.data.id, sizeof(msg.data.id)); + id[4] = '\0'; + D("handle_send_fail received unexpected id '%s' during failure", id); + goto abort; + } + + if (msg.data.size > buffer.size()) { + D("handle_send_fail received oversized packet of length '%u' during failure", + msg.data.size); + goto abort; + } + + if (!ReadFdExactly(s, &buffer[0], msg.data.size)) goto abort; + } + +abort: if (fd >= 0) adb_close(fd); if (do_unlink) adb_unlink(path); return false; @@ -403,18 +430,6 @@ static bool handle_sync_command(int fd, std::vector& buffer) { void file_sync_service(int fd, void* cookie) { std::vector buffer(SYNC_DATA_MAX); - // If there's a problem on the device, we'll send an ID_FAIL message and - // close the socket. Unfortunately the kernel will sometimes throw that - // data away if the other end keeps writing without reading (which is - // the normal case with our protocol --- they won't read until the end). - // So set SO_LINGER to give the client 20s to get around to reading our - // failure response. Without this, the other side's ability to report - // useful errors is reduced. - struct linger l; - l.l_onoff = 1; - l.l_linger = 20; - adb_setsockopt(fd, SOL_SOCKET, SO_LINGER, &l, sizeof(l)); - while (handle_sync_command(fd, buffer)) { } From da8119596fc6b9bb3d2208cc675036efac24fb6b Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 19 Feb 2016 15:55:55 -0800 Subject: [PATCH 19/20] adb: check for an error response from adbd between each write. When sending a file, do a 0-timeout poll to check to see if an error has occurred, so that we can immediately report failure. Bug: http://b/26816782 Change-Id: I4a8aa8408a36940bfda7b0ecfa5d13755f4aa14d (cherry picked from commit afcdcd703e3023dfc60638cf6b67b530ec18cb18) --- adb/file_sync_client.cpp | 31 ++++++++++++++++++++++++++++--- adb/test_device.py | 16 ++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/adb/file_sync_client.cpp b/adb/file_sync_client.cpp index 51fc1433d..85aaa6141 100644 --- a/adb/file_sync_client.cpp +++ b/adb/file_sync_client.cpp @@ -88,7 +88,8 @@ class SyncConnection { : total_bytes_(0), start_time_ms_(CurrentTimeMs()), expected_total_bytes_(0), - expect_multiple_files_(false) { + expect_multiple_files_(false), + expect_done_(false) { max = SYNC_DATA_MAX; // TODO: decide at runtime. std::string error; @@ -117,6 +118,16 @@ class SyncConnection { bool IsValid() { return fd >= 0; } + bool ReceivedError(const char* from, const char* to) { + adb_pollfd pfd = {.fd = fd, .events = POLLIN}; + int rc = adb_poll(&pfd, 1, 0); + if (rc < 0) { + Error("failed to poll: %s", strerror(errno)); + return true; + } + return rc != 0; + } + bool SendRequest(int id, const char* path_and_mode) { size_t path_length = strlen(path_and_mode); if (path_length > 1024) { @@ -175,6 +186,7 @@ class SyncConnection { p += sizeof(SyncRequest); WriteOrDie(lpath, rpath, &buf[0], (p - &buf[0])); + expect_done_ = true; total_bytes_ += data_length; return true; } @@ -220,6 +232,11 @@ class SyncConnection { total_bytes_ += bytes_read; bytes_copied += bytes_read; + // Check to see if we've received an error from the other side. + if (ReceivedError(lpath, rpath)) { + break; + } + ReportProgress(rpath, bytes_copied, total_size); } @@ -228,17 +245,24 @@ class SyncConnection { syncmsg msg; msg.data.id = ID_DONE; msg.data.size = mtime; + expect_done_ = true; return WriteOrDie(lpath, rpath, &msg.data, sizeof(msg.data)); } bool CopyDone(const char* from, const char* to) { syncmsg msg; if (!ReadFdExactly(fd, &msg.status, sizeof(msg.status))) { - Error("failed to copy '%s' to '%s': no ID_DONE: %s", from, to, strerror(errno)); + Error("failed to copy '%s' to '%s': couldn't read from device", from, to); return false; } if (msg.status.id == ID_OKAY) { - return true; + if (expect_done_) { + expect_done_ = false; + return true; + } else { + Error("failed to copy '%s' to '%s': received premature success", from, to); + return true; + } } if (msg.status.id != ID_FAIL) { Error("failed to copy '%s' to '%s': unknown reason %d", from, to, msg.status.id); @@ -357,6 +381,7 @@ class SyncConnection { uint64_t expected_total_bytes_; bool expect_multiple_files_; + bool expect_done_; LinePrinter line_printer_; diff --git a/adb/test_device.py b/adb/test_device.py index afc061a60..18174a2ae 100644 --- a/adb/test_device.py +++ b/adb/test_device.py @@ -766,6 +766,22 @@ class FileOperationsTest(DeviceTest): if host_dir is not None: shutil.rmtree(host_dir) + @requires_non_root + def test_push_error_reporting(self): + """Make sure that errors that occur while pushing a file get reported + + Bug: http://b/26816782 + """ + with tempfile.NamedTemporaryFile() as tmp_file: + tmp_file.write('\0' * 1024 * 1024) + tmp_file.flush() + try: + self.device.push(local=tmp_file.name, remote='/system/') + self.fail("push should not have succeeded") + except subprocess.CalledProcessError as e: + output = e.output + + self.assertIn("Permission denied", output) def _test_pull(self, remote_file, checksum): tmp_write = tempfile.NamedTemporaryFile(mode='wb', delete=False) From fa3e0bc67fa336e23e8fb44b0e1f3c030ad55e48 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 19 Feb 2016 18:14:20 -0800 Subject: [PATCH 20/20] adb: sysdeps_test: improve smoke test. Make sure that adb_poll sets revents for all of the structs passed in. Also, zero initialize all of the adb_pollfd structs in the tests. Change-Id: Ia639679a7e6f77483655f1552e89081c4673aa87 (cherry picked from commit 2275f7da7370d5b403a5d4a9d558c34a38ee5c94) --- adb/sysdeps_test.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/adb/sysdeps_test.cpp b/adb/sysdeps_test.cpp index 3904cc0b5..78efea8ad 100644 --- a/adb/sysdeps_test.cpp +++ b/adb/sysdeps_test.cpp @@ -122,12 +122,14 @@ class sysdeps_poll : public ::testing::Test { }; TEST_F(sysdeps_poll, smoke) { - adb_pollfd pfd[2]; + adb_pollfd pfd[2] = {}; pfd[0].fd = fds[0]; pfd[0].events = POLLRDNORM; pfd[1].fd = fds[1]; pfd[1].events = POLLWRNORM; + pfd[0].revents = -1; + pfd[1].revents = -1; EXPECT_EQ(1, adb_poll(pfd, 2, 0)); EXPECT_EQ(0, pfd[0].revents); EXPECT_EQ(POLLWRNORM, pfd[1].revents); @@ -135,16 +137,18 @@ TEST_F(sysdeps_poll, smoke) { ASSERT_TRUE(WriteFdExactly(fds[1], "foo", 4)); // Wait for the socketpair to be flushed. + pfd[0].revents = -1; EXPECT_EQ(1, adb_poll(pfd, 1, 100)); EXPECT_EQ(POLLRDNORM, pfd[0].revents); - + pfd[0].revents = -1; + pfd[1].revents = -1; EXPECT_EQ(2, adb_poll(pfd, 2, 0)); EXPECT_EQ(POLLRDNORM, pfd[0].revents); EXPECT_EQ(POLLWRNORM, pfd[1].revents); } TEST_F(sysdeps_poll, timeout) { - adb_pollfd pfd; + adb_pollfd pfd = {}; pfd.fd = fds[0]; pfd.events = POLLRDNORM; @@ -158,7 +162,7 @@ TEST_F(sysdeps_poll, timeout) { } TEST_F(sysdeps_poll, invalid_fd) { - adb_pollfd pfd[3]; + adb_pollfd pfd[3] = {}; pfd[0].fd = fds[0]; pfd[0].events = POLLRDNORM; pfd[1].fd = INT_MAX; @@ -179,7 +183,7 @@ TEST_F(sysdeps_poll, invalid_fd) { } TEST_F(sysdeps_poll, duplicate_fd) { - adb_pollfd pfd[2]; + adb_pollfd pfd[2] = {}; pfd[0].fd = fds[0]; pfd[0].events = POLLRDNORM; pfd[1] = pfd[0]; @@ -196,7 +200,7 @@ TEST_F(sysdeps_poll, duplicate_fd) { } TEST_F(sysdeps_poll, disconnect) { - adb_pollfd pfd; + adb_pollfd pfd = {}; pfd.fd = fds[0]; pfd.events = POLLIN;