Skip to content

Commit 34b1cd1

Browse files
committed
WIP: address closed-resource review feedback
1 parent d8ba7aa commit 34b1cd1

8 files changed

Lines changed: 58 additions & 21 deletions

File tree

cuda_core/cuda/core/_memory/_ipc.pyx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ from cuda.core._resource_handles cimport (
1616
deviceptr_import_ipc,
1717
get_last_error,
1818
as_cu,
19-
as_intptr,
2019
as_py,
2120
)
2221

@@ -111,7 +110,7 @@ cdef class IPCBufferDescriptor:
111110

112111

113112
cdef inline int IPCAllocationHandle_check_open(IPCAllocationHandle self) except -1:
114-
if not self._h_fd or as_intptr(self._h_fd) < 0:
113+
if self._h_fd.get() == NULL:
115114
raise RuntimeError("IPCAllocationHandle has been closed")
116115
return 0
117116

@@ -138,10 +137,10 @@ cdef class IPCAllocationHandle:
138137
@property
139138
def is_closed(self) -> bool:
140139
"""Whether this allocation handle has been closed."""
141-
return self._h_fd.get() == NULL or as_intptr(self._h_fd) < 0
140+
return self._h_fd.get() == NULL
142141

143142
def __int__(self) -> int:
144-
if not self._h_fd or as_intptr(self._h_fd) < 0:
143+
if self._h_fd.get() == NULL:
145144
raise ValueError(
146145
f"Cannot convert IPCAllocationHandle to int: the handle (id={id(self)}) is closed."
147146
)

cuda_core/cuda/core/_memory/_memory_pool.pyi

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,13 @@ class _MemPoolAttributes:
4040
class _MemPool(MemoryResource):
4141
def __init__(self) -> None: ...
4242
def close(self) -> None:
43-
"""
44-
Close the memory resource and destroy the associated memory pool
45-
if owned.
43+
"""Release this object's reference to the memory pool.
44+
45+
New allocations and operations requiring this object's pool handle are
46+
rejected afterward. For owned pools, release of the underlying pool's
47+
resources is deferred until all outstanding allocations are freed and
48+
pending free operations complete. :meth:`deallocate` remains available
49+
after :meth:`close` so existing allocations can still be released.
4650
"""
4751
@property
4852
def is_closed(self) -> bool:

cuda_core/cuda/core/_memory/_memory_pool.pyx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,13 @@ cdef class _MemPool(MemoryResource):
127127
self._attributes = None
128128

129129
def close(self) -> None:
130-
"""
131-
Close the memory resource and destroy the associated memory pool
132-
if owned.
130+
"""Release this object's reference to the memory pool.
131+
132+
New allocations and operations requiring this object's pool handle are
133+
rejected afterward. For owned pools, release of the underlying pool's
134+
resources is deferred until all outstanding allocations are freed and
135+
pending free operations complete. :meth:`deallocate` remains available
136+
after :meth:`close` so existing allocations can still be released.
133137
"""
134138
_MP_close(self)
135139

cuda_core/cuda/core/_stream.pyi

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@ class Stream:
7070
Releases the stream handle. For owned streams, this destroys the
7171
underlying CUDA stream. For borrowed streams, this releases the
7272
reference and allows the Python owner to be GC'd.
73+
74+
.. warning::
75+
Do not close :obj:`LEGACY_DEFAULT_STREAM` or
76+
:obj:`PER_THREAD_DEFAULT_STREAM`. They are shared module-level
77+
objects, so closing one invalidates it for the rest of the process.
7378
"""
7479
@property
7580
def is_closed(self) -> bool:

cuda_core/cuda/core/_stream.pyx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,12 @@ cdef class Stream:
200200
Releases the stream handle. For owned streams, this destroys the
201201
underlying CUDA stream. For borrowed streams, this releases the
202202
reference and allows the Python owner to be GC'd.
203+
204+
.. warning::
205+
Do not close :obj:`LEGACY_DEFAULT_STREAM` or
206+
:obj:`PER_THREAD_DEFAULT_STREAM`. They are shared module-level
207+
objects, so closing one invalidates it for the rest of the process.
203208
"""
204-
if self._h_stream and Stream_is_default_token(self):
205-
return
206209
self._h_stream.reset()
207210

208211
@property

cuda_core/cuda/core/graph/_adjacency_set_proxy.pyx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ class AdjacencySetProxy(MutableSet[GraphNode]):
6161
(<_AdjacencySetCore>self._core).check_owner_mutable()
6262
if not isinstance(value, GraphNode):
6363
return
64-
(<_AdjacencySetCore>self._core).check_mutation(value)
6564
if value not in self:
6665
return
66+
(<_AdjacencySetCore>self._core).check_mutation(value)
6767
(<_AdjacencySetCore>self._core).remove_edge(<GraphNode>value)
6868

6969
# --- override for bulk efficiency ---

cuda_core/tests/graph/test_graph_definition_mutation.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,22 @@ def test_add_wrong_type(init_cuda):
389389
node.succ.add(42)
390390

391391

392+
@pytest.mark.agent_authored(model="gpt-5.6")
393+
def test_discard_absent_invalid_value_is_noop(init_cuda):
394+
graph = GraphDefinition()
395+
owner = graph.empty()
396+
neighbor = graph.empty()
397+
destroyed = graph.empty()
398+
owner.succ.add(neighbor)
399+
destroyed.destroy()
400+
401+
foreign = GraphDefinition().empty()
402+
for value in ("not a node", destroyed, foreign, graph._entry):
403+
owner.succ.discard(value)
404+
405+
assert owner.succ == {neighbor}
406+
407+
392408
def test_cross_graph_edge(init_cuda):
393409
"""Adding an edge to a node from a different graph raises ValueError."""
394410
g1 = GraphDefinition()

cuda_core/tests/test_stream.py

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,18 +112,24 @@ def test_per_thread_default_stream():
112112

113113

114114
@pytest.mark.agent_authored(model="gpt-5.6")
115-
@pytest.mark.parametrize("stream", [LEGACY_DEFAULT_STREAM, PER_THREAD_DEFAULT_STREAM])
116-
def test_default_stream_close_is_noop(stream, init_cuda):
117-
"""Closing a process-wide default-stream token must not invalidate it."""
115+
@pytest.mark.parametrize(
116+
("handle", "singleton"),
117+
[
118+
(driver.CU_STREAM_LEGACY, LEGACY_DEFAULT_STREAM),
119+
(driver.CU_STREAM_PER_THREAD, PER_THREAD_DEFAULT_STREAM),
120+
],
121+
)
122+
def test_borrowed_default_stream_token_can_close(handle, singleton, init_cuda):
118123
Device().set_current()
119-
handle = int(stream.handle)
124+
stream = Stream.from_handle(int(handle))
125+
126+
assert stream is not singleton
127+
assert not stream.is_closed
120128

121-
stream.close()
122129
stream.close()
123130

124-
assert not stream.is_closed
125-
assert int(stream.handle) == handle
126-
assert Stream_accept(stream) is stream
131+
assert stream.is_closed
132+
assert not singleton.is_closed
127133

128134

129135
@pytest.mark.agent_authored(model="gpt-5.6")

0 commit comments

Comments
 (0)