Repository navigation
[BUG]: VirtualMemoryResource: four pre-existing defects (grow-rollback access loss, dead fast path, finalizer warnings, handle_type docstring) #2388
Description
Activity
- changed the title
[-][BUG]: VirtualMemoryResource: rollback of a failed grow loses access grants on the original mapping[/-][+][BUG]: VirtualMemoryResource: four pre-existing defects (grow-rollback access loss, dead fast path, finalizer warnings, handle_type docstring)[/+]on Jul 18, 2026 I would like to work on this, starting with defect 2. Happy to fold in the others if you would rather have them as one PR.
Confirming defect 2 against
main:cuMemAddressReservereturnsnew_ptras aCUdeviceptr, and the check at_virtual_memory_resource.py:268compares it against a plainint.CUdeviceptrincuda_bindings/cuda/bindings/driver.pyxdefines__int__and__repr__but no__eq__or__richcmp__, so the comparison falls back to identity and is always False. That makes the whole condition always true, andmodify_allocation()always takes the slow path with a full re-reserve and remap, even when the driver granted exactly the contiguous extension that was requested. Comparingint(new_ptr)fixes it, as noted in the report.One thing i say what is a direct consequence which is worth flagging is the fast path has effectively never run, it is untested in practice, so enabling it is a behavior change rather than a pure no-op cleanup. I would pair the fix with a unit test pinning the comparison semantics, that
CUdeviceptr(x) == xis False so call sites must go throughint(), since that is the failure mode likely to recur at other boundaries, plus a GPU test asserting the buffer's base pointer is preserved across a grow the driver can satisfy contiguously.- added a commit that references this issue
on Jul 22, 2026 - added a commit that references this issue
on Jul 31, 2026 - added 2 commits that reference this issue
on Sep 9, 2026 - added a parent issue
on Sep 17, 2026 - added a commit that references this issue
on Oct 2, 2026 All four items are fixed. #2917 removed the rollback remap (item 1), made the fast path live (item 2), and stopped the finalizer warning (item 3); #2418 corrected the docstring (item 4). Thanks @aryanputta for confirming item 2 against main. Closing.
Component
cuda.core
What happened?
Four pre-existing defects in
VirtualMemoryResource, all found while verifying #2235 (verification details in #2235 (review)); none are introduced by that PR.Rollback of a failed grow loses access grants — if
modify_allocation()fails after the old range has been remapped (slow path), the_remap_oldrollback restores the mapping at the original address but never re-applies the access descriptors, so the rolled-back buffer faults on its next access untilcuMemSetAccessis re-run. Reproduced onmainby forcingcuMemSetAccessto fail during a grow. Likely fix:_remap_oldshould re-apply the resource's access descriptors to the old range (best-effort, matching the remap itself).The grow fast path is dead code — this check compares a
CUdeviceptragainst a plainint, andCUdeviceptr(x) == xis alwaysFalse, somodify_allocation()always takes the slow path (full re-reserve + remap, base pointer changes) even when the driver granted the exact contiguous extension address. Fix: compareint(new_ptr).Warning spam after every slow-path grow — the slow path calls
buf._clear()so the old buffer's destructor won't double-free, but the destructor still callsdeallocate(), emittingWarning: mr.deallocate() failed during Buffer destruction: CUDA_ERROR_INVALID_VALUEat GC after each grow. (Adjacent to the existing TODO referencing Bug in_grow_allocation_fast_path#2049.)handle_typedocstring is wrong — the docstring claims posix_fd is "required for cuMemRetainAllocationHandle"; retain works onhandle_type=Noneallocations (verified on driver r595) and the driver documentation has no such restriction.-- Leo's bot