Skip to content

Commit f7b1684

Browse files
rparolinclaude
andcommitted
pathfinder: use os.add_dll_directory() instead of raw kernel32 call
Per @leofang's review question: replace the direct ctypes kernel32.AddDllDirectory() call with the stdlib os.add_dll_directory() wrapper. Both call the same Win32 API, but the stdlib version drops the hand-maintained argtypes/restype binding and reports failure as OSError instead of a NULL cookie + GetLastError() check. Behavior is unchanged: the returned handle is intentionally discarded (it has no finalizer, so the directory stays on the search path for the process lifetime), and the PATH mutation + failure warning are preserved. Also strip an internal issue number from a test docstring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8623e74 commit f7b1684

2 files changed

Lines changed: 11 additions & 12 deletions

File tree

cuda_pathfinder/cuda/pathfinder/_dynamic_libs/load_dl_windows.py

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,6 @@
4444
]
4545
kernel32.GetModuleFileNameW.restype = ctypes.wintypes.DWORD
4646

47-
# AddDllDirectory (Windows 7+)
48-
kernel32.AddDllDirectory.argtypes = [ctypes.wintypes.LPCWSTR]
49-
kernel32.AddDllDirectory.restype = ctypes.c_void_p # DLL_DIRECTORY_COOKIE
50-
5147

5248
def ctypes_handle_to_unsigned_int(handle: ctypes.wintypes.HMODULE) -> int:
5349
"""Convert ctypes HMODULE to unsigned int."""
@@ -70,16 +66,19 @@ def add_dll_directory(dll_abs_path: str) -> None:
7066
dirpath = os.path.dirname(dll_abs_path)
7167
assert os.path.isdir(dirpath), dll_abs_path
7268

73-
# Add the DLL directory to the native search path. AddDllDirectory only
74-
# affects the LOAD_LIBRARY_SEARCH_USER_DIRS search; PATH is updated
75-
# unconditionally below to also cover legacy dependent-DLL resolution.
76-
cookie = kernel32.AddDllDirectory(dirpath)
77-
if not cookie:
69+
# Add the DLL directory to the native search path via the stdlib wrapper
70+
# around AddDllDirectory. This only affects the LOAD_LIBRARY_SEARCH_USER_DIRS
71+
# search; PATH is updated unconditionally below to also cover legacy
72+
# dependent-DLL resolution. The returned handle is intentionally discarded:
73+
# the directory must stay on the search path for the process lifetime, and
74+
# the handle has no finalizer, so dropping it does not remove the directory.
75+
try:
76+
os.add_dll_directory(dirpath) # type: ignore[attr-defined]
77+
except OSError as e:
7878
# Warn instead of failing silently; the PATH update below is a weaker
7979
# fallback that newer loaders may ignore.
80-
error_code = ctypes.GetLastError() # type: ignore[attr-defined]
8180
warnings.warn(
82-
f"AddDllDirectory({dirpath!r}) failed (error code: {error_code}); "
81+
f"os.add_dll_directory({dirpath!r}) failed ({e}); "
8382
"falling back to process-global PATH mutation for dependent-DLL resolution.",
8483
RuntimeWarning,
8584
stacklevel=2,

cuda_pathfinder/tests/test_find_nvidia_binaries.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ def test_caching_per_utility():
373373

374374

375375
def test_resolve_in_trusted_dirs_returns_absolute_path(tmp_path, monkeypatch, mocker):
376-
"""#374: a match found under a relative search dir must be absolutized.
376+
"""A match found under a relative search dir must be absolutized.
377377
378378
``find_nvidia_binary_utility`` documents an absolute, separator-resolved
379379
result. A relative search dir (e.g. a relative ``CUDA_HOME``) previously

0 commit comments

Comments
 (0)