Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 13e1e3f

Browse files
committed
[Impeller] Fix a race between SwapchainImplVK::Present and WaitForFence
The previous implementation used an fml::CountDownLatch to synchronize between FrameSynchronizer::WaitForFence and the task queued by SwapchainImplVK::Present. fml::CountDownLatch:Wait always waits, even if the count is already zero. So the raster thread would deadlock if the present task signals the latch before FrameSynchronizer::WaitForFence enters the wait. This PR instead uses a semaphore that is set each time SwapchainImplVK::Present is called. FrameSynchronizer::WaitForFence will check the semaphore to determine whether a present task is pending and if so wait for its completion.
1 parent f63caee commit 13e1e3f

File tree

1 file changed

+15
-8
lines changed

1 file changed

+15
-8
lines changed

impeller/renderer/backend/vulkan/swapchain_impl_vk.cc

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
#include "impeller/renderer/backend/vulkan/swapchain_impl_vk.h"
66

7-
#include "fml/synchronization/count_down_latch.h"
7+
#include "fml/synchronization/semaphore.h"
88
#include "impeller/base/validation.h"
99
#include "impeller/renderer/backend/vulkan/command_buffer_vk.h"
1010
#include "impeller/renderer/backend/vulkan/command_encoder_vk.h"
@@ -32,12 +32,11 @@ struct FrameSynchronizer {
3232
std::shared_ptr<CommandBuffer> final_cmd_buffer;
3333
/// @brief A latch that is signaled _after_ a given swapchain image is
3434
/// presented.
35-
std::shared_ptr<fml::CountDownLatch> present_latch;
35+
std::shared_ptr<fml::Semaphore> present_semaphore;
3636
bool is_valid = false;
3737

3838
explicit FrameSynchronizer(const vk::Device& device) {
39-
auto acquire_res = device.createFenceUnique(
40-
vk::FenceCreateInfo{vk::FenceCreateFlagBits::eSignaled});
39+
auto acquire_res = device.createFenceUnique({});
4140
auto render_res = device.createSemaphoreUnique({});
4241
auto present_res = device.createSemaphoreUnique({});
4342
if (acquire_res.result != vk::Result::eSuccess ||
@@ -49,14 +48,21 @@ struct FrameSynchronizer {
4948
acquire = std::move(acquire_res.value);
5049
render_ready = std::move(render_res.value);
5150
present_ready = std::move(present_res.value);
52-
present_latch = std::make_shared<fml::CountDownLatch>(0u);
5351
is_valid = true;
5452
}
5553

5654
~FrameSynchronizer() = default;
5755

5856
bool WaitForFence(const vk::Device& device) {
59-
present_latch->Wait();
57+
if (!present_semaphore) {
58+
return true;
59+
}
60+
61+
if (!present_semaphore->Wait()) {
62+
VALIDATION_LOG << "Present semaphore wait failed";
63+
return false;
64+
}
65+
present_semaphore.reset();
6066
if (auto result = device.waitForFences(
6167
*acquire, // fence
6268
true, // wait all
@@ -71,7 +77,6 @@ struct FrameSynchronizer {
7177
VALIDATION_LOG << "Could not reset fence: " << vk::to_string(result);
7278
return false;
7379
}
74-
present_latch = std::make_shared<fml::CountDownLatch>(1u);
7580
return true;
7681
}
7782
};
@@ -505,7 +510,7 @@ bool SwapchainImplVK::Present(const std::shared_ptr<SwapchainImageVK>& image,
505510
present_info.setWaitSemaphores(*sync->present_ready);
506511

507512
auto result = present_queue_.presentKHR(present_info);
508-
sync->present_latch->CountDown();
513+
sync->present_semaphore->Signal();
509514

510515
switch (result) {
511516
case vk::Result::eErrorOutOfDateKHR:
@@ -529,6 +534,8 @@ bool SwapchainImplVK::Present(const std::shared_ptr<SwapchainImageVK>& image,
529534
}
530535
FML_UNREACHABLE();
531536
};
537+
538+
sync->present_semaphore = std::make_shared<fml::Semaphore>(0u);
532539
if (context.GetSyncPresentation()) {
533540
task();
534541
} else {

0 commit comments

Comments
 (0)