Skip to content

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

Merged
thxCode merged 1 commit into
gpustack:mainfrom
aiwantaozi:fix/detector-init-exception-leak
Jul 6, 2026
Merged

fix(detector): stop leaking memory when a lib init keeps failing#14
thxCode merged 1 commit into
gpustack:mainfrom
aiwantaozi:fix/detector-init-exception-leak

Conversation

@aiwantaozi

Copy link
Copy Markdown
Collaborator

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.

Copilot AI review requested due to automatic review settings July 6, 2026 06:35
@gemini-code-assist

Copy link
Copy Markdown

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@aiwantaozi

Copy link
Copy Markdown
Collaborator Author

/gemini review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 None when 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.

@aiwantaozi aiwantaozi force-pushed the fix/detector-init-exception-leak branch from 4b16d92 to 8747951 Compare July 6, 2026 06:41

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread gpustack_runtime/detector/__utils__.py
@aiwantaozi aiwantaozi force-pushed the fix/detector-init-exception-leak branch from 8747951 to 57f8b44 Compare July 6, 2026 06:51
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.
@aiwantaozi aiwantaozi force-pushed the fix/detector-init-exception-leak branch from 57f8b44 to 81819ff Compare July 6, 2026 06:58
@aiwantaozi aiwantaozi requested a review from thxCode July 6, 2026 07:01

@thxCode thxCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thxCode thxCode merged commit 1d1e894 into gpustack:main Jul 6, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants