fix(detector): stop leaking memory when a lib init keeps failing

gpustack_runtime/detector/__utils__.py

@@ -16,6 +16,38 @@
 from gpustack_runtime import envs
 
 
+def clone_exception(exc: BaseException) -> BaseException:
+    """
+    Build a fresh copy of an exception, without its traceback or chaining state.
+
+    The detector library wrappers cache the init failure and re-raise it on every
+    subsequent call to fail fast. Re-raising the *same* object is a memory leak:
+    CPython appends a new frame to its ``__traceback__`` on each raise, and those
+    frames retain the caller's locals indefinitely (gpustack/gpustack#5342).
+
+    Cloning via ``BaseException.__new__`` preserves the original type and
+    attributes regardless of the class's constructor signature -- some binding
+    error types take an error code and leave ``args`` empty (e.g. amdsmi's
+    ``AmdSmiLibraryException``), so ``type(exc)(*exc.args)`` cannot reconstruct them.
+
+    Args:
+        exc:
+            The exception to clone.
+
+    Returns:
+        A fresh exception of the same type, carrying the same args and attributes
+        but no traceback or ``__cause__``/``__context__`` chain.
+
+    """
+    clone = BaseException.__new__(type(exc))
+    clone.args = exc.args
+    # Copy instance state (custom attributes, and __notes__ if any live here too).
+    # Guard against exotic exception types that don't expose __dict__.
+    if hasattr(exc, "__dict__"):
+        clone.__dict__.update(exc.__dict__)
+    return clone
+
+
 @dataclass
 class PCIDevice:
     address: str

gpustack_runtime/detector/pyamdsmi/__init__.py

@@ -32,7 +32,12 @@ def amdsmi_init(flag=AmdSmiInitFlags.INIT_AMD_GPUS):
 
         if _libInitialized:
             if _libInitializedException is not None:
-                raise _libInitializedException
+                # Re-raise a fresh copy: re-raising the same cached exception object
+                # appends a traceback frame on every call, and those frames retain the
+                # caller's locals, leaking memory over time. See gpustack/gpustack#5342.
+                from ..__utils__ import clone_exception
+
+                raise clone_exception(_libInitializedException) from None
             return
 
         try:

gpustack_runtime/detector/pydcmi/__init__.py

@@ -780,7 +780,12 @@ def dcmi_init():
 
     if _libInitialized:
         if _libInitializedException is not None:
-            raise _libInitializedException
+            # Re-raise a fresh copy: re-raising the same cached exception object
+            # appends a traceback frame on every call, and those frames retain the
+            # caller's locals, leaking memory over time. See gpustack/gpustack#5342.
+            from ..__utils__ import clone_exception
+
+            raise clone_exception(_libInitializedException) from None
         return
 
     try:

gpustack_runtime/detector/pyhgml/__init__.py

@@ -2380,7 +2380,12 @@ def hgmlInitWithFlags(flags):
 
     if _libInitialized:
         if _libInitializedException is not None:
-            raise _libInitializedException
+            # Re-raise a fresh copy: re-raising the same cached exception object
+            # appends a traceback frame on every call, and those frames retain the
+            # caller's locals, leaking memory over time. See gpustack/gpustack#5342.
+            from ..__utils__ import clone_exception
+
+            raise clone_exception(_libInitializedException) from None
         return
 
     try:

gpustack_runtime/detector/pyixml/__init__.py

@@ -2131,7 +2131,12 @@ def nvmlInitWithFlags(flags):
 
     if _libInitialized:
         if _libInitializedException is not None:
-            raise _libInitializedException
+            # Re-raise a fresh copy: re-raising the same cached exception object
+            # appends a traceback frame on every call, and those frames retain the
+            # caller's locals, leaking memory over time. See gpustack/gpustack#5342.
+            from ..__utils__ import clone_exception
+
+            raise clone_exception(_libInitializedException) from None
         return
 
     try:

gpustack_runtime/detector/pymtml/__init__.py

@@ -15,7 +15,12 @@ def mtmlLibraryInit():
 
     if _libInitialized:
         if _libInitializedException is not None:
-            raise _libInitializedException
+            # Re-raise a fresh copy: re-raising the same cached exception object
+            # appends a traceback frame on every call, and those frames retain the
+            # caller's locals, leaking memory over time. See gpustack/gpustack#5342.
+            from ..__utils__ import clone_exception
+
+            raise clone_exception(_libInitializedException) from None
         return
 
     try:

gpustack_runtime/detector/pymxsml/__init__.py

@@ -778,7 +778,12 @@ def mxSmlInit():
 
     if _libInitialized:
         if _libInitializedException is not None:
-            raise _libInitializedException
+            # Re-raise a fresh copy: re-raising the same cached exception object
+            # appends a traceback frame on every call, and those frames retain the
+            # caller's locals, leaking memory over time. See gpustack/gpustack#5342.
+            from ..__utils__ import clone_exception
+
+            raise clone_exception(_libInitializedException) from None
         return
 
     try:
@@ -802,7 +807,12 @@ def mxSmlInitWithFlags(flags):
 
     if _libInitialized:
         if _libInitializedException is not None:
-            raise _libInitializedException
+            # Re-raise a fresh copy: re-raising the same cached exception object
+            # appends a traceback frame on every call, and those frames retain the
+            # caller's locals, leaking memory over time. See gpustack/gpustack#5342.
+            from ..__utils__ import clone_exception
+
+            raise clone_exception(_libInitializedException) from None
         return
 
     try:

gpustack_runtime/detector/pynvml/__init__.py

@@ -16,7 +16,12 @@ def nvmlInitWithFlags(flags):
 
     if _libInitialized:
         if _libInitializedException is not None:
-            raise _libInitializedException
+            # Re-raise a fresh copy: re-raising the same cached exception object
+            # appends a traceback frame on every call, and those frames retain the
+            # caller's locals, leaking memory over time. See gpustack/gpustack#5342.
+            from ..__utils__ import clone_exception
+
+            raise clone_exception(_libInitializedException) from None
         return
 
     try:

gpustack_runtime/detector/pyrocmsmi/__init__.py

@@ -180,7 +180,12 @@ def rsmi_init(flags=0):
 
     if _libInitialized:
         if _libInitializedException is not None:
-            raise _libInitializedException
+            # Re-raise a fresh copy: re-raising the same cached exception object
+            # appends a traceback frame on every call, and those frames retain the
+            # caller's locals, leaking memory over time. See gpustack/gpustack#5342.
+            from ..__utils__ import clone_exception
+
+            raise clone_exception(_libInitializedException) from None
         return
 
     try:

tests/gpustack_runtime/detector/test_detector_utils.py

@@ -0,0 +1,170 @@
+from __future__ import annotations
+
+import sys
+from typing import ClassVar
+
+import pytest
+
+from gpustack_runtime.detector.__utils__ import clone_exception
+
+
+# --------------------------------------------------------------------------- #
+# Exception classes that mimic the real binding error types, to exercise the   #
+# constructor shapes clone_exception has to survive.                           #
+# --------------------------------------------------------------------------- #
+class RocmLikeError(Exception):
+    """Custom __init__ that does NOT call super().__init__ (like ROCMSMIError)."""
+
+    def __init__(self, value):
+        self.value = value
+
+    def __str__(self):
+        return f"rocm {self.value}"
+
+
+class DcmiLikeError(Exception):
+    """Custom __new__(cls, value) that requires a positional arg (like DCMIError)."""
+
+    _mapping: ClassVar[dict] = {}
+
+    def __new__(cls, value):
+        target = DcmiLikeError._mapping.get(value, cls)
+        obj = Exception.__new__(target)
+        obj.value = value
+        return obj
+
+    def __str__(self):
+        return f"dcmi {self.value}"
+
+
+class AmdSmiLikeError(Exception):
+    """super().__init__() with no args -> args == (); state lives in __dict__.
+
+    This is the case that `type(exc)(*exc.args)` CANNOT reconstruct.
+    """
+
+    def __init__(self, err_code):
+        super().__init__()
+        self.err_code = err_code
+        self.err_info = f"info-{err_code}"
+
+    def __str__(self):
+        return f"Error code: {self.err_code} | {self.err_info}"
+
+
+def _tb_len(exc: BaseException) -> int:
+    n = 0
+    tb = exc.__traceback__
+    while tb:
+        n += 1
+        tb = tb.tb_next
+    return n
+
+
+ALL_CASES = [
+    RocmLikeError(-99997),
+    DcmiLikeError(-8007),
+    AmdSmiLikeError(43),
+    ValueError("boom"),
+    RuntimeError("multi", "arg", 3),
+]
+
+
+# --------------------------------------------------------------------------- #
+# Correctness: the clone must be usable in place of the original.              #
+# --------------------------------------------------------------------------- #
+def test_clone_preserves_type():
+    for exc in ALL_CASES:
+        clone = clone_exception(exc)
+        assert type(clone) is type(exc)
+
+
+def test_clone_is_a_distinct_object():
+    for exc in ALL_CASES:
+        clone = clone_exception(exc)
+        assert clone is not exc
+
+
+def test_clone_preserves_args_and_attributes_and_str():
+    for exc in ALL_CASES:
+        clone = clone_exception(exc)
+        assert clone.args == exc.args
+        assert clone.__dict__ == exc.__dict__
+        # str() relies on the preserved state; it must render identically.
+        assert str(clone) == str(exc)
+
+
+def test_clone_still_matches_except_clause():
+    # A caller doing `except RocmLikeError` must still catch the clone.
+    with pytest.raises(RocmLikeError) as info:
+        raise clone_exception(RocmLikeError(-1))
+    assert info.value.value == -1
+
+
+@pytest.mark.skipif(
+    sys.version_info < (3, 11),
+    reason="Exception.add_note() was added in Python 3.11",
+)
+def test_clone_preserves_notes():
+    # __notes__ (add_note, 3.11+) lives in __dict__, so it is carried over by
+    # the __dict__ copy without any special handling.
+    exc = ValueError("boom")
+    exc.add_note("first note")
+    exc.add_note("second note")
+    clone = clone_exception(exc)
+    assert clone.__notes__ == ["first note", "second note"]
+
+
+def test_clone_handles_amdsmi_style_empty_args():
+    # The whole reason we don't use `type(exc)(*exc.args)`: it blows up here.
+    exc = AmdSmiLikeError(7)
+    assert exc.args == ()
+    clone = clone_exception(exc)
+    assert clone.err_code == 7
+    assert clone.err_info == "info-7"
+
+
+# --------------------------------------------------------------------------- #
+# The point of the helper: cloning + re-raising must NOT grow the cached       #
+# exception's traceback (this is the gpustack/gpustack#5342 regression).       #
+# --------------------------------------------------------------------------- #
+def test_reraising_clones_does_not_grow_cached_traceback():
+    # Seed a traceback on the "cached" exception, like a first failed init.
+    with pytest.raises(RocmLikeError) as info:
+        raise RocmLikeError(-99997)
+    cached = info.value
+
+    baseline = _tb_len(cached)
+
+    raised = []
+    for _ in range(500):
+        with pytest.raises(RocmLikeError) as info:
+            raise clone_exception(cached) from None
+        raised.append(info.value)
+
+    # The cached object's traceback is untouched by the re-raises.
+    assert _tb_len(cached) == baseline
+    # Every raise produced a fresh object, never the cached one.
+    assert all(r is not cached for r in raised)
+
+
+def test_contrast_reraising_same_object_would_grow_traceback():
+    # Demonstrates the bug the helper fixes: re-raising the SAME object grows
+    # its traceback unbounded. (Sanity check that our test measures the right
+    # thing; not testing clone_exception here.)
+    with pytest.raises(RocmLikeError) as info:
+        raise RocmLikeError(-99997)
+    cached = info.value
+
+    before = _tb_len(cached)
+    for _ in range(50):
+        with pytest.raises(RocmLikeError):
+            raise cached
+    assert _tb_len(cached) > before
+
+
+def test_clone_severs_cause_and_context_when_raised_from_none():
+    with pytest.raises(ValueError, match="x") as info:
+        raise clone_exception(ValueError("x")) from None
+    assert info.value.__cause__ is None
+    assert info.value.__suppress_context__ is True