Skip to content

Fix race when recycling descriptor sets - #1746

Open
Tyagiquamar wants to merge 1 commit into
vsg-dev:masterfrom
Tyagiquamar:fix/descriptor-pool-recycle-race
Open

Tyagiquamar wants to merge 1 commit into
vsg-dev:masterfrom
Tyagiquamar:fix/descriptor-pool-recycle-race

Conversation

@Tyagiquamar

Copy link
Copy Markdown

Description

freeDescriptorSet() publishes a set for reuse before clearing its pool reference. A concurrent allocateDescriptorSet() 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

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Built vsg_descriptor_pool_recycle with GCC 12.2 and CMake 3.25.1 in Debian Docker, with windowing and shader compilation disabled.
  • On unmodified master, the test failed at iterations 2 and 8 in separate runs. With this fix, 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:10 passed 10/10 runs.
  • Helgrind reported the conflicting DescriptorPool accesses before the fix and no DescriptorPool race after it. Two remaining Helgrind contexts are inside Mesa Lavapipe.
  • A TSan smoke binary could not start in this WSL2 environment (unexpected memory mapping), so I used Helgrind.
  • clang-format --dry-run --Werror passed for the changed C++ lines and test.

Test Configuration:

  • Firmware version: N/A
  • Hardware: Mesa Lavapipe software Vulkan in Docker on WSL2
  • Toolchain: GCC 12.2, CMake 3.25.1
  • SDK: Debian Vulkan headers 1.3.239

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes (there is no existing unit-test suite)
  • Any dependent changes have been merged and published in downstream modules

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Potential race condition in DescriptorPool::freeDescriptorSet

1 participant