Use recursive mutexes to fix deadlocks in loader#1866
Open
charles-lunarg wants to merge 6 commits intoKhronosGroup:mainfrom
Open
Use recursive mutexes to fix deadlocks in loader#1866charles-lunarg wants to merge 6 commits intoKhronosGroup:mainfrom
charles-lunarg wants to merge 6 commits intoKhronosGroup:mainfrom
Conversation
Because the functions are non-global, to enable proper threaded test execution, they must be initialized per InstWrapper.
This improves performance slightly by reducing the amount of symbols that need to be resolved in calls to dlopen.
Requires adding locks to various components of the test framework that need to support multiple calls to it simultaneously.
|
CI Vulkan-Loader build queued with queue ID 654373. |
|
CI Vulkan-Loader build # 3423 running. |
|
CI Vulkan-Loader build queued with queue ID 654391. |
|
CI Vulkan-Loader build # 3424 running. |
|
CI Vulkan-Loader build # 3424 failed. |
All mutexes are now recursive and can be acquired in the same thread. And loader_global_instance_list_lock was removed as it masked the underlying synchronization problems. The fundamental sync problem is that vkCreateInstance and vkCreateDevice must call down the chain to fill out the dispatch tables and during this it must search through the global list of instances/devices. With recursive mutexes, we do not need to do anything special to guarantee that walking the global lists are safe when occurring inside of vkCreateInstance/vkCreateDevice and outside of those calls. This commit adds a new threading test which exercises the debug utils naming functions. It also enables the threading tests to run during regular runs, and adds a github action CI job which enables the thread sanitizer.
charles-lunarg
force-pushed
the
use_recursive_mutexes
branch
from
f6b47a5 to
5b0ed94
Compare
|
CI Vulkan-Loader build queued with queue ID 654430. |
1 similar comment
|
CI Vulkan-Loader build queued with queue ID 654430. |
|
CI Vulkan-Loader build # 3425 running. |
|
CI Vulkan-Loader build # 3425 failed. |
Need to only use the mutex after initialization .
Closed
In debug mode, address sanitizer reports errors that are false positives when loading a library with dlopen for the second time.
|
CI Vulkan-Loader build queued with queue ID 655592. |
1 similar comment
|
CI Vulkan-Loader build queued with queue ID 655592. |
|
CI Vulkan-Loader build # 3428 running. |
|
CI Vulkan-Loader build # 3428 passed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While thread sanitizer has been a build option for a long time, it was not enabled in any tests leading to multiple threading issues persisting in the code.
Fixes #1863
Fixes #1739