diff --git a/CHANGES.md b/CHANGES.md index 7040d695..1473da24 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,12 @@ +3.7.0 (TBD) +=========== + +- Fixed an intermittent `OSError` on Windows when DLLs are loaded or unloaded + concurrently during library discovery (for example when importing conda-forge + OpenCV). Windows module enumeration now uses a snapshot-first approach with + graceful per-module fallbacks. + https://github.com/joblib/threadpoolctl/issues/217 + 3.6.0 (2025-03-13) ================== diff --git a/tests/test_threadpoolctl.py b/tests/test_threadpoolctl.py index ea27cc2b..db41a434 100644 --- a/tests/test_threadpoolctl.py +++ b/tests/test_threadpoolctl.py @@ -5,6 +5,7 @@ import subprocess import sys +import threadpoolctl from threadpoolctl import threadpool_limits, threadpool_info from threadpoolctl import ThreadpoolController from threadpoolctl import _ALL_PREFIXES, _ALL_USER_APIS @@ -12,6 +13,12 @@ from .utils import cython_extensions_compiled from .utils import check_nested_prange_blas from .utils import libopenblas_paths +from .utils import copy_and_load_dll +from .utils import get_openblas_dll_path +from .utils import LONG_PATH_OPENBLAS_DLL +from .utils import make_long_windows_path +from .utils import normalize_windows_path +from .utils import TRUNCATED_PATH_OPENBLAS_DLL from .utils import scipy from .utils import threadpool_info_from_subprocess from .utils import select @@ -613,6 +620,7 @@ def test_architecture(): expected_openblas_architectures = ( # XXX: add more as needed by CI or developer laptops "armv8", + "cooperlake", "haswell", "neoversen1", "prescott", # see: https://github.com/xianyi/OpenBLAS/pull/3485 @@ -792,3 +800,69 @@ def test_custom_controller(): assert mylib_controller.num_threads == 1 assert ThreadpoolController().info() == original_info + + +def test_threadpool_controller_repeated_init(): + """Stress-test repeated library discovery. + + Non-regression test for a Windows-specific problem where DLLs loaded or + unloaded concurrently during discovery could raise an OSError; see + https://github.com/joblib/threadpoolctl/issues/217 + """ + for _ in range(100): + ThreadpoolController() + + +@pytest.mark.skipif(sys.platform != "win32", reason="Windows-only test") +def test_windows_library_path_longer_than_max_path(tmp_path): + """OpenBLAS loaded from a path longer than MAX_PATH is discovered. + + Regression test inspired by the local repro in + https://github.com/joblib/threadpoolctl/pull/189#issuecomment-2714235916 + """ + src_dll = get_openblas_dll_path() + if src_dll is None: + pytest.skip("Requires OpenBLAS on Windows") + + long_path = make_long_windows_path(tmp_path, LONG_PATH_OPENBLAS_DLL, min_length=261) + copy_and_load_dll(src_dll, long_path) + + expected_path = normalize_windows_path(long_path) + openblas_info = ThreadpoolController().select(internal_api="openblas").info() + + long_path_entries = [info for info in openblas_info if len(info["filepath"]) > 260] + assert len(long_path_entries) >= 1 + assert any( + normalize_windows_path(info["filepath"]) == expected_path + for info in long_path_entries + ) + + +@pytest.mark.skipif(sys.platform != "win32", reason="Windows-only test") +def test_windows_library_path_exceeds_internal_limit(tmp_path, monkeypatch): + """Libraries with a path longer than the internal limit are ignored. + + Regression test inspired by the local repro in + https://github.com/joblib/threadpoolctl/pull/189#issuecomment-2714235916 + """ + src_dll = get_openblas_dll_path() + if src_dll is None: + pytest.skip("Requires OpenBLAS on Windows") + + monkeypatch.setattr(threadpoolctl, "_WINDOWS_MAX_LIBRARY_PATH_LENGTH", 300) + + long_path = make_long_windows_path( + tmp_path, TRUNCATED_PATH_OPENBLAS_DLL, min_length=400 + ) + copy_and_load_dll(src_dll, long_path) + + expected_path = normalize_windows_path(long_path) + with pytest.warns(RuntimeWarning, match="path too long"): + info = ThreadpoolController().info() + + filepaths = { + normalize_windows_path(entry["filepath"]) + for entry in info + if "filepath" in entry + } + assert expected_path not in filepaths diff --git a/tests/utils.py b/tests/utils.py index 86fe1b0b..39b39bc3 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,9 +1,12 @@ -import os import json +import os import sys +import ctypes +import shutil import threadpoolctl from glob import glob from os.path import dirname, normpath +from pathlib import Path from subprocess import check_output # Path to shipped openblas for libraries such as numpy or scipy @@ -17,6 +20,17 @@ np.dot(np.ones(1000), np.ones(1000)) libopenblas_patterns.append(os.path.join(np.__path__[0], ".libs", "libopenblas*")) + numpy_site_packages = os.path.dirname(np.__path__[0]) + libopenblas_patterns.append( + os.path.join(numpy_site_packages, "numpy.libs", "libscipy_openblas*.dll") + ) + if sys.platform == "win32": + libopenblas_patterns.append( + os.path.join(sys.prefix, "Library", "bin", "openblas*.dll") + ) + libopenblas_patterns.append( + os.path.join(sys.prefix, "Library", "bin", "libopenblas*.dll") + ) except ImportError: pass @@ -30,6 +44,10 @@ libopenblas_patterns.append( os.path.join(scipy.__path__[0], ".libs", "libopenblas*") ) + scipy_site_packages = os.path.dirname(scipy.__path__[0]) + libopenblas_patterns.append( + os.path.join(scipy_site_packages, "scipy.libs", "libscipy_openblas*.dll") + ) except ImportError: scipy = None @@ -86,3 +104,73 @@ def select(info, **kwargs): ] return selected_info + + +def get_openblas_dll_path(): + """Return a path to an OpenBLAS DLL that can be copied for Windows tests.""" + if libopenblas_paths: + return sorted( + libopenblas_paths, + key=lambda path: ( + 0 + if "libscipy_openblas" in os.path.basename(path).lower() + else 1 if "libopenblas" in os.path.basename(path).lower() else 2 + ), + )[0] + + from threadpoolctl import ThreadpoolController + + controllers = ThreadpoolController().select(internal_api="openblas").lib_controllers + if not controllers: + return None + + filepath = controllers[0].filepath + if os.path.isfile(filepath): + return filepath + return None + + +def normalize_windows_path(path): + """Normalize Windows paths for stable comparisons in tests.""" + path = os.path.abspath(str(path)) + if path.startswith("\\\\?\\"): + path = path[4:] + if path.startswith("UNC\\"): + path = "\\\\" + path[4:] + return os.path.normcase(os.path.normpath(path)) + + +LONG_PATH_OPENBLAS_DLL = "libopenblas_long_path_test.dll" +TRUNCATED_PATH_OPENBLAS_DLL = "libopenblas_path_too_long.dll" + + +def make_long_windows_path(base_dir, filename, min_length=261): + """Nest padded directories under base_dir until base/.../filename >= min_length.""" + padding_segments = ["a" * 100, "b" * 100, "c" * 100, "d" * 100] + current = Path(base_dir) + segment_index = 0 + target = current / filename + + while len(str(target)) < min_length: + current = current / padding_segments[segment_index % len(padding_segments)] + segment_index += 1 + target = current / filename + + target.parent.mkdir(parents=True, exist_ok=True) + return target + + +def to_extended_windows_path(path): + """Return an extended-length path for Windows APIs (> MAX_PATH).""" + path = os.path.abspath(str(path)) + if path.startswith("\\\\?\\"): + return path + if path.startswith("\\\\"): + return "\\\\?\\UNC\\" + path[2:] + return "\\\\?\\" + path + + +def copy_and_load_dll(src_dll, destination): + """Copy a DLL to destination and load it in the current process.""" + shutil.copy2(src_dll, to_extended_windows_path(destination)) + ctypes.CDLL(to_extended_windows_path(destination)) diff --git a/threadpoolctl.py b/threadpoolctl.py index ceed5b88..3af16486 100644 --- a/threadpoolctl.py +++ b/threadpoolctl.py @@ -47,6 +47,10 @@ # disable it while under the scope of the outer OpenMP parallel section. os.environ.setdefault("KMP_DUPLICATE_LIB_OK", "True") +# Hard limit for Windows library paths resolved through GetModuleFileNameExW. +# Kept below unlimited for security reasons; see CHANGES.md and PR #189. +_WINDOWS_MAX_LIBRARY_PATH_LENGTH = 2600 + # Structure to cast the info on dynamically loaded library. See # https://linux.die.net/man/3/dl_iterate_phdr for more details. _SYSTEM_UINT = ctypes.c_uint64 if sys.maxsize > 2**32 else ctypes.c_uint32 @@ -969,7 +973,7 @@ def _load_libraries(self): if sys.platform == "darwin": self._find_libraries_with_dyld() elif sys.platform == "win32": - self._find_libraries_with_enum_process_module_ex() + self._find_libraries_on_windows() elif "pyodide" in sys.modules: self._find_libraries_pyodide() else: @@ -1039,22 +1043,17 @@ def _find_libraries_with_dyld(self): # Store the library controller if it is supported and selected self._make_controller_from_path(filepath) - def _find_libraries_with_enum_process_module_ex(self): + def _find_libraries_on_windows(self): """Loop through loaded libraries and return binders on supported ones This function is expected to work on windows system only. - This code is adapted from code by Philipp Hagemeister @phihag available - at https://stackoverflow.com/questions/17474574 """ - from ctypes.wintypes import DWORD, HMODULE, MAX_PATH - PROCESS_QUERY_INFORMATION = 0x0400 PROCESS_VM_READ = 0x0010 - LIST_LIBRARIES_ALL = 0x03 - ps_api = self._get_windll("Psapi") kernel_32 = self._get_windll("kernel32") + self._setup_windows_module_apis(ps_api, kernel_32) h_process = kernel_32.OpenProcess( PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, False, os.getpid() @@ -1062,57 +1061,219 @@ def _find_libraries_with_enum_process_module_ex(self): if not h_process: # pragma: no cover raise OSError(f"Could not open PID {os.getpid()}") + # Allocate a buffer for long path names; see _WINDOWS_MAX_LIBRARY_PATH_LENGTH. + max_path = _WINDOWS_MAX_LIBRARY_PATH_LENGTH + path_buf = ctypes.create_unicode_buffer(max_path) + try: - buf_count = 256 - needed = DWORD() - # Grow the buffer until it becomes large enough to hold all the - # module headers - while True: - buf = (HMODULE * buf_count)() - buf_size = ctypes.sizeof(buf) - if not ps_api.EnumProcessModulesEx( - h_process, - ctypes.byref(buf), - buf_size, - ctypes.byref(needed), - LIST_LIBRARIES_ALL, - ): - raise OSError("EnumProcessModulesEx failed") - if buf_size >= needed.value: - break - buf_count = needed.value // (buf_size // buf_count) - - count = needed.value // (buf_size // buf_count) - h_modules = map(HMODULE, buf[:count]) - - # Loop through all the module headers and get the library path - # Allocate a buffer for the path 10 times the size of MAX_PATH to take - # into account long path names. - max_path = 10 * MAX_PATH - buf = ctypes.create_unicode_buffer(max_path) - n_size = DWORD() - for h_module in h_modules: - # Get the path of the current module - if not ps_api.GetModuleFileNameExW( - h_process, h_module, ctypes.byref(buf), ctypes.byref(n_size) - ): - raise OSError("GetModuleFileNameEx failed") - filepath = buf.value - - if len(filepath) == max_path: # pragma: no cover - warnings.warn( - "Could not get the full path of a dynamic library (path too " - "long). This library will be ignored and threadpoolctl might " - "not be able to control or display information about all " - f"loaded libraries. Here's the truncated path: {filepath!r}", - RuntimeWarning, + try: + modules = self._snapshot_loaded_modules(kernel_32) + except OSError: + modules = None + + if modules is not None: + for h_module, snapshot_path in modules: + filepath = self._resolve_module_filepath( + ps_api, + h_process, + h_module, + snapshot_path, + max_path, + path_buf, ) - else: - # Store the library controller if it is supported and selected - self._make_controller_from_path(filepath) + if filepath is not None: + self._make_controller_from_path(filepath) + else: + self._find_libraries_with_enum_process_modules_ex( + ps_api, h_process, max_path, path_buf + ) finally: kernel_32.CloseHandle(h_process) + @classmethod + def _setup_windows_module_apis(cls, ps_api, kernel_32): + """Set ctypes signatures for Windows module enumeration APIs.""" + from ctypes.wintypes import BOOL, DWORD, HANDLE, HMODULE + + if getattr(cls, "_windows_module_apis_configured", False): + return + + kernel_32.OpenProcess.argtypes = [DWORD, BOOL, DWORD] + kernel_32.OpenProcess.restype = HANDLE + kernel_32.CloseHandle.argtypes = [HANDLE] + kernel_32.CloseHandle.restype = BOOL + kernel_32.CreateToolhelp32Snapshot.argtypes = [DWORD, DWORD] + kernel_32.CreateToolhelp32Snapshot.restype = HANDLE + kernel_32.Module32FirstW.argtypes = [HANDLE, ctypes.c_void_p] + kernel_32.Module32FirstW.restype = BOOL + kernel_32.Module32NextW.argtypes = [HANDLE, ctypes.c_void_p] + kernel_32.Module32NextW.restype = BOOL + + ps_api.EnumProcessModulesEx.argtypes = [ + HANDLE, + ctypes.POINTER(HMODULE), + DWORD, + ctypes.POINTER(DWORD), + DWORD, + ] + ps_api.EnumProcessModulesEx.restype = BOOL + ps_api.GetModuleFileNameExW.argtypes = [ + HANDLE, + HMODULE, + ctypes.c_wchar_p, + DWORD, + ] + ps_api.GetModuleFileNameExW.restype = BOOL + + cls._windows_module_apis_configured = True + + def _snapshot_loaded_modules(self, kernel_32): + """Return loaded modules as (hModule, snapshot_path) pairs. + + Uses CreateToolhelp32Snapshot for an atomic view of loaded modules, which + is more robust than EnumProcessModulesEx when DLLs are loaded or unloaded + concurrently. + """ + from ctypes.wintypes import DWORD, HANDLE, MAX_PATH + + class MODULEENTRY32W(ctypes.Structure): + _fields_ = [ + ("dwSize", DWORD), + ("th32ModuleID", DWORD), + ("th32ProcessID", DWORD), + ("GlblcntUsage", DWORD), + ("ProccntUsage", DWORD), + ("modBaseAddr", ctypes.POINTER(ctypes.c_byte)), + ("modBaseSize", DWORD), + ("hModule", HANDLE), + ("szModule", ctypes.c_wchar * 256), + ("szExePath", ctypes.c_wchar * MAX_PATH), + ] + + TH32CS_SNAPMODULE = 0x00000008 + TH32CS_SNAPMODULE32 = 0x00000010 + ERROR_BAD_LENGTH = 0x0018 + ERROR_NO_MORE_FILES = 0x0012 + INVALID_HANDLE_VALUE = HANDLE(-1).value + + while True: + snap_handle = kernel_32.CreateToolhelp32Snapshot( + TH32CS_SNAPMODULE | TH32CS_SNAPMODULE32, os.getpid() + ) + if snap_handle == INVALID_HANDLE_VALUE: + err = ctypes.get_last_error() + if err == ERROR_BAD_LENGTH: + continue + msg = ctypes.FormatError(err).strip() + raise OSError(f"CreateToolhelp32Snapshot failed: {msg}") + break + + modules = [] + try: + lib_entry = MODULEENTRY32W() + lib_entry.dwSize = ctypes.sizeof(MODULEENTRY32W) + if not kernel_32.Module32FirstW( + snap_handle, ctypes.byref(lib_entry) + ): # pragma: no cover + err = ctypes.get_last_error() + msg = ctypes.FormatError(err).strip() + raise OSError(f"Module32FirstW failed: {msg}") + + while True: + modules.append((lib_entry.hModule, lib_entry.szExePath)) + if not kernel_32.Module32NextW(snap_handle, ctypes.byref(lib_entry)): + err = ctypes.get_last_error() + if err != ERROR_NO_MORE_FILES: # pragma: no cover + msg = ctypes.FormatError(err).strip() + raise OSError(f"Module32NextW failed: {msg}") + break + finally: + kernel_32.CloseHandle(snap_handle) + + return modules + + def _resolve_module_filepath( + self, + ps_api, + h_process, + h_module, + snapshot_path, + max_path, + path_buf, + ): + """Return the full path for a module, or None if it should be skipped.""" + from ctypes.wintypes import MAX_PATH + + if ps_api.GetModuleFileNameExW(h_process, h_module, path_buf, max_path): + filepath = path_buf.value + if len(filepath) >= max_path - 1: # pragma: no cover + warnings.warn( + "Could not get the full path of a dynamic library (path too " + "long). This library will be ignored and threadpoolctl might " + "not be able to control or display information about all " + f"loaded libraries. Here's the truncated path: {filepath!r}", + RuntimeWarning, + ) + return None + return filepath + + if snapshot_path is not None and len(snapshot_path) < MAX_PATH - 1: + return snapshot_path + + if snapshot_path is not None: # pragma: no cover + warnings.warn( + "Could not get the full path of a dynamic library. This library " + "will be ignored and threadpoolctl might not be able to control or " + f"display information about all loaded libraries. Here's the " + f"truncated path: {snapshot_path!r}", + RuntimeWarning, + ) + return None + + def _find_libraries_with_enum_process_modules_ex( + self, ps_api, h_process, max_path, path_buf + ): + """Fallback Windows module enumeration using EnumProcessModulesEx. + + This code is adapted from code by Philipp Hagemeister @phihag available + at https://stackoverflow.com/questions/17474574 + """ + from ctypes.wintypes import DWORD, HMODULE + + LIST_LIBRARIES_ALL = 0x03 + + buf_count = 256 + needed = DWORD() + # Grow the buffer until it becomes large enough to hold all the + # module headers + while True: + buf = (HMODULE * buf_count)() + buf_size = ctypes.sizeof(buf) + if not ps_api.EnumProcessModulesEx( + h_process, + buf, + buf_size, + ctypes.byref(needed), + LIST_LIBRARIES_ALL, + ): + raise OSError("EnumProcessModulesEx failed") + if buf_size >= needed.value: + break + buf_count = needed.value // (buf_size // buf_count) + + count = needed.value // (buf_size // buf_count) + for h_module in map(HMODULE, buf[:count]): + filepath = self._resolve_module_filepath( + ps_api, + h_process, + h_module, + snapshot_path=None, + max_path=max_path, + path_buf=path_buf, + ) + if filepath is not None: + self._make_controller_from_path(filepath) + def _find_libraries_pyodide(self): """Pyodide specific implementation for finding loaded libraries. @@ -1250,7 +1411,7 @@ def _get_windll(cls, dll_name): """Load a windows DLL""" dll = cls._system_libraries.get(dll_name) if dll is None: - dll = ctypes.WinDLL(f"{dll_name}.dll") + dll = ctypes.WinDLL(f"{dll_name}.dll", use_last_error=True) cls._system_libraries[dll_name] = dll return dll