Add retry logic for FindThreadIDWithSyscall()

components/crash/content/browser/crash_handler_host_linux.cc

Index: components/crash/content/browser/crash_handler_host_linux.cc
diff --git a/components/crash/content/browser/crash_handler_host_linux.cc b/components/crash/content/browser/crash_handler_host_linux.cc
index 393c047450e04e13dd57502f640e152e922269fb..12ebaf97f15f8ec6bfb24bab3da115a7deb2596f 100644
--- a/components/crash/content/browser/crash_handler_host_linux.cc
+++ b/components/crash/content/browser/crash_handler_host_linux.cc
@@ -27,6 +27,7 @@
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "base/threading/platform_thread.h"
 #include "base/threading/thread.h"
 #include "breakpad/src/client/linux/handler/exception_handler.h"
 #include "breakpad/src/client/linux/minidump_writer/linux_dumper.h"
@@ -55,6 +56,12 @@ const size_t kControlMsgSize =
 // The length of the regular payload:
 const size_t kCrashContextSize = sizeof(ExceptionHandler::CrashContext);
 
+// Crashing thread might be in "running" state, i.e. after sys_sendmsg() and
+// before sys_read(). Retry 3 times with interval of 100 ms when translating
+// TID.
+const int kNumRetriesTranslatingTid = 3;
+const int kRetryIntervalTranslatingTidInMs = 100;
+
 // Handles the crash dump and frees the allocated BreakpadInfo struct.
 void CrashDumpTask(CrashHandlerHostLinux* handler,
                    std::unique_ptr<BreakpadInfo> info) {
@@ -281,10 +288,17 @@ void CrashHandlerHostLinux::OnFileCanReadWithoutBlocking(int fd) {
   base::StringAppendF(&expected_syscall_data, "%d 0x%x %p 0x1 ",
                       SYS_read, tid_fd, tid_buf_addr);
   bool syscall_supported = false;
-  pid_t crashing_tid =
-      base::FindThreadIDWithSyscall(crashing_pid,
-                                    expected_syscall_data,
-                                    &syscall_supported);
+  pid_t crashing_tid = -1;
+  for (int i = 0; i < kNumRetriesTranslatingTid; ++i) {
+    crashing_tid = base::FindThreadIDWithSyscall(crashing_pid,
+                                                 expected_syscall_data,
+                                                 &syscall_supported);

[Lei Zhang, 2016/07/22 23:37:36]

If |syscall_supported| comes back false, this should not continue looping,
because it will continue to fail. Though I imagine it should return true on most
modern Linux machines.

[wzhong, 2016/07/23 00:25:11]

Acknowledged.

[wzhong, 2016/07/25 15:12:43]

Done.

+    if (crashing_tid != -1)
+      break;
+
+    base::PlatformThread::Sleep(

[Lei Zhang, 2016/07/22 23:37:36]

It's not good to sleep on the IO (AKA IPC) thread because the browser can't
process any other IPCs while waiting here. Is it possible to refactor the code
to capture the current function state and retry via PostDelayedTask() to the
current thread?

[wzhong, 2016/07/23 00:25:11]

Good point. It should be doable although context needs to be passed around.

How about PlatformThread::YieldCurrentThread()? Seems a little bit of yield or
forced scheduling already allows the crashing thread to catch up.

[wzhong, 2016/07/23 00:30:11]

Actually I'd still prefer PostDelayedTask().

[wzhong, 2016/07/25 15:12:43]

Done.

+        base::TimeDelta::FromMilliseconds(kRetryIntervalTranslatingTidInMs));
+  }
   if (crashing_tid == -1) {
     // We didn't find the thread we want. Maybe it didn't reach
     // sys_read() yet or the thread went away.  We'll just take a