mac: Restart crashpad_handler from the initial client if it dies

client/crashpad_client_mac.cc

Index: client/crashpad_client_mac.cc
diff --git a/client/crashpad_client_mac.cc b/client/crashpad_client_mac.cc
index caf7e91bfd499fd39e607671186f9fe0526accf9..f86a16de5923232ca80e0dee15ad14f217819620 100644
--- a/client/crashpad_client_mac.cc
+++ b/client/crashpad_client_mac.cc
@@ -14,7 +14,9 @@
 
 #include "client/crashpad_client.h"
 
+#include <errno.h>
 #include <mach/mach.h>
+#include <pthread.h>
 #include <sys/wait.h>
 #include <unistd.h>
 
@@ -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,21 +85,42 @@ 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<std::string, std::string>& annotations,
-      const std::vector<std::string>& arguments) {
+      const std::vector<std::string>& arguments,
+      bool restartable) {
     base::mac::ScopedMachReceiveRight receive_right(
         NewMachPort(MACH_PORT_RIGHT_RECEIVE));
     if (receive_right == kMachPortNull) {
@@ -116,25 +143,146 @@ class HandlerStarter final {
     DCHECK_EQ(right_type,
               implicit_cast<mach_msg_type_name_t>(MACH_MSG_TYPE_PORT_SEND));
 
+    scoped_ptr<HandlerStarter> handler_starter;

[Robert Sesek, 2015/11/03 15:50:27]

handler_restarter ?

+    if (restartable) {
+      handler_starter.reset(new HandlerStarter());
+      if (handler_starter->notify_port_ == kMachPortNull) {

[Robert Sesek, 2015/11/03 15:50:27]

Leave a comment here indicating that this is an error condition, but it'll
proceed anyways without restarting?

+        handler_starter.reset();
+      }
+    }
+
     if (!CommonStart(handler,
                      database,
                      url,
                      annotations,
                      arguments,
-                     receive_right.Pass())) {
+                     receive_right.Pass(),
+                     handler_starter.get(),
+                     false)) {
       return base::mac::ScopedMachSendRight();
     }
 
+    if (handler_starter && handler_starter->StartRestartThread(
+            handler, database, url, annotations, arguments)) {
+      // The thread owns the object now.
+      ignore_result(handler_starter.release());
+    }
+
     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_starter 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<std::string, std::string>& annotations,
                           const std::vector<std::string>& arguments,
-                          base::mac::ScopedMachReceiveRight receive_right) {
+                          base::mac::ScopedMachReceiveRight receive_right,
+                          HandlerStarter* handler_starter,
+                          bool restart) {
+    if (handler_starter) {
+      // 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_starter->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_starter->notify_port_.reset();
+      } else {
+        base::mac::ScopedMachSendRight previous_owner(previous);
+        if (!restart) {
+          DCHECK_EQ(previous, kMachPortNull);
+        }
+      }
+
+      if (handler_starter->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_starter->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_starter->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 +343,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 +426,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<std::string, std::string>& annotations,
+                          const std::vector<std::string>& 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<HandlerStarter*>(argument);
+
+    NotifyServer notify_server(self);
+    mach_msg_return_t mr;
+    do {
+      mr = MachMessageServer::Run(&notify_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_ != kMachPortNull && mr == MACH_MSG_SUCCESS);
+
+    delete self;
+
+    return nullptr;
+  }
+
+  base::FilePath handler_;
+  base::FilePath database_;
+  std::string url_;
+  std::map<std::string, std::string> annotations_;
+  std::vector<std::string> arguments_;
+  base::mac::ScopedMachReceiveRight notify_port_;
+  uint64_t last_start_time_;
+
+  DISALLOW_COPY_AND_ASSIGN(HandlerStarter);
 };
 
 }  // namespace
@@ -286,11 +507,20 @@ bool CrashpadClient::StartHandler(
     const base::FilePath& database,
     const std::string& url,
     const std::map<std::string, std::string>& annotations,
-    const std::vector<std::string>& arguments) {
+    const std::vector<std::string>& 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;
   }