Repository navigation
Fix race when recycling descriptor sets - #1746
Tyagiquamar wants to merge 1 commit into
Conversation
|
Thanks for the changes. I'm currently testing as a branch. I feel testing applications are best kept in vsgExamples as want to keep the VSG itself just dedicated to build the library with minimum of compile options so I won't merge the small test program. However, within this branch I am currently trying to reproduce the bug using your test program. So far I haven't got a crash even when bumping up the number of iterations in your test program and when commenting out the keepAlive() fix. Do you see issues when you run your test program when comment out the keepAlive() fix? |
|
I have merge PR #1748 that is in the spirit of this PR but doesn't introduce test program into the build itself and keeps it clear what the scope of the mutex is. I'll close this PR. @Tyagiquamar could you test VSG master to make sure my changes work for you. |
Description
freeDescriptorSet()publishes a set for reuse before clearing its pool reference. A concurrentallocateDescriptorSet()can restore that reference under the mutex, only for the later clear to erase it. Keep the pool alive explicitly and clear the set's reference while holding the same mutex.Fixes #1745
Type of change
How Has This Been Tested?
vsg_descriptor_pool_recyclewith GCC 12.2 and CMake 3.25.1 in Debian Docker, with windowing and shader compilation disabled.VK_ICD_FILENAMES=/usr/share/vulkan/icd.d/lvp_icd.x86_64.json ctest --test-dir /work-build --output-on-failure -R vsg_descriptor_pool_recycle --repeat until-fail:10passed 10/10 runs.DescriptorPoolaccesses before the fix and noDescriptorPoolrace after it. Two remaining Helgrind contexts are inside Mesa Lavapipe.unexpected memory mapping), so I used Helgrind.clang-format --dry-run --Werrorpassed for the changed C++ lines and test.Test Configuration:
Checklist: