[fedonman]

Stream_handle_is_default_token now also matches handle 0, so a Stream over the NULL stream resolves its context and device on every call, as LEGACY_DEFAULT_STREAM and PER_THREAD_DEFAULT_STREAM do since #2485. The synchronous and virtual memory resources, which check the same predicate before recording a deallocation stream, now handle a handle-0 stream like the other two tokens as well. The test switches into a green context so it runs on a single GPU, and there is a 1.3.0 release note.

Closes #3242

[copy-pr-bot]

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.

[coderabbitai]

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Streams wrapping the NULL handle now resolve their context, device, resources, and recorded events using the current context, consistent with other default streams.
• Tests
  • Added coverage verifying that a NULL-handle stream follows context changes.

Handle 0 is now recognized as a default-stream token. NULL-handle streams resolve context-dependent properties and record operations against the current context. A green-context test checks context and event resolution, and the release notes describe the behavior.

Changes: NULL-stream context resolution

Layer / File(s)	Summary
Default-token resolution and validation cuda_core/cuda/core/_stream.pyx, cuda_core/tests/test_green_context.py, cuda_core/docs/source/release/1.3.0-notes.rst	The default-token check now includes handle 0. The test checks context resolution before, during, and after a green-context scope, and checks the recorded event's context. The release notes describe current-context resolution for NULL-stream operations.

Assessment against linked issues

Objective	Addressed	Explanation
NULL-handle streams resolve context-dependent properties and recorded events against the current context [#3242]	✅

Possibly related issues

• cuda.core: release synchronous allocations in stream order through a host callback instead of blocking #2989 — The shared default-token check now includes handle 0, which may relate to resource deallocation recorded on NULL-handle streams.

Possibly related PRs

• NVIDIA/cuda-python#3034 — This PR adds a NULL-stream release note that may need to move to the generated release-note mechanism described in #3034.

Suggested labels: bug, cuda.core

Suggested reviewers: andy-jost

Priority: Normal

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: Low · up to 2a33c

The change appears functional, but its regression test should cover all documented NULL-stream entry points.

Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[coderabbitai]

Nitpick comments: 1

cuda_core/tests/test_green_context.py: 1

565-573: Trivial · Maintainability & Code Quality · Quick win

suggestion: Add NULL-stream coverage for device, resources, and foreign wrappers.

The added test covers context and record() for Stream.from_handle(0) only. The existing default-stream test covers LEGACY_DEFAULT_STREAM and PER_THREAD_DEFAULT_STREAM, but not handle 0. Add device and resources assertions for the NULL stream, and create a foreign object whose __cuda_stream__ returns (0, 0). A handle-0 regression in either path can pass the current tests.

---

Review info

Run configuration

• Source mode: Auto
• Repository: NVIDIA/cuda-python
• Branch: main
• File: .coderabbit.yaml
• Commit: 590a8bf
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: f1a02a0e-4e1e-4be0-81d2-ff9be7ccfb0f

Commits

Reviewing files that changed from the base of the PR and between 590a8bf and 2a33c03.

Files selected for processing: 3

• cuda_core/cuda/core/_stream.pyx
• cuda_core/docs/source/release/1.3.0-notes.rst
• cuda_core/tests/test_green_context.py

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

[leofang]

Stream_handle_is_default_token now also matches handle 0, so a Stream over the NULL stream resolves its context and device on every call, as LEGACY_DEFAULT_STREAM and PER_THREAD_DEFAULT_STREAM do since #2485.

FYI just a note, this is true for the "re-resolve, don't cache" property this PR cares about. It is not true semantically under a green ctx: PTDS with a green ctx current is a hard CUDA_ERROR_INVALID_HANDLE ("CUDA Green Contexts cannot be used with stream-per-thread mode"), while handle 0 and CU_STREAM_LEGACY resolve to the green ctx's own null stream, which is a distinct stream object from the primary ctx's null stream (and only exists if the green ctx was created with CU_GREEN_CTX_DEFAULT_STREAM). Its default sync semantics are also different — PTDS-like non-blocking rather than the legacy sync-with-all, blocking behavior of the primary ctx's NULL stream.

[leofang]

@fedonman could you resolve conflicts?

[rahul05ranjan]

I traced this through the whole chain and it's a clean, correct extension of #2485. The predicate's consumers — Stream_get_ctx and Stream_get_ctx_device (so .context, .device, .resources, .record()) plus the synchronous and virtual memory resources that check the token before recording a deallocation stream — all pick up the handle-0 case automatically from the single-line predicate change, which matches the issue #3242 report exactly.

I specifically checked the one thing that could have broken the whole PR: Stream_check_open tests if not self._h_stream, which looks like it would reject a handle-0 stream as "closed". It doesn't, because _h_stream is a std::shared_ptr<const CUstream> and create_stream_handle_ref boxes the raw handle in a heap StreamBox (_cpp/rt/stream.cpp:111-123) — the shared_ptr is truthy regardless of the CUstream value it holds, so Stream.from_handle(0) passes every Stream_check_open gate. Worth having on record given how close that is.

Two more things I verified deliberately:

• Keeping Stream_is_legacy_default_token at 0x1-only is right: the docstring's quirk is that cuMemcpyWithAttributesAsync/cuMemcpyBatchAsync reject the LEGACY token outright, whereas the NULL stream is a real stream to those entry points — so widening the narrow check would have been a bug, not a gap.
• _from_handle still eagerly caches _h_context at construction (s._h_context = get_stream_context(h_stream)), including for handle 0; the fix works because Stream_get_ctx skips the sticky field whenever the token check is true, and the existing "defense in depth for #2485" comment covers this case now too.

One small doc touch-up to consider: the Note sections on the context, device, resources and record() docstrings still say a default stream means LEGACY_DEFAULT_STREAM or PER_THREAD_DEFAULT_STREAM — after this change the same statement applies to Stream.from_handle(0), so either enumerate it there or point at the predicate. The release note already documents it properly.

The test is well-placed (green context so it runs single-GPU, existing use_context helper, and it asserts both the context switch and that record() resolves against the current context — the exact stale-cache symptom from the issue).