fix(cuda): do not leak contexts when construction fails - #10872

@claude review this pr in depth

Claude finished @0ax1's task in 41s —— View job

Review of #10872: fix(cuda): do not leak contexts when construction fails

Verdict: correct, and ready to merge. I found no bugs. There are two optional wording nits below.

The fix

Before this PR, both constructors had a fallible step between taking ownership of the context and building the CudaContext whose Drop would clean it up:

If the attribute query failed on the old order, new kept a primary-context refcount that nothing released. new_non_primary left a live context on top of the thread's context stack. The PR description is accurate.

Why the new order is safe

• cuDeviceGetAttribute needs no context. It only takes a CUdevice, so it can run before any context exists. result::device::get_attribute (result.rs:145) is a thin wrapper and has no context requirement, so the SAFETY comment device is valid still holds.
• Nothing fallible runs between acquiring and wrapping. After cuDevicePrimaryCtxRetain / cuCtxCreate_v2 succeed, the next step is Self::wrap. There, Arc::new comes before the only remaining fallible call, bind_to_thread().
• A bind_to_thread failure in wrap is cleaned up. Returning Err drops the only Arc, so Drop runs:
  • Its own bind_to_thread failure goes through record_err, which is harmless because the object is being torn down.
  • events is empty, so nothing else needs cleanup.
  • cuDevicePrimaryCtxRelease_v2(device) takes only the device. cuCtxDestroy_v2(ctx) does not require the context to be current, and the driver pops it from the calling thread's stack if it is current there. Either way, the retain or create is undone once.
• Error order is unchanged. init, then the device lookup (including INVALID_VALUE for an ordinal that does not fit in an i32), then the attribute query. The only difference is that the attribute error now comes before the retain or create, not after.
• Call sites. device and wrap are private, with just these two call sites, so the signature changes go no further.

Nits (optional)

1. context.rs:103-104: the doc comment on device overstates things. "Everything that can fail is queried here, before a context is retained or created, so that no failure leaves one behind" isn't literally true, since the retain or create can still fail, and so can bind_to_thread in wrap. Those cases are safe for other reasons: nothing is acquired yet, or Drop handles it. A tighter version:

   Queries everything that does not need a context here, so that nothing fallible runs between acquiring the context and wrapping it in a CudaContext whose Drop releases it.

2. context.rs:120-126: wrap now takes two adjacent bools, has_async_alloc and primary, with context/ordinal between them. A call that swaps them would still compile. With only two private call sites the risk is low, so this is fine as is. If you'd like it a bit clearer, you could put the two bools next to each other or name them at the call site.

Tests

There's no regression test. That's reasonable here: triggering this path means making cuDeviceGetAttribute fail after cuDeviceGet succeeds, which needs driver fault injection that this crate doesn't have.

Checks

I reviewed the code by reading it and did not compile it or run any checks, matching the PR's note that CI runs them. The change only reorders calls and threads one extra bool through, so there is little compile risk.

Merging this PR will improve performance by 11.7%

⚡ 1 improved benchmark
✅ 1876 untouched benchmarks
⏩ 468 skipped benchmarks¹
🗄️ 47 archived benchmarks run²

Performance Changes

_{Comparing ad/cuda-context-construction-leak (509ef36) with develop (eed0212)}