Have each SandboxedProcessLauncherDelegate maintain a zygote.

content/browser/child_process_launcher.cc

 // Copyright 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 "content/browser/child_process_launcher.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 21 matching lines @@
 #include "content/browser/bootstrap_sandbox_manager_mac.h"
 #include "content/browser/mach_broker_mac.h"
 #include "sandbox/mac/bootstrap_sandbox.h"
 #include "sandbox/mac/pre_exec_delegate.h"
 #elif defined(OS_ANDROID)
 #include "base/android/jni_android.h"
 #include "content/browser/android/child_process_launcher_android.h"
 #elif defined(OS_POSIX)
 #include "base/memory/singleton.h"
 #include "content/browser/renderer_host/render_sandbox_host_linux.h"
+#include "content/browser/zygote_host/zygote_communication_linux.h"
 #include "content/browser/zygote_host/zygote_host_impl_linux.h"
 #include "content/common/child_process_sandbox_support_impl_linux.h"
 #endif
 
 #if defined(OS_POSIX)
 #include "base/posix/global_descriptors.h"
 #include "content/browser/file_descriptor_info_impl.h"
 #include "gin/v8_initializer.h"
 #endif
 
 namespace content {
 
 namespace {
 
-typedef base::Callback<void(bool,
+typedef base::Callback<void(ZygoteHandle,
 #if defined(OS_ANDROID)
                             base::ScopedFD,
 #endif
                             base::Process)> NotifyCallback;
 
 void RecordHistogramsOnLauncherThread(base::TimeDelta launch_time) {
   DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
   // Log the launch time, separating out the first one (which will likely be
   // slower due to the rest of the browser initializing at the same time).
   static bool done_first_launch = false;
@@ skipping 13 matching lines @@
                                   const base::TimeTicks begin_launch_time,
                                   base::ScopedFD ipcfd,
                                   base::ProcessHandle handle) {
   // This can be called on the launcher thread or UI thread.
   base::TimeDelta launch_time = base::TimeTicks::Now() - begin_launch_time;
   BrowserThread::PostTask(
       BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
       base::Bind(&RecordHistogramsOnLauncherThread, launch_time));
 
   base::Closure callback_on_client_thread(
-      base::Bind(callback, false, base::Passed(&ipcfd),
+      base::Bind(callback, nullptr, base::Passed(&ipcfd),
                  base::Passed(base::Process(handle))));
   if (BrowserThread::CurrentlyOn(client_thread_id)) {
     callback_on_client_thread.Run();
   } else {
     BrowserThread::PostTask(
         client_thread_id, FROM_HERE, callback_on_client_thread);
   }
 }
 #endif
 
 void LaunchOnLauncherThread(const NotifyCallback& callback,
                             BrowserThread::ID client_thread_id,
                             int child_process_id,
                             SandboxedProcessLauncherDelegate* delegate,
 #if defined(OS_ANDROID)
                             base::ScopedFD ipcfd,
 #endif
                             base::CommandLine* cmd_line) {
   DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
   scoped_ptr<SandboxedProcessLauncherDelegate> delegate_deleter(delegate);
+#if !defined(OS_ANDROID)
+  ZygoteHandle zygote = nullptr;
+#endif
 #if defined(OS_WIN)
-  bool use_zygote = false;
   bool launch_elevated = delegate->ShouldLaunchElevated();
 #elif defined(OS_MACOSX)
-  bool use_zygote = false;
   base::EnvironmentMap env = delegate->GetEnvironment();
   base::ScopedFD ipcfd = delegate->TakeIpcFd();
 #elif defined(OS_POSIX) && !defined(OS_ANDROID)
-  bool use_zygote = delegate->ShouldUseZygote();
   base::EnvironmentMap env = delegate->GetEnvironment();
   base::ScopedFD ipcfd = delegate->TakeIpcFd();
 #endif
   scoped_ptr<base::CommandLine> cmd_line_deleter(cmd_line);
   base::TimeTicks begin_launch_time = base::TimeTicks::Now();
 
   base::Process process;
 #if defined(OS_WIN)
   if (launch_elevated) {
     base::LaunchOptions options;
@@ skipping 62 matching lines @@
   StartChildProcess(
       cmd_line->argv(), child_process_id, std::move(files_to_register), regions,
       base::Bind(&OnChildProcessStartedAndroid, callback, client_thread_id,
                  begin_launch_time, base::Passed(&ipcfd)));
 
 #elif defined(OS_POSIX)
   // We need to close the client end of the IPC channel to reliably detect
   // child termination.
 
 #if !defined(OS_MACOSX)
-  if (use_zygote) {
-    base::ProcessHandle handle = ZygoteHostImpl::GetInstance()->ForkRequest(
+  ZygoteHandle* zygote_handle = delegate->GetZygote();
+  // If |zygote_handle| is null, a zygote should not be used.
+  if (zygote_handle) {
+    // This code runs on the PROCESS_LAUNCHER thread so race conditions are not
+    // an issue with the lazy initialization.
+    if (*zygote_handle == nullptr) {

[Mark Seaborn, 2016/01/19 21:06:43]

So there is still this code for launching the zygote process on demand, then? 
Is that still used (e.g. for plugins)?

Does this leave a small race condition?  chrome/browser/chrome_browser_main.cc
uses PostTask() to run nacl::NaClProcessHost::EarlyZygoteLaunch().  If that task
hasn't run by the time we try to use the zygote, then one zygote would get
launched here, and then a second zygote would get launched by
EarlyZygoteLaunch() (or the DCHECK in EarlyZygoteLaunch() would fail).

Maybe EarlyZygoteLaunch() should check whether the zygote has been launched
already?

Or maybe this would never be an issue in practice because anything that launches
a NaCl process would occur from a task run by PostTask() that is queued later
than EarlyZygoteLaunch()?

[mdempsky, 2016/01/19 21:26:43]

It's not currently used, but we'll start using it once/if we switch to lazily
starting zygotes.
My understanding is it shouldn't be racy: EarlyZygoteLaunch and
LaunchOnLauncherThread are both posted to the PROCESS_LAUNCHER thread, which
should ensure they're ordered.


kerrnel: This does make me realize we should use CreateZygote() here too, and
that CreateZygote() should include
DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);

[Greg K, 2016/01/19 22:20:16]

I talked to Matt, except for NaCl, the zygotes are currently launched in the
main thread initialization, which should be complete before the PROCESS_LAUNCHER
thread is receiving any requests to create rendererers, etc.

+      *zygote_handle = new ZygoteCommunication();
+      (*zygote_handle)->Init();
+    }
+    zygote = *zygote_handle;
+    base::ProcessHandle handle = zygote->ForkRequest(
         cmd_line->argv(), std::move(files_to_register), process_type);
     process = base::Process(handle);
   } else
   // Fall through to the normal posix case below when we're not zygoting.
 #endif  // !defined(OS_MACOSX)
   {
     // Convert FD mapping to FileHandleMappingVector
     base::FileHandleMappingVector fds_to_map =
         files_to_register->GetMappingWithIDAdjustment(
             base::GlobalDescriptors::kBaseDescriptor);
@@ skipping 58 matching lines @@
     broker->GetLock().Release();
 #endif  // defined(OS_MACOSX)
   }
 #endif  // else defined(OS_POSIX)
 #if !defined(OS_ANDROID)
   if (process.IsValid()) {
     RecordHistogramsOnLauncherThread(base::TimeTicks::Now() -
                                      begin_launch_time);
   }
   BrowserThread::PostTask(client_thread_id, FROM_HERE,
-                          base::Bind(callback,
-                                     use_zygote,
-                                     base::Passed(&process)));
+                          base::Bind(callback, zygote, base::Passed(&process)));
 #endif  // !defined(OS_ANDROID)
 }
 
-void TerminateOnLauncherThread(bool zygote, base::Process process) {
+void TerminateOnLauncherThread(ZygoteHandle zygote, base::Process process) {
   DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
 #if defined(OS_ANDROID)
   VLOG(1) << "ChromeProcess: Stopping process with handle "
           << process.Handle();
   StopChildProcess(process.Handle());
 #else
   // Client has gone away, so just kill the process.  Using exit code 0
   // means that UMA won't treat this as a crash.
   process.Terminate(RESULT_CODE_NORMAL_EXIT, false);
   // On POSIX, we must additionally reap the child.
 #if defined(OS_POSIX)
 #if !defined(OS_MACOSX)
   if (zygote) {
     // If the renderer was created via a zygote, we have to proxy the reaping
     // through the zygote process.
-    ZygoteHostImpl::GetInstance()->EnsureProcessTerminated(process.Handle());
+    zygote->EnsureProcessTerminated(process.Handle());
   } else
 #endif  // !OS_MACOSX
     base::EnsureProcessTerminated(std::move(process));
 #endif  // OS_POSIX
 #endif  // defined(OS_ANDROID)
 }
 
 void SetProcessBackgroundedOnLauncherThread(base::Process process,
                                             bool background) {
   DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
   if (process.CanBackgroundProcesses()) {
     process.SetProcessBackgrounded(background);
   }
 #if defined(OS_ANDROID)
   SetChildProcessInForeground(process.Handle(), !background);
 #endif
 }
 
 }  // namespace
 
 ChildProcessLauncher::ChildProcessLauncher(
     SandboxedProcessLauncherDelegate* delegate,
     base::CommandLine* cmd_line,
     int child_process_id,
     Client* client,
     bool terminate_on_shutdown)
     : client_(client),
       termination_status_(base::TERMINATION_STATUS_NORMAL_TERMINATION),
       exit_code_(RESULT_CODE_NORMAL_EXIT),
-      zygote_(false),
+      zygote_(nullptr),
       starting_(true),
 #if defined(ADDRESS_SANITIZER) || defined(LEAK_SANITIZER) ||  \
     defined(MEMORY_SANITIZER) || defined(THREAD_SANITIZER) || \
     defined(UNDEFINED_SANITIZER)
       terminate_child_on_shutdown_(false),
 #else
       terminate_child_on_shutdown_(terminate_on_shutdown),
 #endif
       weak_factory_(this) {
   DCHECK(CalledOnValidThread());
@@ skipping 49 matching lines @@
 #if defined(OS_ANDROID)
                  base::Passed(&ipcfd),
 #endif
                  cmd_line));
 }
 
 void ChildProcessLauncher::UpdateTerminationStatus(bool known_dead) {
   DCHECK(CalledOnValidThread());
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
   if (zygote_) {
-    termination_status_ = ZygoteHostImpl::GetInstance()->
-        GetTerminationStatus(process_.Handle(), known_dead, &exit_code_);
+    termination_status_ = zygote_->GetTerminationStatus(
+        process_.Handle(), known_dead, &exit_code_);
   } else if (known_dead) {
     termination_status_ =
         base::GetKnownDeadTerminationStatus(process_.Handle(), &exit_code_);
   } else {
 #elif defined(OS_MACOSX)
   if (known_dead) {
     termination_status_ =
         base::GetKnownDeadTerminationStatus(process_.Handle(), &exit_code_);
   } else {
 #elif defined(OS_ANDROID)
@@ skipping 12 matching lines @@
   DCHECK(CalledOnValidThread());
   base::Process to_pass = process_.Duplicate();
   BrowserThread::PostTask(BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
                           base::Bind(&SetProcessBackgroundedOnLauncherThread,
                                      base::Passed(&to_pass), background));
 }
 
 void ChildProcessLauncher::DidLaunch(
     base::WeakPtr<ChildProcessLauncher> instance,
     bool terminate_on_shutdown,
-    bool zygote,
+    ZygoteHandle zygote,
 #if defined(OS_ANDROID)
     base::ScopedFD ipcfd,
 #endif
     base::Process process) {
   if (!process.IsValid())
     LOG(ERROR) << "Failed to launch child process";
 
   if (instance.get()) {
     instance->Notify(zygote,
 #if defined(OS_ANDROID)
                      std::move(ipcfd),
 #endif
                      std::move(process));
   } else {
     if (process.IsValid() && terminate_on_shutdown) {
       // On Posix, EnsureProcessTerminated can lead to 2 seconds of sleep!  So
       // don't this on the UI/IO threads.
       BrowserThread::PostTask(BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
                               base::Bind(&TerminateOnLauncherThread, zygote,
                                          base::Passed(&process)));
     }
   }
 }
 
-void ChildProcessLauncher::Notify(
-    bool zygote,
+void ChildProcessLauncher::Notify(ZygoteHandle zygote,
 #if defined(OS_ANDROID)
-    base::ScopedFD ipcfd,
+                                  base::ScopedFD ipcfd,
 #endif
-    base::Process process) {
+                                  base::Process process) {
   DCHECK(CalledOnValidThread());
   starting_ = false;
   process_ = std::move(process);
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
   zygote_ = zygote;
 #endif
   if (process_.IsValid()) {
     client_->OnProcessLaunched();
   } else {
@@ skipping 42 matching lines @@
 }
 
 ChildProcessLauncher::Client* ChildProcessLauncher::ReplaceClientForTest(
     Client* client) {
   Client* ret = client_;
   client_ = client;
   return ret;
 }
 
 }  // namespace content