Update Crashpad to b47bf6c250c6b825dee1c5fbad9152c2c962e828

third_party/crashpad/crashpad/client/crashpad_client_win.cc

Index: third_party/crashpad/crashpad/client/crashpad_client_win.cc
diff --git a/third_party/crashpad/crashpad/client/crashpad_client_win.cc b/third_party/crashpad/crashpad/client/crashpad_client_win.cc
index b9e42c673e04c30dca730a500d8c6d72d3ffab1c..235ef1858e85b421170e4dd56827d24ffeb2196f 100644
--- a/third_party/crashpad/crashpad/client/crashpad_client_win.cc
+++ b/third_party/crashpad/crashpad/client/crashpad_client_win.cc
@@ -22,24 +22,29 @@
 
 #include "base/atomicops.h"
 #include "base/logging.h"
+#include "base/macros.h"
 #include "base/scoped_generic.h"
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/lock.h"
 #include "util/file/file_io.h"
+#include "util/misc/random_string.h"
 #include "util/win/address_types.h"
 #include "util/win/command_line.h"
 #include "util/win/critical_section_with_debug_info.h"
 #include "util/win/get_function.h"
 #include "util/win/handle.h"
+#include "util/win/initial_client_data.h"
 #include "util/win/nt_internals.h"
 #include "util/win/ntstatus_logging.h"
 #include "util/win/process_info.h"
 #include "util/win/registration_protocol_win.h"
-#include "util/win/scoped_handle.h"
 #include "util/win/scoped_process_suspend.h"
 #include "util/win/termination_codes.h"
+#include "util/win/xp_compat.h"
+
+namespace crashpad {
 
 namespace {
 
@@ -48,7 +53,7 @@ namespace {
 HANDLE g_signal_exception = INVALID_HANDLE_VALUE;
 
 // Where we store the exception information that the crash handler reads.
-crashpad::ExceptionInformation g_crash_exception_information;
+ExceptionInformation g_crash_exception_information;
 
 // These handles are never closed. g_signal_non_crash_dump is used to signal to
 // the server to take a dump (not due to an exception), and the server will
@@ -61,7 +66,20 @@ base::Lock* g_non_crash_dump_lock;
 
 // Where we store a pointer to the context information when taking a non-crash
 // dump.
-crashpad::ExceptionInformation g_non_crash_exception_information;
+ExceptionInformation g_non_crash_exception_information;
+
+enum class StartupState : int {
+  kNotReady = 0,   // This must be value 0 because it is the initial value of a
+                   // global AtomicWord.
+  kSucceeded = 1,  // The CreateProcess() for the handler succeeded.
+  kFailed = 2,     // The handler failed to start.
+};
+
+// This is a tri-state of type StartupState. It starts at 0 == kNotReady, and
+// when the handler is known to have started successfully, or failed to start
+// the value will be updated. The unhandled exception filter will not proceed
+// until one of those two cases happens.
+base::subtle::AtomicWord g_handler_startup_state;
 
 // A CRITICAL_SECTION initialized with
 // RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO to force it to be allocated with a
@@ -70,7 +88,37 @@ crashpad::ExceptionInformation g_non_crash_exception_information;
 // list, so this allows the handler to capture all of them.
 CRITICAL_SECTION g_critical_section_with_debug_info;
 
+void SetHandlerStartupState(StartupState state) {
+  DCHECK(state == StartupState::kSucceeded ||
+         state == StartupState::kFailed);
+  base::subtle::Acquire_Store(&g_handler_startup_state,
+                              static_cast<base::subtle::AtomicWord>(state));
+}
+
+StartupState BlockUntilHandlerStartedOrFailed() {
+  // Wait until we know the handler has either succeeded or failed to start.
+  base::subtle::AtomicWord startup_state;
+  while (
+      (startup_state = base::subtle::Release_Load(&g_handler_startup_state)) ==
+      static_cast<int>(StartupState::kNotReady)) {
+    Sleep(1);
+  }
+
+  return static_cast<StartupState>(startup_state);
+}
+
 LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) {
+  if (BlockUntilHandlerStartedOrFailed() == StartupState::kFailed) {
+    // If we know for certain that the handler has failed to start, then abort
+    // here, rather than trying to signal to a handler that will never arrive,
+    // and then sleeping unnecessarily.
+    LOG(ERROR) << "crash server failed to launch, self-terminating";
+    TerminateProcess(GetCurrentProcess(), kTerminationCodeCrashNoDump);
+    return EXCEPTION_CONTINUE_SEARCH;
+  }
+
+  // Otherwise, we know the handler startup has succeeded, and we can continue.
+
   // Tracks whether a thread has already entered UnhandledExceptionHandler.
   static base::subtle::AtomicWord have_crashed;
 
@@ -94,7 +142,7 @@ LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) {
   // signal the crash handler.
   g_crash_exception_information.thread_id = GetCurrentThreadId();
   g_crash_exception_information.exception_pointers =
-      reinterpret_cast<crashpad::WinVMAddress>(exception_pointers);
+      reinterpret_cast<WinVMAddress>(exception_pointers);
 
   // Now signal the crash server, which will take a dump and then terminate us
   // when it's complete.
@@ -110,7 +158,7 @@ LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) {
 
   LOG(ERROR) << "crash server did not respond, self-terminating";
 
-  TerminateProcess(GetCurrentProcess(), crashpad::kTerminationCodeCrashNoDump);
+  TerminateProcess(GetCurrentProcess(), kTerminationCodeCrashNoDump);
 
   return EXCEPTION_CONTINUE_SEARCH;
 }
@@ -121,9 +169,7 @@ std::wstring FormatArgumentString(const std::string& name,
 }
 
 struct ScopedProcThreadAttributeListTraits {
-  static PPROC_THREAD_ATTRIBUTE_LIST InvalidValue() {
-    return nullptr;
-  }
+  static PPROC_THREAD_ATTRIBUTE_LIST InvalidValue() { return nullptr; }
 
   static void Free(PPROC_THREAD_ATTRIBUTE_LIST proc_thread_attribute_list) {
     // This is able to use GET_FUNCTION_REQUIRED() instead of GET_FUNCTION()
@@ -186,74 +232,137 @@ void AddUint64(std::vector<unsigned char>* data_vector, uint64_t data) {
             static_cast<uint32_t>((data & 0xffffffff00000000ULL) >> 32));
 }
 
-}  // namespace
-
-namespace crashpad {
-
-CrashpadClient::CrashpadClient()
-    : ipc_pipe_() {
+//! \brief Creates a randomized pipe name to listen for client registrations
+//!     on and returns its name.
+//!
+//! \param[out] pipe_name The pipe name that will be listened on.
+//! \param[out] pipe_handle The first pipe instance corresponding for the pipe.
+void CreatePipe(std::wstring* pipe_name, ScopedFileHANDLE* pipe_instance) {
+  int tries = 5;
+  std::string pipe_name_base =
+      base::StringPrintf("\\\\.\\pipe\\crashpad_%d_", GetCurrentProcessId());
+  do {
+    *pipe_name = base::UTF8ToUTF16(pipe_name_base + RandomString());
+
+    pipe_instance->reset(CreateNamedPipeInstance(*pipe_name, true));
+
+    // CreateNamedPipe() is documented as setting the error to
+    // ERROR_ACCESS_DENIED if FILE_FLAG_FIRST_PIPE_INSTANCE is specified and the
+    // pipe name is already in use. However it may set the error to other codes
+    // such as ERROR_PIPE_BUSY (if the pipe already exists and has reached its
+    // maximum instance count) or ERROR_INVALID_PARAMETER (if the pipe already
+    // exists and its attributes differ from those specified to
+    // CreateNamedPipe()). Some of these errors may be ambiguous: for example,
+    // ERROR_INVALID_PARAMETER may also occur if CreateNamedPipe() is called
+    // incorrectly even in the absence of an existing pipe by the same name.
+    // Rather than chasing down all of the possible errors that might indicate
+    // that a pipe name is already in use, retry up to a few times on any error.
+  } while (!pipe_instance->is_valid() && --tries);
+
+  PCHECK(pipe_instance->is_valid()) << "CreateNamedPipe";
 }
 
-CrashpadClient::~CrashpadClient() {
-}
+struct BackgroundHandlerStartThreadData {
+  BackgroundHandlerStartThreadData(
+      const base::FilePath& handler,
+      const base::FilePath& database,
+      const base::FilePath& metrics_dir,
+      const std::string& url,
+      const std::map<std::string, std::string>& annotations,
+      const std::vector<std::string>& arguments,
+      const std::wstring& ipc_pipe,
+      ScopedFileHANDLE ipc_pipe_handle)
+      : handler(handler),
+        database(database),
+        metrics_dir(metrics_dir),
+        url(url),
+        annotations(annotations),
+        arguments(arguments),
+        ipc_pipe(ipc_pipe),
+        ipc_pipe_handle(std::move(ipc_pipe_handle)) {}
+
+  base::FilePath handler;
+  base::FilePath database;
+  base::FilePath metrics_dir;
+  std::string url;
+  std::map<std::string, std::string> annotations;
+  std::vector<std::string> arguments;
+  std::wstring ipc_pipe;
+  ScopedFileHANDLE ipc_pipe_handle;
+};
 
-bool CrashpadClient::StartHandler(
-    const base::FilePath& handler,
-    const base::FilePath& database,
-    const base::FilePath& metrics_dir,
-    const std::string& url,
-    const std::map<std::string, std::string>& annotations,
-    const std::vector<std::string>& arguments,
-    bool restartable) {
-  DCHECK(ipc_pipe_.empty());
+// Ensures that SetHandlerStartupState() is called on scope exit. Assumes
+// failure, and on success, SetSuccessful() should be called.
+class ScopedCallSetHandlerStartupState {
+ public:
+  ScopedCallSetHandlerStartupState() : successful_(false) {}
 
-  HANDLE pipe_read;
-  HANDLE pipe_write;
-  SECURITY_ATTRIBUTES security_attributes = {};
-  security_attributes.nLength = sizeof(security_attributes);
-  security_attributes.bInheritHandle = TRUE;
-  if (!CreatePipe(&pipe_read, &pipe_write, &security_attributes, 0)) {
-    PLOG(ERROR) << "CreatePipe";
-    return false;
+  ~ScopedCallSetHandlerStartupState() {
+    SetHandlerStartupState(successful_ ? StartupState::kSucceeded
+                                       : StartupState::kFailed);
   }
-  ScopedFileHandle pipe_read_owner(pipe_read);
-  ScopedFileHandle pipe_write_owner(pipe_write);
 
-  // The new process only needs the write side of the pipe.
-  BOOL rv = SetHandleInformation(pipe_read, HANDLE_FLAG_INHERIT, 0);
-  PLOG_IF(WARNING, !rv) << "SetHandleInformation";
+  void SetSuccessful() { successful_ = true; }
+
+ private:
+  bool successful_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedCallSetHandlerStartupState);
+};
+
+bool StartHandlerProcess(
+    std::unique_ptr<BackgroundHandlerStartThreadData> data) {
+  ScopedCallSetHandlerStartupState scoped_startup_state_caller;
 
   std::wstring command_line;
-  AppendCommandLineArgument(handler.value(), &command_line);
-  for (const std::string& argument : arguments) {
+  AppendCommandLineArgument(data->handler.value(), &command_line);
+  for (const std::string& argument : data->arguments) {
     AppendCommandLineArgument(base::UTF8ToUTF16(argument), &command_line);
   }
-  if (!database.value().empty()) {
-    AppendCommandLineArgument(FormatArgumentString("database",
-                                                   database.value()),
-                              &command_line);
+  if (!data->database.value().empty()) {
+    AppendCommandLineArgument(
+        FormatArgumentString("database", data->database.value()),
+        &command_line);
   }
-  if (!metrics_dir.value().empty()) {
+  if (!data->metrics_dir.value().empty()) {
     AppendCommandLineArgument(
-        FormatArgumentString("metrics-dir", metrics_dir.value()),
+        FormatArgumentString("metrics-dir", data->metrics_dir.value()),
         &command_line);
   }
-  if (!url.empty()) {
-    AppendCommandLineArgument(FormatArgumentString("url",
-                                                   base::UTF8ToUTF16(url)),
-                              &command_line);
+  if (!data->url.empty()) {
+    AppendCommandLineArgument(
+        FormatArgumentString("url", base::UTF8ToUTF16(data->url)),
+        &command_line);
   }
-  for (const auto& kv : annotations) {
+  for (const auto& kv : data->annotations) {
     AppendCommandLineArgument(
         FormatArgumentString("annotation",
                              base::UTF8ToUTF16(kv.first + '=' + kv.second)),
         &command_line);
   }
+
+  ScopedKernelHANDLE this_process(
+      OpenProcess(kXPProcessAllAccess, true, GetCurrentProcessId()));
+  if (!this_process.is_valid()) {
+    PLOG(ERROR) << "OpenProcess";
+    return false;
+  }
+
+  InitialClientData initial_client_data(
+      g_signal_exception,
+      g_signal_non_crash_dump,
+      g_non_crash_dump_done,
+      data->ipc_pipe_handle.get(),
+      this_process.get(),
+      reinterpret_cast<WinVMAddress>(&g_crash_exception_information),
+      reinterpret_cast<WinVMAddress>(&g_non_crash_exception_information),
+      reinterpret_cast<WinVMAddress>(&g_critical_section_with_debug_info));
   AppendCommandLineArgument(
-      base::UTF8ToUTF16(base::StringPrintf("--handshake-handle=0x%x",
-                                           HandleToInt(pipe_write))),
+      base::UTF8ToUTF16(std::string("--initial-client-data=") +
+                        initial_client_data.StringRepresentation()),
       &command_line);
 
+  BOOL rv;
   DWORD creation_flags;
   STARTUPINFOEX startup_info = {};
   startup_info.StartupInfo.dwFlags = STARTF_USESTDHANDLES;
@@ -304,8 +413,12 @@ bool CrashpadClient::StartHandler(
     }
     proc_thread_attribute_list_owner.reset(startup_info.lpAttributeList);
 
-    handle_list.reserve(4);
-    handle_list.push_back(pipe_write);
+    handle_list.reserve(8);
+    handle_list.push_back(g_signal_exception);
+    handle_list.push_back(g_signal_non_crash_dump);
+    handle_list.push_back(g_non_crash_dump_done);
+    handle_list.push_back(data->ipc_pipe_handle.get());
+    handle_list.push_back(this_process.get());
     AddHandleToListIfValidAndInheritable(&handle_list,
                                          startup_info.StartupInfo.hStdInput);
     AddHandleToListIfValidAndInheritable(&handle_list,
@@ -327,7 +440,7 @@ bool CrashpadClient::StartHandler(
   }
 
   PROCESS_INFORMATION process_info;
-  rv = CreateProcess(handler.value().c_str(),
+  rv = CreateProcess(data->handler.value().c_str(),
                      &command_line[0],
                      nullptr,
                      nullptr,
@@ -348,38 +461,125 @@ bool CrashpadClient::StartHandler(
   rv = CloseHandle(process_info.hProcess);
   PLOG_IF(WARNING, !rv) << "CloseHandle process";
 
-  pipe_write_owner.reset();
-
-  uint32_t ipc_pipe_length;
-  if (!LoggingReadFile(pipe_read, &ipc_pipe_length, sizeof(ipc_pipe_length))) {
-    return false;
-  }
+  // It is important to close our side of the pipe here before confirming that
+  // we can communicate with the server. By doing so, the only remaining copy of
+  // the server side of the pipe belongs to the exception handler process we
+  // just spawned. Otherwise, the pipe will continue to exist indefinitely, so
+  // the connection loop will not detect that it will never be serviced.
+  data->ipc_pipe_handle.reset();
 
-  ipc_pipe_.resize(ipc_pipe_length);
-  if (ipc_pipe_length &&
-      !LoggingReadFile(
-          pipe_read, &ipc_pipe_[0], ipc_pipe_length * sizeof(ipc_pipe_[0]))) {
+  // Confirm that the server is waiting for connections before continuing.
+  ClientToServerMessage message = {};
+  message.type = ClientToServerMessage::kPing;
+  ServerToClientMessage response = {};
+  if (!SendToCrashHandlerServer(data->ipc_pipe, message, &response)) {
     return false;
   }
 
+  scoped_startup_state_caller.SetSuccessful();
   return true;
 }
 
-bool CrashpadClient::SetHandlerIPCPipe(const std::wstring& ipc_pipe) {
-  DCHECK(ipc_pipe_.empty());
-  DCHECK(!ipc_pipe.empty());
+DWORD WINAPI BackgroundHandlerStartThreadProc(void* data) {
+  std::unique_ptr<BackgroundHandlerStartThreadData> data_as_ptr(
+      reinterpret_cast<BackgroundHandlerStartThreadData*>(data));
+  return StartHandlerProcess(std::move(data_as_ptr)) ? 0 : 1;
+}
 
-  ipc_pipe_ = ipc_pipe;
+void CommonInProcessInitialization() {
+  // We create this dummy CRITICAL_SECTION with the
+  // RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO flag set to have an entry point
+  // into the doubly-linked list of RTL_CRITICAL_SECTION_DEBUG objects. This
+  // allows us to walk the list at crash time to gather data for !locks. A
+  // debugger would instead inspect ntdll!RtlCriticalSectionList to get the head
+  // of the list. But that is not an exported symbol, so on an arbitrary client
+  // machine, we don't have a way of getting that pointer.
+  InitializeCriticalSectionWithDebugInfoIfPossible(
+      &g_critical_section_with_debug_info);
 
-  return true;
+  g_non_crash_dump_lock = new base::Lock();
 }
 
-std::wstring CrashpadClient::GetHandlerIPCPipe() const {
-  DCHECK(!ipc_pipe_.empty());
-  return ipc_pipe_;
+}  // namespace
+
+CrashpadClient::CrashpadClient() : ipc_pipe_(), handler_start_thread_() {}
+
+CrashpadClient::~CrashpadClient() {}
+
+bool CrashpadClient::StartHandler(
+    const base::FilePath& handler,
+    const base::FilePath& database,
+    const base::FilePath& metrics_dir,
+    const std::string& url,
+    const std::map<std::string, std::string>& annotations,
+    const std::vector<std::string>& arguments,
+    bool restartable,
+    bool asynchronous_start) {
+  DCHECK(ipc_pipe_.empty());
+
+  // Both the pipe and the signalling events have to be created on the main
+  // thread (not the spawning thread) so that they're valid after we return from
+  // this function.
+  ScopedFileHANDLE ipc_pipe_handle;
+  CreatePipe(&ipc_pipe_, &ipc_pipe_handle);
+
+  SECURITY_ATTRIBUTES security_attributes = {0};
+  security_attributes.nLength = sizeof(SECURITY_ATTRIBUTES);
+  security_attributes.bInheritHandle = true;
+
+  g_signal_exception =
+      CreateEvent(&security_attributes, false /* auto reset */, false, nullptr);
+  g_signal_non_crash_dump =
+      CreateEvent(&security_attributes, false /* auto reset */, false, nullptr);
+  g_non_crash_dump_done =
+      CreateEvent(&security_attributes, false /* auto reset */, false, nullptr);
+
+  CommonInProcessInitialization();
+
+  SetUnhandledExceptionFilter(&UnhandledExceptionHandler);
+
+  auto data = new BackgroundHandlerStartThreadData(handler,
+                                                   database,
+                                                   metrics_dir,
+                                                   url,
+                                                   annotations,
+                                                   arguments,
+                                                   ipc_pipe_,
+                                                   std::move(ipc_pipe_handle));
+
+  if (asynchronous_start) {
+    // It is important that the current thread not be synchronized with the
+    // thread that is created here. StartHandler() needs to be callable inside a
+    // DllMain(). In that case, the background thread will not start until the
+    // current DllMain() completes, which would cause deadlock if it was waited
+    // upon.
+    handler_start_thread_.reset(CreateThread(nullptr,
+                                             0,
+                                             &BackgroundHandlerStartThreadProc,
+                                             reinterpret_cast<void*>(data),
+                                             0,
+                                             nullptr));
+    if (!handler_start_thread_.is_valid()) {
+      PLOG(ERROR) << "CreateThread";
+      SetHandlerStartupState(StartupState::kFailed);
+      return false;
+    }
+
+    // In asynchronous mode, we can't report on the overall success or failure
+    // of initialization at this point.
+    return true;
+  } else {
+    return StartHandlerProcess(
+        std::unique_ptr<BackgroundHandlerStartThreadData>(data));
+  }
 }
 
-bool CrashpadClient::UseHandler() {
+bool CrashpadClient::SetHandlerIPCPipe(const std::wstring& ipc_pipe) {
+  DCHECK(ipc_pipe_.empty());
+  DCHECK(!ipc_pipe.empty());
+
+  ipc_pipe_ = ipc_pipe;
+
   DCHECK(!ipc_pipe_.empty());
   DCHECK_EQ(g_signal_exception, INVALID_HANDLE_VALUE);
   DCHECK_EQ(g_signal_non_crash_dump, INVALID_HANDLE_VALUE);
@@ -397,18 +597,10 @@ bool CrashpadClient::UseHandler() {
   message.registration.non_crash_exception_information =
       reinterpret_cast<WinVMAddress>(&g_non_crash_exception_information);
 
-  // We create this dummy CRITICAL_SECTION with the
-  // RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO flag set to have an entry point
-  // into the doubly-linked list of RTL_CRITICAL_SECTION_DEBUG objects. This
-  // allows us to walk the list at crash time to gather data for !locks. A
-  // debugger would instead inspect ntdll!RtlCriticalSectionList to get the head
-  // of the list. But that is not an exported symbol, so on an arbitrary client
-  // machine, we don't have a way of getting that pointer.
-  if (InitializeCriticalSectionWithDebugInfoIfPossible(
-          &g_critical_section_with_debug_info)) {
-    message.registration.critical_section_address =
-        reinterpret_cast<WinVMAddress>(&g_critical_section_with_debug_info);
-  }
+  CommonInProcessInitialization();
+
+  message.registration.critical_section_address =
+      reinterpret_cast<WinVMAddress>(&g_critical_section_with_debug_info);
 
   ServerToClientMessage response = {};
 
@@ -416,6 +608,9 @@ bool CrashpadClient::UseHandler() {
     return false;
   }
 
+  SetHandlerStartupState(StartupState::kSucceeded);
+  SetUnhandledExceptionFilter(&UnhandledExceptionHandler);
+
   // The server returns these already duplicated to be valid in this process.
   g_signal_exception =
       IntToHandle(response.registration.request_crash_dump_event);
@@ -424,19 +619,45 @@ bool CrashpadClient::UseHandler() {
   g_non_crash_dump_done =
       IntToHandle(response.registration.non_crash_dump_completed_event);
 
-  g_non_crash_dump_lock = new base::Lock();
-
-  // In theory we could store the previous handler but it is not clear what
-  // use we have for it.
-  SetUnhandledExceptionFilter(&UnhandledExceptionHandler);
   return true;
 }
 
+std::wstring CrashpadClient::GetHandlerIPCPipe() const {
+  DCHECK(!ipc_pipe_.empty());
+  return ipc_pipe_;
+}
+
+bool CrashpadClient::WaitForHandlerStart() {
+  DCHECK(handler_start_thread_.is_valid());
+  if (WaitForSingleObject(handler_start_thread_.get(), INFINITE) !=
+      WAIT_OBJECT_0) {
+    PLOG(ERROR) << "WaitForSingleObject";
+    return false;
+  }
+
+  DWORD exit_code;
+  if (!GetExitCodeThread(handler_start_thread_.get(), &exit_code)) {
+    PLOG(ERROR) << "GetExitCodeThread";
+    return false;
+  }
+
+  handler_start_thread_.reset();
+  return exit_code == 0;
+}
+
 // static
 void CrashpadClient::DumpWithoutCrash(const CONTEXT& context) {
   if (g_signal_non_crash_dump == INVALID_HANDLE_VALUE ||
       g_non_crash_dump_done == INVALID_HANDLE_VALUE) {
-    LOG(ERROR) << "haven't called UseHandler()";
+    LOG(ERROR) << "not connected";
+    return;
+  }
+
+  if (BlockUntilHandlerStartedOrFailed() == StartupState::kFailed) {
+    // If we know for certain that the handler has failed to start, then abort
+    // here, as we would otherwise wait indefinitely for the
+    // g_non_crash_dump_done event that would never be signalled.
+    LOG(ERROR) << "crash server failed to launch, no dump captured";
     return;
   }
 
@@ -475,7 +696,7 @@ void CrashpadClient::DumpWithoutCrash(const CONTEXT& context) {
 
   g_non_crash_exception_information.thread_id = GetCurrentThreadId();
   g_non_crash_exception_information.exception_pointers =
-      reinterpret_cast<crashpad::WinVMAddress>(&exception_pointers);
+      reinterpret_cast<WinVMAddress>(&exception_pointers);
 
   bool set_event_result = !!SetEvent(g_signal_non_crash_dump);
   PLOG_IF(ERROR, !set_event_result) << "SetEvent";
@@ -487,11 +708,15 @@ void CrashpadClient::DumpWithoutCrash(const CONTEXT& context) {
 // static
 void CrashpadClient::DumpAndCrash(EXCEPTION_POINTERS* exception_pointers) {
   if (g_signal_exception == INVALID_HANDLE_VALUE) {
-    LOG(ERROR) << "haven't called UseHandler(), no dump captured";
-    TerminateProcess(GetCurrentProcess(), kTerminationCodeUseHandlerNotCalled);
+    LOG(ERROR) << "not connected";
+    TerminateProcess(GetCurrentProcess(),
+                     kTerminationCodeNotConnectedToHandler);
     return;
   }
 
+  // We don't need to check for handler startup here, as
+  // UnhandledExceptionHandler() necessarily does that.
+
   UnhandledExceptionHandler(exception_pointers);
 }