debuggerd_client: fix infinite timeouts.

Bug: http://b/36897117
Test: debuggerd_test
Test: debuggerd -b `pidof zygote`
Change-Id: I743433bc420b1c433b82ef62795abb6ea9f5d840

3 files changed, 34 insertions, 9 deletions

diff --git a/debuggerd/client/debuggerd_client.cpp b/debuggerd/client/debuggerd_client.cpp
index 224444f1f..3b8485362 100644
--- a/debuggerd/client/debuggerd_client.cpp
+++ b/debuggerd/client/debuggerd_client.cpp
@@ -58,28 +58,31 @@ static void populate_timeval(struct timeval* tv, const Duration& duration) {
 }
 
 bool debuggerd_trigger_dump(pid_t pid, unique_fd output_fd, DebuggerdDumpType dump_type,
-                            int timeout_ms) {
+                            unsigned int timeout_ms) {
   LOG(INFO) << "libdebuggerd_client: started dumping process " << pid;
   unique_fd sockfd;
   const auto end = std::chrono::steady_clock::now() + std::chrono::milliseconds(timeout_ms);
-  auto time_left = [timeout_ms, &end]() { return end - std::chrono::steady_clock::now(); };
+  auto time_left = [&end]() { return end - std::chrono::steady_clock::now(); };
   auto set_timeout = [timeout_ms, &time_left](int sockfd) {
     if (timeout_ms <= 0) {
-      return -1;
+      return sockfd;
     }
 
     auto remaining = time_left();
     if (remaining < decltype(remaining)::zero()) {
-      LOG(ERROR) << "timeout expired";
+      LOG(ERROR) << "libdebuggerd_client: timeout expired";
       return -1;
     }
+
     struct timeval timeout;
     populate_timeval(&timeout, remaining);
 
     if (setsockopt(sockfd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeout)) != 0) {
+      PLOG(ERROR) << "libdebuggerd_client: failed to set receive timeout";
       return -1;
     }
     if (setsockopt(sockfd, SOL_SOCKET, SO_SNDTIMEO, &timeout, sizeof(timeout)) != 0) {
+      PLOG(ERROR) << "libdebuggerd_client: failed to set send timeout";
       return -1;
     }
 
@@ -158,8 +161,10 @@ bool debuggerd_trigger_dump(pid_t pid, unique_fd output_fd, DebuggerdDumpType du
 
   // Forward output from the pipe to the output fd.
   while (true) {
-    auto remaining_ms = std::chrono::duration_cast<std::chrono::milliseconds>(time_left());
-    if (remaining_ms <= 1ms) {
+    auto remaining_ms = std::chrono::duration_cast<std::chrono::milliseconds>(time_left()).count();
+    if (timeout_ms <= 0) {
+      remaining_ms = -1;
+    } else if (remaining_ms < 0) {
       LOG(ERROR) << "libdebuggerd_client: timeout expired";
       return false;
     }
@@ -168,7 +173,7 @@ bool debuggerd_trigger_dump(pid_t pid, unique_fd output_fd, DebuggerdDumpType du
         .fd = pipe_read.get(), .events = POLLIN, .revents = 0,
     };
 
-    rc = poll(&pfd, 1, remaining_ms.count());
+    rc = poll(&pfd, 1, remaining_ms);
     if (rc == -1) {
       if (errno == EINTR) {
         continue;
diff --git a/debuggerd/client/debuggerd_client_test.cpp b/debuggerd/client/debuggerd_client_test.cpp
index 86d03149b..aff03e598 100644
--- a/debuggerd/client/debuggerd_client_test.cpp
+++ b/debuggerd/client/debuggerd_client_test.cpp
@@ -89,3 +89,23 @@ TEST(debuggerd_client, race) {
 
   EXPECT_EQ(1, found_end) << "\nOutput: \n" << result;
 }
+
+TEST(debuggerd_client, no_timeout) {
+  unique_fd pipe_read, pipe_write;
+  ASSERT_TRUE(Pipe(&pipe_read, &pipe_write));
+
+  pid_t forkpid = fork();
+  ASSERT_NE(-1, forkpid);
+  if (forkpid == 0) {
+    pipe_write.reset();
+    char dummy;
+    TEMP_FAILURE_RETRY(read(pipe_read.get(), &dummy, sizeof(dummy)));
+    exit(0);
+  }
+
+  pipe_read.reset();
+
+  unique_fd output_read, output_write;
+  ASSERT_TRUE(Pipe(&output_read, &output_write));
+  ASSERT_TRUE(debuggerd_trigger_dump(forkpid, std::move(output_write), kDebuggerdBacktrace, 0));
+}
diff --git a/debuggerd/include/debuggerd/client.h b/debuggerd/include/debuggerd/client.h
index 91f143ba5..01de57b40 100644
--- a/debuggerd/include/debuggerd/client.h
+++ b/debuggerd/include/debuggerd/client.h
@@ -28,9 +28,9 @@ enum DebuggerdDumpType {
 };
 
 // Trigger a dump of specified process to output_fd.
-// output_fd is *not* consumed, timeouts <= 0 will wait forever.
+// output_fd is consumed, timeout of 0 will wait forever.
 bool debuggerd_trigger_dump(pid_t pid, android::base::unique_fd output_fd,
-                            enum DebuggerdDumpType dump_type, int timeout_ms);
+                            enum DebuggerdDumpType dump_type, unsigned int timeout_ms);
 
 int dump_backtrace_to_file(pid_t tid, int fd);
 int dump_backtrace_to_file_timeout(pid_t tid, int fd, int timeout_secs);