cuda_graph_t discards CUDA error codes, obscuring capture failures
Nobody has claimed this yet.
- Dominant language
- Cuda
- Stars
- 1k
- Forks
- 233
- Avg merge
- 4d 4h
- Merged PRs (30d)
- 95
Description
Summary
cuopt::routing::detail::cuda_graph_t (cpp/src/routing/cuda_graph.cuh) discards the return code of every CUDA call it makes:
void start_capture(rmm::cuda_stream_view stream)
{
cudaStreamBeginCapture(stream, cudaStreamCaptureModeThreadLocal);
capture_started = true;
}
void end_capture(rmm::cuda_stream_view stream)
{
...
cudaStreamEndCapture(stream, &graph);
...
cudaGraphExecUpdate(instance, graph, &errorNode, &updateResult);
...
cudaGraphInstantiate(&instance, graph);
cudaGraphDestroy(graph);
}
void launch_graph(rmm::cuda_stream_view stream) { cudaGraphLaunch(instance, stream); }
When a capture is invalidated mid-flight, cudaStreamEndCapture returns an error and sets graph to nullptr. That null graph is then passed to cudaGraphInstantiate, and instance is later launched, all unchecked.
Why this matters
This actively obstructed the diagnosis of #1748. The real failure was a single cudaErrorStreamCaptureUnsupported at one line inside a capture region. What CI showed instead was a cascade of downstream cudaErrorStreamCaptureInvalidated errors from unrelated call sites, because the first error was swallowed and execution continued on a null graph.
The failure was found in the end, but the logs pointed away from the cause rather than at it.
Prior attempt and why it was reverted
#1753 initially added RAFT_CUDA_TRY around these calls. That was scope creep on a CI hotfix and was reverted in favour of keeping that PR to the actual fix (see the review discussion there).
The attempt also surfaced two things worth carrying into any future change — both were bugs introduced by adding the checks naively, and both are traps for whoever does this properly:
-
capture_startedmust be cleared before the error propagates.RAFT_CUDA_TRY(cudaStreamEndCapture(...))throws before the flag is reset, leaving a reused wrapper with stale capture state. Store the code, clear the flag, then throw. -
A failed
cudaGraphInstantiateleaks the captured graph. Throwing skips thecudaGraphDestroy(graph)at the end ofend_capture. Destroy before propagating, or use a scope guard.
cpp/src/utilities/manual_cuda_graph.cuh already handles both correctly and is the reference for the ordering.
Open questions for whoever picks this up
cudaGraphExecUpdate. Its failure is a normal, expected path — the code falls back to re-instantiation. But the current code cannot distinguish "update failed, re-instantiate" from a genuine error, since it only inspectsupdateResultand never the API return code.cudaErrorGraphExecUpdateFailureshould feed the existing fallback; anything else is probably a real error.- Should
launch_graphverifygraph_created? Raised in review on #1753: if instantiation failed,instancemay be unset or destroyed. - Testing. There is currently no coverage of these paths, and exercising a failed
cudaGraphInstantiateneeds CUDA fault injection, whichcpp/testsdoes not have. Note also thatcpp/tests/routing/local_search_cand_test.cuis not registered incpp/tests/routing/CMakeLists.txt.
Related
- #1748 — the CUDA 13 capture failures this obscured
- #1753 — the fix, where the error checks were reverted out
- rapidsai/rmm#2518 — the upstream cause
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in cpp/src/routing/cuda_graph.cuh and compare its capture-state and graph cleanup ordering with cpp/src/utilities/manual_cuda_graph.cuh. Review the CUDA return-code handling, the cudaGraphExecUpdate fallback, and the existing routing tests; done means failures propagate without stale capture state or leaked graphs, with coverage added where the test infrastructure permits.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- hpc, testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100