Conversation
The slow path of VirtualMemoryResource.modify_allocation unmapped the old VA range, remapped the physical memory into the new range, freed the old reservation by hand and then reset the old buffer's handle with Buffer._clear(). That reset runs the handle's deleter, which calls mr.deallocate() on the range that was just freed. The failing cuMemRetainAllocationHandle is reported as a CUDAWarning since NVIDIA#2759 and was silently swallowed before. Map the old physical memory into the new range as a second mapping instead, which the VMM APIs allow (virtual aliasing), and then close the old buffer. Closing runs deallocate() on a range that is still mapped, so the old range is unmapped, its reservation freed and its handle reference released through the normal path. The remap-on-rollback undo step, which swallowed its own errors, is no longer needed because the old mapping is never removed before the transaction commits. Issue NVIDIA#2877 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
The slow-path regression test fills the buffer with cuMemsetD8 and grows it at once. Memset is asynchronous with respect to the host, and on WDDM the batched kernel can still be pending when the grow unmaps the old range, so it faults with a sticky CUDA_ERROR_ILLEGAL_ADDRESS that took every later test in the job down with it. Synchronize before the grow. Issue NVIDIA#2877 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
VirtualMemoryResource never dropped the reference that cuMemCreate returns. allocate() registered the release only as a rollback action, which commit discards, and deallocate() releases only the reference it retains itself. The driver frees an allocation only once every mapping is unmapped and every handle reference is released, so every buffer's physical memory stayed allocated after close() until the process exited. Both grow paths leaked the new chunk the same way, and the slow path also kept the reference it retained on the old handle. Release the creation reference right after the mapping is attempted, in allocate() and in both grow paths, and release the retained old handle after it is mapped into the new range. The mapping holds its own reference, so the memory stays alive while mapped and is freed by deallocate(), which unmaps the whole range and releases the one reference it retains. Issue NVIDIA#2882 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Andy-Jost
added
bug
Contributor
Author
|
Closing in favor of #2235, which fixes the same leak (#2344), was approved in July, and expresses the release-on-both-outcomes rule through a |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2882.
VirtualMemoryResourcenever dropped the reference thatcuMemCreatereturns:allocate()registered the release only as a rollback action, whichcommit()discards, anddeallocate()releases only the reference it retains itself. The driver frees an allocation only once every mapping is unmapped and every handle reference is released, so every buffer's physical memory stayed allocated afterclose()until the process exited. On an H100, allocating and closing 64 buffers of 64 MiB left 4096 MiB retained. Both grow paths leaked the new chunk the same way, and the slow path also kept the reference it retained on the old handle.Changes
_virtual_memory_resource.py:allocate()and both grow paths release the creation reference in afinallyright after the mapping is attempted, on success and failure alike; the slow path releases the retained old handle the same way after mapping it into the new range. The mapping holds its own reference, so the memory stays alive while mapped, anddeallocate()frees it by unmapping the whole range and releasing the one reference it retains. Rollback behavior is unchanged: on failure the undo actions unmap and free the range, and the released handle then has no mappings left.tests/test_memory.py:test_vmm_allocator_close_returns_physical_memoryallocates and closes eight 32 MiB buffers, then grows eight more through the slow path and closes them, and checks that free device memory returns each time. The mocked fast-path test now expects the release call after the mapping.Related Work
🤖 Generated with Claude Code