From 89ba94902aa09a10ad52f5f9eeb330c96e23b2ac Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Wed, 25 Jun 2025 15:20:14 -0700 Subject: [PATCH] [Impeller] Make ContextVK hash values globally unique (#171119) ContextVK had been using a thread-local counter to assign the value returned by GetHash. But CommandPoolRecyclerVK was using these values as keys in a process-wide global map. If two engine instances using different task runner threads both created a ContextVK, then the thread-local counter could cause both contexts to share the same hash. So when CommandPoolRecyclerVK::DestroyThreadLocalPools destroys the pools associated with the first context's hash, it will also incorrectly destroy the second context's pools. Fixes https://github.com/flutter/flutter/issues/170141 --- .../renderer/backend/vulkan/context_vk.cc | 6 ++---- .../backend/vulkan/context_vk_unittests.cc | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc index a9a42ad35a5..d75db4942bb 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc @@ -119,11 +119,9 @@ size_t ContextVK::ChooseThreadCountForWorkers(size_t hardware_concurrency) { } namespace { -thread_local uint64_t tls_context_count = 0; +std::atomic_uint64_t context_count = 0; uint64_t CalculateHash(void* ptr) { - // You could make a context once per nanosecond for 584 years on one thread - // before this overflows. - return ++tls_context_count; + return context_count.fetch_add(1); } } // namespace diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc index 3de93ec63e5..61c0d2bee16 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc @@ -392,5 +392,21 @@ TEST(ContextVKTest, AHBSwapchainCapabilitiesCanBeMissing) { } // namespace impeller +TEST(ContextVKTest, HashIsUniqueAcrossThreads) { + uint64_t hash1, hash2; + std::thread thread1([&]() { + auto context = MockVulkanContextBuilder().Build(); + hash1 = context->GetHash(); + }); + std::thread thread2([&]() { + auto context = MockVulkanContextBuilder().Build(); + hash2 = context->GetHash(); + }); + thread1.join(); + thread2.join(); + + EXPECT_NE(hash1, hash2); +} + } // namespace testing } // namespace impeller