[Impeller] Make CreateMockVulkanContext() thread-safe (flutter#45687)

I started using `CreateMockVulkanContext()` in
flutter#45654, and discovered that it was
not thread-safe. This PR does a little bit of refactoring/TLC to ensure
that functions and buffers can be accessed across threads (like the
_real_ `ContextVK`).

The attached test (_test for a test fixture, hoorah!_) fails
consistently before the changes, and passes after. Note that if you try
testing this with `--gtest_repeat=10000` (at least that's the threshold
for me), it'll fail - that's actually due to
flutter/flutter#134482 which in turn is being
fixed in flutter#45686.

(Unblocks flutter/flutter#133198)

---------

Co-authored-by: gaaclarke <30870216+gaaclarke@users.noreply.github.com>

Author: matanlurey

impeller/renderer/backend/vulkan/BUILD.gn

@@ -15,6 +15,7 @@ impeller_component("vulkan_unittests") {
     "resource_manager_vk_unittests.cc",
     "test/mock_vulkan.cc",
     "test/mock_vulkan.h",
+    "test/mock_vulkan_unittests.cc",
   ]
   deps = [
     ":vulkan",

impeller/renderer/backend/vulkan/context_vk_unittests.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "flutter/testing/testing.h"
+#include "flutter/testing/testing.h"  // IWYU pragma: keep
 #include "impeller/renderer/backend/vulkan/command_pool_vk.h"
 #include "impeller/renderer/backend/vulkan/context_vk.h"
 #include "impeller/renderer/backend/vulkan/test/mock_vulkan.h"

impeller/renderer/backend/vulkan/test/mock_vulkan.cc

@@ -3,6 +3,9 @@
 // found in the LICENSE file.
 
 #include "impeller/renderer/backend/vulkan/test/mock_vulkan.h"
+#include <vector>
+#include "fml/macros.h"
+#include "impeller/base/thread_safety.h"
 
 namespace impeller {
 namespace testing {
@@ -16,17 +19,37 @@ struct MockCommandBuffer {
   std::shared_ptr<std::vector<std::string>> called_functions_;
 };
 
-struct MockDevice {
-  MockDevice() : called_functions_(new std::vector<std::string>()) {}
+class MockDevice final {
+ public:
+  explicit MockDevice() : called_functions_(new std::vector<std::string>()) {}
+
   MockCommandBuffer* NewCommandBuffer() {
-    std::unique_ptr<MockCommandBuffer> buffer =
-        std::make_unique<MockCommandBuffer>(called_functions_);
+    auto buffer = std::make_unique<MockCommandBuffer>(called_functions_);
     MockCommandBuffer* result = buffer.get();
+    Lock lock(command_buffers_mutex_);
     command_buffers_.emplace_back(std::move(buffer));
     return result;
   }
-  std::shared_ptr<std::vector<std::string>> called_functions_;
-  std::vector<std::unique_ptr<MockCommandBuffer>> command_buffers_;
+
+  const std::shared_ptr<std::vector<std::string>>& GetCalledFunctions() {
+    return called_functions_;
+  }
+
+  void AddCalledFunction(const std::string& function) {
+    Lock lock(called_functions_mutex_);
+    called_functions_->push_back(function);
+  }
+
+ private:
+  FML_DISALLOW_COPY_AND_ASSIGN(MockDevice);
+
+  Mutex called_functions_mutex_;
+  std::shared_ptr<std::vector<std::string>> called_functions_
+      IPLR_GUARDED_BY(called_functions_mutex_);
+
+  Mutex command_buffers_mutex_;
+  std::vector<std::unique_ptr<MockCommandBuffer>> command_buffers_
+      IPLR_GUARDED_BY(command_buffers_mutex_);
 };
 
 void noop() {}
@@ -147,7 +170,7 @@ VkResult vkCreatePipelineCache(VkDevice device,
                                const VkAllocationCallbacks* pAllocator,
                                VkPipelineCache* pPipelineCache) {
   MockDevice* mock_device = reinterpret_cast<MockDevice*>(device);
-  mock_device->called_functions_->push_back("vkCreatePipelineCache");
+  mock_device->AddCalledFunction("vkCreatePipelineCache");
   *pPipelineCache = reinterpret_cast<VkPipelineCache>(0xb000dead);
   return VK_SUCCESS;
 }
@@ -270,30 +293,30 @@ VkResult vkCreateGraphicsPipelines(
     const VkAllocationCallbacks* pAllocator,
     VkPipeline* pPipelines) {
   MockDevice* mock_device = reinterpret_cast<MockDevice*>(device);
-  mock_device->called_functions_->push_back("vkCreateGraphicsPipelines");
+  mock_device->AddCalledFunction("vkCreateGraphicsPipelines");
   *pPipelines = reinterpret_cast<VkPipeline>(0x99999999);
   return VK_SUCCESS;
 }
 
 void vkDestroyDevice(VkDevice device, const VkAllocationCallbacks* pAllocator) {
   MockDevice* mock_device = reinterpret_cast<MockDevice*>(device);
-  mock_device->called_functions_->push_back("vkDestroyDevice");
+  mock_device->AddCalledFunction("vkDestroyDevice");
   delete reinterpret_cast<MockDevice*>(device);
 }
 
 void vkDestroyPipeline(VkDevice device,
                        VkPipeline pipeline,
                        const VkAllocationCallbacks* pAllocator) {
   MockDevice* mock_device = reinterpret_cast<MockDevice*>(device);
-  mock_device->called_functions_->push_back("vkDestroyPipeline");
+  mock_device->AddCalledFunction("vkDestroyPipeline");
 }
 
 VkResult vkCreateShaderModule(VkDevice device,
                               const VkShaderModuleCreateInfo* pCreateInfo,
                               const VkAllocationCallbacks* pAllocator,
                               VkShaderModule* pShaderModule) {
   MockDevice* mock_device = reinterpret_cast<MockDevice*>(device);
-  mock_device->called_functions_->push_back("vkCreateShaderModule");
+  mock_device->AddCalledFunction("vkCreateShaderModule");
   *pShaderModule = reinterpret_cast<VkShaderModule>(0x11111111);
   return VK_SUCCESS;
 }
@@ -302,14 +325,14 @@ void vkDestroyShaderModule(VkDevice device,
                            VkShaderModule shaderModule,
                            const VkAllocationCallbacks* pAllocator) {
   MockDevice* mock_device = reinterpret_cast<MockDevice*>(device);
-  mock_device->called_functions_->push_back("vkDestroyShaderModule");
+  mock_device->AddCalledFunction("vkDestroyShaderModule");
 }
 
 void vkDestroyPipelineCache(VkDevice device,
                             VkPipelineCache pipelineCache,
                             const VkAllocationCallbacks* pAllocator) {
   MockDevice* mock_device = reinterpret_cast<MockDevice*>(device);
-  mock_device->called_functions_->push_back("vkDestroyPipelineCache");
+  mock_device->AddCalledFunction("vkDestroyPipelineCache");
 }
 
 void vkCmdBindPipeline(VkCommandBuffer commandBuffer,
@@ -351,14 +374,14 @@ void vkFreeCommandBuffers(VkDevice device,
                           uint32_t commandBufferCount,
                           const VkCommandBuffer* pCommandBuffers) {
   MockDevice* mock_device = reinterpret_cast<MockDevice*>(device);
-  mock_device->called_functions_->push_back("vkFreeCommandBuffers");
+  mock_device->AddCalledFunction("vkFreeCommandBuffers");
 }
 
 void vkDestroyCommandPool(VkDevice device,
                           VkCommandPool commandPool,
                           const VkAllocationCallbacks* pAllocator) {
   MockDevice* mock_device = reinterpret_cast<MockDevice*>(device);
-  mock_device->called_functions_->push_back("vkDestroyCommandPool");
+  mock_device->AddCalledFunction("vkDestroyCommandPool");
 }
 
 VkResult vkEndCommandBuffer(VkCommandBuffer commandBuffer) {
@@ -500,7 +523,7 @@ std::shared_ptr<ContextVK> CreateMockVulkanContext(void) {
 std::shared_ptr<std::vector<std::string>> GetMockVulkanFunctions(
     VkDevice device) {
   MockDevice* mock_device = reinterpret_cast<MockDevice*>(device);
-  return mock_device->called_functions_;
+  return mock_device->GetCalledFunctions();
 }
 
 }  // namespace testing

impeller/renderer/backend/vulkan/test/mock_vulkan_unittests.cc

@@ -0,0 +1,37 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/testing/testing.h"  // IWYU pragma: keep
+#include "gtest/gtest.h"
+#include "impeller/renderer/backend/vulkan/command_pool_vk.h"
+#include "impeller/renderer/backend/vulkan/test/mock_vulkan.h"
+
+namespace impeller {
+namespace testing {
+
+TEST(MockVulkanContextTest, IsThreadSafe) {
+  // In a typical app, there is a single ContextVK per app, shared b/w threads.
+  //
+  // This test ensures that the (mock) ContextVK is thread-safe.
+  auto const context = CreateMockVulkanContext();
+
+  // Spawn two threads, and have them create a CommandPoolVK each.
+  std::thread thread1([&context]() {
+    auto const pool = CommandPoolVK::GetThreadLocal(context.get());
+    EXPECT_TRUE(pool);
+  });
+
+  std::thread thread2([&context]() {
+    auto const pool = CommandPoolVK::GetThreadLocal(context.get());
+    EXPECT_TRUE(pool);
+  });
+
+  thread1.join();
+  thread2.join();
+
+  context->Shutdown();
+}
+
+}  // namespace testing
+}  // namespace impeller