Ensure the Chromoting Host process eventually terminates when shut down.

The host process is meant to shut down cleanly when receiving a SIGTERM,
or if there is a configuration error.

This CL implements a timed watchdog, running on a new thread, that
triggers a forced exit of the process, in case it fails to terminate
normally within a reasonable time (for example, if some thread is inside
a blocking call).

BUG=420090
TEST=The watchdog shouldn't normally trigger, but I manually added a Sleep() to some thread to verify that it triggers if a thread is blocked for too long.

Review URL: https://codereview.chromium.org/784243003

Cr-Commit-Position: refs/heads/master@{#307819}

remoting/host/remoting_me2me_host.cc

@@ -61,6 +61,7 @@
 #include "remoting/host/pairing_registry_delegate.h"
 #include "remoting/host/policy_hack/policy_watcher.h"
 #include "remoting/host/session_manager_factory.h"
+#include "remoting/host/shutdown_watchdog.h"
 #include "remoting/host/signaling_connector.h"
 #include "remoting/host/single_window_desktop_environment.h"
 #include "remoting/host/token_validator_factory_impl.h"
@@ -134,6 +135,10 @@ const char kStdinConfigPath[] = "-";
 
 const char kWindowIdSwitchName[] = "window-id";
 
+// Maximum time to wait for clean shutdown to occur, before forcing termination
+// of the process.
+const int kShutdownTimeoutSeconds = 15;
+
 }  // namespace
 
 namespace remoting {
@@ -144,8 +149,12 @@ class HostProcess
       public IPC::Listener,
       public base::RefCountedThreadSafe<HostProcess> {
  public:
+  // |shutdown_watchdog| is armed when shutdown is started, and should be kept
+  // alive as long as possible until the process exits (since destroying the
+  // watchdog disarms it).
   HostProcess(scoped_ptr<ChromotingHostContext> context,
-              int* exit_code_out);
+              int* exit_code_out,
+              ShutdownWatchdog* shutdown_watchdog);
 
   // ConfigWatcher::Delegate interface.
   void OnConfigUpdated(const std::string& serialized_config) override;
@@ -340,10 +349,13 @@ class HostProcess
   bool signal_parent_;
 
   scoped_ptr<PairingRegistry::Delegate> pairing_registry_delegate_;
+
+  ShutdownWatchdog* shutdown_watchdog_;
 };
 
 HostProcess::HostProcess(scoped_ptr<ChromotingHostContext> context,
-                         int* exit_code_out)
+                         int* exit_code_out,
+                         ShutdownWatchdog* shutdown_watchdog)
     : context_(context.Pass()),
       state_(HOST_INITIALIZING),
       use_service_account_(false),
@@ -364,7 +376,8 @@ HostProcess::HostProcess(scoped_ptr<ChromotingHostContext> context,
 #endif  // defined(REMOTING_MULTI_PROCESS)
       self_(this),
       exit_code_out_(exit_code_out),
-      signal_parent_(false) {
+      signal_parent_(false),
+      shutdown_watchdog_(shutdown_watchdog) {
   StartOnUiThread();
 }
 
@@ -1416,6 +1429,9 @@ void HostProcess::ShutdownOnNetworkThread() {
   } else if (state_ == HOST_STOPPING) {
     state_ = HOST_STOPPED;
 
+    shutdown_watchdog_->SetExitCode(*exit_code_out_);
+    shutdown_watchdog_->Arm();
+
     if (policy_watcher_.get()) {
       policy_watcher_->StopWatching(
           base::Bind(&HostProcess::OnPolicyWatcherShutdown, this));
@@ -1479,7 +1495,9 @@ int HostProcessMain() {
   // TODO(wez): The HostProcess holds a reference to itself until Shutdown().
   // Remove this hack as part of the multi-process refactoring.
   int exit_code = kSuccessExitCode;
-  new HostProcess(context.Pass(), &exit_code);
+  ShutdownWatchdog shutdown_watchdog(
+      base::TimeDelta::FromSeconds(kShutdownTimeoutSeconds));
+  new HostProcess(context.Pass(), &exit_code, &shutdown_watchdog);
 
   // Run the main (also UI) message loop until the host no longer needs it.
   message_loop.Run();

remoting/host/shutdown_watchdog.cc

@@ -0,0 +1,39 @@
+// Copyright 2014 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.
+
+#include "remoting/host/shutdown_watchdog.h"
+
+#include <stdlib.h>  // For _exit() on Windows.
+
+#include "base/logging.h"
+
+#if defined(OS_POSIX)
+#include <unistd.h>
+#endif  // defined(OS_POSIX)
+
+namespace remoting {
+
+ShutdownWatchdog::ShutdownWatchdog(const base::TimeDelta& duration)
+    : base::Watchdog(duration, "Shutdown watchdog", true) {
+}
+
+void ShutdownWatchdog::SetExitCode(int exit_code) {
+  base::AutoLock lock(lock_);
+  exit_code_ = exit_code;
+}
+
+void ShutdownWatchdog::Alarm() {
+  // Holding a lock while calling _exit() might not be a safe thing to do, so
+  // make a local copy.
+  int exit_code;
+  {
+    base::AutoLock lock(lock_);
+    exit_code = exit_code_;
+  }
+
+  LOG(ERROR) << "Shutdown watchdog triggered, exiting with code " << exit_code;
+  _exit(exit_code);
+}
+
+}  // namespace remoting

remoting/host/shutdown_watchdog.h

@@ -0,0 +1,42 @@
+// Copyright 2014 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.
+
+#ifndef REMOTING_HOST_SHUTDOWN_WATCHDOG_H_
+#define REMOTING_HOST_SHUTDOWN_WATCHDOG_H_
+
+#include "base/synchronization/lock.h"
+#include "base/threading/watchdog.h"
+
+namespace remoting {
+
+// This implements a watchdog timer that ensures the host process eventually
+// terminates, even if some threads are blocked or being kept alive for
+// some reason. This is not expected to trigger if host shutdown is working
+// correctly (on a normally loaded system). The triggering of the alarm
+// indicates a sign of trouble, and so the Alarm() method will log some
+// diagnostic information before shutting down the process.
+class ShutdownWatchdog : public base::Watchdog {
+ public:
+  // Creates a watchdog that waits for |duration| (after the watchdog is
+  // armed) before shutting down the process.
+  explicit ShutdownWatchdog(const base::TimeDelta& duration);
+
+  // This method should be called to set the process's exit-code before arming
+  // the watchdog. Otherwise an exit-code of 0 is assumed.
+  void SetExitCode(int exit_code);
+
+  void Alarm() override;
+
+ private:
+  int exit_code_;
+
+  // Protects |exit_code_|, since Alarm() gets called on a separate thread.
+  base::Lock lock_;
+
+  DISALLOW_COPY_AND_ASSIGN(ShutdownWatchdog);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_SHUTDOWN_WATCHDOG_H_

remoting/remoting_host_srcs.gypi

@@ -209,6 +209,8 @@
       'host/session_manager_factory.h',
       'host/shaped_desktop_capturer.cc',
       'host/shaped_desktop_capturer.h',
+      'host/shutdown_watchdog.cc',
+      'host/shutdown_watchdog.h',
       'host/signaling_connector.cc',
       'host/signaling_connector.h',
       'host/single_window_desktop_environment.cc',