Skip to content

test(cuda.core): pop the context when the init_cuda leftover warning aborts setup - #3082

Merged
juenglin merged 1 commit into
NVIDIA:mainfrom
juenglin:init-cuda-followup
Oct 13, 2026
Merged

juenglin merged 1 commit into
NVIDIA:mainfrom
juenglin:init-cuda-followup

Conversation

@juenglin

@juenglin juenglin commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Description

Follow-up to #3053. The successful path is unchanged; this makes init_cuda clean up when its setup aborts.

pytest.ini escalates UnreleasedMemoryResourceWarning to an error, and init_cuda raises it during setup, after set_current(). The raise used to happen before the fixture's try, so the finally never ran and the context stayed current. Now everything after set_current() runs inside the try: the CUDA_CORE_TEST_BLOCKING_SYNC flag, the leftover check, and the live-count snapshot. If the warning fires, teardown still synchronizes and pops the context, and it skips the collect comparison that needs the snapshot. A failing set_current() still raises before the try, because nothing has been changed yet that would need undoing.

test_escalated_setup_warning_still_pops_the_context drives that path. It sets the error filter itself, so it does not depend on pytest.ini or command-line warning options. It fails against the previous conftest.py.

The comment on the VA-reservation counter in virtual_memory.cpp now states that memory-pool VA windows are not included: owned pools have their own count, and the device default pool is a borrowed reference that is not counted.

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@copy-pr-bot

copy-pr-bot Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the cuda.core Everything related to the cuda.core module label Oct 12, 2026
@juenglin juenglin added enhancement Any code-related improvements P1 Medium priority - Should do labels Oct 12, 2026
@juenglin juenglin self-assigned this Oct 12, 2026
@juenglin juenglin added this to the cuda.core 1.3.0 milestone Oct 12, 2026
@juenglin juenglin changed the title test(cuda.core): pop the context when the leftover warning aborts init_cuda setup fix(cuda.core): pop the context on an aborted init_cuda setup and roll back an unboxed pool or VA reservation Oct 12, 2026
@juenglin juenglin changed the title fix(cuda.core): pop the context on an aborted init_cuda setup and roll back an unboxed pool or VA reservation cuda.core: harden cleanup when init_cuda setup aborts or a host box allocation throws Oct 12, 2026
@juenglin

Copy link
Copy Markdown
Contributor Author

/ok to test d0bce1a

@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor
Doc Preview CI
Preview removed because the pull request was closed or merged.

…t_cuda setup

UnreleasedMemoryResourceWarning is escalated to an error and was raised before
the fixture try, so teardown never popped the context. Raise it inside the try,
 and correct the VA-count comment that attributed the NVIDIA#2381 pool leaks to
cuMemAddressReserve.
@juenglin juenglin changed the title cuda.core: harden cleanup when init_cuda setup aborts or a host box allocation throws test(cuda.core): pop the context when the init_cuda leftover warning aborts setup Oct 12, 2026
@juenglin juenglin added test Improvements or additions to tests and removed enhancement Any code-related improvements P1 Medium priority - Should do labels Oct 12, 2026
@juenglin

Copy link
Copy Markdown
Contributor Author

/ok to test a32461f

@juenglin
juenglin requested a review from Andy-Jost October 12, 2026 20:28
@juenglin
juenglin marked this pull request as ready for review October 12, 2026 20:28
@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: d77f0dd2-4a00-418f-a1ce-6e5cb66b744b

📥 Commits

Reviewing files that changed from the base of the PR and between 632bfc4 and a32461f.


📒 Files selected for processing (3)
  • cuda_core/cuda/core/_cpp/rt/virtual_memory.cpp
  • cuda_core/tests/conftest.py
  • cuda_core/tests/memory/test_init_cuda_collect_gate.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.



📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • CUDA test setup now cleans up the active device context when initialization fails, helping prevent test failures from leaving the context in an inconsistent state.
  • Other Changes
    • Clarified how live virtual address reservations are counted. No user-facing functionality changes are included in this update.

Walkthrough

The change clarifies which VA reservations the live count includes. It also ensures fixture teardown runs when setup raises and adds a test that checks the CUDA context is cleared.

Changes

CUDA resource accounting and fixture cleanup

Layer / File(s) Summary
Resource counts and fixture cleanup
cuda_core/cuda/core/_cpp/rt/virtual_memory.cpp, cuda_core/tests/conftest.py, cuda_core/tests/memory/test_init_cuda_collect_gate.py
The comment clarifies which VA reservations the live count includes. The fixture skips resource-count checks if setup fails before recording a snapshot, while still synchronizing and unsetting the current device. The test checks that the context is cleared when a setup warning raises as an error.

Priority: ⚪ Not assessed

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a3246

No actionable issue is identified; the PR is mergeable after normal checks.


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

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

@juenglin
juenglin merged commit 90eb8b0 into NVIDIA:main Oct 13, 2026
227 of 229 checks passed
github-actions Bot pushed a commit that referenced this pull request Oct 14, 2026
Removed preview folders for the following PRs:
- PR #2940
- PR #2993
- PR #3060
- PR #3082
- PR #3085
- PR #3086
- PR #3087
- PR #3091
- PR #3096
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.core Everything related to the cuda.core module test Improvements or additions to tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants