Improve process launch handle sharing API.

base/process/launch.h

 // Copyright 2013 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 contains functions for launching subprocesses.
 
 #ifndef BASE_PROCESS_LAUNCH_H_
 #define BASE_PROCESS_LAUNCH_H_
 
 #include <stddef.h>
 
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/base_export.h"
 #include "base/environment.h"
 #include "base/macros.h"
+#include "base/optional.h"

[grt (UTC plus 2), 2017/06/27 08:01:44]

unused?

[brettw, 2017/06/28 21:42:21]

Oh yeah, Optional was my first attempt :/

 #include "base/process/process.h"
 #include "base/process/process_handle.h"
 #include "base/strings/string_piece.h"
 #include "build/build_config.h"
 
 #if defined(OS_POSIX)
 #include "base/posix/file_descriptor_shuffle.h"
 #elif defined(OS_WIN)
 #include <windows.h>
 #endif
 
 #if defined(OS_FUCHSIA)
 #include <magenta/types.h>
 #endif
 
 namespace base {
 
 class CommandLine;
 
-#if defined(OS_WIN)
-typedef std::vector<HANDLE> HandlesToInheritVector;
+#if defined(OS_POSIX)
+typedef std::vector<std::pair<int, int>> FileHandleMappingVector;
 #endif
-// TODO(viettrungluu): Only define this on POSIX?
-typedef std::vector<std::pair<int, int> > FileHandleMappingVector;
 
 // Options for launching a subprocess that are passed to LaunchProcess().
 // The default constructor constructs the object with default options.
 struct BASE_EXPORT LaunchOptions {
 #if defined(OS_POSIX)
   // Delegate to be run in between fork and exec in the subprocess (see
   // pre_exec_delegate below)
   class BASE_EXPORT PreExecDelegate {
    public:
     PreExecDelegate() {}
@@ skipping 15 matching lines @@
 
   // If true, wait for the process to complete.
   bool wait = false;
 
   // If not empty, change to this directory before executing the new process.
   base::FilePath current_directory;
 
 #if defined(OS_WIN)
   bool start_hidden = false;
 
-  // If non-null, inherit exactly the list of handles in this vector (these
-  // handles must be inheritable).
-  HandlesToInheritVector* handles_to_inherit = nullptr;
+  // Windows can inherit handles when it launches child processes.
+  // See https://blogs.msdn.microsoft.com/oldnewthing/20111216-00/?p=8873
+  // for a good overview of Windows handle inheritance.
+  //
+  // Implementation note: it might be nice to implement in terms of
+  // base::Optional<>, but then the natural default state (vector not present)
+  // would be "all inheritable handles" while we want "no inheritance."
+  enum InheritMode {

[grt (UTC plus 2), 2017/06/27 08:01:44]

nit: enum class?

[brettw, 2017/06/28 21:42:21]

Done.

+    // Only those handles in |handles_to_inherit| vector are inherited. If the
+    // vector is empty, no handles are inherited. The handles in the vector must
+    // all be inheritable.
+    INHERIT_SPECIFIC,

[grt (UTC plus 2), 2017/06/27 08:01:44]

nit: kFoo is now preferred
(https://groups.google.com/a/chromium.org/d/topic/chromium-dev/5AqwVQt-Etg/dis...)

[brettw, 2017/06/28 21:42:21]

Done.

 
-  // If true, the new process inherits handles from the parent. In production
-  // code this flag should be used only when running short-lived, trusted
-  // binaries, because open handles from other libraries and subsystems will
-  // leak to the child process, causing errors such as open socket hangs.
-  // Note: If |handles_to_inherit| is non-null, this flag is ignored and only
-  // those handles will be inherited.
-  bool inherit_handles = false;
+    // All handles in the current process which are inheritable are inherited.
+    // In production code this flag should be used only when running
+    // short-lived, trusted binaries, because open handles from other libraries
+    // and subsystems will leak to the child process, causing errors such as
+    // open socket hangs. There are also race conditions that can cause handle
+    // over-sharing.
+    //
+    // |handles_to_inherit| must be null.
+    INHERIT_ALL
+  };
+  InheritMode inherit_mode = INHERIT_SPECIFIC;
+  std::vector<HANDLE> handles_to_inherit;
 
   // If non-null, runs as if the user represented by the token had launched it.
   // Whether the application is visible on the interactive desktop depends on
   // the token belonging to an interactive logon session.
   //
   // To avoid hard to diagnose problems, when specified this loads the
   // environment variables associated with the user and if this operation fails
   // the entire call fails as well.
   UserTokenHandle as_user = nullptr;
 
   // If true, use an empty string for the desktop name.
   bool empty_desktop_name = false;
 
   // If non-null, launches the application in that job object. The process will
   // be terminated immediately and LaunchProcess() will fail if assignment to
   // the job object fails.
   HANDLE job_handle = nullptr;
 
-  // Handles for the redirection of stdin, stdout and stderr. The handles must
-  // be inheritable. Caller should either set all three of them or none (i.e.
-  // there is no way to redirect stderr without redirecting stdin). The
-  // |inherit_handles| flag must be set to true when redirecting stdio stream.
+  // Handles for the redirection of stdin, stdout and stderr. The caller should
+  // either set all three of them or none (i.e. there is no way to redirect
+  // stderr without redirecting stdin).
+  //
+  // The handles must be inheritable. Pseudo handles are used when stdout and
+  // stderr redirect to the console. In that case, GetFileType() will return
+  // FILE_TYPE_CHAR and they're automatically inherited by child processes. See
+  // https://msdn.microsoft.com/en-us/library/windows/desktop/ms682075.aspx
+  // Otherwise, the caller must ensure that the |inherit_mode| and/or
+  // |handles_to_inherit| set so that the handles are inherited.
   HANDLE stdin_handle = nullptr;
   HANDLE stdout_handle = nullptr;
   HANDLE stderr_handle = nullptr;
 
   // If set to true, ensures that the child process is launched with the
   // CREATE_BREAKAWAY_FROM_JOB flag which allows it to breakout of the parent
   // job if any.
   bool force_breakaway_from_job_ = false;
 #else  // !defined(OS_WIN)
   // Set/unset environment variables. These are applied on top of the parent
   // process environment.  Empty (the default) means to inherit the same
   // environment. See AlterEnvironment().
   EnvironmentMap environ;
 
   // Clear the environment for the new process before processing changes from
   // |environ|.
   bool clear_environ = false;
 
-  // If non-null, remap file descriptors according to the mapping of
-  // src fd->dest fd to propagate FDs into the child process.
-  // This pointer is owned by the caller and must live through the
-  // call to LaunchProcess().
-  const FileHandleMappingVector* fds_to_remap = nullptr;
+  // Remap file descriptors according to the mapping of src_fd->dest_fd to
+  // propagate FDs into the child process.
+  FileHandleMappingVector fds_to_remap;
 
   // Each element is an RLIMIT_* constant that should be raised to its
   // rlim_max.  This pointer is owned by the caller and must live through
   // the call to LaunchProcess().
   const std::vector<int>* maximize_rlimits = nullptr;
 
   // If true, start the process in a new process group, instead of
   // inheriting the parent's process group.  The pgid of the child process
   // will be the same as its pid.
   bool new_process_group = false;
@@ skipping 180 matching lines @@
 // multiple threads are running, since at the time the fork happened, the
 // threads could have been in any state (potentially holding locks, etc.).
 // Callers should most likely call execve() in the child soon after calling
 // this.
 BASE_EXPORT pid_t ForkWithFlags(unsigned long flags, pid_t* ptid, pid_t* ctid);
 #endif
 
 }  // namespace base
 
 #endif  // BASE_PROCESS_LAUNCH_H_