Query the newest DLL first in the Windows already-loaded check - #2502
Open
LeSingh1 wants to merge 1 commit into
Open
Query the newest DLL first in the Windows already-loaded check#2502LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
`windows_dlls` is tabulated oldest-first, and every other entry point reverses it "to achieve new -> old search order": `load_with_system_search()` on Windows, and both `check_if_already_loaded_from_elsewhere()` and `load_with_system_search()` on Linux, which share `_candidate_sonames()`. The Windows `check_if_already_loaded_from_elsewhere()` iterates `desc.windows_dlls` forward instead. For a library present in more than one version it therefore reports the oldest already-loaded one, while the system search on the same platform, and both paths on Linux, prefer the newest. Since the already-loaded check runs first, that is the version callers get. The new test is Windows-only: `load_dl_windows` imports `ctypes.wintypes` and requires `ctypes.windll` at module scope, so it cannot be imported at all on other platforms.
Contributor
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.
windows_dllsis tabulated oldest-first, and the other entry points reverse it "to achieve new -> old search order":load_dl_windows.load_with_system_search()—for dll_name in reversed(desc.windows_dlls)load_dl_linux.check_if_already_loaded_from_elsewhere()andload_with_system_search()— both via_candidate_sonames(), which reversesload_dl_windows.check_if_already_loaded_from_elsewhere()iterates forward instead. For a library present in more than one version it reports the oldest already-loaded one, while the system search on the same platform — and both paths on Linux — prefer the newest. Since the already-loaded check runs first inload_nvidia_dynamic_lib, that is the version the caller ends up with.The new test is Windows-only, and deliberately so:
load_dl_windowsdoesimport ctypes.wintypesand raisesRuntimeErrorifctypes.windllis missing, both at module scope, so it cannot be imported on other platforms at all. It stubskernel32.GetModuleHandleWto record the probe order and asserts newest-first. On Linux it is collected and skipped — I confirmed that on a Linux runner: 1276 passed with skips going 4 → 5, no collection error. I have no Windows machine, so the assertion itself has only run in my head; the symmetry withload_with_system_search()25 lines below is the real argument.This pairs with #2499, which fixes the one catalog entry whose
linux_sonameswere tabulated in the wrong order. That one is a data fix, this one is a code fix; they are independent.NOTE: developed with the assistance of an AI coding agent. The new test carries
@pytest.mark.agent_authoredper AGENTS.md. I reviewed and verified the change before submitting.