Reland "Fix multithreaded crash on draw commands on D3D11 backend."

src/libANGLE/Display.cpp

@@ -1440,6 +1440,17 @@ Error Display::releaseContext(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();
     if (refCount > 1)
     {
@@ -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.
-    gl::Context *currentContext   = thread->getContext();
-    Surface *currentDrawSurface   = thread->getCurrentDrawSurface();
-    Surface *currentReadSurface   = thread->getCurrentReadSurface();
     bool changeContextForDeletion = context != currentContext;
 
     // For external context, we cannot change the current native context, and the API user should

src/libANGLE/Display.h

@@ -180,6 +180,11 @@ class Display final : public LabeledObject,
     void destroyImage(Image *image);
     void destroyStream(Stream *stream);
     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);
 
     bool isInitialized() const;

src/libGLESv2/egl_stubs.cpp

@@ -275,12 +275,31 @@ EGLSurface CreateWindowSurface(Thread *thread,
 
 EGLBoolean DestroyContext(Thread *thread, Display *display, gl::Context *context)
 {
+
     ANGLE_EGL_TRY_RETURN(thread, display->prepareForCall(), "eglDestroyContext",
                          GetDisplayIfValid(display), EGL_FALSE);
-    bool contextWasCurrent = context == thread->getContext();
 
-    ANGLE_EGL_TRY_RETURN(thread, display->destroyContext(thread, context), "eglDestroyContext",
-                         GetContextIfValid(display, context), EGL_FALSE);
+    gl::Context *contextForThread = thread->getContext();
+    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)
     {
@@ -288,6 +307,10 @@ EGLBoolean DestroyContext(Thread *thread, Display *display, gl::Context *context
                              "eglDestroyContext", GetContextIfValid(display, context), EGL_FALSE);
         SetContextCurrent(thread, nullptr);
     }
+    else if (shouldMakeCurrent)
+    {
+        SetContextCurrent(thread, contextForThread);
+    }
 
     thread->setSuccess();
     return EGL_TRUE;

src/tests/angle_end2end_tests_main.cpp

@@ -21,7 +21,8 @@ void RegisterContextCompatibilityTests();
 // If we ever move to a text-based expectations format, we should move this list in that file.
 namespace
 {
-const char *kSlowTests[] = {"GLSLTest.VerifyMaxVertexUniformVectors*"};
+const char *kSlowTests[] = {"GLSLTest.VerifyMaxVertexUniformVectors*",
+                            "MultiThreadingTest.MultiContextDeleteDraw*"};
 }  // namespace
 
 int main(int argc, char **argv)

src/tests/egl_tests/EGLMultiContextTest.cpp

@@ -57,6 +57,20 @@ class EGLMultiContextTest : public ANGLETest
     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
 // another thread (with non-shared contexts).  The non-shared context will still share a Vulkan
 // command buffer.

src/tests/gl_tests/MultithreadingTest.cpp

@@ -215,6 +215,80 @@ TEST_P(MultithreadingTest, MultiContextClear)
     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_P(MultithreadingTest, MultiContextDraw)
 {
@@ -701,13 +775,16 @@ ANGLE_INSTANTIATE_TEST(MultithreadingTest,
                        WithNoVirtualContexts(ES2_OPENGLES()),
                        WithNoVirtualContexts(ES3_OPENGLES()),
                        WithNoVirtualContexts(ES3_VULKAN()),
-                       WithNoVirtualContexts(ES3_VULKAN_SWIFTSHADER()));
+                       WithNoVirtualContexts(ES3_VULKAN_SWIFTSHADER()),
+                       WithNoVirtualContexts(ES2_D3D11()),
+                       WithNoVirtualContexts(ES3_D3D11()));
 
 GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(MultithreadingTestES3);
 ANGLE_INSTANTIATE_TEST(MultithreadingTestES3,
                        WithNoVirtualContexts(ES3_OPENGL()),
                        WithNoVirtualContexts(ES3_OPENGLES()),
                        WithNoVirtualContexts(ES3_VULKAN()),
-                       WithNoVirtualContexts(ES3_VULKAN_SWIFTSHADER()));
+                       WithNoVirtualContexts(ES3_VULKAN_SWIFTSHADER()),
+                       WithNoVirtualContexts(ES3_D3D11()));
 
 }  // namespace angle