Skip to content

Fix persistent buffer mapping balance and prevent double map#87

Merged
Mrkol merged 3 commits into
AlexandrShcherbakov:mainfrom
Nerfiti:fix/buffer-vma-persistent-mapping-unmap
May 4, 2026
Merged

Fix persistent buffer mapping balance and prevent double map#87
Mrkol merged 3 commits into
AlexandrShcherbakov:mainfrom
Nerfiti:fix/buffer-vma-persistent-mapping-unmap

Conversation

@Nerfiti

@Nerfiti Nerfiti commented May 4, 2026

Copy link
Copy Markdown
Contributor

Problem

Buffers created with VMA_ALLOCATION_CREATE_MAPPED_BIT receive a
persistently mapped pointer without an explicit vmaMapMemory call.
The destructor's unmap() then triggers a VMA assertion because there
is no matching explicit vmaMapMemory.

Solution

Explicitly call map() in the constructor when the persistent flag
is set, pairing with the unmap() in reset().

Forbid further map() calls via ETNA_VERIFY(mapped == nullptr).
If the user were allowed to call map()/unmap() manually, a single
unmap() would set mapped = nullptr, breaking the pairing with the
destructor's unmap() — the constructor's map() would remain
unmatched forever.

@Nerfiti Nerfiti marked this pull request as ready for review May 4, 2026 12:34

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread etna/source/Buffer.cpp Outdated
// make map() optional if allocationCreate has VMA_ALLOCATION_CREATE_MAPPED_BIT
mapped = reinterpret_cast<std::byte*>(allocInfo.pMappedData);
// make map() forbidden for the user if allocationCreate has VMA_ALLOCATION_CREATE_MAPPED_BIT
if (info.allocationCreate & VMA_ALLOCATION_CREATE_MAPPED_BIT)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: implicit conversion 'VmaAllocationCreateFlags' (aka 'unsigned int') -> 'bool' [readability-implicit-bool-conversion]

Suggested change
if (info.allocationCreate & VMA_ALLOCATION_CREATE_MAPPED_BIT)
if ((info.allocationCreate & VMA_ALLOCATION_CREATE_MAPPED_BIT) != 0u)

@Mrkol Mrkol merged commit e498aab into AlexandrShcherbakov:main May 4, 2026
6 of 7 checks passed
kzubatov added a commit to kzubatov/etna that referenced this pull request May 11, 2026
Mrkol pushed a commit that referenced this pull request May 13, 2026
* Revert "Fix persistent buffer mapping balance and prevent double map (#87)"

This reverts commit e498aab.

* buffer: do not call unmap in reset

if VMA_ALLOCATION_CREATE_MAPPED_BIT is set, the buffer is always mapped
  and unmap should not be called in reset, see https://gpuopen-librariesandsdks.github.io/VulkanMemoryAllocator/html/group__group__alloc.html#ggad9889c10c798b040d59c92f257cae597a11da372cc3a82931c5e5d6146cd9dd1f:~:text=It%20is%20also%20safe,to%20VMA%5FALLOCATION%5FCREATE%5FMAPPED%5FBIT%20flag
If map is called explicitly, instead of creating the buffer using
  VMA_ALLOCATION_CREATE_MAPPED_BIT, unmap must also be called explicitly

Removed map/unmap calls in helper functions, since buffers are created
  with VMA_ALLOCATION_CREATE_MAPPED_BIT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants