Fix: Add thread-safe locking to NVIDIA GPU detection to prevent metrics endpoint hanging by zhukai242 · Pull Request #8 · gpustack/runtime · GitHub
Skip to content

Fix: Add thread-safe locking to NVIDIA GPU detection to prevent metrics endpoint hanging - #8

Open
zhukai242 wants to merge 2 commits into
gpustack:mainfrom
zhukai242:main
Open

zhukai242 wants to merge 2 commits into
gpustack:mainfrom
zhukai242:main

Conversation

@zhukai242

Copy link
Copy Markdown

Problem:
The metrics endpoint would hang after repeated calls, particularly under concurrent load.
Analysis revealed that pynvml.nvmlInit() was getting stuck,
causing the generate_latest() function to block indefinitely.

Root Cause:
The NVML library is not designed to handle frequent initialization/shutdown cycles.
The original detect() method invoked pynvml.nvmlInit() and pynvml.nvmlShutdown() on every request,
which destabilized the library and led to hanging behavior under load.

Solution:

  1. Implement a class-level thread-safe locking mechanism using threading.Lock()
  2. Refactor the detect() method into a wrapper function that acquires the lock before execution
  3. Rename the original detect() implementation to _detect_impl() (per Python private method conventions)
  4. Enforce exclusive access to GPU detection logic, ensuring only one thread can execute it at a time

This prevents concurrent initialization of the NVML library and resolves the hanging issue in the metrics endpoint.

@gemini-code-assist

Copy link
Copy Markdown

@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 aims to address a critical hanging issue in the metrics endpoint by introducing a thread-safe lock for NVIDIA GPU detection. However, the current implementation is vulnerable to Denial of Service (DoS) due to incomplete locking. The threading.Lock used can lead to deadlocks in nested calls and should be replaced with threading.RLock. Furthermore, the fix is incomplete as other methods like is_supported() and get_topology() that interact with the NVML library are not protected, leaving them susceptible to the same concurrency issues.

Comment thread gpustack_runtime/detector/nvidia.py
Comment on lines +96 to +99
with self._detect_lock:
result = self._detect_impl()

return result

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This block can be simplified by returning the result of _detect_impl() directly from within the with statement. This makes the code more concise and avoids an unnecessary intermediate variable.

Suggested change
with self._detect_lock:
result = self._detect_impl()
return result
with self._detect_lock:
return self._detect_impl()

@thxCode

thxCode commented Mar 10, 2026

Copy link
Copy Markdown
Contributor
  1. Make lint green. You can do make lint on the laptop at first.
  2. Check the review comments generated by Gemini.
  3. Provide evidence and the circumstances under which this problem occurred. How much has the corresponding solution been optimized?
  4. If the issue stems from GPUStack calls, you can increase the GPUSTACK_WORKER_STATUS_SYNC_INTERVAL env in the GPUStack worker to avoid this. If the problem is caused by excessive metric scraping frequency, you can adjust your metric scraping settings.

@thxCode

thxCode commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

@thxCode
thxCode force-pushed the main branch 3 times, most recently from 6f14109 to 0618ac2 Compare March 17, 2026 03:00
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.

2 participants