From ee6fc23fb3e436e0a7202a6b3059d484917033a4 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Tue, 3 Nov 2015 19:13:48 -0500 Subject: [PATCH] mac: Restart crashpad_handler from the initial client if it dies BUG=crashpad:68 R=rsesek@chromium.org Review URL: https://codereview.chromium.org/1413033007 . --- client/crashpad_client.h | 7 +- client/crashpad_client_mac.cc | 257 +++++++++++++++++++++++++++-- client/crashpad_client_win.cc | 3 +- handler/win/crashy_test_program.cc | 3 +- tools/mac/run_with_crashpad.cc | 3 +- 5 files changed, 257 insertions(+), 16 deletions(-) diff --git a/client/crashpad_client.h b/client/crashpad_client.h index 18b7b131..05766fba 100644 --- a/client/crashpad_client.h +++ b/client/crashpad_client.h @@ -63,13 +63,18 @@ class CrashpadClient { //! Arguments passed in other parameters and arguments required to perform //! the handshake are the responsibility of this method, and must not be //! specified in this parameter. + //! \param[in] restartable If `true`, the handler will be restarted if it + //! dies, if this behavior is supported. This option is not available on + //! all platforms, and does not function on all OS versions. If it is + //! not supported, it will be ignored. //! //! \return `true` on success, `false` on failure with a message logged. bool StartHandler(const base::FilePath& handler, const base::FilePath& database, const std::string& url, const std::map& annotations, - const std::vector& arguments); + const std::vector& arguments, + bool restartable); #if defined(OS_WIN) || DOXYGEN //! \brief Sets the IPC pipe of a presumably-running Crashpad handler process diff --git a/client/crashpad_client_mac.cc b/client/crashpad_client_mac.cc index caf7e91b..bb6a7805 100644 --- a/client/crashpad_client_mac.cc +++ b/client/crashpad_client_mac.cc @@ -14,7 +14,9 @@ #include "client/crashpad_client.h" +#include #include +#include #include #include @@ -22,9 +24,13 @@ #include "base/mac/mach_logging.h" #include "base/posix/eintr_wrapper.h" #include "base/strings/stringprintf.h" +#include "util/mac/mac_util.h" #include "util/mach/child_port_handshake.h" #include "util/mach/exception_ports.h" #include "util/mach/mach_extensions.h" +#include "util/mach/mach_message.h" +#include "util/mach/notify_server.h" +#include "util/misc/clock.h" #include "util/misc/implicit_cast.h" #include "util/posix/close_multiple.h" @@ -79,24 +85,45 @@ bool SetCrashExceptionPorts(exception_handler_t exception_handler) { MACHINE_THREAD_STATE); } -//! \brief Starts a Crashpad handler. -class HandlerStarter final { +class ScopedPthreadAttrDestroy { public: - //! \brief Starts a Crashpad handler. + explicit ScopedPthreadAttrDestroy(pthread_attr_t* pthread_attr) + : pthread_attr_(pthread_attr) { + } + + ~ScopedPthreadAttrDestroy() { + errno = pthread_attr_destroy(pthread_attr_); + PLOG_IF(WARNING, errno != 0) << "pthread_attr_destroy"; + } + + private: + pthread_attr_t* pthread_attr_; + + DISALLOW_COPY_AND_ASSIGN(ScopedPthreadAttrDestroy); +}; + +//! \brief Starts a Crashpad handler, possibly restarting it if it dies. +class HandlerStarter final : public NotifyServer::DefaultInterface { + public: + ~HandlerStarter() {} + + //! \brief Starts a Crashpad handler initially, as opposed to starting it for + //! subsequent restarts. //! //! All parameters are as in CrashpadClient::StartHandler(). //! //! \return On success, a send right to the Crashpad handler that has been //! started. On failure, `MACH_PORT_NULL` with a message logged. - static base::mac::ScopedMachSendRight Start( + static base::mac::ScopedMachSendRight InitialStart( const base::FilePath& handler, const base::FilePath& database, const std::string& url, const std::map& annotations, - const std::vector& arguments) { + const std::vector& arguments, + bool restartable) { base::mac::ScopedMachReceiveRight receive_right( NewMachPort(MACH_PORT_RIGHT_RECEIVE)); - if (receive_right == kMachPortNull) { + if (!receive_right.is_valid()) { return base::mac::ScopedMachSendRight(); } @@ -116,25 +143,149 @@ class HandlerStarter final { DCHECK_EQ(right_type, implicit_cast(MACH_MSG_TYPE_PORT_SEND)); + scoped_ptr handler_restarter; + if (restartable) { + handler_restarter.reset(new HandlerStarter()); + if (!handler_restarter->notify_port_.is_valid()) { + // This is an error that NewMachPort() would have logged. Proceed anyway + // without the ability to restart. + handler_restarter.reset(); + } + } + if (!CommonStart(handler, database, url, annotations, arguments, - receive_right.Pass())) { + receive_right.Pass(), + handler_restarter.get(), + false)) { return base::mac::ScopedMachSendRight(); } + if (handler_restarter && handler_restarter->StartRestartThread( + handler, database, url, annotations, arguments)) { + // The thread owns the object now. + ignore_result(handler_restarter.release()); + } + + // If StartRestartThread() failed, proceed without the ability to restart. + // handler_restarter will be released when this function returns. + return send_right; } + // NotifyServer::DefaultInterface: + + kern_return_t DoMachNotifyPortDestroyed(notify_port_t notify, + mach_port_t rights, + const mach_msg_trailer_t* trailer, + bool* destroy_request) override { + // The receive right corresponding to this process’ crash exception port is + // now owned by this process. Any crashes that occur before the receive + // right is moved to a new handler process will cause the process to hang in + // an unkillable state prior to OS X 10.10. + + if (notify != notify_port_) { + LOG(WARNING) << "notify port mismatch"; + return KERN_FAILURE; + } + + // If CommonStart() fails, the receive right will die, and this will just + // be called again for another try. + CommonStart(handler_, + database_, + url_, + annotations_, + arguments_, + base::mac::ScopedMachReceiveRight(rights), + this, + true); + + return KERN_SUCCESS; + } + private: + HandlerStarter() + : NotifyServer::DefaultInterface(), + handler_(), + database_(), + url_(), + annotations_(), + arguments_(), + notify_port_(NewMachPort(MACH_PORT_RIGHT_RECEIVE)), + last_start_time_(0) { + } + + //! \brief Starts a Crashpad handler. + //! + //! All parameters are as in CrashpadClient::StartHandler(), with these + //! additions: + //! + //! \param[in] receive_right The receive right to move to the Crashpad + //! handler. The handler will use this receive right to run its exception + //! server. + //! \param[in] handler_restarter If CrashpadClient::StartHandler() was invoked + //! with \a restartable set to `true`, this is the restart state object. + //! Otherwise, this is `nullptr`. + //! \param[in] restart If CrashpadClient::StartHandler() was invoked with \a + //! restartable set to `true` and CommonStart() is being called to restart + //! a previously-started handler, this is `true`. Otherwise, this is + //! `false`. + //! + //! \return `true` on success, `false` on failure, with a message logged. + //! Failures include failure to start the handler process and failure to + //! rendezvous with it via ChildPortHandshake. static bool CommonStart(const base::FilePath& handler, const base::FilePath& database, const std::string& url, const std::map& annotations, const std::vector& arguments, - base::mac::ScopedMachReceiveRight receive_right) { + base::mac::ScopedMachReceiveRight receive_right, + HandlerStarter* handler_restarter, + bool restart) { + if (handler_restarter) { + // The port-destroyed notification must be requested each time. It uses + // a send-once right, so once the notification is received, it won’t be + // sent again unless re-requested. + mach_port_t previous; + kern_return_t kr = + mach_port_request_notification(mach_task_self(), + receive_right.get(), + MACH_NOTIFY_PORT_DESTROYED, + 0, + handler_restarter->notify_port_.get(), + MACH_MSG_TYPE_MAKE_SEND_ONCE, + &previous); + if (kr != KERN_SUCCESS) { + MACH_LOG(WARNING, kr) << "mach_port_request_notification"; + + // This will cause the restart thread to terminate after this restart + // attempt. There’s no longer any need for it, because no more + // port-destroyed notifications can be delivered. + handler_restarter->notify_port_.reset(); + } else { + base::mac::ScopedMachSendRight previous_owner(previous); + DCHECK(restart || !previous_owner.is_valid()); + } + + if (handler_restarter->last_start_time_) { + // If the handler was ever started before, don’t restart it too quickly. + const uint64_t kNanosecondsPerSecond = 1E9; + const uint64_t kMinimumStartInterval = 1 * kNanosecondsPerSecond; + + const uint64_t earliest_next_start_time = + handler_restarter->last_start_time_ + kMinimumStartInterval; + const uint64_t now_time = ClockMonotonicNanoseconds(); + if (earliest_next_start_time > now_time) { + SleepNanoseconds(earliest_next_start_time - now_time); + } + } + + handler_restarter->last_start_time_ = ClockMonotonicNanoseconds(); + } + // Set up the arguments for execve() first. These aren’t needed until // execve() is called, but it’s dangerous to do this in a child process // after fork(). @@ -195,6 +346,15 @@ class HandlerStarter final { if (pid == 0) { // Child process. + if (restart) { + // When restarting, reset the system default crash handler first. + // Otherwise, the crash exception port here will have been inherited + // from the parent process, which was probably using the exception + // server now being restarted. The handler can’t monitor itself for its + // own crashes via this interface. + CrashpadClient::UseSystemDefaultHandler(); + } + // Call setsid(), creating a new process group and a new session, both led // by this process. The new process group has no controlling terminal. // This disconnects it from signals generated by the parent process’ @@ -269,7 +429,71 @@ class HandlerStarter final { return true; } - DISALLOW_IMPLICIT_CONSTRUCTORS(HandlerStarter); + bool StartRestartThread(const base::FilePath& handler, + const base::FilePath& database, + const std::string& url, + const std::map& annotations, + const std::vector& arguments) { + handler_ = handler; + database_ = database; + url_ = url; + annotations_ = annotations; + arguments_ = arguments; + + pthread_attr_t pthread_attr; + errno = pthread_attr_init(&pthread_attr); + if (errno != 0) { + PLOG(WARNING) << "pthread_attr_init"; + return false; + } + ScopedPthreadAttrDestroy pthread_attr_owner(&pthread_attr); + + errno = pthread_attr_setdetachstate(&pthread_attr, PTHREAD_CREATE_DETACHED); + if (errno != 0) { + PLOG(WARNING) << "pthread_attr_setdetachstate"; + return false; + } + + pthread_t pthread; + errno = pthread_create(&pthread, &pthread_attr, RestartThreadMain, this); + if (errno != 0) { + PLOG(WARNING) << "pthread_create"; + return false; + } + + return true; + } + + static void* RestartThreadMain(void* argument) { + HandlerStarter* self = reinterpret_cast(argument); + + NotifyServer notify_server(self); + mach_msg_return_t mr; + do { + mr = MachMessageServer::Run(¬ify_server, + self->notify_port_.get(), + 0, + MachMessageServer::kPersistent, + MachMessageServer::kReceiveLargeError, + kMachMessageTimeoutWaitIndefinitely); + MACH_LOG_IF(ERROR, mr != MACH_MSG_SUCCESS, mr) + << "MachMessageServer::Run"; + } while (self->notify_port_.is_valid() && mr == MACH_MSG_SUCCESS); + + delete self; + + return nullptr; + } + + base::FilePath handler_; + base::FilePath database_; + std::string url_; + std::map annotations_; + std::vector arguments_; + base::mac::ScopedMachReceiveRight notify_port_; + uint64_t last_start_time_; + + DISALLOW_COPY_AND_ASSIGN(HandlerStarter); }; } // namespace @@ -286,11 +510,20 @@ bool CrashpadClient::StartHandler( const base::FilePath& database, const std::string& url, const std::map& annotations, - const std::vector& arguments) { + const std::vector& arguments, + bool restartable) { DCHECK(!exception_port_.is_valid()); - exception_port_ = HandlerStarter::Start( - handler, database, url, annotations, arguments); + // The “restartable” behavior can only be selected on OS X 10.10 and later. In + // previous OS versions, if the initial client were to crash while attempting + // to restart the handler, it would become an unkillable process. + exception_port_ = HandlerStarter::InitialStart( + handler, + database, + url, + annotations, + arguments, + restartable && MacOSXMinorVersion() >= 10); if (!exception_port_.is_valid()) { return false; } diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index 3f23dde1..01df466f 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -126,7 +126,8 @@ bool CrashpadClient::StartHandler( const base::FilePath& database, const std::string& url, const std::map& annotations, - const std::vector& arguments) { + const std::vector& arguments, + bool restartable) { DCHECK(ipc_pipe_.empty()); std::string ipc_pipe = diff --git a/handler/win/crashy_test_program.cc b/handler/win/crashy_test_program.cc index 7d61c256..921638d9 100644 --- a/handler/win/crashy_test_program.cc +++ b/handler/win/crashy_test_program.cc @@ -106,7 +106,8 @@ int CrashyMain(int argc, wchar_t* argv[]) { base::FilePath(argv[2]), std::string(), std::map(), - std::vector())) { + std::vector(), + false)) { LOG(ERROR) << "StartHandler"; return EXIT_FAILURE; } diff --git a/tools/mac/run_with_crashpad.cc b/tools/mac/run_with_crashpad.cc index 0b4653aa..58337aad 100644 --- a/tools/mac/run_with_crashpad.cc +++ b/tools/mac/run_with_crashpad.cc @@ -166,7 +166,8 @@ int RunWithCrashpadMain(int argc, char* argv[]) { base::FilePath(options.database), options.url, options.annotations, - options.arguments)) { + options.arguments, + false)) { return kExitFailure; }