KhronosGroup / KhronosGroup/Vulkan-ValidationLayers

Memory leaks when call down vkCreateInstance fails or other Vulkan API calls fail inside of vkCreateInstance

Open
#5,076 2 comments 1 reaction 0 assignees View on GitHub
Bug
Dominant language
C++
Stars
1k
Forks
504
Avg merge
11h 35m
Merged PRs (30d)
224

Description

**Describe the Issue**
There are two leaks happening, one causing VVL to leak memory directly, and another causing memory leaks in the loader.

1. When VK_ERROR_OUT_OF_HOST_MEMORY is returned from the call to vkEnumeratePhysicalDevices that happens inside of VVL's `vkCreateInstance`, the validation layers cause the loader to leak memory. These Vulkan API calls are made inside `PreCallRecordCreateInstance` so require modification to the generated code. The problem is that when OOM is returned from `vkEnumeratePhysicalDevices`, the loader has successfully completed the vkCreateInstance terminator. OOM from one of the subsequent Vulkan API calls requires that VVL cleanup all vulkan objects before existing, which should only require calling vkDestroyInstance from inside VVL.

A description of the sequence of events
* Loader's vkCreateInstance (trampoline), calls into layer's vkCreateInstance
* VVL's vkCreateInstance is called, initializing local objects and setting up dispatch tables.
* VVL calls down vkCreateInstance
* Loader's terminator_CreateInstance is called and completes successfully
* Control flow returns to VVL, continues setting up objects
* `PreCallRecordCreateInstance` is called on the validation layer objects
* A vulkan API call is made, such as `vkEnumeratePhysicalDevices`, which fails with OOHM (error_out_of_host_memory)
* What happens now is: Ignore the error condition and continue as if nothing happened.

What should happen is that VVL should catch that OOHM, delete the instance by calling vkDestroyInstance, then return immediately.

2. The structures directly leaked by VVL are the validation layer objects allocated here.
https://github.com/KhronosGroup/Vulkan-ValidationLayers/blob/d5426ea20669a4d8de34dd3b89685fba73a21c4f/layers/generated/chassis.cpp#L290

The solution would be to put them in std::unique_ptr<> wrappers immediately instead of having naked pointers laying about.

Contributor guide

Open the contributing guide

Research direction

Start in layers/generated/chassis.cpp around line 290 and trace the vkCreateInstance path through PreCallRecordCreateInstance, including the vkEnumeratePhysicalDevices failure case. Compare the existing object ownership and cleanup flow with the requested vkDestroyInstance behavior; done means VVL-owned objects and the loader are not left leaking after an out-of-host-memory failure.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.