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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/common/child_thread.h"
 
 #include "base/allocator/allocator_extension.h"
 #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"
 #include "content/common/child_process.h"
 #include "content/common/child_process_messages.h"
 #include "content/common/child_trace_message_filter.h"
 #include "content/common/fileapi/file_system_dispatcher.h"
 #include "content/common/quota_dispatcher.h"
 #include "content/common/resource_dispatcher.h"
 #include "content/common/socket_stream_dispatcher.h"
 #include "content/public/common/content_switches.h"
 #include "ipc/ipc_logging.h"
 #include "ipc/ipc_switches.h"
 #include "ipc/ipc_sync_channel.h"
 #include "ipc/ipc_sync_message_filter.h"
 #include "webkit/glue/webkit_glue.h"
 
 #if defined(OS_WIN)
 #include "content/common/handle_enumerator_win.h"
 #endif
 
 using content::ResourceDispatcher;
 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 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
   virtual void OnChannelError() OVERRIDE {
     // For renderer/worker processes:
     // On POSIX, at least, one can install an unload handler which loops
     // forever and leave behind a renderer process which eats 100% CPU forever.
     //
     // This is because the terminate signals (ViewMsg_ShouldClose and the error
     // from the IPC channel) are routed to the main message loop but never
     // processed (because that message loop is stuck in V8).
     //
     // One could make the browser SIGKILL the renderers, but that leaves open a
     // large window where a browser failure (or a user, manually terminating
     // the browser because "it's stuck") will leave behind a process eating all
     // the CPU.
     //
     // So, we install a filter on the channel so that we can process this event
     // here and kill the process.
     //
     // 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();
 }
 
 void ChildThread::Init() {
   on_channel_error_called_ = false;
   message_loop_ = MessageLoop::current();
   channel_.reset(new IPC::SyncChannel(channel_name_,
       IPC::Channel::MODE_CLIENT, this,
       ChildProcess::current()->io_message_loop_proxy(), true,
       ChildProcess::current()->GetShutDownEvent()));
 #ifdef IPC_MESSAGE_LOG_ENABLED
   IPC::Logging::GetInstance()->SetIPCSender(this);
 #endif
 
   resource_dispatcher_.reset(new ResourceDispatcher(this));
   socket_stream_dispatcher_.reset(new SocketStreamDispatcher());
   file_system_dispatcher_.reset(new FileSystemDispatcher());
   quota_dispatcher_.reset(new QuotaDispatcher());
 
   sync_message_filter_ =
       new IPC::SyncMessageFilter(ChildProcess::current()->GetShutDownEvent());
   histogram_message_filter_ = new content::ChildHistogramMessageFilter();
 
   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
   // and single-process mode.
   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() {
 #ifdef IPC_MESSAGE_LOG_ENABLED
   IPC::Logging::GetInstance()->SetIPCSender(NULL);
 #endif
 
   channel_->RemoveFilter(histogram_message_filter_.get());
   channel_->RemoveFilter(sync_message_filter_.get());
 
   // The ChannelProxy object caches a pointer to the IPC thread, so need to
   // reset it as it's not guaranteed to outlive this object.
   // NOTE: this also has the side-effect of not closing the main IPC channel to
   // the browser process.  This is needed because this is the signal that the
   // browser uses to know that this process has died, so we need it to be alive
   // until this process is shut down, and the OS closes the handle
   // automatically.  We used to watch the object handle on Windows to do this,
   // but it wasn't possible to do so on POSIX.
   channel_->ClearIPCTaskRunner();
 }
 
 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();
 }
 
 bool ChildThread::Send(IPC::Message* msg) {
   DCHECK(MessageLoop::current() == message_loop());
   if (!channel_.get()) {
     delete msg;
     return false;
   }
@@ skipping 176 matching lines @@
   }
 
   // The child process shutdown sequence is a request response based mechanism,
   // where we send out an initial feeler request to the child process host
   // instance in the browser to verify if it's ok to shutdown the child process.
   // The browser then sends back a response if it's ok to shutdown. This avoids
   // race conditions if the process refcount is 0 but there's an IPC message
   // inflight that would addref it.
   Send(new ChildProcessHostMsg_ShutdownRequest);
 }
+
+void ChildThread::EnsureConnected() {
+  LOG(INFO) << "ChildThread::EnsureConnected()";
+  base::KillProcess(base::GetCurrentProcessHandle(), 0, false);
+}