diff options
| -rw-r--r-- | remoting/base/stoppable.cc | 43 | ||||
| -rw-r--r-- | remoting/base/stoppable.h | 63 | ||||
| -rw-r--r-- | remoting/host/daemon_process.cc | 35 | ||||
| -rw-r--r-- | remoting/host/daemon_process.h | 13 | ||||
| -rw-r--r-- | remoting/host/daemon_process_win.cc | 14 | ||||
| -rw-r--r-- | remoting/host/win/host_service.cc | 40 | ||||
| -rw-r--r-- | remoting/host/win/host_service.h | 17 | ||||
| -rw-r--r-- | remoting/remoting.gyp | 2 |
8 files changed, 60 insertions, 167 deletions
diff --git a/remoting/base/stoppable.cc b/remoting/base/stoppable.cc deleted file mode 100644 index 99acba4..0000000 --- a/remoting/base/stoppable.cc +++ /dev/null @@ -1,43 +0,0 @@ -// 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() { - CHECK_EQ(state_, kStopped); -} - -void Stoppable::Stop() { - DCHECK(task_runner_->BelongsToCurrentThread()); - - if (state_ == kRunning) { - state_ = kStopping; - } - - // DoStop() can be called multiple times. - 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 deleted file mode 100644 index dddfcdae..0000000 --- a/remoting/base/stoppable.h +++ /dev/null @@ -1,63 +0,0 @@ -// 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. - 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/daemon_process.cc b/remoting/host/daemon_process.cc index b7a0336..d9d05b4 100644 --- a/remoting/host/daemon_process.cc +++ b/remoting/host/daemon_process.cc @@ -37,8 +37,13 @@ std::ostream& operator<<(std::ostream& os, const ScreenResolution& resolution) { } // namespace DaemonProcess::~DaemonProcess() { - DCHECK(!config_watcher_.get()); - DCHECK(desktop_sessions_.empty()); + DCHECK(caller_task_runner()->BelongsToCurrentThread()); + + host_event_logger_.reset(); + weak_factory_.InvalidateWeakPtrs(); + + config_watcher_.reset(); + DeleteAllDesktopSessions(); } void DaemonProcess::OnConfigUpdated(const std::string& serialized_config) { @@ -163,10 +168,10 @@ DaemonProcess::DaemonProcess( scoped_refptr<AutoThreadTaskRunner> caller_task_runner, scoped_refptr<AutoThreadTaskRunner> io_task_runner, const base::Closure& stopped_callback) - : Stoppable(caller_task_runner, stopped_callback), - caller_task_runner_(caller_task_runner), + : caller_task_runner_(caller_task_runner), io_task_runner_(io_task_runner), next_terminal_id_(0), + stopped_callback_(stopped_callback), weak_factory_(this) { DCHECK(caller_task_runner->BelongsToCurrentThread()); } @@ -270,6 +275,16 @@ void DaemonProcess::Initialize() { LaunchNetworkProcess(); } +void DaemonProcess::Stop() { + DCHECK(caller_task_runner()->BelongsToCurrentThread()); + + if (!stopped_callback_.is_null()) { + base::Closure stopped_callback = stopped_callback_; + stopped_callback_.Reset(); + stopped_callback.Run(); + } +} + bool DaemonProcess::WasTerminalIdAllocated(int terminal_id) { return terminal_id < next_terminal_id_; } @@ -352,18 +367,6 @@ void DaemonProcess::OnHostShutdown() { FOR_EACH_OBSERVER(HostStatusObserver, status_observers_, OnShutdown()); } -void DaemonProcess::DoStop() { - DCHECK(caller_task_runner()->BelongsToCurrentThread()); - - host_event_logger_.reset(); - weak_factory_.InvalidateWeakPtrs(); - - config_watcher_.reset(); - DeleteAllDesktopSessions(); - - CompleteStopping(); -} - void DaemonProcess::DeleteAllDesktopSessions() { while (!desktop_sessions_.empty()) { delete desktop_sessions_.front(); diff --git a/remoting/host/daemon_process.h b/remoting/host/daemon_process.h index 99ddb2a..521d706 100644 --- a/remoting/host/daemon_process.h +++ b/remoting/host/daemon_process.h @@ -17,7 +17,6 @@ #include "ipc/ipc_channel.h" #include "ipc/ipc_channel_proxy.h" #include "ipc/ipc_platform_file.h" -#include "remoting/base/stoppable.h" #include "remoting/host/config_file_watcher.h" #include "remoting/host/host_status_monitor.h" #include "remoting/host/worker_process_ipc_delegate.h" @@ -40,8 +39,7 @@ class ScreenResolution; // process running at lower privileges and maintains the list of desktop // sessions. class DaemonProcess - : public Stoppable, - public ConfigFileWatcher::Delegate, + : public ConfigFileWatcher::Delegate, public HostStatusMonitor, public WorkerProcessIpcDelegate { public: @@ -107,6 +105,9 @@ class DaemonProcess // Reads the host configuration and launches the network process. void Initialize(); + // Invokes |stopped_callback_| to ask the owner to delete |this|. + void Stop(); + // Returns true if |terminal_id| is in the range of allocated IDs. I.e. it is // less or equal to the highest ID we have seen so far. bool WasTerminalIdAllocated(int terminal_id); @@ -123,9 +124,6 @@ class DaemonProcess void OnHostStarted(const std::string& xmpp_login); void OnHostShutdown(); - // Stoppable implementation. - virtual void DoStop() OVERRIDE; - // Creates a platform-specific desktop session and assigns a unique ID to it. // An implementation should validate |params| as they are received via IPC. virtual scoped_ptr<DesktopSession> DoCreateDesktopSession( @@ -178,6 +176,9 @@ class DaemonProcess // Keeps track of observers receiving host status notifications. ObserverList<HostStatusObserver> status_observers_; + // Invoked to ask the owner to delete |this|. + base::Closure stopped_callback_; + // Writes host status updates to the system event log. scoped_ptr<HostEventLogger> host_event_logger_; diff --git a/remoting/host/daemon_process_win.cc b/remoting/host/daemon_process_win.cc index e3f2b4d..5d2dc94 100644 --- a/remoting/host/daemon_process_win.cc +++ b/remoting/host/daemon_process_win.cc @@ -60,9 +60,6 @@ class DaemonProcessWin : public DaemonProcess { IPC::PlatformFileForTransit desktop_pipe) OVERRIDE; protected: - // Stoppable implementation. - virtual void DoStop() OVERRIDE; - // DaemonProcess implementation. virtual scoped_ptr<DesktopSession> DoCreateDesktopSession( int terminal_id, @@ -89,10 +86,6 @@ DaemonProcessWin::DaemonProcessWin( } DaemonProcessWin::~DaemonProcessWin() { - // Make sure that the object is completely stopped. The same check exists - // in Stoppable::~Stoppable() but this one helps us to fail early and - // predictably. - CHECK_EQ(stoppable_state(), Stoppable::kStopped); } void DaemonProcessWin::OnChannelConnected(int32 peer_pid) { @@ -138,13 +131,6 @@ bool DaemonProcessWin::OnDesktopSessionAgentAttached( return true; } -void DaemonProcessWin::DoStop() { - DCHECK(caller_task_runner()->BelongsToCurrentThread()); - - network_launcher_.reset(); - DaemonProcess::DoStop(); -} - scoped_ptr<DesktopSession> DaemonProcessWin::DoCreateDesktopSession( int terminal_id, const ScreenResolution& resolution, diff --git a/remoting/host/win/host_service.cc b/remoting/host/win/host_service.cc index 42a8ed0..b3cf6b0 100644 --- a/remoting/host/win/host_service.cc +++ b/remoting/host/win/host_service.cc @@ -25,7 +25,6 @@ #include "base/win/windows_version.h" #include "remoting/base/auto_thread.h" #include "remoting/base/scoped_sc_handle_win.h" -#include "remoting/base/stoppable.h" #include "remoting/host/branding.h" #include "remoting/host/daemon_process.h" #include "remoting/host/host_exit_codes.h" @@ -203,7 +202,8 @@ void HostService::RemoveWtsTerminalObserver(WtsTerminalObserver* observer) { HostService::HostService() : run_routine_(&HostService::RunAsService), service_status_handle_(0), - stopped_event_(true, false) { + stopped_event_(true, false), + weak_factory_(this) { } HostService::~HostService() { @@ -275,17 +275,10 @@ void HostService::CreateLauncher( return; } - child_ = DaemonProcess::Create( + daemon_process_ = DaemonProcess::Create( task_runner, io_task_runner, - base::Bind(&HostService::OnChildStopped, - base::Unretained(this))).PassAs<Stoppable>(); -} - -void HostService::OnChildStopped() { - DCHECK(main_task_runner_->BelongsToCurrentThread()); - - child_.reset(NULL); + base::Bind(&HostService::StopDaemonProcess, weak_ptr_)); } int HostService::RunAsService() { @@ -312,6 +305,7 @@ void HostService::RunAsServiceImpl() { base::MessageLoop message_loop(base::MessageLoop::TYPE_UI); base::RunLoop run_loop; main_task_runner_ = message_loop.message_loop_proxy(); + weak_ptr_ = weak_factory_.GetWeakPtr(); // Register the service control handler. service_status_handle_ = RegisterServiceCtrlHandlerExW( @@ -351,6 +345,7 @@ void HostService::RunAsServiceImpl() { // Run the service. run_loop.Run(); + weak_factory_.InvalidateWeakPtrs(); // Tell SCM that the service is stopped. service_status.dwCurrentState = SERVICE_STOPPED; @@ -366,6 +361,7 @@ int HostService::RunInConsole() { base::MessageLoop message_loop(base::MessageLoop::TYPE_UI); base::RunLoop run_loop; main_task_runner_ = message_loop.message_loop_proxy(); + weak_ptr_ = weak_factory_.GetWeakPtr(); int result = kInitializationFailed; @@ -410,6 +406,8 @@ int HostService::RunInConsole() { } cleanup: + weak_factory_.InvalidateWeakPtrs(); + // 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. @@ -418,6 +416,12 @@ cleanup: return result; } +void HostService::StopDaemonProcess() { + DCHECK(main_task_runner_->BelongsToCurrentThread()); + + daemon_process_.reset(); +} + bool HostService::HandleMessage( HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam, LRESULT* result) { if (message == WM_WTSSESSION_CHANGE) { @@ -438,9 +442,9 @@ BOOL WINAPI HostService::ConsoleControlHandler(DWORD event) { case CTRL_CLOSE_EVENT: case CTRL_LOGOFF_EVENT: case CTRL_SHUTDOWN_EVENT: - self->main_task_runner_->PostTask(FROM_HERE, base::Bind( - &Stoppable::Stop, base::Unretained(self->child_.get()))); - self->stopped_event_.Wait(); + self->main_task_runner_->PostTask( + FROM_HERE, base::Bind(&HostService::StopDaemonProcess, + self->weak_ptr_)); return TRUE; default: @@ -460,14 +464,14 @@ DWORD WINAPI HostService::ServiceControlHandler(DWORD control, case SERVICE_CONTROL_SHUTDOWN: case SERVICE_CONTROL_STOP: - self->main_task_runner_->PostTask(FROM_HERE, base::Bind( - &Stoppable::Stop, base::Unretained(self->child_.get()))); - self->stopped_event_.Wait(); + self->main_task_runner_->PostTask( + FROM_HERE, base::Bind(&HostService::StopDaemonProcess, + self->weak_ptr_)); return NO_ERROR; case SERVICE_CONTROL_SESSIONCHANGE: self->main_task_runner_->PostTask(FROM_HERE, base::Bind( - &HostService::OnSessionChange, base::Unretained(self), event_type, + &HostService::OnSessionChange, self->weak_ptr_, event_type, reinterpret_cast<WTSSESSION_NOTIFICATION*>(event_data)->dwSessionId)); return NO_ERROR; diff --git a/remoting/host/win/host_service.h b/remoting/host/win/host_service.h index aa1d9ab..a591071 100644 --- a/remoting/host/win/host_service.h +++ b/remoting/host/win/host_service.h @@ -11,6 +11,7 @@ #include "base/memory/ref_counted.h" #include "base/memory/singleton.h" +#include "base/memory/weak_ptr.h" #include "base/synchronization/waitable_event.h" #include "net/base/ip_endpoint.h" #include "remoting/host/win/message_window.h" @@ -25,7 +26,7 @@ class SingleThreadTaskRunner; namespace remoting { class AutoThreadTaskRunner; -class Stoppable; +class DaemonProcess; class WtsTerminalObserver; class HostService : public win::MessageWindow::Delegate, @@ -55,8 +56,6 @@ class HostService : public win::MessageWindow::Delegate, // Creates the process launcher. void CreateLauncher(scoped_refptr<AutoThreadTaskRunner> task_runner); - void OnChildStopped(); - // This function handshakes with the service control manager and starts // the service. int RunAsService(); @@ -70,6 +69,9 @@ class HostService : public win::MessageWindow::Delegate, // console application). int RunInConsole(); + // Stops and deletes |daemon_process_|. + void StopDaemonProcess(); + // win::MessageWindow::Delegate interface. virtual bool HandleMessage(HWND hwnd, UINT message, @@ -105,9 +107,10 @@ class HostService : public win::MessageWindow::Delegate, // The list of observers receiving session notifications. std::list<RegisteredObserver> observers_; - scoped_ptr<Stoppable> child_; + scoped_ptr<DaemonProcess> daemon_process_; - // Service message loop. + // Service message loop. |main_task_runner_| must be valid as long as the + // Control+C or service notification handler is registered. scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_; // The action routine to be executed. @@ -119,6 +122,10 @@ class HostService : public win::MessageWindow::Delegate, // A waitable event that is used to wait until the service is stopped. base::WaitableEvent stopped_event_; + // Used to post session change notifications and control events. + base::WeakPtrFactory<HostService> weak_factory_; + base::WeakPtr<HostService> weak_ptr_; + // Singleton. friend struct DefaultSingletonTraits<HostService>; diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp index 9eda077..1b21b38 100644 --- a/remoting/remoting.gyp +++ b/remoting/remoting.gyp @@ -2343,8 +2343,6 @@ 'base/running_average.h', 'base/socket_reader.cc', 'base/socket_reader.h', - 'base/stoppable.cc', - 'base/stoppable.h', 'base/typed_buffer.h', 'base/util.cc', 'base/util.h', |