Uh oh!
There was an error while loading. Please reload this page.
Fix Buffer.from_handle(mr=...) not calling mr.deallocate() - #1625
Conversation
Andy-Jost
commented
Feb 13, 2026
/ok to test c7fdab7f8547c4593f4d705554aa85fc428d5865 |
This comment has been minimized.
This comment has been minimized.
Uh oh!
There was an error while loading. Please reload this page.
| stream = Stream._from_handle(Stream, h_stream) | ||
| mr.deallocate(int(ptr), size, stream) | ||
| except Exception: | ||
| pass # Cannot propagate exceptions from a C++ destructor |
There was a problem hiding this comment.
| pass# Cannot propagate exceptions from a C++ destructor | |
| import sys | |
| print(f"Warning: mr.deallocate() failed during Buffer destruction: {exc}", | |
| file=sys.stderr) |
Just so that something is shown. I think this would mirror the Python interpreter's behavior.
Not a requirement, but would really help debugging I suspect.
| if (gil.acquired() && mr_dealloc_cb) { | ||
| mr_dealloc_cb(mr, b->resource, size, b->h_stream); | ||
| } | ||
| // Decref mr even if callback was skipped (prevent leak when | ||
| // callback is not registered -- should not happen in practice). | ||
| if (gil.acquired()) { | ||
| Py_DECREF(mr); | ||
| } |
There was a problem hiding this comment.
Seems like these branches should be merged into something like:
| if (gil.acquired() && mr_dealloc_cb) { | |
| mr_dealloc_cb(mr, b->resource, size, b->h_stream); | |
| } | |
| // Decref mr even if callback was skipped (prevent leak when | |
| // callback is not registered -- should not happen in practice). | |
| if (gil.acquired()) { | |
| Py_DECREF(mr); | |
| } | |
| if (gil.acquired()) { | |
| if (mr_dealloc_cb) { | |
| mr_dealloc_cb(mr, b->resource, size, b->h_stream); | |
| } | |
| // Decref mr regardless of whether the callback ran, as long as | |
| // we have the GIL. Leak is acceptable during interpreter finalization and shouldn't happen in practice. | |
| Py_DECREF(mr); | |
| } | |
| delete b; |
| gc.collect() | ||
| assert len(mr.active) == 0 | ||
There was a problem hiding this comment.
The two new tests cover close() (no stream) and GC, but neither exercises the path where close(stream=s) forwards the stream through the C++ deleter to mr.deallocate(ptr, size, stream). A third test that verifies the stream argument arrives at mr.deallocate() would close that gap, e.g.:
deftest_mr_deallocate_receives_stream():
device=Device()
device.set_current()
stream=device.create_stream()
received= {}
classStreamCaptureMR(TrackingMR):
defdeallocate(self, ptr, size, stream=None):
received['stream'] =streamsuper().deallocate(ptr, size, stream)
mr=StreamCaptureMR()
buf=mr.allocate(1024)
buf.close(stream)
assertreceived['stream'] isnotNonec7fdab7 to
373cb5fCompareAndy-Jost
commented
Feb 17, 2026
/ok to test 373cb5f5d193a61685b6fa06b44808749ba78c39 |
The RAII resource handle migration broke the contract where Buffer.from_handle(mr=mr) calls mr.deallocate() on close or GC. Add deviceptr_create_with_mr() which invokes a registered callback at destruction time, passing the deallocation stream from the handle. Co-authored-by: Cursor <cursoragent@cursor.com>
373cb5f to
4da1c8aCompareAndy-Jost
commented
Feb 17, 2026
/ok to test 4da1c8a |
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
Summary
Buffer.from_handle(ptr, size, mr=mr)now callsmr.deallocate(ptr, size, stream)when the buffer is closed or garbage collected, restoring the pre-RAII behavior.deviceptr_create_with_mr()to the resource handle infrastructure, whose shared_ptr deleter invokes a registered callback with the pointer, size, and deallocation stream.cuda.core.Streambefore callingmr.deallocate().streamparameter fromBuffer._init.Closes#1619
Changes
resource_handles.hpp/.cpp: AddMRDeallocCallbacktype,register_mr_dealloc_callback(), anddeviceptr_create_with_mr()._resource_handles.pxd/.pyx: Expose the new types and functions to Cython._buffer.pyx: Register the deallocation callback at module init; usedeviceptr_create_with_mrwhenmris provided inBuffer._init; update docstrings._legacy.py: Remove unusedstreamargument fromBuffer._initcalls.tests/helpers/buffers.py: AddTrackingMRtest helper.tests/test_memory.py: Addtest_mr_deallocate_called_on_closeandtest_mr_deallocate_called_on_gc.Test Plan
mr.deallocate()is called on explicitclose()and on GCMade with Cursor