From 172d0d44bc01669d79b769cc37b9a6868709e048 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 18 Jun 2018 14:51:56 -0700 Subject: [PATCH 1/4] adb: add `adb raw` to connect to an arbitrary service. Add a command to connect to an arbitrary service, for debugging purposes. Test: `adb raw shell:ls` Change-Id: I69d4d585e5ecfa7cb8c7a543a2a27df7033b26c7 --- adb/client/commandline.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/adb/client/commandline.cpp b/adb/client/commandline.cpp index 06fcf1639..e07dba7cf 100644 --- a/adb/client/commandline.cpp +++ b/adb/client/commandline.cpp @@ -1795,6 +1795,11 @@ int adb_commandline(int argc, const char** argv) { } else if (!strcmp(argv[0], "track-devices")) { return adb_connect_command("host:track-devices"); + } else if (!strcmp(argv[0], "raw")) { + if (argc != 2) { + return syntax_error("adb raw SERVICE"); + } + return adb_connect_command(argv[1]); } From e44d38d68042afc0cd14ee71e18b2300f2e618b6 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 18 Jun 2018 14:54:40 -0700 Subject: [PATCH 2/4] adb: delete unused members in fdevent. Forgot to clean this up after removing fdevent_install... Test: treehugger Change-Id: I53d21134a4bc8bf7b16210318c6fac5075445b39 --- adb/fdevent.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/adb/fdevent.h b/adb/fdevent.h index 69c40721b..39fa9c262 100644 --- a/adb/fdevent.h +++ b/adb/fdevent.h @@ -32,9 +32,6 @@ typedef void (*fd_func)(int fd, unsigned events, void *userdata); struct fdevent { - fdevent* next = nullptr; - fdevent* prev = nullptr; - unique_fd fd; int force_eof = 0; From ded557fa58ec2c7c2f2bea8b71d75afe21c37168 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 18 Jun 2018 14:55:27 -0700 Subject: [PATCH 3/4] adb: add an id field to fdevent. Tracking fdevents by pointer is dangerous because an fdevent can be destroyed and recreated at the same address. Add a monotonically increasing id field to fdevent for this purpose. Test: treehugger Change-Id: I706b57a9e669290ef2db258f85489a5155fc1152 --- adb/fdevent.cpp | 7 ++++++- adb/fdevent.h | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/adb/fdevent.cpp b/adb/fdevent.cpp index 1b7758c57..098a39dc1 100644 --- a/adb/fdevent.cpp +++ b/adb/fdevent.cpp @@ -21,6 +21,7 @@ #include "fdevent.h" #include +#include #include #include #include @@ -75,6 +76,8 @@ static std::atomic terminate_loop(false); static bool main_thread_valid; static uint64_t main_thread_id; +static uint64_t fdevent_id; + static bool run_needs_flush = false; static auto& run_queue_notify_fd = *new unique_fd(); static auto& run_queue_mutex = *new std::mutex(); @@ -111,7 +114,8 @@ static std::string dump_fde(const fdevent* fde) { if (fde->state & FDE_ERROR) { state += "E"; } - return android::base::StringPrintf("(fdevent %d %s)", fde->fd.get(), state.c_str()); + return android::base::StringPrintf("(fdevent %" PRIu64 ": fd %d %s)", fde->id, fde->fd.get(), + state.c_str()); } void fdevent_install(fdevent* fde, int fd, fd_func func, void* arg) { @@ -125,6 +129,7 @@ fdevent* fdevent_create(int fd, fd_func func, void* arg) { CHECK_GE(fd, 0); fdevent* fde = new fdevent(); + fde->id = fdevent_id++; fde->state = FDE_ACTIVE; fde->fd.reset(fd); fde->func = func; diff --git a/adb/fdevent.h b/adb/fdevent.h index 39fa9c262..d501b8660 100644 --- a/adb/fdevent.h +++ b/adb/fdevent.h @@ -32,6 +32,8 @@ typedef void (*fd_func)(int fd, unsigned events, void *userdata); struct fdevent { + uint64_t id; + unique_fd fd; int force_eof = 0; From 3bbc8164b12f06c819a5b2acdc525199a9ff6f3e Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 18 Jun 2018 16:37:01 -0700 Subject: [PATCH 4/4] adb: detect some spin loops and abort. Track pending fdevents, and abort if we see an fdevent ready for read/write for 5 minutes continuously. Also, add a service that intentionally starts spinning, to test this. Test: manually changed timeout to a minute, `adb raw spin` Change-Id: Ibfa5e7e654996587f745887cb04987b982d79bed --- adb/fdevent.cpp | 49 ++++++++++++++++++++++++++++++++++++++++++++++++ adb/services.cpp | 25 ++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/adb/fdevent.cpp b/adb/fdevent.cpp index 098a39dc1..f98c11a2f 100644 --- a/adb/fdevent.cpp +++ b/adb/fdevent.cpp @@ -35,6 +35,7 @@ #include #include +#include #include #include #include @@ -357,10 +358,56 @@ void fdevent_run_on_main_thread(std::function fn) { } } +static void fdevent_check_spin(uint64_t cycle) { + // Check to see if we're spinning because we forgot about an fdevent + // by keeping track of how long fdevents have been continuously pending. + struct SpinCheck { + fdevent* fde; + std::chrono::steady_clock::time_point timestamp; + uint64_t cycle; + }; + static auto& g_continuously_pending = *new std::unordered_map(); + + auto now = std::chrono::steady_clock::now(); + for (auto* fde : g_pending_list) { + auto it = g_continuously_pending.find(fde->id); + if (it == g_continuously_pending.end()) { + g_continuously_pending[fde->id] = + SpinCheck{.fde = fde, .timestamp = now, .cycle = cycle}; + } else { + it->second.cycle = cycle; + } + } + + for (auto it = g_continuously_pending.begin(); it != g_continuously_pending.end();) { + if (it->second.cycle != cycle) { + it = g_continuously_pending.erase(it); + } else { + // Use an absurdly long window, since all we really care about is + // getting a bugreport eventually. + if (now - it->second.timestamp > std::chrono::minutes(5)) { + LOG(FATAL_WITHOUT_ABORT) << "detected spin in fdevent: " << dump_fde(it->second.fde); +#if defined(__linux__) + int fd = it->second.fde->fd.get(); + std::string fd_path = android::base::StringPrintf("/proc/self/fd/%d", fd); + std::string path; + if (!android::base::Readlink(fd_path, &path)) { + PLOG(FATAL_WITHOUT_ABORT) << "readlink of fd " << fd << " failed"; + } + LOG(FATAL_WITHOUT_ABORT) << "fd " << fd << " = " << path; +#endif + abort(); + } + ++it; + } + } +} + void fdevent_loop() { set_main_thread(); fdevent_run_setup(); + uint64_t cycle = 0; while (true) { if (terminate_loop) { return; @@ -370,6 +417,8 @@ void fdevent_loop() { fdevent_process(); + fdevent_check_spin(cycle++); + while (!g_pending_list.empty()) { fdevent* fde = g_pending_list.front(); g_pending_list.pop_front(); diff --git a/adb/services.cpp b/adb/services.cpp index a757d9066..b613d8312 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -181,6 +181,29 @@ static void reconnect_service(int fd, void* arg) { kick_transport(t); } +static void spin_service(int fd, void*) { + unique_fd sfd(fd); + + if (!__android_log_is_debuggable()) { + WriteFdExactly(sfd.get(), "refusing to spin on non-debuggable build\n"); + return; + } + + // A service that creates an fdevent that's always pending, and then ignores it. + unique_fd pipe_read, pipe_write; + if (!Pipe(&pipe_read, &pipe_write)) { + WriteFdExactly(sfd.get(), "failed to create pipe\n"); + return; + } + + fdevent_run_on_main_thread([fd = pipe_read.release()]() { + fdevent* fde = fdevent_create(fd, [](int, unsigned, void*) {}, nullptr); + fdevent_add(fde, FDE_READ); + }); + + WriteFdExactly(sfd.get(), "spinning\n"); +} + int reverse_service(const char* command, atransport* transport) { int s[2]; if (adb_socketpair(s)) { @@ -328,6 +351,8 @@ int service_to_fd(const char* name, atransport* transport) { reinterpret_cast(1)); } else if (!strcmp(name, "reconnect")) { ret = create_service_thread("reconnect", reconnect_service, transport); + } else if (!strcmp(name, "spin")) { + ret = create_service_thread("spin", spin_service, nullptr); #endif } if (ret >= 0) {