Commit 8b9889bf by Doug Horn Committed by Commit Bot

Fix multithreaded crash on draw commands on D3D11 backend.

A crash can occur if thread A is executing eglDestroyContext while thread B issues a draw call, if the threads are interleaved in such a manner that a makeCurrent occurs without triggering a change to the global context and a dirtyAllState call. We handle that case by explicitly making current the proper contexts in the eglDestroyContext call. A test has been added that triggers a crash without this fix when running on the D3D11 backend. In addition, all of MultithreadingTest is enabled for the D3D11 backend. Test: Ran MultithreadingTest. Test exhibits a crash before this change, and does not after this change. Also ran: dEQP-EGL.functional.sharing.gles2.multithread.* dEQP-EGL.functional.multithread.* Bug: b/183756357 Change-Id: Ic6f76a062868b2f3b4e60d29dc087ec180bfb7cd Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2798591Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Doug Horn <doughorn@google.com>
parent d3e1a7ff
...@@ -1440,6 +1440,17 @@ Error Display::releaseContext(gl::Context *context) ...@@ -1440,6 +1440,17 @@ Error Display::releaseContext(gl::Context *context)
Error Display::destroyContext(const Thread *thread, gl::Context *context) Error Display::destroyContext(const Thread *thread, gl::Context *context)
{ {
return destroyContextWithSurfaces(thread, context, thread->getContext(),
thread->getCurrentDrawSurface(),
thread->getCurrentReadSurface());
}
Error Display::destroyContextWithSurfaces(const Thread *thread,
gl::Context *context,
gl::Context *currentContext,
Surface *currentDrawSurface,
Surface *currentReadSurface)
{
size_t refCount = context->getRefCount(); size_t refCount = context->getRefCount();
if (refCount > 1) if (refCount > 1)
{ {
...@@ -1448,9 +1459,6 @@ Error Display::destroyContext(const Thread *thread, gl::Context *context) ...@@ -1448,9 +1459,6 @@ Error Display::destroyContext(const Thread *thread, gl::Context *context)
} }
// This is the last reference for this context, so we can destroy it now. // This is the last reference for this context, so we can destroy it now.
gl::Context *currentContext = thread->getContext();
Surface *currentDrawSurface = thread->getCurrentDrawSurface();
Surface *currentReadSurface = thread->getCurrentReadSurface();
bool changeContextForDeletion = context != currentContext; bool changeContextForDeletion = context != currentContext;
// For external context, we cannot change the current native context, and the API user should // For external context, we cannot change the current native context, and the API user should
......
...@@ -180,6 +180,11 @@ class Display final : public LabeledObject, ...@@ -180,6 +180,11 @@ class Display final : public LabeledObject,
void destroyImage(Image *image); void destroyImage(Image *image);
void destroyStream(Stream *stream); void destroyStream(Stream *stream);
Error destroyContext(const Thread *thread, gl::Context *context); Error destroyContext(const Thread *thread, gl::Context *context);
Error destroyContextWithSurfaces(const Thread *thread,
gl::Context *context,
gl::Context *currentContext,
Surface *currentDrawSurface,
Surface *currentReadSurface);
void destroySync(Sync *sync); void destroySync(Sync *sync);
bool isInitialized() const; bool isInitialized() const;
......
...@@ -275,12 +275,31 @@ EGLSurface CreateWindowSurface(Thread *thread, ...@@ -275,12 +275,31 @@ EGLSurface CreateWindowSurface(Thread *thread,
EGLBoolean DestroyContext(Thread *thread, Display *display, gl::Context *context) EGLBoolean DestroyContext(Thread *thread, Display *display, gl::Context *context)
{ {
ANGLE_EGL_TRY_RETURN(thread, display->prepareForCall(), "eglDestroyContext", ANGLE_EGL_TRY_RETURN(thread, display->prepareForCall(), "eglDestroyContext",
GetDisplayIfValid(display), EGL_FALSE); GetDisplayIfValid(display), EGL_FALSE);
bool contextWasCurrent = context == thread->getContext();
ANGLE_EGL_TRY_RETURN(thread, display->destroyContext(thread, context), "eglDestroyContext", gl::Context *contextForThread = thread->getContext();
GetContextIfValid(display, context), EGL_FALSE); bool contextWasCurrent = context == contextForThread;
bool shouldMakeCurrent =
!contextWasCurrent && !context->isExternal() && context->getRefCount() <= 1;
// Display can't access the current global context, but does exhibit a context switch,
// so ensuring the current global context is correct needs to happen here.
Surface *currentDrawSurface = thread->getCurrentDrawSurface();
Surface *currentReadSurface = thread->getCurrentReadSurface();
if (shouldMakeCurrent)
{
SetContextCurrent(thread, context);
}
ANGLE_EGL_TRY_RETURN(
thread,
display->destroyContextWithSurfaces(thread, context, contextForThread, currentDrawSurface,
currentReadSurface),
"eglDestroyContext", GetContextIfValid(display, context), EGL_FALSE);
if (contextWasCurrent) if (contextWasCurrent)
{ {
...@@ -288,6 +307,10 @@ EGLBoolean DestroyContext(Thread *thread, Display *display, gl::Context *context ...@@ -288,6 +307,10 @@ EGLBoolean DestroyContext(Thread *thread, Display *display, gl::Context *context
"eglDestroyContext", GetContextIfValid(display, context), EGL_FALSE); "eglDestroyContext", GetContextIfValid(display, context), EGL_FALSE);
SetContextCurrent(thread, nullptr); SetContextCurrent(thread, nullptr);
} }
else if (shouldMakeCurrent)
{
SetContextCurrent(thread, contextForThread);
}
thread->setSuccess(); thread->setSuccess();
return EGL_TRUE; return EGL_TRUE;
......
...@@ -21,7 +21,8 @@ void RegisterContextCompatibilityTests(); ...@@ -21,7 +21,8 @@ void RegisterContextCompatibilityTests();
// If we ever move to a text-based expectations format, we should move this list in that file. // If we ever move to a text-based expectations format, we should move this list in that file.
namespace namespace
{ {
const char *kSlowTests[] = {"GLSLTest.VerifyMaxVertexUniformVectors*"}; const char *kSlowTests[] = {"GLSLTest.VerifyMaxVertexUniformVectors*",
"MultiThreadingTest.MultiContextDeleteDraw*"};
} // namespace } // namespace
int main(int argc, char **argv) int main(int argc, char **argv)
......
...@@ -57,6 +57,20 @@ class EGLMultiContextTest : public ANGLETest ...@@ -57,6 +57,20 @@ class EGLMultiContextTest : public ANGLETest
GLuint mTexture; GLuint mTexture;
}; };
// Test that calling eglDeleteContext on a context that is not current succeeds.
TEST_P(EGLMultiContextTest, TestContextDestroySimple)
{
EGLWindow *window = getEGLWindow();
EGLDisplay dpy = window->getDisplay();
EGLContext context1 = window->createContext(EGL_NO_CONTEXT);
EGLContext context2 = window->createContext(EGL_NO_CONTEXT);
EXPECT_EGL_TRUE(eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, context1));
EXPECT_EGL_TRUE(eglDestroyContext(dpy, context2));
EXPECT_EGL_SUCCESS();
}
// Test that a compute shader running in one thread will still work when rendering is happening in // Test that a compute shader running in one thread will still work when rendering is happening in
// another thread (with non-shared contexts). The non-shared context will still share a Vulkan // another thread (with non-shared contexts). The non-shared context will still share a Vulkan
// command buffer. // command buffer.
......
...@@ -215,6 +215,80 @@ TEST_P(MultithreadingTest, MultiContextClear) ...@@ -215,6 +215,80 @@ TEST_P(MultithreadingTest, MultiContextClear)
runMultithreadedGLTest(testBody, 72); runMultithreadedGLTest(testBody, 72);
} }
// Verify that threads can interleave eglDestroyContext and draw calls without
// any crashes.
TEST_P(MultithreadingTest, MultiContextDeleteDraw)
{
// Skip this test on non-D3D11 backends, as it has the potential to time-out
// and this test was originally intended to catch a crash on the D3D11 backend.
ANGLE_SKIP_TEST_IF(!platformSupportsMultithreading());
ANGLE_SKIP_TEST_IF(!IsD3D11());
EGLWindow *window = getEGLWindow();
EGLDisplay dpy = window->getDisplay();
EGLConfig config = window->getConfig();
std::thread t1 = std::thread([&]() {
// 5000 is chosen here as it reliably reproduces the former crash.
for (int i = 0; i < 5000; i++)
{
EGLContext ctx1 = window->createContext(EGL_NO_CONTEXT);
EGLContext ctx2 = window->createContext(EGL_NO_CONTEXT);
EXPECT_EGL_TRUE(eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, ctx2));
EXPECT_EGL_TRUE(eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, ctx1));
EXPECT_EGL_TRUE(eglDestroyContext(dpy, ctx2));
EXPECT_EGL_TRUE(eglDestroyContext(dpy, ctx1));
}
});
std::thread t2 = std::thread([&]() {
EGLint pbufferAttributes[] = {
EGL_WIDTH, 256, EGL_HEIGHT, 256, EGL_NONE, EGL_NONE,
};
EGLSurface surface = eglCreatePbufferSurface(dpy, config, pbufferAttributes);
EXPECT_EGL_SUCCESS();
auto ctx = window->createContext(EGL_NO_CONTEXT);
EXPECT_EGL_TRUE(eglMakeCurrent(dpy, surface, surface, ctx));
constexpr size_t kIterationsPerThread = 512;
constexpr size_t kDrawsPerIteration = 512;
ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::UniformColor());
glUseProgram(program);
GLint colorLocation = glGetUniformLocation(program, essl1_shaders::ColorUniform());
auto quadVertices = GetQuadVertices();
GLBuffer vertexBuffer;
glBindBuffer(GL_ARRAY_BUFFER, vertexBuffer);
glBufferData(GL_ARRAY_BUFFER, sizeof(GLfloat) * 3 * 6, quadVertices.data(), GL_STATIC_DRAW);
GLint positionLocation = glGetAttribLocation(program, essl1_shaders::PositionAttrib());
glEnableVertexAttribArray(positionLocation);
glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, 0);
for (size_t iteration = 0; iteration < kIterationsPerThread; iteration++)
{
const GLColor color(static_cast<GLubyte>(15151 % 255),
static_cast<GLubyte>(iteration % 255), 0, 255);
const angle::Vector4 floatColor = color.toNormalizedVector();
glUniform4fv(colorLocation, 1, floatColor.data());
for (size_t draw = 0; draw < kDrawsPerIteration; draw++)
{
EXPECT_EGL_TRUE(eglMakeCurrent(dpy, surface, surface, ctx));
glDrawArrays(GL_TRIANGLES, 0, 6);
}
}
});
t1.join();
t2.join();
}
// Test that multiple threads can draw and readback pixels successfully at the same time // Test that multiple threads can draw and readback pixels successfully at the same time
TEST_P(MultithreadingTest, MultiContextDraw) TEST_P(MultithreadingTest, MultiContextDraw)
{ {
...@@ -701,13 +775,16 @@ ANGLE_INSTANTIATE_TEST(MultithreadingTest, ...@@ -701,13 +775,16 @@ ANGLE_INSTANTIATE_TEST(MultithreadingTest,
WithNoVirtualContexts(ES2_OPENGLES()), WithNoVirtualContexts(ES2_OPENGLES()),
WithNoVirtualContexts(ES3_OPENGLES()), WithNoVirtualContexts(ES3_OPENGLES()),
WithNoVirtualContexts(ES3_VULKAN()), WithNoVirtualContexts(ES3_VULKAN()),
WithNoVirtualContexts(ES3_VULKAN_SWIFTSHADER())); WithNoVirtualContexts(ES3_VULKAN_SWIFTSHADER()),
WithNoVirtualContexts(ES2_D3D11()),
WithNoVirtualContexts(ES3_D3D11()));
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(MultithreadingTestES3); GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(MultithreadingTestES3);
ANGLE_INSTANTIATE_TEST(MultithreadingTestES3, ANGLE_INSTANTIATE_TEST(MultithreadingTestES3,
WithNoVirtualContexts(ES3_OPENGL()), WithNoVirtualContexts(ES3_OPENGL()),
WithNoVirtualContexts(ES3_OPENGLES()), WithNoVirtualContexts(ES3_OPENGLES()),
WithNoVirtualContexts(ES3_VULKAN()), WithNoVirtualContexts(ES3_VULKAN()),
WithNoVirtualContexts(ES3_VULKAN_SWIFTSHADER())); WithNoVirtualContexts(ES3_VULKAN_SWIFTSHADER()),
WithNoVirtualContexts(ES3_D3D11()));
} // namespace angle } // namespace angle
...@@ -1080,7 +1080,7 @@ void ANGLETestBase::draw3DTexturedQuad(GLfloat positionAttribZ, ...@@ -1080,7 +1080,7 @@ void ANGLETestBase::draw3DTexturedQuad(GLfloat positionAttribZ,
bool ANGLETestBase::platformSupportsMultithreading() const bool ANGLETestBase::platformSupportsMultithreading() const
{ {
return (IsOpenGLES() && IsAndroid()) || IsVulkan(); return (IsOpenGLES() && IsAndroid()) || IsVulkan() || IsD3D11();
} }
void ANGLETestBase::checkD3D11SDKLayersMessages() void ANGLETestBase::checkD3D11SDKLayersMessages()
......
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