From fec1efd104faa38061fe91c31ae0cea12a8c94e5 Mon Sep 17 00:00:00 2001 From: flutteractionsbot <154381524+flutteractionsbot@users.noreply.github.com> Date: Thu, 28 Aug 2025 10:41:54 -0700 Subject: [PATCH] [CP-stable][Impeller] Terminate the fence waiter but do not reset it during ContextVK shutdown (#174647) This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request) Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request. ### Issue Link: What is the link to the issue this cherry-pick is addressing? https://github.com/flutter/flutter/issues/171691 ### Changelog Description: Fixes a race that can cause crashes in the Impeller Vulkan back end. ### Impact Description: Crashes in apps running on Impeller/Vulkan. ### Workaround: Disable Impeller ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: The crash does not happen consistently. One way that I have been able to reproduce it is: * run Gallery with Impeller/Vulkan on a slow phone * scroll through the Shrine screen and start a lot of image decodes * quickly exit the app before the decodes have completed Without the fix, the app will often crash if an image decode tries to access the Vulkan context after it was partially shut down while exiting the app. --- .../impeller/renderer/backend/vulkan/context_vk.cc | 2 +- .../renderer/backend/vulkan/fence_waiter_vk.cc | 5 ++++- .../backend/vulkan/fence_waiter_vk_unittests.cc | 14 ++++++++++---- 3 files changed, 15 insertions(+), 6 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 d75db4942bb..3220f7ec946 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc @@ -601,7 +601,7 @@ void ContextVK::Shutdown() { // pointers ensures that cleanup happens in a correct order. // // tl;dr: Without it, we get thread::join failures on shutdown. - fence_waiter_.reset(); + fence_waiter_->Terminate(); resource_manager_.reset(); raster_message_loop_->Terminate(); diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.cc index ddd617a5803..73755abbc7e 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.cc @@ -59,7 +59,6 @@ FenceWaiterVK::FenceWaiterVK(std::weak_ptr device_holder) FenceWaiterVK::~FenceWaiterVK() { Terminate(); - waiter_thread_->join(); } bool FenceWaiterVK::AddFence(vk::UniqueFence fence, @@ -207,9 +206,13 @@ bool FenceWaiterVK::Wait() { void FenceWaiterVK::Terminate() { { std::scoped_lock lock(wait_set_mutex_); + if (terminate_) { + return; + } terminate_ = true; } wait_set_cv_.notify_one(); + waiter_thread_->join(); } } // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/fence_waiter_vk_unittests.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/fence_waiter_vk_unittests.cc index ef2ff5d40cb..6998dbc2da9 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/fence_waiter_vk_unittests.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/fence_waiter_vk_unittests.cc @@ -116,14 +116,20 @@ TEST(FenceWaiterVKTest, InProgressFencesStillWaitIfTerminated) { raw_fence = MockFence::GetRawPointer(fence); waiter->AddFence(std::move(fence), [&signal]() { signal.Signal(); }); - // Terminate the waiter. - waiter->Terminate(); + // Signal the fence after a delay. + std::thread thread([&]() { + std::this_thread::sleep_for(std::chrono::milliseconds{100u}); + raw_fence->SetStatus(vk::Result::eSuccess); + }); - // Signal the fence. - raw_fence->SetStatus(vk::Result::eSuccess); + // Terminate the waiter. This will block until all pending fences are + // signalled. + waiter->Terminate(); // This will hang if the fence was not signalled. signal.Wait(); + + thread.join(); } } // namespace testing