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

2 files changed, 28 insertions, 2 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index d687acb..a2a5530 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -56,6 +56,7 @@
 // TODO(port): get rid of this include. It's used just to provide declarations
 // and stub definitions for classes we encouter during the porting effort.
 #include "chrome/common/temp_scaffolding_stubs.h"
+#include <signal.h>
 #endif
 
 // TODO(port): several win-only methods have been pulled out of this, but
@@ -180,6 +181,12 @@ void RunUIMessageLoop(BrowserProcess* browser_process) {
 #endif
 }
 
+#if defined(OS_POSIX)
+// See comment below, where sigaction is called.
+void SIGCHLDHandler(int signal) {
+}
+#endif
+
 }  // namespace
 
 // Main routine for running as the Browser process.
@@ -201,6 +208,14 @@ int BrowserMain(const MainFunctionParams& parameters) {
   tracked_objects::AutoTracking tracking_objects;
 #endif
 
+#if defined(OS_POSIX)
+  // We need to accept SIGCHLD, even though our handler is a no-op because
+  // otherwise we cannot wait on children. (According to POSIX 2001.)
+  struct sigaction action = {0};
+  action.sa_handler = SIGCHLDHandler;
+  CHECK(sigaction(SIGCHLD, &action, NULL) == 0);
+#endif
+
   // Do platform-specific things (such as finishing initializing Cocoa)
   // prior to instantiating the message loop. This could be turned into a
   // broadcast notification.
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index 0e25051..c7c01f8 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -668,13 +668,24 @@ void BrowserRenderProcessHost::OnChannelError() {
   DCHECK(process_.handle());
   DCHECK(channel_.get());
 
-  if (base::DidProcessCrash(process_.handle())) {
+  bool child_exited;
+  if (base::DidProcessCrash(&child_exited, process_.handle())) {
     NotificationService::current()->Notify(
         NotificationType::RENDERER_PROCESS_CRASHED,
         Source<RenderProcessHost>(this), NotificationService::NoDetails());
   }
 
-  process_.Close();
+  // POSIX: If the process crashed, then the kernel closed the socket for it
+  // and so the child has already died by the time we get here. Since
+  // DidProcessCrash called waitpid with WNOHANG, it'll reap the process.
+  // However, if DidProcessCrash didn't reap the child, we'll need to in
+  // ~BrowserRenderProcessHost via ProcessWatcher. So we can't close the handle
+  // here.
+  //
+  // This is moot on Windows where |child_exited| will always be true.
+  if (child_exited)
+    process_.Close();
+
   channel_.reset();
 
   // This process should detach all the listeners, causing the object to be