gpu: Thread-safe command buffer state lookup.

gpu/ipc/client/command_buffer_proxy_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 "gpu/ipc/client/command_buffer_proxy_impl.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/callback.h"
@@ skipping 112 matching lines @@
 
   if (!handled) {
     LOG(ERROR) << "Gpu process sent invalid message.";
     OnGpuAsyncMessageError(gpu::error::kInvalidGpuMessage,
                            gpu::error::kLostContext);
   }
   return handled;
 }
 
 void CommandBufferProxyImpl::OnChannelError() {
-  std::unique_ptr<base::AutoLock> lock;
+  base::Optional<base::AutoLock> lock;
   if (lock_)
-    lock.reset(new base::AutoLock(*lock_));
+    lock.emplace(*lock_);
+  base::AutoLock state_lock(state_lock_);
 
   gpu::error::ContextLostReason context_lost_reason =
       gpu::error::kGpuChannelLost;
   if (shared_state_shm_ && shared_state_shm_->memory()) {
     // The GPU process might have intentionally been crashed
     // (exit_on_context_lost), so try to find out the original reason.
     TryUpdateStateDontReportError();
     if (last_state_.error == gpu::error::kLostContext)
       context_lost_reason = last_state_.context_lost_reason;
   }
@@ skipping 57 matching lines @@
   shared_state()->Initialize();
 
   // This handle is owned by the GPU process and must be passed to it or it
   // will leak. In otherwords, do not early out on error between here and the
   // sending of the CreateCommandBuffer IPC below.
   base::SharedMemoryHandle handle =
       channel->ShareToGpuProcess(shared_state_shm_->handle());
   if (!base::SharedMemory::IsHandleValid(handle))
     return false;
 
-
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/125248 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "125248 CommandBufferProxyImpl::Initialize"));
 
   // Route must be added before sending the message, otherwise messages sent
   // from the GPU process could race against adding ourselves to the filter.
   channel->AddRouteWithTaskRunner(route_id_, AsWeakPtr(), task_runner);
 
   // We're blocking the UI thread, which is generally undesirable.
   // In this case we need to wait for this before we can show any UI /anyway/,
   // so it won't cause additional jank.
   // TODO(piman): Make this asynchronous (http://crbug.com/125248).
   bool result = false;
   bool sent = channel->Send(new GpuChannelMsg_CreateCommandBuffer(
       config, route_id_, handle, &result, &capabilities_));
   if (!sent || !result) {
     DLOG(ERROR) << "Failed to send GpuChannelMsg_CreateCommandBuffer.";
     channel->RemoveRoute(route_id_);
     return false;
   }
 
   channel_ = std::move(channel);
   callback_thread_ = std::move(task_runner);
 
   return true;
 }
 
 gpu::CommandBuffer::State CommandBufferProxyImpl::GetLastState() {
+  base::AutoLock lock(state_lock_);
+  TryUpdateState();
   return last_state_;
 }
 
 int32_t CommandBufferProxyImpl::GetLastToken() {

[piman, 2016/12/02 19:21:48]

nit: maybe return GetLastState().token ?

Maybe we don't really need this call anymore, though. the cost of TryUpdateState
and the lock are orders of magnitude larger than the cost of copying the entire
gpu::CommandBuffer::State

[sunnyps, 2016/12/07 03:31:22]

Got rid of both GetLastToken and GetLastError. This requires GetLastState to
call TryUpdateState but that is OK because we don't call it as often as before.

+  base::AutoLock lock(state_lock_);
   TryUpdateState();
   return last_state_.token;
 }
 
 void CommandBufferProxyImpl::Flush(int32_t put_offset) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   if (last_state_.error != gpu::error::kNoError)
     return;
 
   TRACE_EVENT1("gpu", "CommandBufferProxyImpl::Flush", "put_offset",
                put_offset);
 
   bool put_offset_changed = last_put_offset_ != put_offset;
   last_put_offset_ = put_offset;
   last_barrier_put_offset_ = put_offset;
 
@@ skipping 13 matching lines @@
     }
     CleanupFlushedReleases(highest_verified_flush_id);
   }
 
   if (put_offset_changed)
     latency_info_.clear();
 }
 
 void CommandBufferProxyImpl::OrderingBarrier(int32_t put_offset) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   if (last_state_.error != gpu::error::kNoError)
     return;
 
   TRACE_EVENT1("gpu", "CommandBufferProxyImpl::OrderingBarrier", "put_offset",
                put_offset);
 
   bool put_offset_changed = last_barrier_put_offset_ != put_offset;
   last_barrier_put_offset_ = put_offset;
 
   if (channel_) {
@@ skipping 30 matching lines @@
   CheckLock();
   swap_buffers_completion_callback_ = callback;
 }
 
 void CommandBufferProxyImpl::SetUpdateVSyncParametersCallback(
     const UpdateVSyncParametersCallback& callback) {
   CheckLock();
   update_vsync_parameters_completion_callback_ = callback;
 }
 
-void CommandBufferProxyImpl::WaitForTokenInRange(int32_t start, int32_t end) {
+gpu::CommandBuffer::State CommandBufferProxyImpl::WaitForTokenInRange(
+    int32_t start,
+    int32_t end) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   TRACE_EVENT2("gpu", "CommandBufferProxyImpl::WaitForToken", "start", start,
                "end", end);
+  // Error needs to be checked in case the state was updated on another thread.
+  // We need to make sure that the reentrant context loss callback is called so
+  // that the share group is also lost before this method returns.
+  if (last_state_.error != gpu::error::kNoError) {
+    OnGpuStateError();

[piman, 2016/12/02 19:21:48]

I don't think OnGpuStateError is right, because that will post tasks and call
OnGpuControlLostContext many times. I think you only need to call
OnGpuControlLostContextMaybeReentrant here. the task to call
OnGpuControlLostContext will have been posted by the thread that set the error.

[sunnyps, 2016/12/07 03:31:22]

DisconnectChannel (the posted task) early outs if it's already called once.
Nonetheless, I've changed it because every call to WaitForGetOffset/Token would
post a new task otherwise.

+    return last_state_;
+  }
   TryUpdateState();
   if (!InRange(start, end, last_state_.token) &&
       last_state_.error == gpu::error::kNoError) {
     gpu::CommandBuffer::State state;
     if (Send(new GpuCommandBufferMsg_WaitForTokenInRange(route_id_, start, end,
-                                                         &state)))
+                                                         &state))) {
       SetStateFromSyncReply(state);
+    }
   }
   if (!InRange(start, end, last_state_.token) &&
       last_state_.error == gpu::error::kNoError) {
     LOG(ERROR) << "GPU state invalid after WaitForTokenInRange.";
     OnGpuSyncReplyError();
   }
+  return last_state_;
 }
 
-void CommandBufferProxyImpl::WaitForGetOffsetInRange(int32_t start,
-                                                     int32_t end) {
+gpu::CommandBuffer::State CommandBufferProxyImpl::WaitForGetOffsetInRange(
+    int32_t start,
+    int32_t end) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   TRACE_EVENT2("gpu", "CommandBufferProxyImpl::WaitForGetOffset", "start",
                start, "end", end);
+  // Error needs to be checked in case the state was updated on another thread.
+  // We need to make sure that the reentrant context loss callback is called so
+  // that the share group is also lost before this method returns.
+  if (last_state_.error != gpu::error::kNoError) {
+    OnGpuStateError();

[piman, 2016/12/02 19:21:48]

Ditto

[sunnyps, 2016/12/07 03:31:22]

Done.

+    return last_state_;
+  }
   TryUpdateState();
   if (!InRange(start, end, last_state_.get_offset) &&
       last_state_.error == gpu::error::kNoError) {
     gpu::CommandBuffer::State state;
     if (Send(new GpuCommandBufferMsg_WaitForGetOffsetInRange(route_id_, start,
                                                              end, &state)))
       SetStateFromSyncReply(state);
   }
   if (!InRange(start, end, last_state_.get_offset) &&
       last_state_.error == gpu::error::kNoError) {
     LOG(ERROR) << "GPU state invalid after WaitForGetOffsetInRange.";
     OnGpuSyncReplyError();
   }
+  return last_state_;
 }
 
 void CommandBufferProxyImpl::SetGetBuffer(int32_t shm_id) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   if (last_state_.error != gpu::error::kNoError)
     return;
 
   Send(new GpuCommandBufferMsg_SetGetBuffer(route_id_, shm_id));
   last_put_offset_ = -1;
 }
 
 scoped_refptr<gpu::Buffer> CommandBufferProxyImpl::CreateTransferBuffer(
     size_t size,
     int32_t* id) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   *id = -1;
 
   if (last_state_.error != gpu::error::kNoError)
     return NULL;
 
   int32_t new_id = channel_->ReserveTransferBufferId();
 
   std::unique_ptr<base::SharedMemory> shared_memory(
       channel_->factory()->AllocateSharedMemory(size));
   if (!shared_memory) {
@@ skipping 23 matching lines @@
   Send(new GpuCommandBufferMsg_RegisterTransferBuffer(route_id_, new_id, handle,
                                                       size));
   *id = new_id;
   scoped_refptr<gpu::Buffer> buffer(
       gpu::MakeBufferFromSharedMemory(std::move(shared_memory), size));
   return buffer;
 }
 
 void CommandBufferProxyImpl::DestroyTransferBuffer(int32_t id) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   if (last_state_.error != gpu::error::kNoError)
     return;
 
   Send(new GpuCommandBufferMsg_DestroyTransferBuffer(route_id_, id));
 }
 
 void CommandBufferProxyImpl::SetGpuControlClient(GpuControlClient* client) {
   CheckLock();
   gpu_control_client_ = client;
 }
 
 gpu::Capabilities CommandBufferProxyImpl::GetCapabilities() {
   return capabilities_;
 }
 
 int32_t CommandBufferProxyImpl::CreateImage(ClientBuffer buffer,
                                             size_t width,
                                             size_t height,
                                             unsigned internal_format) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   if (last_state_.error != gpu::error::kNoError)
     return -1;
 
   int32_t new_id = channel_->ReserveImageId();
 
   gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager =
       channel_->gpu_memory_buffer_manager();
   gfx::GpuMemoryBuffer* gpu_memory_buffer =
       reinterpret_cast<gfx::GpuMemoryBuffer*>(buffer);
   DCHECK(gpu_memory_buffer);
@@ skipping 40 matching lines @@
 
     gpu_memory_buffer_manager->SetDestructionSyncToken(gpu_memory_buffer,
                                                        sync_token);
   }
 
   return new_id;
 }
 
 void CommandBufferProxyImpl::DestroyImage(int32_t id) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   if (last_state_.error != gpu::error::kNoError)
     return;
 
   Send(new GpuCommandBufferMsg_DestroyImage(route_id_, id));
 }
 
 int32_t CommandBufferProxyImpl::CreateGpuMemoryBufferImage(
     size_t width,
     size_t height,
     unsigned internal_format,
     unsigned usage) {
   CheckLock();
   std::unique_ptr<gfx::GpuMemoryBuffer> buffer(
       channel_->gpu_memory_buffer_manager()->CreateGpuMemoryBuffer(
           gfx::Size(width, height),
           gpu::DefaultBufferFormatForImageFormat(internal_format),
           gfx::BufferUsage::SCANOUT, gpu::kNullSurfaceHandle));
   if (!buffer)
     return -1;
 
   int32_t result =
       CreateImage(buffer->AsClientBuffer(), width, height, internal_format);
   return result;
 }
 
 uint32_t CommandBufferProxyImpl::CreateStreamTexture(uint32_t texture_id) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   if (last_state_.error != gpu::error::kNoError)
     return 0;
 
   int32_t stream_id = channel_->GenerateRouteID();
   bool succeeded = false;
   Send(new GpuCommandBufferMsg_CreateStreamTexture(route_id_, texture_id,
                                                    stream_id, &succeeded));
   if (!succeeded) {
     DLOG(ERROR) << "GpuCommandBufferMsg_CreateStreamTexture returned failure";
     return 0;
@@ skipping 32 matching lines @@
   return release != 0 && release < next_fence_sync_release_;
 }
 
 bool CommandBufferProxyImpl::IsFenceSyncFlushed(uint64_t release) {
   CheckLock();
   return release != 0 && release <= flushed_fence_sync_release_;
 }
 
 bool CommandBufferProxyImpl::IsFenceSyncFlushReceived(uint64_t release) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   if (last_state_.error != gpu::error::kNoError)
     return false;
 
   if (release <= verified_fence_sync_release_)
     return true;
 
   // Check if we have actually flushed the fence sync release.
   if (release <= flushed_fence_sync_release_) {
     DCHECK(!flushed_release_flush_id_.empty());
     // Check if it has already been validated by another context.
     UpdateVerifiedReleases(channel_->GetHighestValidatedFlushID(stream_id_));
     if (release <= verified_fence_sync_release_)
       return true;
 
     // Has not been validated, validate it now.
     UpdateVerifiedReleases(
         channel_->ValidateFlushIDReachedServer(stream_id_, false));
     return release <= verified_fence_sync_release_;
   }
 
   return false;
 }
 
+bool CommandBufferProxyImpl::IsFenceSyncReleased(uint64_t release) {
+  base::AutoLock lock(state_lock_);
+  TryUpdateStateThreadSafe();
+  return release <= last_state_.release_count;
+}
+
 void CommandBufferProxyImpl::SignalSyncToken(const gpu::SyncToken& sync_token,
                                              const base::Closure& callback) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   if (last_state_.error != gpu::error::kNoError)
     return;
 
   uint32_t signal_id = next_signal_id_++;
   Send(new GpuCommandBufferMsg_SignalSyncToken(route_id_, sync_token,
                                                signal_id));
   signal_tasks_.insert(std::make_pair(signal_id, callback));
 }
 
 bool CommandBufferProxyImpl::CanWaitUnverifiedSyncToken(
@@ skipping 14 matching lines @@
 
   if (release_stream_id != stream_id_)
     channel_->FlushPendingStream(release_stream_id);
 
   return true;
 }
 
 void CommandBufferProxyImpl::SignalQuery(uint32_t query,
                                          const base::Closure& callback) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   if (last_state_.error != gpu::error::kNoError)
     return;
 
   // Signal identifiers are hidden, so nobody outside of this class will see
   // them. (And thus, they cannot save them.) The IDs themselves only last
   // until the callback is invoked, which will happen as soon as the GPU
   // catches upwith the command buffer.
   // A malicious caller trying to create a collision by making next_signal_id
   // would have to make calls at an astounding rate (300B/s) and even if they
   // could do that, all they would do is to prevent some callbacks from getting
   // called, leading to stalled threads and/or memory leaks.
   uint32_t signal_id = next_signal_id_++;
   Send(new GpuCommandBufferMsg_SignalQuery(route_id_, query, signal_id));
   signal_tasks_.insert(std::make_pair(signal_id, callback));
 }
 
 void CommandBufferProxyImpl::TakeFrontBuffer(const gpu::Mailbox& mailbox) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   if (last_state_.error != gpu::error::kNoError)
     return;
 
   Send(new GpuCommandBufferMsg_TakeFrontBuffer(route_id_, mailbox));
 }
 
 void CommandBufferProxyImpl::ReturnFrontBuffer(const gpu::Mailbox& mailbox,
                                                const gpu::SyncToken& sync_token,
                                                bool is_lost) {
   CheckLock();
+  base::AutoLock lock(state_lock_);
   if (last_state_.error != gpu::error::kNoError)
     return;
 
   Send(new GpuCommandBufferMsg_WaitSyncToken(route_id_, sync_token));
   Send(new GpuCommandBufferMsg_ReturnFrontBuffer(route_id_, mailbox, is_lost));
 }
 
 gpu::error::Error CommandBufferProxyImpl::GetLastError() {

[piman, 2016/12/02 19:21:48]

nit: same as GetLastToken, I don't think we need this any more.

+  base::AutoLock lock(state_lock_);
+  TryUpdateState();
   return last_state_.error;
 }
 
 bool CommandBufferProxyImpl::Send(IPC::Message* msg) {
-  // Caller should not intentionally send a message if the context is lost.
-  DCHECK(last_state_.error == gpu::error::kNoError);
   DCHECK(channel_);
+  state_lock_.AssertAcquired();
+  DCHECK_EQ(gpu::error::kNoError, last_state_.error);
 
-  if (!msg->is_sync()) {
-    bool result = channel_->Send(msg);
-    // Send() should always return true for async messages.
-    DCHECK(result);
-    return true;
+  state_lock_.Release();
+
+  // Call is_sync() before sending message.
+  bool is_sync = msg->is_sync();
+  bool result = channel_->Send(msg);
+  // Send() should always return true for async messages.
+  DCHECK(is_sync || result);
+
+  state_lock_.Acquire();
+
+  if (last_state_.error != gpu::error::kNoError) {
+    // Context might have been lost on another thread while we were waiting for
+    // message send/reply.
+    OnGpuStateError();

[piman, 2016/12/02 19:21:48]

Ditto, I think you only want to call OnGpuControlLostContextMaybeReentrant

[sunnyps, 2016/12/07 03:31:22]

Done.

+    return false;
   }
 
-  if (channel_->Send(msg))
-    return true;
+  if (!result) {
+    // Flag the command buffer as lost. Defer deleting the channel until
+    // OnChannelError is called after returning to the message loop in case it
+    // is referenced elsewhere.
+    DVLOG(1) << "CommandBufferProxyImpl::Send failed. Losing context.";
+    OnClientError(gpu::error::kLostContext);
+    return false;
+  }
 
-  // Flag the command buffer as lost. Defer deleting the channel until
-  // OnChannelError is called after returning to the message loop in case
-  // it is referenced elsewhere.
-  DVLOG(1) << "CommandBufferProxyImpl::Send failed. Losing context.";
-  OnClientError(gpu::error::kLostContext);
-  return false;
+  return true;
 }
 
 void CommandBufferProxyImpl::SetStateFromSyncReply(
     const gpu::CommandBuffer::State& state) {
-  DCHECK(last_state_.error == gpu::error::kNoError);
+  CheckLock();
+  state_lock_.AssertAcquired();
   // Handle wraparound. It works as long as we don't have more than 2B state
   // updates in flight across which reordering occurs.
   if (state.generation - last_state_.generation < 0x80000000U)
     last_state_ = state;
   if (last_state_.error != gpu::error::kNoError)
     OnGpuStateError();
 }
 
-void CommandBufferProxyImpl::TryUpdateState() {
+gpu::CommandBuffer::State CommandBufferProxyImpl::TryUpdateState() {

[piman, 2016/12/02 19:21:48]

I don't think it's necessary for this (and other TryUpdateState version) to
return State. I don't think any caller uses the return value. Also, because
they're all called with state_lock_ held, callers can (and do) use last_state_
without loss of consistency.

[sunnyps, 2016/12/07 03:31:22]

Done.

+  CheckLock();
+  state_lock_.AssertAcquired();
   if (last_state_.error == gpu::error::kNoError) {
     shared_state()->Read(&last_state_);
     if (last_state_.error != gpu::error::kNoError)
       OnGpuStateError();
   }
+  return last_state_;
 }
 
-void CommandBufferProxyImpl::TryUpdateStateDontReportError() {
+gpu::CommandBuffer::State CommandBufferProxyImpl::TryUpdateStateThreadSafe() {
+  state_lock_.AssertAcquired();
+  if (last_state_.error == gpu::error::kNoError) {
+    shared_state()->Read(&last_state_);
+    if (last_state_.error != gpu::error::kNoError) {
+      callback_thread_->PostTask(
+          FROM_HERE,
+          base::Bind(&CommandBufferProxyImpl::LockAndDisconnectChannel,
+                     weak_this_));
+    }
+  }
+  return last_state_;
+}
+
+gpu::CommandBuffer::State
+CommandBufferProxyImpl::TryUpdateStateDontReportError() {
+  state_lock_.AssertAcquired();
   if (last_state_.error == gpu::error::kNoError)
     shared_state()->Read(&last_state_);
+  return last_state_;
 }
 
 void CommandBufferProxyImpl::UpdateVerifiedReleases(uint32_t verified_flush) {
   while (!flushed_release_flush_id_.empty()) {
     const std::pair<uint64_t, uint32_t>& front_item =
         flushed_release_flush_id_.front();
     if (front_item.second > verified_flush)
       break;
     verified_fence_sync_release_ = front_item.first;
     flushed_release_flush_id_.pop();
@@ skipping 48 matching lines @@
   }
 }
 
 void CommandBufferProxyImpl::OnUpdateVSyncParameters(base::TimeTicks timebase,
                                                      base::TimeDelta interval) {
   if (!update_vsync_parameters_completion_callback_.is_null())
     update_vsync_parameters_completion_callback_.Run(timebase, interval);
 }
 
 void CommandBufferProxyImpl::OnGpuSyncReplyError() {
+  CheckLock();
+  state_lock_.AssertAcquired();
   last_state_.error = gpu::error::kLostContext;
   last_state_.context_lost_reason = gpu::error::kInvalidGpuMessage;
   // This method may be inside a callstack from the GpuControlClient (we got a
   // bad reply to something we are sending to the GPU process). So avoid
   // re-entering the GpuControlClient here.
   DisconnectChannelInFreshCallStack();
 }
 
 void CommandBufferProxyImpl::OnGpuAsyncMessageError(
     gpu::error::ContextLostReason reason,
     gpu::error::Error error) {
   CheckLock();
+  state_lock_.AssertAcquired();
   last_state_.error = error;
   last_state_.context_lost_reason = reason;
   // This method only occurs when receiving IPC messages, so we know it's not in
   // a callstack from the GpuControlClient.
   DisconnectChannel();
 }
 
 void CommandBufferProxyImpl::OnGpuStateError() {
-  DCHECK(last_state_.error != gpu::error::kNoError);
+  CheckLock();
+  state_lock_.AssertAcquired();
+  DCHECK_NE(gpu::error::kNoError, last_state_.error);
   // This method may be inside a callstack from the GpuControlClient (we
   // encountered an error while trying to perform some action). So avoid
   // re-entering the GpuControlClient here.
   DisconnectChannelInFreshCallStack();
 }
 
 void CommandBufferProxyImpl::OnClientError(gpu::error::Error error) {
   CheckLock();
+  state_lock_.AssertAcquired();
   last_state_.error = error;
   last_state_.context_lost_reason = gpu::error::kUnknown;
   // This method may be inside a callstack from the GpuControlClient (we
   // encountered an error while trying to perform some action). So avoid
   // re-entering the GpuControlClient here.
   DisconnectChannelInFreshCallStack();
 }
 
 void CommandBufferProxyImpl::DisconnectChannelInFreshCallStack() {
   CheckLock();
+  state_lock_.AssertAcquired();
   // Inform the GpuControlClient of the lost state immediately, though this may
   // be a re-entrant call to the client so we use the MaybeReentrant variant.
   if (gpu_control_client_)
     gpu_control_client_->OnGpuControlLostContextMaybeReentrant();
   // Create a fresh call stack to keep the |channel_| alive while we unwind the
   // stack in case things will use it, and give the GpuChannelClient a chance to
   // act fully on the lost context.
   callback_thread_->PostTask(
       FROM_HERE, base::Bind(&CommandBufferProxyImpl::LockAndDisconnectChannel,
                             weak_this_));
@@ skipping 10 matching lines @@
   CheckLock();
   // Prevent any further messages from being sent, and ensure we only call
   // the client for lost context a single time.
   if (!channel_)
     return;
   channel_->FlushPendingStream(stream_id_);
   channel_->Send(new GpuChannelMsg_DestroyCommandBuffer(route_id_));
   channel_->RemoveRoute(route_id_);
   channel_ = nullptr;
   if (gpu_control_client_)
     gpu_control_client_->OnGpuControlLostContext();

[piman, 2016/12/02 19:21:48]

There are some paths where state_lock_ is held (e.g. coming from
OnChannelError->OnGpuAsyncMessageError->DisconnectChannel).
We don't want to keep that lock held while calling this callback, otherwise it
can lead to deadlocks if the client tries to e.g. call more GL functions from
there, or get the last state, etc.

Also, GpuChannelMsg_DestroyCommandBuffer is sync and can be arbitrary slow, so
it could hurt responsiveness of IsFenceSyncReleased (though admittedly this
would only happen on a context loss, maybe it's not a super big deal).

I think we should release state_lock_ before calling DisconnectChannel in
OnGpuAsyncMessageError.  This would also be consistent with the fact that it is
*not* held in the LockAndDisconnectChannel path.

[sunnyps, 2016/12/07 03:31:22]

Changed it so that the OnGpuAsyncMessageError releases state_lock_ when calling
DisconnectChannel.

 }
 
 }  // namespace gpu