[Impeller] Terminate the fence waiter but do not reset it during ContextVK shutdown (#173085)

ContextVK::Shutdown was clearing the context's fence_waiter_. This could
cause crashes if a pending task such as image decoding is still holding
a reference to the context after ContextVK::Shutdown is called.

The image decode task will submit a command buffer through the context,
and CommandQueueVK::Submit will get a null pointer deference when it
tries to use the fence waiter.

This PR changes ContextVK::Shutdown to terminate the fence waiter
instead of clearing it. FenceWaiterVK::Terminate will now stop the
waiter thread and wait for the thread to exit.

This ensures that the fence waiter thread is stopped in
ContextVK::Shutdown even if something else is holding a reference to the
FenceWaiterVK. Tasks like image decoding will now get an error result
instead of a crash when submitting a command buffer after context
shutdown.
This commit is contained in:
Jason Simmons 2025-08-04 10:58:57 -07:00 committed by GitHub
parent 69264e4a60
commit 2831e61a9d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 15 additions and 6 deletions

View File

@ -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();

View File

@ -59,7 +59,6 @@ FenceWaiterVK::FenceWaiterVK(std::weak_ptr<DeviceHolderVK> 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

View File

@ -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