Index: remoting/host/win/worker_process_launcher.cc |
diff --git a/remoting/host/win/worker_process_launcher.cc b/remoting/host/win/worker_process_launcher.cc |
index 6ca563cf8939e86084b4f42474b1eae250510f3b..c2ea19e46e23cb01cdd0878c9405d53f4bfa33d0 100644 |
--- a/remoting/host/win/worker_process_launcher.cc |
+++ b/remoting/host/win/worker_process_launcher.cc |
@@ -4,6 +4,7 @@ |
#include "remoting/host/win/worker_process_launcher.h" |
+#include "base/location.h" |
#include "base/logging.h" |
#include "base/single_thread_task_runner.h" |
#include "base/time.h" |
@@ -13,6 +14,7 @@ |
#include "ipc/ipc_listener.h" |
#include "ipc/ipc_message.h" |
#include "net/base/backoff_entry.h" |
+#include "remoting/host/chromoting_messages.h" |
#include "remoting/host/worker_process_ipc_delegate.h" |
using base::TimeDelta; |
@@ -44,6 +46,7 @@ const net::BackoffEntry::Policy kDefaultBackoffPolicy = { |
false, |
}; |
+const int kDelayTimeoutSeconds = 5; |
Wez
2013/03/07 01:54:35
nit: Naming. e.g. kCrashRequestTimeoutSeconds?
Wez
2013/03/07 01:54:35
5s is probably too short for many systems, BTW.
alexeypa (please no reviews)
2013/03/07 21:28:30
Done.
alexeypa (please no reviews)
2013/03/07 21:28:30
It is enough because the timeout is, strictly spea
Wez
2013/03/08 01:45:15
Right, but if the timeout is short then on slower,
|
namespace remoting { |
@@ -52,7 +55,7 @@ namespace remoting { |
// method. In case of error the channel is closed and the worker process is |
// terminated. |
class WorkerProcessLauncher::Core |
- : public base::RefCountedThreadSafe<WorkerProcessLauncher::Core>, |
+ : public base::RefCountedThreadSafe<Core, Core>, |
public base::win::ObjectWatcher::Delegate, |
public IPC::Listener { |
public: |
@@ -75,9 +78,10 @@ class WorkerProcessLauncher::Core |
// to |this| as soon as Stop() returns. |
void Stop(); |
- // Sends an IPC message to the worker process. The message will be silently |
- // dropped if Send() is called before Start() or after stutdown has been |
- // initiated. |
+ // See WorkerProcessLauncher::Crash(). |
+ void Crash(const tracked_objects::Location& location); |
+ |
+ // See WorkerProcessLauncher::Send(). |
void Send(IPC::Message* message); |
// base::win::ObjectWatcher::Delegate implementation. |
@@ -88,8 +92,13 @@ class WorkerProcessLauncher::Core |
virtual void OnChannelConnected(int32 peer_pid) OVERRIDE; |
virtual void OnChannelError() OVERRIDE; |
+ // Destroys |Core| instances on the |caller_task_runner_| thread. This is |
+ // needed because base::OneShotTimer instances must be destroyed on the same |
+ // thread they were created on. |
+ static void Destruct(const Core* core); |
Wez
2013/03/07 01:54:35
We use an explicit separate struct e.g. CoreTraits
alexeypa (please no reviews)
2013/03/07 21:28:30
WorkerProcessLauncher::Core is a private class dec
Wez
2013/03/08 01:45:15
Surely the friend declaration would be in the Core
alexeypa (please no reviews)
2013/03/08 17:43:19
No, there will be one in WorkerProcessLauncher and
|
+ |
private: |
- friend class base::RefCountedThreadSafe<Core>; |
+ friend class base::DeleteHelper<Core>; |
virtual ~Core(); |
// Attempts to launch the worker process. Schedules next launch attempt if |
@@ -116,21 +125,21 @@ class WorkerProcessLauncher::Core |
typedef BOOL (WINAPI * GetNamedPipeClientProcessIdFn)(HANDLE, DWORD*); |
GetNamedPipeClientProcessIdFn get_named_pipe_client_pid_; |
+ // The timer used to delay termination of the worker process when an IPC error |
+ // occured or when Crash() request is pending |
+ base::OneShotTimer<Core> delay_timer_; |
Wez
2013/03/07 01:54:35
nit: Naming.
alexeypa (please no reviews)
2013/03/07 21:28:30
Done.
|
+ |
// True if IPC messages should be passed to |worker_delegate_|. |
bool ipc_enabled_; |
- // The timer used to delay termination of the process in the case of an IPC |
- // error. |
- scoped_ptr<base::OneShotTimer<Core> > ipc_error_timer_; |
- |
// Launch backoff state. |
net::BackoffEntry launch_backoff_; |
// Timer used to delay recording a successfull launch. |
- scoped_ptr<base::OneShotTimer<Core> > launch_success_timer_; |
+ base::OneShotTimer<Core> launch_success_timer_; |
// Timer used to schedule the next attempt to launch the process. |
- scoped_ptr<base::OneShotTimer<Core> > launch_timer_; |
+ base::OneShotTimer<Core> launch_timer_; |
// Used to determine when the launched process terminates. |
base::win::ObjectWatcher process_watcher_; |
@@ -160,11 +169,6 @@ WorkerProcessLauncher::Core::Core( |
launch_backoff_(&kDefaultBackoffPolicy), |
stopping_(false) { |
DCHECK(caller_task_runner_->BelongsToCurrentThread()); |
- |
- // base::OneShotTimer must be destroyed on the same thread it was created on. |
- ipc_error_timer_.reset(new base::OneShotTimer<Core>()); |
- launch_success_timer_.reset(new base::OneShotTimer<Core>()); |
- launch_timer_.reset(new base::OneShotTimer<Core>()); |
} |
void WorkerProcessLauncher::Core::Start() { |
@@ -184,6 +188,29 @@ void WorkerProcessLauncher::Core::Stop() { |
} |
} |
+void WorkerProcessLauncher::Core::Crash( |
+ const tracked_objects::Location& location) { |
+ DCHECK(caller_task_runner_->BelongsToCurrentThread()); |
+ |
+ // Ask the worker process to crash voluntarily if it is still connected. |
+ if (ipc_enabled_) { |
+ Send(new ChromotingDaemonMsg_Crash(location.function_name(), |
+ location.file_name(), |
+ location.line_number())); |
+ } |
+ |
+ // Close the channel and ignore any not yet processed messages. |
+ launcher_delegate_->CloseChannel(); |
+ ipc_enabled_ = false; |
+ |
+ // Give the worker process some time to crash. |
+ if (!delay_timer_.IsRunning()) { |
+ delay_timer_.Start(FROM_HERE, |
+ base::TimeDelta::FromSeconds(kDelayTimeoutSeconds), this, |
+ &Core::StopWorker); |
Wez
2013/03/07 01:54:35
StopWorker "stops the worker asynchronously". Is i
alexeypa (please no reviews)
2013/03/07 21:28:30
It is tougher than Chuck Norris (as long as we are
|
+ } |
+} |
+ |
void WorkerProcessLauncher::Core::Send(IPC::Message* message) { |
DCHECK(caller_task_runner_->BelongsToCurrentThread()); |
@@ -241,19 +268,28 @@ void WorkerProcessLauncher::Core::OnChannelError() { |
// here allows the worker to exit completely and so, notify |
// |process_watcher_|. As the result KillProcess() will not be called and |
// the original exit code reported by the worker process will be retrieved. |
- ipc_error_timer_->Start(FROM_HERE, base::TimeDelta::FromSeconds(5), |
- this, &Core::StopWorker); |
+ if (!delay_timer_.IsRunning()) { |
+ delay_timer_.Start(FROM_HERE, |
+ base::TimeDelta::FromSeconds(kDelayTimeoutSeconds), this, |
+ &Core::StopWorker); |
+ } |
+} |
+ |
+// static |
+void WorkerProcessLauncher::Core::Destruct(const Core* core) { |
+ core->caller_task_runner_->DeleteSoon(FROM_HERE, core); |
} |
WorkerProcessLauncher::Core::~Core() { |
+ DCHECK(caller_task_runner_->BelongsToCurrentThread()); |
DCHECK(stopping_); |
} |
void WorkerProcessLauncher::Core::LaunchWorker() { |
DCHECK(caller_task_runner_->BelongsToCurrentThread()); |
DCHECK(!ipc_enabled_); |
- DCHECK(!launch_success_timer_->IsRunning()); |
- DCHECK(!launch_timer_->IsRunning()); |
+ DCHECK(!launch_success_timer_.IsRunning()); |
+ DCHECK(!launch_timer_.IsRunning()); |
DCHECK(!process_exit_event_.IsValid()); |
DCHECK(process_watcher_.GetWatchedObject() == NULL); |
@@ -264,8 +300,8 @@ void WorkerProcessLauncher::Core::LaunchWorker() { |
ipc_enabled_ = true; |
// Record a successful launch once the process has been running for at |
// least two seconds. |
- launch_success_timer_->Start(FROM_HERE, base::TimeDelta::FromSeconds(2), |
- this, &Core::RecordSuccessfulLaunch); |
+ launch_success_timer_.Start(FROM_HERE, base::TimeDelta::FromSeconds(2), |
+ this, &Core::RecordSuccessfulLaunch); |
return; |
} |
@@ -289,8 +325,8 @@ void WorkerProcessLauncher::Core::StopWorker() { |
scoped_refptr<Core> self = this; |
// Record a launch failure if the process exited too soon. |
- if (launch_success_timer_->IsRunning()) { |
- launch_success_timer_->Stop(); |
+ if (launch_success_timer_.IsRunning()) { |
+ launch_success_timer_.Stop(); |
launch_backoff_.InformOfRequest(false); |
} |
@@ -310,23 +346,20 @@ void WorkerProcessLauncher::Core::StopWorker() { |
process_watcher_.StopWatching(); |
} |
- ipc_error_timer_->Stop(); |
+ delay_timer_.Stop(); |
process_exit_event_.Close(); |
// Do not relaunch the worker process if the caller has asked us to stop. |
- if (stopping_) { |
- ipc_error_timer_.reset(); |
- launch_timer_.reset(); |
+ if (stopping_) |
return; |
- } |
if (launcher_delegate_->IsPermanentError(launch_backoff_.failure_count())) { |
if (!stopping_) |
worker_delegate_->OnPermanentError(); |
} else { |
// Schedule the next attempt to launch the worker process. |
- launch_timer_->Start(FROM_HERE, launch_backoff_.GetTimeUntilRelease(), |
- this, &Core::LaunchWorker); |
+ launch_timer_.Start(FROM_HERE, launch_backoff_.GetTimeUntilRelease(), this, |
+ &Core::LaunchWorker); |
} |
} |
@@ -344,6 +377,10 @@ WorkerProcessLauncher::~WorkerProcessLauncher() { |
core_ = NULL; |
} |
+void WorkerProcessLauncher::Crash(const tracked_objects::Location& location) { |
+ core_->Crash(location); |
+} |
+ |
void WorkerProcessLauncher::Send(IPC::Message* message) { |
core_->Send(message); |
} |