cuda.core: reject operations on closed resource objects by Andy-Jost · Pull Request #2635 · NVIDIA/cuda-python

Andy-Jost · 2026-08-14T19:57:22Z

Description

Add a consistent closed-state contract across cuda.core resource objects so released native handles are rejected before reaching CUDA. Closeable resources now expose is_closed, while GraphDefinition and GraphNode expose is_valid for graph-lifetime invalidation. This change also adds tests ensuring bool(obj) returns true for closed and invalid objects, to retain backwards compatibility.

Active methods validate their own state and accepted resource arguments. Stream_accept() now rejects closed streams and graph builders, which also makes Buffer.set_deallocation_stream() and Buffer.close(stream=...) reject a closed stream without replacing the buffer's saved deallocation recipe. Closing CUDA default-stream tokens remains a no-op.

The same validation covers events, buffers, memory pools, IPC handles, contexts, compiler resources, graphs, arrays, textures, surfaces, and graphics resources. Tests cover named lifecycle state, backward-compatible truthiness, idempotent cleanup, safe inspection, cross-object validation, graph invalidation, and deallocation-stream failure atomicity.

Checklist

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

github-actions · 2026-08-14T20:12:26Z

Doc Preview CI
🚀 View preview at https://nvidia.github.io/cuda-python/pr-preview/pr-2635/
https://nvidia.github.io/cuda-python/pr-preview/pr-2635/cuda-core/
https://nvidia.github.io/cuda-python/pr-preview/pr-2635/cuda-bindings/
https://nvidia.github.io/cuda-python/pr-preview/pr-2635/cuda-pathfinder/
Preview will be ready when the GitHub Pages deployment is complete.

Andy-Jost · 2026-08-14T20:14:47Z

        q_bc.put(buffer)
-        buffer.close()

-        # Wait for C to receive before exiting.
+        # Queue serialization runs in a feeder thread. Keep the buffer open
+        # until C has received it and the parent releases this process.
        event_b.wait(timeout=CHILD_TIMEOUT_SEC)
+        buffer.close()


This fixes a latent bug. multiprocessing.Queue.put requires its argument to remain valid until received.

Add consistent liveness checks so closed handles cannot reach CUDA as valid resources, including graph and cross-object operations.

The allocation-handle constructor is intentionally unsupported on Windows, so limit its close-state test to supported platforms.

Replace lifecycle-dependent truthiness with explicit is_closed and is_valid properties while preserving the historical truth value of cuda.core objects.

…-validation

mdboom

General comment -- move the helper functions to inline implementations in the .pxd. Then I see a mix of calling these helper functions and doing an if closed: raise(...). Is there a reason for that difference? If not, maybe consistently use the helper functions?

mdboom · 2026-08-17T21:20:17Z

    return tuple(out)


+cdef int Buffer_check_open(Buffer self) except -1:


Since it's called on basically every path, inlining should help. Since this is used from other modules, move the implementation to the .pxd and add the inline keyword.

mdboom · 2026-08-17T21:21:08Z

 ) except? -1

 cdef int MP_raise_release_threshold(_MemPool self) except? -1
+cdef int MP_check_open(_MemPool self) except -1


Move the implementation here and inline?

mdboom · 2026-08-17T21:23:32Z


 def _get_int_attr(buf: Buffer, attribute: Any) -> int:
+    if buf.is_closed:
+        raise RuntimeError("Buffer has been closed")


Buffer_check_open(buf)?

I don't think the Cython function can be called from a pure Python module.

Ah. I missed that this was a .py file. Makes sense.

mdboom · 2026-08-17T21:23:59Z

+cdef int MP_check_open(_MemPool self) except -1:
+    if not self._h_pool:
+        raise RuntimeError(f"{self.__class__.__name__} has been closed")
+    return 0


Move to .pxd and inline.

Centralize open and valid state checks so hot Cython call paths use one consistent implementation.

Andy-Jost · 2026-08-18T19:27:37Z

General comment -- move the helper functions to inline implementations in the .pxd. Then I see a mix of calling these helper functions and doing an if closed: raise(...). Is there a reason for that difference? If not, maybe consistently use the helper functions?

Generally agree. The latest change consolidates open-state checks in inline functions. Shared Cython checks live in .pxd files, and module-local checks live in .pyx files. The remaining exceptions involve pure Python modules or circular Cython dependencies.

Details:

_managed_buffer.py is pure Python, so it cannot call the cdef checker from _buffer.pxd. It now uses a local Python _check_open() helper.
_virtual_memory_resource.py is also pure Python. It has only one check, so it uses buf.is_closed directly.
_stream.pyx cannot directly cimport GB_check_open without creating a circular dependency. Stream_accept() instead accesses arg.stream, whose property calls the local inline checker.

Expect the shared Context checker message so the green-context test matches the standardized validation path.

juenglin · 2026-08-18T21:49:25Z

Python's precedent is closed.

>>> open("README.md", "r").closed
False

We don't need to follow it, of course.

mdboom

Looks much better from the human-review standpoint.

Claude flagged a few things that seem like legitimate issues, but I don't have the full context to evaluate them.

mdboom · 2026-08-19T19:08:42Z

        reference and allows the Python owner to be GC'd.
        """
+        if self._h_stream and Stream_is_default_token(self):
+            return


I think this one is legit, but I don't fully understand how these special Stream singletons work.

From Claude:

Stream.close() early-returns for any stream whose raw handle equals CU_STREAM_LEGACY/CU_STREAM_PER_THREAD, because Stream_is_default_token() compares raw handle values, not identity against the two default-stream singletons. Stream.from_handle(1) / Stream.from_handle(2) — legitimate user-created borrowed wrappers — become permanently un-closeable: the owner reference is never released and is_closed stays False forever, contradicting the method's own docstring. Fix: guard on self is LEGACY_DEFAULT_STREAM or self is PER_THREAD_DEFAULT_STREAM, not on handle value. The new test (test_raw_null_stream_is_live_until_closed) uses from_handle(0), which happens to sidestep this exact case.

mdboom · 2026-08-19T19:11:24Z

            asynchronously. Must be passed explicitly; pass
            ``device.default_stream`` to use the default stream.
        """
        cdef Stream s = Stream_accept(stream)


I think whether Claude is right about this depends on whether double-calling cuMemFreeAsync is an error.

From Claude:

_MemPool.deallocate() (backing DeviceMemoryResource/PinnedMemoryResource/ManagedMemoryResource) never got an MP_check_open(self) call, unlike every sibling entry point in the same file (allocate, attributes, MP_raise_release_threshold, __reduce__, peer_accessible_by). After mr.close(), calling mr.deallocate(ptr, size, stream=...) sails straight through to cuMemFreeAsync against a destroyed pool — exactly the failure class this PR is meant to close everywhere else.

mdboom · 2026-08-19T19:13:04Z

        # Unpickling performs a live CUDA IPC import from descriptor bytes in the
        # pickle stream. Only deserialize Buffers from a trusted principal.
        # Must not serialize the parent's stream!
+        Buffer_check_open(self)


This one may or may not be legit. From Claude:

Adding Buffer_check_open(self) to __reduce__ breaks the queue.put(buffer); buffer.close() pattern: multiprocessing.Queue.put() serializes on a background feeder thread, so a race lets close() win and __reduce__ raises inside the feeder thread, where multiprocessing logs-and-discards it — the consumer just hangs with no error at the put() call site. The PR's own test (test_send_buffers.py:134) had to be reordered to work around this. This needs to be called out in the release notes (currently only mention rejecting closed resources, not this pickling/queue-handoff hazard).

mdboom · 2026-08-19T19:14:41Z

+    cdef inline void check_owner_mutable(self) except *:
+        if as_cu(self._h_graph) == NULL:
+            raise RuntimeError("GraphDefinition is no longer valid")
+        if as_cu(self._h_node) == NULL:


From Claude:

check_owner_mutable() treats any NULL _h_node as "destroyed," but GraphDefinition._entry (the virtual entry node) legitimately has _h_node == NULL by design — GN_check_valid correctly exempts it via _is_entry, but _AdjacencySetCore never captures that flag for the owner node. So graph_def._entry.succ.add(node) incorrectly raises "GraphNode has been destroyed" for a node that's actually valid. Exposure is limited (private _entry, not routed through this path by the public API today), but it's a real inconsistency in the exact contract this PR establishes, and untested.

mdboom · 2026-08-19T19:19:11Z

+        return self._h_fd.get() == NULL or as_intptr(self._h_fd) < 0
+
    def __int__(self) -> int:
        if not self._h_fd or as_intptr(self._h_fd) < 0:


Minor nit:

Suggested change

if not self._h_fd or as_intptr(self._h_fd) < 0:

if not self.is_closed:

mdboom · 2026-08-19T19:19:51Z

+    @property
+    def is_closed(self) -> bool:
+        """Whether this allocation handle has been closed."""
+        return self._h_fd.get() == NULL or as_intptr(self._h_fd) < 0


Minor DRY nit:

Suggested change

return self._h_fd.get() == NULL or as_intptr(self._h_fd) < 0

return IPCAllocationHandle_check_open(self)

mdboom · 2026-08-19T19:22:51Z

+        (<_AdjacencySetCore>self._core).check_owner_mutable()
        if not isinstance(value, GraphNode):
            return
+        (<_AdjacencySetCore>self._core).check_mutation(value)


Claude points out that this causes a diversion from the Python MutableSet.discard convention -- that an invalid value should just return and never raise. Therefore this check maybe belongs right before the remove_edge call.

Andy-Jost · 2026-08-20T20:23:20Z

Python's precedent is closed.
>>> open("README.md", "r").closed
False
We don't need to follow it, of course.

cuda.core consistently names Boolean properties as is_*.

Andy-Jost added this to the cuda.core 1.2.0 milestone Aug 14, 2026

Andy-Jost added bug Something isn't working P0 High priority - Must do! cuda.core Everything related to the cuda.core module labels Aug 14, 2026

Andy-Jost self-assigned this Aug 14, 2026

Andy-Jost commented Aug 14, 2026

View reviewed changes

Andy-Jost requested review from juenglin, leofang, mdboom and rparolin August 14, 2026 20:18

Andy-Jost force-pushed the ajost/closed-resource-validation branch from 0539baa to 31f74c3 Compare August 14, 2026 20:29

Andy-Jost mentioned this pull request Aug 14, 2026

cuda.core: allow updating Buffer deallocation streams #2602

Merged

Andy-Jost added 3 commits August 14, 2026 15:50

cuda.core: reject operations on closed resources

cf3875f

Add consistent liveness checks so closed handles cannot reach CUDA as valid resources, including graph and cross-object operations.

test(cuda.core): skip POSIX IPC handle test on Windows

6737bfa

The allocation-handle constructor is intentionally unsupported on Windows, so limit its close-state test to supported platforms.

test(cuda.core): use shared instrumentation for closed streams

84db682

Andy-Jost force-pushed the ajost/closed-resource-validation branch from 31f74c3 to 84db682 Compare August 14, 2026 22:51

Andy-Jost added 2 commits August 17, 2026 12:10

cuda.core: expose named resource state without changing truthiness

615d7d8

Replace lifecycle-dependent truthiness with explicit is_closed and is_valid properties while preserving the historical truth value of cuda.core objects.

Merge remote-tracking branch 'origin/main' into ajost/closed-resource…

bd21d3c

…-validation

mdboom requested changes Aug 17, 2026

View reviewed changes

Andy-Jost requested a review from mdboom August 18, 2026 16:51

Andy-Jost added 2 commits August 18, 2026 12:05

Merge origin/main into ajost/closed-resource-validation

250a442

cuda.core: inline resource state validation

fe2d27d

Centralize open and valid state checks so hot Cython call paths use one consistent implementation.

test(cuda.core): align closed context error check

868142e

Expect the shared Context checker message so the green-context test matches the standardized validation path.

mdboom requested changes Aug 19, 2026

View reviewed changes

		return tuple(out)


		cdef int Buffer_check_open(Buffer self) except -1:

	if not self._h_fd or as_intptr(self._h_fd) < 0:
	if not self.is_closed:

	return self._h_fd.get() == NULL or as_intptr(self._h_fd) < 0
	return IPCAllocationHandle_check_open(self)

Uh oh!

Conversation

Andy-Jost commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

github-actions Bot commented Aug 14, 2026

Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdboom left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andy-Jost commented Aug 18, 2026

Uh oh!

juenglin commented Aug 18, 2026

Uh oh!

mdboom left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andy-Jost commented Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Andy-Jost commented Aug 14, 2026 •

edited

Loading