Fix ExternalImageTarget EGLImage race

Race may happen when ExternalImageTarget EGLImage is destroyed while its
GLES Texture/Renderbuffer target is modified/destroyed.

Fixed by providing `egl::Image` with `egl::ContextMutex` even when
`context` is `nullptr`.

This CL also changes `SharedContextMutex` merging rules when `mRank` is
equal - now priority goes to the `lockedMutex`. This is done to prevent
unnecessary `mRoot` update of Context mutex when merging with
`egl::Image` only mutex.

Bug: angleproject:6957
Change-Id: I823e53b98f70ed3eaca191e8be5b168dc07899f6
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4720835
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
This commit is contained in:
Igor Nazarov
2023-07-26 19:55:56 +03:00
committed by Angle LUCI CQ
parent af5bf5b824
commit 8aa3ca9d17
3 changed files with 31 additions and 26 deletions

View File

@@ -11,6 +11,7 @@
#include "common/debug.h"
#include "common/utilities.h"
#include "libANGLE/Context.h"
#include "libANGLE/Display.h"
#include "libANGLE/Renderbuffer.h"
#include "libANGLE/Texture.h"
#include "libANGLE/angletypes.h"
@@ -310,7 +311,8 @@ Image::Image(rx::EGLImplFactory *factory,
const AttributeMap &attribs)
: mState(id, target, buffer, attribs),
mImplementation(factory->createImage(mState, context, target, attribs)),
mOrphanedAndNeedsInit(false)
mOrphanedAndNeedsInit(false),
mSharedContextMutex(nullptr)
{
ASSERT(mImplementation != nullptr);
ASSERT(buffer != nullptr);
@@ -321,10 +323,6 @@ Image::Image(rx::EGLImplFactory *factory,
mSharedContextMutex = context->getContextMutex();
mSharedContextMutex->addRef();
}
else
{
mSharedContextMutex = nullptr;
}
mState.source->addImageSource(this);
}
@@ -499,6 +497,12 @@ rx::ImageImpl *Image::getImplementation() const
Error Image::initialize(const Display *display, const gl::Context *context)
{
if (kIsSharedContextMutexEnabled && mSharedContextMutex == nullptr)
{
mSharedContextMutex = display->getSharedContextMutexManager()->create();
mSharedContextMutex->addRef();
}
if (IsExternalImageTarget(mState.target))
{
ExternalImageSibling *externalSibling = rx::GetAs<ExternalImageSibling>(mState.source);

View File

@@ -241,28 +241,29 @@ void SharedContextMutex<Mutex>::Merge(SharedContextMutex *lockedMutex,
// Decide the new "root". See mRank comment for more details...
// Make "otherLockedRoot" the root of the "merged" mutex
if (lockedRoot->mRank > otherLockedRoot->mRank)
SharedContextMutex *oldRoot = otherLockedRoot;
SharedContextMutex *newRoot = lockedRoot;
if (oldRoot->mRank > newRoot->mRank)
{
// So the "lockedRoot" is lower rank.
std::swap(lockedRoot, otherLockedRoot);
std::swap(oldRoot, newRoot);
}
else if (lockedRoot->mRank == otherLockedRoot->mRank)
else if (oldRoot->mRank == newRoot->mRank)
{
++otherLockedRoot->mRank;
++newRoot->mRank;
}
// Update the structure
for (SharedContextMutex *const leaf : lockedRoot->mLeaves)
for (SharedContextMutex *const leaf : oldRoot->mLeaves)
{
ASSERT(leaf->getRoot() == lockedRoot);
leaf->setNewRoot(otherLockedRoot);
ASSERT(leaf->getRoot() == oldRoot);
leaf->setNewRoot(newRoot);
}
lockedRoot->mLeaves.clear();
lockedRoot->setNewRoot(otherLockedRoot);
oldRoot->mLeaves.clear();
oldRoot->setNewRoot(newRoot);
// Leave only the "merged" mutex locked. "lockedRoot" already merged, need to use "doUnlock()"
lockedRoot->doUnlock();
// Leave only the "merged" mutex locked. "oldRoot" already merged, need to use "doUnlock()"
oldRoot->doUnlock();
}
template <class Mutex>

View File

@@ -219,7 +219,7 @@ class SharedContextMutex final : public ContextMutex
// - the mutex_1 has no other references (only in the mutex_2);
// - have other mutex_3 "root";
// - mutex_1 pointer is cached on the stack during locking of mutex_2 (thread A);
// - marge mutex_2 and mutex_3 (thread B):
// - merge mutex_3 and mutex_2 (thread B):
// * now "leaf" mutex_2 stores reference to mutex_3 "root";
// * old "root" mutex_1 becomes a "leaf" of mutex_3;
// * old "root" mutex_1 has no references and gets destroyed.
@@ -228,27 +228,27 @@ class SharedContextMutex final : public ContextMutex
std::vector<SharedContextMutex *> mOldRoots;
// mRank is used to fix a problem of indefinite grows of mOldRoots:
// - merge mutex_1 and mutex_2 -> mutex_2 is "root" of mutex_1 (mOldRoots == 0);
// - merge mutex_2 and mutex_1 -> mutex_2 is "root" of mutex_1 (mOldRoots == 0);
// - destroy mutex_2;
// - merge mutex_1 and mutex_3 -> mutex_3 is "root" of mutex_1 (mOldRoots == 1);
// - merge mutex_3 and mutex_1 -> mutex_3 is "root" of mutex_1 (mOldRoots == 1);
// - destroy mutex_3;
// - merge mutex_1 and mutex_4 -> mutex_4 is "root" of mutex_1 (mOldRoots == 2);
// - merge mutex_4 and mutex_1 -> mutex_4 is "root" of mutex_1 (mOldRoots == 2);
// - destroy mutex_4;
// - continuing this pattern can lead to indefinite grows of mOldRoots, while pick number of
// mutexes is only 2.
// Fix details using mRank:
// - initially "mRank == 0" and only relevant for "root" mutexes;
// - merging mutexes with equal mRank of their "root"s, will use second (otherMutex) "root"
// - merging mutexes with equal mRank of their "root"s, will use first (lockedMutex) "root"
// mutex as a new "root" and increase its mRank by 1;
// - otherwise, "root" mutex with a highest rank will be used without changing the mRank;
// - this way, "stronger" (with a higher mRank) "root" mutex will "protect" its "leaves" from
// "mRoot" replacement and therefore - mOldRoots grows.
// Lets look at the problematic pattern with the mRank:
// - merge mutex_1 and mutex_2 -> mutex_2 is "root" (mRank == 1) of mutex_1 (mOldRoots == 0);
// - merge mutex_2 and mutex_1 -> mutex_2 is "root" (mRank == 1) of mutex_1 (mOldRoots == 0);
// - destroy mutex_2;
// - merge mutex_1 and mutex_3 -> mutex_2 is "root" (mRank == 1) of mutex_3 (mOldRoots == 0);
// - merge mutex_3 and mutex_1 -> mutex_2 is "root" (mRank == 1) of mutex_3 (mOldRoots == 0);
// - destroy mutex_3;
// - merge mutex_1 and mutex_4 -> mutex_2 is "root" (mRank == 1) of mutex_4 (mOldRoots == 0);
// - merge mutex_4 and mutex_1 -> mutex_2 is "root" (mRank == 1) of mutex_4 (mOldRoots == 0);
// - destroy mutex_4;
// - no mOldRoots grows at all;
// - minumum number of mutexes to reach mOldRoots size of N => 2^(N+1).