From def0b7a7b0c4ad3c2f27ddddffcbba7892594da6 Mon Sep 17 00:00:00 2001 From: "andresantoso@chromium.org" Date: Mon, 15 Sep 2014 22:48:18 +0000 Subject: Mac: Add support for in-process crash reporting to Breakpad. Add new option BREAKPAD_IN_PROCESS. If YES, Breakpad will write the dump file in-process and then launch the reporter executable as a child process. Originally reviewed at https://codereview.chromium.org/571523004/ BUG=chromium:414239 R=mark@chromium.org Review URL: https://breakpad.appspot.com/1714002 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1375 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/client/apple/Framework/BreakpadDefines.h | 1 + src/client/mac/Framework/Breakpad.h | 5 +++ src/client/mac/Framework/Breakpad.mm | 58 +++++++++++++++++++++++++++ src/client/mac/crash_generation/ConfigFile.mm | 2 +- src/client/mac/crash_generation/Inspector.h | 1 - src/client/mac/crash_generation/Inspector.mm | 51 ++--------------------- 6 files changed, 69 insertions(+), 49 deletions(-) (limited to 'src/client') diff --git a/src/client/apple/Framework/BreakpadDefines.h b/src/client/apple/Framework/BreakpadDefines.h index 8986cd1b..410a5a6f 100644 --- a/src/client/apple/Framework/BreakpadDefines.h +++ b/src/client/apple/Framework/BreakpadDefines.h @@ -62,6 +62,7 @@ #define BREAKPAD_EMAIL "BreakpadEmail" #define BREAKPAD_SERVER_TYPE "BreakpadServerType" #define BREAKPAD_SERVER_PARAMETER_DICT "BreakpadServerParameters" +#define BREAKPAD_IN_PROCESS "BreakpadInProcess" // The keys below are NOT user supplied, and are used internally. #define BREAKPAD_PROCESS_START_TIME "BreakpadProcStartTime" diff --git a/src/client/mac/Framework/Breakpad.h b/src/client/mac/Framework/Breakpad.h index dbc620eb..dc7e45d1 100644 --- a/src/client/mac/Framework/Breakpad.h +++ b/src/client/mac/Framework/Breakpad.h @@ -158,6 +158,11 @@ typedef bool (*BreakpadFilterCallback)(int exception_type, // but pass as URL parameters when // uploading theminidump to the crash // server. +// +// BREAKPAD_IN_PROCESS A boolean NSNumber value. If YES, Breakpad +// will write the dump file in-process and then +// launch the reporter executable as a child +// process. //============================================================================= // The BREAKPAD_PRODUCT, BREAKPAD_VERSION and BREAKPAD_URL are // required to have non-NULL values. By default, the BREAKPAD_PRODUCT diff --git a/src/client/mac/Framework/Breakpad.mm b/src/client/mac/Framework/Breakpad.mm index fe592bf3..3b4b6674 100644 --- a/src/client/mac/Framework/Breakpad.mm +++ b/src/client/mac/Framework/Breakpad.mm @@ -44,6 +44,7 @@ #import "client/mac/Framework/Breakpad.h" #import "client/mac/Framework/OnDemandServer.h" #import "client/mac/handler/protected_memory_allocator.h" +#include "common/mac/launch_reporter.h" #import "common/mac/MachIPC.h" #import "common/simple_string_dictionary.h" @@ -173,6 +174,8 @@ class Breakpad { } bool Initialize(NSDictionary *parameters); + bool InitializeInProcess(NSDictionary *parameters); + bool InitializeOutOfProcess(NSDictionary *parameters); bool ExtractParameters(NSDictionary *parameters); @@ -188,6 +191,17 @@ class Breakpad { int exception_subcode, mach_port_t crashing_thread); + // Dispatches to HandleMinidump(). + // This gets called instead of ExceptionHandlerDirectCallback when running + // with the BREAKPAD_IN_PROCESS option. + static bool HandleMinidumpCallback(const char *dump_dir, + const char *minidump_id, + void *context, + bool succeeded); + + // This is only used when BREAKPAD_IN_PROCESS is YES. + bool HandleMinidump(const char *dump_dir, const char *minidump_id); + // Since ExceptionHandler (w/o namespace) is defined as typedef in OSX's // MachineExceptions.h, we have to explicitly name the handler. google_breakpad::ExceptionHandler *handler_; // The actual handler (STRONG) @@ -265,6 +279,21 @@ bool Breakpad::ExceptionHandlerDirectCallback(void *context, crashing_thread); } +//============================================================================= +bool Breakpad::HandleMinidumpCallback(const char *dump_dir, + const char *minidump_id, + void *context, + bool succeeded) { + Breakpad *breakpad = (Breakpad *)context; + + // If our context is damaged or something, just return false to indicate that + // the handler should continue without us. + if (!breakpad || !succeeded) + return false; + + return breakpad->HandleMinidump(dump_dir, minidump_id); +} + //============================================================================= #pragma mark - @@ -326,6 +355,25 @@ bool Breakpad::Initialize(NSDictionary *parameters) { return false; } + if ([[parameters objectForKey:@BREAKPAD_IN_PROCESS] boolValue]) + return InitializeInProcess(parameters); + else + return InitializeOutOfProcess(parameters); +} + +//============================================================================= +bool Breakpad::InitializeInProcess(NSDictionary* parameters) { + handler_ = + new (gBreakpadAllocator->Allocate( + sizeof(google_breakpad::ExceptionHandler))) + google_breakpad::ExceptionHandler( + config_params_->GetValueForKey(BREAKPAD_DUMP_DIRECTORY), + 0, &HandleMinidumpCallback, this, true, 0); + return true; +} + +//============================================================================= +bool Breakpad::InitializeOutOfProcess(NSDictionary* parameters) { // Get path to Inspector executable. NSString *inspectorPathString = KeyValue(@BREAKPAD_INSPECTOR_LOCATION); @@ -710,6 +758,16 @@ bool Breakpad::HandleException(int exception_type, return false; } +//============================================================================= +bool Breakpad::HandleMinidump(const char *dump_dir, const char *minidump_id) { + google_breakpad::ConfigFile config_file; + config_file.WriteFile(dump_dir, config_params_, dump_dir, minidump_id); + google_breakpad::LaunchReporter( + config_params_->GetValueForKey(BREAKPAD_REPORTER_EXE_LOCATION), + config_file.GetFilePath()); + return true; +} + //============================================================================= //============================================================================= diff --git a/src/client/mac/crash_generation/ConfigFile.mm b/src/client/mac/crash_generation/ConfigFile.mm index dbb0f24d..acab7de8 100644 --- a/src/client/mac/crash_generation/ConfigFile.mm +++ b/src/client/mac/crash_generation/ConfigFile.mm @@ -36,7 +36,7 @@ #include #import "client/apple/Framework/BreakpadDefines.h" -#import "GTMDefines.h" +#import "common/mac/GTMDefines.h" namespace google_breakpad { diff --git a/src/client/mac/crash_generation/Inspector.h b/src/client/mac/crash_generation/Inspector.h index 7e2eec80..67123551 100644 --- a/src/client/mac/crash_generation/Inspector.h +++ b/src/client/mac/crash_generation/Inspector.h @@ -138,7 +138,6 @@ class Inspector { bool InspectTask(); kern_return_t SendAcknowledgement(); - void LaunchReporter(const char *inConfigFilePath); // The bootstrap port in which the inspector is registered and into which it // must check in. diff --git a/src/client/mac/crash_generation/Inspector.mm b/src/client/mac/crash_generation/Inspector.mm index d226ca38..dc6f4808 100644 --- a/src/client/mac/crash_generation/Inspector.mm +++ b/src/client/mac/crash_generation/Inspector.mm @@ -43,6 +43,7 @@ #import "common/mac/MachIPC.h" #include "common/mac/bootstrap_compat.h" +#include "common/mac/launch_reporter.h" #import "GTMDefines.h" @@ -76,7 +77,9 @@ void Inspector::Inspect(const char *receive_port_name) { if (wrote_minidump) { // Ask the user if he wants to upload the crash report to a server, // and do so if he agrees. - LaunchReporter(config_file_.GetFilePath()); + LaunchReporter( + config_params_.GetValueForKey(BREAKPAD_REPORTER_EXE_LOCATION), + config_file_.GetFilePath()); } else { fprintf(stderr, "Inspection of crashed process failed\n"); } @@ -355,51 +358,5 @@ kern_return_t Inspector::SendAcknowledgement() { return KERN_INVALID_NAME; } -//============================================================================= -void Inspector::LaunchReporter(const char *inConfigFilePath) { - // Extract the path to the reporter executable. - const char *reporterExecutablePath = - config_params_.GetValueForKey(BREAKPAD_REPORTER_EXE_LOCATION); - - // Setup and launch the crash dump sender. - const char *argv[3]; - argv[0] = reporterExecutablePath; - argv[1] = inConfigFilePath; - argv[2] = NULL; - - // Launch the reporter - pid_t pid = fork(); - - // If we're in the child, load in our new executable and run. - // The parent will not wait for the child to complete. - if (pid == 0) { - execv(argv[0], (char * const *)argv); - config_file_.Unlink(); // launch failed - get rid of config file - _exit(1); - } - - // Wait until the Reporter child process exits. - // - - // We'll use a timeout of one minute. - int timeoutCount = 60; // 60 seconds - - while (timeoutCount-- > 0) { - int status; - pid_t result = waitpid(pid, &status, WNOHANG); - - if (result == 0) { - // The child has not yet finished. - sleep(1); - } else if (result == -1) { - // error occurred. - break; - } else { - // child has finished - break; - } - } -} - } // namespace google_breakpad -- cgit v1.2.1