diff --git a/adb/socket_test.cpp b/adb/socket_test.cpp index 6b400565f..f587fdb30 100644 --- a/adb/socket_test.cpp +++ b/adb/socket_test.cpp @@ -42,7 +42,9 @@ struct ThreadArg { class LocalSocketTest : public FdeventTest {}; -constexpr auto SLEEP_FOR_FDEVENT = 100ms; +static void WaitForFdeventLoop() { + std::this_thread::sleep_for(100ms); +} TEST_F(LocalSocketTest, smoke) { // Join two socketpairs with a chain of intermediate socketpairs. @@ -98,7 +100,7 @@ TEST_F(LocalSocketTest, smoke) { ASSERT_EQ(0, adb_close(last[1])); // Wait until the local sockets are closed. - std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); + WaitForFdeventLoop(); ASSERT_EQ(GetAdditionalLocalSocketCount(), fdevent_installed_count()); TerminateThread(thread); } @@ -144,13 +146,15 @@ TEST_F(LocalSocketTest, close_socket_with_packet) { PrepareThread(); std::thread thread(CloseWithPacketThreadFunc, &arg); - // Wait until the fdevent_loop() starts. - std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); + + WaitForFdeventLoop(); ASSERT_EQ(0, adb_close(cause_close_fd[0])); - std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); + + WaitForFdeventLoop(); EXPECT_EQ(1u + GetAdditionalLocalSocketCount(), fdevent_installed_count()); ASSERT_EQ(0, adb_close(socket_fd[0])); - std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); + + WaitForFdeventLoop(); ASSERT_EQ(GetAdditionalLocalSocketCount(), fdevent_installed_count()); TerminateThread(thread); } @@ -167,10 +171,11 @@ TEST_F(LocalSocketTest, read_from_closing_socket) { PrepareThread(); std::thread thread(CloseWithPacketThreadFunc, &arg); - // Wait until the fdevent_loop() starts. - std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); + + WaitForFdeventLoop(); ASSERT_EQ(0, adb_close(cause_close_fd[0])); - std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); + + WaitForFdeventLoop(); EXPECT_EQ(1u + GetAdditionalLocalSocketCount(), fdevent_installed_count()); // Verify if we can read successfully. @@ -179,7 +184,7 @@ TEST_F(LocalSocketTest, read_from_closing_socket) { ASSERT_EQ(true, ReadFdExactly(socket_fd[0], buf.data(), buf.size())); ASSERT_EQ(0, adb_close(socket_fd[0])); - std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); + WaitForFdeventLoop(); ASSERT_EQ(GetAdditionalLocalSocketCount(), fdevent_installed_count()); TerminateThread(thread); } @@ -199,12 +204,12 @@ TEST_F(LocalSocketTest, write_error_when_having_packets) { PrepareThread(); std::thread thread(CloseWithPacketThreadFunc, &arg); - // Wait until the fdevent_loop() starts. - std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); + + WaitForFdeventLoop(); EXPECT_EQ(2u + GetAdditionalLocalSocketCount(), fdevent_installed_count()); ASSERT_EQ(0, adb_close(socket_fd[0])); - std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); + WaitForFdeventLoop(); ASSERT_EQ(GetAdditionalLocalSocketCount(), fdevent_installed_count()); TerminateThread(thread); } @@ -242,8 +247,7 @@ TEST_F(LocalSocketTest, flush_after_shutdown) { adb_close(head_fd[0]); adb_close(tail_fd[0]); - // Wait until the local sockets are closed. - std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); + WaitForFdeventLoop(); ASSERT_EQ(GetAdditionalLocalSocketCount(), fdevent_installed_count()); TerminateThread(thread); } @@ -285,14 +289,13 @@ TEST_F(LocalSocketTest, close_socket_in_CLOSE_WAIT_state) { PrepareThread(); std::thread thread(CloseRdHupSocketThreadFunc, &arg); - // Wait until the fdevent_loop() starts. - std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); + WaitForFdeventLoop(); EXPECT_EQ(1u + GetAdditionalLocalSocketCount(), fdevent_installed_count()); // Wait until the client closes its socket. client_thread.join(); - std::this_thread::sleep_for(SLEEP_FOR_FDEVENT); + WaitForFdeventLoop(); ASSERT_EQ(GetAdditionalLocalSocketCount(), fdevent_installed_count()); TerminateThread(thread); } diff --git a/adb/sysdeps_test.cpp b/adb/sysdeps_test.cpp index fd1988252..79cebe6b5 100644 --- a/adb/sysdeps_test.cpp +++ b/adb/sysdeps_test.cpp @@ -121,10 +121,13 @@ TEST_F(sysdeps_poll, invalid_fd) { adb_pollfd pfd[3] = {}; pfd[0].fd = fds[0]; pfd[0].events = POLLRDNORM; + pfd[0].revents = ~0; pfd[1].fd = INT_MAX; pfd[1].events = POLLRDNORM; + pfd[1].revents = ~0; pfd[2].fd = fds[1]; pfd[2].events = POLLWRNORM; + pfd[2].revents = ~0; ASSERT_TRUE(WriteFdExactly(fds[1], "foo", 4)); @@ -136,6 +139,17 @@ TEST_F(sysdeps_poll, invalid_fd) { EXPECT_EQ(POLLRDNORM, pfd[0].revents); EXPECT_EQ(POLLNVAL, pfd[1].revents); EXPECT_EQ(POLLWRNORM, pfd[2].revents); + + // Make sure that we return immediately if an invalid FD is given. + pfd[0].fd = fds[0]; + pfd[0].events = POLLRDNORM; + pfd[0].revents = ~0; + pfd[1].fd = INT_MAX; + pfd[1].events = POLLRDNORM; + pfd[1].revents = ~0; + EXPECT_EQ(2, adb_poll(pfd, 2, -1)); + EXPECT_EQ(POLLRDNORM, pfd[0].revents); + EXPECT_EQ(POLLNVAL, pfd[1].revents); } TEST_F(sysdeps_poll, duplicate_fd) { diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp index 62f4ac831..7d35fb67b 100644 --- a/adb/sysdeps_win32.cpp +++ b/adb/sysdeps_win32.cpp @@ -529,6 +529,7 @@ extern int adb_poll(adb_pollfd* fds, size_t nfds, int timeout) { 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) { @@ -549,6 +550,11 @@ extern int adb_poll(adb_pollfd* fds, size_t nfds, int timeout) { return skipped; } + // If we have any invalid FDs in our FD set, make sure to return immediately. + if (skipped > 0) { + timeout = 0; + } + int result = WSAPoll(sockets.data(), sockets.size(), timeout); if (result == SOCKET_ERROR) { _socket_set_errno(WSAGetLastError()); @@ -560,7 +566,7 @@ extern int adb_poll(adb_pollfd* fds, size_t nfds, int timeout) { original[i]->revents = sockets[i].revents; } - // WSAPoll appears to return the number of unique FDs with avaiable events, instead of how many + // WSAPoll appears to return the number of unique FDs with available 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;