Introducing remoting::Stoppable helper base class implementing asynchronous shutdown on a specific thread.

Review URL: https://chromiumcodereview.appspot.com/10796099

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149273 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 189 insertions, 60 deletions

diff --git a/remoting/base/stoppable.cc b/remoting/base/stoppable.cc
new file mode 100644
index 0000000..88b3c95
--- /dev/null
+++ b/remoting/base/stoppable.cc
@@ -0,0 +1,41 @@
+// Copyright (c) 2012 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/base/stoppable.h"
+
+#include "base/message_loop.h"
+#include "base/single_thread_task_runner.h"
+
+namespace remoting {
+
+Stoppable::Stoppable(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    const base::Closure& stopped_callback)
+    : state_(kRunning),
+      stopped_callback_(stopped_callback),
+      task_runner_(task_runner) {
+}
+
+Stoppable::~Stoppable() {
+  DCHECK_EQ(state_, kStopped);
+}
+
+void Stoppable::Stop() {
+  DCHECK(task_runner_->BelongsToCurrentThread());
+
+  if (state_ == kRunning) {
+    state_ = kStopping;
+    DoStop();
+  }
+}
+
+void Stoppable::CompleteStopping() {
+  DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK_EQ(state_, kStopping);
+
+  state_ = kStopped;
+  task_runner_->PostTask(FROM_HERE, stopped_callback_);
+}
+
+}  // namespace remoting
diff --git a/remoting/base/stoppable.h b/remoting/base/stoppable.h
new file mode 100644
index 0000000..4d58a59
--- /dev/null
+++ b/remoting/base/stoppable.h
@@ -0,0 +1,63 @@
+// Copyright (c) 2012 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_BASE_STOPPABLE_H_
+#define REMOTING_BASE_STOPPABLE_H_
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/memory/ref_counted.h"
+
+namespace base {
+class SingleThreadTaskRunner;
+}  // namespace base
+
+namespace remoting {
+
+// A helper base class that implements asynchronous shutdown on a specific
+// thread.
+class Stoppable {
+ public:
+  // Constructs an object and stores the callback to be posted to |task_runner|
+  // once the object has been shutdown completely.
+  explicit Stoppable(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+                     const base::Closure& stopped_callback);
+  virtual ~Stoppable();
+
+  // Initiates shutdown. It can be called by both the owner of the object and
+  // the object itself resulting in the same shutdown sequence.
+  void Stop();
+
+ protected:
+  // Called by derived classes to notify about shutdown completion. Posts
+  // the completion task on |task_runner_| message loop.
+  void CompleteStopping();
+
+  // Derived classes have to override this method to implement the shutdown
+  // logic.
+  virtual void DoStop() = 0;
+
+  enum State {
+    kRunning,
+    kStopping,
+    kStopped
+  };
+
+  State stoppable_state() const { return state_; }
+
+ private:
+  State state_;
+
+  // A callback to be called when shutdown is completed.
+  base::Closure stopped_callback_;
+
+  // The target task runner where the shutdown notification will be posted.
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(Stoppable);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_BASE_STOPPABLE_H_
diff --git a/remoting/host/win/host_service.cc b/remoting/host/win/host_service.cc
index 707f9ec..7cf331a 100644
--- a/remoting/host/win/host_service.cc
+++ b/remoting/host/win/host_service.cc
@@ -19,12 +19,14 @@
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
+#include "base/single_thread_task_runner.h"
 #include "base/stringprintf.h"
 #include "base/threading/thread.h"
 #include "base/utf_string_conversions.h"
 #include "base/win/wrapped_window_proc.h"
 #include "remoting/base/breakpad.h"
 #include "remoting/base/scoped_sc_handle_win.h"
+#include "remoting/base/stoppable.h"
 #include "remoting/host/branding.h"
 #include "remoting/host/usage_stats_consent.h"
 #include "remoting/host/win/host_service_resource.h"
@@ -82,11 +84,9 @@ namespace remoting {
 
 HostService::HostService() :
   console_session_id_(kInvalidSessionId),
-  message_loop_(NULL),
   run_routine_(&HostService::RunAsService),
   service_name_(kWindowsServiceName),
   service_status_handle_(0),
-  shutting_down_(false),
   stopped_event_(true, false) {
 }
 
@@ -94,20 +94,20 @@ HostService::~HostService() {
 }
 
 void HostService::AddWtsConsoleObserver(WtsConsoleObserver* observer) {
-  DCHECK(message_loop_->message_loop_proxy()->BelongsToCurrentThread());
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   console_observers_.AddObserver(observer);
 }
 
 void HostService::RemoveWtsConsoleObserver(WtsConsoleObserver* observer) {
-  DCHECK(message_loop_->message_loop_proxy()->BelongsToCurrentThread());
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   console_observers_.RemoveObserver(observer);
+}
 
-  // Stop the service if there are no more observers.
-  if (!console_observers_.might_have_observers()) {
-    message_loop_->PostTask(FROM_HERE, MessageLoop::QuitClosure());
-  }
+void HostService::OnLauncherShutdown() {
+  launcher_.reset(NULL);
+  main_task_runner_->PostTask(FROM_HERE, MessageLoop::QuitClosure());
 }
 
 void HostService::OnSessionChange() {
@@ -116,10 +116,7 @@ void HostService::OnSessionChange() {
   // the console session is still the same every time a session change
   // notification event is posted. This also takes care of coalescing multiple
   // events into one since we look at the latest state.
-  uint32 console_session_id = kInvalidSessionId;
-  if (!shutting_down_) {
-    console_session_id = WTSGetActiveConsoleSessionId();
-  }
+  uint32 console_session_id = WTSGetActiveConsoleSessionId();
   if (console_session_id_ != console_session_id) {
     if (console_session_id_ != kInvalidSessionId) {
       FOR_EACH_OBSERVER(WtsConsoleObserver,
@@ -145,7 +142,9 @@ BOOL WINAPI HostService::ConsoleControlHandler(DWORD event) {
     case CTRL_CLOSE_EVENT:
     case CTRL_LOGOFF_EVENT:
     case CTRL_SHUTDOWN_EVENT:
-      self->message_loop_->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+      self->main_task_runner_->PostTask(FROM_HERE, base::Bind(
+          &WtsSessionProcessLauncher::Stop,
+          base::Unretained(self->launcher_.get())));
       self->stopped_event_.Wait();
       return TRUE;
 
@@ -195,27 +194,26 @@ int HostService::Run() {
   return (this->*run_routine_)();
 }
 
-void HostService::RunMessageLoop() {
+void HostService::RunMessageLoop(MessageLoop* message_loop) {
   // Launch the I/O thread.
   base::Thread io_thread(kIoThreadName);
   base::Thread::Options io_thread_options(MessageLoop::TYPE_IO, 0);
   if (!io_thread.StartWithOptions(io_thread_options)) {
     LOG(FATAL) << "Failed to start the I/O thread";
-    shutting_down_ = true;
     stopped_event_.Signal();
     return;
   }
 
-  WtsSessionProcessLauncher launcher(this, host_binary_,
-                                     message_loop_->message_loop_proxy(),
-                                     io_thread.message_loop_proxy());
+  // Create the session process launcher.
+  launcher_.reset(new WtsSessionProcessLauncher(
+      base::Bind(&HostService::OnLauncherShutdown, base::Unretained(this)),
+      this,
+      host_binary_,
+      main_task_runner_,
+      io_thread.message_loop_proxy()));
 
   // Run the service.
-  message_loop_->Run();
-
-  // Clean up the observers by emulating detaching from the console.
-  shutting_down_ = true;
-  OnSessionChange();
+  message_loop->Run();
 
   // Release the control handler.
   stopped_event_.Signal();
@@ -240,7 +238,7 @@ int HostService::RunInConsole() {
   MessageLoop message_loop(MessageLoop::TYPE_UI);
 
   // Allow other threads to post to our message loop.
-  message_loop_ = &message_loop;
+  main_task_runner_ = message_loop.message_loop_proxy();
 
   int result = kErrorExitCode;
 
@@ -278,14 +276,14 @@ int HostService::RunInConsole() {
 
   // Post a dummy session change notification to peek up the current console
   // session.
-  message_loop.PostTask(FROM_HERE, base::Bind(
+  main_task_runner_->PostTask(FROM_HERE, base::Bind(
       &HostService::OnSessionChange, base::Unretained(this)));
 
   // Subscribe to session change notifications.
   if (WTSRegisterSessionNotification(window,
                                      NOTIFY_FOR_ALL_SESSIONS) != FALSE) {
     // Run the service.
-    RunMessageLoop();
+    RunMessageLoop(&message_loop);
 
     WTSUnRegisterSessionNotification(window);
     result = kSuccessExitCode;
@@ -305,7 +303,6 @@ cleanup:
   // it crashes nothing is going to be broken because of it.
   SetConsoleCtrlHandler(&HostService::ConsoleControlHandler, FALSE);
 
-  message_loop_ = NULL;
   return result;
 }
 
@@ -320,12 +317,14 @@ DWORD WINAPI HostService::ServiceControlHandler(DWORD control,
 
     case SERVICE_CONTROL_SHUTDOWN:
     case SERVICE_CONTROL_STOP:
-      self->message_loop_->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+      self->main_task_runner_->PostTask(FROM_HERE, base::Bind(
+          &WtsSessionProcessLauncher::Stop,
+          base::Unretained(self->launcher_.get())));
       self->stopped_event_.Wait();
       return NO_ERROR;
 
     case SERVICE_CONTROL_SESSIONCHANGE:
-      self->message_loop_->PostTask(FROM_HERE, base::Bind(
+      self->main_task_runner_->PostTask(FROM_HERE, base::Bind(
           &HostService::OnSessionChange, base::Unretained(self)));
       return NO_ERROR;
 
@@ -339,7 +338,7 @@ VOID WINAPI HostService::ServiceMain(DWORD argc, WCHAR* argv[]) {
 
   // Allow other threads to post to our message loop.
   HostService* self = HostService::GetInstance();
-  self->message_loop_ = &message_loop;
+  self->main_task_runner_ = message_loop.message_loop_proxy();
 
   // Register the service control handler.
   self->service_status_handle_ =
@@ -370,11 +369,11 @@ VOID WINAPI HostService::ServiceMain(DWORD argc, WCHAR* argv[]) {
 
   // Post a dummy session change notification to peek up the current console
   // session.
-  message_loop.PostTask(FROM_HERE, base::Bind(
+  self->main_task_runner_->PostTask(FROM_HERE, base::Bind(
       &HostService::OnSessionChange, base::Unretained(self)));
 
   // Run the service.
-  self->RunMessageLoop();
+  self->RunMessageLoop(&message_loop);
 
   // Tell SCM that the service is stopped.
   service_status.dwCurrentState = SERVICE_STOPPED;
@@ -385,8 +384,6 @@ VOID WINAPI HostService::ServiceMain(DWORD argc, WCHAR* argv[]) {
         << "Failed to report service status to the service control manager";
     return;
   }
-
-  self->message_loop_ = NULL;
 }
 
 LRESULT CALLBACK HostService::SessionChangeNotificationProc(HWND hwnd,
diff --git a/remoting/host/win/host_service.h b/remoting/host/win/host_service.h
index ff6d1d7..7dacfd3 100644
--- a/remoting/host/win/host_service.h
+++ b/remoting/host/win/host_service.h
@@ -8,18 +8,24 @@
 #include <windows.h>
 
 #include "base/file_path.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/singleton.h"
 #include "base/observer_list.h"
 #include "base/synchronization/waitable_event.h"
-
 #include "remoting/host/win/wts_console_monitor.h"
 
 class CommandLine;
 class MessageLoop;
 
+namespace base {
+class SingleThreadTaskRunner;
+}  // namespace base
+
 namespace remoting {
 
+class Stoppable;
 class WtsConsoleObserver;
+class WtsSessionProcessLauncher;
 
 class HostService : public WtsConsoleMonitor {
  public:
@@ -40,12 +46,14 @@ class HostService : public WtsConsoleMonitor {
   HostService();
   ~HostService();
 
+  void OnLauncherShutdown();
+
   // Notifies the service of changes in session state.
   void OnSessionChange();
 
   // This is a common entry point to the main service loop called by both
   // RunAsService() and RunInConsole().
-  void RunMessageLoop();
+  void RunMessageLoop(MessageLoop* message_loop);
 
   // This function handshakes with the service control manager and starts
   // the service.
@@ -78,11 +86,13 @@ class HostService : public WtsConsoleMonitor {
   // to the physical console.
   ObserverList<WtsConsoleObserver> console_observers_;
 
+  scoped_ptr<WtsSessionProcessLauncher> launcher_;
+
   // The host binary name.
   FilePath host_binary_;
 
   // Service message loop.
-  MessageLoop* message_loop_;
+  scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
 
   // The action routine to be executed.
   int (HostService::*run_routine_)();
@@ -93,9 +103,6 @@ class HostService : public WtsConsoleMonitor {
   // The service status handle.
   SERVICE_STATUS_HANDLE service_status_handle_;
 
-  // True if the service is being stopped.
-  bool shutting_down_;
-
   // A waitable event that is used to wait until the service is stopped.
   base::WaitableEvent stopped_event_;
 
diff --git a/remoting/host/win/wts_session_process_launcher.cc b/remoting/host/win/wts_session_process_launcher.cc
index ef57e09..0a7ee1b 100644
--- a/remoting/host/win/wts_session_process_launcher.cc
+++ b/remoting/host/win/wts_session_process_launcher.cc
@@ -16,7 +16,7 @@
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/logging.h"
-#include "base/message_loop_proxy.h"
+#include "base/single_thread_task_runner.h"
 #include "base/process_util.h"
 #include "base/rand_util.h"
 #include "base/stringprintf.h"
@@ -212,11 +212,13 @@ namespace remoting {
 const uint32 kInvalidSessionId = 0xffffffff;
 
 WtsSessionProcessLauncher::WtsSessionProcessLauncher(
+    const base::Closure& stopped_callback,
     WtsConsoleMonitor* monitor,
     const FilePath& host_binary,
-    scoped_refptr<base::MessageLoopProxy> main_message_loop,
-    scoped_refptr<base::MessageLoopProxy> ipc_message_loop)
-    : host_binary_(host_binary),
+    scoped_refptr<base::SingleThreadTaskRunner> main_message_loop,
+    scoped_refptr<base::SingleThreadTaskRunner> ipc_message_loop)
+    : Stoppable(main_message_loop, stopped_callback),
+      host_binary_(host_binary),
       main_message_loop_(main_message_loop),
       ipc_message_loop_(ipc_message_loop),
       monitor_(monitor),
@@ -225,14 +227,16 @@ WtsSessionProcessLauncher::WtsSessionProcessLauncher(
 }
 
 WtsSessionProcessLauncher::~WtsSessionProcessLauncher() {
+  monitor_->RemoveWtsConsoleObserver(this);
+  if (state_ != StateDetached) {
+    OnSessionDetached();
+  }
+
   DCHECK(state_ == StateDetached);
   DCHECK(!timer_.IsRunning());
   DCHECK(process_.handle() == NULL);
   DCHECK(process_watcher_.GetWatchedObject() == NULL);
   DCHECK(chromoting_channel_.get() == NULL);
-  if (monitor_ != NULL) {
-    monitor_->RemoveWtsConsoleObserver(this);
-  }
 }
 
 void WtsSessionProcessLauncher::LaunchProcess() {
@@ -328,12 +332,7 @@ void WtsSessionProcessLauncher::OnObjectSignaled(HANDLE object) {
   state_ = StateStarting;
 
   if (stop_trying) {
-    OnSessionDetached();
-
-    // N.B. The service will stop once the last observer is removed from
-    // the list.
-    monitor_->RemoveWtsConsoleObserver(this);
-    monitor_ = NULL;
+    Stop();
     return;
   }
 
@@ -386,6 +385,11 @@ void WtsSessionProcessLauncher::OnSendSasToConsole() {
 
 void WtsSessionProcessLauncher::OnSessionAttached(uint32 session_id) {
   DCHECK(main_message_loop_->BelongsToCurrentThread());
+
+  if (stoppable_state() != Stoppable::kRunning) {
+    return;
+  }
+
   DCHECK(state_ == StateDetached);
   DCHECK(!timer_.IsRunning());
   DCHECK(process_.handle() == NULL);
@@ -463,4 +467,12 @@ void WtsSessionProcessLauncher::OnSessionDetached() {
   session_token_.Close();
 }
 
+void WtsSessionProcessLauncher::DoStop() {
+  if (state_ != StateDetached) {
+    OnSessionDetached();
+  }
+
+  CompleteStopping();
+}
+
 } // namespace remoting
diff --git a/remoting/host/win/wts_session_process_launcher.h b/remoting/host/win/wts_session_process_launcher.h
index d52d98f..c4dce87 100644
--- a/remoting/host/win/wts_session_process_launcher.h
+++ b/remoting/host/win/wts_session_process_launcher.h
@@ -18,11 +18,11 @@
 #include "base/win/scoped_handle.h"
 #include "base/win/object_watcher.h"
 #include "ipc/ipc_channel.h"
-
+#include "remoting/base/stoppable.h"
 #include "remoting/host/win/wts_console_observer.h"
 
 namespace base {
-class MessageLoopProxy;
+class SingleThreadTaskRunner;
 } // namespace base
 
 namespace IPC {
@@ -39,7 +39,8 @@ class SasInjector;
 class WtsConsoleMonitor;
 
 class WtsSessionProcessLauncher
-    : public base::win::ObjectWatcher::Delegate,
+    : public Stoppable,
+      public base::win::ObjectWatcher::Delegate,
       public IPC::Listener,
       public WtsConsoleObserver {
  public:
@@ -48,10 +49,11 @@ class WtsSessionProcessLauncher
   // |monitor| should happen on |main_message_loop|. |ipc_message_loop| has
   // to be an I/O message loop.
   WtsSessionProcessLauncher(
+      const base::Closure& stopped_callback,
       WtsConsoleMonitor* monitor,
       const FilePath& host_binary,
-      scoped_refptr<base::MessageLoopProxy> main_message_loop,
-      scoped_refptr<base::MessageLoopProxy> ipc_message_loop);
+      scoped_refptr<base::SingleThreadTaskRunner> main_message_loop,
+      scoped_refptr<base::SingleThreadTaskRunner> ipc_message_loop);
 
   virtual ~WtsSessionProcessLauncher();
 
@@ -65,6 +67,10 @@ class WtsSessionProcessLauncher
   virtual void OnSessionAttached(uint32 session_id) OVERRIDE;
   virtual void OnSessionDetached() OVERRIDE;
 
+ protected:
+  // Stoppable implementation.
+  virtual void DoStop() OVERRIDE;
+
  private:
   // Attempts to launch the host process in the current console session.
   // Schedules next launch attempt if creation of the process fails for any
@@ -88,10 +94,10 @@ class WtsSessionProcessLauncher
   base::OneShotTimer<WtsSessionProcessLauncher> timer_;
 
   // The main service message loop.
-  scoped_refptr<base::MessageLoopProxy> main_message_loop_;
+  scoped_refptr<base::SingleThreadTaskRunner> main_message_loop_;
 
   // Message loop used by the IPC channel.
-  scoped_refptr<base::MessageLoopProxy> ipc_message_loop_;
+  scoped_refptr<base::SingleThreadTaskRunner> ipc_message_loop_;
 
   // This pointer is used to unsubscribe from session attach and detach events.
   WtsConsoleMonitor* monitor_;
diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp
index 70f9818..bfd57da 100644
--- a/remoting/remoting.gyp
+++ b/remoting/remoting.gyp
@@ -584,6 +584,7 @@
             '../base/base.gyp:base_static',
             '../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
             '../ipc/ipc.gyp:ipc',
+            'remoting_base',
             'remoting_breakpad',
             'remoting_version_resources',
           ],
@@ -1165,6 +1166,8 @@
         'base/rate_counter.h',
         'base/running_average.cc',
         'base/running_average.h',
+        'base/stoppable.cc',
+        'base/stoppable.h',
         'base/util.cc',
         'base/util.h',
       ],