Check that a new pyccel build imports before caching it

CHANGELOG.md

@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+- `PyccelCache` imports a new build once in a subprocess before it stores it: a build the
+  interpreter cannot import, or one that crashes it (an extension linked to `libpython` in a
+  Python that has it built in, such as uv's CPython 3.12 on macOS), is a `RuntimeError` and is
+  not stored, instead of a segmentation fault in every process that loads it.
+- The `PyccelCache` key includes the NumPy version, so environments that share an interpreter
+  but not NumPy no longer share builds.
+
 ### Added
 - scope-profiler regions around the coarse operations (`Kernel` calls as
   `cunumpy.kernel.<name>`, CUDA and Pyccel compilation, `sort_by_key`,

docs/src/content/docs/kernels/dispatch.md

@@ -257,15 +257,19 @@ inside `with xp.kernels.use_host_kernel_implementation("numpy"):` (or set
 
 `epyccel` compiles again on every call, in every process. `PyccelCache(name)`
 compiles a module once with the `pyccel compile` command and keeps the build
-on disk, keyed on the module source, the OpenMP setting, the Pyccel and Python
-versions and the platform, so only the first run after an edit compiles:
+on disk, keyed on the module source, the OpenMP setting, the Pyccel, NumPy and
+Python versions and the platform, so only the first run after an edit compiles:
 
 - builds go to `~/.cache/<name>/kernels`, or the directory in
   `<NAME>_KERNEL_CACHE`; if that is not writable, to `<tmp>/<name>-kernels`;
 - modules are built with OpenMP; `<NAME>_OPENMP=0` builds them serial, except
   modules that call the OpenMP runtime;
 - builds are published with an atomic rename, so MPI ranks compiling the same
   module on a cold cache never load a partial build;
+- a new build is imported once in a subprocess before it is published; if that
+  import fails or crashes the interpreter (an extension linked to `libpython` in
+  a Python that has it built in), `compile` raises instead and nothing is
+  stored, so the process that uses the cache does not crash;
 - the key covers the module's own source only: after editing a module the
   kernel imports, edit the kernel or clear the cache.
 

src/cunumpy/_pyccel_cache.py

@@ -2,10 +2,12 @@
 
 :class:`PyccelCache` builds a module with the ``pyccel compile`` command line
 interface, in a subprocess, once. It keys a build on the module source, the
-target language, the OpenMP setting, the Pyccel and Python versions and the
-platform, and stores it under a per-package cache directory. Later processes,
-and other MPI ranks, import the stored extension directly; only the first run
-after an edit compiles.
+target language, the OpenMP setting, the Pyccel, NumPy and Python versions and
+the platform, and stores it under a per-package cache directory. Later
+processes, and other MPI ranks, import the stored extension directly; only the
+first run after an edit compiles. A new build is imported once in a subprocess
+before it is stored, so a build this interpreter cannot load is an error, not a
+crash of the process that uses it.
 
 cunumpy needs no Pyccel to import this module: Pyccel is looked up when a
 module is compiled, and a missing Pyccel is an :class:`ImportError` there, which
@@ -64,20 +66,77 @@ def _writable(directory: Path) -> bool:
     return os.access(directory, os.W_OK | os.X_OK)
 
 
-def _load_extension(directory: Path, stem: str) -> ModuleType | None:
+def _extension_path(directory: Path, stem: str) -> Path | None:
     for suffix in importlib.machinery.EXTENSION_SUFFIXES:
         path = directory / f"{stem}{suffix}"
-        if not path.is_file():
-            continue
+        if path.is_file():
+            return path
+    return None
+
+
+def _load_extension(directory: Path, stem: str) -> ModuleType | None:
+    path = _extension_path(directory, stem)
+    if path is not None:
         spec = importlib.util.spec_from_file_location(stem, path)
-        if spec is None or spec.loader is None:
-            continue
-        extension = importlib.util.module_from_spec(spec)
-        spec.loader.exec_module(extension)
-        return extension
+        if spec is not None and spec.loader is not None:
+            extension = importlib.util.module_from_spec(spec)
+            spec.loader.exec_module(extension)
+            return extension
     return None
 
 
+#: Imports the extension at ``argv[1]`` as module ``argv[2]``.
+_IMPORT_CHECK = (
+    "import importlib.util, sys\n"
+    "spec = importlib.util.spec_from_file_location(sys.argv[2], sys.argv[1])\n"
+    "spec.loader.exec_module(importlib.util.module_from_spec(spec))\n"
+)
+
+
+def _check_importable(directory: Path, stem: str) -> None:
+    """Import a new build in a subprocess; raise if this interpreter cannot.
+
+    A build that crashes the interpreter on import (an extension linked to
+    ``libpython`` loaded into a Python with ``libpython`` built in, as with some
+    standalone CPython builds, runs two Pythons in one process) would otherwise
+    crash every process that loads it from the cache.
+    """
+    path = _extension_path(directory, stem)
+    if path is None:
+        raise ImportError(f"pyccel compile wrote no extension module for {stem}")
+    result = subprocess.run(
+        [sys.executable, "-c", _IMPORT_CHECK, str(path), stem],
+        capture_output=True,
+        text=True,
+        check=False,
+        cwd=directory,
+    )
+    if result.returncode == 0:
+        return
+    how = (
+        f"crashed with signal {-result.returncode}"
+        if result.returncode < 0
+        else f"failed ({result.returncode})"
+    )
+    hint = (
+        "\nAn extension linked to libpython crashes a Python that has libpython "
+        "built in; `otool -L` (macOS) or `ldd` shows what it links."
+        if result.returncode < 0
+        else ""
+    )
+    raise RuntimeError(
+        f"importing the new build of {stem} with {sys.executable} {how}; it is "
+        f"not stored:\n{result.stdout}{result.stderr}{hint}"
+    )
+
+
+def _numpy_version() -> str:
+    try:
+        return importlib.metadata.version("numpy")
+    except importlib.metadata.PackageNotFoundError:
+        return "none"
+
+
 def _pyccel_version() -> str:
     try:
         return importlib.metadata.version("pyccel")
@@ -183,9 +242,10 @@ def _openmp_for(self, module: ModuleType) -> bool:
     def build_key(self, module: ModuleType, language: str | None = None) -> str:
         """Return the key of `module`'s build, which changes with anything that changes it.
 
-        The module's own source, the language, the OpenMP setting, the Pyccel
-        and Python versions and the platform. Modules the kernel imports are not
-        part of it: edit the kernel (or clear the cache) after changing one.
+        The module's own source, the language, the OpenMP setting, the Pyccel,
+        NumPy and Python versions and the platform. Modules the kernel imports
+        are not part of it: edit the kernel (or clear the cache) after changing
+        one.
 
         Parameters
         ----------
@@ -211,6 +271,7 @@ def build_key(self, module: ModuleType, language: str | None = None) -> str:
             _LAYOUT,
             f"openmp={self._openmp_for(module)}",
             _pyccel_version(),
+            f"numpy={_numpy_version()}",
             sys.version,
             platform.platform(),
         ):
@@ -270,8 +331,9 @@ def compile(self, module: ModuleType, *, language: str | None = None) -> ModuleT
         ImportError
             If Pyccel is not installed.
         RuntimeError
-            With the output of ``pyccel compile`` if the build fails; callers
-            decide on a fallback.
+            With the output of ``pyccel compile`` if the build fails, or of
+            the import if this interpreter cannot import the new build (it is
+            then not stored); callers decide on a fallback.
         PermissionError
             If neither the cache nor the fallback directory is writable.
         """
@@ -293,6 +355,7 @@ def compile(self, module: ModuleType, *, language: str | None = None) -> ModuleT
         ).absolute()
         try:
             build = self._build(module, language, staging)
+            _check_importable(build, stem)
             try:
                 # Atomic, so concurrent builders never see a partial directory.
                 os.rename(build, target)

tests/unit/test_pyccel_cache.py

@@ -1,5 +1,6 @@
 """PyccelCache: compile once, keep the build on disk, reuse it in later processes."""
 
+import importlib.machinery
 import importlib.metadata
 import importlib.util
 import os
@@ -99,6 +100,51 @@ def test_key_changes_with_the_source_and_the_openmp_setting(
     assert cache.build_key(edited) != key
 
 
+def test_key_changes_with_the_numpy_version(make_module, cache, monkeypatch):
+    # a build is compiled against one NumPy's C API
+    monkeypatch.setattr(_pyccel_cache, "_pyccel_version", lambda: "test")
+    module = make_module()
+    monkeypatch.setattr(_pyccel_cache, "_numpy_version", lambda: "2.4.0")
+    key = cache.build_key(module)
+    monkeypatch.setattr(_pyccel_cache, "_numpy_version", lambda: "2.5.0")
+    assert cache.build_key(module) != key
+
+
+def _fake_build(content):
+    """A ``PyccelCache._build`` that writes `content` as the extension module."""
+
+    def build(self, module, language, staging):
+        output = staging / "build"
+        output.mkdir()
+        stem = module.__name__.rsplit(".", 1)[-1]
+        suffix = importlib.machinery.EXTENSION_SUFFIXES[0]
+        (output / f"{stem}{suffix}").write_bytes(content)
+        return output
+
+    return build
+
+
+def test_a_build_that_cannot_be_imported_is_not_stored(make_module, cache, monkeypatch):
+    # no compiler needed: the "build" is a file no interpreter can import
+    monkeypatch.setattr(_pyccel_cache, "_pyccel_version", lambda: "test")
+    monkeypatch.setattr(xp.kernels.PyccelCache, "_build", _fake_build(b"not a library"))
+    with pytest.raises(RuntimeError, match="not stored"):
+        cache.compile(make_module())
+    assert not list(cache.cache_root().iterdir())
+
+
+def test_a_build_that_crashes_the_interpreter_names_the_signal(
+    make_module, cache, monkeypatch
+):
+    monkeypatch.setattr(_pyccel_cache, "_pyccel_version", lambda: "test")
+    monkeypatch.setattr(xp.kernels.PyccelCache, "_build", _fake_build(b""))
+    crashed = SimpleNamespace(returncode=-11, stdout="", stderr="Segmentation fault")
+    monkeypatch.setattr(_pyccel_cache.subprocess, "run", lambda *a, **k: crashed)
+    with pytest.raises(RuntimeError, match="signal 11(.|\n)*libpython"):
+        cache.compile(make_module())
+    assert not list(cache.cache_root().iterdir())
+
+
 def test_a_module_calling_the_openmp_runtime_keeps_openmp(
     make_module, cache, monkeypatch
 ):