Repository navigation
test(cuda.core): keep the cycle collector out of the latch window in test_error_timing_incomplete - #3310
Merged
Conversation
…test_error_timing_incomplete The test failed with "DID NOT RAISE" after about 60 s, the latch timeout, on one CI row. The likely cause is a finalizer that the cycle collector runs while the latch holds the stream: a free of memory synchronizes the context and waits for the latch. Collect before the launch and disable the collector until the latch is released. Create the end event before the launch. Use a 10 s latch and fail with the blocked time when the record call stalls. Closes NVIDIA#3296. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Andy-Jost
added
bug
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Latch timing and cleanup cuda_core/tests/test_event.py |
The test sets a 10-second latch timeout, prepares the event before launching the latch, and disables garbage collection during the latch window. It checks recording latency and incomplete-event subtraction, then releases the latch and restores garbage collection if it was previously enabled. |
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
Reliably check subtraction while the second event is incomplete and always release the latch during cleanup [#3296] |
❓ | The test adds timing checks and cleanup. The issue does not identify the blocking operation, so the available evidence does not confirm that the change prevents the intermittent failure. |
Possibly related issues
Suggested labels
test
Suggested reviewers
carterbox, leofang
Change: Bug fix · Severity of issue fixed: Medium
Merge Risk: 
55565
-
A delayed test can still fail without a timing diagnostic. This is a narrow CI reliability risk.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
-
-
- Source mode: Auto
- Repository: NVIDIA/cuda-python
- Branch: main
- File: .coderabbit.yaml
- Commit: 80b3359
- Layers: Repository YAML
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
7ff23b41-5f70-4595-8bb1-5badaed58a60
-
cuda_core/tests/test_event.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.
-
Contributor
|
Start the blocked-time check before the latch launch, so that a stall in the launch or the record reports the time. Re-enable the collector in its own finally block. Request init_cuda, as the test builds a pinned memory resource through the latch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jpascucci-nv
approved these changes
Nov 2, 2026
| latch = LatchKernel(device) | ||
| @pytest.mark.agent_authored(model="claude-opus-5-5") | ||
| def test_error_timing_incomplete(init_cuda): | ||
| device = init_cuda |
Contributor
There was a problem hiding this comment.
Is this change coherent, instead of device = Device(), device.set_current()? It looks like we're not testing that behavior now. (this may be fine)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
test_error_timing_incompletefailed with "DID NOT RAISE" on 3 of the last 3 nightlies, always repetition 3 of 5 on the Windows T4 x2 TCC row, and each failure took about 60 s, the latch timeout. The test's own calls do not wait for the latch, so the host most likely blocked in a finalizer that the cycle collector ran while the latch held the stream: a free of memory synchronizes the context and waits for the latch. The analysis is in #3296.This change keeps the collector out of that window and makes a future stall fail fast with the blocked time instead of "DID NOT RAISE".
Changes
cuda_core/tests/test_event.py:test_error_timing_incompletecollects garbage and disables the collector before the latch launch, and releases the latch and enables the collector again in afinallyblock.Related Work
Closes #3296.
🤖 Generated with Claude Code