Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
jerry411 opened this issue Jul 18, 2024 · 4 comments
Labels

Comments

@jerry411
Copy link

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
Copy link
Contributor

You're right, this looks like a bug!

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

@Agrael1
Copy link
Contributor

Agrael1 commented Jul 22, 2024

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
Copy link
Contributor

Agrael1 commented Jul 22, 2024

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
Copy link
Contributor

Fixed by #1925

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants