Skip to content

Commit

Permalink
Fix a really bad race condition during game shutdown.
Browse files Browse the repository at this point in the history
  • Loading branch information
hrydgard committed Sep 20, 2023
1 parent b8353c6 commit 3783afd
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 1 deletion.
8 changes: 8 additions & 0 deletions Common/GPU/Vulkan/VulkanRenderManager.cpp
Expand Up @@ -199,6 +199,14 @@ VKRGraphicsPipeline::~VKRGraphicsPipeline() {
desc->Release();
}

void VKRGraphicsPipeline::BlockUntilCompiled() {
for (size_t i = 0; i < (size_t)RenderPassType::TYPE_COUNT; i++) {
if (pipeline[i]) {
pipeline[i]->BlockUntilReady();
}
}
}

void VKRGraphicsPipeline::QueueForDeletion(VulkanContext *vulkan) {
// Can't destroy variants here, the pipeline still lives for a while.
vulkan->Delete().QueueCallback([](VulkanContext *vulkan, void *p) {
Expand Down
4 changes: 4 additions & 0 deletions Common/GPU/Vulkan/VulkanRenderManager.h
Expand Up @@ -131,6 +131,10 @@ struct VKRGraphicsPipeline {
// This deletes the whole VKRGraphicsPipeline, you must remove your last pointer to it when doing this.
void QueueForDeletion(VulkanContext *vulkan);

// This blocks until any background compiles are finished.
// Used during game shutdown before we clear out shaders that these compiles depend on.
void BlockUntilCompiled();

u32 GetVariantsBitmask() const;

void LogCreationFailure() const;
Expand Down
6 changes: 5 additions & 1 deletion GPU/Vulkan/GPU_Vulkan.cpp
Expand Up @@ -186,12 +186,16 @@ GPU_Vulkan::~GPU_Vulkan() {
}

SaveCache(shaderCachePath_);

// Super important to delete pipeline manager FIRST, before clearing shaders, so we wait for all pending pipelines to finish compiling.
delete pipelineManager_;
pipelineManager_ = nullptr;

// Note: We save the cache in DeviceLost
DestroyDeviceObjects();
drawEngine_.DeviceLost();
shaderManager_->ClearShaders();

delete pipelineManager_;
// other managers are deleted in ~GPUCommonHW.

if (draw_) {
Expand Down
9 changes: 9 additions & 0 deletions GPU/Vulkan/PipelineManagerVulkan.cpp
Expand Up @@ -29,6 +29,15 @@ PipelineManagerVulkan::PipelineManagerVulkan(VulkanContext *vulkan) : pipelines_
}

PipelineManagerVulkan::~PipelineManagerVulkan() {
// Block on all pipelines to make sure any background compiles are done.
// This is very important to do before we start trying to tear down the shaders - otherwise, we might
// be deleting shaders before queued pipeline creations that use them are performed.
pipelines_.Iterate([&](const VulkanPipelineKey &key, VulkanPipeline *value) {
if (value->pipeline) {
value->pipeline->BlockUntilCompiled();
}
});

Clear();
if (pipelineCache_ != VK_NULL_HANDLE)
vulkan_->Delete().QueueDeletePipelineCache(pipelineCache_);
Expand Down

0 comments on commit 3783afd

Please sign in to comment.