From 0117d157257810420dcd854490d5a1d6bd5629d2 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 06:36:07 -0400 Subject: [PATCH] [MG_Test] (Disaggregated): P5e id - the band case, and the exemption scope stops exempting an unbarriered apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - New DirectGLESSlotTable.ACompositeHandleDoesNotGrowTheOrdinaryTable (with its G2/G14 skip twin) pins the band: a composite handle grows m_band by four entries and the ORDINARY table by none, the lookup finds it, an ordinary slot beside it disturbs neither space, ForEachLive reports the composite at the slot THE CLIENT MINTED rather than at its band index, and §4.3's recycle answer holds inside the band. It asserts on the two spaces' CAPACITIES, not on liveness: the table holds one live entry either way, so LiveCount could not see the growth this case exists to catch. - RemoteGuards.AnAllocatorProbeInsideAnExemptionScopeOnTheApplyThreadIsAllowed is DELETED. It asserted that a named scope exempts unconditionally, which is the rule CONTRACT-P5E §4.4 retires. Its replacement, ...FromTheApplyThreadIsFatalEvenAtTheOldDebtSites, opens the same scope with the applier's CurrentRecordBarriered forced false and expects HandleOf to abort. The hook is the production field ApplyOne stamps, not a test-only branch beside it; the texture and the table are built on the CLIENT thread, because a TextureObject2D's constructor mints and building it on the apply thread would abort at the mint with the same diagnostic - a case passing for the wrong reason. - The green half of the deleted case needs no replacement: every Espryt integration-split entry runs the frontend-keyed scope's surviving sites on the apply thread behind barriered records, so a guard that stopped exempting would take the whole lane down. - Unit 2203 -> 2204 (the new name only adds); integration-split unchanged at 111. --- MobileGL/MG_Test/SanityTest.cpp | 76 ++++++++++++++++++++++ MobileGL/MG_Test/Wire/RemoteClientTest.cpp | 51 ++++++++++++--- 2 files changed, 119 insertions(+), 8 deletions(-) diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index 98da37b5..80beaa1a 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -3410,6 +3410,12 @@ namespace { BackendSlotTable; using FakeSharedKindSlotTable = MobileGL::MG_Backend::DirectGLES:: BackendSlotTable; + // P5e (id): ShaderCso is the ONE kind with a composite band, so the band case needs a table + // of that kind. It never touches MGPipeSlots() - the whole case runs through the by-handle + // overloads - and the holder list is per TABLE TYPE, so this instantiation shares nothing + // with the real g_backendProgramObjects. + using FakeShaderCsoSlotTable = MobileGL::MG_Backend::DirectGLES:: + BackendSlotTable; // A log file path no other process and no other case can be writing to: the pid keeps two // SanityTest processes on one host apart, the counter keeps two cases in one process apart. @@ -4932,6 +4938,72 @@ TEST(DirectGLESSlotTable, AGenerationBehindTheLiveTwinIsRefusedRatherThanAdopted EXPECT_EQ(table.FindByHandle(MG_Pipe::MGPipeHandle{FakeSlotTable::kMaxHandleSlot, 1u}), nullptr); } +// P5e (id), CONTRACT-P5E §4.3's last paragraph: THE COMPOSITE BAND, and why it is a P5e +// prerequisite rather than a follow-up. +// +// A program-pipeline composite's ShaderCso slot comes out of the reserved top 1/16 of the slot +// space (kMGPipeShaderCsoCompositeSlotBase = 983040). EntryAt indexes BY SLOT and resizes to +// it, so before the band a single composite grew g_backendProgramObjects to ~983k entries of +// ~40 B - ~40 MB for one glBindProgramPipeline. That was reachable only through +// GetOrCreate(StatePtr) for a composite program; after id's rekey the by-handle resolution IS +// the ordinary path (ResolveProgramTwin), so every composite bind would pay it. +// +// THE RED: make EntryAt index m_slots unconditionally again (drop the SlotIsBanded branch) and +// the first assertion below goes from 0 to 983044. LiveCount alone could not see that - the +// table would hold exactly one live entry either way - which is why the case asserts on the +// two spaces' CAPACITIES and not on liveness. +TEST(DirectGLESSlotTable, ACompositeHandleDoesNotGrowTheOrdinaryTable) { + using namespace MobileGL; + using namespace MobileGL::MG_Backend::DirectGLES; + + FakeShaderCsoSlotTable table; + const MG_Pipe::MGPipeHandle composite{MG_Pipe::kMGPipeShaderCsoCompositeSlotBase + 3u, 1u}; + ASSERT_TRUE(MG_Pipe::MGPipeIsCompositeShaderSlot(composite.Slot)); + + auto& twin = table.GetOrCreate(composite); + twin = MakeShared(); + twin->marker = 0x5A; + + EXPECT_EQ(table.OrdinaryCapacityForTest(), 0u) + << "a composite slot grew the ORDINARY table - one program pipeline is ~40 MB of twin " + "entries for a single live program"; + EXPECT_EQ(table.CompositeCapacityForTest(), 4u) + << "the band is indexed by (slot - base), so slot base+3 needs exactly four entries"; + EXPECT_EQ(table.LiveCount(), 1u); + EXPECT_EQ(table.CompositeLiveCount(), 1u) << "the live composite was counted in the wrong space"; + + auto* const found = table.FindByHandle(composite); + ASSERT_NE(found, nullptr) << "the band entry is invisible to the lookup that has to find it"; + EXPECT_EQ((*found)->marker, 0x5A); + + // An ordinary ShaderCso lands in the ordinary space and disturbs neither the band's + // contents nor its size: the two are dense against their OWN high-water marks. + auto& ordinary = table.GetOrCreate(MG_Pipe::MGPipeHandle{5u, 1u}); + ordinary = MakeShared(); + EXPECT_EQ(table.OrdinaryCapacityForTest(), 6u); + EXPECT_EQ(table.CompositeCapacityForTest(), 4u); + EXPECT_EQ(table.CompositeLiveCount(), 1u); + + // ForEachLive reports the composite at THE SLOT THE CLIENT MINTED, never the band index - + // the handle it hands over is the one a caller turns straight back into a record lookup. + Vector walked; + table.ForEachLive([&](MG_Pipe::MGPipeHandle handle, const SharedPtr&) { + walked.push_back(handle.Slot); + }); + ASSERT_EQ(walked.size(), 2u); + EXPECT_EQ(walked[0], 5u) << "the ordinary space is walked first, at its own slot"; + EXPECT_EQ(walked[1], composite.Slot) + << "the band was reported at its INDEX rather than at the client's slot, so a caller " + "resolving the record for it would read another program's"; + + // §4.3's recycle answer holds inside the band exactly as it does outside it. + auto& recycled = table.GetOrCreate(MG_Pipe::MGPipeHandle{composite.Slot, 2u}); + EXPECT_EQ(recycled, nullptr) << "a successor at a recycled composite slot inherited its " + "predecessor's driver program"; + EXPECT_EQ(table.FindByHandle(composite), nullptr) << "the predecessor's handle still resolves"; + EXPECT_EQ(table.CompositeCapacityForTest(), 4u) << "the recycle re-grew the band"; +} + #else // G2 wants the pull and the push build to list the SAME ctest entries. The twin table only // exists under MOBILEGL_PIPE_PUSH, so in the pull build each case above keeps its name and @@ -5012,6 +5084,10 @@ TEST(DirectGLESSlotTable, AGenerationBehindTheLiveTwinIsRefusedRatherThanAdopted GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; } +TEST(DirectGLESSlotTable, ACompositeHandleDoesNotGrowTheOrdinaryTable) { + GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; +} + TEST(DirectGLESSlotTable, ADeathNoticeForEveryP4aKindIsIdempotent) { GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH"; } diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index cba77bd5..301bd552 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -1858,9 +1858,18 @@ MGL_BUFFER_GUARD_TEST(BufferHasDefinedContentFromTheApplyThreadIsFatalByName, 70 // exempting the apply thread, which is exactly the leak the thread_local used to prevent; // (b) drop the increment altogether and (a) goes red - the exemption stops exempting and // the debt's own sites (DirectGLES' HandleOf probes) abort the server; -// (c) drop the guard's `MGPipeReverseAnnouncementScope::ActiveOnApplyThread()` / +// (c) drop the guard's `MagmaP7AllocatorDebtScope::ActiveOnApplyThread()` / // `FrontendKeyedRegistryScope::ActiveOnApplyThread()` rows and (b) - the control - stops // being the only aborting arm. +// +// P5e (id), CONTRACT-P5E §4.4 REPLACED (b). The case that used to sit there - +// `AnAllocatorProbeInsideAnExemptionScopeOnTheApplyThreadIsAllowed` - asserted that a scope +// exempts UNCONDITIONALLY, which is exactly the rule §4.4 retires: an exemption is a debt the +// CLIENT'S WAIT pays for, so it holds only while the record being applied is barriered. Its +// replacement below drives the same scope with the barriered flag forced false and expects the +// abort. The green half of the old case did not need a test of its own: every Espryt +// `integration-split` entry runs the frontend-keyed scope's surviving sites on the apply thread +// behind barriered records, so a guard that stopped exempting would take the whole lane down. TEST(RemoteGuards, AnAllocatorProbeFromTheApplyThreadOutsideEveryScopeIsFatalByName) { const auto child = RunInChild([] { StartControlSession(); @@ -1875,21 +1884,47 @@ TEST(RemoteGuards, AnAllocatorProbeFromTheApplyThreadOutsideEveryScopeIsFatalByN ExpectNamedAbort(child, "Fatal{RoleViolation, \"MGPipeSlots\"}"); } -TEST(RemoteGuards, AnAllocatorProbeInsideAnExemptionScopeOnTheApplyThreadIsAllowed) { +// P5e (id) RED-ONCE, CONTRACT-P5E §4.4. The exemption scope is OPEN and the probe is the one +// the debt is named for - `HandleOf`, the frontend-keyed registry's own entry - and it still +// aborts, because the record being applied is unbarriered: the client is running ahead and the +// allocator's free list and lifetimeId -> slot map are moving under the read. +// +// THE HOOK IS THE APPLIER FIELD ITSELF and that is deliberate: `CurrentRecordBarriered` is what +// PipeApplier::ApplyOne stamps from MGPipeBarriered() on every record, so forcing it here drives +// the production path rather than a test-only branch beside it. Until ra lands the wait rule +// MGPipeBarriered answers true for every record, so this is the ONLY way to reach the arm - and +// the arm has to exist before ra, because ra is what makes it reachable in production. +// +// The red: put §4.4's `if (MGPipeApplierCurrentRecordIsBarriered())` back to an unconditional +// exemption in MGPipeRefuseAllocatorFromApplyThread and this case stops aborting - red by +// expectation, which is the shape a guard case fails in. +// EVERYTHING BUT THE PROBE RUNS ON THE CLIENT THREAD, which is the buffer/texture guard cases' +// shape one block down and is load-bearing here rather than tidy: a TextureObject2D's +// constructor MINTS a resource handle, and building it on the apply thread would abort at the +// mint with the same diagnostic - a case that passes for a reason other than the one it names. +TEST(RemoteGuards, AnAllocatorProbeInsideAnExemptionScopeFromTheApplyThreadIsFatalEvenAtTheOldDebtSites) { + struct Probe { + TestTextureTable* table; + MG_State::GLState::TextureObject2D* texture; + }; const auto child = RunInChild([] { StartControlSession(); + MG_State::GLState::TextureObject2D texture(46); + TestTextureTable table; + Probe probe{&table, &texture}; Srv::ServerLoopInstance().RunOnApplyThread( - +[](void*) -> MobileGLResult { + +[](void* self) -> MobileGLResult { + auto& probe = *static_cast(self); + // The record being applied is one the client did NOT park behind. + MG_Pipe::MGPipeApplier().CurrentRecordBarriered = false; const MG_Pipe::MGPipeFrontendKeyedRegistryScope frontendKeyedRegistry; - MG_Pipe::MGPipeRefuseAllocatorFromApplyThread("FindByLifetimeId"); - const MG_Pipe::MGPipeReverseAnnouncementScope reverseAnnouncement; - MG_Pipe::MGPipeRefuseAllocatorFromApplyThread("Acquire"); + probe.table->HandleOf(probe.texture); return MOBILEGL_OK; }, - nullptr); + &probe); ClientSessionInstance().Stop(); }); - ExpectChildSuccess(child); + ExpectNamedAbort(child, "Fatal{RoleViolation, \"MGPipeSlots\"}"); } // THE ONE THAT PINS THE SINGLE-WRITER ARGUMENT. The scope is open on the GL thread for the