commit c9c94a758d0f73e208634159be9302594a9a6e07
author Josh Gao <jmgao@google.com> Tue May 30 15:47:05 2017 -0700
committer Josh Gao <jmgao@google.com> Wed Jun 07 14:11:28 2017 -0700
tree 174d6c7ea5ea52c72829f662a2bb32b2d5ca2c51
parent cd5f59f4144feac3db708270cf77d1dcdf889d7a

crash_dump: defer pausing threads until we're ready.

Don't pause the threads we're going to dump until after we're about to
fetch their backtraces.

Bug: http://b/62112103
Test: debuggerd_test
Change-Id: Id7ab0464842b35f98f3b3ebc42fb76161d8afbd2

debuggerd/Android.bp

diff --git a/debuggerd/Android.bp b/debuggerd/Android.bp
index 5565cfd..42c3023 100644
--- a/debuggerd/Android.bp
+++ b/debuggerd/Android.bp
@@ -7,6 +7,7 @@
         "-Wno-nullability-completeness",
         "-Os",
     ],
+    cpp_std: "experimental",
 
     local_include_dirs: ["include"],
 }

debuggerd/crash_dump.cpp

diff --git a/debuggerd/crash_dump.cpp b/debuggerd/crash_dump.cpp
index df7201d..3ca9c92 100644
--- a/debuggerd/crash_dump.cpp
+++ b/debuggerd/crash_dump.cpp
@@ -95,11 +95,6 @@
     return false;
   }
 
-  // Put the task into ptrace-stop state.
-  if (ptrace(PTRACE_INTERRUPT, tid, 0, 0) != 0) {
-    PLOG(FATAL) << "failed to interrupt thread " << tid;
-  }
-
   return true;
 }
 
@@ -284,36 +279,38 @@
   // Die if we take too long.
   alarm(2);
 
+  std::string process_name = get_process_name(main_tid);
   std::string attach_error;
 
   std::map<pid_t, std::string> threads;
 
   {
-    ATRACE_NAME("ptrace");
+    ATRACE_NAME("ptrace_interrupt");
+
     // Seize the main thread.
     if (!ptrace_seize_thread(target_proc_fd, main_tid, &attach_error)) {
       LOG(FATAL) << attach_error;
     }
 
-    // Seize the siblings.
-    {
-      std::set<pid_t> siblings;
-      if (!android::procinfo::GetProcessTids(target, &siblings)) {
-        PLOG(FATAL) << "failed to get process siblings";
-      }
+    threads.emplace(main_tid, get_thread_name(main_tid));
 
-      // but not the already attached main thread.
-      siblings.erase(main_tid);
-      // or the handler pseudothread.
-      siblings.erase(pseudothread_tid);
+    // Seize its siblings.
+    std::set<pid_t> siblings;
+    if (!android::procinfo::GetProcessTids(target, &siblings)) {
+      PLOG(FATAL) << "failed to get process siblings";
+    }
 
-      for (pid_t sibling_tid : siblings) {
-        if (!ptrace_seize_thread(target_proc_fd, sibling_tid, &attach_error)) {
-          LOG(WARNING) << attach_error;
-        } else {
-          threads.emplace(sibling_tid, get_thread_name(sibling_tid));
-        }
+    // but not the already attached main thread.
+    siblings.erase(main_tid);
+    // or the handler pseudothread.
+    siblings.erase(pseudothread_tid);
+
+    for (pid_t sibling_tid : siblings) {
+      if (!ptrace_seize_thread(target_proc_fd, sibling_tid, &attach_error)) {
+        LOG(WARNING) << attach_error;
+        continue;
       }
+      threads.emplace(sibling_tid, get_thread_name(sibling_tid));
     }
   }
 
@@ -334,9 +331,6 @@
     populate_open_files_list(target, &open_files);
   }
 
-  std::string process_name = get_process_name(main_tid);
-  threads.emplace(main_tid, get_thread_name(main_tid));
-
   // Drop our capabilities now that we've attached to the threads we care about.
   drop_capabilities();
 
@@ -347,6 +341,16 @@
     tombstoned_connected = tombstoned_connect(target, &tombstoned_socket, &output_fd, dump_type_enum);
   }
 
+  // Pause the threads.
+  {
+    ATRACE_NAME("ptrace_interrupt");
+    for (const auto& [sibling_tid, _] : threads) {
+      if (ptrace(PTRACE_INTERRUPT, sibling_tid, 0, 0) != 0) {
+        PLOG(FATAL) << "failed to interrupt thread " << sibling_tid;
+      }
+    }
+  }
+
   // Write a '\1' to stdout to tell the crashing process to resume.
   // It also restores the value of PR_SET_DUMPABLE at this point.
   if (TEMP_FAILURE_RETRY(write(STDOUT_FILENO, "\1", 1)) == -1) {