Synchronize the parent and child processes during launch, ensuring that the

child doesn't try to check in with the Mach broker before the parent is ready.
This resolves a race between the browser and its child processes.

base::LaunchOptions gets a new member, synchronize, currently Mac-only
although there's nothing Mac-specific about the implementation. When set,
it allows base::LaunchProcess clients to request that the child process block
until released by the new base::LaunchSynchronize call. When creating child
processes, the child process launcher now requests this new synchronization
behavior to give it a chance to set up the Mach broker to expect the child
before the child tries to check in with it.

BUG=94295
TEST=Chrome should work. You see fewer of this logged:
     Unknown process (pid) is sending Mach IPC messages!
     But you still might see some of these due to bug 94543.
Review URL: http://codereview.chromium.org/7740036

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

1 files changed, 112 insertions, 13 deletions

diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc
index 059b7d7..06fa779 100644
--- a/base/process_util_posix.cc
+++ b/base/process_util_posix.cc
@@ -531,32 +531,72 @@ char** AlterEnvironment(const environment_vector& changes,
 bool LaunchProcess(const std::vector<std::string>& argv,
                    const LaunchOptions& options,
                    ProcessHandle* process_handle) {
-  pid_t pid;
-  InjectiveMultimap fd_shuffle1, fd_shuffle2;
+  size_t fd_shuffle_size = 0;
   if (options.fds_to_remap) {
-    fd_shuffle1.reserve(options.fds_to_remap->size());
-    fd_shuffle2.reserve(options.fds_to_remap->size());
+    fd_shuffle_size = options.fds_to_remap->size();
   }
+
+#if defined(OS_MACOSX)
+  if (options.synchronize) {
+    // When synchronizing, the "read" end of the synchronization pipe needs
+    // to make it to the child process. This is handled by mapping it back to
+    // itself.
+    ++fd_shuffle_size;
+  }
+#endif  // defined(OS_MACOSX)
+
+  InjectiveMultimap fd_shuffle1;
+  InjectiveMultimap fd_shuffle2;
+  fd_shuffle1.reserve(fd_shuffle_size);
+  fd_shuffle2.reserve(fd_shuffle_size);
+
   scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
   scoped_array<char*> new_environ;
   if (options.environ)
     new_environ.reset(AlterEnvironment(*options.environ, GetEnvironment()));
 
-  if (options.clone_flags) {
+#if defined(OS_MACOSX)
+  int synchronization_pipe_fds[2];
+  file_util::ScopedFD synchronization_read_fd;
+  file_util::ScopedFD synchronization_write_fd;
+
+  if (options.synchronize) {
+    // wait means "don't return from LaunchProcess until the child exits", and
+    // synchronize means "return from LaunchProcess but don't let the child
+    // run until LaunchSynchronize is called". These two options are highly
+    // incompatible.
+    CHECK(!options.wait);
+
+    // Create the pipe used for synchronization.
+    if (HANDLE_EINTR(pipe(synchronization_pipe_fds)) != 0) {
+      PLOG(ERROR) << "pipe";
+      return false;
+    }
+
+    // The parent process will only use synchronization_write_fd as the write
+    // side of the pipe. It can close the read side as soon as the child
+    // process has forked off. The child process will only use
+    // synchronization_read_fd as the read side of the pipe. In that process,
+    // the write side can be closed as soon as it has forked.
+    synchronization_read_fd.reset(&synchronization_pipe_fds[0]);
+    synchronization_write_fd.reset(&synchronization_pipe_fds[1]);
+  }
+#endif  // OS_MACOSX
+
+  pid_t pid;
 #if defined(OS_LINUX)
+  if (options.clone_flags) {
     pid = syscall(__NR_clone, options.clone_flags, 0, 0, 0);
-#else
-    pid = -1;  // hygiene; prevent clang warnings
-    NOTREACHED() << "Tried to use clone() on non-Linux system";
+  } else
 #endif
-  } else {
+  {
     pid = fork();
   }
+
   if (pid < 0) {
     PLOG(ERROR) << "fork";
     return false;
-  }
-  if (pid == 0) {
+  } else if (pid == 0) {
     // Child process
 
     // DANGER: fork() rule: in the child, if you don't end up doing exec*(),
@@ -589,12 +629,20 @@ bool LaunchProcess(const std::vector<std::string>& argv,
         _exit(127);
       }
     }
+
 #if defined(OS_MACOSX)
     RestoreDefaultExceptionHandler();
-#endif
+#endif  // defined(OS_MACOSX)
 
     ResetChildSignalHandlersToDefaults();
 
+#if defined(OS_MACOSX)
+    if (options.synchronize) {
+      // The "write" side of the synchronization pipe belongs to the parent.
+      synchronization_write_fd.reset();  // closes synchronization_pipe_fds[1]
+    }
+#endif  // defined(OS_MACOSX)
+
 #if 0
     // When debugging it can be helpful to check that we really aren't making
     // any hidden calls to malloc.
@@ -602,7 +650,7 @@ bool LaunchProcess(const std::vector<std::string>& argv,
         reinterpret_cast<void*>(reinterpret_cast<intptr_t>(malloc) & ~4095);
     mprotect(malloc_thunk, 4096, PROT_READ | PROT_WRITE | PROT_EXEC);
     memset(reinterpret_cast<void*>(malloc), 0xff, 8);
-#endif
+#endif  // 0
 
     // DANGER: no calls to malloc are allowed from now on:
     // http://crbug.com/36678
@@ -616,6 +664,16 @@ bool LaunchProcess(const std::vector<std::string>& argv,
       }
     }
 
+#if defined(OS_MACOSX)
+    if (options.synchronize) {
+      // Remap the read side of the synchronization pipe back onto itself,
+      // ensuring that it won't be closed by CloseSuperfluousFds.
+      int keep_fd = *synchronization_read_fd.get();
+      fd_shuffle1.push_back(InjectionArc(keep_fd, keep_fd, false));
+      fd_shuffle2.push_back(InjectionArc(keep_fd, keep_fd, false));
+    }
+#endif  // defined(OS_MACOSX)
+
     if (options.environ)
       SetEnvironment(new_environ.get());
 
@@ -625,10 +683,29 @@ bool LaunchProcess(const std::vector<std::string>& argv,
 
     CloseSuperfluousFds(fd_shuffle2);
 
+#if defined(OS_MACOSX)
+    if (options.synchronize) {
+      // Do a blocking read to wait until the parent says it's OK to proceed.
+      // The byte that's read here is written by LaunchSynchronize.
+      char read_char;
+      int read_result =
+          HANDLE_EINTR(read(*synchronization_read_fd.get(), &read_char, 1));
+      if (read_result != 1) {
+        RAW_LOG(ERROR, "LaunchProcess: synchronization read: error");
+        _exit(127);
+      }
+
+      // The pipe is no longer useful. Don't let it live on in the new process
+      // after exec.
+      synchronization_read_fd.reset();  // closes synchronization_pipe_fds[0]
+    }
+#endif  // defined(OS_MACOSX)
+
     for (size_t i = 0; i < argv.size(); i++)
       argv_cstr[i] = const_cast<char*>(argv[i].c_str());
     argv_cstr[argv.size()] = NULL;
     execvp(argv_cstr[0], argv_cstr.get());
+
     RAW_LOG(ERROR, "LaunchProcess: failed to execvp:");
     RAW_LOG(ERROR, argv_cstr[0]);
     _exit(127);
@@ -644,17 +721,39 @@ bool LaunchProcess(const std::vector<std::string>& argv,
 
     if (process_handle)
       *process_handle = pid;
+
+#if defined(OS_MACOSX)
+    if (options.synchronize) {
+      // The "read" side of the synchronization pipe belongs to the child.
+      synchronization_read_fd.reset();  // closes synchronization_pipe_fds[0]
+      *options.synchronize = new int(*synchronization_write_fd.release());
+    }
+#endif  // defined(OS_MACOSX)
   }
 
   return true;
 }
 
+
 bool LaunchProcess(const CommandLine& cmdline,
                    const LaunchOptions& options,
                    ProcessHandle* process_handle) {
   return LaunchProcess(cmdline.argv(), options, process_handle);
 }
 
+#if defined(OS_MACOSX)
+void LaunchSynchronize(LaunchSynchronizationHandle handle) {
+  int synchronization_fd = *handle;
+  file_util::ScopedFD synchronization_fd_closer(&synchronization_fd);
+  delete handle;
+
+  // Write a '\0' character to the pipe.
+  if (HANDLE_EINTR(write(synchronization_fd, "", 1)) != 1) {
+    PLOG(ERROR) << "write";
+  }
+}
+#endif  // defined(OS_MACOSX)
+
 ProcessMetrics::~ProcessMetrics() { }
 
 bool EnableInProcessStackDumping() {