[Chromoting] Moving common logic responsible for launching child processes to...

[Chromoting] Moving common logic responsible for launching child processes to WorkerProcessLauncher class. Launches processes are expected to connect back via a Chromium IPC channel (and their identify can be verified at this point). The class also monitors lifetime of the launched process invoking the normal shutdown sequence in case of premature death of the worker process.

BUG=134694

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

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

remoting/base/stoppable.cc

@@ -26,8 +26,10 @@ void Stoppable::Stop() {
 
   if (state_ == kRunning) {
     state_ = kStopping;
-    DoStop();
   }
+
+  // DoStop() can be called multiple times.
+  DoStop();
 }
 
 void Stoppable::CompleteStopping() {

remoting/host/win/launch_process_with_token.cc

@@ -345,7 +345,7 @@ bool CreateSessionToken(uint32 session_id, ScopedHandle* token_out) {
 bool LaunchProcessWithToken(const FilePath& binary,
                             const CommandLine::StringType& command_line,
                             HANDLE user_token,
-                            base::Process* process_out) {
+                            ScopedHandle* process_out) {
   FilePath::StringType application_name = binary.value();
 
   base::win::ScopedProcessInformation process_info;
@@ -404,7 +404,7 @@ bool LaunchProcessWithToken(const FilePath& binary,
   }
 
   CHECK(process_info.IsValid());
-  process_out->set_handle(process_info.TakeProcessHandle());
+  process_out->Set(process_info.TakeProcessHandle());
   return true;
 }
 

remoting/host/win/launch_process_with_token.h

@@ -10,7 +10,6 @@
 
 #include "base/command_line.h"
 #include "base/file_path.h"
-#include "base/process_util.h"
 #include "base/win/scoped_handle.h"
 
 namespace remoting {
@@ -24,7 +23,7 @@ bool CreateSessionToken(uint32 session_id, base::win::ScopedHandle* token_out);
 bool LaunchProcessWithToken(const FilePath& binary,
                             const CommandLine::StringType& command_line,
                             HANDLE user_token,
-                            base::Process* process_out);
+                            base::win::ScopedHandle* process_out);
 
 } // namespace remoting
 

remoting/host/win/worker_process_launcher.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.
+
+#include "remoting/host/win/worker_process_launcher.h"
+
+#include <windows.h>
+#include <sddl.h>
+#include <limits>
+
+#include "base/base_switches.h"
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/logging.h"
+#include "base/single_thread_task_runner.h"
+#include "base/process_util.h"
+#include "base/rand_util.h"
+#include "base/stringprintf.h"
+#include "base/utf_string_conversions.h"
+#include "base/win/scoped_handle.h"
+#include "ipc/ipc_channel_proxy.h"
+#include "ipc/ipc_message.h"
+
+using base::win::ScopedHandle;
+
+namespace {
+
+// Match the pipe name prefix used by Chrome IPC channels so that the client
+// could use Chrome IPC APIs instead of connecting manually.
+const char kChromePipeNamePrefix[] = "\\\\.\\pipe\\chrome.";
+
+} // namespace
+
+namespace remoting {
+
+WorkerProcessLauncher::Delegate::~Delegate() {
+}
+
+WorkerProcessLauncher::WorkerProcessLauncher(
+    Delegate* delegate,
+    const base::Closure& stopped_callback,
+    scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
+    scoped_refptr<base::SingleThreadTaskRunner> ipc_task_runner)
+    : Stoppable(main_task_runner, stopped_callback),
+      delegate_(delegate),
+      main_task_runner_(main_task_runner),
+      ipc_task_runner_(ipc_task_runner) {
+}
+
+WorkerProcessLauncher::~WorkerProcessLauncher() {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+}
+
+void WorkerProcessLauncher::Start(const std::string& pipe_sddl) {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+  DCHECK(ipc_channel_.get() == NULL);
+  DCHECK(!pipe_.IsValid());
+  DCHECK(!process_exit_event_.IsValid());
+  DCHECK(process_watcher_.GetWatchedObject() == NULL);
+
+  std::string channel_name = GenerateRandomChannelId();
+  if (CreatePipeForIpcChannel(channel_name, pipe_sddl, &pipe_)) {
+    // Wrap the pipe into an IPC channel.
+    ipc_channel_.reset(new IPC::ChannelProxy(
+        IPC::ChannelHandle(pipe_),
+        IPC::Channel::MODE_SERVER,
+        this,
+        ipc_task_runner_));
+
+    // Launch the process and attach an object watcher to the returned process
+    // handle so that we get notified if the process terminates.
+    if (delegate_->DoLaunchProcess(channel_name, &process_exit_event_)) {
+      if (process_watcher_.StartWatching(process_exit_event_, this)) {
+        return;
+      }
+
+      delegate_->DoKillProcess(CONTROL_C_EXIT);
+    }
+  }
+
+  Stop();
+}
+
+void WorkerProcessLauncher::Send(IPC::Message* message) {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+
+  ipc_channel_->Send(message);
+}
+
+void WorkerProcessLauncher::OnObjectSignaled(HANDLE object) {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+  DCHECK(process_watcher_.GetWatchedObject() == NULL);
+
+  Stop();
+}
+
+bool WorkerProcessLauncher::OnMessageReceived(const IPC::Message& message) {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+  DCHECK(ipc_channel_.get() != NULL);
+  DCHECK(pipe_.IsValid());
+  DCHECK(process_exit_event_.IsValid());
+
+  return delegate_->OnMessageReceived(message);
+}
+
+void WorkerProcessLauncher::OnChannelConnected(int32 peer_pid) {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+  DCHECK(ipc_channel_.get() != NULL);
+  DCHECK(pipe_.IsValid());
+  DCHECK(process_exit_event_.IsValid());
+
+  // Get the actual peer's PID (i.e. reported by the OS) instead of the PID
+  // reported by the peer itself (|peer_pid|).
+  DWORD actual_peer_pid;
+  if (!GetNamedPipeClientProcessId(pipe_, &actual_peer_pid)) {
+    LOG_GETLASTERROR(ERROR) << "Failed to query the peer's PID";
+    Stop();
+    return;
+  }
+
+  delegate_->OnChannelConnected(actual_peer_pid);
+}
+
+void WorkerProcessLauncher::OnChannelError() {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+  DCHECK(ipc_channel_.get() != NULL);
+  DCHECK(pipe_.IsValid());
+  DCHECK(process_exit_event_.IsValid());
+
+  Stop();
+}
+
+void WorkerProcessLauncher::DoStop() {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+
+  ipc_channel_.reset();
+  pipe_.Close();
+
+  // Kill the process if it has been started already.
+  if (process_watcher_.GetWatchedObject() != NULL) {
+    delegate_->DoKillProcess(CONTROL_C_EXIT);
+    return;
+  }
+
+  DCHECK(ipc_channel_.get() == NULL);
+  DCHECK(!pipe_.IsValid());
+  DCHECK(process_watcher_.GetWatchedObject() == NULL);
+
+  process_exit_event_.Close();
+  CompleteStopping();
+}
+
+// Creates the server end of the Chromoting IPC channel.
+bool WorkerProcessLauncher::CreatePipeForIpcChannel(
+    const std::string& channel_name,
+    const std::string& pipe_sddl,
+    ScopedHandle* pipe_out) {
+  // Create security descriptor for the channel.
+  SECURITY_ATTRIBUTES security_attributes;
+  security_attributes.nLength = sizeof(security_attributes);
+  security_attributes.bInheritHandle = FALSE;
+
+  ULONG security_descriptor_length = 0;
+  if (!ConvertStringSecurityDescriptorToSecurityDescriptor(
+          UTF8ToUTF16(pipe_sddl).c_str(),
+          SDDL_REVISION_1,
+          reinterpret_cast<PSECURITY_DESCRIPTOR*>(
+              &security_attributes.lpSecurityDescriptor),
+          &security_descriptor_length)) {
+    LOG_GETLASTERROR(ERROR) <<
+        "Failed to create a security descriptor for the Chromoting IPC channel";
+    return false;
+  }
+
+  // Convert the channel name to the pipe name.
+  std::string pipe_name(kChromePipeNamePrefix);
+  pipe_name.append(channel_name);
+
+  // Create the server end of the pipe. This code should match the code in
+  // IPC::Channel with exception of passing a non-default security descriptor.
+  base::win::ScopedHandle pipe;
+  pipe.Set(CreateNamedPipe(
+      UTF8ToUTF16(pipe_name).c_str(),
+      PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED | FILE_FLAG_FIRST_PIPE_INSTANCE,
+      PIPE_TYPE_BYTE | PIPE_READMODE_BYTE,
+      1,
+      IPC::Channel::kReadBufferSize,
+      IPC::Channel::kReadBufferSize,
+      5000,
+      &security_attributes));
+  if (!pipe.IsValid()) {
+    LOG_GETLASTERROR(ERROR) <<
+        "Failed to create the server end of the Chromoting IPC channel";
+    LocalFree(security_attributes.lpSecurityDescriptor);
+    return false;
+  }
+
+  LocalFree(security_attributes.lpSecurityDescriptor);
+
+  *pipe_out = pipe.Pass();
+  return true;
+}
+
+// N.B. Copied from src/content/common/child_process_host_impl.cc
+std::string WorkerProcessLauncher::GenerateRandomChannelId() {
+  return base::StringPrintf("%d.%p.%d",
+                            base::GetCurrentProcId(), this,
+                            base::RandInt(0, std::numeric_limits<int>::max()));
+}
+
+} // namespace remoting

remoting/host/win/worker_process_launcher.h

+// 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_HOST_WIN_WORKER_PROCESS_LAUNCHER_H_
+#define REMOTING_HOST_WIN_WORKER_PROCESS_LAUNCHER_H_
+
+#include <windows.h>
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/process.h"
+#include "base/win/scoped_handle.h"
+#include "base/win/object_watcher.h"
+#include "ipc/ipc_channel.h"
+#include "remoting/base/stoppable.h"
+
+namespace base {
+class SingleThreadTaskRunner;
+} // namespace base
+
+namespace IPC {
+class ChannelProxy;
+class Message;
+} // namespace IPC
+
+namespace remoting {
+
+// Launches a worker process that is controlled via an IPC channel. All
+// interaction with the spawned process is through the IPC::Listener and Send()
+// method. In case of error the channel is closed and the worker process is
+// terminated.
+//
+// WorkerProcessLauncher object is good for one process launch attempt only.
+class WorkerProcessLauncher
+    : public Stoppable,
+      public base::win::ObjectWatcher::Delegate,
+      public IPC::Listener {
+ public:
+  class Delegate {
+   public:
+    virtual ~Delegate();
+
+    // Starts the worker process and passes |channel_name| to it.
+    // |process_exit_event_out| receives a handle that becomes signalled once
+    // the launched process has been terminated.
+    virtual bool DoLaunchProcess(
+        const std::string& channel_name,
+        base::win::ScopedHandle* process_exit_event_out) = 0;
+
+    // Terminates the worker process with the given exit code.
+    virtual void DoKillProcess(DWORD exit_code) = 0;
+
+    // Notifies that a client has been connected to the channel. |peer_pid|
+    // is the peer process's ID that the delegate can use to verify identity of
+    // the client. The verification code has to make sure that the client
+    // process's PID will not be assigned to another process (for instance by
+    // keeping an opened handle of the client process).
+    virtual void OnChannelConnected(DWORD peer_pid) = 0;
+
+    // Processes messages sent by the client.
+    virtual bool OnMessageReceived(const IPC::Message& message) = 0;
+  };
+
+  // Creates the launcher.
+  // |delegate| will be able to receive messages sent over the channel once
+  // the worker has been started and until it is stopped by Stop() or an error
+  // occurs.
+  //
+  // |stopped_callback| and |main_task_runner| are passed to the underlying
+  // |Stoppable| implementation. The caller should call all the methods on this
+  // class on the |main_task_runner| thread. |ipc_task_runner| is used to
+  // perform background IPC I/O.
+  WorkerProcessLauncher(
+      Delegate* delegate,
+      const base::Closure& stopped_callback,
+      scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
+      scoped_refptr<base::SingleThreadTaskRunner> ipc_task_runner);
+  virtual ~WorkerProcessLauncher();
+
+  // Starts the worker process.
+  void Start(const std::string& pipe_sddl);
+
+  // 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.
+  void Send(IPC::Message* message);
+
+  // base::win::ObjectWatcher::Delegate implementation.
+  virtual void OnObjectSignaled(HANDLE object) OVERRIDE;
+
+  // IPC::Listener implementation.
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
+  virtual void OnChannelConnected(int32 peer_pid) OVERRIDE;
+  virtual void OnChannelError() OVERRIDE;
+
+ protected:
+  // Stoppable implementation.
+  virtual void DoStop() OVERRIDE;
+
+ private:
+  // Creates the server end of the Chromoting IPC channel.
+  bool CreatePipeForIpcChannel(const std::string& channel_name,
+                               const std::string& pipe_sddl,
+                               base::win::ScopedHandle* pipe_out);
+
+  // Generates random channel ID.
+  std::string GenerateRandomChannelId();
+
+  Delegate* delegate_;
+
+  // The main service message loop.
+  scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
+
+  // Message loop used by the IPC channel.
+  scoped_refptr<base::SingleThreadTaskRunner> ipc_task_runner_;
+
+  // Used to determine when the launched process terminates.
+  base::win::ObjectWatcher process_watcher_;
+
+  // A waiting handle that becomes signalled once the launched process has
+  // been terminated.
+  base::win::ScopedHandle process_exit_event_;
+
+  // The IPC channel connecting to the launched process.
+  scoped_ptr<IPC::ChannelProxy> ipc_channel_;
+
+  // The server end of the pipe.
+  base::win::ScopedHandle pipe_;
+
+  DISALLOW_COPY_AND_ASSIGN(WorkerProcessLauncher);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_WIN_WORKER_PROCESS_LAUNCHER_H_

remoting/host/win/wts_session_process_launcher.h

@@ -15,9 +15,9 @@
 #include "base/time.h"
 #include "base/timer.h"
 #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/worker_process_launcher.h"
 #include "remoting/host/win/wts_console_observer.h"
 
 namespace base {
@@ -36,13 +36,13 @@ class WtsConsoleMonitor;
 
 class WtsSessionProcessLauncher
     : public Stoppable,
-      public base::win::ObjectWatcher::Delegate,
-      public IPC::Listener,
+      public WorkerProcessLauncher::Delegate,
       public WtsConsoleObserver {
  public:
-  // Constructs a WtsSessionProcessLauncher object. All interaction with
-  // |monitor| should happen on |main_message_loop|. |ipc_message_loop| has
-  // to be an I/O message loop.
+  // Constructs a WtsSessionProcessLauncher object. |stopped_callback| and
+  // |main_message_loop| are passed to the undelying |Stoppable| implementation.
+  // All interaction with |monitor| should happen on |main_message_loop|.
+  // |ipc_message_loop| must be an I/O message loop.
   WtsSessionProcessLauncher(
       const base::Closure& stopped_callback,
       WtsConsoleMonitor* monitor,
@@ -51,10 +51,12 @@ class WtsSessionProcessLauncher
 
   virtual ~WtsSessionProcessLauncher();
 
-  // base::win::ObjectWatcher::Delegate implementation.
-  virtual void OnObjectSignaled(HANDLE object) OVERRIDE;
-
-  // IPC::Listener implementation.
+  // WorkerProcessLauncher::Delegate implementation.
+  virtual bool DoLaunchProcess(
+      const std::string& channel_name,
+      base::win::ScopedHandle* process_exit_event_out) OVERRIDE;
+  virtual void DoKillProcess(DWORD exit_code) OVERRIDE;
+  virtual void OnChannelConnected(DWORD peer_pid) OVERRIDE;
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
   // WtsConsoleObserver implementation.
@@ -71,10 +73,16 @@ class WtsSessionProcessLauncher
   // reason.
   void LaunchProcess();
 
+  // Called when the launcher reports the process to be stopped.
+  void OnLauncherStopped();
+
   // Sends the Secure Attention Sequence to the session represented by
   // |session_token_|.
   void OnSendSasToConsole();
 
+  // |true| if this object is currently attached to the console session.
+  bool attached_;
+
   // Time of the last launch attempt.
   base::Time launch_time_;
 
@@ -93,29 +101,13 @@ class WtsSessionProcessLauncher
   // This pointer is used to unsubscribe from session attach and detach events.
   WtsConsoleMonitor* monitor_;
 
-  // The handle of the process injected into the console session.
-  base::Process process_;
+  scoped_ptr<WorkerProcessLauncher> launcher_;
 
-  // Used to determine when the launched process terminates.
-  base::win::ObjectWatcher process_watcher_;
+  base::win::ScopedHandle worker_process_;
 
   // The token to be used to launch a process in a different session.
   base::win::ScopedHandle session_token_;
 
-  // Defines the states the process launcher can be in.
-  enum State {
-    StateDetached,
-    StateStarting,
-    StateAttached,
-  };
-
-  // Current state of the process launcher.
-  State state_;
-
-  // The Chromoting IPC channel connecting the service to the per-session
-  // process.
-  scoped_ptr<IPC::ChannelProxy> chromoting_channel_;
-
   scoped_ptr<SasInjector> sas_injector_;
 
   DISALLOW_COPY_AND_ASSIGN(WtsSessionProcessLauncher);

remoting/remoting.gyp

@@ -620,6 +620,8 @@
             'host/win/host_service_resource.h',
             'host/win/launch_process_with_token.cc',
             'host/win/launch_process_with_token.h',
+            'host/win/worker_process_launcher.cc',
+            'host/win/worker_process_launcher.h',
             'host/win/wts_console_monitor.h',
             'host/win/wts_console_observer.h',
             'host/win/wts_session_process_launcher.cc',