[MG_Backend] (P7): the descriptor memos fold in a wire-store destroy epoch, so a VkBuffer handle the mid-frame reclaim freed and the next mint handed back cannot revive a set baked to the dead store

- M2's defer path destroys a wire store MID-FRAME (the serial-0 park, the sweep's prefix walk, the watermark); before it a wire store only died at the boundary that also clears UniformManager's memos (BeginFrame from Present or a drain). Those memos are keyed on the VkBuffer HANDLE - m_descriptorReuseMemo hashes the VkDescriptorBufferInfo words, FastRebindMemo compares uboBuffer - and on the wire arm the direct-bind UBO and the SSBO descriptor name the store's own handle. A store destroyed at a park, its handle value re-minted by the next Create (a heap pointer under lavapipe, LIFO out of glibc's tcache), and a draw resolving the same (handle, range) reused a set whose descriptor still pointed at the dead store's memory: silent wrong bytes, no Fatal (ID-P7-43, the fable review's must-fix).
- VkBufferManager gains m_wireStoreDestroyEpoch, bumped on every path that destroys a wire store's VkBuffer (DeferWireRelease's immediate destroy, SweepDeferredWireReleases once per sweep that freed anything, DestroyAllDeferredWireReleases, Shutdown's record teardown) and never reset, with GetWireStoreDestroyEpoch(). UniformManager mixes it into the descriptor-reuse signature and records it in FastRebindMemo, refusing the fast rebind when it moved; all under MOBILEGL_BUILD_DISAGGREGATED. Deliberately not m_sliceEpochCounter, which every WriteWireBuffer bumps and would defeat the memo on every glBufferSubData. On the wire arm only the signature is reached (SetupWireDraw passes no sampler hint), the FastRebindMemo field is the defensive half.
- Two comments from the same review: SweepDeferredWireReleases no longer claims GetSyncPointSubmitIndex() never decreases - it steps back by one across an abandoned recording (RecreateSwapchain, the minimized Present), and the prefix walk is conservative across that, not wrong; and the header's "floor is not a proof" keeps its decision on its first reason alone, the second was a stale citation of a fix that is on the integration tree.
- Section 9 partition exception for UniformManager.{h,cpp} granted by the integrator for this fix.
This commit is contained in:
2026-09-22 22:51:20 -04:00
parent efa19cb16f
commit 4d20746831
4 changed files with 75 additions and 9 deletions
@@ -3027,6 +3027,12 @@ namespace MobileGL::MG_Backend::DirectVulkan {
(samplerBindingOverrides == nullptr || samplerBindingOverrides->empty());
if (cacheable && samplerDescriptorsUnchangedHint && m_fastRebindMemo.valid &&
m_fastRebindMemo.frameIndex == frameIndex &&
#if MOBILEGL_BUILD_DISAGGREGATED
// A wire store destroyed since the memo was taken may have handed its handle
// value to a later mint (see FastRebindMemo): "same VkBuffer" then names a
// different store, so the memo is refused and the full walk re-records it.
m_fastRebindMemo.wireStoreDestroyEpoch == m_bufferManager->GetWireStoreDestroyEpoch() &&
#endif
m_fastRebindMemo.programLifetimeId == program.GetLifetimeId() &&
m_fastRebindMemo.programHash == programObj.hash) {
VkBuffer uboBuffer = VK_NULL_HANDLE;
@@ -3323,6 +3329,16 @@ namespace MobileGL::MG_Backend::DirectVulkan {
}
};
mixWords(&programObj.descriptorSetLayout, sizeof(programObj.descriptorSetLayout));
#if MOBILEGL_BUILD_DISAGGREGATED
// P7 M2 round 2 (ID-P7-43): the buffer infos below name WIRE stores by VkBuffer
// handle, and a wire store can be destroyed mid-frame (VkBufferManager::
// DeferredWireRelease) with its handle value re-minted before this frame's memo
// is cleared. Folding the destroy epoch in makes every entry taken before such a
// destroy miss, so a byte-identical info can never revive a set baked to a dead
// store. NOT the slice epoch: that moves on every glBufferSubData and would make
// the memo miss on every draw.
mix64(m_bufferManager->GetWireStoreDestroyEpoch());
#endif
for (const auto& write : writes) {
mix64((static_cast<Uint64>(write.dstBinding) << 40) ^
(static_cast<Uint64>(write.descriptorType) << 8) ^
@@ -3377,7 +3393,11 @@ namespace MobileGL::MG_Backend::DirectVulkan {
m_fastRebindMemo = FastRebindMemo{
/*valid=*/true, frameIndex, program.GetLifetimeId(), programObj.hash,
fastRebindUboBinding, bufferInfos[0].buffer,
bufferInfos[0].range, descriptorSet};
bufferInfos[0].range, descriptorSet,
#if MOBILEGL_BUILD_DISAGGREGATED
m_bufferManager->GetWireStoreDestroyEpoch(),
#endif
};
} else {
m_fastRebindMemo.valid = false;
}
@@ -442,6 +442,15 @@ namespace MobileGL::MG_Backend::DirectVulkan {
// drops the hint upstream; an arena wrap or growth resolves a different
// VkBuffer and misses. AcquireDescriptorSet's per-frame cursor only
// advances, so the recorded set is never re-written within its frame.
// P7 M2 round 2 (ID-P7-43): a WIRE store can die MID-FRAME on the reclaim
// path (VkBufferManager::DeferredWireRelease), and the next mint can hand
// its VkBuffer handle value back - a heap pointer under lavapipe - so "the
// same VkBuffer+range" is no longer proof of the same store. The memo
// therefore records VkBufferManager::GetWireStoreDestroyEpoch() and misses
// once any wire store has been destroyed since; the descriptor-reuse
// signature below folds the same epoch in. On the wire arm this memo is
// never consulted (SetupWireDraw passes no sampler hint), so the field is
// the defensive half; the signature is the half the wire arm reaches.
struct FastRebindMemo {
Bool valid = false;
Uint32 frameIndex = 0;
@@ -451,6 +460,9 @@ namespace MobileGL::MG_Backend::DirectVulkan {
VkBuffer uboBuffer = VK_NULL_HANDLE;
VkDeviceSize uboRange = 0;
VkDescriptorSet set = VK_NULL_HANDLE;
#if MOBILEGL_BUILD_DISAGGREGATED
Uint64 wireStoreDestroyEpoch = 0;
#endif
};
FastRebindMemo m_fastRebindMemo;
@@ -590,6 +590,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
// DeferredWireRelease - so there is nothing to wait for.
if (m_deferredBufferReleases.empty() || lastUseSerial == 0) {
buffer.Destroy();
++m_wireStoreDestroyEpoch;
if (m_wireStoreCount > 0) --m_wireStoreCount;
return;
}
@@ -662,11 +663,17 @@ namespace MobileGL::MG_Backend::DirectVulkan {
// instead of submitted.
dead = m_deferredWireReleases.size();
} else {
// The list is in park order and GetSyncPointSubmitIndex() never decreases, so the
// submit indices are non-decreasing and the dead entries are a PREFIX: the first
// entry the renderer will not call complete proves none after it is either. That
// bounds the walk to a couple of fence polls, which matters because this runs on
// every glBufferData the wire carries.
// The list is in park order and GetSyncPointSubmitIndex() steps back only when a
// pending recording is abandoned instead of submitted (RecreateSwapchain and the
// minimized Present force-clear the recording flags, so `m_submitCounter + 1`
// becomes `m_submitCounter`). Everywhere else the submit indices are non-decreasing
// and the dead entries are a PREFIX: the first entry the renderer will not call
// complete proves none after it is either. Across an abandonment a later entry can
// carry the LOWER index; the walk then stops at the earlier, higher one and holds
// both until the next submission takes that index and retires - conservative, never
// early, and the idle rule above retires them regardless. That bounds the walk to a
// couple of fence polls, which matters because this runs on every glBufferData the
// wire carries.
while (dead < m_deferredWireReleases.size() &&
pVulkanRenderer->IsSubmitIndexComplete(m_deferredWireReleases[dead].submitIndex)) {
++dead;
@@ -678,6 +685,9 @@ namespace MobileGL::MG_Backend::DirectVulkan {
m_deferredWireBytes -= entry.bytes;
entry.buffer.Destroy();
}
// One bump per sweep that destroyed anything is enough: what a memo needs to know
// is "some handle it may name has been freed since", not how many.
++m_wireStoreDestroyEpoch;
m_deferredWireReleases.erase(m_deferredWireReleases.begin(),
m_deferredWireReleases.begin() + static_cast<std::ptrdiff_t>(dead));
const Uint64 destroyedCount = static_cast<Uint64>(dead);
@@ -686,6 +696,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
}
void VkBufferManager::DestroyAllDeferredWireReleases() {
if (!m_deferredWireReleases.empty()) ++m_wireStoreDestroyEpoch;
for (auto& entry : m_deferredWireReleases) {
entry.buffer.Destroy();
if (m_wireStoreCount > 0) --m_wireStoreCount;
@@ -738,6 +749,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
#if MOBILEGL_BUILD_DISAGGREGATED
// DestroyAllDeferredReleases above emptied the parked list; this destroys the stores the
// records still hold, so nothing this arm minted outlives the count.
if (!m_wireBuffers.empty()) ++m_wireStoreDestroyEpoch;
m_wireBuffers.clear();
m_wireStoreCount = 0;
m_wireStoreCountPeak = 0;
@@ -208,6 +208,13 @@ namespace MobileGL::MG_Backend::DirectVulkan {
// back, and none was persistently mapped, in between - so a memo of resolved
// slices needs no per-buffer re-check. See AcquirePersistentMap for the mapping half.
Uint64 GetSliceEpochCounter() const { return m_sliceEpochCounter; }
#if MOBILEGL_BUILD_DISAGGREGATED
// Bumped every time this manager destroys a WIRE store's VkBuffer (see
// m_wireStoreDestroyEpoch). Unchanged since a memo was taken means no VkBuffer handle
// that memo names can have been freed and re-minted in between, which is the one
// fact a handle-keyed memo of wire descriptors needs and cannot read off the handle.
Uint64 GetWireStoreDestroyEpoch() const { return m_wireStoreDestroyEpoch; }
#endif
// Highest frame serial whose GPU work is known complete; serials at or
// below it may be considered signaled. Drives IsResourceBusy and the
// backend GL fence objects.
@@ -283,9 +290,19 @@ namespace MobileGL::MG_Backend::DirectVulkan {
//
// THE FRAME-SERIAL FLOOR IS DELIBERATELY NOT A PROOF HERE. It cannot move inside a
// frame (NotifyFrameSerialComplete refuses the current serial), so it frees nothing in
// the one-present replay this exists for, and on this package's base it is the floor
// B3 found unsound (a serial with two submissions was declared complete on the first
// fence, 6a92a10dd): a free on that floor is a GPU use-after-free, not a wrong pixel.
// the one-present replay this exists for; the submit index above is the fact that CAN
// move mid-frame, and it is a fence observation rather than a count.
//
// AND EVERY DESTROY HERE HAPPENS MID-FRAME, which the memos above this manager were not
// written for. UniformManager's descriptor memos are keyed on the VkBuffer HANDLE, and
// before M2 a wire store only ever died at the same boundary that clears those memos
// (UniformManager::BeginFrame, from Present or a drain). A store destroyed here can
// have its handle value re-minted by the next Create - a heap pointer under lavapipe -
// while a memo still maps that handle to a descriptor set baked to the dead store's
// memory: silent wrong bytes, no Fatal (ID-P7-43). m_wireStoreDestroyEpoch is the
// fact the memos fold in: every wire-store destroy bumps it, so a memo taken before
// the destroy cannot match after it. Deliberately NOT m_sliceEpochCounter, which every
// WriteWireBuffer bumps and which would defeat the memo on every glBufferSubData.
struct DeferredWireRelease {
VkBufferObject buffer;
Uint64 lastUseSerial = 0;
@@ -367,6 +384,11 @@ namespace MobileGL::MG_Backend::DirectVulkan {
Uint64 m_wireStoreCountPeak = 0;
Uint64 m_deferredWireBytesPeak = 0;
Uint64 m_wireDeferredSyncs = 0;
// See GetWireStoreDestroyEpoch and the DeferredWireRelease comment. Bumped on every
// path that destroys a wire store's VkBuffer, and never reset (not even by Shutdown),
// for m_sliceEpochCounter's reason: a memo taken before a re-initialize must not match
// a handle minted after it.
Uint64 m_wireStoreDestroyEpoch = 0;
#endif
// Size m_liveResources had just after the last sweep; the next sweep waits for it to double.
SizeT m_liveResourcesLastPruned = 0;