From 0df4753ba63aa187b8769cf38deea09cf8830ef4 Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Sat, 25 Jan 2020 09:35:50 +0000 Subject: [PATCH] Vulkan: Fix crashes on Device destruction if Device::Initialize fails If Device creation fails, several things are just partially initialized and the destroy sequence crashes dereferencing null data. This commit marks the Vulkan device as lost until after it is created. This avoids parts of the destroy sequence which are unecessary since the Device was never successfully created and no commands are in flight. Bug: chromium:1043095 Change-Id: I8e121709fa19b215e118a615b639380d1db1f3f2 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15460 Reviewed-by: Kai Ninomiya Commit-Queue: Corentin Wallez --- src/dawn_native/vulkan/DeviceVk.cpp | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp index 7ddf0bea78..f31ea969c6 100644 --- a/src/dawn_native/vulkan/DeviceVk.cpp +++ b/src/dawn_native/vulkan/DeviceVk.cpp @@ -50,6 +50,9 @@ namespace dawn_native { namespace vulkan { if (descriptor != nullptr) { ApplyToggleOverrides(descriptor); } + + // Set the device as lost until successfully created. + mLossStatus = LossStatus::AlreadyLost; } MaybeError Device::Initialize() { @@ -91,14 +94,22 @@ namespace dawn_native { namespace vulkan { mDescriptorSetService = nullptr; + // The frontend asserts DynamicUploader is destructed by the backend. + // It is usually destructed in Destroy(), but Destroy isn't always called if device + // initialization failed. + mDynamicUploader = nullptr; + // We still need to properly handle Vulkan object deletion even if the device has been lost, // so the Deleter and vkDevice cannot be destroyed in Device::Destroy(). // We need handle deleting all child objects by calling Tick() again with a large serial to // force all operations to look as if they were completed, and delete all objects before // destroying the Deleter and vkDevice. - mCompletedSerial = std::numeric_limits::max(); - mDeleter->Tick(mCompletedSerial); - mDeleter = nullptr; + // The Deleter may be null if initialization failed. + if (mDeleter != nullptr) { + mCompletedSerial = std::numeric_limits::max(); + mDeleter->Tick(mCompletedSerial); + mDeleter = nullptr; + } // VkQueues are destroyed when the VkDevice is destroyed // The VkDevice is needed to destroy child objects, so it must be destroyed last after all @@ -406,6 +417,8 @@ namespace dawn_native { namespace vulkan { DAWN_TRY(CheckVkSuccess(fn.CreateDevice(physicalDevice, &createInfo, nullptr, &mVkDevice), "vkCreateDevice")); + // Device created. Mark it as alive. + mLossStatus = LossStatus::Alive; return usedKnobs; }