From cdbb90ec6968f369a93a8071576f44208eddabea Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Mon, 12 Dec 2016 20:57:48 -0800 Subject: [PATCH] win: Add timeout argument to WaitForHandlerStart() As brought up in https://codereview.chromium.org/2475863004/, there's the potential for failed startup if StartHandlerProcess() hangs for whatever reason. Add a timeout to the wait function so that this case can attempt to log an error. R=mark@chromium.org BUG=655788, 656800, 565063 Change-Id: Ib08cd0641daa6a6cefabb773ffe470227b51958c Reviewed-on: https://chromium-review.googlesource.com/419060 Reviewed-by: Mark Mentovai --- client/crashpad_client.h | 5 ++++- client/crashpad_client_win.cc | 12 +++++++++--- client/crashpad_client_win_test.cc | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/client/crashpad_client.h b/client/crashpad_client.h index 844512f1..f6f3395c 100644 --- a/client/crashpad_client.h +++ b/client/crashpad_client.h @@ -173,9 +173,12 @@ class CrashpadClient { //! //! This method should not be used unless `asynchronous_start` was `true`. //! + //! \param[in] timeout_ms The number of milliseconds to wait for a result from + //! the background launch, or `0xffffffff` to block indefinitely. + //! //! \return `true` if the hander startup succeeded, `false` otherwise, and an //! error message will have been logged. - bool WaitForHandlerStart(); + bool WaitForHandlerStart(unsigned int timeout_ms); //! \brief Requests that the handler capture a dump even though there hasn't //! been a crash. diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index becfc019..7a3d0047 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -689,10 +689,16 @@ std::wstring CrashpadClient::GetHandlerIPCPipe() const { return ipc_pipe_; } -bool CrashpadClient::WaitForHandlerStart() { +bool CrashpadClient::WaitForHandlerStart(unsigned int timeout_ms) { DCHECK(handler_start_thread_.is_valid()); - if (WaitForSingleObject(handler_start_thread_.get(), INFINITE) != - WAIT_OBJECT_0) { + DWORD result = WaitForSingleObject(handler_start_thread_.get(), timeout_ms); + if (result == WAIT_TIMEOUT) { + LOG(ERROR) << "WaitForSingleObject timed out"; + return false; + } else if (result == WAIT_ABANDONED) { + LOG(ERROR) << "WaitForSingleObject abandoned"; + return false; + } else if (result != WAIT_OBJECT_0) { PLOG(ERROR) << "WaitForSingleObject"; return false; } diff --git a/client/crashpad_client_win_test.cc b/client/crashpad_client_win_test.cc index 94a9b0cc..47e7add9 100644 --- a/client/crashpad_client_win_test.cc +++ b/client/crashpad_client_win_test.cc @@ -39,7 +39,7 @@ void StartAndUseHandler() { std::vector(), true, true)); - ASSERT_TRUE(client.WaitForHandlerStart()); + ASSERT_TRUE(client.WaitForHandlerStart(INFINITE)); } class StartWithInvalidHandles final : public WinMultiprocess {