fix: ore buffer per-frame update race (#12976) cc34cea963

* add ore GM exposing the per-frame buffer update race

* fix Metal ore buffer update race with per-write versioning

* add ore_buffer_update_between_draws goldens for metal d3d11 and gl

* fix Vulkan ore buffer update race with per-write versioning

* fix D3D12 ore buffer update race with per-write versioning

* fix WebGPU ore buffer update race with per-write versioning

* add ore_buffer_update_between_draws goldens for vulkan d3d12 and webgpu

* address review notes and tighten comments for the ore buffer race fix

* fix wgpu partial-update shadow seed and harden ore buffer race edge cases

* add multi-frame ore buffer race regression unit test

* scope ore buffer race test to racy backends, fixing linux headless GL abort

* free manager-less GPUResources on release instead of leaking them

* harden ore buffer orphaning against gpu allocation failure on metal, vulkan and wgpu

* make d3d12 buffer orphaning retry on alloc failure, matching the other backends

* wgpu: don't cache a failed bind group creation and correct the shadow comment

* wgpu/vulkan: mark UBOs bound only after a successful resolve, and restore the wgpu bind group label

* fail makeBuffer cleanly on gpu allocation failure (metal, vulkan, wgpu)

* harden ore bind group OOM handling and orphan-failure diagnostics across vulkan/metal/wgpu

Co-authored-by: Luigi Rosso <luigi-rosso@users.noreply.github.com>

Author: luigi-rosso

.rive_head

@@ -1 +1 @@
-9f3eef86340a3ef53568116c197fd343a9e1d4e3
+cc34cea96329320072dcc92cc4d302db44369dbe

renderer/include/rive/renderer/ore/ore_context_d3d12.hpp

@@ -44,6 +44,12 @@ class ContextD3D12 : public Context
     void endFrame() override;
     void waitForGPU() override;
 
+    // Frame numbers from the host, used by BufferD3D12 to version backings:
+    // a backing bound in `currentFrameNumber` is reclaimable once
+    // `safeFrameNumber` reaches it.
+    uint64_t currentFrameNumber() const { return m_currentFrameNumber; }
+    uint64_t safeFrameNumber() const { return m_safeFrameNumber; }
+
     rcp<TextureView> wrapCanvasTexture(gpu::RenderCanvas* canvas) override;
     rcp<TextureView> wrapRiveTexture(gpu::Texture* gpuTex,
                                      uint32_t width,
@@ -58,13 +64,20 @@ class ContextD3D12 : public Context
     friend class RenderPassD3D12;
     friend class BindGroupD3D12;
     friend class TextureD3D12;
+    friend class BufferD3D12;
 
     ContextD3D12(rcp<rive::gpu::GPUResourceManager> manager) :
         Context(std::move(manager))
     {}
 
     // D3D12 implementation helpers — defined in ore_context_d3d12.cpp.
     rcp<Buffer> d3d12MakeBuffer(const BufferDesc& desc);
+    // Create + persistently map one UPLOAD-heap backing of exactly
+    // `alignedSize` bytes. Shared by buffer creation and orphan-on-bound.
+    bool d3d12AllocBufferBacking(
+        UINT64 alignedSize,
+        Microsoft::WRL::ComPtr<ID3D12Resource>& outResource,
+        void** outMapped);
     rcp<Texture> d3d12MakeTexture(const TextureDesc& desc);
     rcp<TextureView> d3d12MakeTextureView(const TextureViewDesc& desc);
     rcp<Sampler> d3d12MakeSampler(const SamplerDesc& desc);
@@ -109,6 +122,9 @@ class ContextD3D12 : public Context
     Microsoft::WRL::ComPtr<ID3D12DescriptorHeap> m_d3dGpuSamplerHeap;
     UINT m_d3dGpuSrvAllocated = 0;
     UINT m_d3dGpuSamplerAllocated = 0;
+    // Host frame numbers captured at beginFrame for backing reclamation.
+    uint64_t m_currentFrameNumber = 0;
+    uint64_t m_safeFrameNumber = 0;
     // Helpers called by RenderPass to allocate and resolve descriptor handles.
     UINT d3d12AllocGpuSrvSlots(UINT count);
     UINT d3d12AllocGpuSamplerSlots(UINT count);

renderer/include/rive/renderer/ore/ore_context_metal.hpp

@@ -8,6 +8,9 @@
 
 #import <Metal/Metal.h>
 
+#include <atomic>
+#include <memory>
+
 namespace rive::ore
 {
 
@@ -49,6 +52,16 @@ class ContextMetal : public Context
 
     ShaderTarget shaderTarget() const override { return ShaderTarget::msl; }
 
+    // Buffer versioning serials. currentSerial is the command buffer being
+    // recorded. completedSerial is the highest the GPU has finished, so a
+    // backing at or below it can be recycled. See ore_buffer_metal.mm.
+    uint64_t currentSerial() const { return m_currentSerial; }
+    uint64_t completedSerial() const
+    {
+        return m_completedSerial->load(std::memory_order_relaxed);
+    }
+    id<MTLDevice> device() const { return m_mtlDevice; }
+
     ContextMetal(const ContextMetal&) = delete;
     ContextMetal& operator=(const ContextMetal&) = delete;
 
@@ -79,6 +92,14 @@ class ContextMetal : public Context
     id<MTLCommandBuffer> m_mtlCommandBuffer = nil;
 
     std::vector<rcp<BindGroup>> m_deferredBindGroups;
+
+    // Serial of the command buffer being recorded, bumped each frame.
+    uint64_t m_currentSerial = 0;
+    // Highest serial the GPU has finished. Written on the completion handler
+    // thread, read on the recording thread, hence atomic. shared_ptr so the
+    // handler stays valid even if the context dies first.
+    std::shared_ptr<std::atomic<uint64_t>> m_completedSerial =
+        std::make_shared<std::atomic<uint64_t>>(0);
 };
 
 } // namespace rive::ore

renderer/include/rive/renderer/ore/ore_context_vulkan.hpp

@@ -158,6 +158,17 @@ class ContextVulkan : public Context
     // `vkDeferDestroy` if needed.
     VkDescriptorSetLayout m_vkEmptyDSL = VK_NULL_HANDLE;
     VkDescriptorSetLayout vkGetOrCreateEmptyDSL();
+
+    // Allocate a descriptor set from the active generation, rolling a fresh one
+    // on capacity. The returned pool ref keeps the set alive. Used by
+    // makeBindGroup and by BindGroupVulkan when a UBO orphans onto a new
+    // backing and needs a freshly written set.
+    struct DescriptorSetAllocation
+    {
+        VkDescriptorSet set = VK_NULL_HANDLE;
+        rcp<DescriptorPoolGeneration> pool;
+    };
+    DescriptorSetAllocation vkAllocateDescriptorSet(VkDescriptorSetLayout dsl);
     // Render pass cache — typically <10 entries, keyed on format+load/store
     // ops.
     std::vector<std::pair<VKRenderPassKey, VkRenderPass>> m_vkRenderPassCache;

renderer/include/rive/renderer/ore/ore_context_wgpu.hpp

@@ -68,6 +68,12 @@ class ContextWGPU : public Context
     // Use this to select between GLES and Vulkan GLSL shader source.
     bool isGLES() const { return m_wgpuBackend == WGPUBackend::OpenGLES; }
 
+    // Monotonic frame serial for buffer versioning. A backing bound this frame
+    // is not reused until a later frame, when its WriteBuffer is ordered after
+    // this frame's submit. The host does not thread frame numbers to the wgpu
+    // backend, so the context owns the serial.
+    uint64_t currentFrameSerial() const { return m_frameSerial; }
+
     ContextWGPU(const ContextWGPU&) = delete;
     ContextWGPU& operator=(const ContextWGPU&) = delete;
 
@@ -87,6 +93,7 @@ class ContextWGPU : public Context
     wgpu::Device m_wgpuDevice;
     wgpu::Queue m_wgpuQueue;
     wgpu::CommandEncoder m_wgpuCommandEncoder;
+    uint64_t m_frameSerial = 0;
 };
 
 } // namespace rive::ore

renderer/src/gpu_resource.cpp

@@ -11,9 +11,13 @@ GPUResource::~GPUResource() {}
 void GPUResource::onRefCntReachedZero() const
 {
     // GPUResourceManager will hold off on deleting "this" until its safe frame
-    // number has been reached.
+    // number has been reached. With no manager (the Metal Ore backend keeps
+    // native objects alive via ARC and command-buffer retention rather than a
+    // manager) there is no deferred reclaim, so delete now instead of leaking.
     if (m_manager)
         m_manager->onRenderingResourceReleased(const_cast<GPUResource*>(this));
+    else
+        delete const_cast<GPUResource*>(this);
 }
 
 GPUResourceManager::~GPUResourceManager()

renderer/src/ore/d3d12/ore_bind_group_d3d12.hpp

@@ -7,6 +7,7 @@
 namespace rive::ore
 {
 class ContextD3D12;
+class BufferD3D12;
 
 class BindGroupD3D12 : public LITE_RTTI_OVERRIDE(BindGroup, BindGroupD3D12)
 {
@@ -19,17 +20,21 @@ class BindGroupD3D12 : public LITE_RTTI_OVERRIDE(BindGroup, BindGroupD3D12)
 private:
     friend class ContextD3D12;
     friend class RenderPassD3D12;
-    // UBO (CBV) slots — GPU virtual addresses and sizes indexed by native slot.
-    D3D12_GPU_VIRTUAL_ADDRESS m_d3dUBOAddresses[8] = {};
+    // UBO slots. Live buffer back-refs, offsets and sizes indexed by native
+    // slot. The GPU-VA is resolved from the buffer at apply time so a post-bind
+    // update that orphaned the backing is seen. Buffers kept alive by
+    // m_retainedBuffers.
+    BufferD3D12* m_d3dUBOBuffers[8] = {};
+    uint32_t m_d3dUBOOffsets[8] = {};
     uint32_t m_d3dUBOSizes[8] = {};
     uint8_t m_d3dUBOSlotMask = 0;
     uint8_t m_d3dUBODynamicOffsetMask = 0;
-    // SRV (texture) slots — CPU descriptor handles and texture back-refs.
+    // SRV texture slots. CPU descriptor handles and texture back-refs.
     D3D12_CPU_DESCRIPTOR_HANDLE m_d3dSrvHandles[8] = {};
     Texture* m_d3dSrvTextures[8] =
-        {}; // Weak refs; kept alive by m_retainedViews.
+        {}; // Weak refs kept alive by m_retainedViews
     uint8_t m_d3dSrvSlotMask = 0;
-    // Sampler slots — CPU descriptor handles.
+    // Sampler slots. CPU descriptor handles.
     D3D12_CPU_DESCRIPTOR_HANDLE m_d3dSamplerHandles[8] = {};
     uint8_t m_d3dSamplerSlotMask = 0;
 };

renderer/src/ore/d3d12/ore_buffer_d3d12.cpp

@@ -13,18 +13,81 @@
 namespace rive::ore
 {
 
-// --- Public method definitions (D3D12-only builds) ---
-// When both D3D11 and D3D12 are compiled, the combined
-// ore_context_d3d11_d3d12.cpp file provides these methods with dispatch.
+// Guarded by ORE_BACKEND_D3D12. The Windows build compiles both the d3d11 and
+// d3d12 dirs with both backend macros defined; BufferD3D11 and BufferD3D12 are
+// distinct classes under their own guards and the context factory selects the
+// concrete type at runtime, so there is no shared dispatch file.
 #if defined(ORE_BACKEND_D3D12)
 
+void BufferD3D12::markBound()
+{
+    m_currentSerial = m_ctx->currentFrameNumber();
+    m_boundSinceUpdate = true;
+}
+
+bool BufferD3D12::acquireFreshBacking(uint32_t writeOffset, uint32_t writeSize)
+{
+    // Retire the current backing since an in flight bind may still read it.
+    void* oldMapped = m_d3dMappedPtr;
+    m_retired.push_back(
+        {std::move(m_d3dBuffer), m_d3dMappedPtr, m_currentSerial});
+
+    // Reuse a retired backing the GPU has finished with, else allocate one.
+    // Require the bind to predate the current frame, not just safeFrameNumber,
+    // so a host that ever reports safe == current cannot hand back an in flight
+    // backing.
+    const uint64_t safe = m_ctx->safeFrameNumber();
+    const uint64_t current = m_ctx->currentFrameNumber();
+    for (auto it = m_retired.begin(); it != m_retired.end(); ++it)
+    {
+        if (it->serial <= safe && it->serial < current)
+        {
+            m_d3dBuffer = std::move(it->resource);
+            m_d3dMappedPtr = it->mapped;
+            m_retired.erase(it);
+            break;
+        }
+    }
+    if (m_d3dBuffer == nullptr &&
+        !m_ctx->d3d12AllocBufferBacking(m_allocatedSize,
+                                        m_d3dBuffer,
+                                        &m_d3dMappedPtr))
+    {
+        // Allocation failed. Keep the backing we just retired so we do not
+        // write through a null map. This reraces this one update but the data
+        // is already there, and it avoids a crash.
+        m_ctx->setLastError("ore: D3D12 buffer backing allocation failed; "
+                            "reusing in flight backing for this update");
+        Backing& old = m_retired.back();
+        m_d3dBuffer = std::move(old.resource);
+        m_d3dMappedPtr = old.mapped;
+        m_retired.pop_back();
+        return false; // Kept the current backing; retry orphaning next update.
+    }
+
+    m_currentSerial = 0;
+    // Carry contents forward so a partial update keeps untouched bytes. Skip
+    // when this update covers the whole buffer, and skip the self copy when the
+    // reused backing is the one we just retired.
+    if (!(writeOffset == 0 && writeSize == m_size) &&
+        m_d3dMappedPtr != oldMapped)
+        memcpy(m_d3dMappedPtr, oldMapped, m_size);
+    return true;
+}
+
 void BufferD3D12::update(const void* data, uint32_t size, uint32_t offset)
 {
     assert(offset + size <= m_size);
     assert(m_d3dBuffer != nullptr);
     assert(m_d3dMappedPtr != nullptr);
 
-    // UPLOAD heap buffer is persistently mapped — write directly.
+    if (m_boundSinceUpdate)
+    {
+        // On allocation failure keep the current backing and retry next update.
+        if (acquireFreshBacking(offset, size))
+            m_boundSinceUpdate = false;
+    }
+    // UPLOAD heap buffer is persistently mapped so write directly.
     memcpy(static_cast<uint8_t*>(m_d3dMappedPtr) + offset, data, size);
 }
 

renderer/src/ore/d3d12/ore_buffer_d3d12.hpp

@@ -2,11 +2,15 @@
 #include "rive/renderer/ore/ore_buffer.hpp"
 #include <d3d12.h>
 #include <wrl/client.h>
+#include <vector>
 
 namespace rive::ore
 {
 class ContextD3D12;
 
+// A write after a bind orphans onto a fresh backing instead of racing the GPU
+// still reading the bound one. Bindings resolve the live backing at apply time.
+// Immutable or never rebound buffers stay at a single backing.
 class BufferD3D12 : public LITE_RTTI_OVERRIDE(Buffer, BufferD3D12)
 {
 public:
@@ -18,13 +22,42 @@ class BufferD3D12 : public LITE_RTTI_OVERRIDE(Buffer, BufferD3D12)
     ~BufferD3D12() override = default; // ComPtr released automatically
     void update(const void* data, uint32_t size, uint32_t offset) override;
 
+    // GPU-VA of the live backing. Call at apply time, not creation time.
+    D3D12_GPU_VIRTUAL_ADDRESS currentGpuVA() const
+    {
+        return m_d3dBuffer->GetGPUVirtualAddress();
+    }
+
+    // Stamp the live backing with this frame so a later update() orphans
+    // instead of overwriting memory the GPU may still read.
+    void markBound();
+
 private:
     friend class ContextD3D12;
     friend class RenderPassD3D12;
     friend class TextureD3D12; // for staging buffer access during texture
                                // uploads
-    // UPLOAD heap — persistently mapped; GPU reads directly.
+
+    // Move to a fresh backing, copying current contents so a partial update
+    // keeps untouched bytes. The pending write's range lets a full-buffer
+    // update skip the copy. Returns false if a fresh backing could not be
+    // allocated, in which case the current backing is kept.
+    bool acquireFreshBacking(uint32_t writeOffset, uint32_t writeSize);
+
+    // Live backing in an UPLOAD heap, persistently mapped so the GPU reads it.
     Microsoft::WRL::ComPtr<ID3D12Resource> m_d3dBuffer;
     void* m_d3dMappedPtr = nullptr;
+
+    struct Backing
+    {
+        Microsoft::WRL::ComPtr<ID3D12Resource> resource;
+        void* mapped = nullptr;
+        uint64_t serial = 0; // frame it was last bound in
+    };
+    std::vector<Backing> m_retired; // rotated-out backings awaiting GPU retire
+    uint64_t m_currentSerial = 0;   // frame the live backing was last bound in
+    bool m_boundSinceUpdate = false;
+    UINT64 m_allocatedSize = 0; // aligned size for fresh backings
+    ContextD3D12* m_ctx = nullptr;
 };
 } // namespace rive::ore