New NaCl zygote implementation 2

base/process_util_posix.cc

 // Copyright (c) 2011 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 <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <signal.h>
 #include <stdlib.h>
 #include <sys/resource.h>
@@ skipping 490 matching lines @@
   ret[k] = NULL;
   return ret;
 }
 
 bool LaunchAppImpl(
     const std::vector<std::string>& argv,
     const environment_vector& env_changes,
     const file_handle_mapping_vector& fds_to_remap,
     bool wait,
     ProcessHandle* process_handle,
-    bool start_new_process_group) {
+    bool start_new_process_group,
+    bool use_clone,
+    int clone_flags) {
   pid_t pid;
   InjectiveMultimap fd_shuffle1, fd_shuffle2;
   fd_shuffle1.reserve(fds_to_remap.size());
   fd_shuffle2.reserve(fds_to_remap.size());
   scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
   scoped_array<char*> new_environ(AlterEnvironment(env_changes, environ));
 
-  pid = fork();
+  if (use_clone) {
+#if defined(OS_LINUX)
+    pid = syscall(__NR_clone, clone_flags, 0, 0, 0);
+#else
+    NOTREACHED();
+    LOG(ERROR) << "Tried to use clone() on non-Linux system.";

[darin (slow to review), 2011/06/24 16:32:19]

it seems like we should only reach this code in a debug build.  i recommend
collapsing NOTREACHED and LOG(ERROR) together into just:

  NOTREACHED() << "Tried to use clone() on non-Linux system.";

i.e., there doesn't seem to be much of a benefit to shipping the string "Tried
to use clone() on non-Linux system." in release builds on other platforms.

[Brad Chen, 2011/06/24 16:52:12]

Done.

+#endif
+  } else {
+    pid = fork();
+  }
   if (pid < 0) {
     PLOG(ERROR) << "fork";
     return false;
   }
   if (pid == 0) {
     // Child process
 
     // DANGER: fork() rule: in the child, if you don't end up doing exec*(),
     // you call _exit() instead of exit(). This is because _exit() does not
     // call any previously-registered (in the parent) exit handlers, which
@@ skipping 80 matching lines @@
   return true;
 }
 
 bool LaunchApp(
     const std::vector<std::string>& argv,
     const environment_vector& env_changes,
     const file_handle_mapping_vector& fds_to_remap,
     bool wait,
     ProcessHandle* process_handle) {
   return LaunchAppImpl(argv, env_changes, fds_to_remap,
-                       wait, process_handle, false);
+                       wait, process_handle,
+                       false,  // don't start new process group
+                       false,  // don't use clone()
+                       0);     // clone flags
 }
 
 bool LaunchAppInNewProcessGroup(
     const std::vector<std::string>& argv,
     const environment_vector& env_changes,
     const file_handle_mapping_vector& fds_to_remap,
     bool wait,
     ProcessHandle* process_handle) {
   return LaunchAppImpl(argv, env_changes, fds_to_remap, wait,
-                       process_handle, true);
+                       process_handle,
+                       true,   // start new process group
+                       false,  // don't use clone()
+                       0);     // clone flags
+}
+
+BASE_API bool LaunchAppWithClone(const std::vector<std::string>& argv,
+                                 const file_handle_mapping_vector& fds_to_remap,
+                                 bool wait, ProcessHandle* process_handle,
+                                 int clone_flags) {
+  base::environment_vector no_env;
+  return LaunchAppImpl(argv, no_env, fds_to_remap, wait, process_handle,
+                       false,  // don't start new process group
+                       true,  // use clone()

[darin (slow to review), 2011/06/24 16:32:19]

nit: elsewhere, you stack the comments in a column.  maybe it should be like
that here as well?

nit: doesn't need to be part of this CL, but generally when you end up with a
callsite that consists of a list of booleans, it is helpful to define some
enums.  that can help make the callsites self documenting so you don't need to
type out the comments and hope that they don't get out of sync with the code.

[Brad Chen, 2011/06/24 16:52:12]

Fixed.
Will fix on a separate CL.

+                       clone_flags);
 }
 
 bool LaunchApp(const std::vector<std::string>& argv,
                const file_handle_mapping_vector& fds_to_remap,
                bool wait, ProcessHandle* process_handle) {
   base::environment_vector no_env;
   return LaunchApp(argv, no_env, fds_to_remap, wait, process_handle);
 }
 
 bool LaunchApp(const CommandLine& cl,
@@ skipping 364 matching lines @@
                       const ProcessFilter* filter) {
   bool exited_cleanly =
       WaitForProcessesToExit(executable_name, wait_milliseconds,
                              filter);
   if (!exited_cleanly)
     KillProcesses(executable_name, exit_code, filter);
   return exited_cleanly;
 }
 
 }  // namespace base