Ensure we don't have zombie child processes that get started but never get a chance to connect to t…

content/common/child_thread.cc

Index: content/common/child_thread.cc
===================================================================
--- content/common/child_thread.cc	(revision 150143)
+++ content/common/child_thread.cc	(working copy)
@@ -8,6 +8,7 @@
 #include "base/command_line.h"
 #include "base/message_loop.h"
 #include "base/process.h"
+#include "base/process_util.h"
 #include "base/string_util.h"
 #include "base/tracked_objects.h"
 #include "content/common/child_histogram_message_filter.h"
@@ -34,8 +35,17 @@
 
 namespace {
 
+// How long to wait for a connection to the browser process before giving up.
+const int kConnectionTimeoutS = 15;
+
+// This isn't needed on Windows because there the sandbox's job objects
+// terminates child processes automatically. For unsanboxed processes (i.e.

[Avi (use Gerrit), 2012/08/07 04:50:51]

s/terminates/terminate/ to match quantity.
s/unsanboxed/unsandboxed/ typo fix.

[jam, 2012/08/07 04:54:28]

updated objects to object since there's only one
Done

+// plugins), PluginThread has EnsureTerminateMessageFilter.
 #if defined(OS_POSIX)
 
+// How long to wait after the browser process goes away to kill ourselves.
+const int kSuicideTimeoutS = 5;
+
 class SuicideOnChannelErrorFilter : public IPC::ChannelProxy::MessageFilter {
  public:
   // IPC::ChannelProxy::MessageFilter
@@ -59,29 +69,40 @@
     // We want to kill this process after giving it 30 seconds to run the exit
     // handlers. SIGALRM has a default disposition of terminating the
     // application.
-    LOG(INFO) << "SuicideOnChannelErrorFilter::OnChannelError";
-    if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kChildCleanExit))
+    if (CommandLine::ForCurrentProcess()->HasSwitch(
+        switches::kChildCleanExit)) {
       alarm(30);
-    else
-      _exit(0);
+    } else {
+      MessageLoop::current()->PostDelayedTask(
+          FROM_HERE,
+          base::Bind(&SuicideOnChannelErrorFilter::Exit, this),
+          base::TimeDelta::FromSeconds(kSuicideTimeoutS));

[Avi (use Gerrit), 2012/08/07 04:50:51]

This change from immediate exit to delayed suicide is not explained.

[jam, 2012/08/07 04:54:28]

On second thoughts, I'll remove that from this change.

+    }
   }
 
  protected:
   virtual ~SuicideOnChannelErrorFilter() {}
+
+ private:
+  void Exit() {
+    _exit(0);
+  }
 };
 
 #endif  // OS(POSIX)
 
 }  // namespace
 
-ChildThread::ChildThread() {
+ChildThread::ChildThread()
+    : ALLOW_THIS_IN_INITIALIZER_LIST(channel_connected_factory_(this)) {
   channel_name_ = CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
       switches::kProcessChannelID);
   Init();
 }
 
 ChildThread::ChildThread(const std::string& channel_name)
-    : channel_name_(channel_name) {
+    : channel_name_(channel_name),
+      ALLOW_THIS_IN_INITIALIZER_LIST(channel_connected_factory_(this)) {
   Init();
 }
 
@@ -108,7 +129,6 @@
   channel_->AddFilter(histogram_message_filter_.get());
   channel_->AddFilter(sync_message_filter_.get());
   channel_->AddFilter(new ChildTraceMessageFilter());
-  LOG(INFO) << "ChildThread::Init";
 
 #if defined(OS_POSIX)
   // Check that --process-type is specified so we don't do this in unit tests
@@ -116,6 +136,12 @@
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kProcessType))
     channel_->AddFilter(new SuicideOnChannelErrorFilter());
 #endif
+
+  MessageLoop::current()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&ChildThread::EnsureConnected,
+                 channel_connected_factory_.GetWeakPtr()),
+      base::TimeDelta::FromSeconds(kConnectionTimeoutS));
 }
 
 ChildThread::~ChildThread() {
@@ -138,11 +164,10 @@
 }
 
 void ChildThread::OnChannelConnected(int32 peer_pid) {
-  LOG(INFO) << "ChildThread::OnChannelConnected";
+  channel_connected_factory_.InvalidateWeakPtrs();
 }
 
 void ChildThread::OnChannelError() {
-  LOG(INFO) << "ChildThread::OnChannelError";
   set_on_channel_error_called(true);
   MessageLoop::current()->Quit();
 }
@@ -339,3 +364,8 @@
   // inflight that would addref it.
   Send(new ChildProcessHostMsg_ShutdownRequest);
 }
+
+void ChildThread::EnsureConnected() {
+  LOG(INFO) << "ChildThread::EnsureConnected()";
+  base::KillProcess(base::GetCurrentProcessHandle(), 0, false);
+}