From 676a190308b1492bdb6c1f8889247d4dfedefaf1 Mon Sep 17 00:00:00 2001 From: Joshua Peraza Date: Mon, 30 Sep 2019 13:56:12 -0700 Subject: [PATCH] linux: fix --monitor-self The metrics recording signal handler doesn't need to be re-installed on Linux because the handler installed by StartHandler() restores the previously installed handler by default. Reinstalling the metrics handler results in a crash dump loop in which each signal handler restores the other. Change-Id: Ieef40c74bfc69f6e0caef9809f33cfcaa10f0d03 Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1832153 Reviewed-by: Mark Mentovai Commit-Queue: Joshua Peraza --- client/crashpad_client_linux.cc | 1 - handler/handler_main.cc | 29 ++++++----------------------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/client/crashpad_client_linux.cc b/client/crashpad_client_linux.cc index c3ebbd4c..f1368ec7 100644 --- a/client/crashpad_client_linux.cc +++ b/client/crashpad_client_linux.cc @@ -352,7 +352,6 @@ bool CrashpadClient::StartHandler( const std::vector& arguments, bool restartable, bool asynchronous_start) { - DCHECK(!restartable); DCHECK(!asynchronous_start); ScopedFileHandle client_sock, handler_sock; diff --git a/handler/handler_main.cc b/handler/handler_main.cc index 9fce9fc1..7d9701c0 100644 --- a/handler/handler_main.cc +++ b/handler/handler_main.cc @@ -263,8 +263,6 @@ class CallMetricsRecordNormalExit { #if defined(OS_MACOSX) || defined(OS_LINUX) || defined(OS_ANDROID) -Signals::OldActions g_old_crash_signal_handlers; - void HandleCrashSignal(int sig, siginfo_t* siginfo, void* context) { MetricsRecordExit(Metrics::LifetimeMilestone::kCrashed); @@ -299,9 +297,7 @@ void HandleCrashSignal(int sig, siginfo_t* siginfo, void* context) { } Metrics::HandlerCrashed(metrics_code); - struct sigaction* old_action = - g_old_crash_signal_handlers.ActionForSignal(sig); - Signals::RestoreHandlerAndReraiseSignalOnReturn(siginfo, old_action); + Signals::RestoreHandlerAndReraiseSignalOnReturn(siginfo, nullptr); } void HandleTerminateSignal(int sig, siginfo_t* siginfo, void* context) { @@ -309,13 +305,13 @@ void HandleTerminateSignal(int sig, siginfo_t* siginfo, void* context) { Signals::RestoreHandlerAndReraiseSignalOnReturn(siginfo, nullptr); } -#if defined(OS_MACOSX) - void ReinstallCrashHandler() { // This is used to re-enable the metrics-recording crash handler after // MonitorSelf() sets up a Crashpad exception handler. On macOS, the // metrics-recording handler uses signals and the Crashpad handler uses Mach // exceptions, so there’s nothing to re-enable. + // On Linux, the signal handler installed by StartHandler() restores the + // previously installed signal handler by default. } void InstallCrashHandler() { @@ -325,6 +321,8 @@ void InstallCrashHandler() { Signals::InstallTerminateHandlers(HandleTerminateSignal, 0, nullptr); } +#if defined(OS_MACOSX) + struct ResetSIGTERMTraits { static struct sigaction* InvalidValue() { return nullptr; @@ -349,21 +347,6 @@ void HandleSIGTERM(int sig, siginfo_t* siginfo, void* context) { g_exception_handler_server->Stop(); } -#else - -void ReinstallCrashHandler() { - // This is used to re-enable the metrics-recording crash handler after - // MonitorSelf() sets up a Crashpad signal handler. - Signals::InstallCrashHandlers( - HandleCrashSignal, 0, &g_old_crash_signal_handlers); -} - -void InstallCrashHandler() { - ReinstallCrashHandler(); - - Signals::InstallTerminateHandlers(HandleTerminateSignal, 0, nullptr); -} - #endif // OS_MACOSX #elif defined(OS_WIN) @@ -473,7 +456,7 @@ void MonitorSelf(const Options& options) { // instance of crashpad_handler to be writing metrics at a time, and it should // be the primary instance. CrashpadClient crashpad_client; -#if defined(OS_LINUX) || defined(OS_ANDROID) +#if defined(OS_ANDROID) if (!crashpad_client.StartHandlerAtCrash(executable_path, options.database, base::FilePath(),