From 46bab1fd341d0d1c725ce1e9cf15ebbfe5e36c37 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 29 Oct 2026 22:57:49 -0700 Subject: [PATCH 1/3] cuda.core: preserve graph identity across native handle reuse Signed-off-by: Ralf W. Grosse-Kunstleve --- .../cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md | 7 +- .../cuda/core/_cpp/rt/REGISTRY_DESIGN.md | 15 +++ cuda_core/cuda/core/_cpp/rt/graph.cpp | 11 +- cuda_core/cuda/core/_cpp/rt/internal.hpp | 19 +++ cuda_core/tests/cython/build_tests.py | 11 +- cuda_core/tests/cython/test_cython.py | 3 +- .../cython/test_get_cuda_native_handle.pyx | 3 +- .../tests/cython/test_graph_registry.pyx | 110 ++++++++++++++++++ 8 files changed, 168 insertions(+), 11 deletions(-) create mode 100644 cuda_core/tests/cython/test_graph_registry.pyx diff --git a/cuda_core/cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md b/cuda_core/cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md index fdaf77785b2..0d40d6dd06c 100644 --- a/cuda_core/cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md +++ b/cuda_core/cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md @@ -172,8 +172,11 @@ metadata for the root graph and subgraphs. Each `GraphBox` stores: When CUDA destroys a child graph, the associated `GraphBox` is cleared and moved to `GraphHierarchy::graveyard`. That invalidates existing handles and preserves the storage until the hierarchy's last shared owner is released. -Removing the registry entry also prevents corruption if a future CUDA graph -reuses the raw handle value. +CUDA may reuse a child's raw handle before the old metadata is retired. +Child lookup checks the parent box and owner node, and cleanup removes only +its own resource generation's registry entry. This keeps old invalidation or +hierarchy destruction from adopting or erasing an independent graph's newer +metadata; see [registry identity](REGISTRY_DESIGN.md#reused-graph-handles). The process-wide graph registry maps a live `CUgraph` to its canonical `GraphHandle`. Node handles are scoped to their `GraphBox`, where they can all diff --git a/cuda_core/cuda/core/_cpp/rt/REGISTRY_DESIGN.md b/cuda_core/cuda/core/_cpp/rt/REGISTRY_DESIGN.md index f4c760163e6..96b9cb05519 100644 --- a/cuda_core/cuda/core/_cpp/rt/REGISTRY_DESIGN.md +++ b/cuda_core/cuda/core/_cpp/rt/REGISTRY_DESIGN.md @@ -32,6 +32,21 @@ with default metadata, losing information that was set at creation. Instances: `context_registry`, `stream_registry`, `event_registry`, `kernel_registry`, `graph_registry`, and one node registry per `GraphBox`. +### Reused graph handles + +CUDA can recycle a child graph's raw address after native destruction but +before its old metadata is retired. A child-graph lookup therefore accepts a +cached handle only when its parent `GraphBox` and owning node match the +requested relationship. Independent hierarchies must not share stale metadata +merely because CUDA reused an address. + +Graph cleanup uses identity-conditional unregistering: it erases the raw key +only if the entry is expired or still names the retiring resource slot. Compare +`shared_ptr::get()`, not its control block, because distinct graph generations +within one hierarchy share ownership but have different stable resource slots. +The temporary strong handle used for this check is released after unlocking +the registry mutex; its deleter may re-enter the registry. + ## Level 2: Resource Handle -> Python Object (Cython) `_node_registry` in `_graph_node.pyx` is a `WeakValueDictionary` diff --git a/cuda_core/cuda/core/_cpp/rt/graph.cpp b/cuda_core/cuda/core/_cpp/rt/graph.cpp index 13720e6a9b1..24532c3ea39 100644 --- a/cuda_core/cuda/core/_cpp/rt/graph.cpp +++ b/cuda_core/cuda/core/_cpp/rt/graph.cpp @@ -304,7 +304,7 @@ create_graph_handle(CUgraph graph) [](GraphHierarchy *hierarchy) { for (const GraphBox& box : hierarchy->graphs) { if (box.resource) { - graph_registry.unregister_handle(box.resource); + graph_registry.unregister_handle(box.resource, &box.resource); } } GraphBox *root = hierarchy->root(); @@ -330,11 +330,14 @@ create_child_graph_handle(CUgraph child_graph, const GraphHandle &h_parent, CUgr if (!child_graph || !h_parent || !owner_node) { return {}; } + GraphBox *parent = get_box(h_parent); if (GraphHandle h_graph = graph_registry.lookup(child_graph)) { - return h_graph; + const GraphBox *child = get_box(h_graph); + if (child->parent == parent && child->owner_node == owner_node) { + return h_graph; + } } - GraphBox *parent = get_box(h_parent); GraphHierarchy *hierarchy = parent->hierarchy; GraphBox &child = hierarchy->graphs.emplace_back(child_graph, hierarchy, parent, owner_node); @@ -659,7 +662,7 @@ retire_graph(GraphHierarchy &hierarchy, std::list::iterator graph) noe } } if (graph->resource) { - graph_registry.unregister_handle(graph->resource); + graph_registry.unregister_handle(graph->resource, &graph->resource); graph->resource = nullptr; } graph->attachments.clear(); diff --git a/cuda_core/cuda/core/_cpp/rt/internal.hpp b/cuda_core/cuda/core/_cpp/rt/internal.hpp index 270ee486aa2..d23286c0f78 100644 --- a/cuda_core/cuda/core/_cpp/rt/internal.hpp +++ b/cuda_core/cuda/core/_cpp/rt/internal.hpp @@ -193,6 +193,25 @@ class HandleRegistry { map_.erase(key); } + // A reused raw key may already name a newer resource generation. + void + unregister_handle(const Key &key, const typename Handle::element_type *expected) noexcept + { + // Releasing the last strong owner may re-enter this registry, so keep + // it alive until after the mutex is released. + Handle h; + { + std::lock_guard lock(mutex_); + auto it = map_.find(key); + if (it != map_.end()) { + h = it->second.lock(); + if (!h || h.get() == expected) { + map_.erase(it); + } + } + } + } + void register_handles(const std::vector &handles) { diff --git a/cuda_core/tests/cython/build_tests.py b/cuda_core/tests/cython/build_tests.py index 39539e3100c..0ae2d3531e6 100644 --- a/cuda_core/tests/cython/build_tests.py +++ b/cuda_core/tests/cython/build_tests.py @@ -8,6 +8,8 @@ so `cimport cuda.bindings.*` fails to locate the .pxd files. We resolve the namespace package's source root from `cuda.bindings.__file__` and pass it via `include_path=` so cythonize finds the .pxd tree on every platform. +Tests that cimport cuda.core also need this checkout's Core source root on +that search path; the editable install's Python finder does not supply it. When CUDA_PYTHON_CYTHON_CACHE_DIR is set, cythonize uses the same cache namespacing and include-path aliasing as the package build (``_build_shared.py``). @@ -54,6 +56,7 @@ def _bindings_source_root() -> Path: def _cythonize_tests(pyx_files): + core_source_root = Path(__file__).resolve().parents[2] cache_path = build_shared._cython_cache_path( "cuda-core-cython-tests", compiler_directives=_COMPILER_DIRECTIVES, @@ -68,7 +71,7 @@ def _cythonize_tests(pyx_files): if cache_path is None: return cythonize( pyx_files, - include_path=[str(_bindings_source_root())], + include_path=[str(core_source_root), str(_bindings_source_root())], **cythonize_kwargs, ) @@ -78,11 +81,12 @@ def _cythonize_tests(pyx_files): stdlib_target = Path(Cython.__file__).parent / "Includes" with ( build_shared._stable_cython_alias(stdlib_target, Path(".cython-stdlib-tests")) as rel_stdlib, + build_shared._stable_cython_alias(core_source_root, Path(".cython-core-tests")) as rel_core, build_shared._stable_cython_alias(_bindings_source_root(), Path(".cython-bindings-tests")) as rel_bindings, ): return cythonize( pyx_files, - include_path=[".", rel_bindings, rel_stdlib], + include_path=[".", rel_core, rel_bindings, rel_stdlib], **cythonize_kwargs, ) @@ -101,6 +105,9 @@ def main() -> None: # are stable regardless of the invoking cwd. os.chdir(script_dir) ext_modules = _cythonize_tests(pyx_files) + # Core's runtime consumer headers require C++17 with either toolchain. + for extension in ext_modules: + extension.extra_compile_args.append("/std:c++17" if os.name == "nt" else "-std=c++17") sys.argv = [sys.argv[0], "build_ext", "--inplace"] setup(name="cuda_core_cython_tests", ext_modules=ext_modules) diff --git a/cuda_core/tests/cython/test_cython.py b/cuda_core/tests/cython/test_cython.py index 87b97e8a2a4..ff40e9d841c 100644 --- a/cuda_core/tests/cython/test_cython.py +++ b/cuda_core/tests/cython/test_cython.py @@ -1,4 +1,4 @@ -# SPDX-FileCopyrightText: Copyright (c) 2021-2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-FileCopyrightText: Copyright (c) 2021-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # # SPDX-License-Identifier: Apache-2.0 @@ -21,6 +21,7 @@ def wrapped(*args, **kwargs): cython_test_modules = [ "test_get_cuda_native_handle", + "test_graph_registry", ] diff --git a/cuda_core/tests/cython/test_get_cuda_native_handle.pyx b/cuda_core/tests/cython/test_get_cuda_native_handle.pyx index 1480a3b96bd..fa56c586a61 100644 --- a/cuda_core/tests/cython/test_get_cuda_native_handle.pyx +++ b/cuda_core/tests/cython/test_get_cuda_native_handle.pyx @@ -1,9 +1,8 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # # SPDX-License-Identifier: Apache-2.0 # distutils: language = c++ -# distutils: extra_compile_args = -std=c++17 from libc.stdint cimport intptr_t diff --git a/cuda_core/tests/cython/test_graph_registry.pyx b/cuda_core/tests/cython/test_graph_registry.pyx new file mode 100644 index 00000000000..8cd3d9270ae --- /dev/null +++ b/cuda_core/tests/cython/test_graph_registry.pyx @@ -0,0 +1,110 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +# distutils: language = c++ +# distutils: include_dirs = ../../cuda/core + +"""Deterministic raw-handle reuse tests against the real graph registry. + +Parents are real empty CUDA graphs. Stack-local addresses stand in for reused +child and owner handles only inside the metadata API; they are never passed to +CUDA. Distinct addresses per invocation also keep parallel workers independent. +The parents retain ownership of their real graphs and destroy them normally. +""" + +from libc.stdint cimport uintptr_t +from cuda.bindings.cydriver cimport CUgraph, CUgraphNode +from cuda.core._rt cimport ( + GraphHandle, + as_cu, + create_child_graph_handle, + invalidate_child_graph_state, +) +from cuda.core.graph._graph_definition cimport GraphDefinition + +import pytest + + +@pytest.mark.agent_authored(model="gpt-6") +def test_child_graph_registry_rejects_foreign_parent(init_cuda): + """Reusing both raw addresses must not alias an independent parent tree.""" + cdef uintptr_t tokens[2] + cdef CUgraph raw_child = &tokens[0] + cdef CUgraphNode owner = &tokens[1] + cdef GraphDefinition old_parent = GraphDefinition() + cdef GraphDefinition new_parent = GraphDefinition() + cdef GraphHandle old_child = create_child_graph_handle( + raw_child, old_parent._h_graph, owner + ) + cdef GraphHandle same_child = create_child_graph_handle( + raw_child, old_parent._h_graph, owner + ) + assert same_child.get() == old_child.get() + + cdef GraphHandle new_child = create_child_graph_handle( + raw_child, new_parent._h_graph, owner + ) + assert new_child.get() != old_child.get() + assert as_cu(new_child) == raw_child + + invalidate_child_graph_state(old_parent._h_graph, owner) + assert as_cu(old_child) == NULL + assert as_cu(same_child) == NULL + assert as_cu(new_child) == raw_child + + same_child = create_child_graph_handle(raw_child, new_parent._h_graph, owner) + assert same_child.get() == new_child.get() + + +@pytest.mark.agent_authored(model="gpt-6") +def test_child_graph_registry_rejects_foreign_owner_in_same_parent(init_cuda): + """Retiring an old owner preserves the replacement within one hierarchy.""" + cdef uintptr_t tokens[3] + cdef CUgraph raw_child = &tokens[0] + cdef CUgraphNode old_owner = &tokens[1] + cdef CUgraphNode new_owner = &tokens[2] + cdef GraphDefinition parent = GraphDefinition() + cdef GraphHandle old_child = create_child_graph_handle( + raw_child, parent._h_graph, old_owner + ) + cdef GraphHandle new_child = create_child_graph_handle( + raw_child, parent._h_graph, new_owner + ) + assert new_child.get() != old_child.get() + + invalidate_child_graph_state(parent._h_graph, old_owner) + assert as_cu(old_child) == NULL + assert as_cu(new_child) == raw_child + + cdef GraphHandle same_child = create_child_graph_handle( + raw_child, parent._h_graph, new_owner + ) + assert same_child.get() == new_child.get() + + +@pytest.mark.agent_authored(model="gpt-6") +def test_graph_hierarchy_deletion_preserves_replacement_mapping(init_cuda): + """An old tree's destructor must not erase another tree's newer entry.""" + cdef uintptr_t tokens[3] + cdef CUgraph raw_child = &tokens[0] + cdef CUgraphNode old_owner = &tokens[1] + cdef CUgraphNode new_owner = &tokens[2] + cdef GraphDefinition old_parent = GraphDefinition() + cdef GraphDefinition new_parent = GraphDefinition() + cdef GraphHandle old_child = create_child_graph_handle( + raw_child, old_parent._h_graph, old_owner + ) + cdef GraphHandle new_child = create_child_graph_handle( + raw_child, new_parent._h_graph, new_owner + ) + assert new_child.get() != old_child.get() + + # Drop every owner of the old hierarchy so its real CUDA root is destroyed. + old_child.reset() + old_parent = None + assert as_cu(new_child) == raw_child + + cdef GraphHandle same_child = create_child_graph_handle( + raw_child, new_parent._h_graph, new_owner + ) + assert same_child.get() == new_child.get() From 974a7ad7f5bcc844266393eea5ef863e66616ceb Mon Sep 17 00:00:00 2001 From: Andy Jost Date: Fri, 30 Oct 2026 13:04:35 -0700 Subject: [PATCH 2/3] cuda.core: scope the child-graph registry to the parent GraphBox The process-wide graph registry mapped a raw CUgraph to its canonical GraphHandle. Every handle of a hierarchy aliases the hierarchy's single control block, so a lockable registry entry proved only that the hierarchy was alive, not that the child graph was. CUDA destroys a child graph inside cuGraphDestroyNode, cuGraphNodeSetParams, or capture invalidation, and the box is retired only after the driver call returns. In that window the map could hand one hierarchy's recycled address to another. The only reader, create_child_graph_handle, always knows the parent, and roots are never looked up. Each GraphBox now holds the registry of its direct child graphs, next to its node registry. A recycled address can only collide inside one hierarchy, where mutations are externally synchronized and every retire precedes the matching registration. The conditional unregister_handle overload, the bulk register_handles, the static registry, and the hierarchy deleter's unregister loop are gone. The same-parent registry test modeled two live generations at one address under one parent, which the synchronization contract excludes, and is removed. Written by Codex (gpt-6.1-sol); reviewed by Claude. Co-Authored-By: Claude Fable 5.1 --- .../cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md | 28 +++++++------- .../cuda/core/_cpp/rt/REGISTRY_DESIGN.md | 28 ++++++++------ cuda_core/cuda/core/_cpp/rt/graph.cpp | 38 ++++++++++--------- cuda_core/cuda/core/_cpp/rt/internal.hpp | 31 --------------- .../tests/cython/test_graph_registry.pyx | 28 +------------- 5 files changed, 51 insertions(+), 102 deletions(-) diff --git a/cuda_core/cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md b/cuda_core/cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md index 0d40d6dd06c..adc6daf8be4 100644 --- a/cuda_core/cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md +++ b/cuda_core/cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md @@ -167,21 +167,19 @@ metadata for the root graph and subgraphs. Each `GraphBox` stores: - the `CUgraph` - its parent box and owning node, when it is a child graph - a `GraphAttachmentMap` from nodes to node attachments (`NodeAttachment*`) -- a per-graph weak registry to canonicalize node handles - -When CUDA destroys a child graph, the associated `GraphBox` is cleared and -moved to `GraphHierarchy::graveyard`. That invalidates existing handles and -preserves the storage until the hierarchy's last shared owner is released. -CUDA may reuse a child's raw handle before the old metadata is retired. -Child lookup checks the parent box and owner node, and cleanup removes only -its own resource generation's registry entry. This keeps old invalidation or -hierarchy destruction from adopting or erasing an independent graph's newer -metadata; see [registry identity](REGISTRY_DESIGN.md#reused-graph-handles). - -The process-wide graph registry maps a live `CUgraph` to its canonical -`GraphHandle`. Node handles are scoped to their `GraphBox`, where they can all -be invalidated when CUDA destroys that graph. They use separate -`GraphNodeBox::resource` fields and are nulled before the graph box is retired. +- weak registries to canonicalize direct child graph and node handles + +A retired `GraphBox` is cleared and moved to `GraphHierarchy::graveyard`. +That invalidates existing handles and preserves the storage until the +hierarchy's last shared owner is released. + +Each `GraphBox` holds two registries: its direct child graphs keyed by +`CUgraph` and its nodes keyed by `CUgraphNode`. Both are scoped to the box, +so a raw handle recycled by CUDA in another hierarchy cannot resolve to this +hierarchy's metadata; see +[registry identity](REGISTRY_DESIGN.md#reused-graph-handles). Node handles use +separate `GraphNodeBox::resource` fields and are nulled before the graph box +is retired. ## Invariants diff --git a/cuda_core/cuda/core/_cpp/rt/REGISTRY_DESIGN.md b/cuda_core/cuda/core/_cpp/rt/REGISTRY_DESIGN.md index 96b9cb05519..357e8a23e8b 100644 --- a/cuda_core/cuda/core/_cpp/rt/REGISTRY_DESIGN.md +++ b/cuda_core/cuda/core/_cpp/rt/REGISTRY_DESIGN.md @@ -30,22 +30,26 @@ Without this level, a round-tripped handle would produce a new Box with default metadata, losing information that was set at creation. Instances: `context_registry`, `stream_registry`, `event_registry`, -`kernel_registry`, `graph_registry`, and one node registry per `GraphBox`. +`kernel_registry`, and, per `GraphBox`, one registry for its direct child +graphs and one for its nodes. ### Reused graph handles -CUDA can recycle a child graph's raw address after native destruction but -before its old metadata is retired. A child-graph lookup therefore accepts a -cached handle only when its parent `GraphBox` and owning node match the -requested relationship. Independent hierarchies must not share stale metadata -merely because CUDA reused an address. +A `GraphHandle` weak reference locks whenever any handle of its hierarchy is +alive, so a lockable entry does not prove that a child graph is alive. CUDA +destroys child graphs inside node operations and capture invalidation, before +cuda.core retires their boxes. A process-wide map keyed by raw `CUgraph` could +therefore resolve a recycled address to another hierarchy's metadata. -Graph cleanup uses identity-conditional unregistering: it erases the raw key -only if the entry is expired or still names the retiring resource slot. Compare -`shared_ptr::get()`, not its control block, because distinct graph generations -within one hierarchy share ownership but have different stable resource slots. -The temporary strong handle used for this check is released after unlocking -the registry mutex; its deleter may re-enter the registry. +Scoping the child registry to its parent `GraphBox` removes that case by +construction: `create_child_graph_handle` always knows the parent, and roots +are never looked up. Within one hierarchy, external synchronization and the +retire-before-register order in `graph_commit_child_graph_update` keep the +entry for an address on its live box, as long as every child graph that CUDA +destroys is retired before that address is registered again. The one path +that does not retire is a conditional-body capture that ends invalidated; the +parent graph is already invalid there (issue #2918), and the registry does +not guard it. ## Level 2: Resource Handle -> Python Object (Cython) diff --git a/cuda_core/cuda/core/_cpp/rt/graph.cpp b/cuda_core/cuda/core/_cpp/rt/graph.cpp index 24532c3ea39..f2d4cf4b25b 100644 --- a/cuda_core/cuda/core/_cpp/rt/graph.cpp +++ b/cuda_core/cuda/core/_cpp/rt/graph.cpp @@ -82,6 +82,8 @@ struct GraphBox : GraphBoxBase { CUgraphNode owner_node = nullptr; // Node in parent that owns this graph. GraphAttachmentMap attachments; // Non-owning attachment index. HandleRegistry node_handles; + // See REGISTRY_DESIGN.md (Level 1: Driver Handle -> Resource Handle) + HandleRegistry children; // Canonical handles of the direct child graphs. GraphBox( CUgraph resource_, GraphHierarchy *hierarchy_, GraphBox *parent_ = nullptr, CUgraphNode owner_node_ = nullptr @@ -104,10 +106,6 @@ struct GraphHierarchy { } }; -// See REGISTRY_DESIGN.md (Level 1: Driver Handle -> Resource Handle) -using GraphRegistry = HandleRegistry; -static GraphRegistry graph_registry; - // Immutable resource owners for one version of a graph node's parameters. // Inheriting DeferredCleanupItem lets CUDA's user-object destructor enqueue // the payload without destroying owners on the callback thread. @@ -144,6 +142,17 @@ get_box(const GraphHandle &h) noexcept return const_cast(static_cast(value)); } +// Register child handles with their immediate parents. Each handle aliases a +// GraphBox whose parent is set; a hierarchy's root is never registered. +void +register_child_handles(const std::vector &handles) +{ + for (const GraphHandle &h : handles) { + GraphBox *box = get_box(h); + box->parent->children.register_handle(box->resource, h); + } +} + // Rekey a staged attachment map from source nodes to their cloned nodes. // The caller must release the GIL before calling this function. CUresult @@ -302,11 +311,6 @@ create_graph_handle(CUgraph graph) auto hierarchy = std::shared_ptr( new GraphHierarchy{}, [](GraphHierarchy *hierarchy) { - for (const GraphBox& box : hierarchy->graphs) { - if (box.resource) { - graph_registry.unregister_handle(box.resource, &box.resource); - } - } GraphBox *root = hierarchy->root(); if (root && root->resource) { GILReleaseGuard gil; @@ -320,7 +324,6 @@ create_graph_handle(CUgraph graph) GraphBox &root = hierarchy->graphs.emplace_back(graph, hierarchy.get()); GraphHandle h_graph(hierarchy, &root.resource); - graph_registry.register_handle(graph, h_graph); return h_graph; } @@ -331,9 +334,8 @@ create_child_graph_handle(CUgraph child_graph, const GraphHandle &h_parent, CUgr return {}; } GraphBox *parent = get_box(h_parent); - if (GraphHandle h_graph = graph_registry.lookup(child_graph)) { - const GraphBox *child = get_box(h_graph); - if (child->parent == parent && child->owner_node == owner_node) { + if (GraphHandle h_graph = parent->children.lookup(child_graph)) { + if (get_box(h_graph)->owner_node == owner_node) { return h_graph; } } @@ -342,7 +344,7 @@ create_child_graph_handle(CUgraph child_graph, const GraphHandle &h_parent, CUgr GraphBox &child = hierarchy->graphs.emplace_back(child_graph, hierarchy, parent, owner_node); GraphHandle h_child(h_parent, &child.resource); - graph_registry.register_handle(child_graph, h_child); + parent->children.register_handle(child_graph, h_child); return h_child; } @@ -398,7 +400,7 @@ publish_child_graph_update(PreparedChildGraphUpdateState &state, GraphHandle *ou GraphBox *parent = get_box(state.h_parent); parent->hierarchy->graphs.splice(parent->hierarchy->graphs.end(), state.replacement); *out_child = state.handles.front(); - graph_registry.register_handles(state.handles); + register_child_handles(state.handles); } CUresult @@ -632,7 +634,7 @@ graph_clone_attachments(const GraphHandle &h_clone, const GraphHandle &h_source) } clone->hierarchy->graphs.splice(clone->hierarchy->graphs.end(), subgraphs); - graph_registry.register_handles(handles); + register_child_handles(handles); return CUDA_SUCCESS; } @@ -662,7 +664,9 @@ retire_graph(GraphHierarchy &hierarchy, std::list::iterator graph) noe } } if (graph->resource) { - graph_registry.unregister_handle(graph->resource, &graph->resource); + if (graph->parent) { + graph->parent->children.unregister_handle(graph->resource); + } graph->resource = nullptr; } graph->attachments.clear(); diff --git a/cuda_core/cuda/core/_cpp/rt/internal.hpp b/cuda_core/cuda/core/_cpp/rt/internal.hpp index d23286c0f78..382c4901f11 100644 --- a/cuda_core/cuda/core/_cpp/rt/internal.hpp +++ b/cuda_core/cuda/core/_cpp/rt/internal.hpp @@ -16,7 +16,6 @@ #include #include #include -#include namespace cuda_core::rt::detail { @@ -193,36 +192,6 @@ class HandleRegistry { map_.erase(key); } - // A reused raw key may already name a newer resource generation. - void - unregister_handle(const Key &key, const typename Handle::element_type *expected) noexcept - { - // Releasing the last strong owner may re-enter this registry, so keep - // it alive until after the mutex is released. - Handle h; - { - std::lock_guard lock(mutex_); - auto it = map_.find(key); - if (it != map_.end()) { - h = it->second.lock(); - if (!h || h.get() == expected) { - map_.erase(it); - } - } - } - } - - void - register_handles(const std::vector &handles) - { - std::lock_guard lock(mutex_); - for (const Handle &h : handles) { - if (h) { - map_[*h] = h; - } - } - } - Handle lookup(const Key &key) { diff --git a/cuda_core/tests/cython/test_graph_registry.pyx b/cuda_core/tests/cython/test_graph_registry.pyx index 8cd3d9270ae..5a5e0b984e0 100644 --- a/cuda_core/tests/cython/test_graph_registry.pyx +++ b/cuda_core/tests/cython/test_graph_registry.pyx @@ -4,7 +4,7 @@ # distutils: language = c++ # distutils: include_dirs = ../../cuda/core -"""Deterministic raw-handle reuse tests against the real graph registry. +"""Deterministic raw-handle reuse tests across independent parent registries. Parents are real empty CUDA graphs. Stack-local addresses stand in for reused child and owner handles only inside the metadata API; they are never passed to @@ -56,32 +56,6 @@ def test_child_graph_registry_rejects_foreign_parent(init_cuda): assert same_child.get() == new_child.get() -@pytest.mark.agent_authored(model="gpt-6") -def test_child_graph_registry_rejects_foreign_owner_in_same_parent(init_cuda): - """Retiring an old owner preserves the replacement within one hierarchy.""" - cdef uintptr_t tokens[3] - cdef CUgraph raw_child = &tokens[0] - cdef CUgraphNode old_owner = &tokens[1] - cdef CUgraphNode new_owner = &tokens[2] - cdef GraphDefinition parent = GraphDefinition() - cdef GraphHandle old_child = create_child_graph_handle( - raw_child, parent._h_graph, old_owner - ) - cdef GraphHandle new_child = create_child_graph_handle( - raw_child, parent._h_graph, new_owner - ) - assert new_child.get() != old_child.get() - - invalidate_child_graph_state(parent._h_graph, old_owner) - assert as_cu(old_child) == NULL - assert as_cu(new_child) == raw_child - - cdef GraphHandle same_child = create_child_graph_handle( - raw_child, parent._h_graph, new_owner - ) - assert same_child.get() == new_child.get() - - @pytest.mark.agent_authored(model="gpt-6") def test_graph_hierarchy_deletion_preserves_replacement_mapping(init_cuda): """An old tree's destructor must not erase another tree's newer entry.""" From 9b5d12695dc16b3392e941997706218c676cf013 Mon Sep 17 00:00:00 2001 From: Andy Jost Date: Fri, 30 Oct 2026 13:39:50 -0700 Subject: [PATCH 3/3] cuda.core: key child graphs by address and owner node A child graph is identified within its parent by its CUgraph and the node that owns it. Keying the per-parent registry by that pair makes the owner check part of the lookup. A box whose graph CUDA destroyed before cuda.core retired it keeps a key distinct from a newer child that reuses its address under another owner, so retiring the stale box cannot erase the newer entry. The same-parent registry test is restored; it passes under the pair key. Co-Authored-By: Claude Opus 5.5 --- .../cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md | 6 +-- .../cuda/core/_cpp/rt/REGISTRY_DESIGN.md | 14 +++---- cuda_core/cuda/core/_cpp/rt/graph.cpp | 38 +++++++++++++++---- .../tests/cython/test_graph_registry.pyx | 28 +++++++++++++- 4 files changed, 67 insertions(+), 19 deletions(-) diff --git a/cuda_core/cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md b/cuda_core/cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md index adc6daf8be4..02897f727cd 100644 --- a/cuda_core/cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md +++ b/cuda_core/cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md @@ -174,9 +174,9 @@ That invalidates existing handles and preserves the storage until the hierarchy's last shared owner is released. Each `GraphBox` holds two registries: its direct child graphs keyed by -`CUgraph` and its nodes keyed by `CUgraphNode`. Both are scoped to the box, -so a raw handle recycled by CUDA in another hierarchy cannot resolve to this -hierarchy's metadata; see +`CUgraph` and owner node, and its nodes keyed by `CUgraphNode`. Both are +scoped to the box, so a raw handle recycled by CUDA in another hierarchy +cannot resolve to this hierarchy's metadata; see [registry identity](REGISTRY_DESIGN.md#reused-graph-handles). Node handles use separate `GraphNodeBox::resource` fields and are nulled before the graph box is retired. diff --git a/cuda_core/cuda/core/_cpp/rt/REGISTRY_DESIGN.md b/cuda_core/cuda/core/_cpp/rt/REGISTRY_DESIGN.md index 357e8a23e8b..003887f296c 100644 --- a/cuda_core/cuda/core/_cpp/rt/REGISTRY_DESIGN.md +++ b/cuda_core/cuda/core/_cpp/rt/REGISTRY_DESIGN.md @@ -43,13 +43,13 @@ therefore resolve a recycled address to another hierarchy's metadata. Scoping the child registry to its parent `GraphBox` removes that case by construction: `create_child_graph_handle` always knows the parent, and roots -are never looked up. Within one hierarchy, external synchronization and the -retire-before-register order in `graph_commit_child_graph_update` keep the -entry for an address on its live box, as long as every child graph that CUDA -destroys is retired before that address is registered again. The one path -that does not retire is a conditional-body capture that ends invalidated; the -parent graph is already invalid there (issue #2918), and the registry does -not guard it. +are never looked up. Within the parent, an entry is keyed by the child's +`CUgraph` and its owner node, which together identify a child graph. If CUDA +destroys a child before cuda.core retires its box, the owner node is still +alive, so a newer child at the recycled address has a different owner and its +own entry. Retiring the stale box erases only the stale entry. Within one +owner, external synchronization and the retire-before-register order in +`graph_commit_child_graph_update` keep one live entry per key. ## Level 2: Resource Handle -> Python Object (Cython) diff --git a/cuda_core/cuda/core/_cpp/rt/graph.cpp b/cuda_core/cuda/core/_cpp/rt/graph.cpp index f2d4cf4b25b..09261623154 100644 --- a/cuda_core/cuda/core/_cpp/rt/graph.cpp +++ b/cuda_core/cuda/core/_cpp/rt/graph.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -69,6 +70,29 @@ using GraphAttachmentMap = std::map; struct GraphHierarchy; +// A child graph is identified by its address and its owner node in the parent. +// A box that CUDA destroyed before cuda.core retired it keeps a key distinct +// from a newer child that reuses its address under another owner node. +struct ChildGraphKey { + CUgraph graph; + CUgraphNode owner_node; + + bool + operator==(const ChildGraphKey &other) const noexcept + { + return graph == other.graph && owner_node == other.owner_node; + } +}; + +struct ChildGraphKeyHash { + std::size_t + operator()(const ChildGraphKey &key) const noexcept + { + std::size_t h = std::hash{}(key.graph); + return h ^ (std::hash{}(key.owner_node) + 0x9e3779b9 + (h << 6) + (h >> 2)); + } +}; + // Standard-layout alias target for GraphHandle. struct GraphBoxBase { CUgraph resource = nullptr; @@ -83,7 +107,7 @@ struct GraphBox : GraphBoxBase { GraphAttachmentMap attachments; // Non-owning attachment index. HandleRegistry node_handles; // See REGISTRY_DESIGN.md (Level 1: Driver Handle -> Resource Handle) - HandleRegistry children; // Canonical handles of the direct child graphs. + HandleRegistry children; // Direct child graphs. GraphBox( CUgraph resource_, GraphHierarchy *hierarchy_, GraphBox *parent_ = nullptr, CUgraphNode owner_node_ = nullptr @@ -149,7 +173,7 @@ register_child_handles(const std::vector &handles) { for (const GraphHandle &h : handles) { GraphBox *box = get_box(h); - box->parent->children.register_handle(box->resource, h); + box->parent->children.register_handle({box->resource, box->owner_node}, h); } } @@ -334,17 +358,15 @@ create_child_graph_handle(CUgraph child_graph, const GraphHandle &h_parent, CUgr return {}; } GraphBox *parent = get_box(h_parent); - if (GraphHandle h_graph = parent->children.lookup(child_graph)) { - if (get_box(h_graph)->owner_node == owner_node) { - return h_graph; - } + if (GraphHandle h_graph = parent->children.lookup({child_graph, owner_node})) { + return h_graph; } GraphHierarchy *hierarchy = parent->hierarchy; GraphBox &child = hierarchy->graphs.emplace_back(child_graph, hierarchy, parent, owner_node); GraphHandle h_child(h_parent, &child.resource); - parent->children.register_handle(child_graph, h_child); + parent->children.register_handle({child_graph, owner_node}, h_child); return h_child; } @@ -665,7 +687,7 @@ retire_graph(GraphHierarchy &hierarchy, std::list::iterator graph) noe } if (graph->resource) { if (graph->parent) { - graph->parent->children.unregister_handle(graph->resource); + graph->parent->children.unregister_handle({graph->resource, graph->owner_node}); } graph->resource = nullptr; } diff --git a/cuda_core/tests/cython/test_graph_registry.pyx b/cuda_core/tests/cython/test_graph_registry.pyx index 5a5e0b984e0..09ddca2f706 100644 --- a/cuda_core/tests/cython/test_graph_registry.pyx +++ b/cuda_core/tests/cython/test_graph_registry.pyx @@ -4,7 +4,7 @@ # distutils: language = c++ # distutils: include_dirs = ../../cuda/core -"""Deterministic raw-handle reuse tests across independent parent registries. +"""Deterministic raw-handle reuse tests against the per-parent child registries. Parents are real empty CUDA graphs. Stack-local addresses stand in for reused child and owner handles only inside the metadata API; they are never passed to @@ -56,6 +56,32 @@ def test_child_graph_registry_rejects_foreign_parent(init_cuda): assert same_child.get() == new_child.get() +@pytest.mark.agent_authored(model="gpt-6") +def test_child_graph_registry_rejects_foreign_owner_in_same_parent(init_cuda): + """Retiring an old owner preserves the replacement within one hierarchy.""" + cdef uintptr_t tokens[3] + cdef CUgraph raw_child = &tokens[0] + cdef CUgraphNode old_owner = &tokens[1] + cdef CUgraphNode new_owner = &tokens[2] + cdef GraphDefinition parent = GraphDefinition() + cdef GraphHandle old_child = create_child_graph_handle( + raw_child, parent._h_graph, old_owner + ) + cdef GraphHandle new_child = create_child_graph_handle( + raw_child, parent._h_graph, new_owner + ) + assert new_child.get() != old_child.get() + + invalidate_child_graph_state(parent._h_graph, old_owner) + assert as_cu(old_child) == NULL + assert as_cu(new_child) == raw_child + + cdef GraphHandle same_child = create_child_graph_handle( + raw_child, parent._h_graph, new_owner + ) + assert same_child.get() == new_child.get() + + @pytest.mark.agent_authored(model="gpt-6") def test_graph_hierarchy_deletion_preserves_replacement_mapping(init_cuda): """An old tree's destructor must not erase another tree's newer entry."""