Commit f2749096 by Jamie Madill Committed by Commit Bot

Fix racy global in gl::Compiler.

We introduce a new Display Global Mutex in egl::Display that can guard access to gActiveCompilers in Compiler.cpp. Was detected by running MultithreadingTest against TSAN. Bug: b/168744561 Change-Id: I97866b60a173f60899cd0406fe0f71000035c0cf Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2415181 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarCourtney Goeltzenleuchter <courtneygo@google.com> Reviewed-by: 's avatarYuly Novikov <ynovikov@chromium.org>
parent 1c79e9ea
...@@ -9,6 +9,8 @@ ...@@ -9,6 +9,8 @@
#include "libANGLE/Compiler.h" #include "libANGLE/Compiler.h"
#include "common/debug.h" #include "common/debug.h"
#include "libANGLE/Context.h"
#include "libANGLE/Display.h"
#include "libANGLE/State.h" #include "libANGLE/State.h"
#include "libANGLE/renderer/CompilerImpl.h" #include "libANGLE/renderer/CompilerImpl.h"
#include "libANGLE/renderer/GLImplFactory.h" #include "libANGLE/renderer/GLImplFactory.h"
...@@ -56,7 +58,7 @@ ShShaderSpec SelectShaderSpec(GLint majorVersion, ...@@ -56,7 +58,7 @@ ShShaderSpec SelectShaderSpec(GLint majorVersion,
} // anonymous namespace } // anonymous namespace
Compiler::Compiler(rx::GLImplFactory *implFactory, const State &state) Compiler::Compiler(rx::GLImplFactory *implFactory, const State &state, egl::Display *display)
: mImplementation(implFactory->createCompiler()), : mImplementation(implFactory->createCompiler()),
mSpec(SelectShaderSpec(state.getClientMajorVersion(), mSpec(SelectShaderSpec(state.getClientMajorVersion(),
state.getClientMinorVersion(), state.getClientMinorVersion(),
...@@ -72,11 +74,14 @@ Compiler::Compiler(rx::GLImplFactory *implFactory, const State &state) ...@@ -72,11 +74,14 @@ Compiler::Compiler(rx::GLImplFactory *implFactory, const State &state)
const gl::Caps &caps = state.getCaps(); const gl::Caps &caps = state.getCaps();
const gl::Extensions &extensions = state.getExtensions(); const gl::Extensions &extensions = state.getExtensions();
if (gActiveCompilers == 0)
{ {
sh::Initialize(); std::lock_guard<std::mutex> lock(display->getDisplayGlobalMutex());
if (gActiveCompilers == 0)
{
sh::Initialize();
}
++gActiveCompilers;
} }
++gActiveCompilers;
sh::InitBuiltInResources(&mResources); sh::InitBuiltInResources(&mResources);
mResources.MaxVertexAttribs = caps.maxVertexAttributes; mResources.MaxVertexAttribs = caps.maxVertexAttributes;
...@@ -211,8 +216,11 @@ Compiler::Compiler(rx::GLImplFactory *implFactory, const State &state) ...@@ -211,8 +216,11 @@ Compiler::Compiler(rx::GLImplFactory *implFactory, const State &state)
mResources.SubPixelBits = static_cast<int>(caps.subPixelBits); mResources.SubPixelBits = static_cast<int>(caps.subPixelBits);
} }
Compiler::~Compiler() Compiler::~Compiler() = default;
void Compiler::onDestroy(const Context *context)
{ {
std::lock_guard<std::mutex> lock(context->getDisplay()->getDisplayGlobalMutex());
for (auto &pool : mPools) for (auto &pool : mPools)
{ {
for (ShCompilerInstance &instance : pool) for (ShCompilerInstance &instance : pool)
......
...@@ -31,7 +31,9 @@ class State; ...@@ -31,7 +31,9 @@ class State;
class Compiler final : public RefCountObjectNoID class Compiler final : public RefCountObjectNoID
{ {
public: public:
Compiler(rx::GLImplFactory *implFactory, const State &data); Compiler(rx::GLImplFactory *implFactory, const State &data, egl::Display *display);
void onDestroy(const Context *context) override;
ShCompilerInstance getInstance(ShaderType shaderType); ShCompilerInstance getInstance(ShaderType shaderType);
void putInstance(ShCompilerInstance &&instance); void putInstance(ShCompilerInstance &&instance);
......
...@@ -1367,7 +1367,7 @@ Compiler *Context::getCompiler() const ...@@ -1367,7 +1367,7 @@ Compiler *Context::getCompiler() const
{ {
if (mCompiler.get() == nullptr) if (mCompiler.get() == nullptr)
{ {
mCompiler.set(this, new Compiler(mImplementation.get(), mState)); mCompiler.set(this, new Compiler(mImplementation.get(), mState, mDisplay));
} }
return mCompiler.get(); return mCompiler.get();
} }
......
...@@ -255,6 +255,8 @@ class Display final : public LabeledObject, ...@@ -255,6 +255,8 @@ class Display final : public LabeledObject,
egl::Error handleGPUSwitch(); egl::Error handleGPUSwitch();
std::mutex &getDisplayGlobalMutex() { return mDisplayGlobalMutex; }
private: private:
Display(EGLenum platform, EGLNativeDisplayType displayId, Device *eglDevice); Display(EGLenum platform, EGLNativeDisplayType displayId, Device *eglDevice);
...@@ -323,6 +325,8 @@ class Display final : public LabeledObject, ...@@ -323,6 +325,8 @@ class Display final : public LabeledObject,
std::mutex mScratchBufferMutex; std::mutex mScratchBufferMutex;
std::vector<angle::ScratchBuffer> mScratchBuffers; std::vector<angle::ScratchBuffer> mScratchBuffers;
std::vector<angle::ScratchBuffer> mZeroFilledBuffers; std::vector<angle::ScratchBuffer> mZeroFilledBuffers;
std::mutex mDisplayGlobalMutex;
}; };
} // namespace egl } // namespace egl
......
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