Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions cuda_core/cuda/core/_memory/_device_memory_resource.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,13 @@ class DeviceMemoryResource(_MemPool):
-------
The registered mapped memory resource. If one was previously registered
with the given key, it is returned.

Raises
------
RuntimeError
If the memory resource is not IPC-enabled.
ValueError
If the memory resource already has a different UUID.
"""
@classmethod
def from_allocation_handle(cls, device_id: Device | int, alloc_handle: int | IPCAllocationHandle) -> DeviceMemoryResource:
Expand Down
7 changes: 7 additions & 0 deletions cuda_core/cuda/core/_memory/_device_memory_resource.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,13 @@ cdef class DeviceMemoryResource(_MemPool):
-------
The registered mapped memory resource. If one was previously registered
with the given key, it is returned.

Raises
------
RuntimeError
If the memory resource is not IPC-enabled.
ValueError
If the memory resource already has a different UUID.
"""
return <DeviceMemoryResource>(_ipc.MP_register(self, uuid))

Expand Down
12 changes: 10 additions & 2 deletions cuda_core/cuda/core/_memory/_ipc.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,11 @@ cdef _MemPool MP_from_allocation_handle(cls, alloc_handle):
# Register it.
if uuid is not None:
registered = self.register(uuid)
assert registered is self
if registered is not self:
raise RuntimeError(
"internal cuda.core error, please report: register() returned a "
"different memory resource than the one being registered"
)

return self

Expand All @@ -476,7 +480,11 @@ cdef _MemPool MP_register(_MemPool self, uuid):
return existing
if not self.is_ipc_enabled:
raise RuntimeError("Memory resource is not IPC-enabled")
assert self.uuid is None or self.uuid == uuid
if self.uuid is not None and self.uuid != uuid:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

important: Check the resource UUID before the registry lookup. If a UUID already belongs to another resource, MP_register returns that resource at Lines 477–480. The new check never runs, so mr.register(other_uuid) accepts a UUID that differs from mr.uuid. Move the mismatch check before the existing-entry return.

raise ValueError(
f"Cannot register memory resource with UUID {uuid}: "
f"the resource already has UUID {self.uuid}"
)
registry[uuid] = self
self._ipc_data._alloc_handle._uuid = uuid
return self
Expand Down
7 changes: 7 additions & 0 deletions cuda_core/cuda/core/_memory/_pinned_memory_resource.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,13 @@ class PinnedMemoryResource(_MemPool):
-------
The registered mapped memory resource. If one was previously registered
with the given key, it is returned.

Raises
------
RuntimeError
If the memory resource is not IPC-enabled.
ValueError
If the memory resource already has a different UUID.
"""
@classmethod
def from_allocation_handle(cls, alloc_handle: int | IPCAllocationHandle) -> PinnedMemoryResource:
Expand Down
7 changes: 7 additions & 0 deletions cuda_core/cuda/core/_memory/_pinned_memory_resource.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,13 @@ cdef class PinnedMemoryResource(_MemPool):
-------
The registered mapped memory resource. If one was previously registered
with the given key, it is returned.

Raises
------
RuntimeError
If the memory resource is not IPC-enabled.
ValueError
If the memory resource already has a different UUID.
"""
return <PinnedMemoryResource>(_ipc.MP_register(self, uuid))

Expand Down
7 changes: 7 additions & 0 deletions cuda_core/docs/source/release/1.3.0-notes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,13 @@ New features
Fixes and enhancements
----------------------

- ``register()`` on a :class:`~_memory.DeviceMemoryResource` or a
:class:`PinnedMemoryResource` now raises ``ValueError`` when the resource
already has a different UUID. The check used to be an ``assert``, so under
``python -O`` a mismatched UUID silently rewrote the resource's UUID and
corrupted the IPC registry.
(`#2697 <https://github.com/NVIDIA/cuda-python/issues/2697>`__)

- A :class:`~_memory.DeviceMemoryResource` or a :class:`VirtualMemoryResource`
sent through ``multiprocessing`` now names its device, and the
``VirtualMemoryResource`` ``peers``, by UUID. The receiving process maps
Expand Down
31 changes: 31 additions & 0 deletions cuda_core/tests/memory_ipc/test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,37 @@ def test_register_rejects_non_ipc_memory_resource(mempool_device):
DeviceMemoryResource.from_registry(key)


@pytest.mark.human_authored
def test_register_rejects_mismatched_uuid(ipc_memory_resource):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

sed -n '1,140p' cuda_core/tests/memory_ipc/test_errors.py
sed -n '425,475p' cuda_core/tests/conftest.py
rg -n 'ipc_memory_resource|supports.*vmm|VirtualMemoryResource' cuda_core/tests/memory_ipc/conftest.py cuda_core/tests/conftest.py

Repository: NVIDIA/cuda-python

Length of output: 8888


🏁 Script executed:

sed -n '470,510p' cuda_core/tests/conftest.py
sed -n '140,230p' cuda_core/cuda/core/_memory/_virtual_memory_resource.pyi
nl -ba cuda_core/tests/memory_ipc/test_errors.py | sed -n '85,125p'

Repository: NVIDIA/cuda-python

Length of output: 7957


important: Restrict this test to pool resources.

When VMM IPC is supported, ipc_memory_resource also yields VirtualMemoryResource. That resource has no uuid or register API, so the test fails at mr.uuid before it checks UUID rejection. Limit this test to the device and pinned fixture parameters.

--- "a/cuda_core/tests/memory_ipc/test_errors.py"
+++ "b/cuda_core/tests/memory_ipc/test_errors.py"
@@ -94,8 +94,9 @@
         DeviceMemoryResource.from_registry(key)
 
 
+@pytest.mark.parametrize("ipc_memory_resource", ["device", "pinned"], indirect=True)
 @pytest.mark.human_authored
 def test_register_rejects_mismatched_uuid(ipc_memory_resource):
     """register() rejects a UUID that does not match the resource's own.
 
     The check is an explicit ValueError (not a bare ``assert``) so it remains

"""register() rejects a UUID that does not match the resource's own.

The check is an explicit ValueError (not a bare ``assert``) so it remains
enforced even when CPython runs with ``-O``, where assertions are removed
(see #2697). Registering under a foreign key would otherwise rewrite the
resource's UUID and silently corrupt the IPC registry.
"""
mr = ipc_memory_resource
if not is_pool_resource(mr):
pytest.skip("allocation handles and the registry are pool-only")
assert mr.is_ipc_enabled

# The resource must already carry its own UUID (assigned on creation).
original = mr.uuid
assert original is not None
other = uuid.uuid4()
while other == mr.uuid:
other = uuid.uuid4()

with pytest.raises(ValueError, match="already has UUID"):
mr.register(other)

# A rejected registration must leave the resource's own UUID unchanged.
assert mr.uuid == original

# Registering under the resource's own UUID still succeeds.
assert mr.register(mr.uuid) is mr


@pytest.mark.skipif(os.name == "nt", reason="IPC allocation handles are not supported on Windows")
@pytest.mark.agent_authored(model="gpt-5.6")
def test_ipc_allocation_handle_state_tracks_close():
Expand Down
Loading