fix(detector): stop leaking memory when a lib init keeps failing#14
Conversation
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
There was a problem hiding this comment.
Pull request overview
Fixes an unbounded memory growth issue in detector wrapper libraries by avoiding re-raising the same cached exception instance on repeated failed init() calls (CPython grows __traceback__ on each raise, retaining caller locals).
Changes:
- Add
clone_exception()utility to create a fresh exception instance that preserves type/args/attributes but avoids reusing the same exception object. - Update multiple detector wrapper init functions to
raise clone_exception(_libInitializedException) from Nonewhen a cached init failure exists. - Add unit tests validating cloning behavior across tricky exception shapes and verifying that repeated raises do not grow the cached exception’s traceback.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| gpustack_runtime/detector/utils.py | Introduces clone_exception() helper used to avoid traceback growth when re-raising cached init failures. |
| gpustack_runtime/detector/pyrocmsmi/init.py | Re-raises a cloned cached init exception to prevent traceback-based memory growth. |
| gpustack_runtime/detector/pyamdsmi/init.py | Re-raises a cloned cached init exception to prevent traceback-based memory growth. |
| gpustack_runtime/detector/pynvml/init.py | Re-raises a cloned cached init exception to prevent traceback-based memory growth. |
| gpustack_runtime/detector/pydcmi/init.py | Re-raises a cloned cached init exception to prevent traceback-based memory growth. |
| gpustack_runtime/detector/pymtml/init.py | Re-raises a cloned cached init exception to prevent traceback-based memory growth. |
| gpustack_runtime/detector/pyhgml/init.py | Re-raises a cloned cached init exception to prevent traceback-based memory growth. |
| gpustack_runtime/detector/pyixml/init.py | Re-raises a cloned cached init exception to prevent traceback-based memory growth. |
| gpustack_runtime/detector/pymxsml/init.py | Re-raises a cloned cached init exception to prevent traceback-based memory growth. |
| tests/gpustack_runtime/detector/test_utils.py | Adds focused tests to ensure cloning preserves behavior and prevents cached traceback growth across repeated raises. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4b16d92 to
8747951
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a clone_exception utility function to prevent memory leaks caused by repeatedly re-raising cached exception objects, which accumulates traceback frames. This utility is integrated across multiple GPU detector libraries and verified with comprehensive unit tests. The feedback suggests improving the robustness of clone_exception by safely handling exceptions that lack a __dict__ attribute (such as those from C extensions or using __slots__) and preserving additional attributes like __notes__ and __suppress_context__.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
8747951 to
57f8b44
Compare
The detector wrappers cache the library init failure and re-raise the same exception object on every subsequent init() call to fail fast. Re-raising one object is a leak: CPython appends a frame to its __traceback__ on each raise, and those frames retain the caller's (detect()) locals indefinitely. On a hybrid NVIDIA+AMD host where ROCm SMI init fails permanently, detect() calls rsmi_init() every heartbeat/metrics tick, so anonymous RSS grows without bound (gpustack/gpustack#5342). Raise a fresh clone of the cached exception instead, via a new clone_exception() helper. It clones with BaseException.__new__ + a copy of args/__dict__ rather than type(exc)(*exc.args), because some binding error types (e.g. amdsmi's AmdSmiLibraryException) take an error code and leave args empty, so they can't be reconstructed from args. Applied across all wrappers with the pattern: pyrocmsmi, pyamdsmi, pynvml, pydcmi, pymtml, pyhgml, pyixml, pymxsml.
57f8b44 to
81819ff
Compare
The detector wrappers cache the library init failure and re-raise the same exception object on every subsequent init() call to fail fast. Re-raising one object is a leak: CPython appends a frame to its traceback on each raise, and those frames retain the caller's (detect()) locals indefinitely.
On a hybrid NVIDIA+AMD host where ROCm SMI init fails permanently, detect() calls rsmi_init() every heartbeat/metrics tick, so anonymous RSS grows without bound (gpustack/gpustack#5342).
Raise a fresh clone of the cached exception instead, via a new clone_exception() helper. It clones with BaseException.new + a copy of args/dict rather than type(exc)(*exc.args), because some binding error types (e.g. amdsmi's AmdSmiLibraryException) take an error code and leave args empty, so they can't be reconstructed from args.
Applied across all wrappers with the pattern: pyrocmsmi, pyamdsmi, pynvml, pydcmi, pymtml, pyhgml, pyixml, pymxsml.