KhronosGroup / Vulkan-Hpp

Open-Source Vulkan C++ API
Apache License 2.0
3.11k stars 307 forks source link

SharedCommandBuffer constructor without SharedCommandPool parameter causes assertion fail on destroy #1922

Closed jerry411 closed 2 months ago

jerry411 commented 3 months ago

SharedCommandBuffer can be created using standard way as most shared handles (with 2 parameters: standard handle + SharedDevice as owner).

But in case of SharedCommandBuffer with just these 2 parameters, PoolFreeShared() = default; on line 399 in vulkan_shared.hpp is called. This causes m_destroy and m_dispatch properties being set to nullptr. Later, when destroy method is called (line 410 in vulkan_shared.hpp), VULKAN_HPP_ASSERT( m_destroy && m_dispatch ); obviously fails.

The second constructor (PoolFreeShared on line 402) which correctly sets both m_destroy and m_dispatch is called only when SharedCommandBuffer constructor is called with additional parameter (SharedCommandPool) after owner (SharedDevice).

This looks like a bug. Either constructor without SharedCommandPool parameter should not exist (because it will always cause assertion fail on destroy), or that assertion in destroy method should be adjusted to accommodate this possibility.

asuessenbach commented 2 months ago

You're right, this looks like a bug!

@Agrael1 Would you please have a look at this issue?

Agrael1 commented 2 months ago

That is weird, SharedCommandBuffer should not have constructor with handle and device, it should have handle, device and pool only. I will check it ASAP

Agrael1 commented 2 months ago

Ok, so the solution is actually quite interesting here: SharedCommandPool is the mandatory argument here, but only if you have everything set up correctly. If the argument is absent, the destruction will not happen, but this is still valid, since those objects are destroyed when the pool is destroyed. I will replace the assertion with if-statement. Thanks for noticing!

asuessenbach commented 2 months ago

Fixed by #1925