Identify the hung thread using the Wait Chain Traversal API

chrome/chrome_watcher/chrome_watcher_main.cc

Index: chrome/chrome_watcher/chrome_watcher_main.cc
diff --git a/chrome/chrome_watcher/chrome_watcher_main.cc b/chrome/chrome_watcher/chrome_watcher_main.cc
index 1976c7ae099e09150b70fcb2ba2f9a86afdb4df5..25c337b2b5769d17e8f6bf40120fe284d1733d65 100644
--- a/chrome/chrome_watcher/chrome_watcher_main.cc
+++ b/chrome/chrome_watcher/chrome_watcher_main.cc
@@ -25,6 +25,7 @@
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/thread_task_runner_handle.h"
@@ -41,6 +42,7 @@
 #include "third_party/kasko/kasko_features.h"
 
 #if BUILDFLAG(ENABLE_KASKO)
+#include "chrome/chrome_watcher/wait_chain_util_win.h"
 #include "components/crash/content/app/crashpad.h"
 #include "syzygy/kasko/api/reporter.h"
 #endif
@@ -232,6 +234,19 @@ void GetKaskoCrashReportsBaseDir(const base::char16* browser_data_directory,
   }
 }
 
+void AddCrashKey(const wchar_t* key,
+                 const wchar_t* value,
+                 std::vector<kasko::api::CrashKey>* crash_keys) {
+  DCHECK(key);
+  DCHECK(value);
+  DCHECK(crash_keys);
+
+  kasko::api::CrashKey crash_key;
+  std::wcsncpy(crash_key.name, key, kasko::api::CrashKey::kNameMaxLength);

[manzagop (departed), 2016/03/31 20:24:45]

Do you need a -1 to for \0?

[Patrick Monette, 2016/04/05 19:03:12]

I'm not sure but I've gone for the safe way and added the null-termination.

+  std::wcsncpy(crash_key.value, value, kasko::api::CrashKey::kValueMaxLength);
+  crash_keys->push_back(crash_key);
+}
+
 void DumpHungBrowserProcess(DWORD main_thread_id,
                             const base::string16& channel,
                             const base::Process& process) {
@@ -241,7 +256,35 @@ void DumpHungBrowserProcess(DWORD main_thread_id,
 
   // Add a special crash key to distinguish reports generated for a hung
   // process.
-  annotations.push_back(kasko::api::CrashKey{L"hung-process", L"1"});
+  AddCrashKey(L"hung-process", L"1", &annotations);
+
+  // Use the Wait Chain Traversal API to determine the hung thread. Defaults to
+  // UI thread on error.
+  std::vector<WAITCHAIN_NODE_INFO> wait_chain;
+  bool is_deadlock = false;
+
+  DWORD hung_thread_id = main_thread_id;
+  if (GetThreadWaitChain(main_thread_id, &wait_chain, &is_deadlock)) {

[manzagop (departed), 2016/03/29 13:57:48]

IIUC this can cross process boundaries and the chain could end with a thread in
a different process?

[Sigurður Ásgeirsson, 2016/03/29 16:57:32]

oh - interesting - this is true.

[Patrick Monette, 2016/04/05 19:03:12]

Done.

+    // The last thread in the wait chain is nominated as the hung thread.
+    DCHECK(wait_chain.back().ObjectType == WctThreadType);

[manzagop (departed), 2016/03/29 13:57:48]

How sure are we the last element is a thread?
I guess there's always a thread that holds the lock. What about cycles, or if
the thread exited with the lock?
Should we search from the back instead?

[Patrick Monette, 2016/04/05 19:03:12]

Done. In some cases, it's possible that the last element is not a thread.

+    hung_thread_id = wait_chain.back().ThreadObject.ThreadId;
+
+    // The entire wait chain is added to the crash report via crash keys.
+    //
+    // As an example (key : value):
+    // hung-process-is-deadlock  : false
+    // hung-process-wait-chain-00 : Thread #10242 with status Blocked
+    // hung-process-wait-chain-01 : Lock of type ThreadWait with status Owned
+    // hung-process-wait-chain-02 : Thread #77221 with status Blocked
+    //
+    AddCrashKey(L"hung-process-is-deadlock", is_deadlock ? L"true" : L"false",
+                &annotations);
+    for (size_t i = 0; i < wait_chain.size(); i++) {
+      AddCrashKey(
+          base::StringPrintf(L"hung-process-wait-chain-%02zu", i).c_str(),
+          WaitChainNodeToString(wait_chain[i]).c_str(), &annotations);
+    }
+  }
 
   std::vector<const base::char16*> key_buffers;
   std::vector<const base::char16*> value_buffers;
@@ -252,7 +295,7 @@ void DumpHungBrowserProcess(DWORD main_thread_id,
   key_buffers.push_back(nullptr);
   value_buffers.push_back(nullptr);
 
-  // Synthesize an exception for the main thread. Populate the record with the
+  // Synthesize an exception for the hung thread. Populate the record with the
   // current context of the thread to get the stack trace bucketed on the crash
   // backend.
   CONTEXT thread_context = {};
@@ -260,29 +303,29 @@ void DumpHungBrowserProcess(DWORD main_thread_id,
   exception_record.ExceptionCode = EXCEPTION_ARRAY_BOUNDS_EXCEEDED;
   EXCEPTION_POINTERS exception_pointers = {&exception_record, &thread_context};
 
-  base::win::ScopedHandle main_thread(::OpenThread(
+  base::win::ScopedHandle hung_thread(::OpenThread(
       THREAD_SUSPEND_RESUME | THREAD_GET_CONTEXT | THREAD_QUERY_INFORMATION,
-      FALSE, main_thread_id));
+      FALSE, hung_thread_id));
 
   bool have_context = false;
-  if (main_thread.IsValid()) {
-    DWORD suspend_count = ::SuspendThread(main_thread.Get());
+  if (hung_thread.IsValid()) {
+    DWORD suspend_count = ::SuspendThread(hung_thread.Get());
     const DWORD kSuspendFailed = static_cast<DWORD>(-1);
     if (suspend_count != kSuspendFailed) {
       // Best effort capture of the context.
       thread_context.ContextFlags = CONTEXT_FLOATING_POINT | CONTEXT_SEGMENTS |
                                     CONTEXT_INTEGER | CONTEXT_CONTROL;
-      if (::GetThreadContext(main_thread.Get(), &thread_context) == TRUE)
+      if (::GetThreadContext(hung_thread.Get(), &thread_context) == TRUE)
         have_context = true;
 
-      ::ResumeThread(main_thread.Get());
+      ::ResumeThread(hung_thread.Get());
     }
   }
 
   // TODO(erikwright): Make the dump-type channel-dependent.
   if (have_context) {
     kasko::api::SendReportForProcess(
-        process.Handle(), main_thread_id, &exception_pointers,
+        process.Handle(), hung_thread_id, &exception_pointers,
         kasko::api::LARGER_DUMP_TYPE, key_buffers.data(), value_buffers.data());
   } else {
     kasko::api::SendReportForProcess(process.Handle(), 0, nullptr,