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

remoting/host/host_service_win.cc

 // 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.
 //
 // This file implements the Windows service controlling Me2Me host processes
 // running within user sessions.
 
 #include "remoting/host/host_service_win.h"
 
 #include <windows.h>
 #include <wtsapi32.h>
 #include <stdio.h>
 
 #include "base/at_exit.h"
 #include "base/base_paths.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/path_service.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/auto_message_loop.h"
+#include "remoting/base/auto_thread.h"
 #include "remoting/base/breakpad.h"
 #include "remoting/base/scoped_sc_handle_win.h"
+#include "remoting/base/shutdownable.h"
 #include "remoting/host/branding.h"
 #include "remoting/host/host_service_resource.h"
 #include "remoting/host/usage_stats_consent.h"
 #include "remoting/host/wts_console_observer_win.h"
 #include "remoting/host/wts_session_process_launcher_win.h"
 
 using base::StringPrintf;
 
 namespace {
 
@@ skipping 37 matching lines @@
 void usage(const char* program_name) {
   fprintf(stderr, kUsageMessage, program_name);
 }
 
 }  // namespace
 
 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) {
 }
 
 HostService::~HostService() {
 }
 
 void HostService::AddWtsConsoleObserver(WtsConsoleObserver* observer) {
-  DCHECK(message_loop_->message_loop_proxy()->BelongsToCurrentThread());
+  DCHECK(message_loop_->BelongsToCurrentThread());
 
   console_observers_.AddObserver(observer);
 }
 
 void HostService::RemoveWtsConsoleObserver(WtsConsoleObserver* observer) {
-  DCHECK(message_loop_->message_loop_proxy()->BelongsToCurrentThread());
+  DCHECK(message_loop_->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(Shutdownable* /* launcher */) {
+  launcher_.reset(NULL);
+  message_loop_ = NULL;
 }
 
 void HostService::OnSessionChange() {
   // WTSGetActiveConsoleSessionId is a very cheap API. It basically reads
   // a single value from shared memory. Therefore it is better to check if
   // 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,
                         console_observers_,
                         OnSessionDetached());
     }
 
     console_session_id_ = console_session_id;
 
     if (console_session_id_ != kInvalidSessionId) {
       FOR_EACH_OBSERVER(WtsConsoleObserver,
                         console_observers_,
                         OnSessionAttached(console_session_id_));
     }
   }
 }
 
 BOOL WINAPI HostService::ConsoleControlHandler(DWORD event) {
   HostService* self = HostService::GetInstance();
   switch (event) {
     case CTRL_C_EVENT:
     case CTRL_BREAK_EVENT:
     case CTRL_CLOSE_EVENT:
     case CTRL_LOGOFF_EVENT:
     case CTRL_SHUTDOWN_EVENT:
-      self->message_loop_->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+      self->message_loop_->PostTask(FROM_HERE, base::Bind(
+          &WtsSessionProcessLauncher::Shutdown,
+          base::Unretained(self->launcher_.get())));
       self->stopped_event_.Wait();
       return TRUE;
 
     default:
       return FALSE;
   }
 }
 
 HostService* HostService::GetInstance() {
   return Singleton<HostService>::get();
@@ skipping 29 matching lines @@
     run_routine_ = &HostService::RunInConsole;
   }
 
   return true;
 }
 
 int HostService::Run() {
   return (this->*run_routine_)();
 }
 
-void HostService::RunMessageLoop() {
+bool HostService::BeforeMessageLoop() {
   // Launch the I/O thread.
-  base::Thread io_thread(kIoThreadName);
+  scoped_refptr<AutoThread> io_thread(
+      new AutoThread(kIoThreadName, message_loop_));

[Sergey Ulanov, 2012/07/23 22:44:37]

Why do we need thread to be refcounted?

Can we use base::SequencedWorkerPool instead as it is a preferred way to create
IO threads anyway?

[alexeypa (please no reviews), 2012/07/23 23:44:53]

To make sure it is alive until all objects depending on it being live are gone.
base::SequencedWorkerPool may be a better alternative. Let me see it is going to
work.

+
   base::Thread::Options io_thread_options(MessageLoop::TYPE_IO, 0);
-  if (!io_thread.StartWithOptions(io_thread_options)) {
+  if (!io_thread->StartWithOptions(io_thread_options)) {
     LOG(FATAL) << "Failed to start the I/O thread";
-    shutting_down_ = true;
-    stopped_event_.Signal();
-    return;
+    return false;
   }
 
-  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_,
+      message_loop_,
+      io_thread));
+  return true;
+}
+void HostService::RunMessageLoop() {
   // Run the service.
-  message_loop_->Run();
-
-  // Clean up the observers by emulating detaching from the console.
-  shutting_down_ = true;
-  OnSessionChange();
+  if (BeforeMessageLoop()) {
+    message_loop_->Run();
+  }
 
   // Release the control handler.
   stopped_event_.Signal();
 }
 
 int HostService::RunAsService() {
   SERVICE_TABLE_ENTRYW dispatch_table[] = {
     { const_cast<LPWSTR>(service_name_.c_str()), &HostService::ServiceMain },
     { NULL, NULL }
   };
 
   if (!StartServiceCtrlDispatcherW(dispatch_table)) {
     LOG_GETLASTERROR(ERROR)
         << "Failed to connect to the service control manager";
     return kErrorExitCode;
   }
 
   return kSuccessExitCode;
 }
 
 int HostService::RunInConsole() {
-  MessageLoop message_loop(MessageLoop::TYPE_UI);
+  AutoMessageLoop message_loop(MessageLoop::TYPE_UI);

[Sergey Ulanov, 2012/07/23 22:44:37]

You are creating ref-counted object on stack. That's never safe - the object
will try to delete itself when ref-count goes to zero.

[alexeypa (please no reviews), 2012/07/23 23:44:53]

I control the destruction logic for this object. AutoMessageLoop::OnDestruct()
will be called when the last reference goes away. It will stop the loop but do
not delete AutoMessageLoop.

Basically the use case for AutoMessageLoop and AutoThread is: the main loop is
create in the stack; all other loops are create in heap and use the main one to
destroy themselves; they a;so reference the main loop to make sure it stay alive
long enough.

 
   // Allow other threads to post to our message loop.
-  message_loop_ = &message_loop;
+  message_loop_ = scoped_refptr<AutoMessageLoop>(&message_loop);
 
   int result = kErrorExitCode;
 
   // Subscribe to Ctrl-C and other console events.
   if (!SetConsoleCtrlHandler(&HostService::ConsoleControlHandler, TRUE)) {
     LOG_GETLASTERROR(ERROR)
         << "Failed to set console control handler";
     return result;
   }
 
@@ skipping 43 matching lines @@
   }
 
   if (atom != 0) {
     UnregisterClass(MAKEINTATOM(atom), instance);
   }
 
   // Unsubscribe from console events. Ignore the exit code. There is nothing
   // we can do about it now and the program is about to exit anyway. Even if
   // it crashes nothing is going to be broken because of it.
   SetConsoleCtrlHandler(&HostService::ConsoleControlHandler, FALSE);
-
-  message_loop_ = NULL;
   return result;
 }
 
 DWORD WINAPI HostService::ServiceControlHandler(DWORD control,
                                                 DWORD event_type,
                                                 LPVOID event_data,
                                                 LPVOID context) {
   HostService* self = reinterpret_cast<HostService*>(context);
   switch (control) {
     case SERVICE_CONTROL_INTERROGATE:
       return NO_ERROR;
 
     case SERVICE_CONTROL_SHUTDOWN:
     case SERVICE_CONTROL_STOP:
-      self->message_loop_->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+      self->message_loop_->PostTask(FROM_HERE, base::Bind(
+          &WtsSessionProcessLauncher::Shutdown,
+          base::Unretained(self->launcher_.get())));
       self->stopped_event_.Wait();
       return NO_ERROR;
 
     case SERVICE_CONTROL_SESSIONCHANGE:
       self->message_loop_->PostTask(FROM_HERE, base::Bind(
           &HostService::OnSessionChange, base::Unretained(self)));
       return NO_ERROR;
 
     default:
       return ERROR_CALL_NOT_IMPLEMENTED;
   }
 }
 
 VOID WINAPI HostService::ServiceMain(DWORD argc, WCHAR* argv[]) {
-  MessageLoop message_loop;
+  AutoMessageLoop message_loop(MessageLoop::TYPE_DEFAULT);

[Sergey Ulanov, 2012/07/23 22:44:37]

Why do we need MessageLoop to be refcounted here? If we just need it to be
destroyed later than ServiceMain() returns (why?) then maybe allocate it on heap
and just leak it when we return?

[alexeypa (please no reviews), 2012/07/23 23:44:53]

It doe snot have to outlive ServiceMain(). It is refcounted to express
dependencies between the message loop and objects using it. This refcounting has
nothing to do with memory allocation. The way those message loops are supposed
to be allocated is documented next to the definition of AutoThread and
AutoMessageLoop.

 
   // Allow other threads to post to our message loop.
   HostService* self = HostService::GetInstance();
-  self->message_loop_ = &message_loop;
+  self->message_loop_ = scoped_refptr<AutoMessageLoop>(&message_loop);
 
   // Register the service control handler.
   self->service_status_handle_ =
       RegisterServiceCtrlHandlerExW(self->service_name_.c_str(),
                                     &HostService::ServiceControlHandler,
                                     self);
   if (self->service_status_handle_ == 0) {
     LOG_GETLASTERROR(ERROR)
         << "Failed to register the service control handler";
     return;
@@ skipping 25 matching lines @@
 
   // Tell SCM that the service is stopped.
   service_status.dwCurrentState = SERVICE_STOPPED;
   service_status.dwControlsAccepted = 0;
 
   if (!SetServiceStatus(self->service_status_handle_, &service_status)) {
     LOG_GETLASTERROR(ERROR)
         << "Failed to report service status to the service control manager";
     return;
   }
-
-  self->message_loop_ = NULL;
 }
 
 LRESULT CALLBACK HostService::SessionChangeNotificationProc(HWND hwnd,
                                                             UINT message,
                                                             WPARAM wparam,
                                                             LPARAM lparam) {
   switch (message) {
     case WM_WTSSESSION_CHANGE: {
       HostService* self = HostService::GetInstance();
       self->OnSessionChange();
@@ skipping 38 matching lines @@
   }
 
   remoting::HostService* service = remoting::HostService::GetInstance();
   if (!service->InitWithCommandLine(command_line)) {
     usage(argv[0]);
     return kUsageExitCode;
   }
 
   return service->Run();
 }