From 09855472f421dd249ec2721ad255ffc15acab2c1 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 19 Feb 2016 10:42:40 -0800 Subject: [PATCH 1/2] 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 --- 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 69469c4e9ffac450f6807a06ca35edcbf2071071 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 19 Feb 2016 13:41:33 -0800 Subject: [PATCH 2/2] adb: change unsigned to uint32_t in sync struct definitions. Change-Id: I9757ab853cfad1a2e1393ef32bcab222ab84acef --- 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; };