Fix potential shutdown deadlock in UI service

If the GpuPtr is disconnected while there are pending buffer allocations
in flight, this can lead to the main thread deadlocking on a WaitableEvent
that never signals.

Fixes that by tracking pending allocations and ensuring they're always
signalled on connection error or other shutdown events.

BUG=
R=sadrul@chromium.org

Review-Url: https://codereview.chromium.org/2651443002
Cr-Commit-Position: refs/heads/master@{#445434}

services/ui/public/cpp/gpu/client_gpu_memory_buffer_manager.cc

@@ -22,13 +22,6 @@ namespace ui {
 
 namespace {
 
-void OnGpuMemoryBufferAllocated(gfx::GpuMemoryBufferHandle* ret_handle,
-                                base::WaitableEvent* wait,
-                                const gfx::GpuMemoryBufferHandle& handle) {
-  *ret_handle = handle;
-  wait->Signal();
-}
-
 void NotifyDestructionOnCorrectThread(
     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
     const DestructionCallback& callback,
@@ -58,12 +51,24 @@ ClientGpuMemoryBufferManager::~ClientGpuMemoryBufferManager() {
 
 void ClientGpuMemoryBufferManager::InitThread(mojom::GpuPtrInfo gpu_info) {
   gpu_.Bind(std::move(gpu_info));
+  gpu_.set_connection_error_handler(
+      base::Bind(&ClientGpuMemoryBufferManager::DisconnectGpuOnThread,
+                 base::Unretained(this)));
   weak_ptr_ = weak_ptr_factory_.GetWeakPtr();
 }
 
 void ClientGpuMemoryBufferManager::TearDownThread() {
   weak_ptr_factory_.InvalidateWeakPtrs();
+  DisconnectGpuOnThread();
+}
+
+void ClientGpuMemoryBufferManager::DisconnectGpuOnThread() {
+  if (!gpu_.is_bound())
+    return;
   gpu_.reset();
+  for (auto& waiter : pending_allocation_waiters_)
+    waiter->Signal();
+  pending_allocation_waiters_.clear();
 }
 
 void ClientGpuMemoryBufferManager::AllocateGpuMemoryBufferOnThread(
@@ -76,9 +81,24 @@ void ClientGpuMemoryBufferManager::AllocateGpuMemoryBufferOnThread(
   // |handle| and |wait| are both on the stack, and will be alive until |wait|
   // is signaled. So it is safe for OnGpuMemoryBufferAllocated() to operate on
   // these.
+  pending_allocation_waiters_.insert(wait);
   gpu_->CreateGpuMemoryBuffer(
       gfx::GpuMemoryBufferId(++counter_), size, format, usage,
-      base::Bind(&OnGpuMemoryBufferAllocated, handle, wait));
+      base::Bind(
+          &ClientGpuMemoryBufferManager::OnGpuMemoryBufferAllocatedOnThread,
+          base::Unretained(this), handle, wait));
+}
+
+void ClientGpuMemoryBufferManager::OnGpuMemoryBufferAllocatedOnThread(
+    gfx::GpuMemoryBufferHandle* ret_handle,
+    base::WaitableEvent* wait,
+    const gfx::GpuMemoryBufferHandle& handle) {
+  auto it = pending_allocation_waiters_.find(wait);
+  DCHECK(it != pending_allocation_waiters_.end());
+  pending_allocation_waiters_.erase(it);
+
+  *ret_handle = handle;
+  wait->Signal();
 }
 
 void ClientGpuMemoryBufferManager::DeletedGpuMemoryBuffer(

services/ui/public/cpp/gpu/client_gpu_memory_buffer_manager.h

@@ -6,6 +6,7 @@
 #define SERVICES_UI_PUBLIC_CPP_GPU_CLIENT_GPU_MEMORY_BUFFER_MANAGER_H_
 
 #include <memory>
+#include <set>
 #include <vector>
 
 #include "base/macros.h"
@@ -29,11 +30,16 @@ class ClientGpuMemoryBufferManager : public gpu::GpuMemoryBufferManager {
  private:
   void InitThread(mojom::GpuPtrInfo gpu_info);
   void TearDownThread();
+  void DisconnectGpuOnThread();
   void AllocateGpuMemoryBufferOnThread(const gfx::Size& size,
                                        gfx::BufferFormat format,
                                        gfx::BufferUsage usage,
                                        gfx::GpuMemoryBufferHandle* handle,
                                        base::WaitableEvent* wait);
+  void OnGpuMemoryBufferAllocatedOnThread(
+      gfx::GpuMemoryBufferHandle* ret_handle,
+      base::WaitableEvent* wait,
+      const gfx::GpuMemoryBufferHandle& handle);
   void DeletedGpuMemoryBuffer(gfx::GpuMemoryBufferId id,
                               const gpu::SyncToken& sync_token);
 
@@ -45,11 +51,13 @@ class ClientGpuMemoryBufferManager : public gpu::GpuMemoryBufferManager {
       gpu::SurfaceHandle surface_handle) override;
   void SetDestructionSyncToken(gfx::GpuMemoryBuffer* buffer,
                                const gpu::SyncToken& sync_token) override;
+
   int counter_ = 0;
   // TODO(sad): Explore the option of doing this from an existing thread.
   base::Thread thread_;
   mojom::GpuPtr gpu_;
   base::WeakPtr<ClientGpuMemoryBufferManager> weak_ptr_;
+  std::set<base::WaitableEvent*> pending_allocation_waiters_;
   base::WeakPtrFactory<ClientGpuMemoryBufferManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ClientGpuMemoryBufferManager);