diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index b6dfca5c..e49a19b7 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -12,6 +12,7 @@ #include "DirectGLES.h" #include "BackendObject_DirectGLES.h" #include +#include #include #include @@ -171,6 +172,48 @@ namespace MobileGL::MG_Backend::DirectGLES { } #if MOBILEGL_PIPE_PUSH + namespace { + // P2 step e2's dispatcher: the frontend told us an object died, so free its slot and + // drop its twin NOW. One entry point for all six kinds, because the answer is the same + // for all six - which is why the notice carries the kind rather than there being six + // ops tables. + // + // A notice that arrives after exit() has begun is dropped: past that point the twin's + // destructor must not call into the driver (see InProcessTeardown()), and the process + // is about to hand every GPU object back anyway. + void OnFrontendStateObjectDestroyed(MG_Pipe::MGPipeKind kind, Uint64 lifetimeId) { + if (InProcessTeardown()) return; + switch (kind) { + case MG_Pipe::MGPipeKind::Texture: + TextureImpl::g_backendTextureObjects.DestroyByLifetimeId(lifetimeId); + break; + case MG_Pipe::MGPipeKind::Framebuffer: + FramebufferImpl::g_backendFramebufferObjects.DestroyByLifetimeId(lifetimeId); + break; + case MG_Pipe::MGPipeKind::Renderbuffer: + RenderbufferImpl::g_backendRenderbufferObjects.DestroyByLifetimeId(lifetimeId); + break; + case MG_Pipe::MGPipeKind::SamplerCso: + SamplerImpl::g_backendSamplerObjects.DestroyByLifetimeId(lifetimeId); + break; + case MG_Pipe::MGPipeKind::ShaderCso: + PrgramImpl::g_backendProgramObjects.DestroyByLifetimeId(lifetimeId); + break; + case MG_Pipe::MGPipeKind::VertexElementsCso: + VertexArrayImpl::g_backendVertexArrayObjects.DestroyByLifetimeId(lifetimeId); + break; + default: + // Buffer already has its own death signal (BufferBackendOps::OnDestroy) and + // every other kind has no backend twin table here. + break; + } + } + + const MG_State::GLState::StateObjectDeathOps g_glesStateObjectDeathOps = { + .OnDestroyed = OnFrontendStateObjectDestroyed, + }; + } // namespace + Bool ResolveEsprytSlotTablesArm() { // Resolved once and latched by the inline EsprytSlotTablesEnabled() in SlotTables.h: // the two arms of StateBackendObjectRegistry keep their twins in different containers, @@ -196,6 +239,12 @@ namespace MobileGL::MG_Backend::DirectGLES { "is clear but MOBILEGL_PIPE_LEGACY_MEMOS=0\"}"); std::abort(); } + if (bitSet) { + // The notice is only consumable on the handle arm (the legacy registry keys on + // the frontend ADDRESS, which is gone by the time a destructor speaks), so it is + // installed exactly where it can be answered. Once per process, cold. + MG_State::GLState::SetStateObjectDeathOps(&g_glesStateObjectDeathOps); + } return bitSet; #else // The legacy arm is not compiled, so the handle arm is the only arm. The bit still @@ -204,6 +253,7 @@ namespace MobileGL::MG_Backend::DirectGLES { MGLOG_D("MGPipe: kMGPipeSubsystemEsprytSlots is clear but this build has no " "legacy twin registry; running the handle arm anyway"); } + MG_State::GLState::SetStateObjectDeathOps(&g_glesStateObjectDeathOps); return true; #endif } diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 61edde38..587a2782 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -417,6 +417,19 @@ namespace MobileGL::MG_Backend::DirectGLES { return nullptr; } + // P2 step e2. The legacy arm cannot answer this at all - its key is the frontend heap + // ADDRESS and the object is already gone by the time the notice arrives - so there it + // is a no-op and the garbage sweep stays its only death signal. That asymmetry is not + // an oversight: it is the A/B the compile-time arm exists to make measurable + // (ARCHITECTURE.md 9.6), and announced-versus-discovered death is one of the things + // being measured. + Bool DestroyByLifetimeId(Uint64 lifetimeId) { + if (EsprytSlotTablesEnabled()) { + return m_slotTable.DestroyByLifetimeId(lifetimeId); + } + return false; + } + // fn(const StatePtr& state, const BackendPtr& twin) over every live entry. The legacy // begin()/end() handed out the map key, i.e. the raw frontend address - exactly the // identity the backend must stop reading - and handed it out for entries whose state diff --git a/MobileGL/MG_Backend/DirectGLES/SlotTables.h b/MobileGL/MG_Backend/DirectGLES/SlotTables.h index 61e40b73..8d381957 100644 --- a/MobileGL/MG_Backend/DirectGLES/SlotTables.h +++ b/MobileGL/MG_Backend/DirectGLES/SlotTables.h @@ -33,17 +33,19 @@ // * Slots are dense per kind, which is what lets the server side (ARCHITECTURE.md 10.1, // MG_Remote/Server/PipeObjectTables) be an array rather than an object graph. // -// What has NOT changed, deliberately, and is this file's one departure from the P2 brief -// (recorded in the package result file): a frontend object's death is still discovered rather -// than announced. The brief's step e2 - a BufferBackendOps-shaped OnDestroy for the other six -// kinds - has to be installed in MG_State/GLState/{Texture,Framebuffer,Renderbuffer,Sampler, -// Program,VertexArray}State/*, and the P2 file-ownership table gives every one of those files -// to another package. So the table keeps ONE weak_ptr per entry and uses it for exactly one -// thing: ReclaimDeadSlots() frees the slot - and the twin, and the driver storage it owns - -// once the frontend object is gone. That is a liveness sweep, not an identity test, and it is -// what bumps Gen, which is precisely the ABA defence: a slot is only ever handed out again -// after it was freed. When e2 lands, ReclaimDeadSlots() becomes the fallback path of an -// explicit Destroy(handle) and the sweep call sites go away. +// Death: announced where the package may announce it, discovered everywhere else. +// DestroyByLifetimeId() below is step e2's backend half and it is complete - it drops the twin +// and returns the slot the moment the frontend object's last SharedPtr goes - and the notice +// that drives it (MG_State/GLState/StateObjectDeathNotice.h, BufferBackendOps' shape) is +// registered for all six kinds. What is only PARTLY wired is the firing side: a destructor has +// to raise the notice, and of the six object classes the P2 file-ownership table gives +// {Texture,Framebuffer,Sampler,VertexArray}State/* to other packages, so only ProgramObject +// and RenderbufferObject fire it here. The four that do not still rely on the sweep, which is +// why the table keeps ONE weak_ptr per entry and uses it for exactly one thing: +// ReclaimDeadSlots() frees the slot - and the twin, and the driver storage it owns - once the +// frontend object is gone. That is a liveness sweep, not an identity test, and it is what +// bumps Gen, which is precisely the ABA defence: a slot is only ever handed out again after it +// was freed. The four remaining one-line destructor calls retire the sweep entirely. // // Because the sweep is still the only death signal, this table carries BOTH of the drivers // the registry it replaces carries, and for the same reasons: @@ -221,6 +223,40 @@ namespace MobileGL::MG_Backend::DirectGLES { m_isCollecting = false; } + // P2 step e2's backend half: the frontend object with this lifetime id has just been + // DESTROYED, so drop its twin and return its slot now rather than waiting for a sweep + // to notice the weak_ptr expired. Announced death is what the sweep is a stand-in for; + // it frees the driver storage the twin owns at the moment the application let go of + // the object, which is what Managers.h's "dead gigabytes" note is about. + // + // Returns whether this table held the slot. The slot goes back to the allocator ONLY + // then, and this is not defensive: two holders of one kind already exist (the + // ScopedDirectGLESTextureBindings fixture's saved copy is a second live table of kind + // Texture; Magma's subsystem-4 table shares the VertexElementsCso kind), and a table + // that never twinned the object must not free a slot the other one still names. + Bool DestroyByLifetimeId(Uint64 lifetimeId) { + const MG_Pipe::MGPipeHandle handle = + MG_Pipe::MGPipeSlots().FindByLifetimeId(kKind, lifetimeId); + if (MG_Pipe::MGPipeHandleIsNull(handle)) return false; + if (handle.Slot >= m_slots.size()) return false; + // Same ordering rule as ReclaimDeadSlots(): the twin's destructor is a driver call + // and could re-enter GetOrCreate and resize m_slots, so nothing that outlives it + // may be a reference into the vector. + BackendPtr dead; + { + Entry& entry = m_slots[handle.Slot]; + if (!entry.Live || entry.Gen != handle.Gen) return false; + dead = std::move(entry.backend); + entry.backend.reset(); + entry.stateRef.reset(); + entry.Live = false; + } + if (m_memoHandle.Slot == handle.Slot) ForgetHandle(); + MG_Pipe::MGPipeSlots().Free(kKind, handle); + dead.reset(); + return true; + } + void CollectGarbageIfNeeded() { ++m_gcTick; if (m_gcTick < kGCInterval) return; diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp index 76c7e1cb..f2ace10c 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp @@ -14,6 +14,7 @@ #include #include #include +#include const char* kDefaultFragmentShaderSource = R"(#version 460 core layout(location = 0) out vec4 FragColor; @@ -28,7 +29,20 @@ namespace MobileGL::MG_State::GLState { return s_nextProgramLifetimeId.fetch_add(1, std::memory_order_relaxed); } - ProgramObject::~ProgramObject() { CancelLink(); } + ProgramObject::~ProgramObject() { + CancelLink(); +#if MOBILEGL_PIPE_PUSH + // P2 step e2: ANNOUNCE the death instead of leaving the backend to discover it in a + // garbage sweep. This is the last SharedPtr to this object dropping - not + // glDeleteProgram, which only marks the name and leaves a still-bound object very much + // alive - so it is the exact moment the backend's twin, and the driver storage that + // twin owns, stop being reachable. The notice carries the lifetime id because the + // object no longer exists to be passed, and because the lifetime id 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). + NotifyStateObjectDestroyed(MG_Pipe::MGPipeKind::ShaderCso, m_lifetimeId); +#endif + } // EnsureLinkJoined() is defined inline in ProgramObject.h (see the comment there for // why: ~1200 call sites, no LTO). Only its blocking half lives here. diff --git a/MobileGL/MG_State/GLState/RenderbufferState/RenderbufferObject.cpp b/MobileGL/MG_State/GLState/RenderbufferState/RenderbufferObject.cpp index 91bfca50..4e68b9d7 100644 --- a/MobileGL/MG_State/GLState/RenderbufferState/RenderbufferObject.cpp +++ b/MobileGL/MG_State/GLState/RenderbufferState/RenderbufferObject.cpp @@ -8,6 +8,7 @@ #include "RenderbufferObject.h" #include +#include #include @@ -26,6 +27,20 @@ namespace MobileGL { RenderbufferObject::RenderbufferObject(Uint externalIndex) : m_externalIndex(externalIndex) {} +#if MOBILEGL_PIPE_PUSH + RenderbufferObject::~RenderbufferObject() { + // P2 step e2: ANNOUNCE the death instead of leaving the backend to discover it in a + // garbage sweep. This is the last SharedPtr to this object dropping - not + // glDeleteRenderbuffers, which only marks the name and leaves a still-bound object very much + // alive - so it is the exact moment the backend's twin, and the driver storage that + // twin owns, stop being reachable. The notice carries the lifetime id because the + // object no longer exists to be passed, and because the lifetime id 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). + NotifyStateObjectDestroyed(MG_Pipe::MGPipeKind::Renderbuffer, m_lifetimeId); + } +#endif + Uint RenderbufferObject::GetExternalIndex() const { return m_externalIndex; } diff --git a/MobileGL/MG_State/GLState/RenderbufferState/RenderbufferObject.h b/MobileGL/MG_State/GLState/RenderbufferState/RenderbufferObject.h index 1e1f304b..8b8a3003 100644 --- a/MobileGL/MG_State/GLState/RenderbufferState/RenderbufferObject.h +++ b/MobileGL/MG_State/GLState/RenderbufferState/RenderbufferObject.h @@ -25,6 +25,12 @@ namespace MobileGL { using TargetEnum = RenderbufferTarget; RenderbufferObject(Uint externalIndex); +#if MOBILEGL_PIPE_PUSH + // P2 step e2. Out of line, and declared only where there is a notice to raise: + // in a pull build this class stays trivially destructible, which is what keeps + // the pull build's symbol set byte-for-byte the pre-P2 one (G1). + ~RenderbufferObject(); +#endif Uint GetExternalIndex() const; void SetInternalFormat(TextureInternalFormat format); diff --git a/MobileGL/MG_State/GLState/StateObjectDeathNotice.h b/MobileGL/MG_State/GLState/StateObjectDeathNotice.h new file mode 100644 index 00000000..ba7ce54e --- /dev/null +++ b/MobileGL/MG_State/GLState/StateObjectDeathNotice.h @@ -0,0 +1,62 @@ +// MobileGL - MobileGL/MG_State/GLState/StateObjectDeathNotice.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 + +#pragma once +#include + +#if MOBILEGL_PIPE_PUSH +#include + +// P2 step e2, the frontend half: TELL the backend that a state object died, instead of +// leaving it to discover the death in a garbage sweep. +// +// Today the only kind that announces its own death is Buffer, through BufferBackendOps +// (BufferState/BufferObject.h) - a frontend-declared ops table that the backend fills in at +// context bring-up. This is the same shape for the other six kinds, with two differences that +// follow from what the notice is for: +// +// * it carries {kind, lifetimeId} and NOT the object, because by the time the last +// SharedPtr has dropped there is no object left to pass, and the lifetime id is exactly +// the key the client slot allocator resolves a handle from (ARCHITECTURE.md 4.2); +// * it is one entry point for every kind rather than one ops table per kind, because the +// backend's answer is the same for all six: free the slot, drop the twin. +// +// It exists only under MOBILEGL_PIPE_PUSH. A pull build has no slot allocator, no handle and +// nothing that could consume the notice, and G1 requires its symbol set to be byte-for-byte +// the pre-P2 one - so in that build this header declares nothing at all and the call sites +// compile to nothing. +// +// The pointer is written once, at backend bring-up, and read from state-object destructors. +// It is deliberately a plain pointer and not an atomic: the destructors and the bring-up run +// on the context thread, exactly as BufferBackendOps' g_bufferBackendOps does. +namespace MobileGL::MG_State::GLState { + + struct StateObjectDeathOps { + // The last SharedPtr to the frontend object with this lifetime id has dropped. + // Called from the object's destructor, so the object must NOT be touched. + void (*OnDestroyed)(MG_Pipe::MGPipeKind kind, Uint64 lifetimeId) = nullptr; + }; + + inline const StateObjectDeathOps* g_stateObjectDeathOps = nullptr; + + inline void SetStateObjectDeathOps(const StateObjectDeathOps* ops) { + g_stateObjectDeathOps = ops; + } + + inline const StateObjectDeathOps* GetStateObjectDeathOps() { + return g_stateObjectDeathOps; + } + + inline void NotifyStateObjectDestroyed(MG_Pipe::MGPipeKind kind, Uint64 lifetimeId) { + const StateObjectDeathOps* ops = g_stateObjectDeathOps; + if (ops == nullptr || ops->OnDestroyed == nullptr) return; + ops->OnDestroyed(kind, lifetimeId); + } + +} // namespace MobileGL::MG_State::GLState +#endif // MOBILEGL_PIPE_PUSH diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index 7c83f907..59fd3c1b 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -39,6 +39,9 @@ #include #include #include +#include +#include +#include #include #include @@ -3396,6 +3399,95 @@ TEST(DirectGLESSlotTable, GetOrCreateToleratesANullStateObject) { EXPECT_EQ(table.Find(nullptr), nullptr); EXPECT_TRUE(MG_Pipe::MGPipeHandleIsNull(table.HandleOf(nullptr))); } +// P2 step e2, the backend half. A sweep is a stand-in for a death notice; this is the notice. +// Nothing below calls CollectGarbage*: the slot comes back, and the twin goes, at the moment +// the frontend says the object is gone - which for a texture atlas or a renderbuffer is the +// difference between freeing a driver allocation now and freeing it 64 creations from now. +TEST(DirectGLESSlotTable, AnAnnouncedDeathReturnsTheSlotWithoutASweep) { + using namespace MobileGL; + + auto& slots = MG_Pipe::MGPipeSlots(); + const Uint32 liveBefore = slots.LiveCount(MG_Pipe::MGPipeKind::Query); + + FakeSlotTable table; + auto object = MakeShared(0x1E2A0001ull); + table.GetOrCreate(object) = MakeShared(); + const MG_Pipe::MGPipeHandle handle = table.HandleOf(object.get()); + ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(handle)); + EXPECT_EQ(slots.LiveCount(MG_Pipe::MGPipeKind::Query), liveBefore + 1u); + + // The object is STILL ALIVE here, which is the point: the sweep's weak_ptr test cannot + // fire, so anything that changes is the notice's doing and nothing else's. + EXPECT_TRUE(table.DestroyByLifetimeId(object->GetLifetimeId())); + EXPECT_EQ(table.LiveCount(), 0u) << "the twin survived its own destroy notice"; + EXPECT_EQ(table.FindByHandle(handle), nullptr); + EXPECT_EQ(table.Find(object.get()), nullptr); + EXPECT_EQ(slots.LiveCount(MG_Pipe::MGPipeKind::Query), liveBefore) + << "the slot was not returned to the allocator"; + + // Idempotent, and a table that does not hold the slot says so rather than freeing it out + // from under whoever does. Both matter once two holders of one kind exist. + EXPECT_FALSE(table.DestroyByLifetimeId(object->GetLifetimeId())); + FakeSlotTable other; + EXPECT_FALSE(other.DestroyByLifetimeId(object->GetLifetimeId())); +} + +// The firing side of e2, on the two of the six object classes whose files this package owns. +// The notice has to arrive when the LAST SharedPtr drops - not when glDeleteProgram marks the +// name, because a still-bound object goes on living - so the object is simply dropped here. +TEST(DirectGLESSlotTable, AProgramAndARenderbufferAnnounceTheirOwnDeath) { + using namespace MobileGL; + + struct Notice { + MG_Pipe::MGPipeKind kind = MG_Pipe::MGPipeKind::None; + Uint64 lifetimeId = 0; + }; + static Vector notices; + notices.clear(); + const MG_State::GLState::StateObjectDeathOps recording = { + .OnDestroyed = [](MG_Pipe::MGPipeKind kind, Uint64 lifetimeId) { + notices.push_back(Notice{kind, lifetimeId}); + }, + }; + const MG_State::GLState::StateObjectDeathOps* previous = + MG_State::GLState::GetStateObjectDeathOps(); + MG_State::GLState::SetStateObjectDeathOps(&recording); + + Uint64 programId = 0; + Uint64 renderbufferId = 0; + { + auto program = MakeShared(0u); + programId = program->GetLifetimeId(); + auto renderbuffer = MakeShared(0u); + renderbufferId = renderbuffer->GetLifetimeId(); + EXPECT_TRUE(notices.empty()) << "a live object announced its own death"; + } + + MG_State::GLState::SetStateObjectDeathOps(previous); + + ASSERT_EQ(notices.size(), 2u); + // Destruction is reverse of construction, so the renderbuffer speaks first. + EXPECT_EQ(notices[0].kind, MG_Pipe::MGPipeKind::Renderbuffer); + EXPECT_EQ(notices[0].lifetimeId, renderbufferId); + EXPECT_EQ(notices[1].kind, MG_Pipe::MGPipeKind::ShaderCso); + EXPECT_EQ(notices[1].lifetimeId, programId); +} + +// ... and that the backend actually installs a consumer for it, rather than the two halves +// each being fine on their own. Registered from ResolveEsprytSlotTablesArm(), i.e. exactly +// when the arm that can answer a notice is the arm that runs. +TEST(DirectGLESSlotTable, TheHandleArmInstallsTheDeathNoticeConsumer) { + using namespace MobileGL; + + if (!MG_Backend::DirectGLES::EsprytSlotTablesEnabled()) { + GTEST_SKIP() << "the legacy arm keys on the frontend address and cannot answer a notice"; + } + ASSERT_NE(MG_State::GLState::GetStateObjectDeathOps(), nullptr) + << "the handle arm runs but nothing consumes a death notice, so every twin still waits " + "for a garbage sweep"; + EXPECT_NE(MG_State::GLState::GetStateObjectDeathOps()->OnDestroyed, nullptr); +} + // The gate on MAJOR 1 of the round-2 review: this binary's OTHER 82 cases - among them every // D13 "must not break" item - are only evidence about this package if they run on the arm this // package wrote. Before EsprytSlotArmEnvironment existed they did not, in any build directory @@ -3451,4 +3543,16 @@ TEST(DirectGLESSlotTable, GetOrCreateToleratesANullStateObject) { TEST(DirectGLESSlotTable, TheTwinRegistryCasesInThisBinaryRunOnTheHandleArm) { GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; } + +TEST(DirectGLESSlotTable, AnAnnouncedDeathReturnsTheSlotWithoutASweep) { + GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; +} + +TEST(DirectGLESSlotTable, AProgramAndARenderbufferAnnounceTheirOwnDeath) { + GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; +} + +TEST(DirectGLESSlotTable, TheHandleArmInstallsTheDeathNoticeConsumer) { + GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; +} #endif // MOBILEGL_PIPE_PUSH