Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pipeline/shader race-condition-during-shutdown crash fix #18183

Merged
merged 3 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions Common/GPU/Vulkan/VulkanImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,12 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int depth, i

res = vkCreateImageView(vulkan_->GetDevice(), &view_info, NULL, &view_);
if (res != VK_SUCCESS) {
ERROR_LOG(G3D, "vkCreateImageView failed: %s", VulkanResultToString(res));
// This leaks the image.
ERROR_LOG(G3D, "vkCreateImageView failed: %s. Destroying image.", VulkanResultToString(res));
_assert_(res == VK_ERROR_OUT_OF_HOST_MEMORY || res == VK_ERROR_OUT_OF_DEVICE_MEMORY || res == VK_ERROR_TOO_MANY_OBJECTS);
vmaDestroyImage(vulkan_->Allocator(), image_, allocation_);
view_ = VK_NULL_HANDLE;
image_ = VK_NULL_HANDLE;
allocation_ = VK_NULL_HANDLE;
return false;
}
vulkan_->SetDebugName(view_, VK_OBJECT_TYPE_IMAGE_VIEW, tag_);
Expand All @@ -141,6 +144,7 @@ bool VulkanTexture::CreateDirect(VkCommandBuffer cmd, int w, int h, int depth, i
if (view_info.viewType == VK_IMAGE_VIEW_TYPE_2D) {
view_info.viewType = VK_IMAGE_VIEW_TYPE_2D_ARRAY;
res = vkCreateImageView(vulkan_->GetDevice(), &view_info, NULL, &arrayView_);
// Assume that if the above view creation succeeded, so will this.
_assert_(res == VK_SUCCESS);
vulkan_->SetDebugName(arrayView_, VK_OBJECT_TYPE_IMAGE_VIEW, tag_);
}
Expand Down
12 changes: 12 additions & 0 deletions Common/GPU/Vulkan/VulkanRenderManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ using namespace PPSSPP_VK;

// renderPass is an example of the "compatibility class" or RenderPassType type.
bool VKRGraphicsPipeline::Create(VulkanContext *vulkan, VkRenderPass compatibleRenderPass, RenderPassType rpType, VkSampleCountFlagBits sampleCount, double scheduleTime, int countToCompile) {
// Good torture test to test the shutdown-while-precompiling-shaders issue on PC where it's normally
// hard to catch because shaders compile so fast.
// sleep_ms(200);

bool multisample = RenderPassTypeHasMultisample(rpType);
if (multisample) {
if (sampleCount_ != VK_SAMPLE_COUNT_FLAG_BITS_MAX_ENUM) {
Expand Down Expand Up @@ -195,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
9 changes: 4 additions & 5 deletions GPU/Vulkan/TextureCacheVulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -474,11 +474,6 @@ void TextureCacheVulkan::BuildTexture(TexCacheEntry *const entry) {

delete entry->vkTex;

char texName[64]{};
snprintf(texName, sizeof(texName), "tex_%08x_%s", entry->addr, GeTextureFormatToString((GETextureFormat)entry->format, gstate.getClutPaletteFormat()));
entry->vkTex = new VulkanTexture(vulkan, texName);
VulkanTexture *image = entry->vkTex;

VkImageLayout imageLayout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL;
VkImageUsageFlags usage = VK_IMAGE_USAGE_TRANSFER_SRC_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_SAMPLED_BIT;

Expand Down Expand Up @@ -507,6 +502,10 @@ void TextureCacheVulkan::BuildTexture(TexCacheEntry *const entry) {
default: mapping = &VULKAN_8888_SWIZZLE; break; // no swizzle
}

char texName[64]{};
snprintf(texName, sizeof(texName), "tex_%08x_%s", entry->addr, GeTextureFormatToString((GETextureFormat)entry->format, gstate.getClutPaletteFormat()));
entry->vkTex = new VulkanTexture(vulkan, texName);
VulkanTexture *image = entry->vkTex;
bool allocSuccess = image->CreateDirect(cmdInit, plan.createW, plan.createH, plan.depth, plan.levelsToCreate, actualFmt, imageLayout, usage, mapping);
if (!allocSuccess && !lowMemoryMode_) {
WARN_LOG_REPORT(G3D, "Texture cache ran out of GPU memory; switching to low memory mode");
Expand Down