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,6 +35,12 @@
 
 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 object
+// terminates child processes automatically. For unsandboxed processes (i.e.
+// plugins), PluginThread has EnsureTerminateMessageFilter.

[Avi (use Gerrit), 2012/08/07 04:57:48]

EnsureTerminateMessageFilter? So we've seen this before? :) Every time we fix an
issue with plugins we should check to see if the misbehavedness of them extends
to a lesser extent to other things.

[jam, 2012/08/07 05:07:46]

What happened is plugins got fixed for all platforms initially (since they're
not sandboxed and so the sandbox' job object doesn't quit them). workers/utility
processes quit automatically on windows because of the job object. but on linux,
that isn't the case, so this filter got added only for renderers. i saw this
issue for workers as well, so i made it happen for all child processes on linux
in another change last week.

 #if defined(OS_POSIX)
 
 class SuicideOnChannelErrorFilter : public IPC::ChannelProxy::MessageFilter {
@@ -59,7 +66,6 @@
     // 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))
       alarm(30);
     else
@@ -74,14 +80,16 @@
 
 }  // 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 +116,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 +123,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 +151,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 +351,8 @@
   // inflight that would addref it.
   Send(new ChildProcessHostMsg_ShutdownRequest);
 }
+
+void ChildThread::EnsureConnected() {
+  LOG(INFO) << "ChildThread::EnsureConnected()";
+  base::KillProcess(base::GetCurrentProcessHandle(), 0, false);
+}