Android: Fix ChildThreadImpl::ShutdownThread()

content/child/child_thread_impl.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/child/child_thread_impl.h"
 
 #include <signal.h>
 
 #include <string>
 
@@ skipping 41 matching lines @@
 #include "content/child/websocket_dispatcher.h"
 #include "content/common/child_process_messages.h"
 #include "content/common/in_process_child_thread_params.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 "ipc/mojo/ipc_channel_mojo.h"
 
+#if defined(OS_ANDROID)
+#include "base/thread_task_runner_handle.h"
+#endif
+
 #if defined(TCMALLOC_TRACE_MEMORY_SUPPORTED)
 #include "third_party/tcmalloc/chromium/src/gperftools/heap-profiler.h"
 #endif
 
 using tracked_objects::ThreadData;
 
 namespace content {
 namespace {
 
 // How long to wait for a connection to the browser process before giving up.
@@ skipping 82 matching lines @@
 #endif
   }
 
  protected:
   ~SuicideOnChannelErrorFilter() override {}
 };
 
 #endif  // OS(POSIX)
 
 #if defined(OS_ANDROID)
-ChildThreadImpl* g_child_thread = NULL;
-bool g_child_thread_initialized = false;
+// A class that allows for triggering a clean shutdown from another
+// thread through draining the main thread's msg loop.
+class QuitClosure {
+ public:
+  QuitClosure();
+  ~QuitClosure();
 
-// A lock protects g_child_thread.
-base::LazyInstance<base::Lock>::Leaky g_lazy_child_thread_lock =
-    LAZY_INSTANCE_INITIALIZER;
+  void BindToMainThread();
+  void PostQuitFromNonMainThread();
 
-// base::ConditionVariable has an explicit constructor that takes
-// a base::Lock pointer as parameter. The base::DefaultLazyInstanceTraits
-// doesn't handle the case. Thus, we need our own class here.
-struct CondVarLazyInstanceTraits {
-  static const bool kRegisterOnExit = false;
-#ifndef NDEBUG
-  static const bool kAllowedToAccessOnNonjoinableThread = true;
-#endif
+ private:
+  static void PostClosure(
+      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+      base::Closure closure);
 
-  static base::ConditionVariable* New(void* instance) {
-    return new (instance) base::ConditionVariable(
-        g_lazy_child_thread_lock.Pointer());
-  }
-  static void Delete(base::ConditionVariable* instance) {
-    instance->~ConditionVariable();
-  }
+  base::Lock lock_;
+  base::ConditionVariable cond_var_;
+  base::Closure closure_;
 };
 
-// A condition variable that synchronize threads initializing and waiting
-// for g_child_thread.
-base::LazyInstance<base::ConditionVariable, CondVarLazyInstanceTraits>
-    g_lazy_child_thread_cv = LAZY_INSTANCE_INITIALIZER;
-
-void QuitMainThreadMessageLoop() {
-  base::MessageLoop::current()->Quit();
+QuitClosure::QuitClosure() : cond_var_(&lock_) {
 }
 
+QuitClosure::~QuitClosure() {
+}
+
+void QuitClosure::PostClosure(
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+    base::Closure closure) {
+  task_runner->PostTask(FROM_HERE, closure);
+}
+
+void QuitClosure::BindToMainThread() {
+  {
+    base::AutoLock lock(lock_);
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner(
+        base::ThreadTaskRunnerHandle::Get());
+    base::Closure quit_closure =
+        base::MessageLoop::current()->QuitWhenIdleClosure();
+    closure_ =
+        base::Bind(&QuitClosure::PostClosure, task_runner, quit_closure);
+  }
+  cond_var_.Signal();

[piman, 2015/05/19 22:36:29]

nit: You can signal while the lock is taken, it's actually the usual pattern.
But this works (I don't think it matters in this case).

[no sievers, 2015/05/19 22:41:55]

Done.

+}
+
+void QuitClosure::PostQuitFromNonMainThread() {
+  base::AutoLock lock(lock_);
+  while (closure_.is_null())
+    cond_var_.Wait();
+
+  closure_.Run();
+}
+
+base::LazyInstance<QuitClosure> g_quit_closure = LAZY_INSTANCE_INITIALIZER;
 #endif
 
 }  // namespace
 
 ChildThread* ChildThread::Get() {
   return ChildThreadImpl::current();
 }
 
 ChildThreadImpl::Options::Options()
     : channel_name(base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
@@ skipping 187 matching lines @@
       connection_timeout = temp;
   }
 
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::Bind(&ChildThreadImpl::EnsureConnected,
                  channel_connected_factory_.GetWeakPtr()),
       base::TimeDelta::FromSeconds(connection_timeout));
 
 #if defined(OS_ANDROID)
-  {
-    base::AutoLock lock(g_lazy_child_thread_lock.Get());
-    g_child_thread = this;
-    g_child_thread_initialized = true;
-  }
-  // Signalling without locking is fine here because only
-  // one thread can wait on the condition variable.
-  g_lazy_child_thread_cv.Get().Signal();
+  g_quit_closure.Get().BindToMainThread();
 #endif
 
 #if defined(TCMALLOC_TRACE_MEMORY_SUPPORTED)
   trace_memory_controller_.reset(new base::trace_event::TraceMemoryController(
       message_loop_->message_loop_proxy(), ::HeapProfilerWithPseudoStackStart,
       ::HeapProfilerStop, ::GetHeapProfile));
 #endif
 
   base::trace_event::MemoryDumpManager::GetInstance()->Initialize();
 
   shared_bitmap_manager_.reset(
       new ChildSharedBitmapManager(thread_safe_sender()));
 
   gpu_memory_buffer_manager_.reset(
       new ChildGpuMemoryBufferManager(thread_safe_sender()));
 
   discardable_shared_memory_manager_.reset(
       new ChildDiscardableSharedMemoryManager(thread_safe_sender()));
 }
 
 ChildThreadImpl::~ChildThreadImpl() {
   // ChildDiscardableSharedMemoryManager has to be destroyed while
   // |thread_safe_sender_| is still valid.
   discardable_shared_memory_manager_.reset();
 
-#if defined(OS_ANDROID)
-  {
-    base::AutoLock lock(g_lazy_child_thread_lock.Get());
-    g_child_thread = nullptr;
-  }
-#endif
-
 #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
@@ skipping 174 matching lines @@
 ChildThreadImpl* ChildThreadImpl::current() {
   return g_lazy_tls.Pointer()->Get();
 }
 
 #if defined(OS_ANDROID)
 // The method must NOT be called on the child thread itself.
 // It may block the child thread if so.
 void ChildThreadImpl::ShutdownThread() {
   DCHECK(!ChildThreadImpl::current()) <<
       "this method should NOT be called from child thread itself";
-  {
-    base::AutoLock lock(g_lazy_child_thread_lock.Get());
-    while (!g_child_thread_initialized)
-      g_lazy_child_thread_cv.Get().Wait();
-
-    // g_child_thread may already have been destructed while we didn't hold the
-    // lock.
-    if (!g_child_thread)
-      return;
-
-    DCHECK_NE(base::MessageLoop::current(), g_child_thread->message_loop());
-    g_child_thread->message_loop()->PostTask(
-        FROM_HERE, base::Bind(&QuitMainThreadMessageLoop));
-  }
+  g_quit_closure.Get().PostQuitFromNonMainThread();
 }
 #endif
 
 void ChildThreadImpl::OnProcessFinalRelease() {
   if (on_channel_error_called_) {
     base::MessageLoop::current()->Quit();
     return;
   }
 
   // The child process shutdown sequence is a request response based mechanism,
@@ skipping 16 matching lines @@
 
 void ChildThreadImpl::OnProcessBackgrounded(bool background) {
   // Set timer slack to maximum on main thread when in background.
   base::TimerSlack timer_slack = base::TIMER_SLACK_NONE;
   if (background)
     timer_slack = base::TIMER_SLACK_MAXIMUM;
   base::MessageLoop::current()->SetTimerSlack(timer_slack);
 }
 
 }  // namespace content