diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index f8247e9b..81afc524 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -52,6 +52,7 @@ add_executable(MobileGLIntegrationTest Main.cpp Harness/HeadlessGL.cpp Harness/BackendCapsPeek.cpp + Harness/PipeSlotPeek.cpp Scenarios/OrientationScenario.cpp Scenarios/CrossFrameBufferScenario.cpp Scenarios/ResidentIndexScenario.cpp diff --git a/MobileGL/MG_IntegrationTest/Harness/PipeSlotPeek.cpp b/MobileGL/MG_IntegrationTest/Harness/PipeSlotPeek.cpp new file mode 100644 index 00000000..8e810a5f --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Harness/PipeSlotPeek.cpp @@ -0,0 +1,47 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Harness/PipeSlotPeek.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +#include "PipeSlotPeek.h" + +#if !defined(__ANDROID__) +#include +#if MOBILEGL_PIPE_PUSH +#include +#define MGITEST_PIPE_SLOT_PEEK_LIVE 1 +#endif +#endif + +namespace MGITest { + +#if defined(MGITEST_PIPE_SLOT_PEEK_LIVE) + namespace { + MobileGL::MG_Pipe::MGPipeKind Translate(PipeSlotKind kind) { + switch (kind) { + case PipeSlotKind::Buffer: return MobileGL::MG_Pipe::MGPipeKind::Buffer; + default: return MobileGL::MG_Pipe::MGPipeKind::VertexElementsCso; + } + } + } // namespace + + bool PeekPipeSlotLiveCount(PipeSlotKind kind, unsigned* outLive) { + if (outLive == nullptr) return false; + *outLive = static_cast(MobileGL::MG_Pipe::MGPipeSlots().LiveCount(Translate(kind))); + return true; + } + + bool PeekPipeSlotHighWater(PipeSlotKind kind, unsigned* outHighWater) { + if (outHighWater == nullptr) return false; + *outHighWater = static_cast(MobileGL::MG_Pipe::MGPipeSlots().HighWater(Translate(kind))); + return true; + } +#else + bool PeekPipeSlotLiveCount(PipeSlotKind, unsigned*) { return false; } + bool PeekPipeSlotHighWater(PipeSlotKind, unsigned*) { return false; } +#endif + +} // namespace MGITest diff --git a/MobileGL/MG_IntegrationTest/Harness/PipeSlotPeek.h b/MobileGL/MG_IntegrationTest/Harness/PipeSlotPeek.h new file mode 100644 index 00000000..55c95c53 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Harness/PipeSlotPeek.h @@ -0,0 +1,44 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Harness/PipeSlotPeek.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// The CLIENT slot allocator's occupancy, read from a scenario. +// +// It exists for one assertion, P3a's C-1: a frontend object that dies must return its +// MGPipeHandle slot WHATEVER BACKEND IS RUNNING. That question has no answer in the GL API - +// the leak it rules out is entirely inside the library, and it is invisible in pixels, in GL +// names and in `glGetError` - so the only honest observable is the allocator's own live count +// and high-water mark. Reading them is what makes the case fail on the backend it actually +// failed on (DirectVulkan, which installs no StateObjectDeathOps) rather than only on the one +// where a backend-owned free happened to exist. +// +// A separate translation unit for BackendCapsPeek.h's reason, verbatim: the scenario sources +// include the GL headers with prototypes and MobileGL's umbrella header is not meant to meet +// them in one file. + +#pragma once + +namespace MGITest { + + // Which client-side object kind to ask about. Mirrors MG_Pipe::MGPipeKind for exactly the + // kinds a scenario has a reason to count, so that the enum does not travel through this + // header and the GL headers together. + enum class PipeSlotKind { + Buffer, + VertexElementsCso, + }; + + // Live slots of this kind right now, and one past the highest slot ever handed out. + // Both return false, touching nothing, where the allocator is out of reach: in a PULL + // build there is no allocator at all (it is `#if MOBILEGL_PIPE_PUSH`), and on Android this + // module links the shipping libMobileGL.so built -fvisibility=hidden, so no internal symbol + // resolves. A caller that gets false must SKIP rather than pass - "could not look" is not + // "did not leak". + bool PeekPipeSlotLiveCount(PipeSlotKind kind, unsigned* outLive); + bool PeekPipeSlotHighWater(PipeSlotKind kind, unsigned* outHighWater); + +} // namespace MGITest diff --git a/MobileGL/MG_IntegrationTest/Scenarios/HandleRecycleScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/HandleRecycleScenario.cpp index e0d48765..c82724e9 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/HandleRecycleScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/HandleRecycleScenario.cpp @@ -120,6 +120,7 @@ #include #include "../Harness/HeadlessGL.h" +#include "../Harness/PipeSlotPeek.h" #include "../Harness/ScenarioFixture.h" #ifdef GLAPI @@ -992,5 +993,104 @@ void main() { oColor = texture(uTex, vUv); } glDeleteTextures(2, cleanup); } + // ------------------------------------------------------------------------------------ + // P3a C-1: the recycle has to give the SLOT back, not only refuse to alias. + // + // Everything above asks "did the replacement inherit the dead object's state". This asks + // the other half of the same identity contract, which no case in this file could see: + // when the dead object is not replaced at all, does the client hand its {slot, gen} + // back? Until C-1 the answer under DirectVulkan was NO. The mint is the client's + // (MGPipeVertexInputEmitter::EmitVertexElements acquires a VertexElementsCso slot at + // every validate point with a VAO bound) and the only free in the tree was Espryt's + // StateObjectDeathOps consumer - so under Magma, which installs none deliberately + // (MagmaPipeArms.h: "an allocator here would grow by one SlotState plus one map node + // per object EVER created, for the life of the process, on a platform with an LMK"), + // every VAO ever created held its slot and its ~1.3 KB applier record until the process + // died, on the shipped 0x1ff mask, until create_vertex_elements began tripping + // Fatal{ProtocolCorruption} permanently at kMGPipeMaxVertexElementsSlots. + // + // THE OBSERVABLE IS THE ALLOCATOR, not pixels: this leak produces correct pictures the + // whole way to the fatal, which is exactly why the four cases above ran green over it. + // It runs on BOTH backends' Handles lanes; DirectVulkan is where it was red. + TEST_F(HandleRecycleScenario, DestroyedVertexArraysReturnTheirVertexElementsSlots) { + if (!Ready()) return; + SkipUnlessTheArmIsAssertableHere(); + if (IsSkipped()) return; + if (m_arm != Arm::Handles) { + GTEST_SKIP() << "the client mints a VertexElementsCso slot only when the vertex-input " + "subsystem is on, and only the Handles arm pins it (0x1ff). The Legacy " + "and AbaControl lanes run MOBILEGL_PIPE_PUSH=0, where there is no " + "allocator to leak from."; + } + + unsigned liveBefore = 0; + unsigned highWaterBefore = 0; + if (!MGITest::PeekPipeSlotLiveCount(MGITest::PipeSlotKind::VertexElementsCso, &liveBefore) || + !MGITest::PeekPipeSlotHighWater(MGITest::PipeSlotKind::VertexElementsCso, + &highWaterBefore)) { + GTEST_SKIP() << "the client slot allocator is out of reach from this module (a pull " + "build has none, and the Android link resolves no internal symbol), so " + "'could not look' would be reported as 'did not leak'"; + } + + // One shared VBO: the case is about the VAOs, and a per-round buffer would churn the + // Buffer kind's slots alongside them and blur which allocator answered. + const GLuint buffer = MakeQuadBuffer(0.0f, 1.0f, 0.0f); + + // Each round creates a vertex array, DRAWS with it - which is what mints the slot and + // publishes the applier's record; a VAO that never reaches a validate point has + // neither - unbinds it and deletes it. glGenVertexArrays hands the same name back + // every time, exactly as a chunk renderer's does, so a death path that keyed on the + // GL NAME rather than on the lifetime id would look correct here too - which is why + // the assertion is on the allocator and not on the name. + constexpr int kChurn = 48; + unsigned peakLive = liveBefore; + for (int round = 0; round < kChurn; ++round) { + GLuint vao = 0; + glGenVertexArrays(1, &vao); + ConfigureQuadVao(vao, buffer); + const Image image = DrawQuadAndRead(vao); + if (round == 0) { + // One picture check, so a green here cannot mean "the draws never happened + // and therefore nothing was ever minted". + ExpectWholeViewportIs(image, "green", "the churn's first draw"); + } + unsigned live = 0; + ASSERT_TRUE(MGITest::PeekPipeSlotLiveCount(MGITest::PipeSlotKind::VertexElementsCso, + &live)); + if (live > peakLive) peakLive = live; + glBindVertexArray(0); + glDeleteVertexArrays(1, &vao); + } + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "the churn left a GL error behind"; + + unsigned liveAfter = 0; + unsigned highWaterAfter = 0; + ASSERT_TRUE(MGITest::PeekPipeSlotLiveCount(MGITest::PipeSlotKind::VertexElementsCso, + &liveAfter)); + ASSERT_TRUE(MGITest::PeekPipeSlotHighWater(MGITest::PipeSlotKind::VertexElementsCso, + &highWaterAfter)); + std::cout << "[ HandleRecycle ] backend=" << Gl().BackendName() << " VertexElementsCso live " + << liveBefore << " -> " << liveAfter << " (peak " << peakLive << "), high water " + << highWaterBefore << " -> " << highWaterAfter << " over " << kChurn + << " create/draw/destroy rounds" << std::endl; + + EXPECT_EQ(liveAfter, liveBefore) + << kChurn << " vertex arrays were created, drawn with and destroyed and " << (liveAfter - liveBefore) + << " VertexElementsCso slots never came back. Each one holds a SlotState, a " + "lifetime-id map node and the applier's ~1.3 KB record for the life of the " + "process, and past kMGPipeMaxVertexElementsSlots every create_vertex_elements " + "trips Fatal{ProtocolCorruption} for good. Backend " + << Gl().BackendName(); + EXPECT_LE(highWaterAfter - highWaterBefore, 4u) + << "the CSO slot space grew with the churn instead of recycling one slot; the " + "frees are not reaching the allocator's free list"; + EXPECT_LE(peakLive - liveBefore, 2u) + << "more than one churned vertex array was live at the allocator at once, so the " + "deaths are arriving late rather than at the destructor"; + + glDeleteBuffers(1, &buffer); + } + } // namespace } // namespace MGITest diff --git a/MobileGL/MG_Test/Pipe/VertexInputEmitTest.cpp b/MobileGL/MG_Test/Pipe/VertexInputEmitTest.cpp index 43014f27..b826465a 100644 --- a/MobileGL/MG_Test/Pipe/VertexInputEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/VertexInputEmitTest.cpp @@ -47,12 +47,16 @@ #include "Includes.h" #include #if MOBILEGL_PIPE_PUSH +#include + #include #include #include #include #include +#include #include +#include #endif using namespace MobileGL; @@ -126,7 +130,9 @@ namespace { X(VertexInputEmit, ABaseInstanceChangeAloneStillEmitsTheVertexBufferSet) \ X(VertexInputEmit, AnUnchangedSetWithAnUnchangedBaseInstanceEmitsNothing) \ X(VertexInputEmit, RebindingTheSameVaoEmitsABindAndNoCreate) \ - X(VertexInputEmit, PingPongingBetweenTwoVaosNeverRecreatesEither) + X(VertexInputEmit, PingPongingBetweenTwoVaosNeverRecreatesEither) \ + X(VertexInputEmit, DestroyedVertexArraysReturnTheirCsoSlotsAndRecords) \ + X(VertexInputEmit, ADoubleReleaseOfAVertexElementsSlotIsHarmless) #define MGL_DECLARE_PULL_SKIP(Suite, Name) \ TEST(Suite, Name) { GTEST_SKIP() << "compiled only under MOBILEGL_PIPE_PUSH"; } @@ -500,6 +506,110 @@ namespace { EXPECT_GT(Emitter().EmitVertexElements(Ctx()), 0u) << "the default VAO is a VAO and has a handle"; EXPECT_EQ(Emitter().CreateCount(), 3u); } + + // ============================ C-1: the death path ============================ + // + // THE LEAK THIS RULES OUT, and why it is a client case rather than a backend one. Every + // validate point with a VAO bound calls MGPipeSlots().Acquire(VertexElementsCso, ...) and + // MGPipeApplyCreateVertexElements, and the applier's record is an Array<...,32> pair - + // about 1.3 KB per slot. Until C-1 the ONLY thing that ever returned one was DirectGLES' + // StateObjectDeathOps table, so under any backend that installs none - which is what + // DirectVulkan/Magma deliberately does, MagmaPipeArms.h says why - a VAO's slot and its + // record were held for the life of the process, on the shipped 0x1ff mask. Sodium and + // Create churn a VAO per chunk section; 10 k of them is 13 MB of records plus 10 k + // SlotState entries plus 10 k map nodes, monotonic, on a platform with an LMK, and past + // kMGPipeMaxVertexElementsSlots every create_vertex_elements becomes a permanent + // Fatal{ProtocolCorruption}. + // + // This binary installs NO StateObjectDeathOps at all, which is exactly the shape of the + // backend the leak was invisible under. Before C-1 the two EXPECTs on LiveCount below read + // `live + kChurn` and `HighWater` grew by kChurn; after it, both come back. + TEST(VertexInputEmit, DestroyedVertexArraysReturnTheirCsoSlotsAndRecords) { + EmitterScope scope; + ASSERT_EQ(MG_State::GLState::GetStateObjectDeathOps(), nullptr) + << "this case is the NO-death-ops backend; with a consumer installed it would be " + "measuring Espryt's free instead of the client's"; + + auto& slots = MGPipeSlots(); + const Uint32 liveBefore = slots.LiveCount(MGPipeKind::VertexElementsCso); + const Uint32 highWaterBefore = slots.HighWater(MGPipeKind::VertexElementsCso); + + // Every round is one VAO, configured, drawn with (which is what mints the slot AND + // publishes the record), then deleted. The names are reused, exactly as a chunk + // renderer's are - MG_State hands out a fresh lifetime id per object anyway, so a + // recycled NAME must not be what returns the slot. + constexpr int kChurn = 64; + Uint32 peakLive = 0; + for (int round = 0; round < kChurn; ++round) { + // Through the name allocator, so MarkVertexArrayForDeletion recognises the name and + // actually drops the slot's reference - and so the recycled NAME is part of the + // shape, exactly as a chunk renderer's is. The lifetime id is fresh every round + // whatever the name does, which is what the free has to key on. + Vector names; + Ctx().GenVertexArrayNames(1, names); + ASSERT_EQ(names.size(), 1u) << "round " << round; + const Uint name = names[0]; + Ctx().CreateVertexArrayObject(name); + Ctx().BindVertexArray(name); + const SharedPtr vao = Ctx().GetBoundVertexArray(); + ASSERT_TRUE(vao) << "round " << round; + vao->SetAttributeFormat(0, 4, DataType::Float32, false, 16, 0, false); + vao->EnableAttribute(0); + ASSERT_GT(Emitter().EmitVertexElements(Ctx()), 0u) << "round " << round; + peakLive = std::max(peakLive, slots.LiveCount(MGPipeKind::VertexElementsCso)); + // Unbind first: a still-bound VAO goes on living, which is the whole reason the + // death path hangs off the destructor and not off glDeleteVertexArrays. + Ctx().BindVertexArray(0); + Ctx().MarkVertexArrayForDeletion(name); + } + + EXPECT_EQ(slots.LiveCount(MGPipeKind::VertexElementsCso), liveBefore) + << kChurn << " vertex arrays were created and destroyed and the client kept their " + "CSO slots; under a backend that installs no death notice consumer " + "that is one SlotState, one map node and a ~1.3 KB applier record per " + "VAO, for the life of the process"; + // The default VAO plus one recycled slot, i.e. the churn recycles instead of growing. + EXPECT_LE(slots.HighWater(MGPipeKind::VertexElementsCso) - highWaterBefore, 3u) + << "the CSO slot space grew with the churn instead of being recycled"; + EXPECT_LE(peakLive, liveBefore + 2u) << "more than one churned VAO was live at once"; + EXPECT_EQ(MGPipeApplier().RefusedVertexInputCalls, 0u) + << "a death path emitted delete_vertex_elements for a record the applier never had"; + } + + // Espryt's death notice frees the same slot the client's death path frees, and after C-1 + // both run. The allocator's Free is what makes that safe - it refuses a slot that is not + // live at that generation, and the Gen bump rides the NEXT handout rather than the free - + // so a second release cannot skip a generation, cannot double-push the free list, and + // cannot take a slot away from the successor that has meanwhile been given it. + TEST(VertexInputEmit, ADoubleReleaseOfAVertexElementsSlotIsHarmless) { + EmitterScope scope; + auto& slots = MGPipeSlots(); + const Uint32 liveBefore = slots.LiveCount(MGPipeKind::VertexElementsCso); + + const SharedPtr vao = MakeVao(11); + vao->EnableAttribute(0); + ASSERT_GT(Emitter().EmitVertexElements(Ctx()), 0u); + const MGPipeHandle handle = Emitter().BoundHandle(); + ASSERT_FALSE(MGPipeHandleIsNull(handle)); + const Uint64 lifetimeId = vao->GetLifetimeId(); + + // The notice's half, by hand and FIRST - the order Espryt's consumer runs in. + slots.Free(MGPipeKind::VertexElementsCso, handle); + EXPECT_EQ(slots.LiveCount(MGPipeKind::VertexElementsCso), liveBefore); + + // ...and then the client's, which must find nothing and say so rather than corrupt the + // free list or emit a delete for a record it has already forgotten. + EXPECT_FALSE(MGPipeEmitVertexElementsDestroyAndFree(lifetimeId)) + << "a second release resolved a handle the first one retired"; + EXPECT_EQ(slots.LiveCount(MGPipeKind::VertexElementsCso), liveBefore); + + // The successor takes the recycled slot with a MOVED generation, which is the property + // a double free would have broken. + const MGPipeHandle successor = slots.Acquire(MGPipeKind::VertexElementsCso, lifetimeId + 1); + EXPECT_EQ(successor.Slot, handle.Slot); + EXPECT_NE(successor.Gen, handle.Gen); + slots.Free(MGPipeKind::VertexElementsCso, successor); + } #endif // MOBILEGL_PIPE_PUSH } // namespace