Skip to content

Commit 57bced8

Browse files
Andy-Jostclaude
andcommitted
test(cuda.core): address review feedback on deallocation-stream PR
- Parametrize test_from_handle_mr_records_default_stream, test_from_handle_mr_records_explicit_stream, and test_from_handle_stream_requires_mr with [Buffer, ManagedBuffer] to cover the ManagedBuffer.from_handle entry point directly. - Add test_close_with_default_stream_requires_context covering the _require_deallocation_stream_context guard in Buffer_close. - Lift Stream_accept and default_stream to module-level imports. - Replace _require_deallocation_stream_context (a pre-flight that duplicated make_deallocation_stream's context check) with _apply_deallocation_stream, which calls set_deallocation_stream once and translates CUDA_ERROR_INVALID_CONTEXT into a descriptive RuntimeError. Removes the redundant cuCtxGetCurrent call on the default-stream success path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ffce5f4 commit 57bced8

2 files changed

Lines changed: 77 additions & 33 deletions

File tree

cuda_core/cuda/core/_memory/_buffer.pyx

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ from cuda.core._resource_handles cimport (
2323
as_intptr,
2424
as_cu,
2525
get_current_context,
26-
get_stream_context,
2726
set_deallocation_stream,
2827
)
2928
from cuda.core.typing import DevicePointerType
@@ -75,19 +74,21 @@ cdef void _mr_dealloc_callback(
7574
register_mr_dealloc_callback(_mr_dealloc_callback)
7675

7776

78-
cdef inline void _require_deallocation_stream_context(Stream s) except *:
79-
"""Default-stream tokens need a current context to pin into the free recipe."""
80-
cdef ContextHandle h_ctx
81-
if get_stream_context(s._h_stream):
82-
return
83-
h_ctx = get_current_context()
84-
if h_ctx:
85-
return
86-
raise RuntimeError(
87-
"Cannot record a default deallocation stream when no CUDA context is "
88-
"current. Call Device.set_current() first, or pass stream= with a "
89-
"non-default Stream."
90-
)
77+
cdef inline void _apply_deallocation_stream(
78+
const DevicePtrHandle& h_ptr, const StreamHandle& h_stream) except *:
79+
"""Record h_stream as the deallocation stream for h_ptr.
80+
81+
Translates CUDA_ERROR_INVALID_CONTEXT (default-stream token with no current
82+
context) into a descriptive RuntimeError instead of a raw CUDAError.
83+
"""
84+
cdef cydriver.CUresult status = set_deallocation_stream(h_ptr, h_stream)
85+
if status == cydriver.CUresult.CUDA_ERROR_INVALID_CONTEXT:
86+
raise RuntimeError(
87+
"Cannot record a default deallocation stream when no CUDA context is "
88+
"current. Call Device.set_current() first, or pass stream= with a "
89+
"non-default Stream."
90+
)
91+
HANDLE_RETURN(status)
9192

9293

9394
__all__ = ['Buffer', 'MemoryResource']
@@ -214,9 +215,8 @@ cdef class Buffer:
214215
cdef Stream s
215216
if mr is not None:
216217
s = Stream_accept(default_stream() if stream is None else stream)
217-
_require_deallocation_stream_context(s)
218218
self._h_ptr = deviceptr_create_with_mr(c_ptr, size, mr)
219-
HANDLE_RETURN(set_deallocation_stream(self._h_ptr, s._h_stream))
219+
_apply_deallocation_stream(self._h_ptr, s._h_stream)
220220
else:
221221
self._h_ptr = deviceptr_create_with_owner(c_ptr, owner)
222222
self._size = size
@@ -675,8 +675,7 @@ cdef inline void Buffer_close(Buffer self, object stream):
675675
# Update deallocation stream if provided
676676
if stream is not None:
677677
s = Stream_accept(stream)
678-
_require_deallocation_stream_context(s)
679-
HANDLE_RETURN(set_deallocation_stream(self._h_ptr, s._h_stream))
678+
_apply_deallocation_stream(self._h_ptr, s._h_stream)
680679
# Reset handle - RAII deleter will free the memory (and release owner ref in C++)
681680
self._h_ptr.reset()
682681
self._size = 0

cuda_core/tests/test_memory.py

Lines changed: 60 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
)
4646
from cuda.core._dlpack import DLDeviceType
4747
from cuda.core._memory._ipc import IPCBufferDescriptor
48+
from cuda.core._stream import Stream_accept, default_stream
4849
from cuda.core._utils.cuda_utils import CUDAError, handle_return
4950
from cuda.core.typing import (
5051
ManagedMemoryLocationType,
@@ -514,16 +515,15 @@ def deallocate(self, ptr, size, *, stream=None):
514515
assert received["stream"].handle == stream.handle
515516

516517

517-
def test_from_handle_mr_records_default_stream():
518-
"""When a Buffer is minted via :meth:`Buffer.from_handle` with ``mr`` but
519-
without an explicit ``stream=``, the deallocation stream is recorded at
518+
@pytest.mark.parametrize("buffer_type", [Buffer, ManagedBuffer])
519+
def test_from_handle_mr_records_default_stream(buffer_type):
520+
"""When a Buffer/ManagedBuffer is minted via :meth:`from_handle` with ``mr``
521+
but without an explicit ``stream=``, the deallocation stream is recorded at
520522
creation as ``default_stream()`` (not chosen later in the destructor).
521523
See `#2497`.
522524
"""
523525
import gc
524526

525-
from cuda.core._stream import Stream_accept, default_stream
526-
527527
device = Device()
528528
device.set_current()
529529
captured = {}
@@ -551,7 +551,7 @@ def deallocate(self, ptr, size, *, stream):
551551

552552
mr = StrictCapturingMR()
553553
# ptr=1 is fine because StrictCapturingMR.deallocate does not free.
554-
buf = Buffer.from_handle(1, 1024, mr=mr)
554+
buf = buffer_type.from_handle(1, 1024, mr=mr)
555555
del buf
556556
gc.collect()
557557

@@ -560,12 +560,11 @@ def deallocate(self, ptr, size, *, stream):
560560

561561

562562
@pytest.mark.agent_authored(model="cursor-grok-4.5")
563-
def test_from_handle_mr_records_explicit_stream():
564-
"""Buffer.from_handle(..., mr=mr, stream=s) stores s for teardown."""
563+
@pytest.mark.parametrize("buffer_type", [Buffer, ManagedBuffer])
564+
def test_from_handle_mr_records_explicit_stream(buffer_type):
565+
"""Buffer/ManagedBuffer.from_handle(..., mr=mr, stream=s) stores s for teardown."""
565566
import gc
566567

567-
from cuda.core._stream import Stream_accept
568-
569568
device = Device()
570569
device.set_current()
571570
stream = device.create_stream()
@@ -591,20 +590,68 @@ def deallocate(self, ptr, size, *, stream):
591590
captured["stream"] = Stream_accept(stream)
592591

593592
mr = StrictCapturingMR()
594-
buf = Buffer.from_handle(1, 1024, mr=mr, stream=stream)
593+
buf = buffer_type.from_handle(1, 1024, mr=mr, stream=stream)
595594
del buf
596595
gc.collect()
597596

598597
assert captured["stream"].handle == stream.handle
599598

600599

601600
@pytest.mark.agent_authored(model="cursor-grok-4.5")
602-
def test_from_handle_stream_requires_mr():
601+
@pytest.mark.parametrize("buffer_type", [Buffer, ManagedBuffer])
602+
def test_from_handle_stream_requires_mr(buffer_type):
603603
device = Device()
604604
device.set_current()
605605
stream = device.create_stream()
606606
with pytest.raises(ValueError, match="stream requires a memory resource"):
607-
Buffer.from_handle(1, 1024, stream=stream)
607+
buffer_type.from_handle(1, 1024, stream=stream)
608+
609+
610+
@pytest.mark.agent_authored(model="claude-sonnet-4-6")
611+
def test_close_with_default_stream_requires_context():
612+
"""Buffer.close(stream=default_stream()) raises when no context is current.
613+
614+
``default_stream()`` has no bound context, so the close path must find
615+
a current context to anchor the free. Without one it should raise rather
616+
than silently record an unusable stream handle.
617+
"""
618+
device = Device()
619+
device.set_current()
620+
stream = device.create_stream()
621+
622+
class NoopMR(MemoryResource):
623+
@property
624+
def is_device_accessible(self):
625+
return True
626+
627+
@property
628+
def is_host_accessible(self):
629+
return False
630+
631+
@property
632+
def device_id(self):
633+
return device.device_id
634+
635+
def allocate(self, size, *, stream):
636+
raise NotImplementedError
637+
638+
def deallocate(self, ptr, size, *, stream):
639+
pass
640+
641+
mr = NoopMR()
642+
# Use a real stream at creation so _init succeeds without a current context later.
643+
buf = Buffer.from_handle(1, 1024, mr=mr, stream=stream)
644+
645+
previous = handle_return(driver.cuCtxPopCurrent())
646+
assert int(previous) != 0
647+
try:
648+
assert int(handle_return(driver.cuCtxGetCurrent())) == 0
649+
with pytest.raises(RuntimeError, match="no CUDA context is current"):
650+
buf.close(stream=default_stream())
651+
finally:
652+
handle_return(driver.cuCtxSetCurrent(previous))
653+
654+
buf.close() # clean up using the recorded stream (which carries a context)
608655

609656

610657
@pytest.mark.agent_authored(model="cursor-grok-4.5")
@@ -648,8 +695,6 @@ def deallocate(self, ptr, size, *, stream):
648695
@pytest.mark.parametrize("buffer_type", [Buffer, ManagedBuffer])
649696
def test_from_handle_mr_explicit_stream_without_current_context(buffer_type):
650697
"""A context-bound stream makes owning from_handle context-independent."""
651-
from cuda.core._stream import Stream_accept
652-
653698
device = Device()
654699
device.set_current()
655700
stream = device.create_stream()

0 commit comments

Comments
 (0)