Commit bceafa43 authored by Bo Liu's avatar Bo Liu Committed by Commit Bot

gpu: Avoid SharedImageBatchAccessManager deadlock

SharedImageBatchAccessManager has a lock, and
SharedImageBackingEglImage has a lock. There are cases where one lock is
acquired while trying to acquire the other, and the order is not
guaranteed, leading to deadlock in under the right circumstances. In
particular:

* On one thread, SharedImageBatchAccessManager::EndBatchReadAccess calls
  SharedImageBackingEglImage::SetEndReadFence. This acquires the batch
  lock first and the backing lock second.
* On another thread, SharedImageBackingEglImage::EndRead calls
  SharedImageBatchAccessManager::IsDoingBatchReads, which acquires the
  locks in the opposite order.
If the same backing is being read on two threads and both managed to
acquire the first lock, then this leads to a deadlock.

Fix by not holding the backing lock when calling into
SharedImageBatchAccessManager.

Bug: 1043566,1046101
Change-Id: I6d6f8031443f41f775eb87287fdcebf28c4fc8e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2078746Reviewed-by: default avatarKhushal <khushalsagar@chromium.org>
Reviewed-by: default avatarPeng Huang <penghuang@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Auto-Submit: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745518}
parent f73c9cfb
......@@ -207,14 +207,16 @@ bool SharedImageBackingEglImage::BeginRead(
void SharedImageBackingEglImage::EndRead(
const SharedImageRepresentation* reader) {
AutoLock auto_lock(this);
{
AutoLock auto_lock(this);
if (!active_readers_.contains(reader)) {
DLOG(ERROR) << "Attempt to end read to a SharedImageBacking without a "
"successful begin read";
return;
if (!active_readers_.contains(reader)) {
DLOG(ERROR) << "Attempt to end read to a SharedImageBacking without a "
"successful begin read";
return;
}
active_readers_.erase(reader);
}
active_readers_.erase(reader);
// For batch reads, we only need to create 1 fence after the last
// EndRead() for the whole batch of reads. Hence we just register this backing
......@@ -224,10 +226,11 @@ void SharedImageBackingEglImage::EndRead(
// batch reads/regular reads, we create 1 fence per EndRead().
if (batch_access_manager_->IsDoingBatchReads()) {
batch_access_manager_->RegisterEglBackingForEndReadFence(this);
} else {
read_fences_[gl::g_current_gl_context] =
base::MakeRefCounted<gl::SharedGLFenceEGL>();
return;
}
AutoLock auto_lock(this);
read_fences_[gl::g_current_gl_context] =
base::MakeRefCounted<gl::SharedGLFenceEGL>();
}
gles2::Texture* SharedImageBackingEglImage::GenEGLImageSibling() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment