From 8137f3170df7543e5213c051443f3157d168d859 Mon Sep 17 00:00:00 2001 From: "thestig@chromium.org" Date: Thu, 2 Sep 2010 21:28:50 +0000 Subject: Linux: Handle renderer and plugin crashes on a separate thread. Also set retries / timeouts for wget to attempt to limit the duration of each crash handler. Move some code out of headers while we're at it. BUG=54071 TEST=change /usr/bin/wget to sleep forever, visit about:crash in a Breakpad-enabled build, make sure other renderers continue to work. Review URL: http://codereview.chromium.org/3308007 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@58403 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/app/breakpad_linux.cc | 2 + chrome/browser/crash_handler_host_linux.cc | 107 ++++++++++++++++++++++------- chrome/browser/crash_handler_host_linux.h | 30 ++++---- 3 files changed, 100 insertions(+), 39 deletions(-) diff --git a/chrome/app/breakpad_linux.cc b/chrome/app/breakpad_linux.cc index 42d7b93..3619a71 100644 --- a/chrome/app/breakpad_linux.cc +++ b/chrome/app/breakpad_linux.cc @@ -558,6 +558,8 @@ pid_t HandleCrashDump(const BreakpadInfo& info) { header, post_file, kUploadURL, + "--timeout=10", // Set a timeout so we don't hang forever. + "--tries=1", // Don't retry if the upload fails. "-O", // output reply to fd 3 "/dev/fd/3", NULL, diff --git a/chrome/browser/crash_handler_host_linux.cc b/chrome/browser/crash_handler_host_linux.cc index e46c60d..06ceb96 100644 --- a/chrome/browser/crash_handler_host_linux.cc +++ b/chrome/browser/crash_handler_host_linux.cc @@ -20,6 +20,8 @@ #include "base/path_service.h" #include "base/rand_util.h" #include "base/string_util.h" +#include "base/task.h" +#include "base/thread.h" #include "breakpad/src/client/linux/handler/exception_handler.h" #include "breakpad/src/client/linux/minidump_writer/linux_dumper.h" #include "breakpad/src/client/linux/minidump_writer/minidump_writer.h" @@ -30,14 +32,27 @@ using google_breakpad::ExceptionHandler; +namespace { + +// Handles the crash dump and frees the allocated BreakpadInfo struct. +void CrashDumpTask(BreakpadInfo* info) { + HandleCrashDump(*info); + delete[] info->filename; + delete[] info->process_type; + delete[] info->crash_url; + delete[] info->guid; + delete[] info->distro; + delete info; +} + +} // namespace + // Since classes derived from CrashHandlerHostLinux are singletons, it's only // destroyed at the end of the processes lifetime, which is greater in span than // the lifetime of the IO message loop. DISABLE_RUNNABLE_METHOD_REFCOUNT(CrashHandlerHostLinux); -CrashHandlerHostLinux::CrashHandlerHostLinux() - : process_socket_(-1), - browser_socket_(-1) { +CrashHandlerHostLinux::CrashHandlerHostLinux() { int fds[2]; // We use SOCK_SEQPACKET rather than SOCK_DGRAM to prevent the process from // sending datagrams to other sockets on the system. The sandbox may prevent @@ -62,9 +77,17 @@ CrashHandlerHostLinux::CrashHandlerHostLinux() CrashHandlerHostLinux::~CrashHandlerHostLinux() { HANDLE_EINTR(close(process_socket_)); HANDLE_EINTR(close(browser_socket_)); + + // If we are quitting and there are crash dumps in the queue, discard them. + uploader_thread_->message_loop()->QuitNow(); } void CrashHandlerHostLinux::Init() { + SetProcessType(); + uploader_thread_.reset( + new base::Thread(std::string(process_type_ + "_crash_uploader").c_str())); + uploader_thread_->Start(); + MessageLoopForIO* ml = MessageLoopForIO::current(); CHECK(ml->WatchFileDescriptor( browser_socket_, true /* persistent */, @@ -95,24 +118,26 @@ void CrashHandlerHostLinux::OnFileCanReadWithoutBlocking(int fd) { struct msghdr msg = {0}; struct iovec iov[6]; char crash_context[kCrashContextSize]; - char guid[kGuidSize + 1]; - char crash_url[kMaxActiveURLSize + 1]; - char distro[kDistroSize + 1]; + char* guid = new char[kGuidSize + 1]; + char* crash_url = new char[kMaxActiveURLSize + 1]; + char* distro = new char[kDistroSize + 1]; char* tid_buf_addr = NULL; int tid_fd = -1; char control[kControlMsgSize]; - const ssize_t expected_msg_size = sizeof(crash_context) + sizeof(guid) + - sizeof(crash_url) + sizeof(distro) + + const ssize_t expected_msg_size = sizeof(crash_context) + + kGuidSize + 1 + + kMaxActiveURLSize + 1 + + kDistroSize + 1 + sizeof(tid_buf_addr) + sizeof(tid_fd); iov[0].iov_base = crash_context; iov[0].iov_len = sizeof(crash_context); iov[1].iov_base = guid; - iov[1].iov_len = sizeof(guid); + iov[1].iov_len = kGuidSize + 1; iov[2].iov_base = crash_url; - iov[2].iov_len = sizeof(crash_url); + iov[2].iov_len = kMaxActiveURLSize + 1; iov[3].iov_base = distro; - iov[3].iov_len = sizeof(distro); + iov[3].iov_len = kDistroSize + 1; iov[4].iov_base = &tid_buf_addr; iov[4].iov_len = sizeof(tid_buf_addr); iov[5].iov_base = &tid_fd; @@ -247,6 +272,7 @@ void CrashHandlerHostLinux::OnFileCanReadWithoutBlocking(int fd) { bool upload = true; FilePath dumps_path("/tmp"); + PathService::Get(base::DIR_TEMP, &dumps_path); if (getenv(env_vars::kHeadless)) { upload = false; PathService::Get(chrome::DIR_CRASH_DUMPS, &dumps_path); @@ -276,20 +302,55 @@ void CrashHandlerHostLinux::OnFileCanReadWithoutBlocking(int fd) { // Sanitize the string data a bit more guid[kGuidSize] = crash_url[kMaxActiveURLSize] = distro[kDistroSize] = 0; - BreakpadInfo info; - info.filename = minidump_filename.c_str(); - info.process_type = process_type_.c_str(); - info.process_type_length = process_type_.length(); - info.crash_url = crash_url; - info.crash_url_length = strlen(crash_url); - info.guid = guid; - info.guid_length = strlen(guid); - info.distro = distro; - info.distro_length = strlen(distro); - info.upload = upload; - HandleCrashDump(info); + BreakpadInfo* info = new BreakpadInfo; + + char* minidump_filename_str = new char[minidump_filename.length() + 1]; + minidump_filename.copy(minidump_filename_str, minidump_filename.length()); + minidump_filename_str[minidump_filename.length()] = '\0'; + info->filename = minidump_filename_str; + + info->process_type_length = process_type_.length(); + char* process_type_str = new char[info->process_type_length + 1]; + process_type_.copy(process_type_str, info->process_type_length); + process_type_str[info->process_type_length] = '\0'; + info->process_type = process_type_str; + + info->crash_url_length = strlen(crash_url); + info->crash_url = crash_url; + + info->guid_length = strlen(guid); + info->guid = guid; + + info->distro_length = strlen(distro); + info->distro = distro; + + info->upload = upload; + + uploader_thread_->message_loop()->PostTask( + FROM_HERE, + NewRunnableFunction(&CrashDumpTask, info)); } void CrashHandlerHostLinux::WillDestroyCurrentMessageLoop() { file_descriptor_watcher_.StopWatchingFileDescriptor(); } + +PluginCrashHandlerHostLinux::PluginCrashHandlerHostLinux() { +} + +PluginCrashHandlerHostLinux::~PluginCrashHandlerHostLinux() { +} + +void PluginCrashHandlerHostLinux::SetProcessType() { + process_type_ = "plugin"; +} + +RendererCrashHandlerHostLinux::RendererCrashHandlerHostLinux() { +} + +RendererCrashHandlerHostLinux::~RendererCrashHandlerHostLinux() { +} + +void RendererCrashHandlerHostLinux::SetProcessType() { + process_type_ = "renderer"; +} diff --git a/chrome/browser/crash_handler_host_linux.h b/chrome/browser/crash_handler_host_linux.h index 625ac8e..7aa8dbe 100644 --- a/chrome/browser/crash_handler_host_linux.h +++ b/chrome/browser/crash_handler_host_linux.h @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -8,8 +8,13 @@ #include -#include "base/singleton.h" #include "base/message_loop.h" +#include "base/scoped_ptr.h" +#include "base/singleton.h" + +namespace base { +class Thread; +} // This is the base class for singleton objects which crash dump renderers and // plugins on Linux. We perform the crash dump from the browser because it @@ -51,6 +56,7 @@ class CrashHandlerHostLinux : public MessageLoopForIO::Watcher, int process_socket_; int browser_socket_; MessageLoopForIO::FileDescriptorWatcher file_descriptor_watcher_; + scoped_ptr uploader_thread_; DISALLOW_COPY_AND_ASSIGN(CrashHandlerHostLinux); }; @@ -58,14 +64,10 @@ class CrashHandlerHostLinux : public MessageLoopForIO::Watcher, class PluginCrashHandlerHostLinux : public CrashHandlerHostLinux { private: friend struct DefaultSingletonTraits; - PluginCrashHandlerHostLinux() { - SetProcessType(); - } - virtual ~PluginCrashHandlerHostLinux() {} + PluginCrashHandlerHostLinux(); + virtual ~PluginCrashHandlerHostLinux(); - virtual void SetProcessType() { - process_type_ = "plugin"; - } + virtual void SetProcessType(); DISALLOW_COPY_AND_ASSIGN(PluginCrashHandlerHostLinux); }; @@ -73,14 +75,10 @@ class PluginCrashHandlerHostLinux : public CrashHandlerHostLinux { class RendererCrashHandlerHostLinux : public CrashHandlerHostLinux { private: friend struct DefaultSingletonTraits; - RendererCrashHandlerHostLinux() { - SetProcessType(); - } - virtual ~RendererCrashHandlerHostLinux() {} + RendererCrashHandlerHostLinux(); + virtual ~RendererCrashHandlerHostLinux(); - virtual void SetProcessType() { - process_type_ = "renderer"; - } + virtual void SetProcessType(); DISALLOW_COPY_AND_ASSIGN(RendererCrashHandlerHostLinux); }; -- cgit v1.1