[controllers] fix: clear state on thread start failure

[Wangmerlyn]

Summary

• clear CUDA controller _thread and _stop_evt when Thread.start() raises
• clear MPS controller _thread and _stop_evt when Thread.start() raises
• add CUDA and MPS regressions mirroring the existing ROCm cleanup behavior

Why

A thread/resource startup failure can happen before a keep worker ever runs. CUDA and MPS previously left a stale thread object and stop event behind on that path, while ROCm already cleaned them up. This aligns all single-GPU controllers with the startup lifecycle contract in AGENTS.md.

Local verification

• RED before fix: the two new tests failed because _thread retained the fake FailingThread object.
• GREEN after fix:
  • PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=$PWD/src pytest -p no:cacheprovider tests/cuda_controller/test_keep_and_release.py::test_cuda_keep_clears_state_when_thread_start_fails tests/macm_controller/test_macm_backoff.py::test_macm_keep_clears_state_when_thread_start_fails -q (2 passed)
• Controller shard:
  • PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=$PWD/src pytest -p no:cacheprovider tests/cuda_controller/test_keep_and_release.py tests/macm_controller/test_macm_backoff.py tests/rocm_controller/test_rocm_backoff.py -q (63 passed, 2 skipped)
• Wider backend/controller shard:
  • PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=$PWD/src pytest -p no:cacheprovider tests/cuda_controller tests/macm_controller tests/rocm_controller tests/single_gpu_controller tests/global_controller tests/utilities/test_platform_manager.py tests/utilities/test_gpu_info.py -q (259 passed, 11 skipped)
• pre-commit run --all-files --show-diff-on-failure
• Full suite:
  • PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=$PWD/src pytest -p no:cacheprovider tests -q (1022 passed, 11 skipped)
• Final pre-commit gate:
  • git diff --check
  • focused regression tests again (2 passed)

Local review

Local subagent review completed with no Critical, Important, or Minor findings. Reviewer confirmed the cleanup preserves the original exception, avoids unnecessary cache/vendor cleanup, matches ROCm behavior, and needs no docs update because the AGENTS lifecycle contract already covers this behavior.

Summary by CodeRabbit

• Bug Fixes
  • Improved GPU keep/startup handling so a failed background thread launch no longer leaves the controller in a partially initialized state.
  • If startup fails, internal controller state is now cleared before the error is returned.
• Tests
  • Added coverage to verify controller state is reset when thread startup fails on both CUDA and macOS GPU paths.

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[coderabbitai]

📝 Walkthrough

Walkthrough

CudaGPUController.keep() and MacMGPUController.keep() now wrap thread startup in try/except blocks that reset _thread and _stop_evt to None and re-raise the exception if start() fails. New tests verify this cleanup behavior for both controllers.

Changes

Thread Start Failure Cleanup

Layer / File(s)	Summary
CUDA controller cleanup src/keep_gpu/single_gpu_controller/cuda_gpu_controller.py, tests/cuda_controller/test_keep_and_release.py	keep() now catches exceptions from self._thread.start(), resets _thread and _stop_evt to None, and re-raises; a new test verifies this behavior when thread start fails.
MacM controller cleanup src/keep_gpu/single_gpu_controller/macm_gpu_controller.py, tests/macm_controller/test_macm_backoff.py	keep() now catches exceptions from self._thread.start(), resets _thread and _stop_evt to None, and re-raises; a new test verifies this behavior when thread start fails.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

• Wangmerlyn/KeepGPU#101: Modifies the same controller keep() startup failure handling and thread/stop-event state cleanup logic.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely matches the main change: clearing controller state when thread startup fails.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/cuda-mps-thread-start-cleanup

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/keep_gpu/single_gpu_controller/cuda_gpu_controller.py (1)

127-132: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider extracting shared thread-start cleanup helper.

The try/except-reset-reraise block is now duplicated verbatim across CUDA, MacM, and ROCm controllers (ROCm additionally calls _shutdown_rocm_smi()). Could be pulled into a small helper on BaseGPUController (e.g., accepting an optional cleanup callback) to avoid drift between the three.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/keep_gpu/single_gpu_controller/cuda_gpu_controller.py` around lines 127 -
132, The thread-start failure cleanup in CudaGPUController.start is duplicated
across multiple GPU controllers, so extract it into a shared helper on
BaseGPUController to keep the reset-and-reraise behavior consistent. Move the
self._thread/self._stop_evt reset logic into a small reusable method that wraps
thread.start() and accepts an optional cleanup callback for controller-specific
work like ROCm’s _shutdown_rocm_smi(), then update the CUDA, MacM, and ROCm
start paths to call that helper instead of inlining the try/except block.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/keep_gpu/single_gpu_controller/cuda_gpu_controller.py`:
- Around line 127-132: The thread-start failure cleanup in
CudaGPUController.start is duplicated across multiple GPU controllers, so
extract it into a shared helper on BaseGPUController to keep the
reset-and-reraise behavior consistent. Move the self._thread/self._stop_evt
reset logic into a small reusable method that wraps thread.start() and accepts
an optional cleanup callback for controller-specific work like ROCm’s
_shutdown_rocm_smi(), then update the CUDA, MacM, and ROCm start paths to call
that helper instead of inlining the try/except block.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 45b8f5e5-dc30-49ec-a27e-a8a0b1efbc6c

📥 Commits

Reviewing files that changed from the base of the PR and between 9b0c10d and b0c158f.

📒 Files selected for processing (4)

• src/keep_gpu/single_gpu_controller/cuda_gpu_controller.py
• src/keep_gpu/single_gpu_controller/macm_gpu_controller.py
• tests/cuda_controller/test_keep_and_release.py
• tests/macm_controller/test_macm_backoff.py

[Wangmerlyn]

Resolved CodeRabbit nitpick: I checked the suggested shared helper and am leaving the cleanup explicit in this PR.

The duplicated block is intentionally tiny and backend-local, while ROCm has controller-specific vendor cleanup and CUDA/MPS do not. Moving this into BaseGPUController would couple the base class to private thread lifecycle fields and broaden a targeted startup-state fix into a refactor. This branch keeps the root-cause fix minimal, aligns CUDA/MPS with the existing ROCm behavior, and preserves the AGENTS.md "Linus simple" guidance.