Merge 247572 "Linux: tear down Zygote at browser shutdown."

> Linux: tear down Zygote at browser shutdown.
> 
> In BrowserMainLoop::ShutdownThreadsAndCleanUp(), we explicitly call
> a new ZygoteHostImpl::TearDownAfterLastChild API to notify the Zygote
> to exit after its last child died.
> 
> The notification is to simply close the IPC channel to the Zygote.
> Before this patch, this would happen implicitly when the browser process
> died.
> 
> To implement this, we add a lightweight child tracking mechanism to ZygoteHostImpl.
> 
> BUG=334345
> R=piman@chromium.org
> 
> Review URL: https://codereview.chromium.org/148443006

TBR=jln@chromium.org

Review URL: https://codereview.chromium.org/166163002

git-svn-id: svn://svn.chromium.org/chrome/branches/1750/src@251263 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 101 insertions, 22 deletions

diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc
index 9b4a1ef..1930bc0 100644
--- a/content/browser/browser_main_loop.cc
+++ b/content/browser/browser_main_loop.cc
@@ -784,6 +784,10 @@ void BrowserMainLoop::ShutdownThreadsAndCleanUp() {
   }
 #endif
 
+#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
+  ZygoteHostImpl::GetInstance()->TearDownAfterLastChild();
+#endif  // defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
+
   // The device monitors are using |system_monitor_| as dependency, so delete
   // them before |system_monitor_| goes away.
   // On Mac and windows, the monitor needs to be destroyed on the same thread
diff --git a/content/browser/zygote_host/zygote_host_impl_linux.cc b/content/browser/zygote_host/zygote_host_impl_linux.cc
index 044dedc..18e9b69 100644
--- a/content/browser/zygote_host/zygote_host_impl_linux.cc
+++ b/content/browser/zygote_host/zygote_host_impl_linux.cc
@@ -52,16 +52,18 @@ ZygoteHost* ZygoteHost::GetInstance() {
 
 ZygoteHostImpl::ZygoteHostImpl()
     : control_fd_(-1),
+      control_lock_(),
       pid_(-1),
       init_(false),
       using_suid_sandbox_(false),
+      sandbox_binary_(),
       have_read_sandbox_status_word_(false),
-      sandbox_status_(0) {}
+      sandbox_status_(0),
+      child_tracking_lock_(),
+      list_of_running_zygote_children_(),
+      should_teardown_after_last_child_exits_(false) {}
 
-ZygoteHostImpl::~ZygoteHostImpl() {
-  if (init_)
-    close(control_fd_);
-}
+ZygoteHostImpl::~ZygoteHostImpl() { TearDown(); }
 
 // static
 ZygoteHostImpl* ZygoteHostImpl::GetInstance() {
@@ -217,8 +219,56 @@ void ZygoteHostImpl::Init(const std::string& sandbox_cmd) {
   // We don't wait for the reply. We'll read it in ReadReply.
 }
 
+void ZygoteHostImpl::TearDownAfterLastChild() {
+  bool do_teardown = false;
+  {
+    base::AutoLock lock(child_tracking_lock_);
+    should_teardown_after_last_child_exits_ = true;
+    do_teardown = list_of_running_zygote_children_.empty();
+  }
+  if (do_teardown) {
+    TearDown();
+  }
+}
+
+// Note: this is also called from the destructor.
+void ZygoteHostImpl::TearDown() {
+  base::AutoLock lock(control_lock_);
+  if (control_fd_ > -1) {
+    // Closing the IPC channel will act as a notification to exit
+    // to the Zygote.
+    if (IGNORE_EINTR(close(control_fd_))) {
+      PLOG(ERROR) << "Could not close Zygote control channel.";
+      NOTREACHED();
+    }
+    control_fd_ = -1;
+  }
+}
+
+void ZygoteHostImpl::ZygoteChildBorn(pid_t process) {
+  base::AutoLock lock(child_tracking_lock_);
+  bool new_element_inserted =
+      list_of_running_zygote_children_.insert(process).second;
+  DCHECK(new_element_inserted);
+}
+
+void ZygoteHostImpl::ZygoteChildDied(pid_t process) {
+  bool do_teardown = false;
+  {
+    base::AutoLock lock(child_tracking_lock_);
+    size_t num_erased = list_of_running_zygote_children_.erase(process);
+    DCHECK_EQ(1U, num_erased);
+    do_teardown = should_teardown_after_last_child_exits_ &&
+                  list_of_running_zygote_children_.empty();
+  }
+  if (do_teardown) {
+    TearDown();
+  }
+}
+
 bool ZygoteHostImpl::SendMessage(const Pickle& data,
                                  const std::vector<int>* fds) {
+  DCHECK_NE(-1, control_fd_);
   CHECK(data.size() <= kZygoteMaxMessageLength)
       << "Trying to send too-large message to zygote (sending " << data.size()
       << " bytes, max is " << kZygoteMaxMessageLength << ")";
@@ -233,6 +283,7 @@ bool ZygoteHostImpl::SendMessage(const Pickle& data,
 }
 
 ssize_t ZygoteHostImpl::ReadReply(void* buf, size_t buf_len) {
+  DCHECK_NE(-1, control_fd_);
   // At startup we send a kZygoteCommandGetSandboxStatus request to the zygote,
   // but don't wait for the reply. Thus, the first time that we read from the
   // zygote, we get the reply to that request.
@@ -336,6 +387,7 @@ pid_t ZygoteHostImpl::ForkRequest(
   AdjustRendererOOMScore(pid, kLowestRendererOomScore);
 #endif
 
+  ZygoteChildBorn(pid);
   return pid;
 }
 
@@ -415,6 +467,7 @@ void ZygoteHostImpl::EnsureProcessTerminated(pid_t process) {
   pickle.WriteInt(process);
   if (!SendMessage(pickle, NULL))
     LOG(ERROR) << "Failed to send Reap message to zygote";
+  ZygoteChildDied(process);
 }
 
 base::TerminationStatus ZygoteHostImpl::GetTerminationStatus(
@@ -427,10 +480,6 @@ base::TerminationStatus ZygoteHostImpl::GetTerminationStatus(
   pickle.WriteBool(known_dead);
   pickle.WriteInt(handle);
 
-  // Set this now to handle the early termination cases.
-  if (exit_code)
-    *exit_code = RESULT_CODE_NORMAL_EXIT;
-
   static const unsigned kMaxMessageLength = 128;
   char buf[kMaxMessageLength];
   ssize_t len;
@@ -441,26 +490,33 @@ base::TerminationStatus ZygoteHostImpl::GetTerminationStatus(
     len = ReadReply(buf, sizeof(buf));
   }
 
+  // Set this now to handle the error cases.
+  if (exit_code)
+    *exit_code = RESULT_CODE_NORMAL_EXIT;
+  int status = base::TERMINATION_STATUS_NORMAL_TERMINATION;
+
   if (len == -1) {
     LOG(WARNING) << "Error reading message from zygote: " << errno;
-    return base::TERMINATION_STATUS_NORMAL_TERMINATION;
   } else if (len == 0) {
     LOG(WARNING) << "Socket closed prematurely.";
-    return base::TERMINATION_STATUS_NORMAL_TERMINATION;
+  } else {
+    Pickle read_pickle(buf, len);
+    int tmp_status, tmp_exit_code;
+    PickleIterator iter(read_pickle);
+    if (!read_pickle.ReadInt(&iter, &tmp_status) ||
+        !read_pickle.ReadInt(&iter, &tmp_exit_code)) {
+      LOG(WARNING)
+          << "Error parsing GetTerminationStatus response from zygote.";
+    } else {
+      if (exit_code)
+        *exit_code = tmp_exit_code;
+      status = tmp_status;
+    }
   }
 
-  Pickle read_pickle(buf, len);
-  int status, tmp_exit_code;
-  PickleIterator iter(read_pickle);
-  if (!read_pickle.ReadInt(&iter, &status) ||
-      !read_pickle.ReadInt(&iter, &tmp_exit_code)) {
-    LOG(WARNING) << "Error parsing GetTerminationStatus response from zygote.";
-    return base::TERMINATION_STATUS_NORMAL_TERMINATION;
+  if (status != base::TERMINATION_STATUS_STILL_RUNNING) {
+    ZygoteChildDied(handle);
   }
-
-  if (exit_code)
-    *exit_code = tmp_exit_code;
-
   return static_cast<base::TerminationStatus>(status);
 }
 
diff --git a/content/browser/zygote_host/zygote_host_impl_linux.h b/content/browser/zygote_host/zygote_host_impl_linux.h
index 95591a4..08044fa 100644
--- a/content/browser/zygote_host/zygote_host_impl_linux.h
+++ b/content/browser/zygote_host/zygote_host_impl_linux.h
@@ -5,6 +5,7 @@
 #ifndef CONTENT_BROWSER_ZYGOTE_HOST_ZYGOTE_HOST_IMPL_LINUX_H_
 #define CONTENT_BROWSER_ZYGOTE_HOST_ZYGOTE_HOST_IMPL_LINUX_H_
 
+#include <set>
 #include <string>
 #include <vector>
 
@@ -26,6 +27,9 @@ class CONTENT_EXPORT ZygoteHostImpl : public ZygoteHost {
 
   void Init(const std::string& sandbox_cmd);
 
+  // After the last known Zygote child exits, notify the Zygote to exit.
+  void TearDownAfterLastChild();
+
   // Tries to start a process of type indicated by process_type.
   // Returns its pid on success, otherwise
   // base::kNullProcessHandle;
@@ -62,6 +66,16 @@ class CONTENT_EXPORT ZygoteHostImpl : public ZygoteHost {
   ZygoteHostImpl();
   virtual ~ZygoteHostImpl();
 
+  // Notify the Zygote to exit immediately. This object should not be
+  // used afterwards.
+  void TearDown();
+
+  // Should be called every time a Zygote child is born.
+  void ZygoteChildBorn(pid_t process);
+
+  // Should be called every time a Zygote child died.
+  void ZygoteChildDied(pid_t process);
+
   // Sends |data| to the zygote via |control_fd_|.  If |fds| is non-NULL, the
   // included file descriptors will also be passed.  The caller is responsible
   // for acquiring |control_lock_|.
@@ -80,6 +94,11 @@ class CONTENT_EXPORT ZygoteHostImpl : public ZygoteHost {
   std::string sandbox_binary_;
   bool have_read_sandbox_status_word_;
   int sandbox_status_;
+  // A lock protecting list_of_running_zygote_children_ and
+  // should_teardown_after_last_child_exits_.
+  base::Lock child_tracking_lock_;
+  std::set<pid_t> list_of_running_zygote_children_;
+  bool should_teardown_after_last_child_exits_;
 };
 
 }  // namespace content