From 2f3ed034f378e99f27bd733b6a452df47739117c Mon Sep 17 00:00:00 2001 From: Michael Krebs Date: Tue, 21 Aug 2012 20:17:03 -0700 Subject: [PATCH] crash-reporter: Also ignore renamed Chrome threads At some point Chrome started naming its threads such that crashes report a different executable name. We've been getting a *lot* of crash-reporter errors with names like "supplied_Compositor" in particular, so consider those to be Chrome crashes that should be ignored. Unfortunately, the thread names can be arbitrary. With this CL we check the entire range of possible names. Maybe someday someone can use the core file to determine that a process was originally named "chrome". BUG=chrome-os-partner:12045 TEST=Ran unittests Change-Id: Ia82d619bd1ee4367129640dc6c7be5ce258a68bb Reviewed-on: https://gerrit.chromium.org/gerrit/31084 Commit-Ready: Michael Krebs Tested-by: Michael Krebs Reviewed-by: Ben Chan --- crash_reporter/user_collector.cc | 81 ++++++++++++++++++++++++++- crash_reporter/user_collector_test.cc | 42 ++++++++++++++ 2 files changed, 121 insertions(+), 2 deletions(-) diff --git a/crash_reporter/user_collector.cc b/crash_reporter/user_collector.cc index fca205848..c96d272e3 100644 --- a/crash_reporter/user_collector.cc +++ b/crash_reporter/user_collector.cc @@ -17,6 +17,7 @@ #include "base/file_util.h" #include "base/logging.h" +#include "base/stl_util.h" #include "base/string_split.h" #include "base/string_util.h" #include "base/stringprintf.h" @@ -526,6 +527,83 @@ bool UserCollector::ParseCrashAttributes(const std::string &crash_attributes, return re.FullMatch(crash_attributes, pid, signal, kernel_supplied_name); } +/* Returns true if the given executable name matches that of Chrome. This + * includes checks for threads that Chrome has renamed. */ +static bool IsChromeExecName(const std::string &exec) { + static const char *kChromeNames[] = { + "chrome", + /* These come from the use of base::PlatformThread::SetName() directly */ + "CrBrowserMain", "CrRendererMain", "CrUtilityMain", "CrPPAPIMain", + "CrPPAPIBrokerMain", "CrPluginMain", "CrWorkerMain", "CrGpuMain", + "BrokerEvent", "CrVideoRenderer", "CrShutdownDetector", + "UsbEventHandler", "CrNaClMain", "CrServiceMain", + /* These thread names come from the use of base::Thread */ + "Gamepad polling thread", "Chrome_InProcGpuThread", + "Chrome_DragDropThread", "Renderer::FILE", "VC manager", + "VideoCaptureModuleImpl", "JavaBridge", "VideoCaptureManagerThread", + "Geolocation", "Geolocation_wifi_provider", + "Device orientation polling thread", "Chrome_InProcRendererThread", + "NetworkChangeNotifier", "Watchdog", "inotify_reader", + "cf_iexplore_background_thread", "BrowserWatchdog", + "Chrome_HistoryThread", "Chrome_SyncThread", "Chrome_ShellDialogThread", + "Printing_Worker", "Chrome_SafeBrowsingThread", "SimpleDBThread", + "D-Bus thread", "AudioThread", "NullAudioThread", "V4L2Thread", + "ChromotingClientDecodeThread", "Profiling_Flush", + "worker_thread_ticker", "AudioMixerAlsa", "AudioMixerCras", + "FakeAudioRecordingThread", "CaptureThread", + "Chrome_WebSocketproxyThread", "ProcessWatcherThread", + "Chrome_CameraThread", "import_thread", "NaCl_IOThread", + "Chrome_CloudPrintJobPrintThread", "Chrome_CloudPrintProxyCoreThread", + "DaemonControllerFileIO", "ChromotingMainThread", + "ChromotingEncodeThread", "ChromotingDesktopThread", + "ChromotingIOThread", "ChromotingFileIOThread", + "Chrome_libJingle_WorkerThread", "Chrome_ChildIOThread", + "GLHelperThread", "RemotingHostPlugin", + // "PAC thread #%d", // not easy to check because of "%d" + "Chrome_DBThread", "Chrome_WebKitThread", "Chrome_FileThread", + "Chrome_FileUserBlockingThread", "Chrome_ProcessLauncherThread", + "Chrome_CacheThread", "Chrome_IOThread", "Cache Thread", "File Thread", + "ServiceProcess_IO", "ServiceProcess_File", + "extension_crash_uploader", "gpu-process_crash_uploader", + "plugin_crash_uploader", "renderer_crash_uploader", + /* These come from the use of webkit_glue::WebThreadImpl */ + "Compositor", "Browser Compositor", + // "WorkerPool/%d", // not easy to check because of "%d" + /* These come from the use of base::Watchdog */ + "Startup watchdog thread Watchdog", "Shutdown watchdog thread Watchdog", + /* These come from the use of AudioDeviceThread::Start */ + "AudioDevice", "AudioInputDevice", + /* These come from the use of MessageLoopFactory::GetMessageLoop */ + "GpuVideoDecoder", "RtcVideoDecoderThread", "PipelineThread", + "AudioDecoderThread", "VideoDecoderThread", + /* These come from the use of MessageLoopFactory::GetMessageLoopProxy */ + "CaptureVideoDecoderThread", "CaptureVideoDecoder", + /* These come from the use of base::SimpleThread */ + "LocalInputMonitor/%d", // "%d" gets lopped off for kernel-supplied + /* These come from the use of base::DelegateSimpleThread */ + "ipc_channel_nacl reader thread/%d", "plugin_audio_input_thread/%d", + "plugin_audio_thread/%d", + /* These come from the use of base::SequencedWorkerPool */ + "BrowserBlockingWorker%d/%d", // "%d" gets lopped off for kernel-supplied + }; + static std::set chrome_names; + + /* Initialize a set of chrome names, for efficient lookup */ + if (chrome_names.empty()) { + for (size_t i = 0; i < arraysize(kChromeNames); i++) { + std::string check_name(kChromeNames[i]); + chrome_names.insert(check_name); + // When checking a kernel-supplied name, it should be truncated to 15 + // chars. See PR_SET_NAME in + // http://www.kernel.org/doc/man-pages/online/pages/man2/prctl.2.html, + // although that page misleads by saying "16 bytes". + chrome_names.insert("supplied_" + std::string(check_name, 0, 15)); + } + } + + return ContainsKey(chrome_names, exec); +} + bool UserCollector::ShouldDump(bool has_owner_consent, bool is_developer, bool handle_chrome_crashes, @@ -536,8 +614,7 @@ bool UserCollector::ShouldDump(bool has_owner_consent, // Treat Chrome crashes as if the user opted-out. We stop counting Chrome // crashes towards user crashes, so user crashes really mean non-Chrome // user-space crashes. - if ((exec == "chrome" || exec == "supplied_chrome") && - !handle_chrome_crashes) { + if (!handle_chrome_crashes && IsChromeExecName(exec)) { *reason = "ignoring - chrome crash"; return false; } diff --git a/crash_reporter/user_collector_test.cc b/crash_reporter/user_collector_test.cc index 8fcbaefd6..290451292 100644 --- a/crash_reporter/user_collector_test.cc +++ b/crash_reporter/user_collector_test.cc @@ -151,6 +151,24 @@ TEST_F(UserCollectorTest, ShouldDumpChromeOverridesDeveloperImage) { EXPECT_FALSE(collector_.ShouldDump(false, false, false, "chrome", &reason)); EXPECT_EQ("ignoring - chrome crash", reason); + EXPECT_FALSE(collector_.ShouldDump(false, false, false, + "supplied_Compositor", &reason)); + EXPECT_EQ("ignoring - chrome crash", reason); + EXPECT_FALSE(collector_.ShouldDump(false, false, false, + "supplied_PipelineThread", &reason)); + EXPECT_EQ("ignoring - chrome crash", reason); + EXPECT_FALSE(collector_.ShouldDump(false, false, false, + "Chrome_ChildIOThread", &reason)); + EXPECT_EQ("ignoring - chrome crash", reason); + EXPECT_FALSE(collector_.ShouldDump(false, false, false, + "supplied_Chrome_ChildIOT", &reason)); + EXPECT_EQ("ignoring - chrome crash", reason); + EXPECT_FALSE(collector_.ShouldDump(false, false, false, + "supplied_ChromotingClien", &reason)); + EXPECT_EQ("ignoring - chrome crash", reason); + EXPECT_FALSE(collector_.ShouldDump(false, false, false, + "supplied_LocalInputMonit", &reason)); + EXPECT_EQ("ignoring - chrome crash", reason); // When running a developer image, test that chrome crashes are handled // when the "handle_chrome_crashes" flag is set. @@ -158,6 +176,30 @@ TEST_F(UserCollectorTest, ShouldDumpChromeOverridesDeveloperImage) { "chrome", &reason)); EXPECT_EQ("developer build - not testing - always dumping", reason); + EXPECT_TRUE(collector_.ShouldDump(false, true, true, + "supplied_Compositor", &reason)); + EXPECT_EQ("developer build - not testing - always dumping", + reason); + EXPECT_TRUE(collector_.ShouldDump(false, true, true, + "supplied_PipelineThread", &reason)); + EXPECT_EQ("developer build - not testing - always dumping", + reason); + EXPECT_TRUE(collector_.ShouldDump(false, true, true, + "Chrome_ChildIOThread", &reason)); + EXPECT_EQ("developer build - not testing - always dumping", + reason); + EXPECT_TRUE(collector_.ShouldDump(false, true, true, + "supplied_Chrome_ChildIOT", &reason)); + EXPECT_EQ("developer build - not testing - always dumping", + reason); + EXPECT_TRUE(collector_.ShouldDump(false, true, true, + "supplied_ChromotingClien", &reason)); + EXPECT_EQ("developer build - not testing - always dumping", + reason); + EXPECT_TRUE(collector_.ShouldDump(false, true, true, + "supplied_LocalInputMonit", &reason)); + EXPECT_EQ("developer build - not testing - always dumping", + reason); } TEST_F(UserCollectorTest, ShouldDumpUseConsentProductionImage) {