diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp index f13551f3..ae55c6b0 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp @@ -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(write.dstBinding) << 40) ^ (static_cast(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; } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h index 06e0c6f6..990abb10 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h @@ -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; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp index 4328b94a..5c1ab34e 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp @@ -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(dead)); const Uint64 destroyedCount = static_cast(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; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h index 5f481cc4..8992264b 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h @@ -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;