Skip to content

Commit c5dbc8d

Browse files
rwgkAndy-Jostclaude
authored
cuda.core: preserve graph identity when CUDA reuses handles (#3294)
* cuda.core: preserve graph identity across native handle reuse Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com> Co-authored-by: Andy Jost <ajost@nvidia.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 2ff8747 commit c5dbc8d

8 files changed

Lines changed: 200 additions & 46 deletions

File tree

‎cuda_core/cuda/core/_cpp/rt/GRAPH_ATTACHMENTS.md‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -171,18 +171,19 @@ metadata for the root graph and subgraphs. Each `GraphBox` stores:
171171
- the `CUgraph`
172172
- its parent box and owning node, when it is a child graph
173173
- a `GraphAttachmentMap` from nodes to node attachments (`NodeAttachment*`)
174-
- a per-graph weak registry to canonicalize node handles
175-
176-
When CUDA destroys a child graph, the associated `GraphBox` is cleared and
177-
moved to `GraphHierarchy::graveyard`. That invalidates existing handles and
178-
preserves the storage until the hierarchy's last shared owner is released.
179-
Removing the registry entry also prevents corruption if a future CUDA graph
180-
reuses the raw handle value.
181-
182-
The process-wide graph registry maps a live `CUgraph` to its canonical
183-
`GraphHandle`. Node handles are scoped to their `GraphBox`, where they can all
184-
be invalidated when CUDA destroys that graph. They use separate
185-
`GraphNodeBox::resource` fields and are nulled before the graph box is retired.
174+
- weak registries to canonicalize direct child graph and node handles
175+
176+
A retired `GraphBox` is cleared and moved to `GraphHierarchy::graveyard`.
177+
That invalidates existing handles and preserves the storage until the
178+
hierarchy's last shared owner is released.
179+
180+
Each `GraphBox` holds two registries: its direct child graphs keyed by
181+
`CUgraph` and owner node, and its nodes keyed by `CUgraphNode`. Both are
182+
scoped to the box, so a raw handle recycled by CUDA in another hierarchy
183+
cannot resolve to this hierarchy's metadata; see
184+
[registry identity](REGISTRY_DESIGN.md#reused-graph-handles). Node handles use
185+
separate `GraphNodeBox::resource` fields and are nulled before the graph box
186+
is retired.
186187

187188
## Invariants
188189

‎cuda_core/cuda/core/_cpp/rt/REGISTRY_DESIGN.md‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,26 @@ Without this level, a round-tripped handle would produce a new Box
3030
with default metadata, losing information that was set at creation.
3131

3232
Instances: `context_registry`, `stream_registry`, `event_registry`,
33-
`kernel_registry`, `graph_registry`, and one node registry per `GraphBox`.
33+
`kernel_registry`, and, per `GraphBox`, one registry for its direct child
34+
graphs and one for its nodes.
35+
36+
### Reused graph handles
37+
38+
A `GraphHandle` weak reference locks whenever any handle of its hierarchy is
39+
alive, so a lockable entry does not prove that a child graph is alive. CUDA
40+
destroys child graphs inside node operations and capture invalidation, before
41+
cuda.core retires their boxes. A process-wide map keyed by raw `CUgraph` could
42+
therefore resolve a recycled address to another hierarchy's metadata.
43+
44+
Scoping the child registry to its parent `GraphBox` removes that case by
45+
construction: `create_child_graph_handle` always knows the parent, and roots
46+
are never looked up. Within the parent, an entry is keyed by the child's
47+
`CUgraph` and its owner node, which together identify a child graph. If CUDA
48+
destroys a child before cuda.core retires its box, the owner node is still
49+
alive, so a newer child at the recycled address has a different owner and its
50+
own entry. Retiring the stale box erases only the stale entry. Within one
51+
owner, external synchronization and the retire-before-register order in
52+
`graph_commit_child_graph_update` keep one live entry per key.
3453

3554
## Level 2: Resource Handle -> Python Object (Cython)
3655

‎cuda_core/cuda/core/_cpp/rt/graph.cpp‎

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <array>
1212
#include <cstddef>
1313
#include <cstdlib>
14+
#include <functional>
1415
#include <list>
1516
#include <map>
1617
#include <memory>
@@ -69,6 +70,29 @@ using GraphAttachmentMap = std::map<CUgraphNode, NodeAttachment *>;
6970

7071
struct GraphHierarchy;
7172

73+
// A child graph is identified by its address and its owner node in the parent.
74+
// A box that CUDA destroyed before cuda.core retired it keeps a key distinct
75+
// from a newer child that reuses its address under another owner node.
76+
struct ChildGraphKey {
77+
CUgraph graph;
78+
CUgraphNode owner_node;
79+
80+
bool
81+
operator==(const ChildGraphKey &other) const noexcept
82+
{
83+
return graph == other.graph && owner_node == other.owner_node;
84+
}
85+
};
86+
87+
struct ChildGraphKeyHash {
88+
std::size_t
89+
operator()(const ChildGraphKey &key) const noexcept
90+
{
91+
std::size_t h = std::hash<const void *>{}(key.graph);
92+
return h ^ (std::hash<const void *>{}(key.owner_node) + 0x9e3779b9 + (h << 6) + (h >> 2));
93+
}
94+
};
95+
7296
// Standard-layout alias target for GraphHandle.
7397
struct GraphBoxBase {
7498
CUgraph resource = nullptr;
@@ -82,6 +106,8 @@ struct GraphBox : GraphBoxBase {
82106
CUgraphNode owner_node = nullptr; // Node in parent that owns this graph.
83107
GraphAttachmentMap attachments; // Non-owning attachment index.
84108
HandleRegistry<CUgraphNode, GraphNodeHandle> node_handles;
109+
// See REGISTRY_DESIGN.md (Level 1: Driver Handle -> Resource Handle)
110+
HandleRegistry<ChildGraphKey, GraphHandle, ChildGraphKeyHash> children; // Direct child graphs.
85111

86112
GraphBox(
87113
CUgraph resource_, GraphHierarchy *hierarchy_, GraphBox *parent_ = nullptr, CUgraphNode owner_node_ = nullptr
@@ -104,10 +130,6 @@ struct GraphHierarchy {
104130
}
105131
};
106132

107-
// See REGISTRY_DESIGN.md (Level 1: Driver Handle -> Resource Handle)
108-
using GraphRegistry = HandleRegistry<CUgraph, GraphHandle>;
109-
static GraphRegistry graph_registry;
110-
111133
// Immutable resource owners for one version of a graph node's parameters.
112134
// Inheriting DeferredCleanupItem lets CUDA's user-object destructor enqueue
113135
// the payload without destroying owners on the callback thread.
@@ -144,6 +166,17 @@ get_box(const GraphHandle &h) noexcept
144166
return const_cast<GraphBox *>(static_cast<const GraphBox *>(value));
145167
}
146168

169+
// Register child handles with their immediate parents. Each handle aliases a
170+
// GraphBox whose parent is set; a hierarchy's root is never registered.
171+
void
172+
register_child_handles(const std::vector<GraphHandle> &handles)
173+
{
174+
for (const GraphHandle &h : handles) {
175+
GraphBox *box = get_box(h);
176+
box->parent->children.register_handle({box->resource, box->owner_node}, h);
177+
}
178+
}
179+
147180
// Rekey a staged attachment map from source nodes to their cloned nodes.
148181
// The caller must release the GIL before calling this function.
149182
CUresult
@@ -302,11 +335,6 @@ create_graph_handle(CUgraph graph)
302335
auto hierarchy = std::shared_ptr<GraphHierarchy>(
303336
new GraphHierarchy{},
304337
[](GraphHierarchy *hierarchy) {
305-
for (const GraphBox& box : hierarchy->graphs) {
306-
if (box.resource) {
307-
graph_registry.unregister_handle(box.resource);
308-
}
309-
}
310338
GraphBox *root = hierarchy->root();
311339
if (root && root->resource) {
312340
GILReleaseGuard gil;
@@ -320,7 +348,6 @@ create_graph_handle(CUgraph graph)
320348
GraphBox &root = hierarchy->graphs.emplace_back(graph, hierarchy.get());
321349

322350
GraphHandle h_graph(hierarchy, &root.resource);
323-
graph_registry.register_handle(graph, h_graph);
324351
return h_graph;
325352
}
326353

@@ -330,16 +357,16 @@ create_child_graph_handle(CUgraph child_graph, const GraphHandle &h_parent, CUgr
330357
if (!child_graph || !h_parent || !owner_node) {
331358
return {};
332359
}
333-
if (GraphHandle h_graph = graph_registry.lookup(child_graph)) {
360+
GraphBox *parent = get_box(h_parent);
361+
if (GraphHandle h_graph = parent->children.lookup({child_graph, owner_node})) {
334362
return h_graph;
335363
}
336364

337-
GraphBox *parent = get_box(h_parent);
338365
GraphHierarchy *hierarchy = parent->hierarchy;
339366
GraphBox &child = hierarchy->graphs.emplace_back(child_graph, hierarchy, parent, owner_node);
340367

341368
GraphHandle h_child(h_parent, &child.resource);
342-
graph_registry.register_handle(child_graph, h_child);
369+
parent->children.register_handle({child_graph, owner_node}, h_child);
343370
return h_child;
344371
}
345372

@@ -395,7 +422,7 @@ publish_child_graph_update(PreparedChildGraphUpdateState &state, GraphHandle *ou
395422
GraphBox *parent = get_box(state.h_parent);
396423
parent->hierarchy->graphs.splice(parent->hierarchy->graphs.end(), state.replacement);
397424
*out_child = state.handles.front();
398-
graph_registry.register_handles(state.handles);
425+
register_child_handles(state.handles);
399426
}
400427

401428
CUresult
@@ -629,7 +656,7 @@ graph_clone_attachments(const GraphHandle &h_clone, const GraphHandle &h_source)
629656
}
630657

631658
clone->hierarchy->graphs.splice(clone->hierarchy->graphs.end(), subgraphs);
632-
graph_registry.register_handles(handles);
659+
register_child_handles(handles);
633660
return CUDA_SUCCESS;
634661
}
635662

@@ -659,7 +686,9 @@ retire_graph(GraphHierarchy &hierarchy, std::list<GraphBox>::iterator graph) noe
659686
}
660687
}
661688
if (graph->resource) {
662-
graph_registry.unregister_handle(graph->resource);
689+
if (graph->parent) {
690+
graph->parent->children.unregister_handle({graph->resource, graph->owner_node});
691+
}
663692
graph->resource = nullptr;
664693
}
665694
graph->attachments.clear();

‎cuda_core/cuda/core/_cpp/rt/internal.hpp‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
#include <type_traits>
1717
#include <unordered_map>
1818
#include <utility>
19-
#include <vector>
2019

2120
namespace cuda_core::rt::detail {
2221

@@ -193,17 +192,6 @@ class HandleRegistry {
193192
map_.erase(key);
194193
}
195194

196-
void
197-
register_handles(const std::vector<Handle> &handles)
198-
{
199-
std::lock_guard<std::mutex> lock(mutex_);
200-
for (const Handle &h : handles) {
201-
if (h) {
202-
map_[*h] = h;
203-
}
204-
}
205-
}
206-
207195
Handle
208196
lookup(const Key &key)
209197
{

‎cuda_core/tests/cython/build_tests.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
so `cimport cuda.bindings.*` fails to locate the .pxd files. We resolve
99
the namespace package's source root from `cuda.bindings.__file__` and pass
1010
it via `include_path=` so cythonize finds the .pxd tree on every platform.
11+
Tests that cimport cuda.core also need this checkout's Core source root on
12+
that search path; the editable install's Python finder does not supply it.
1113
1214
When CUDA_PYTHON_CYTHON_CACHE_DIR is set, cythonize uses the same cache
1315
namespacing and include-path aliasing as the package build (``_build_shared.py``).
@@ -54,6 +56,7 @@ def _bindings_source_root() -> Path:
5456

5557

5658
def _cythonize_tests(pyx_files):
59+
core_source_root = Path(__file__).resolve().parents[2]
5760
cache_path = build_shared._cython_cache_path(
5861
"cuda-core-cython-tests",
5962
compiler_directives=_COMPILER_DIRECTIVES,
@@ -68,7 +71,7 @@ def _cythonize_tests(pyx_files):
6871
if cache_path is None:
6972
return cythonize(
7073
pyx_files,
71-
include_path=[str(_bindings_source_root())],
74+
include_path=[str(core_source_root), str(_bindings_source_root())],
7275
**cythonize_kwargs,
7376
)
7477

@@ -78,11 +81,12 @@ def _cythonize_tests(pyx_files):
7881
stdlib_target = Path(Cython.__file__).parent / "Includes"
7982
with (
8083
build_shared._stable_cython_alias(stdlib_target, Path(".cython-stdlib-tests")) as rel_stdlib,
84+
build_shared._stable_cython_alias(core_source_root, Path(".cython-core-tests")) as rel_core,
8185
build_shared._stable_cython_alias(_bindings_source_root(), Path(".cython-bindings-tests")) as rel_bindings,
8286
):
8387
return cythonize(
8488
pyx_files,
85-
include_path=[".", rel_bindings, rel_stdlib],
89+
include_path=[".", rel_core, rel_bindings, rel_stdlib],
8690
**cythonize_kwargs,
8791
)
8892

@@ -101,6 +105,9 @@ def main() -> None:
101105
# are stable regardless of the invoking cwd.
102106
os.chdir(script_dir)
103107
ext_modules = _cythonize_tests(pyx_files)
108+
# Core's runtime consumer headers require C++17 with either toolchain.
109+
for extension in ext_modules:
110+
extension.extra_compile_args.append("/std:c++17" if os.name == "nt" else "-std=c++17")
104111

105112
sys.argv = [sys.argv[0], "build_ext", "--inplace"]
106113
setup(name="cuda_core_cython_tests", ext_modules=ext_modules)

‎cuda_core/tests/cython/test_cython.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# SPDX-FileCopyrightText: Copyright (c) 2021-2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
1+
# SPDX-FileCopyrightText: Copyright (c) 2021-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
#
33
# SPDX-License-Identifier: Apache-2.0
44

@@ -21,6 +21,7 @@ def wrapped(*args, **kwargs):
2121

2222
cython_test_modules = [
2323
"test_get_cuda_native_handle",
24+
"test_graph_registry",
2425
]
2526

2627

‎cuda_core/tests/cython/test_get_cuda_native_handle.pyx‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
#
33
# SPDX-License-Identifier: Apache-2.0
44

55
# distutils: language = c++
6-
# distutils: extra_compile_args = -std=c++17
76

87
from libc.stdint cimport intptr_t
98

0 commit comments

Comments
 (0)