Mus: Avoid deadlock during teardown

services/ui/gpu/gpu_main.cc

Index: services/ui/gpu/gpu_main.cc
diff --git a/services/ui/gpu/gpu_main.cc b/services/ui/gpu/gpu_main.cc
index b52ae1f8278b2d078d9df5d07315f19d69ca1b61..790ba33752f1b247438fc393a10d3c6346179b97 100644
--- a/services/ui/gpu/gpu_main.cc
+++ b/services/ui/gpu/gpu_main.cc
@@ -41,7 +41,10 @@ std::unique_ptr<base::MessagePump> CreateMessagePumpMac() {
 namespace ui {
 
 GpuMain::GpuMain()
-    : gpu_thread_("GpuThread"), io_thread_("GpuIOThread"), weak_factory_(this) {
+    : gpu_thread_("GpuThread"),
+      io_thread_("GpuIOThread"),
+      compositor_thread_("DisplayCompositorThread"),
+      weak_factory_(this) {
   base::Thread::Options thread_options;
 
 #if defined(OS_WIN)
@@ -73,10 +76,21 @@ GpuMain::GpuMain()
   thread_options.priority = base::ThreadPriority::DISPLAY;
 #endif
   CHECK(io_thread_.StartWithOptions(thread_options));
+
+  // Start the compositor thread.
+  compositor_thread_.Start();
 }
 
 GpuMain::~GpuMain() {
   // Unretained() is OK here since the thread/task runner is owned by |this|.
+  compositor_thread_.task_runner()->PostTask(
+      FROM_HERE,
+      base::Bind(&GpuMain::TearDownOnCompositorThread, base::Unretained(this)));
+  // Block the main thread until the compositor thread terminates which blocks
+  // on the gpu thread. The Stop must be initiated from here instead of the gpu
+  // thread to avoid deadlock.
+  compositor_thread_.Stop();
+
   gpu_thread_.task_runner()->PostTask(
       FROM_HERE,
       base::Bind(&GpuMain::TearDownOnGpuThread, base::Unretained(this)));
@@ -109,10 +123,15 @@ void GpuMain::InitOnGpuThread() {
     }
     gpu_service_internal_.reset(new GpuServiceInternal(
         gpu_init_->gpu_info(), gpu_init_->TakeWatchdogThread(),
-        gpu_memory_buffer_factory_.get(), io_thread_.task_runner()));
+        gpu_memory_buffer_factory_.get(), io_thread_.task_runner(),
+        compositor_thread_.task_runner()));
   }
 }
 
+void GpuMain::TearDownOnCompositorThread() {
+  gpu_service_internal_->DestroyDisplayCompositor();

[sadrul, 2016/11/24 05:07:17]

It is possible to be here without having the |gpu_service_internal_| been
created at all, I think? (e.g. gpu-thread hasn't run InitOnGpuThread() yet,
which means gpu_service_internal_ is still uninitialized)

Can we move DisplayCompositor ownership out of GpuServiceInternal, and into here
in GpuMain instead?

[Fady Samuel, 2016/11/24 05:21:55]

The problem is "CreateDisplayCompositor" is part of the GpuServiceInternal
interface. I have filed a bug to pull that out:
https://bugs.chromium.org/p/chromium/issues/detail?id=668357

I'll check that gpu_service_internal_ is not null here, and do the right thing
tomorrow or Friday.

+}
+
 void GpuMain::TearDownOnGpuThread() {
   gpu_service_internal_.reset();
   gpu_memory_buffer_factory_.reset();