Ensure we don't have zombie child processes that get started but never get a...

Ensure we don't have zombie child processes that get started but never get a chance to connect to the browser process. Since they never connected, they will never have an OnChannelError and so they don't get destroyed.

This was causing the sharding_supervisor script to hang on Linux since there were child processes that didn't exit. It looks like there are race conditions in some tests where child processes start but the browser closes before they can connect. Probably we only see this on Linux because it's faster than other platforms or the shutdown timing is different.

BUG=140054
Review URL: https://chromiumcodereview.appspot.com/10855013

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

content/common/child_thread.cc

@@ -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 @@ using tracked_objects::ThreadData;
 
 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.
 #if defined(OS_POSIX)
 
 class SuicideOnChannelErrorFilter : public IPC::ChannelProxy::MessageFilter {
@@ -59,7 +66,6 @@ class SuicideOnChannelErrorFilter : public IPC::ChannelProxy::MessageFilter {
     // 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 @@ class SuicideOnChannelErrorFilter : public IPC::ChannelProxy::MessageFilter {
 
 }  // 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 @@ void ChildThread::Init() {
   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 @@ void ChildThread::Init() {
   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 @@ ChildThread::~ChildThread() {
 }
 
 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 @@ void ChildThread::OnProcessFinalRelease() {
   // inflight that would addref it.
   Send(new ChildProcessHostMsg_ShutdownRequest);
 }
+
+void ChildThread::EnsureConnected() {
+  LOG(INFO) << "ChildThread::EnsureConnected()";
+  base::KillProcess(base::GetCurrentProcessHandle(), 0, false);
+}

content/common/child_thread.h

@@ -7,6 +7,7 @@
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/shared_memory.h"
 #include "base/tracked_objects.h"
 #include "content/common/content_export.h"
@@ -100,16 +101,6 @@ class CONTENT_EXPORT ChildThread : public IPC::Listener, public IPC::Sender {
   void OnProcessFinalRelease();
 
   virtual bool OnControlMessageReceived(const IPC::Message& msg);
-  virtual void OnShutdown();
-
-#ifdef IPC_MESSAGE_LOG_ENABLED
-  virtual void OnSetIPCLoggingEnabled(bool enable);
-#endif
-
-  virtual void OnSetProfilerStatus(tracked_objects::ThreadData::Status status);
-  virtual void OnGetChildProfilerData(int sequence_number);
-
-  virtual void OnDumpHandles();
 
   void set_on_channel_error_called(bool on_channel_error_called) {
     on_channel_error_called_ = on_channel_error_called;
@@ -123,10 +114,20 @@ class CONTENT_EXPORT ChildThread : public IPC::Listener, public IPC::Sender {
  private:
   void Init();
 
+  // IPC message handlers.
+  void OnShutdown();
+  void OnSetProfilerStatus(tracked_objects::ThreadData::Status status);
+  void OnGetChildProfilerData(int sequence_number);
+  void OnDumpHandles();
+#ifdef IPC_MESSAGE_LOG_ENABLED
+  void OnSetIPCLoggingEnabled(bool enable);
+#endif
 #if defined(USE_TCMALLOC)
   void OnGetTcmallocStats();
 #endif
 
+  void EnsureConnected();
+
   std::string channel_name_;
   scoped_ptr<IPC::SyncChannel> channel_;
 
@@ -154,6 +155,8 @@ class CONTENT_EXPORT ChildThread : public IPC::Listener, public IPC::Sender {
 
   scoped_refptr<content::ChildHistogramMessageFilter> histogram_message_filter_;
 
+  base::WeakPtrFactory<ChildThread> channel_connected_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(ChildThread);
 };