fix(cuda): harden driver error handling, access tracking and host copies - #10871

@claude review this pr in depth

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

Review of PR #10871: CUDA driver error handling, access tracking and host copies

• Gather context
• Read the driver diffs (context, stream, memory, launch, sys, module)
• Read the downstream changes (pinned.rs, executor.rs, stream.rs)
• Review the tests
• Post review

I didn't find any blocking problems. The error-handling and tracking fixes look right, and each commit does what its message says. The main thing I'd change is that the new check for page-locked memory fails open. Details below, most important first.

1. borrowed() treats every driver error as pageable memory (medium)

vortex-cuda/driver/src/memory.rs, fn borrowed:

let known = unsafe { sys::cuPointerGetAttribute(...) }.result().is_ok();
if known && memory_type == sys::CU_MEMORYTYPE_HOST { wait_for } else { none }

The only error that means "pageable memory" is CUDA_ERROR_INVALID_VALUE. Other errors make the copy skip the wait without any signal, for example CUDA_ERROR_INVALID_CONTEXT / NOT_INITIALIZED, or a DEINITIALIZED during shutdown. This is now a soundness question, for two reasons:

• borrowed() runs before self.bind() in both memcpy_htod and memcpy_dtoh. So the query can run on a thread with no current context. Today's callers (clone_htod, copy_to_device*) happen to allocate first, which binds the context, but nothing guarantees that.
• VortexCudaStream::copy_to_device (vortex-cuda/src/stream.rs) no longer keeps data alive in the future (_keep_alive was removed). If the query fails on page-locked data, the source can be freed while the DMA still reads it. Before this PR, the future kept it alive.

Suggestion: match on the error. Return SyncOnDrop::none() only for CUDA_ERROR_INVALID_VALUE, and return SyncOnDrop::wait_for(stream) for anything else. Optionally call stream.make_current() before the query. Keeping let _keep_alive = data; in copy_to_device also costs nothing and adds a second safeguard.

Fix this →

2. recorded_errors_do_not_leak_frees allocates 4 GiB (low to medium, flaky test)

vortex-cuda/driver/src/tests.rs: let bytes = 4 << 30; with alloc_zeros(bytes)?. On a GPU with 8 GiB or less, or when other tests in the same process allocate at the same time (cargo test threads share the default pool), this returns CUDA_ERROR_OUT_OF_MEMORY. Because of the ?, the test then fails instead of skipping.

The leak check only needs an allocation clearly larger than what other tests use at the same time. Something like 512 MiB would do, or skip the test on OOM.

Fix this →

3. memcpy_dtoh now synchronizes twice for page-locked targets (nit, performance)

memcpy_dtoh already ends with cuStreamSynchronize. Because the slice HostSlice impls now go through borrowed(), every memcpy_dtoh also pays a cuPointerGetAttribute. For page-locked targets it then runs a second cuStreamSynchronize when the Wait guard drops. Both are harmless but unnecessary.

If it matters, the dtoh path could skip the guard, for example with an internal non-guarded slice accessor. The PR description already mentions the extra query, so this is optional.

4. A cross-context access now panics (nit, behaviour change)

The assert_eq! in Access::before is sound. It runs before the lock, so the mutex isn't poisoned, and it matches the module's "caller bugs panic" policy. It is still a public behaviour change: CUDA itself allows a device pointer to be used from another context on the same device. PooledPinnedBuffer::transfer_to_device already rejects this, and I couldn't find any in-tree caller that relies on it. It might be worth a line in the changelog or the DevicePtr docs.

Things I checked that look correct

• record_err keeps the first error (compare_exchange(0, …)). check_err still swaps the value to 0, so later errors are recorded again once the first one is checked. ✅
• make_current vs bind_to_thread. Cleanup paths (free_raw, before_free, after, event and stream drops) no longer take an unrelated pending error, so a recorded error can no longer leak a free. User-facing synchronize() still calls check_err first, and the new _in_place variants are pub(crate). ✅
• Access::after failure paths. Before, the Write branch did state.write.take() before record_raw, so a failed record dropped the previous write. Now the event comes back from the state on success, or goes back to the pool when it was freshly taken. The Read branch does the same thing. ✅
• access() falls back to a context-wide sync. All copy paths call self.bind()? after device_ptr*, which returns the recorded error, and launch uses before(..)?, so in those paths the fallback is mostly redundant. It does matter for raw-pointer users such as pooled_read_at/file.rs::copy_to_device. That function binds before device_ptr_mut and then launches kernels without checking errors again, so keeping the fallback is the right call. ✅
• launch now tracks every access even if an earlier one fails, and keeps the first error. ✅
• CudaSlice::drop leaks the memory when before_free fails, instead of freeing memory that other streams may still read without the waits. release still returns the events to the pool, which is fine: waits that were already enqueued captured the earlier event state. ✅
• Empty allocations. alloc_raw(0) returns 0 after bind(), and free_raw(0) does nothing. This matches cuMemAllocAsync's behaviour and fixes the cuMemAlloc path. ✅
• The fits::<S> overflow check in all four transmute* methods, and the new alignment requirements in their docs. ✅
• Copy bounds use the copied slice, not HostSlice::len. Both asserts now run on the slice the trait impl returns, and the ShortHostSlice test covers both directions. ✅
• Sealing DeviceSlice. The only impls are CudaSlice, CudaView and CudaViewMut. DevicePtr / DevicePtrMut inherit the seal through their supertrait. ✅
• Guard drop order. In memcpy_htod, the target-access guard (inside memcpy_htod_async) drops before the source Wait guard. In memcpy_dtoh, the source access guard drops before the target guard. Both orders are correct. ✅
• pinned.rs. Removing PinnedByteBufferView and using memcpy_htod_async plus one checked event.record keeps the same safety argument, and removes the second cuEventRecord per transfer. Pinned memory doesn't seem to escape into host ByteBuffers (device_read_at, ensure_on_device), so the new synchronous behaviour shouldn't hit the hot read paths. ✅
• New sys.rs constants match cuda.h: CU_MEMPOOL_ATTR_USED_MEM_CURRENT = 7, CU_POINTER_ATTRIBUTE_MEMORY_TYPE = 2, CU_MEMORYTYPE_HOST = 1. ✅

Checks

I only read the code. I didn't compile it or run any tests: this runner has no GPU, and the review workflow is read-only.

Merging this PR will improve performance by 13.48%

⚡ 2 improved benchmarks
✅ 2305 untouched benchmarks
⏩ 38 skipped benchmarks¹
🗄️ 47 archived benchmarks run²

Performance Changes

_{Comparing ad/cuda-driver-access-tracking-fixes (f22338a) with develop (246a397)}