From 77c6441ced21884fd7410cac494c1cb20ce35197 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Thu, 14 Nov 2024 14:22:43 -0600 Subject: [PATCH 1/5] prefer wheel-provided libraries, use RTLD_LOCAL --- .pre-commit-config.yaml | 6 ++-- VERSION | 2 +- python/libucx/libucx/load.py | 54 +++++++++++++++++++++++++++++------- 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 476bc6d..a1c6461 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -2,7 +2,7 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.5.0 + rev: v5.0.0 hooks: - id: trailing-whitespace - id: end-of-file-fixer @@ -11,12 +11,12 @@ repos: hooks: - id: isort - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.4.4 + rev: v0.7.3 hooks: - id: ruff - id: ruff-format - repo: https://github.com/rapidsai/pre-commit-hooks - rev: v0.0.3 + rev: v0.4.0 hooks: - id: verify-copyright diff --git a/VERSION b/VERSION index 092afa1..7ff093c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.17.0 +1.14.1.post2 diff --git a/python/libucx/libucx/load.py b/python/libucx/libucx/load.py index 86dd287..ca3d2cf 100644 --- a/python/libucx/libucx/load.py +++ b/python/libucx/libucx/load.py @@ -33,19 +33,53 @@ ] +# Loading with RTLD_LOCAL adds the library itself to the loader's +# loaded library cache without loading any symbols into the global +# namespace. This allows libraries that express a dependency on +# a library to be loaded later and successfully satisfy that dependency +# without polluting the global symbol table with symbols from +# that library that could conflict with symbols from other DSOs. +PREFERRED_LOAD_FLAG = ctypes.RTLD_LOCAL + + +def _load_system_installation(soname: str): + """Try to dlopen() the library indicated by ``soname`` + Raises ``OSError`` if library cannot be loaded. + """ + return ctypes.CDLL(soname, PREFERRED_LOAD_FLAG) + + +def _load_wheel_installation(soname: str): + """Try to dlopen() the library indicated by ``soname`` + Returns ``None`` if the library cannot be loaded. + """ + if os.path.isfile(lib := os.path.join(os.path.dirname(__file__), "lib", soname)): + return ctypes.CDLL(lib, PREFERRED_LOAD_FLAG) + return None + + def load_library(): - # Dynamically load libucx.so. Prefer a system library if one is present to - # avoid clobbering symbols that other packages might expect, but if no - # other library is present use the one in the wheel. + """Dynamically load UCX libraries""" + prefer_system_installation = ( + os.getenv("RAPIDS_LIBUCX_PREFER_SYSTEM_LIBRARY", "false").lower() != "false" + ) + libraries = [] for lib in UCX_LIBRARIES: - try: - libucx_lib = ctypes.CDLL(lib, ctypes.RTLD_GLOBAL) - except OSError: - libucx_lib = ctypes.CDLL( - os.path.join(os.path.dirname(__file__), "lib", lib), - ctypes.RTLD_GLOBAL, - ) + if prefer_system_installation: + # Prefer a system library if one is present to + # avoid clobbering symbols that other packages might expect, but if no + # other library is present use the one in the wheel. + try: + libucx_lib = _load_system_installation(lib) + except OSError: + libucx_lib = _load_wheel_installation(lib) + else: + # Prefer the libraries bundled in this package. If they aren't found + # (which might be the case in builds where the library was prebuilt + # before packaging the wheel), look for a system installation. + libucx_lib = _load_wheel_installation(lib) + libraries.append(libucx_lib) return libraries From 5df54d47dbd626c95ace71e95be9c32c429b0e9a Mon Sep 17 00:00:00 2001 From: James Lamb Date: Thu, 14 Nov 2024 15:12:49 -0600 Subject: [PATCH 2/5] 1.15.0.post2 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 7ff093c..6af060a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.14.1.post2 +1.15.0.post2 From 285f51407853bf0292b4b7107a8c15b46a975717 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Thu, 14 Nov 2024 18:01:10 -0600 Subject: [PATCH 3/5] try RTLD_GLOBAL --- python/libucx/libucx/load.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/libucx/libucx/load.py b/python/libucx/libucx/load.py index ca3d2cf..9155b2c 100644 --- a/python/libucx/libucx/load.py +++ b/python/libucx/libucx/load.py @@ -39,7 +39,7 @@ # a library to be loaded later and successfully satisfy that dependency # without polluting the global symbol table with symbols from # that library that could conflict with symbols from other DSOs. -PREFERRED_LOAD_FLAG = ctypes.RTLD_LOCAL +PREFERRED_LOAD_FLAG = ctypes.RTLD_GLOBAL def _load_system_installation(soname: str): From 1823d96777c31f2cd66c9148b28883853fc1e9e6 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Mon, 18 Nov 2024 19:00:43 +0000 Subject: [PATCH 4/5] Don't load libucm directly so that the others are safe in any order, then switch to RTLD_LOCAL. --- python/libucx/libucx/load.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/python/libucx/libucx/load.py b/python/libucx/libucx/load.py index 9155b2c..7a0f957 100644 --- a/python/libucx/libucx/load.py +++ b/python/libucx/libucx/load.py @@ -16,18 +16,9 @@ import ctypes import os -# IMPORTANT: The load order here matters! libucm.so depends on symbols in libucs.so, but -# it does not express this via a DT_NEEDED entry, presumably because libucs.so also has -# a dependency on libucm.so and the libraries are attempting to avoid a circular -# dependency. Moreover, it seems like if libucs.so is not loaded before libuct.so and -# libucp.so something is set up incorrectly, perhaps with the atexit handlers, because -# on library close there is a double free issue. Therefore, libucs.so must be loaded -# first. The other libraries may then be loaded in any order. The libraries themselves -# all have $ORIGIN RPATHs to find each other. UCX_LIBRARIES = [ "libucs.so", "libucs_signal.so", - "libucm.so", "libuct.so", "libucp.so", ] @@ -39,7 +30,7 @@ # a library to be loaded later and successfully satisfy that dependency # without polluting the global symbol table with symbols from # that library that could conflict with symbols from other DSOs. -PREFERRED_LOAD_FLAG = ctypes.RTLD_GLOBAL +PREFERRED_LOAD_FLAG = ctypes.RTLD_LOCAL def _load_system_installation(soname: str): From c0c103faf54ea0720a622aa7aedad482014d7bb6 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Mon, 18 Nov 2024 19:03:23 +0000 Subject: [PATCH 5/5] Note that one patch will no longer be needed in 1.17 --- python/libucx/setup.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/python/libucx/setup.py b/python/libucx/setup.py index 7888842..a26a42a 100644 --- a/python/libucx/setup.py +++ b/python/libucx/setup.py @@ -67,6 +67,10 @@ def run(self): subprocess.run(["make", "install"]) # The config file built into UCX is not relocatable. We need to fix # that so that we can package up UCX and distribute it in a wheel. + # This was fixed in + # https://github.com/openucx/ucx/commit/c3437851f4abb5b7da966109d3762966ac30c476 + # and merged into version 1.17, so we can remove this patch once we + # no longer need to support older versions. subprocess.run( [ "sed",