Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(864)

Unified Diff: chrome/chrome_watcher/chrome_watcher_main.cc

Issue 1834463002: Identify the hung thread using the Wait Chain Traversal API (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Grt comments Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« base/win/wait_chain_unittest.cc ('K') | « base/win/wait_chain_unittest.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..7e86127106a994a697f5e44ba80ead4aea2789bd 100644
--- a/chrome/chrome_watcher/chrome_watcher_main.cc
+++ b/chrome/chrome_watcher/chrome_watcher_main.cc
@@ -5,6 +5,7 @@
#include <windows.h>
#include <sddl.h>
+#include <algorithm>
#include <utility>
#include "base/at_exit.h"
@@ -15,6 +16,7 @@
#include "base/environment.h"
#include "base/file_version_info.h"
#include "base/files/file_path.h"
+#include "base/format_macros.h"
#include "base/logging_win.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
@@ -25,6 +27,8 @@
#include "base/strings/string16.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_piece.h"
+#include "base/strings/string_util.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 +45,7 @@
#include "third_party/kasko/kasko_features.h"
#if BUILDFLAG(ENABLE_KASKO)
+#include "base/win/wait_chain.h"
#include "components/crash/content/app/crashpad.h"
#include "syzygy/kasko/api/reporter.h"
#endif
@@ -232,6 +237,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);
+
+ crash_keys->resize(crash_keys->size() + 1);
+ kasko::api::CrashKey& crash_key = crash_keys->back();
+ base::wcslcpy(crash_key.name, key, kasko::api::CrashKey::kNameMaxLength);
+ base::wcslcpy(crash_key.value, value, kasko::api::CrashKey::kValueMaxLength);
+}
+
void DumpHungBrowserProcess(DWORD main_thread_id,
const base::string16& channel,
const base::Process& process) {
@@ -241,7 +259,47 @@ 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. The wait chain may point to a different thread in a
+ // different process for the hung thread.
+ DWORD hung_thread_id = main_thread_id;
+ base::Process hung_process = process.Duplicate();
+
+ base::win::WaitChainNodeVector wait_chain;
+ bool is_deadlock = false;
+ if (base::win::GetThreadWaitChain(main_thread_id, &wait_chain,
+ &is_deadlock)) {
+ // The last thread in the wait chain is nominated as the hung thread.
+ auto it = std::find_if(wait_chain.rbegin(), wait_chain.rend(),
+ [](const WAITCHAIN_NODE_INFO& node) {
+ return node.ObjectType == WctThreadType;
+ });
+ DCHECK(it != wait_chain.rend());
+
+ // DO NOT COMMIT: I'll add the same check as Pierre-Antoine for the process
+ // id race.
+ hung_process = base::Process::Open(it->ThreadObject.ProcessId);
+ hung_thread_id = it->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-%02" PRIuS, i).c_str(),
+ base::win::WaitChainNodeToString(wait_chain[i]).c_str(),
+ &annotations);
+ }
+ }
std::vector<const base::char16*> key_buffers;
std::vector<const base::char16*> value_buffers;
@@ -252,7 +310,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,32 +318,32 @@ 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,
+ hung_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,
+ kasko::api::SendReportForProcess(hung_process.Handle(), 0, nullptr,
kasko::api::LARGER_DUMP_TYPE,
key_buffers.data(), value_buffers.data());
}
« base/win/wait_chain_unittest.cc ('K') | « base/win/wait_chain_unittest.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698