From 498076be35cff8e5dfd688955e5eda8433227061 Mon Sep 17 00:00:00 2001 From: Aizal Khan Date: Fri, 23 Oct 2026 17:22:56 +0530 Subject: [PATCH] clear icd debug list slot after destroying messenger or callback terminator_DestroyDebugUtilsMessengerEXT and terminator_DestroyDebugReportCallbackEXT call the driver destroy on the per-ICD list slot and release the used-object slot, but leave the destroyed driver handle in the list. The next create is handed the same index, and its error rollback treats a non-NULL slot as a handle this call created, so a driver create failing on a reused slot passes the driver a handle it already destroyed. The rollback loops left their own destroyed handle behind in the same way. Reset the slot to VK_NULL_HANDLE after each driver destroy, matching terminator_DestroySurfaceKHR. The test ICD now returns VK_ERROR_OUT_OF_HOST_MEMORY when the application allocator refuses its handle allocation, and two regression tests cover the reused slot path. --- loader/debug_utils.c | 4 + tests/framework/icd/test_icd.cpp | 6 ++ tests/loader_alloc_callback_tests.cpp | 115 ++++++++++++++++++++++++++ 3 files changed, 125 insertions(+) diff --git a/loader/debug_utils.c b/loader/debug_utils.c index 395334c6f..1c14eb72e 100644 --- a/loader/debug_utils.c +++ b/loader/debug_utils.c @@ -253,6 +253,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateDebugUtilsMessengerEXT(VkInstanc NULL != icd_term->dispatch.DestroyDebugUtilsMessengerEXT) { icd_term->dispatch.DestroyDebugUtilsMessengerEXT( icd_term->instance, icd_term->debug_utils_messenger_list.list[next_index], pAllocator); + icd_term->debug_utils_messenger_list.list[next_index] = VK_NULL_HANDLE; } } } @@ -288,6 +289,7 @@ VKAPI_ATTR void VKAPI_CALL terminator_DestroyDebugUtilsMessengerEXT(VkInstance i NULL != icd_term->dispatch.DestroyDebugUtilsMessengerEXT) { icd_term->dispatch.DestroyDebugUtilsMessengerEXT( icd_term->instance, icd_term->debug_utils_messenger_list.list[*debug_messenger_index], pAllocator); + icd_term->debug_utils_messenger_list.list[*debug_messenger_index] = VK_NULL_HANDLE; } } @@ -538,6 +540,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateDebugReportCallbackEXT(VkInstanc NULL != icd_term->dispatch.DestroyDebugReportCallbackEXT) { icd_term->dispatch.DestroyDebugReportCallbackEXT( icd_term->instance, icd_term->debug_report_callback_list.list[next_index], pAllocator); + icd_term->debug_report_callback_list.list[next_index] = VK_NULL_HANDLE; } } } @@ -572,6 +575,7 @@ VKAPI_ATTR void VKAPI_CALL terminator_DestroyDebugReportCallbackEXT(VkInstance i NULL != icd_term->dispatch.DestroyDebugReportCallbackEXT) { icd_term->dispatch.DestroyDebugReportCallbackEXT( icd_term->instance, icd_term->debug_report_callback_list.list[*debug_report_index], pAllocator); + icd_term->debug_report_callback_list.list[*debug_report_index] = VK_NULL_HANDLE; } } diff --git a/tests/framework/icd/test_icd.cpp b/tests/framework/icd/test_icd.cpp index 56094061e..54ba4f402 100644 --- a/tests/framework/icd/test_icd.cpp +++ b/tests/framework/icd/test_icd.cpp @@ -475,6 +475,9 @@ test_vkCreateDebugUtilsMessengerEXT(VkInstance instance, [[maybe_unused]] const if (pAllocator) { new_handle_ptr = (uint8_t*)pAllocator->pfnAllocation(pAllocator->pUserData, sizeof(uint8_t*), 8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT); + if (nullptr == new_handle_ptr) { + return VK_ERROR_OUT_OF_HOST_MEMORY; + } } else { new_handle_ptr = new uint8_t; } @@ -526,6 +529,9 @@ test_vkCreateDebugReportCallbackEXT(VkInstance instance, [[maybe_unused]] const if (pAllocator) { new_handle_ptr = (uint8_t*)pAllocator->pfnAllocation(pAllocator->pUserData, sizeof(uint8_t*), 8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT); + if (nullptr == new_handle_ptr) { + return VK_ERROR_OUT_OF_HOST_MEMORY; + } } else { new_handle_ptr = new uint8_t; } diff --git a/tests/loader_alloc_callback_tests.cpp b/tests/loader_alloc_callback_tests.cpp index 3af2c9ff1..6de581c87 100644 --- a/tests/loader_alloc_callback_tests.cpp +++ b/tests/loader_alloc_callback_tests.cpp @@ -728,6 +728,121 @@ TEST(Allocation, DebugMessengerAndReportCallbackUseObjectAllocator) { ASSERT_TRUE(instance_tracker.empty()); } +// vkDestroyDebugUtilsMessengerEXT used to leave the destroyed driver handle behind in each ICD's debug_utils_messenger_list +// slot. The slot index is handed to the next create, whose error rollback treats a non-NULL slot as a handle this call +// created, so a driver create failing on a reused slot made the rollback destroy a driver handle a second time. +TEST(Allocation, DebugUtilsMessengerCreateFailOnReusedSlot) { + FrameworkEnvironment env{}; + env.add_icd(TEST_ICD_PATH_VERSION_2) + .set_min_icd_interface_version(5) + .add_instance_extension(VK_EXT_DEBUG_UTILS_EXTENSION_NAME) + .add_physical_device({}); + + MemoryTracker instance_tracker{}; + + VkInstance instance = VK_NULL_HANDLE; + InstanceCreateInfo inst_create_info{}; + inst_create_info.add_extension(VK_EXT_DEBUG_UTILS_EXTENSION_NAME); + ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkCreateInstance(inst_create_info.get(), instance_tracker.get(), &instance)); + + PFN_vkCreateDebugUtilsMessengerEXT create_messenger = reinterpret_cast( + env.vulkan_functions.vkGetInstanceProcAddr(instance, "vkCreateDebugUtilsMessengerEXT")); + PFN_vkDestroyDebugUtilsMessengerEXT destroy_messenger = reinterpret_cast( + env.vulkan_functions.vkGetInstanceProcAddr(instance, "vkDestroyDebugUtilsMessengerEXT")); + ASSERT_NE(create_messenger, nullptr); + ASSERT_NE(destroy_messenger, nullptr); + + VkDebugUtilsMessengerCreateInfoEXT messenger_info{VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT}; + messenger_info.messageSeverity = VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT; + messenger_info.messageType = VK_DEBUG_UTILS_MESSAGE_TYPE_GENERAL_BIT_EXT; + messenger_info.pfnUserCallback = noop_debug_utils_callback; + + auto& icd_details = env.get_test_icd(0).created_instance_details.begin()->second; + + size_t fail_index = 0; + VkResult result = VK_ERROR_OUT_OF_HOST_MEMORY; + while (result == VK_ERROR_OUT_OF_HOST_MEMORY && fail_index <= 100) { + // Create and destroy a messenger so slot 0 has held a driver handle that is now destroyed. + MemoryTracker object_tracker{}; + VkDebugUtilsMessengerEXT messenger = VK_NULL_HANDLE; + ASSERT_EQ(VK_SUCCESS, create_messenger(instance, &messenger_info, object_tracker.get(), &messenger)); + destroy_messenger(instance, messenger, object_tracker.get()); + ASSERT_TRUE(object_tracker.empty()); + ASSERT_EQ(icd_details.messenger_handles.size(), 0u); + + // Create again into the same slot, failing one allocation at a time until the create succeeds. One of these + // iterations fails the driver's own allocation, so the driver returns VK_ERROR_OUT_OF_HOST_MEMORY and the + // loader rolls back with the slot still holding the handle destroyed above. + MemoryTracker failing_tracker({false, 0, true, fail_index}); + VkDebugUtilsMessengerEXT retried_messenger = VK_NULL_HANDLE; + result = create_messenger(instance, &messenger_info, failing_tracker.get(), &retried_messenger); + if (result == VK_SUCCESS) { + destroy_messenger(instance, retried_messenger, failing_tracker.get()); + } + ASSERT_TRUE(failing_tracker.empty()); + ASSERT_EQ(icd_details.messenger_handles.size(), 0u); + fail_index++; + } + ASSERT_EQ(VK_SUCCESS, result); + + env.vulkan_functions.vkDestroyInstance(instance, instance_tracker.get()); + ASSERT_TRUE(instance_tracker.empty()); +} + +// Same as above for vkDestroyDebugReportCallbackEXT and each ICD's debug_report_callback_list. +TEST(Allocation, DebugReportCallbackCreateFailOnReusedSlot) { + FrameworkEnvironment env{}; + env.add_icd(TEST_ICD_PATH_VERSION_2) + .set_min_icd_interface_version(5) + .add_instance_extension(VK_EXT_DEBUG_REPORT_EXTENSION_NAME) + .add_physical_device({}); + + MemoryTracker instance_tracker{}; + + VkInstance instance = VK_NULL_HANDLE; + InstanceCreateInfo inst_create_info{}; + inst_create_info.add_extension(VK_EXT_DEBUG_REPORT_EXTENSION_NAME); + ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkCreateInstance(inst_create_info.get(), instance_tracker.get(), &instance)); + + PFN_vkCreateDebugReportCallbackEXT create_callback = reinterpret_cast( + env.vulkan_functions.vkGetInstanceProcAddr(instance, "vkCreateDebugReportCallbackEXT")); + PFN_vkDestroyDebugReportCallbackEXT destroy_callback = reinterpret_cast( + env.vulkan_functions.vkGetInstanceProcAddr(instance, "vkDestroyDebugReportCallbackEXT")); + ASSERT_NE(create_callback, nullptr); + ASSERT_NE(destroy_callback, nullptr); + + VkDebugReportCallbackCreateInfoEXT callback_info{VK_STRUCTURE_TYPE_DEBUG_REPORT_CALLBACK_CREATE_INFO_EXT}; + callback_info.flags = VK_DEBUG_REPORT_ERROR_BIT_EXT; + callback_info.pfnCallback = noop_debug_report_callback; + + auto& icd_details = env.get_test_icd(0).created_instance_details.begin()->second; + + size_t fail_index = 0; + VkResult result = VK_ERROR_OUT_OF_HOST_MEMORY; + while (result == VK_ERROR_OUT_OF_HOST_MEMORY && fail_index <= 100) { + MemoryTracker object_tracker{}; + VkDebugReportCallbackEXT callback = VK_NULL_HANDLE; + ASSERT_EQ(VK_SUCCESS, create_callback(instance, &callback_info, object_tracker.get(), &callback)); + destroy_callback(instance, callback, object_tracker.get()); + ASSERT_TRUE(object_tracker.empty()); + ASSERT_EQ(icd_details.callback_handles.size(), 0u); + + MemoryTracker failing_tracker({false, 0, true, fail_index}); + VkDebugReportCallbackEXT retried_callback = VK_NULL_HANDLE; + result = create_callback(instance, &callback_info, failing_tracker.get(), &retried_callback); + if (result == VK_SUCCESS) { + destroy_callback(instance, retried_callback, failing_tracker.get()); + } + ASSERT_TRUE(failing_tracker.empty()); + ASSERT_EQ(icd_details.callback_handles.size(), 0u); + fail_index++; + } + ASSERT_EQ(VK_SUCCESS, result); + + env.vulkan_functions.vkDestroyInstance(instance, instance_tracker.get()); + ASSERT_TRUE(instance_tracker.empty()); +} + // Test failure during vkCreateInstance to make sure we don't leak memory if // one of the out-of-memory conditions trigger. TEST(Allocation, CreateInstanceIntentionalAllocFailWithSettingsFilePresent) {