Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2424)

Unified Diff: remoting/host/daemon_process_win.cc

Issue 11040065: [Chromoting] Reimplemented the worker process launcher to take into account the encountered issues: (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Count the worker exiting too such as a failure. Created 8 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: remoting/host/daemon_process_win.cc
diff --git a/remoting/host/daemon_process_win.cc b/remoting/host/daemon_process_win.cc
index b299ed131f357986db00f912b50df39e8a4ab908..c29ebb644562729cc56c64c005836448824af3aa 100644
--- a/remoting/host/daemon_process_win.cc
+++ b/remoting/host/daemon_process_win.cc
@@ -8,6 +8,8 @@
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/logging.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
#include "base/path_service.h"
#include "base/single_thread_task_runner.h"
#include "base/time.h"
@@ -16,6 +18,7 @@
#include "base/win/scoped_handle.h"
#include "remoting/host/host_exit_codes.h"
#include "remoting/host/win/launch_process_with_token.h"
+#include "remoting/host/win/unprivileged_process_delegate.h"
#include "remoting/host/win/worker_process_launcher.h"
using base::win::ScopedHandle;
@@ -23,22 +26,9 @@ using base::TimeDelta;
namespace {
-// The minimum and maximum delays between attempts to launch the networking
-// process.
-const int kMaxLaunchDelaySeconds = 60;
-const int kMinLaunchDelaySeconds = 1;
-
const FilePath::CharType kMe2meHostBinaryName[] =
FILE_PATH_LITERAL("remoting_host.exe");
-// The IPC channel name is passed to the networking process in the command line.
-const char kDaemonPipeSwitchName[] = "daemon-pipe";
-
-// The command line parameters that should be copied from the service's command
-// line to the network process.
-const char* kCopiedSwitchNames[] = {
- "host-config", switches::kV, switches::kVModule };
-
// The security descriptor of the daemon IPC endpoint. It gives full access
// to LocalSystem and denies access by anyone else.
const char kDaemonPipeSecurityDescriptor[] = "O:SYG:SYD:(A;;GA;;;SY)";
@@ -47,26 +37,17 @@ const char kDaemonPipeSecurityDescriptor[] = "O:SYG:SYD:(A;;GA;;;SY)";
namespace remoting {
-class DaemonProcessWin : public DaemonProcess,
- public WorkerProcessLauncher::Delegate {
+class DaemonProcessWin : public DaemonProcess {
public:
DaemonProcessWin(scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
const base::Closure& stopped_callback);
virtual ~DaemonProcessWin();
- virtual void OnChannelConnected() OVERRIDE;
-
// Sends an IPC message to the worker process. This method can be called only
// after successful Start() and until Stop() is called or an error occurred.
virtual void Send(IPC::Message* message) OVERRIDE;
- // WorkerProcessLauncher::Delegate implementation.
- virtual bool DoLaunchProcess(
- const std::string& channel_name,
- ScopedHandle* process_exit_event_out) OVERRIDE;
- virtual void DoKillProcess(DWORD exit_code) OVERRIDE;
-
protected:
// Stoppable implementation.
virtual void DoStop() OVERRIDE;
@@ -75,25 +56,8 @@ class DaemonProcessWin : public DaemonProcess,
virtual void LaunchNetworkProcess() OVERRIDE;
private:
- // Called when the launcher reports the worker process has stopped.
- void OnLauncherStopped();
-
- // True if the network process is connected to the daemon.
- bool connected_;
-
- // Time of the last launch attempt.
- base::Time launch_time_;
-
- // Current backoff delay.
- base::TimeDelta launch_backoff_;
-
- // Timer used to schedule the next attempt to launch the process.
- base::OneShotTimer<DaemonProcessWin> timer_;
-
scoped_ptr<WorkerProcessLauncher> launcher_;
- ScopedHandle network_process_;
-
DISALLOW_COPY_AND_ASSIGN(DaemonProcessWin);
};
@@ -101,8 +65,7 @@ DaemonProcessWin::DaemonProcessWin(
scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
const base::Closure& stopped_callback)
- : DaemonProcess(main_task_runner, io_task_runner, stopped_callback),
- connected_(false) {
+ : DaemonProcess(main_task_runner, io_task_runner, stopped_callback) {
}
DaemonProcessWin::~DaemonProcessWin() {
@@ -115,162 +78,38 @@ DaemonProcessWin::~DaemonProcessWin() {
void DaemonProcessWin::LaunchNetworkProcess() {
DCHECK(main_task_runner()->BelongsToCurrentThread());
DCHECK(launcher_.get() == NULL);
- DCHECK(!network_process_.IsValid());
- DCHECK(!timer_.IsRunning());
-
- launch_time_ = base::Time::Now();
- launcher_.reset(new WorkerProcessLauncher(
- this, this,
- base::Bind(&DaemonProcessWin::OnLauncherStopped, base::Unretained(this)),
- main_task_runner(),
- io_task_runner()));
- launcher_->Start(kDaemonPipeSecurityDescriptor);
-}
-
-void DaemonProcessWin::OnChannelConnected() {
- connected_ = true;
- DaemonProcess::OnChannelConnected();
-}
-
-void DaemonProcessWin::Send(IPC::Message* message) {
- if (connected_) {
- launcher_->Send(message);
- } else {
- delete message;
- }
-}
-
-bool DaemonProcessWin::DoLaunchProcess(
- const std::string& channel_name,
- ScopedHandle* process_exit_event_out) {
- DCHECK(main_task_runner()->BelongsToCurrentThread());
- DCHECK(!network_process_.IsValid());
// Construct the host binary name.
FilePath dir_path;
if (!PathService::Get(base::DIR_EXE, &dir_path)) {
LOG(ERROR) << "Failed to get the executable file name.";
- return false;
- }
- FilePath host_binary = dir_path.Append(kMe2meHostBinaryName);
-
- // Create the host process command line, passing the name of the IPC channel
- // to use and copying known switches from the service's command line.
- CommandLine command_line(host_binary);
- command_line.AppendSwitchNative(kDaemonPipeSwitchName,
- UTF8ToWide(channel_name));
- command_line.CopySwitchesFrom(*CommandLine::ForCurrentProcess(),
- kCopiedSwitchNames,
- _countof(kCopiedSwitchNames));
-
- ScopedHandle token;
- if (!OpenProcessToken(GetCurrentProcess(),
- MAXIMUM_ALLOWED,
- token.Receive())) {
- LOG_GETLASTERROR(FATAL) << "Failed to open process token";
- return false;
- }
-
- // Try to launch the process and attach an object watcher to the returned
- // handle so that we get notified when the process terminates.
- // TODO(alexeypa): Pass a restricted process token.
- // See http://crbug.com/134694.
- ScopedHandle worker_thread;
- if (!LaunchProcessWithToken(host_binary,
- command_line.GetCommandLineString(),
- token,
- 0,
- &network_process_,
- &worker_thread)) {
- return false;
- }
-
- ScopedHandle process_exit_event;
- if (!DuplicateHandle(GetCurrentProcess(),
- network_process_,
- GetCurrentProcess(),
- process_exit_event.Receive(),
- SYNCHRONIZE,
- FALSE,
- 0)) {
- LOG_GETLASTERROR(ERROR) << "Failed to duplicate a handle";
- DoKillProcess(CONTROL_C_EXIT);
- return false;
+ Stop();
+ return;
}
- *process_exit_event_out = process_exit_event.Pass();
- return true;
+ scoped_ptr<UnprivilegedProcessDelegate> delegate(
+ new UnprivilegedProcessDelegate(main_task_runner(), io_task_runner(),
+ dir_path.Append(kMe2meHostBinaryName)));
+ launcher_.reset(new WorkerProcessLauncher(main_task_runner(),
+ io_task_runner(),
+ delegate.Pass(),
+ this,
+ kDaemonPipeSecurityDescriptor));
}
-void DaemonProcessWin::DoKillProcess(DWORD exit_code) {
- DCHECK(main_task_runner()->BelongsToCurrentThread());
- CHECK(network_process_.IsValid());
-
- TerminateProcess(network_process_, exit_code);
-}
-
-void DaemonProcessWin::DoStop() {
- DCHECK(main_task_runner()->BelongsToCurrentThread());
-
- timer_.Stop();
-
- if (launcher_.get() != NULL) {
- launcher_->Stop();
- }
-
- // Early exit if we're still waiting for |launcher_| to stop.
+void DaemonProcessWin::Send(IPC::Message* message) {
if (launcher_.get() != NULL) {
- return;
+ launcher_->Send(message);
+ } else {
+ delete message;
}
-
- DaemonProcess::DoStop();
}
-void DaemonProcessWin::OnLauncherStopped() {
+void DaemonProcessWin::DoStop() {
DCHECK(main_task_runner()->BelongsToCurrentThread());
- CHECK(network_process_.IsValid());
-
- DWORD exit_code = CONTROL_C_EXIT;
- if (!::GetExitCodeProcess(network_process_, &exit_code)) {
- LOG_GETLASTERROR(INFO)
- << "Failed to query the exit code of the worker process";
- exit_code = CONTROL_C_EXIT;
- }
-
- network_process_.Close();
- connected_ = false;
- launcher_.reset(NULL);
-
- // Do not relaunch the network process if the caller has asked us to stop.
- if (stoppable_state() != Stoppable::kRunning) {
- Stop();
- return;
- }
- // Stop trying to restart the worker process if its process exited due to
- // misconfiguration.
- if (kMinPermanentErrorExitCode <= exit_code &&
- exit_code <= kMaxPermanentErrorExitCode) {
- Stop();
- return;
- }
-
- // Expand the backoff interval if the process has died quickly or reset it
- // if it was up longer than the maximum backoff delay.
- base::TimeDelta delta = base::Time::Now() - launch_time_;
- if (delta < base::TimeDelta() ||
- delta >= base::TimeDelta::FromSeconds(kMaxLaunchDelaySeconds)) {
- launch_backoff_ = base::TimeDelta();
- } else {
- launch_backoff_ = std::max(
- launch_backoff_ * 2, TimeDelta::FromSeconds(kMinLaunchDelaySeconds));
- launch_backoff_ = std::min(
- launch_backoff_, TimeDelta::FromSeconds(kMaxLaunchDelaySeconds));
- }
-
- // Try to launch the worker process.
- timer_.Start(FROM_HERE, launch_backoff_,
- this, &DaemonProcessWin::LaunchNetworkProcess);
+ launcher_.reset();
+ DaemonProcess::DoStop();
}
scoped_ptr<DaemonProcess> DaemonProcess::Create(

Powered by Google App Engine
This is Rietveld 408576698