POSIX: don't spawn zombies.

http://codereview.chromium.org/93147

BUG=9401


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

4 files changed, 38 insertions, 16 deletions

diff --git a/base/process_posix.cc b/base/process_posix.cc
index ff8cf92..c9a7dfc 100644
--- a/base/process_posix.cc
+++ b/base/process_posix.cc
@@ -18,8 +18,9 @@ void Process::Terminate(int result_code) {
   // result_code isn't supportable.
   if (!process_)
     return;
-  // Wait so we clean up the zombie
-  KillProcess(process_, result_code, true);
+  // We don't wait here. It's the responsibility of other code to reap the
+  // child.
+  KillProcess(process_, result_code, false);
 }
 
 bool Process::IsProcessBackgrounded() const {
diff --git a/base/process_util.h b/base/process_util.h
index 9eb8690..a692870 100644
--- a/base/process_util.h
+++ b/base/process_util.h
@@ -172,9 +172,13 @@ bool KillProcessById(ProcessId process_id, int exit_code, bool wait);
 #endif
 
 // Get the termination status (exit code) of the process and return true if the
-// status indicates the process crashed.  It is an error to call this if the
-// process hasn't terminated yet.
-bool DidProcessCrash(ProcessHandle handle);
+// status indicates the process crashed. |child_exited| is set to true iff the
+// child process has terminated. (|child_exited| may be NULL.)
+//
+// On Windows, it is an error to call this if the process hasn't terminated
+// yet. On POSIX, |child_exited| is set correctly since we detect terminate in
+// a different manner on POSIX.
+bool DidProcessCrash(bool* child_exited, ProcessHandle handle);
 
 // Waits for process to exit. In POSIX systems, if the process hasn't been
 // signaled then puts the exit code in |exit_code|; otherwise it's considered
diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc
index 2e3fd55..efe7442 100644
--- a/base/process_util_posix.cc
+++ b/base/process_util_posix.cc
@@ -56,23 +56,25 @@ ProcessId GetProcId(ProcessHandle process) {
 // entry structure.  Ignores specified exit_code; posix can't force that.
 // Returns true if this is successful, false otherwise.
 bool KillProcess(ProcessHandle process_id, int exit_code, bool wait) {
-  bool result = false;
+  bool result = kill(process_id, SIGTERM) == 0;
 
-  int status = kill(process_id, SIGTERM);
-  if (!status && wait) {
+  if (result && wait) {
     int tries = 60;
     // The process may not end immediately due to pending I/O
     while (tries-- > 0) {
-      int pid = waitpid(process_id, &status, WNOHANG);
-      if (pid == process_id) {
-        result = true;
+      int pid = waitpid(process_id, NULL, WNOHANG);
+      if (pid == process_id)
         break;
-      }
+
       sleep(1);
     }
+
+    result = kill(process_id, SIGKILL) == 0;
   }
+
   if (!result)
     DLOG(ERROR) << "Unable to terminate process.";
+
   return result;
 }
 
@@ -141,13 +143,24 @@ void RaiseProcessToHighPriority() {
   // setpriority() or sched_getscheduler, but these all require extra rights.
 }
 
-bool DidProcessCrash(ProcessHandle handle) {
+bool DidProcessCrash(bool* child_exited, ProcessHandle handle) {
   int status;
-  if (waitpid(handle, &status, WNOHANG)) {
-    // I feel like dancing!
+  const int result = waitpid(handle, &status, WNOHANG);
+  if (result == -1) {
+    LOG(ERROR) << "waitpid failed pid:" << handle << " errno:" << errno;
+    if (child_exited)
+      *child_exited = false;
+    return false;
+  } else if (result == 0) {
+    // the child hasn't exited yet.
+    if (child_exited)
+      *child_exited = false;
     return false;
   }
 
+  if (child_exited)
+    *child_exited = true;
+
   if (WIFSIGNALED(status)) {
     switch(WTERMSIG(status)) {
       case SIGSEGV:
diff --git a/base/process_util_win.cc b/base/process_util_win.cc
index 13fec9a..fc05664 100644
--- a/base/process_util_win.cc
+++ b/base/process_util_win.cc
@@ -270,8 +270,12 @@ bool KillProcess(ProcessHandle process, int exit_code, bool wait) {
   return result;
 }
 
-bool DidProcessCrash(ProcessHandle handle) {
+bool DidProcessCrash(bool* child_exited, ProcessHandle handle) {
   DWORD exitcode = 0;
+
+  if (child_exited)
+    *child_exited = true;  // On Windows it an error to call this function if
+                           // the child hasn't already exited.
   if (!::GetExitCodeProcess(handle, &exitcode)) {
     NOTREACHED();
     return false;