Vulkan: Add mutex around queueSubmit related data

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -583,6 +583,7 @@ angle::Result CommandQueue::submitFrame(vk::Context *context,
                                         egl::ContextPriority priority,
                                         const VkSubmitInfo &submitInfo,
                                         const vk::Shared<vk::Fence> &sharedFence,
+                                        vk::ResourceUseList *resourceList,
                                         vk::GarbageList *currentGarbage,
                                         vk::CommandPool *commandPool,
                                         vk::PrimaryCommandBuffer &&commandBuffer)
@@ -596,8 +597,8 @@ angle::Result CommandQueue::submitFrame(vk::Context *context,
     CommandBatch &batch = scopedBatch.get();
     batch.fence.copy(device, sharedFence);
 
-    ANGLE_TRY(
-        renderer->queueSubmit(context, priority, submitInfo, &batch.fence.get(), &batch.serial));
+    ANGLE_TRY(renderer->queueSubmit(context, priority, submitInfo, resourceList, &batch.fence.get(),
+                                    &batch.serial));
 
     if (!currentGarbage->empty())
     {
@@ -798,7 +799,7 @@ void ContextVk::onDestroy(const gl::Context *context)
 
     mCommandQueue.destroy(device);
 
-    mResourceUseList.releaseResourceUses();
+    mRenderer->releaseSharedResources(&mResourceUseList);
 
     mUtils.destroy(mRenderer);
 
@@ -1710,6 +1711,7 @@ void ContextVk::addOverlayUsedBuffersCount(vk::CommandBufferHelper *commandBuffe
 }
 
 angle::Result ContextVk::submitFrame(const VkSubmitInfo &submitInfo,
+                                     vk::ResourceUseList *resourceList,
                                      vk::PrimaryCommandBuffer &&commandBuffer)
 {
     if (vk::CommandBufferHelper::kEnableCommandStreamDiagnostics)
@@ -1719,7 +1721,8 @@ angle::Result ContextVk::submitFrame(const VkSubmitInfo &submitInfo,
 
     ANGLE_TRY(ensureSubmitFenceInitialized());
     ANGLE_TRY(mCommandQueue.submitFrame(this, mContextPriority, submitInfo, mSubmitFence,
-                                        &mCurrentGarbage, &mCommandPool, std::move(commandBuffer)));
+                                        resourceList, &mCurrentGarbage, &mCommandPool,
+                                        std::move(commandBuffer)));
 
     onRenderPassFinished();
     mComputeDirtyBits |= mNewComputeCommandBufferDirtyBits;
@@ -4207,16 +4210,13 @@ angle::Result ContextVk::flushImpl(const vk::Semaphore *signalSemaphore)
 
     ANGLE_VK_TRY(this, mPrimaryCommands.end());
 
-    Serial serial = getCurrentQueueSerial();
-    mResourceUseList.releaseResourceUsesAndUpdateSerials(serial);
-
     waitForSwapchainImageIfNecessary();
 
     VkSubmitInfo submitInfo = {};
     InitializeSubmitInfo(&submitInfo, mPrimaryCommands, mWaitSemaphores, mWaitSemaphoreStageMasks,
                          signalSemaphore);
 
-    ANGLE_TRY(submitFrame(submitInfo, std::move(mPrimaryCommands)));
+    ANGLE_TRY(submitFrame(submitInfo, &mResourceUseList, std::move(mPrimaryCommands)));
 
     ANGLE_TRY(startPrimaryCommandBuffer());
 

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -75,6 +75,7 @@ class CommandQueue final : angle::NonCopyable
                               egl::ContextPriority priority,
                               const VkSubmitInfo &submitInfo,
                               const vk::Shared<vk::Fence> &sharedFence,
+                              vk::ResourceUseList *resourceList,
                               vk::GarbageList *currentGarbage,
                               vk::CommandPool *commandPool,
                               vk::PrimaryCommandBuffer &&commandBuffer);
@@ -873,6 +874,7 @@ class ContextVk : public ContextImpl, public vk::Context
     void writeAtomicCounterBufferDriverUniformOffsets(uint32_t *offsetsOut, size_t offsetsSize);
 
     angle::Result submitFrame(const VkSubmitInfo &submitInfo,
+                              vk::ResourceUseList *resourceList,
                               vk::PrimaryCommandBuffer &&commandBuffer);
     angle::Result memoryBarrierImpl(GLbitfield barriers, VkPipelineStageFlags stageMask);
 

src/libANGLE/renderer/vulkan/RendererVk.cpp

@@ -489,6 +489,14 @@ bool RendererVk::hasSharedGarbage()
     return !mSharedGarbage.empty();
 }
 
+void RendererVk::releaseSharedResources(vk::ResourceUseList *resourceList)
+{
+    // resource list may access same resources referenced by garbage collection so need to protect
+    // that access with a lock.
+    std::lock_guard<std::mutex> lock(mGarbageMutex);
+    resourceList->releaseResourceUses();
+}
+
 void RendererVk::onDestroy()
 {
     if (getFeatures().enableCommandProcessingThread.enabled)
@@ -507,7 +515,11 @@ void RendererVk::onDestroy()
     }
 
     // Then assign an infinite "last completed" serial to force garbage to delete.
-    mLastCompletedQueueSerial = Serial::Infinite();
+    {
+        std::lock_guard<std::mutex> lock(mQueueSerialMutex);
+        mLastCompletedQueueSerial = Serial::Infinite();
+    }
+
     (void)cleanupGarbage(true);
     ASSERT(!hasSharedGarbage());
 
@@ -567,8 +579,11 @@ void RendererVk::onDestroy()
 
 void RendererVk::notifyDeviceLost()
 {
-    mLastCompletedQueueSerial = mLastSubmittedQueueSerial;
-    mDeviceLost               = true;
+    {
+        std::lock_guard<std::mutex> lock(mQueueSerialMutex);
+        mLastCompletedQueueSerial = mLastSubmittedQueueSerial;
+    }
+    mDeviceLost = true;
     mDisplay->notifyDeviceLost();
 }
 
@@ -2157,6 +2172,7 @@ bool RendererVk::hasBufferFormatFeatureBits(VkFormat format, const VkFormatFeatu
 angle::Result RendererVk::queueSubmit(vk::Context *context,
                                       egl::ContextPriority priority,
                                       const VkSubmitInfo &submitInfo,
+                                      vk::ResourceUseList *resourceList,
                                       const vk::Fence *fence,
                                       Serial *serialOut)
 {
@@ -2170,16 +2186,21 @@ angle::Result RendererVk::queueSubmit(vk::Context *context,
     }
     {
         std::lock_guard<decltype(mQueueMutex)> lock(mQueueMutex);
+        std::lock_guard<std::mutex> serialLock(mQueueSerialMutex);
         VkFence handle = fence ? fence->getHandle() : VK_NULL_HANDLE;
         ANGLE_VK_TRY(context, vkQueueSubmit(mQueues[priority], 1, &submitInfo, handle));
+
+        if (resourceList)
+        {
+            resourceList->releaseResourceUsesAndUpdateSerials(mCurrentQueueSerial);
+        }
+        *serialOut                = mCurrentQueueSerial;
+        mLastSubmittedQueueSerial = mCurrentQueueSerial;
+        mCurrentQueueSerial       = mQueueSerialFactory.generate();
     }
 
     ANGLE_TRY(cleanupGarbage(false));
 
-    *serialOut                = mCurrentQueueSerial;
-    mLastSubmittedQueueSerial = mCurrentQueueSerial;
-    mCurrentQueueSerial       = mQueueSerialFactory.generate();
-
     return angle::Result::Continue;
 }
 
@@ -2194,7 +2215,7 @@ angle::Result RendererVk::queueSubmitOneOff(vk::Context *context,
     submitInfo.commandBufferCount = 1;
     submitInfo.pCommandBuffers    = primary.ptr();
 
-    ANGLE_TRY(queueSubmit(context, priority, submitInfo, fence, serialOut));
+    ANGLE_TRY(queueSubmit(context, priority, submitInfo, nullptr, fence, serialOut));
 
     mPendingOneOffCommands.push_back({*serialOut, std::move(primary)});
 
@@ -2321,13 +2342,14 @@ bool RendererVk::hasFormatFeatureBits(VkFormat format, const VkFormatFeatureFlag
 
 angle::Result RendererVk::cleanupGarbage(bool block)
 {
+    Serial lastCompletedQueueSerial = getLastCompletedQueueSerial();
     std::lock_guard<std::mutex> lock(mGarbageMutex);
 
     for (auto garbageIter = mSharedGarbage.begin(); garbageIter != mSharedGarbage.end();)
     {
         // Possibly 'counter' should be always zero when we add the object to garbage.
         vk::SharedGarbage &garbage = *garbageIter;
-        if (garbage.destroyIfComplete(this, mLastCompletedQueueSerial))
+        if (garbage.destroyIfComplete(this, lastCompletedQueueSerial))
         {
             garbageIter = mSharedGarbage.erase(garbageIter);
         }
@@ -2363,6 +2385,7 @@ uint64_t RendererVk::getMaxFenceWaitTimeNs() const
 
 void RendererVk::onCompletedSerial(Serial serial)
 {
+    std::lock_guard<std::mutex> lock(mQueueSerialMutex);
     if (serial > mLastCompletedQueueSerial)
     {
         mLastCompletedQueueSerial = serial;
@@ -2396,7 +2419,7 @@ angle::Result RendererVk::getCommandBufferOneOff(vk::Context *context,
     }
 
     if (!mPendingOneOffCommands.empty() &&
-        mPendingOneOffCommands.front().serial < mLastCompletedQueueSerial)
+        mPendingOneOffCommands.front().serial < getLastCompletedQueueSerial())
     {
         *commandBufferOut = std::move(mPendingOneOffCommands.front().commandBuffer);
         mPendingOneOffCommands.pop_front();

src/libANGLE/renderer/vulkan/RendererVk.h

@@ -84,6 +84,7 @@ class RendererVk : angle::NonCopyable
     void notifyDeviceLost();
     bool isDeviceLost() const;
     bool hasSharedGarbage();
+    void releaseSharedResources(vk::ResourceUseList *resourceList);
 
     std::string getVendorString() const;
     std::string getRendererDescription() const;
@@ -180,6 +181,7 @@ class RendererVk : angle::NonCopyable
     angle::Result queueSubmit(vk::Context *context,
                               egl::ContextPriority priority,
                               const VkSubmitInfo &submitInfo,
+                              vk::ResourceUseList *resourceList,
                               const vk::Fence *fence,
                               Serial *serialOut);
     angle::Result queueWaitIdle(vk::Context *context, egl::ContextPriority priority);
@@ -246,9 +248,22 @@ class RendererVk : angle::NonCopyable
     std::string getAndClearLastValidationMessage(uint32_t *countSinceLastClear);
 
     uint64_t getMaxFenceWaitTimeNs() const;
-    Serial getCurrentQueueSerial() const { return mCurrentQueueSerial; }
-    Serial getLastSubmittedQueueSerial() const { return mLastSubmittedQueueSerial; }
-    Serial getLastCompletedQueueSerial() const { return mLastCompletedQueueSerial; }
+
+    ANGLE_INLINE Serial getCurrentQueueSerial()
+    {
+        std::lock_guard<std::mutex> lock(mQueueSerialMutex);
+        return mCurrentQueueSerial;
+    }
+    ANGLE_INLINE Serial getLastSubmittedQueueSerial()
+    {
+        std::lock_guard<std::mutex> lock(mQueueSerialMutex);
+        return mLastSubmittedQueueSerial;
+    }
+    ANGLE_INLINE Serial getLastCompletedQueueSerial()
+    {
+        std::lock_guard<std::mutex> lock(mQueueSerialMutex);
+        return mLastCompletedQueueSerial;
+    }
 
     void onCompletedSerial(Serial serial);
 
@@ -334,6 +349,7 @@ class RendererVk : angle::NonCopyable
     AtomicSerialFactory mQueueSerialFactory;
     AtomicSerialFactory mShaderSerialFactory;
 
+    std::mutex mQueueSerialMutex;
     Serial mLastCompletedQueueSerial;
     Serial mLastSubmittedQueueSerial;
     Serial mCurrentQueueSerial;

src/libANGLE/renderer/vulkan/SyncVk.cpp

@@ -192,7 +192,7 @@ angle::Result SyncHelperNativeFence::initializeWithFd(ContextVk *contextVk, int
         Serial serialOut;
         VkSubmitInfo submitInfo = {};
         submitInfo.sType        = VK_STRUCTURE_TYPE_SUBMIT_INFO;
-        ANGLE_TRY(renderer->queueSubmit(contextVk, contextVk->getPriority(), submitInfo,
+        ANGLE_TRY(renderer->queueSubmit(contextVk, contextVk->getPriority(), submitInfo, nullptr,
                                         &fence.get(), &serialOut));
 
         VkFenceGetFdInfoKHR fenceGetFdInfo = {};

src/tests/gl_tests/MultithreadingTest.cpp

@@ -5,8 +5,8 @@
 //
 // MulithreadingTest.cpp : Tests of multithreaded rendering
 
+#include "platform/FeaturesVk.h"
 #include "test_utils/ANGLETest.h"
-
 #include "test_utils/gl_raii.h"
 #include "util/EGLWindow.h"
 
@@ -202,6 +202,111 @@ TEST_P(MultithreadingTest, MultiContextDraw)
     runMultithreadedGLTest(testBody, 4);
 }
 
+// Test that multiple threads can draw and read back pixels correctly.
+// Using eglSwapBuffers stresses race conditions around use of QueueSerials.
+TEST_P(MultithreadingTest, MultiContextDrawWithSwapBuffers)
+{
+    ANGLE_SKIP_TEST_IF(!platformSupportsMultithreading());
+
+    EGLWindow *window = getEGLWindow();
+    EGLDisplay dpy    = window->getDisplay();
+
+    auto testBody = [dpy](EGLSurface surface, size_t thread) {
+        constexpr size_t kIterationsPerThread = 100;
+        constexpr size_t kDrawsPerIteration   = 10;
+
+        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++)
+        {
+            // Base the clear color on the thread and iteration indexes so every clear color is
+            // unique
+            const GLColor color(static_cast<GLubyte>(thread % 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++)
+            {
+                glDrawArrays(GL_TRIANGLES, 0, 6);
+            }
+
+            EXPECT_EGL_TRUE(eglSwapBuffers(dpy, surface));
+            EXPECT_EGL_SUCCESS();
+
+            EXPECT_PIXEL_COLOR_EQ(0, 0, color);
+        }
+    };
+    runMultithreadedGLTest(testBody, 32);
+}
+
+// Test that ANGLE handles multiple threads creating and destroying resources (vertex buffer in this
+// case). Disable defer_flush_until_endrenderpass so that glFlush will issue work to GPU in order to
+// maximize the chance we resources can be destroyed at the wrong time.
+TEST_P(MultithreadingTest, MultiContextCreateAndDeleteResources)
+{
+    ANGLE_SKIP_TEST_IF(!platformSupportsMultithreading());
+
+    EGLWindow *window = getEGLWindow();
+    EGLDisplay dpy    = window->getDisplay();
+
+    auto testBody = [dpy](EGLSurface surface, size_t thread) {
+        constexpr size_t kIterationsPerThread = 32;
+        constexpr size_t kDrawsPerIteration   = 1;
+
+        ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::UniformColor());
+        glUseProgram(program);
+
+        GLint colorLocation = glGetUniformLocation(program, essl1_shaders::ColorUniform());
+
+        auto quadVertices = GetQuadVertices();
+
+        for (size_t iteration = 0; iteration < kIterationsPerThread; iteration++)
+        {
+            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);
+
+            // Base the clear color on the thread and iteration indexes so every clear color is
+            // unique
+            const GLColor color(static_cast<GLubyte>(thread % 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++)
+            {
+                glDrawArrays(GL_TRIANGLES, 0, 6);
+            }
+
+            EXPECT_EGL_TRUE(eglSwapBuffers(dpy, surface));
+            EXPECT_EGL_SUCCESS();
+
+            EXPECT_PIXEL_COLOR_EQ(0, 0, color);
+        }
+        glFinish();
+    };
+    runMultithreadedGLTest(testBody, 32);
+}
+
 TEST_P(MultithreadingTest, MultiCreateContext)
 {
     // Supported by CGL, GLX, and WGL (https://anglebug.com/4725)