Skip manifests already found when scanning search paths - #2033
Open
Wint3rNight wants to merge 1 commit into
Open
Wint3rNight wants to merge 1 commit into
Wint3rNight wants to merge 1 commit into
Conversation
Duplicate elimination runs on the search path strings, so it cannot catch the case where one path names a directory and another names a file inside that directory. The two strings differ, but the directory scan and the explicit file path resolve to the same manifest, so the driver is loaded twice and its physical devices are reported twice. Both call sites in add_data_files resolve to a full path before reaching add_if_manifest_file, so comparing the resolved name is enough to spot the repeat. The duplication is only visible with Vulkan 1.1 or newer, because GPDP2 gates the Linux physical device sort, which is why the existing tests at 1.0 did not catch it.
|
Author Wint3rNight not on autobuild list. Waiting for curator authorization before starting CI build. |
1 similar comment
|
Author Wint3rNight not on autobuild list. Waiting for curator authorization before starting CI build. |
|
CI Vulkan-Loader build queued with queue ID 119687. |
|
CI Vulkan-Loader build # 3766 running. |
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
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.
Fixes #1874.
Duplicate elimination runs on the search path strings, so it does not catch the case where one path names a directory and another names a file inside that directory. The two strings differ, but the directory scan and the explicit file path resolve to the same manifest, so the driver is loaded twice and its physical devices are reported twice.
With
VK_DRIVER_FILESset to a folder plus two manifests inside that folder,vkEnumeratePhysicalDevicesreports 9 devices where 5 exist, matching the log in the issue:Both call sites in
add_data_filesresolve to a full path before callingadd_if_manifest_file, so comparing the resolved name catches the repeat. The existingcopy_str_to_string_list_if_uniquehelper already does exactly this.This is only visible with Vulkan 1.1 or newer, because GPDP2 gates the Linux physical device sort. The existing env var tests run at 1.0, which is why CI never caught it.
Added
EnvVarICDOverrideSetup.DirAndFileOverlapDoesNotDuplicate, which fails on main with 9 devices and passes with the fix. Full suite is 703/703.Two notes:
add_data_filesalso serves layer discovery, so layers get the same protection. No existing test changes behaviour.loader_get_fullpathdoes not canonicalise symlinks or..segments, so two different spellings of the same file are still not detected. That is outside what this issue reports and I have left it alone.The issue also suggests running these env var cases at both 1.0 and 1.1, since much loader logic depends on GPDP2. That is a broader testing change and I have kept it out of this PR, but happy to follow up if useful.