gfxrecon_replay: Define VMA_ASSERT to avoid assert crashes #1244
Conversation
|
CI gfxreconstruct build queued with queue ID 35360. |
|
CI gfxreconstruct build # 3169 running. |
The debug compiled version of gfxrecon_replay would crash with an assert failure in VMA because the captured app did not destroy all images or buffers that it created before calling vkDestroyDevice. This fix changes the assert failure to a warning.
46f777c to
20e558f
Compare
|
CI gfxreconstruct build queued with queue ID 35378. |
|
CI gfxreconstruct build # 3170 running. |
|
CI gfxreconstruct build # 3170 passed. |
Yes, I have. |
There was a problem hiding this comment.
I think change to LOG_ERROR so that a user could enable GFXRECON_BREAK_ON_ERROR and kick into the debugger, and then add a note to USAGE*md that in debug mode a user may see a VMA assert and they can kick into the debugger with BREAK_ON_ERROR and then this is good to go.
|
I'm wondering if calling GFXRECON_LOG_ERROR is the right thing to do. I don't think the situation being identified is necessarily an error - the app has called vkDestroyDevice without first destroying all the images/buffers it created. That's not a fatal error, just a potential resource leak. And it is not a resource leak if the next thing the app does is exit, which would be the most common case. That's why I coded it using GRXRECON_LOG_WARNING. |
|
I would agree with @bradgrantham-lunarg and say it is. If an application triggers on an Assert, that indicates something is not right and a warning is not strong enough of a message. |
Changed VMA_ASSERT failures to generate a replay error rather than a warning. Modifed USAGE_desktop_Vulkan.md to document that VMA errors can be trapped using GFXRECON_LOG_BREAK_ON_ERROR.
|
CI gfxreconstruct build queued with queue ID 38306. |
|
CI gfxreconstruct build # 3176 running. |
Yes, that's interesting. It is invalid usage to destroy a device without destroying all its child objects. However, usually we LOG_ERROR on an unexpected internal situation, like couldn't open a file, or we can't find a Wrapper for an object. We generally don't report on the application's behavior (with a few exceptions). So we're just in a limbo where GFXR replay tries not to editorialize on the captured functions unless it would cause internal errors, and we've left that kind of error checking to VVL. But VMA has the opposite principle - they by default assert and crash in this circumstance, probably assuming it's running under the real app and the user wants this behavior in DEBUG mode. So at 30,000 feet view we're kind of not using VMA the way it should be used? 🤷 If we could make LOG_WARNING kick into the debugger or otherwise allow a user debugging to break, that would be ideal. LOG_ERROR in the code and BREAK_ON_ERROR env variable seem expedient and a reasonable compromise but if you have an idea @davidlunarg I want to hear it. P.S. https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-vkDestroyDevice-device-00378, vkDeviceMemory is a child object as far as I read the spec - An exception to this is when there is a parent/child relationship between objects. In this case, the application must not destroy a parent object before its children, https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#fundamentals-objectmodel-lifetime |
|
CI gfxreconstruct build # 3176 passed. |
The debug compiled version of gfxrecon_replay would crash with an assert failure in VMA because the captured app did not destroy all images or buffers that it created before calling vkDestroyDevice. This fix changes the assert failure to a warning.