Skip to content

Commit 657bbaf

Browse files
Andy-Jostclaude
andauthored
cuda.core: fix pool setup and builder teardown under stream capture (#2838)
* cuda.core: fix pool setup and builder teardown under stream capture DeviceMemoryResource(device) with no options raises the release threshold of the driver's pool with cuMemPoolGetAttribute and cuMemPoolSetAttribute. The driver refuses both as potentially unsafe calls while the calling thread is inside a global or thread-local capture, and it invalidates the capture. Device.memory_resource constructs the resource lazily, so a first allocation could invalidate a capture in progress. Make the two calls in relaxed capture mode and restore the thread's previous mode afterwards. A failure to restore the mode is attached to the propagating error as a note. Ending an invalidated capture made the builder destroy a graph the driver had already destroyed. cuStreamEndCapture returns a NULL graph for an invalidated (or unjoined) capture and releases the capture graph itself, but the builder kept the owning handle it took from cuStreamGetCaptureInfo and its deleter called cuGraphDestroy again: a use-after-free that segfaulted at close() or garbage collection. Add invalidate_root_graph_state to retire the hierarchy when the driver discards the root graph, and route end_building(), close() and __dealloc__ through one GB_end_capture helper that settles graph ownership from the end-capture result. end_building() now ends an invalidated capture and raises the driver error; the builder then holds no graph (new CAPTURE_INVALIDATED state), and complete(), debug_dot_print(), graph_definition, embed() and Graph.update() say so. close() closes the builder before raising. end_building() on a forked builder is rejected with RuntimeError instead of invalidating the capture. Fixes #2834. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * cuda.core: scope the invalidated-capture recovery to top-level builders The driver also discards the body graph of a conditional node when the body capture ends invalidated, and the parent graph keeps referring to it, which cuda.core cannot repair. Say so in end_building(), the release note, and the GB_end_capture comment. Scope the DeviceMemoryResource note to the default-pool constructor, since cuMemPoolCreate is still refused under capture. Drop the end_building() call on a forked builder from the skip path of test_graph_conditional_on_forked_builder. Issue #2834 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * cuda.core: close unjoined forks when GraphBuilder.join fails join() waits on each forked builder's stream and then closes it. When a wait raised partway through, the forks the loop had not reached stayed open with capturing streams, and destroying them later during garbage collection crashed the interpreter. Close every fork the loop did not reach before the error propagates. The capture cannot complete without the work captured on those forks, so end_building() then raises the driver's unjoined-capture error and the builder closes cleanly, which #2834 made possible. Issue #2776 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> * cuda.core: fold the join cleanup into the capture-teardown release note PR #2881 is folded into this PR: its join() cleanup relies on the invalidated-capture handling here, and the #2776 crash is the same double destroy of a graph the driver already discarded. Merge the two release-note entries and correct the test docstring, which attributed the crash to the fork rather than to the builder's teardown. Issue #2834, #2776 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * cuda.core: point the conditional-body limitation at issue #2918 An invalidated capture of a conditional body leaves the parent graph invalid, and ending the parent capture afterwards crashes inside the driver (#2918, a CUDA driver bug reproduced with the driver API alone). Say so in the end_building docstring and the release note instead of explaining the mechanism. Issue #2834 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * cuda.core: keep GraphBuilder.join's cleanup going when a close fails join() closes the builders it did not join before its error propagates. That sweep ran in a finally block and called close() on each builder. A builder from another capture makes the root's wait fail, and the driver invalidates that builder's capture as well, so its close() raised too. The raise stopped the sweep, stranded the later builders with capturing streams, and replaced the merge error with the close error. Run the sweep in an except block, close each builder through GB_close, which returns the driver status instead of raising, and attach a failed close to the propagating error as a note (error handling policy). The original error is re-raised unchanged. Review feedback on #2838. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(cuda.core): cover a failing close inside GraphBuilder.join's cleanup The existing test provokes the join failure with a forked builder, whose close() cannot raise, so the sweep's own failure path went untested. Join two separate primaries into a capture: the driver refuses the cross-capture wait and invalidates the other capture, so closing that builder fails. The test checks that every builder is closed, that the merge error propagates, and that the failed close is attached as a note (or reported as a CUDAWarning on Python 3.10). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * cuda.core: factor retire_graph out of the graph invalidation sweeps invalidate_child_graph_state and invalidate_root_graph_state repeated the per-box retirement: detach node handles, drop the registry entry and attachments, move the box to the graveyard. Both now call retire_graph and differ only in which boxes they select. No behavior change. Review feedback on #2838. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 1db44ec commit 657bbaf

11 files changed

Lines changed: 488 additions & 70 deletions

File tree

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,12 @@ void invalidate_child_graph_state(
357357
const GraphHandle& h_parent,
358358
CUgraphNode owner_node) noexcept;
359359

360+
// Invalidate cuda.core state for a root graph that CUDA destroyed itself, such
361+
// as the graph of an invalidated capture ended by cuStreamEndCapture. The
362+
// owning handle then no longer calls cuGraphDestroy. No-op unless h_root is
363+
// the live root of its hierarchy.
364+
void invalidate_root_graph_state(const GraphHandle& h_root) noexcept;
365+
360366
// ============================================================================
361367
// Graph exec handle functions
362368
// ============================================================================

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

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,26 @@ static const GraphNodeBox* get_box(const GraphNodeHandle& h) {
679679
);
680680
}
681681

682+
// Retire one box of a graph that CUDA has destroyed: detach its node handles,
683+
// drop its registry entry and attachments, and move it to the graveyard.
684+
// `graph` must be an iterator into hierarchy.graphs; the splice invalidates it.
685+
static void retire_graph(
686+
GraphHierarchy& hierarchy,
687+
std::list<GraphBox>::iterator graph) noexcept {
688+
for (auto& entry : graph->node_handles.drain()) {
689+
if (GraphNodeHandle h_node = entry.second.lock()) {
690+
get_box(h_node)->resource = nullptr;
691+
}
692+
}
693+
if (graph->resource) {
694+
graph_registry.unregister_handle(graph->resource);
695+
graph->resource = nullptr;
696+
}
697+
graph->attachments.clear();
698+
hierarchy.graveyard.splice(
699+
hierarchy.graveyard.end(), hierarchy.graphs, graph);
700+
}
701+
682702
// graphs is ordered parent-before-child. Nulling a selected box marks its
683703
// later descendants, whose parent pointers remain valid after list splicing.
684704
// This permits one allocation-free sweep of the hierarchy.
@@ -701,21 +721,28 @@ void invalidate_child_graph_state(
701721
graph->owner_node == owner_node;
702722
bool is_descendant = graph->parent &&
703723
!graph->parent->resource;
704-
if (!is_owned_root && !is_descendant) {
705-
continue;
724+
if (is_owned_root || is_descendant) {
725+
retire_graph(hierarchy, graph);
706726
}
727+
}
728+
}
707729

708-
// Empty node_handles and invalidate each one.
709-
for (auto& entry : graph->node_handles.drain()) {
710-
if (GraphNodeHandle h_node = entry.second.lock()) {
711-
get_box(h_node)->resource = nullptr;
712-
}
713-
}
714-
graph_registry.unregister_handle(graph->resource);
715-
graph->resource = nullptr;
716-
graph->attachments.clear();
717-
hierarchy.graveyard.splice(
718-
hierarchy.graveyard.end(), hierarchy.graphs, graph);
730+
// CUDA destroyed the root graph and, with it, every child. Retire every box
731+
// so that no registry entry resolves to the dead graphs and the hierarchy's
732+
// deleter finds no root to destroy.
733+
void invalidate_root_graph_state(const GraphHandle& h_root) noexcept {
734+
if (!h_root) {
735+
return;
736+
}
737+
738+
GraphBox* root = get_box(h_root);
739+
if (!root->resource || root->parent) {
740+
return;
741+
}
742+
GraphHierarchy& hierarchy = *root->hierarchy;
743+
for (auto it = hierarchy.graphs.begin();
744+
it != hierarchy.graphs.end();) {
745+
retire_graph(hierarchy, it++);
719746
}
720747
}
721748

‎cuda_core/cuda/core/_memory/_memory_pool.pyx‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ from cuda.core._rt cimport (
2222
create_mempool_handle,
2323
deviceptr_alloc_from_pool,
2424
get_last_error,
25+
attach_rollback_failure,
2526
as_cu,
2627
as_py,
2728
)
@@ -311,24 +312,48 @@ cdef int MP_raise_release_threshold(_MemPool self) except? -1:
311312
By default the release threshold is 0, meaning memory is returned to
312313
the OS as soon as there are no active suballocations. Setting it to
313314
ULLONG_MAX avoids repeated OS round-trips.
315+
316+
Pool attribute reads and writes are potentially unsafe calls while the
317+
calling thread is inside a global or thread-local stream capture: the
318+
driver refuses them and invalidates the capture. Capture mode is a
319+
per-thread property, so the thread is switched to relaxed mode around the
320+
two calls and its previous mode is restored afterwards. This has no
321+
observable effect when the thread is not capturing. The attribute write
322+
executes immediately rather than being recorded into a graph, which is the
323+
intent for a process-wide pool setting.
314324
"""
315325
MP_check_open(self)
316326
cdef cydriver.cuuint64_t current_threshold
317327
cdef cydriver.cuuint64_t max_threshold = ULLONG_MAX
328+
cdef cydriver.CUstreamCaptureMode mode = cydriver.CU_STREAM_CAPTURE_MODE_RELAXED
329+
cdef cydriver.CUresult err
330+
cdef cydriver.CUresult restore_err
318331
with nogil:
319-
HANDLE_RETURN(
320-
cydriver.cuMemPoolGetAttribute(
321-
as_cu(self._h_pool),
322-
cydriver.CUmemPool_attribute.CU_MEMPOOL_ATTR_RELEASE_THRESHOLD,
323-
&current_threshold
324-
)
332+
# On return, mode holds the thread's previous capture mode.
333+
HANDLE_RETURN(cydriver.cuThreadExchangeStreamCaptureMode(&mode))
334+
err = cydriver.cuMemPoolGetAttribute(
335+
as_cu(self._h_pool),
336+
cydriver.CUmemPool_attribute.CU_MEMPOOL_ATTR_RELEASE_THRESHOLD,
337+
&current_threshold
325338
)
326-
if current_threshold == 0:
327-
HANDLE_RETURN(cydriver.cuMemPoolSetAttribute(
339+
if err == cydriver.CUDA_SUCCESS and current_threshold == 0:
340+
err = cydriver.cuMemPoolSetAttribute(
328341
as_cu(self._h_pool),
329342
cydriver.CUmemPool_attribute.CU_MEMPOOL_ATTR_RELEASE_THRESHOLD,
330343
&max_threshold
331-
))
344+
)
345+
restore_err = cydriver.cuThreadExchangeStreamCaptureMode(&mode)
346+
try:
347+
HANDLE_RETURN(err)
348+
except:
349+
if restore_err != cydriver.CUDA_SUCCESS:
350+
# The attribute error propagates with the restore failure attached
351+
# as a note (error handling policy).
352+
attach_rollback_failure(
353+
b"cuThreadExchangeStreamCaptureMode", restore_err,
354+
b"failed to restore the thread's stream capture mode")
355+
raise
356+
HANDLE_RETURN(restore_err)
332357
return 0
333358

334359

‎cuda_core/cuda/core/_rt.pxd‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,7 @@ cdef cydriver.CUresult graph_commit_child_graph_update(
305305
PreparedChildGraphUpdate& prepared, GraphHandle* out_child) except+
306306
cdef void invalidate_child_graph_state(
307307
const GraphHandle& h_parent, cydriver.CUgraphNode owner_node) noexcept
308+
cdef void invalidate_root_graph_state(const GraphHandle& h_root) noexcept
308309

309310
# Graph exec handles
310311
cdef GraphExecHandle create_graph_exec_handle(

‎cuda_core/cuda/core/_rt.pyx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,8 @@ cdef extern from "_cpp/rt/rt.hpp" namespace "cuda_core::rt":
212212
PreparedChildGraphUpdate& prepared, GraphHandle* out_child) except+
213213
void invalidate_child_graph_state "cuda_core::rt::invalidate_child_graph_state" (
214214
const GraphHandle& h_parent, cydriver.CUgraphNode owner_node) noexcept
215+
void invalidate_root_graph_state "cuda_core::rt::invalidate_root_graph_state" (
216+
const GraphHandle& h_root) noexcept
215217

216218
# Graph exec handles
217219
GraphExecHandle create_graph_exec_handle "cuda_core::rt::create_graph_exec_handle" (

‎cuda_core/cuda/core/graph/_graph_builder.pyi‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,11 @@ class GraphBuilder:
136136
@staticmethod
137137
def _init(stream: Stream): ...
138138
def close(self):
139-
"""Destroy the graph builder."""
139+
"""Destroy the graph builder.
140+
141+
A builder that is still building ends its capture first. The builder
142+
is closed even when that fails; the driver error is raised afterwards.
143+
"""
140144
@property
141145
def is_closed(self) -> bool:
142146
"""Whether this graph builder has been closed."""
@@ -212,7 +216,22 @@ class GraphBuilder:
212216
def is_building(self) -> bool:
213217
"""Returns True if the graph builder is currently building."""
214218
def end_building(self) -> GraphBuilder:
215-
"""Ends the building process."""
219+
"""Ends the building process.
220+
221+
Raises
222+
------
223+
RuntimeError
224+
If the builder is not building. A forked builder is ended by
225+
:meth:`join`, not by this method.
226+
CUDAError
227+
If the capture was invalidated, for example by a CUDA call that is
228+
not permitted while capturing. The capture is ended and the
229+
builder holds no graph afterwards, so :meth:`complete` and
230+
:attr:`graph_definition` are unavailable. This applies to a
231+
top-level builder. An invalidated capture of a conditional body
232+
leaves the parent builder's graph invalid as well; ending it may
233+
crash the process (see issue #2918).
234+
"""
216235
def complete(self, options: GraphCompleteOptions | None=None) -> Graph:
217236
"""Completes the graph builder and returns the built :obj:`~graph.Graph` object.
218237
@@ -261,6 +280,9 @@ class GraphBuilder:
261280
"""Joins multiple graph builders into a single graph builder.
262281
263282
The returned builder inherits work dependencies from the provided builders.
283+
If joining fails partway, the builders that were not joined are closed
284+
before the error propagates, so none is left capturing. A driver error
285+
from one of those closes is attached to the propagating error as a note.
264286
265287
Parameters
266288
----------

0 commit comments

Comments
 (0)