Skip to content

Commit 3be7141

Browse files
authored
[VK] Fix crash on exit when using VVL (#219)
VVL has internal state which conflicts requires a specific cleanup ordering. Because the VKContext relied on the global dtor, this order was not respected. There are 2 solutions: - use atexit - require an explicit call to vkDestroyDevice. Thing is, users of `Device` except the object to be usable as a simple singleton, with no explicit `destroy` to call. For this reasons, it seems using `atexit` is the cleanest option from the user perspective.
1 parent c4f5806 commit 3be7141

File tree

3 files changed

+19
-4
lines changed

3 files changed

+19
-4
lines changed

include/API/Device.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ class Device {
6262
#endif
6363

6464
#ifdef OFFLOADTEST_ENABLE_VULKAN
65-
static llvm::Error initializeVXDevices();
65+
static llvm::Error initializeVKDevices();
66+
static void cleanupVKDevices();
6667
#endif
6768

6869
#ifdef OFFLOADTEST_ENABLE_METAL

lib/API/Device.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
#include "llvm/Support/Error.h"
1717

18+
#include <cstdlib>
1819
#include <memory>
1920

2021
using namespace offloadtest;
@@ -59,8 +60,14 @@ llvm::Error Device::initialize() {
5960
return Err;
6061
#endif
6162
#ifdef OFFLOADTEST_ENABLE_VULKAN
62-
if (auto Err = initializeVXDevices())
63+
if (auto Err = initializeVKDevices())
6364
return Err;
65+
// Validation layers have internal state which require a specific destruction
66+
// ordering. Relying on the global dtor call for this is unreliable and can
67+
// cause a null-deref in the validation layers during the final
68+
// vkDestroyInstance. This is a known limitation of the validation layers
69+
// which explicitely requires using atexit.
70+
atexit(Device::cleanupVKDevices);
6471
#endif
6572
#ifdef OFFLOADTEST_ENABLE_METAL
6673
if (auto Err = initializeMtlDevices())

lib/API/VK/Device.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ class VKContext {
770770
llvm::SmallVector<std::shared_ptr<VKDevice>> Devices;
771771

772772
VKContext() = default;
773-
~VKContext() { vkDestroyInstance(Instance, NULL); }
773+
~VKContext() { cleanup(); }
774774
VKContext(const VKContext &) = delete;
775775

776776
public:
@@ -779,6 +779,11 @@ class VKContext {
779779
return Ctx;
780780
}
781781

782+
void cleanup() {
783+
vkDestroyInstance(Instance, NULL);
784+
Instance = VK_NULL_HANDLE;
785+
}
786+
782787
llvm::Error initialize() {
783788
// Create a Vulkan 1.1 instance to determine the API version
784789
VkApplicationInfo AppInfo = {};
@@ -851,6 +856,8 @@ class VKContext {
851856
};
852857
} // namespace
853858

854-
llvm::Error Device::initializeVXDevices() {
859+
llvm::Error Device::initializeVKDevices() {
855860
return VKContext::instance().initialize();
856861
}
862+
863+
void Device::cleanupVKDevices() { VKContext::instance().cleanup(); }

0 commit comments

Comments
 (0)