diff options
Diffstat (limited to 'src/client/windows')
4 files changed, 143 insertions, 41 deletions
diff --git a/src/client/windows/crash_generation/crash_generation_client.cc b/src/client/windows/crash_generation/crash_generation_client.cc index b0d3d04d..1e86dcec 100644 --- a/src/client/windows/crash_generation/crash_generation_client.cc +++ b/src/client/windows/crash_generation/crash_generation_client.cc @@ -178,6 +178,10 @@ CrashGenerationClient::~CrashGenerationClient() { // // Returns true if the registration is successful; false otherwise. bool CrashGenerationClient::Register() { + if (IsRegistered()) { + return true; + } + HANDLE pipe = ConnectToServer(); if (!pipe) { return false; diff --git a/src/client/windows/handler/exception_handler.cc b/src/client/windows/handler/exception_handler.cc index 59a63144..14c2cf86 100644 --- a/src/client/windows/handler/exception_handler.cc +++ b/src/client/windows/handler/exception_handler.cc @@ -73,7 +73,8 @@ ExceptionHandler::ExceptionHandler(const wstring& dump_path, handler_types, dump_type, pipe_name, - NULL, + NULL, // pipe_handle + NULL, // crash_generation_client custom_info); } @@ -91,11 +92,33 @@ ExceptionHandler::ExceptionHandler(const wstring& dump_path, callback_context, handler_types, dump_type, - NULL, + NULL, // pipe_name pipe_handle, + NULL, // crash_generation_client custom_info); } +ExceptionHandler::ExceptionHandler( + const wstring& dump_path, + FilterCallback filter, + MinidumpCallback callback, + void* callback_context, + int handler_types, + MINIDUMP_TYPE dump_type, + CrashGenerationClient* crash_generation_client, + const CustomClientInfo* custom_info) { + Initialize(dump_path, + filter, + callback, + callback_context, + handler_types, + MiniDumpNormal, + NULL, // pipe_name + NULL, // pipe_handle + crash_generation_client, + custom_info); +} + ExceptionHandler::ExceptionHandler(const wstring &dump_path, FilterCallback filter, MinidumpCallback callback, @@ -107,20 +130,23 @@ ExceptionHandler::ExceptionHandler(const wstring &dump_path, callback_context, handler_types, MiniDumpNormal, - NULL, - NULL, - NULL); + NULL, // pipe_name + NULL, // pipe_handle + NULL, // crash_generation_client + NULL); // custom_info } -void ExceptionHandler::Initialize(const wstring& dump_path, - FilterCallback filter, - MinidumpCallback callback, - void* callback_context, - int handler_types, - MINIDUMP_TYPE dump_type, - const wchar_t* pipe_name, - HANDLE pipe_handle, - const CustomClientInfo* custom_info) { +void ExceptionHandler::Initialize( + const wstring& dump_path, + FilterCallback filter, + MinidumpCallback callback, + void* callback_context, + int handler_types, + MINIDUMP_TYPE dump_type, + const wchar_t* pipe_name, + HANDLE pipe_handle, + CrashGenerationClient* crash_generation_client, + const CustomClientInfo* custom_info) { LONG instance_count = InterlockedIncrement(&instance_count_); filter_ = filter; callback_ = callback; @@ -149,23 +175,20 @@ void ExceptionHandler::Initialize(const wstring& dump_path, handler_return_value_ = false; handle_debug_exceptions_ = false; - // Attempt to use out-of-process if user has specified a pipe. - if (pipe_name != NULL || pipe_handle != NULL) { - assert(!(pipe_name && pipe_handle)); - - scoped_ptr<CrashGenerationClient> client; - if (pipe_name) { - client.reset( - new CrashGenerationClient(pipe_name, - dump_type_, - custom_info)); - } else { - client.reset( - new CrashGenerationClient(pipe_handle, - dump_type_, - custom_info)); - } + // Attempt to use out-of-process if user has specified a pipe or a + // crash generation client. + scoped_ptr<CrashGenerationClient> client; + if (crash_generation_client) { + client.reset(crash_generation_client); + } else if (pipe_name) { + client.reset( + new CrashGenerationClient(pipe_name, dump_type_, custom_info)); + } else if (pipe_handle) { + client.reset( + new CrashGenerationClient(pipe_handle, dump_type_, custom_info)); + } + if (client.get() != NULL) { // If successful in registering with the monitoring process, // there is no need to setup in-process crash generation. if (client->Register()) { diff --git a/src/client/windows/handler/exception_handler.h b/src/client/windows/handler/exception_handler.h index ec9aabcd..2c8ff7d9 100644 --- a/src/client/windows/handler/exception_handler.h +++ b/src/client/windows/handler/exception_handler.h @@ -195,6 +195,27 @@ class ExceptionHandler { HANDLE pipe_handle, const CustomClientInfo* custom_info); + // ExceptionHandler that ENSURES out-of-process dump generation. Expects a + // crash generation client that is already registered with a crash generation + // server. Takes ownership of the passed-in crash_generation_client. + // + // Usage example: + // crash_generation_client = new CrashGenerationClient(..); + // if (crash_generation_client->Register()) { + // // Registration with the crash generation server succeeded. + // // Out-of-process dump generation is guaranteed. + // g_handler = new ExceptionHandler(.., crash_generation_client, ..); + // return true; + // } + ExceptionHandler(const wstring& dump_path, + FilterCallback filter, + MinidumpCallback callback, + void* callback_context, + int handler_types, + MINIDUMP_TYPE dump_type, + CrashGenerationClient* crash_generation_client, + const CustomClientInfo* custom_info); + ~ExceptionHandler(); // Get and set the minidump path. @@ -264,6 +285,7 @@ class ExceptionHandler { MINIDUMP_TYPE dump_type, const wchar_t* pipe_name, HANDLE pipe_handle, + CrashGenerationClient* crash_generation_client, const CustomClientInfo* custom_info); // Function pointer type for MiniDumpWriteDump, which is looked up diff --git a/src/client/windows/unittests/exception_handler_death_test.cc b/src/client/windows/unittests/exception_handler_death_test.cc index c4cb9930..ba7be920 100644 --- a/src/client/windows/unittests/exception_handler_death_test.cc +++ b/src/client/windows/unittests/exception_handler_death_test.cc @@ -54,6 +54,11 @@ const char kFailureIndicator[] = "failure"; // Utility function to test for a path's existence. BOOL DoesPathExist(const TCHAR *path_name); +enum OutOfProcGuarantee { + OUT_OF_PROC_GUARANTEED, + OUT_OF_PROC_BEST_EFFORT, +}; + class ExceptionHandlerDeathTest : public ::testing::Test { protected: // Member variable for each test that they can use @@ -62,7 +67,7 @@ class ExceptionHandlerDeathTest : public ::testing::Test { // Actually constructs a temp path name. virtual void SetUp(); // A helper method that tests can use to crash. - void DoCrashAccessViolation(); + void DoCrashAccessViolation(const OutOfProcGuarantee out_of_proc_guarantee); void DoCrashPureVirtualCall(); }; @@ -140,12 +145,37 @@ void clientDumpCallback(void *dump_context, gDumpCallbackCalled = true; } -void ExceptionHandlerDeathTest::DoCrashAccessViolation() { - google_breakpad::ExceptionHandler *exc = - new google_breakpad::ExceptionHandler( - temp_path_, NULL, NULL, NULL, - google_breakpad::ExceptionHandler::HANDLER_ALL, MiniDumpNormal, kPipeName, - NULL); +void ExceptionHandlerDeathTest::DoCrashAccessViolation( + const OutOfProcGuarantee out_of_proc_guarantee) { + google_breakpad::ExceptionHandler *exc = NULL; + + if (out_of_proc_guarantee == OUT_OF_PROC_GUARANTEED) { + google_breakpad::CrashGenerationClient *client = + new google_breakpad::CrashGenerationClient(kPipeName, + MiniDumpNormal, + NULL); // custom_info + ASSERT_TRUE(client->Register()); + exc = new google_breakpad::ExceptionHandler( + temp_path_, + NULL, // filter + NULL, // callback + NULL, // callback_context + google_breakpad::ExceptionHandler::HANDLER_ALL, + MiniDumpNormal, + client, + NULL); // custom_info + } else { + ASSERT_TRUE(out_of_proc_guarantee == OUT_OF_PROC_BEST_EFFORT); + exc = new google_breakpad::ExceptionHandler( + temp_path_, + NULL, // filter + NULL, // callback + NULL, // callback_context + google_breakpad::ExceptionHandler::HANDLER_ALL, + MiniDumpNormal, + kPipeName, + NULL); // custom_info + } // Disable GTest SEH handler testing::DisableExceptionHandlerInScope disable_exception_handler; @@ -170,15 +200,38 @@ TEST_F(ExceptionHandlerDeathTest, OutOfProcTest) { ASSERT_TRUE(DoesPathExist(temp_path_)); std::wstring dump_path(temp_path_); google_breakpad::CrashGenerationServer server( - kPipeName, NULL, NULL, NULL, &clientDumpCallback, NULL, NULL, NULL, NULL, - NULL, true, &dump_path); + kPipeName, NULL, NULL, NULL, &clientDumpCallback, NULL, NULL, NULL, NULL, + NULL, true, &dump_path); + + // This HAS to be EXPECT_, because when this test case is executed in the + // child process, the server registration will fail due to the named pipe + // being the same. + EXPECT_TRUE(server.Start()); + gDumpCallbackCalled = false; + ASSERT_DEATH(this->DoCrashAccessViolation(OUT_OF_PROC_BEST_EFFORT), ""); + EXPECT_TRUE(gDumpCallbackCalled); +} + +TEST_F(ExceptionHandlerDeathTest, OutOfProcGuaranteedTest) { + // This is similar to the previous test (OutOfProcTest). The only difference + // is that in this test, the crash generation client is created and registered + // with the crash generation server outside of the ExceptionHandler + // constructor which allows breakpad users to opt out of the default + // in-process dump generation when the registration with the crash generation + // server fails. + + ASSERT_TRUE(DoesPathExist(temp_path_)); + std::wstring dump_path(temp_path_); + google_breakpad::CrashGenerationServer server( + kPipeName, NULL, NULL, NULL, &clientDumpCallback, NULL, NULL, NULL, NULL, + NULL, true, &dump_path); // This HAS to be EXPECT_, because when this test case is executed in the // child process, the server registration will fail due to the named pipe // being the same. EXPECT_TRUE(server.Start()); - EXPECT_FALSE(gDumpCallbackCalled); - ASSERT_DEATH(this->DoCrashAccessViolation(), ""); + gDumpCallbackCalled = false; + ASSERT_DEATH(this->DoCrashAccessViolation(OUT_OF_PROC_GUARANTEED), ""); EXPECT_TRUE(gDumpCallbackCalled); } |