3party/crashpad
0
0
Fork 0
mirror of https://github.com/chromium/crashpad.git synced 2025-03-09 14:06:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

mac: Scope crashpad_handler’s SIGTERM handler more broadly

Browse Source 
Previously, there was a window after starting the upload thread but
before the SIGTERM handler was installed, where receipt of SIGTERM
could have interrupted an in-progress upload. There was also the
possibility that a second SIGTERM sent after the exception handler
stopped running would interrupt an in-progress upload. By pulling the
signal handler out of ExceptionHandlerServer and into the main
function, these races are avoided.

BUG=crashpad:25
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1429353002 .
This commit is contained in:
Mark Mentovai 2015-11-09 16:29:25 -05:00
parent ee58effe14
commit 4a7d599b64
3 changed files with 106 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
handler/handler_main.cc
View File

@ -20,10 +20,12 @@
#include <map>
#include <string>
#include "base/auto_reset.h"
#include "base/files/file_path.h"
#include "base/files/scoped_file.h"
#include "base/logging.h"
#include "base/memory/scoped_ptr.h"
#include "base/scoped_generic.h"
#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "client/crash_report_database.h"
@ -38,6 +40,8 @@
#if defined(OS_MACOSX)
#include <libgen.h>
#include <signal.h>
#include "base/mac/scoped_mach_port.h"
#include "handler/mac/crash_report_exception_handler.h"
#include "handler/mac/exception_handler_server.h"
@ -46,6 +50,7 @@
#include "util/posix/close_stdio.h"
#elif defined(OS_WIN)
#include <windows.h>
#include "handler/win/crash_report_exception_handler.h"
#include "util/win/exception_handler_server.h"
#include "util/win/handle.h"
@ -80,6 +85,31 @@ void Usage(const base::FilePath& me) {
ToolSupport::UsageTail(me);
}
#if defined(OS_MACOSX)
struct ResetSIGTERMTraits {
static struct sigaction* InvalidValue() {
return nullptr;
}
static void Free(struct sigaction* sa) {
int rv = sigaction(SIGTERM, sa, nullptr);
PLOG_IF(ERROR, rv != 0) << "sigaction";
}
};
using ScopedResetSIGTERM =
base::ScopedGeneric<struct sigaction*, ResetSIGTERMTraits>;
ExceptionHandlerServer* g_exception_handler_server;
// This signal handler is only operative when being run from launchd.
void HandleSIGTERM(int sig, siginfo_t* siginfo, void* context) {
DCHECK(g_exception_handler_server);
g_exception_handler_server->Stop();
}
#endif // OS_MACOSX
} // namespace
int HandlerMain(int argc, char* argv[]) {
@ -285,6 +315,26 @@ int HandlerMain(int argc, char* argv[]) {
ExceptionHandlerServer exception_handler_server(
receive_right.Pass(), !options.mach_service.empty());
base::AutoReset<ExceptionHandlerServer*> reset_g_exception_handler_server(
&g_exception_handler_server, &exception_handler_server);
struct sigaction old_sa;
ScopedResetSIGTERM reset_sigterm;
if (!options.mach_service.empty()) {
// When running from launchd, no no-senders notification could ever be
// triggered, because launchd maintains a send right to the service. When
// launchd wants the job to exit, it will send a SIGTERM. See
// launchd.plist(5).
//
// Set up a SIGTERM handler that will call exception_handler_server.Stop().
struct sigaction sa = {};
sigemptyset(&sa.sa_mask);
sa.sa_flags = SA_SIGINFO;
sa.sa_sigaction = HandleSIGTERM;
int rv = sigaction(SIGTERM, &sa, &old_sa);
PCHECK(rv == 0) << "sigaction";
reset_sigterm.reset(&old_sa);
}
#elif defined(OS_WIN)
ExceptionHandlerServer exception_handler_server(!options.pipe_name.empty());

124
handler/mac/exception_handler_server.cc
View File

@ -14,12 +14,7 @@
#include "handler/mac/exception_handler_server.h"
#include <signal.h>
#include "base/auto_reset.h"
#include "base/logging.h"
#include "base/memory/scoped_ptr.h"
#include "base/scoped_generic.h"
#include "base/mac/mach_logging.h"
#include "util/mach/composite_mach_message_server.h"
#include "util/mach/mach_extensions.h"
@ -31,58 +26,12 @@ namespace crashpad {
namespace {
struct ResetSIGTERMTraits {
static struct sigaction* InvalidValue() {
return nullptr;
}
static void Free(struct sigaction* sa) {
int rv = sigaction(SIGTERM, sa, nullptr);
PLOG_IF(ERROR, rv != 0) << "sigaction";
}
};
using ScopedResetSIGTERM =
base::ScopedGeneric<struct sigaction*, ResetSIGTERMTraits>;
mach_port_t g_signal_notify_port;
// This signal handler is only operative when being run from launchd. It causes
// the exception handler server to stop running by sending it a synthesized
// no-senders notification.
void HandleSIGTERM(int sig, siginfo_t* siginfo, void* context) {
DCHECK(g_signal_notify_port);
// mach_no_senders_notification_t defines the receive side of this structure,
// with a trailer element that’s undesirable for the send side.
struct {
mach_msg_header_t header;
NDR_record_t ndr;
mach_msg_type_number_t mscount;
} no_senders_notification = {};
no_senders_notification.header.msgh_bits =
MACH_MSGH_BITS(MACH_MSG_TYPE_MAKE_SEND_ONCE, 0);
no_senders_notification.header.msgh_size = sizeof(no_senders_notification);
no_senders_notification.header.msgh_remote_port = g_signal_notify_port;
no_senders_notification.header.msgh_local_port = MACH_PORT_NULL;
no_senders_notification.header.msgh_id = MACH_NOTIFY_NO_SENDERS;
no_senders_notification.ndr = NDR_record;
no_senders_notification.mscount = 0;
kern_return_t kr = mach_msg(&no_senders_notification.header,
MACH_SEND_MSG,
sizeof(no_senders_notification),
0,
MACH_PORT_NULL,
MACH_MSG_TIMEOUT_NONE,
MACH_PORT_NULL);
MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_msg";
}
class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
public NotifyServer::DefaultInterface {
public:
ExceptionHandlerServerRun(
mach_port_t exception_port,
mach_port_t notify_port,
bool launchd,
UniversalMachExcServer::Interface* exception_interface)
: UniversalMachExcServer::Interface(),
@ -92,11 +41,9 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
composite_mach_message_server_(),
exception_interface_(exception_interface),
exception_port_(exception_port),
notify_port_(NewMachPort(MACH_PORT_RIGHT_RECEIVE)),
notify_port_(notify_port),
running_(true),
launchd_(launchd) {
CHECK(notify_port_.is_valid());
composite_mach_message_server_.AddHandler(&mach_exc_server_);
composite_mach_message_server_.AddHandler(&notify_server_);
}
@ -108,9 +55,6 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
DCHECK(running_);
kern_return_t kr;
scoped_ptr<base::AutoReset<mach_port_t>> reset_signal_notify_port;
struct sigaction old_sa;
ScopedResetSIGTERM reset_sigterm;
if (!launchd_) {
// Request that a no-senders notification for exception_port_ be sent to
// notify_port_.
@ -119,33 +63,11 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
exception_port_,
MACH_NOTIFY_NO_SENDERS,
0,
notify_port_.get(),
notify_port_,
MACH_MSG_TYPE_MAKE_SEND_ONCE,
&previous);
MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_port_request_notification";
if (previous != MACH_PORT_NULL) {
kr = mach_port_deallocate(mach_task_self(), previous);
MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_port_deallocate";
}
} else {
// A real no-senders notification would never be triggered, because
// launchd maintains a send right to the service. When launchd wants the
// job to exit, it will send a SIGTERM. See launchd.plist(5).
//
// Set up a SIGTERM handler that will cause Run() to return (incidentally,
// by sending a synthetic no-senders notification).
struct sigaction sa = {};
sigemptyset(&sa.sa_mask);
sa.sa_flags = SA_SIGINFO;
sa.sa_sigaction = HandleSIGTERM;
int rv = sigaction(SIGTERM, &sa, &old_sa);
PCHECK(rv == 0) << "sigaction";
reset_sigterm.reset(&old_sa);
DCHECK(!g_signal_notify_port);
reset_signal_notify_port.reset(new base::AutoReset<mach_port_t>(
&g_signal_notify_port, notify_port_.get()));
base::mac::ScopedMachSendRight previous_owner(previous);
}
// A single CompositeMachMessageServer will dispatch both exception messages
@ -165,7 +87,7 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_port_insert_member";
kr = mach_port_insert_member(
mach_task_self(), notify_port_.get(), server_port_set.get());
mach_task_self(), notify_port_, server_port_set.get());
MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_port_insert_member";
// Run the server in kOneShot mode so that running_ can be reevaluated after
@ -249,7 +171,7 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
CompositeMachMessageServer composite_mach_message_server_;
UniversalMachExcServer::Interface* exception_interface_; // weak
mach_port_t exception_port_; // weak
base::mac::ScopedMachReceiveRight notify_port_;
mach_port_t notify_port_; // weak
bool running_;
bool launchd_;
@ -262,8 +184,10 @@ ExceptionHandlerServer::ExceptionHandlerServer(
base::mac::ScopedMachReceiveRight receive_port,
bool launchd)
: receive_port_(receive_port.Pass()),
notify_port_(NewMachPort(MACH_PORT_RIGHT_RECEIVE)),
launchd_(launchd) {
CHECK(receive_port_.is_valid());
CHECK(notify_port_.is_valid());
}
ExceptionHandlerServer::~ExceptionHandlerServer() {
@ -272,8 +196,38 @@ ExceptionHandlerServer::~ExceptionHandlerServer() {
void ExceptionHandlerServer::Run(
UniversalMachExcServer::Interface* exception_interface) {
ExceptionHandlerServerRun run(
receive_port_.get(), launchd_, exception_interface);
receive_port_.get(), notify_port_.get(), launchd_, exception_interface);
run.Run();
}
void ExceptionHandlerServer::Stop() {
// Cause the exception handler server to stop running by sending it a
// synthesized no-senders notification.
//
// mach_no_senders_notification_t defines the receive side of this structure,
// with a trailer element that’s undesirable for the send side.
struct {
mach_msg_header_t header;
NDR_record_t ndr;
mach_msg_type_number_t mscount;
} no_senders_notification = {};
no_senders_notification.header.msgh_bits =
MACH_MSGH_BITS(MACH_MSG_TYPE_MAKE_SEND_ONCE, 0);
no_senders_notification.header.msgh_size = sizeof(no_senders_notification);
no_senders_notification.header.msgh_remote_port = notify_port_.get();
no_senders_notification.header.msgh_local_port = MACH_PORT_NULL;
no_senders_notification.header.msgh_id = MACH_NOTIFY_NO_SENDERS;
no_senders_notification.ndr = NDR_record;
no_senders_notification.mscount = 0;
kern_return_t kr = mach_msg(&no_senders_notification.header,
MACH_SEND_MSG,
sizeof(no_senders_notification),
0,
MACH_PORT_NULL,
MACH_MSG_TIMEOUT_NONE,
MACH_PORT_NULL);
MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_msg";
}
} // namespace crashpad

24
handler/mac/exception_handler_server.h
View File

@ -34,8 +34,8 @@ class ExceptionHandlerServer {
//! notifications will be received on.
//! \param[in] launchd If `true`, the exception handler is being run from
//! launchd. \a receive_port is not monitored for no-senders
//! notifications, and instead, the expected “quit” signal is receipt of
//! `SIGTERM`.
//! notifications, and instead, Stop() must be called to provide a “quit”
//! signal.
ExceptionHandlerServer(base::mac::ScopedMachReceiveRight receive_port,
bool launchd);
~ExceptionHandlerServer();
@ -47,11 +47,10 @@ class ExceptionHandlerServer {
//! This method monitors the receive port for exception messages and, if
//! not being run by launchd, no-senders notifications. It continues running
//! until it has no more clients, indicated by the receipt of a no-senders
//! notification, or if being run by launchd, receipt of `SIGTERM`. When not
//! being run by launchd, it is important to assure that a send right exists
//! in a client (or has been queued by `mach_msg()` to be sent to a client)
//! prior to calling this method, or it will detect that it is sender-less and
//! return immediately.
//! notification, or until Stop() is called. When not being run by launchd, it
//! is important to assure that a send right exists in a client (or has been
//! queued by `mach_msg()` to be sent to a client) prior to calling this
//! method, or it will detect that it is sender-less and return immediately.
//!
//! All exception messages will be passed to \a exception_interface.
//!
@ -63,8 +62,19 @@ class ExceptionHandlerServer {
//! this method will continue running normally.
void Run(UniversalMachExcServer::Interface* exception_interface);
//! \brief Stops a running exception-handling server.
//!
//! The normal mode of operation is to call Stop() while Run() is running. It
//! is expected that Stop() would be called from a signal handler.
//!
//! If Stop() is called before Run() it will cause Run() to return as soon as
//! it is called. It is harmless to call Stop() after Run() has already
//! returned, or to call Stop() after it has already been called.
void Stop();
private:
base::mac::ScopedMachReceiveRight receive_port_;
base::mac::ScopedMachReceiveRight notify_port_;
bool launchd_;
DISALLOW_COPY_AND_ASSIGN(ExceptionHandlerServer);