chrome: respect --child-clean-exit.

content/zygote/zygote_linux.cc

Index: content/zygote/zygote_linux.cc
diff --git a/content/zygote/zygote_linux.cc b/content/zygote/zygote_linux.cc
index 893002281218e863a33bfeca97cab5f949d23efe..b8cec023c64bb9cf265ee13b7fb41490da85653e 100644
--- a/content/zygote/zygote_linux.cc
+++ b/content/zygote/zygote_linux.cc
@@ -5,6 +5,7 @@
 #include "content/zygote/zygote_linux.h"
 
 #include <fcntl.h>
+#include <pthread.h>
 #include <string.h>
 #include <sys/socket.h>
 #include <sys/types.h>
@@ -24,6 +25,7 @@
 #include "content/common/set_process_title.h"
 #include "content/common/zygote_commands_linux.h"
 #include "content/public/common/content_descriptors.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/result_codes.h"
 #include "content/public/common/sandbox_linux.h"
 #include "content/public/common/zygote_fork_delegate_linux.h"
@@ -110,7 +112,12 @@ bool Zygote::HandleRequestFromBrowser(int fd) {
 
   if (len == 0 || (len == -1 && errno == ECONNRESET)) {
     // EOF from the browser. We should die.
-    _exit(0);
+    if (CommandLine::ForCurrentProcess()->
+        HasSwitch(switches::kChildCleanExit))
+      // Wait for all children to exit first.

[Markus (顧孟勤), 2013/07/18 23:26:47]

This comment is super confusing. This might very well be correct code -- I don't
understand things well enough to judge that.

But the comment doesn't match the code. pthread_exec() is essentially the same
as syscall(__NR_exit). And _exit() is essentially the same as
syscall(__NR_exit_group).

The difference is that the former terminates the current thread only. The latter
terminates all threads in the current process.

Neither one of them has any impact on any child processes. The difference is
strictly limited to threads.

So, a comment talking about children has me even more confused than no comment
at all.

[asharif1, 2013/07/19 03:01:32]

I will try to clarify this comment in the next patchset (another 14 hours or
so).

But for an explanation, here it is:

The old code calls _exit(), which exits the zygote process immediately (without
waiting for renderer processes to exit).

The goal of my CL is to have the zygote wait on all the renderer processes to
exit, and have the browser wait on the zygote processes to exit. In this way, I
will ensure that the browser process is the last one to exit with
--child-clean-exit. Currently such a situation does not happen, and it causes
problems on ChromeOS because the ChromeOS session_manager sends a SIGKILL to
-browser_process_pid as soon as it terminates. See this thread for details:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-os-dev/mGxMV8...

Now that the goal is understood, let's see what this code does.

When the browser process wants to exit (user clicks close or sends SIGINT to the
browser process), it closes the channel to the renderer process and asks the
zygote to reap the renderer. The zygote calls EnsureProcessGetsReaped() around
like 185. If you look at the implementation of this, it actually reaps the
renderer in a separate, non-joinable thread.

The browser also closes the channel to the zygote so this code waits until the
renderer(s) are reaped on the background thread.

+      pthread_exit(NULL);

[jln (very slow on Chromium), 2013/07/18 23:26:05]

What are you trying to do here ?

You're exiting the current thread and definitely not waiting for children.

This is the same as _exit() since this process is monothreaded.

[asharif1, 2013/07/19 03:01:32]

Although this process seems single-threaded, close to the browser exiting, it is
not. This is because we get a reap request to reap the renderer close to the EOF
from the browser. The reap request is handled using a detatched thread (see
kill_posix.cc for details) which calls waitpid(). Specifically look at
backgroundreaper.

The goal of my code is to ensure that reaper actually reaps the renderer before
the zygote quits. So we must not call _exit() here as that will exit
immediately.

Please also read my other comment above. I will update the comment tomorrow.

+    else
+      _exit(0);
     return false;
   }
 
@@ -175,7 +182,10 @@ void Zygote::HandleReapRequest(int fd,
     actual_child = child;
   }
 
-  base::EnsureProcessTerminated(actual_child);
+  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kChildCleanExit))
+    base::EnsureProcessGetsReaped(actual_child);
+  else
+    base::EnsureProcessTerminated(actual_child);
 }
 
 void Zygote::HandleGetTerminationStatus(int fd,