[Fix] (MG_Impl, MG_State, DirectGLES): give the vertex-elements CSO a backend-neutral death path - the client minted every VAO's slot and only Espryt ever freed one, so under Magma each VAO leaked a slot and a 1.3 KB applier record for the life of the process

MobileGL/MG_Backend/DirectGLES/Managers.cpp

@@ -215,6 +215,18 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 PrgramImpl::g_backendProgramObjects.DestroyByLifetimeId(lifetimeId);
                 break;
             case MG_Pipe::MGPipeKind::VertexElementsCso:
+                // P3a C-1: this is now the SECOND path, not the only one. The client speaks the
+                // whole death itself (MGPipeEmitVertexElementsDestroyAndFree: delete the
+                // applier record, raise this notice, free the slot), because the slot is minted
+                // client-side on every backend and a backend that installs no death ops - which
+                // Magma deliberately does not - otherwise leaked the slot and the record per
+                // VAO for the life of the process. What is left here is the one thing only this
+                // side can do: drop the driver VAO the twin owns. It is raised while the handle
+                // still resolves, so OnFrontendObjectDestroyed's shared free (which the other
+                // five kinds still depend on) is simply the one that gets there first; the
+                // client's own Free right after it is then a no-op, because Free refuses a slot
+                // that is no longer live at that generation and the Gen bump rides the next
+                // handout rather than the free. Double release, no corruption, no abort.
                 VertexArrayImpl::g_backendVertexArrayObjects.DestroyByLifetimeId(lifetimeId);
                 break;
             default:

MobileGL/MG_Impl/Pipe/PipeFill.cpp

@@ -14,6 +14,9 @@
 // (CMakeLists.txt appends it to SOURCE_FILES there).
 #include <MG_State/GLState/Core.h>
 #include <MG_State/GLState/BufferState/BufferState.h>
+// C-1: the vertex-elements CSO's death path raises the backend notice from here, between the
+// applier's delete and the slot free, so that the whole order lives in one place.
+#include <MG_State/GLState/StateObjectDeathNotice.h>
 #include <MG_Backend/MGPipe/PipeInputs.h>
 #include <MG_Impl/Pipe/CsoCache.h>
 #include <MG_Impl/Pipe/PipeFill.h>
@@ -762,6 +765,57 @@ namespace MobileGL::MG_Pipe {
         return published;
     }
 
+    Bool MGPipeEmitVertexElementsDestroyAndFree(Uint64 lifetimeId) {
+        // C-1. THE SAME SHAPE AS MGPipeEmitResourceDestroyAndFree ABOVE, and for the same
+        // reason: whatever mints a handle owns the death of that handle, and the mint for this
+        // kind is MGPipeVertexInputEmitter::EmitVertexElements - i.e. the client, on every
+        // backend. Espryt's StateObjectDeathOps notice used to be the only free, so under a
+        // backend that installs none the slot and the applier's record leaked per VAO, for
+        // ever. It is now the SECOND, redundant path (Managers.cpp's
+        // OnFrontendStateObjectDestroyed) and it must stay idempotent, which it is: the
+        // notice resolves through the same lifetimeId -> slot map this function frees, and
+        // MGPipeSlotAllocator::Free refuses a slot that is not live at that generation.
+        // May be the null handle: no slot is minted for a VAO that no draw ever validated with
+        // and no backend twin table ever looked up. That case still raises the notice below -
+        // see there.
+        const MGPipeHandle handle =
+            MGPipeSlots().FindByLifetimeId(MGPipeKind::VertexElementsCso, lifetimeId);
+
+        // ASKED, NOT ASSUMED. A slot is not evidence of a record: DirectGLES mints one from
+        // BackendSlotTable::GetOrCreate at every VAO sync, whether or not bit 8 asked this
+        // client to emit a create - the shipping 0x7f A/B control arm is exactly that
+        // configuration. delete_vertex_elements on a handle the applier has no record for is a
+        // refusal, and the refusal asserts (PipeApply.cpp's ResolveVertexElements), i.e. it
+        // stops a verify build.
+        MGPipeVertexInputEmitter& emitter = MGPipeVertexInputEmitterInstance();
+        const Bool published = emitter.RecordIsPublished(handle);
+        if (published) {
+            MGPHandleOnly only{};
+            only.Handle = handle;
+            only.Kind = static_cast<Uint32>(MGPipeKind::VertexElementsCso);
+            MGPipeApplyDeleteVertexElements(only);
+            emitter.NoteRecordDestroyed(handle);
+        }
+
+        // THE ORDER IS D-L's, WITH THE BACKEND NOTICE IN THE MIDDLE, and each of the three
+        // positions is load-bearing:
+        //   * the applier's record is dropped FIRST, while nothing else can have re-handed the
+        //     slot out, so a recycled slot cannot inherit a field;
+        //   * the death notice is raised SECOND, because it resolves the handle through the
+        //     allocator and a backend told after the Free below could no longer find its twin
+        //     - which would move the leak from the client to the driver VAO. It is raised
+        //     UNCONDITIONALLY, exactly as ~VertexArrayObject raised it before C-1: whether a
+        //     slot exists is this client's business, and a consumer that records notices (the
+        //     P2 e2 gate does) must not stop seeing this class announce itself;
+        //   * the slot goes back LAST. Espryt's notice frees it too; that Free and this one
+        //     are the same call on the same handle and the second is a no-op, because Free
+        //     bumps no generation (the bump rides the next handout) and refuses a slot that is
+        //     no longer live at this generation.
+        MG_State::GLState::NotifyStateObjectDestroyed(MGPipeKind::VertexElementsCso, lifetimeId);
+        if (!MGPipeHandleIsNull(handle)) MGPipeSlots().Free(MGPipeKind::VertexElementsCso, handle);
+        return published;
+    }
+
     void MGPipeSetPoisonOmission(const char* verb, const char* field) {
         if (verb == nullptr || field == nullptr) {
             g_omission = PoisonOmission{};

MobileGL/MG_Impl/Pipe/VertexInputEmit.h

@@ -288,12 +288,61 @@ namespace MobileGL::MG_Pipe {
         Uint64 VertexBufferSetCount() const { return m_bufferSets; }
         Uint64 IndexBufferSetCount() const { return m_indexSets; }
 
+        // ---- C-1: "does the applier hold a record for exactly this handle?" ----
+        //
+        // The CSO's death path (MGPipeEmitVertexElementsDestroyAndFree) needs that answer and
+        // MUST NOT GUESS IT FROM THE SLOT. A VertexElementsCso slot can exist with no record
+        // behind it, because a backend that keys its twins on the handle mints the slot itself
+        // (DirectGLES' BackendSlotTable::GetOrCreate -> MGPipeSlots().Acquire) whether or not
+        // bit 8 ever asked this client to emit anything - which is exactly what a
+        // MOBILEGL_PIPE_PUSH=0x7f lane runs. delete_vertex_elements on such a handle is a
+        // REFUSED call, and the applier's resolver asserts on a refusal
+        // (PipeApply.cpp's ResolveVertexElements), i.e. a stop in a verify build.
+        //
+        // Kept OUT of Reset(), unlike the create/bind latch beside it, and for the mirror
+        // image of Reset()'s own reason: "a fresh context is a fresh server" is true of the
+        // per-context half of this table, and object RECORDS are precisely what
+        // MGPipeApplierReset does not clear (PipeApply.h's two halves). This half tracks those
+        // records, so it lives exactly as long as they do.
+        Bool RecordIsPublished(MGPipeHandle handle) const {
+            if (MGPipeHandleIsNull(handle)) return false;
+            const SizeT slot = handle.Slot;
+            if (slot >= m_latch.size()) return false;
+            const Latch& latch = m_latch[slot];
+            return latch.RecordLive && latch.RecordGen == handle.Gen;
+        }
+
+        // The record named by `handle` is gone from the applier. Also drops the bound-handle
+        // memo when it named it, so the client's idea of BoundVertexElements and the applier's
+        // (which MGPipeApplyDeleteVertexElements just cleared for the same handle) stay in
+        // step rather than diverging until the next bind happens to correct it.
+        void NoteRecordDestroyed(MGPipeHandle handle) {
+            if (MGPipeHandleIsNull(handle)) return;
+            const SizeT slot = handle.Slot;
+            if (slot < m_latch.size() && m_latch[slot].RecordGen == handle.Gen) {
+                m_latch[slot] = Latch{};
+            }
+            if (m_boundHandle == handle) {
+                m_boundHandle = kMGPipeNullHandle;
+                m_boundLifetimeId = 0;
+            }
+        }
+
         // A fresh context is a fresh server: the applier's records are gone, so every latch
         // this emitter holds describes objects the server no longer has. Called from the
         // validate point's FreshlyPrimed arm beside MGPipeApplierReset and the suppressor's
         // InvalidateAll, for the same reason they are.
+        //
+        // The PER-CONTEXT half only - see RecordIsPublished above for why RecordLive/RecordGen
+        // survive. Re-creating a configuration the applier already holds is a bounded
+        // over-fire (MGPipeApplyCreateVertexElements starts the record over); forgetting that
+        // it holds one at all would leak the record and its slot at the object's death.
         void Reset() {
-            m_latch.clear();
+            for (Latch& latch : m_latch) {
+                latch.Published = false;
+                latch.Gen = 0;
+                latch.ConfigVersion = 0;
+            }
             m_boundHandle = kMGPipeNullHandle;
             m_boundLifetimeId = 0;
         }
@@ -302,9 +351,16 @@ namespace MobileGL::MG_Pipe {
 
     private:
         struct Latch {
+            // The PER-CONTEXT half: "has this emitter told THIS server about this handle's
+            // configuration". Cleared by Reset() at every make-current.
             Bool Published = false;
             Uint32 Gen = 0;
             Uint32 ConfigVersion = 0;
+            // The RECORD half: "does the applier hold a create_vertex_elements record at this
+            // slot, for this generation". Lives as long as the record does - see
+            // RecordIsPublished.
+            Bool RecordLive = false;
+            Uint32 RecordGen = 0;
         };
 
         static MGPHandleOnly HandleOnly(MGPipeHandle handle) {
@@ -345,6 +401,10 @@ namespace MobileGL::MG_Pipe {
             latch.Published = true;
             latch.Gen = handle.Gen;
             latch.ConfigVersion = configVersion;
+            // THE ONE PRODUCER of the record half: a create that reached the applier is the
+            // only thing that makes delete_vertex_elements a legal call for this handle.
+            latch.RecordLive = true;
+            latch.RecordGen = handle.Gen;
             return sizeof(MGPVertexElements) + kAttribBytes + kBindingBytes;
         }
 

MobileGL/MG_Pipe/PipeMutation.h

@@ -124,6 +124,22 @@ namespace MobileGL::MG_Pipe {
     // re-handed-out, or a backend object nobody releases.
     Bool MGPipeEmitResourceDestroyAndFree(MG_State::GLState::BufferObject& buffer);
 
+    // THE VERTEX-ELEMENTS CSO's DEATH, and it is BACKEND-NEUTRAL - which is the whole point.
+    // Before this, the only thing that ever returned a VertexElementsCso slot was DirectGLES'
+    // StateObjectDeathOps table; under any backend that installs none - DirectVulkan/Magma,
+    // which keeps its own age-reclaimed identity table on purpose - every VAO ever created
+    // held its slot and its ~1.3 KB applier record for the life of the process, on the shipped
+    // 0x1ff mask, and past 65536 slots every create_vertex_elements became a permanent
+    // Fatal{ProtocolCorruption}. The client mints the slot, so the client is where the death
+    // has to be spoken from.
+    //
+    // Takes the lifetime id and not the object for StateObjectDeathNotice.h's reason: the last
+    // SharedPtr has already dropped by the time this runs, and the lifetime id is what the
+    // slot allocator resolves the handle from. Returns whether delete_vertex_elements went
+    // out, i.e. whether the applier actually held a record - see the definition for why that
+    // is asked rather than assumed.
+    Bool MGPipeEmitVertexElementsDestroyAndFree(Uint64 lifetimeId);
+
     void MGPipeEmitResourceCreate(MG_State::GLState::BufferObject& buffer);
     void MGPipeEmitResourceRespecify(MG_State::GLState::BufferObject& buffer);
     void MGPipeEmitResourceSubData(MG_State::GLState::BufferObject& buffer, SizeT offset, SizeT size);

MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.cpp

@@ -8,8 +8,6 @@
 
 #include "VertexArrayObject.h"
 
-#include <MG_State/GLState/StateObjectDeathNotice.h>
-
 #include <atomic>
 #include <MG_Pipe/PipeMutation.h>
 
@@ -42,15 +40,24 @@ namespace MobileGL::MG_State::GLState {
 
 #if MOBILEGL_PIPE_PUSH
     VertexArrayObject::~VertexArrayObject() {
-        // P2 step e2: ANNOUNCE the death instead of leaving the backend to discover it in a
+        // P2 step e2 / P3a C-1: ANNOUNCE the death instead of leaving the backend to discover
-        // garbage sweep. This is the last SharedPtr to this object dropping - not the
+        // it in a garbage sweep, and RETURN THE CLIENT'S OWN SLOT while doing it. This is the
-        // glDelete* that only marks the name and leaves a still-bound object very much
+        // last SharedPtr to this object dropping - not the glDelete* that only marks the name
-        // alive - so it is the exact moment the backend's twin, and the driver storage
+        // and leaves a still-bound object very much alive - so it is the exact moment the
-        // that twin owns, stop being reachable. The notice carries the lifetime id
+        // backend's twin, the driver storage that twin owns, the applier's vertex-elements
-        // because the object no longer exists to be passed, and because the lifetime id
+        // record and the {slot, gen} that names all three stop being reachable.
-        // is what the client slot allocator resolves the handle from. No-op unless a
+        //
-        // backend registered the ops (a pull build declares none at all).
+        // ALL FOUR OF THOSE GO THROUGH ONE HELPER, and it is the client's rather than a
-        NotifyStateObjectDestroyed(MG_Pipe::MGPipeKind::VertexElementsCso, m_lifetimeId);
+        // backend's: the handle is minted by MGPipeVertexInputEmitter on every backend, so a
+        // death path that only exists inside a backend's death-ops table is no path at all
+        // under a backend that installs none - which is what DirectVulkan/Magma does on
+        // purpose, and what made every VAO leak a slot and a ~1.3 KB applier record for the
+        // life of the process on the shipped mask. The helper emits delete_vertex_elements,
+        // raises the notice (a no-op unless a backend registered the ops) and frees the slot,
+        // in that fixed order; MG_Pipe/PipeMutation.h and its definition say why each position
+        // is where it is. The buffer's death has exactly this shape one file over
+        // (BufferObject.cpp -> MGPipeEmitResourceDestroyAndFree).
+        MG_Pipe::MGPipeEmitVertexElementsDestroyAndFree(m_lifetimeId);
     }
 #endif