Catcrafts/Crafter.Graphics
2
0
Fork 0
Code Issues 1 Pull requests Projects Releases Packages Wiki Activity Actions

fix(device): fence-keyed deferred resource-deletion queue (#101)

Browse source 
Since #40 dropped the per-frame vkQueueWaitIdle, frames are pipelined:
a resource the CPU is done with may still be read by the GPU for up to
numFrames-1 more frames. VulkanBuffer::Resize's destroy-and-recreate
path was therefore a live use-after-free (#63), no longer masked by the
wait-idle.

Device gains a monotonic, frame-counter-keyed deletion queue:
EnqueueDeletion tags {buffer, memory} with the current frameCounter;
ReclaimDeletions (called per frame after the fence wait) frees entries
once framesInFlight frames have elapsed; DrainDeletions frees everything
after a wait-idle. VulkanBuffer::DeferredClear hands handles to the queue
and nulls the handle, and Resize uses it instead of immediate Clear().

Window::Render sets framesInFlight at init, reclaims after the per-image
fence wait, bumps Device::frameCounter once per frame, and drains on the
resize / OUT_OF_DATE wait-idle paths. The destructor keeps immediate
Clear() (callers destroying mid-flight remain responsible, unchanged).

Adds the DeferredDeletion test: drives the retire timing on a real
headless device with real buffers, stepping Device::frameCounter to pin
the exact reclaim frame, asserting validation stays silent.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
catbot 2026-06-17 13:21:12 +00:00
commit cb012d7068
6 changed files with 312 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

171
tests/DeferredDeletion/main.cpp Normal file
View file

@ -0,0 +1,171 @@
/*
Crafter®.Graphics
Copyright (C) 2026 Catcrafts®
catcrafts.net
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
License version 3.0 as published by the Free Software Foundation;
This library is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
Lesser General Public License for more details.
You should have received a copy of the GNU Lesser General Public
License along with this library; if not, write to the Free Software
Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
// Issue #101: fence-keyed deferred resource-deletion queue. Since #40 dropped
// the per-frame vkQueueWaitIdle, a buffer the CPU is done with may still be
// read by the GPU for up to framesInFlight-1 more frames — so destroying it
// immediately (VulkanBuffer::Clear / the old Resize path) is a use-after-free.
// VulkanBuffer::DeferredClear() / Resize() now hand the handles to Device's
// deletion queue, tagged with the current frameCounter; ReclaimDeletions frees
// an entry only once framesInFlight frames have elapsed, and DrainDeletions
// (called after a wait-idle) frees everything unconditionally.
//
// The retire-timing is the load-bearing logic: free too early and it's the
// very UAF the queue exists to prevent; free too late and resources leak. This
// test drives it on a real headless Vulkan device (real VkBuffer/VkDeviceMemory
// so the frees actually execute and the validation layer can object), manually
// stepping Device::frameCounter to pin down the exact frame an entry is
// reclaimed on. It needs no swapchain/window — the queue itself is window-free.
#include "vulkan/vulkan.h"
#include <cstdlib>
import Crafter.Graphics;
import std;
using namespace Crafter;
namespace {
int failures = 0;
void Check(bool ok, std::string_view what) {
std::println("{} {}", ok ? "PASS" : "FAIL", what);
if (!ok) ++failures;
}
constexpr VkBufferUsageFlags2 USAGE =
VK_BUFFER_USAGE_2_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_2_SHADER_DEVICE_ADDRESS_BIT;
constexpr auto DEVICE_LOCAL = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
} // namespace
int main() {
Device::Initialize();
// No Window here, so set the in-flight depth ourselves (Window::Render
// normally does this at init). Use the real window depth so the timing
// matches production.
Device::framesInFlight = Window::numFrames;
Device::frameCounter = 0;
Device::deletionQueue.clear();
// ── DeferredClear enqueues, nulls the handle, and retires after exactly
// framesInFlight frames ──────────────────────────────────────────────
{
VulkanBuffer<float, false> buf;
buf.Create(USAGE, DEVICE_LOCAL, 16);
Check(buf.buffer != VK_NULL_HANDLE, "Create produced a real buffer handle");
buf.DeferredClear();
Check(buf.buffer == VK_NULL_HANDLE,
"DeferredClear nulls the handle so the buffer no longer owns it");
Check(Device::deletionQueue.size() == 1,
"DeferredClear enqueues exactly one pending deletion");
// Walk right up to the retire frame: still queued at every frame before
// frameCounter reaches retireAfter + framesInFlight.
for (std::uint64_t f = 0; f < Window::numFrames; ++f) {
Device::frameCounter = f;
Device::ReclaimDeletions();
Check(Device::deletionQueue.size() == 1,
std::format("entry survives ReclaimDeletions at frame {} "
"(< retire frame {})", f, Window::numFrames));
}
// At retireAfter(0) + framesInFlight it is finally freed.
Device::frameCounter = Window::numFrames;
Device::ReclaimDeletions();
Check(Device::deletionQueue.empty(),
std::format("entry is reclaimed once frameCounter reaches {}",
Window::numFrames));
}
// ── Resize's reallocate path defers the OLD allocation, not the new one ──
{
Device::frameCounter = 100;
Device::deletionQueue.clear();
VulkanBuffer<float, false> buf;
buf.Create(USAGE, DEVICE_LOCAL, 4);
VkBuffer original = buf.buffer;
// Grow past capacity → forces destroy+recreate, which must DEFER the old
// buffer (the #63 UAF this whole queue exists to close).
buf.Resize(USAGE, DEVICE_LOCAL, 64);
Check(buf.buffer != VK_NULL_HANDLE && buf.buffer != original,
"Resize past capacity allocates a fresh buffer");
Check(Device::deletionQueue.size() == 1
&& Device::deletionQueue.front().buffer == original,
"Resize defers the OLD allocation to the deletion queue");
Check(Device::deletionQueue.front().retireAfter == 100,
"deferred entry is tagged with the frameCounter at enqueue time");
// Not yet due (tagged at 100, retires at 100 + framesInFlight).
Device::frameCounter = 100 + Window::numFrames - 1;
Device::ReclaimDeletions();
Check(Device::deletionQueue.size() == 1,
"old allocation is not freed before its retire frame");
Device::frameCounter = 100 + Window::numFrames;
Device::ReclaimDeletions();
Check(Device::deletionQueue.empty(),
"old allocation is freed on its retire frame");
buf.Clear(); // immediate cleanup of the live buffer (no GPU work pending)
}
// ── DrainDeletions frees everything regardless of retire frame ───────────
{
Device::frameCounter = 0;
Device::deletionQueue.clear();
VulkanBuffer<float, false> a;
VulkanBuffer<float, false> b;
a.Create(USAGE, DEVICE_LOCAL, 8);
b.Create(USAGE, DEVICE_LOCAL, 8);
a.DeferredClear();
b.DeferredClear();
Check(Device::deletionQueue.size() == 2, "two entries queued");
// Far from any retire frame — ReclaimDeletions would free nothing here.
Device::DrainDeletions();
Check(Device::deletionQueue.empty(),
"DrainDeletions frees every queued entry unconditionally");
}
// ── EnqueueDeletion ignores a null handle (nothing to free) ──────────────
{
Device::deletionQueue.clear();
Device::EnqueueDeletion(VK_NULL_HANDLE, VK_NULL_HANDLE);
Check(Device::deletionQueue.empty(),
"EnqueueDeletion drops a VK_NULL_HANDLE buffer");
}
Check(Device::validationErrorCount == 0,
std::format("no Vulkan validation errors across the run ({} seen)",
Device::validationErrorCount));
if (failures != 0) {
std::println("{} check(s) failed", failures);
return EXIT_FAILURE;
}
std::println("all checks passed");
return EXIT_SUCCESS;
}