mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-11-07 01:53:24 +09:00
[MG_Backend, MG_State, MG_Pipe, MG_Remote, MG_Impl, MG_IntegrationTest, docs] (P7): magma installs a state-object death table, and the inproc arm stops losing every object_death
CONTRACT-P7 §5.5. DirectVulkan had no StateObjectDeathOps at all, so under a Magma
inproc session NotifyStateObjectDestroyed reached nobody and the server was never told
that a frontend object had died. Package L measured the shape: with the WireTables
install condition made backend-agnostic, CtWireScenario's two death cases PASS on Magma
spawn and tcp - the remote client has no local backend, so the client-side emitter
installed there answers the notice - and FAIL on inproc, where the install condition
deliberately stands aside for "the backend's own dispatcher" and Magma had none.
g_magmaStateObjectDeathOps is that dispatcher. It mirrors g_glesStateObjectDeathOps
(DirectGLES/Managers.cpp:335) with ONE arm rather than two: Espryt's other arm destroys
a backend twin keyed by lifetime id, and Magma keeps no such twin - its two
client-minted kinds are keyed on {slot, gen} by MagmaPipeIdentityTables and the other
six are still reached from their frontend objects - so what is left is exactly the emit
of object_death off the apply thread. Installed from
BackendObject_DirectVulkan::Initialize(), i.e. ServerLoop::CreateBackend, step 1 of
InitServerRoleCommon: the one place BOTH Magma server roles pass through, and before any
client session exists. #if MOBILEGL_BUILD_DISAGGREGATED && MOBILEGL_PIPE_PUSH, so the
pull build compiles nothing new.
The two skip guards in CtWireScenario go with it. Every other "Magma installs none" site
the audit found is comment text whose ARGUMENT survives, and each is reworded rather than
deleted: the table emits the wire record and frees no client slot, so P4a's
backend-neutral NotifyAndFree is still the only thing that returns one, HandleRecycle's
C-1 case still measures the only protection the allocator has, and MagmaPipeArms' age
sweep is still the only thing that retires an identity slot.
The framebuffer death case now reads the client's allocator beside the server's tally.
That correction matters: the plan called this a slot leak provable with PipeSlotPeek, and
it is not - it has been backend-neutral since P4a. What leaked was the SERVER's twin, and
a framebuffer has no wire delete opcode at all, so object_death is its only delivery.
Two installers cannot race although the global is last-writer-wins with no stacking:
Magma's runs only in a process that owns a DirectVulkan backend, and
InstallClientWireTables' arm runs only under Transport == Spawn, which is by construction
the process that owns none.
RED-ONCE: `if (false) InstallStateObjectDeathOps();` reds the two Split cases alone
(`Expected: (afterCounters.deaths) > (deathsBefore), actual: 0 vs 0`, CtWireScenario.cpp:264)
and leaves spawn and tcp green - the exact split of responsibility the comment claims.
Restored: 8/8. Recorded in docs/Disaggregated/notes/p7/magma-c.md.
This commit is contained in:
@@ -307,8 +307,9 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
// 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
|
||||
// Magma did not until P7 wave 2 package C, and whose table even now only EMITS
|
||||
// the wire record - 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
|
||||
|
||||
@@ -367,6 +367,15 @@ namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
void BackendObject_DirectVulkan::Initialize() {
|
||||
#if MOBILEGL_BUILD_DISAGGREGATED
|
||||
VkBufferManager::RegisterWireResourceOps();
|
||||
#if MOBILEGL_PIPE_PUSH
|
||||
// P7 wave 2 package C (CONTRACT-P7 §5.5). BESIDE the resource ops and for the same
|
||||
// reason: this is the one place both Magma server roles pass through - the inproc
|
||||
// role and the spawn/TCP session child - and ServerLoop::CreateBackend calls it at
|
||||
// step 1 of InitServerRoleCommon, before the client session exists and therefore
|
||||
// before any frontend object can die unheard. A per-transport install is how one of
|
||||
// the two roles ends up without it.
|
||||
InstallStateObjectDeathOps();
|
||||
#endif
|
||||
#endif
|
||||
m_initialized = true;
|
||||
}
|
||||
|
||||
@@ -21,6 +21,13 @@
|
||||
#include <Config.h>
|
||||
#include <MG_Remote/Server/ServerLoop.h>
|
||||
#include <MG_Pipe/PipeApply.h>
|
||||
#if MOBILEGL_PIPE_PUSH
|
||||
// P7 wave 2 package C (CONTRACT-P7 §5.5): Magma's own death-notice table. Both headers are
|
||||
// push-only and reached here for the same reason DirectGLES/Managers.cpp reaches them - the
|
||||
// notice is declared by the frontend and answered by whichever backend is running.
|
||||
#include <MG_Remote/Client/WireTables.h>
|
||||
#include <MG_State/GLState/StateObjectDeathNotice.h>
|
||||
#endif
|
||||
#endif
|
||||
#include <atomic>
|
||||
#include <bit>
|
||||
@@ -55,6 +62,66 @@ namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
std::atomic<Uint64> g_rendererGeneration{1};
|
||||
} // namespace
|
||||
|
||||
#if MOBILEGL_BUILD_DISAGGREGATED && MOBILEGL_PIPE_PUSH
|
||||
namespace {
|
||||
// P7 wave 2 package C, CONTRACT-P7 §5.5: MAGMA'S MIRROR OF g_glesStateObjectDeathOps
|
||||
// (DirectGLES/Managers.cpp:335), and the one arm of that table Magma has any work in.
|
||||
//
|
||||
// WHAT THE NOTICE IS FOR HERE, and it is not what it is for on Espryt. Espryt's arm
|
||||
// destroys a BACKEND TWIN keyed by lifetime id; Magma keeps no such twin - its two
|
||||
// client-minted kinds (VertexElementsCso, Buffer) are keyed on {slot, gen} by
|
||||
// MagmaPipeIdentityTables and every other kind is still reached from its frontend
|
||||
// object. So the switch Espryt runs under `#else` has nothing to do on this backend
|
||||
// and this table is exactly the EMIT arm: with a transport up and off the apply
|
||||
// thread, the death crosses as the `object_death` record
|
||||
// (MG_Remote::Client::EmitObjectDeathRecord, CONTRACT-P5C §5.2).
|
||||
//
|
||||
// WHY THAT MATTERS, measured rather than argued (package L's probe,
|
||||
// notes/p7/magma-two-process-first-run.md §6): under SPAWN the client process has no
|
||||
// DirectVulkan backend at all, so InstallClientWireTables' own emitter already
|
||||
// answered the notice and CtWireScenario's two death cases PASSED. Under INPROC the
|
||||
// server role's backend is a thread of this process, Transport is Split rather than
|
||||
// Spawn, and WireTables' install condition deliberately stands aside for "the
|
||||
// backend's own dispatcher" - which on Magma did not exist. Both cases therefore
|
||||
// FAILED on inproc with `deaths` stuck at 0. This table is that dispatcher.
|
||||
//
|
||||
// FRAMEBUFFER IS WHY THE GAP WAS NOT MERELY COSMETIC. The client-side SLOT is freed
|
||||
// backend-neutrally by P4a's per-kind helpers (MG_Impl/Pipe/PipeFill.cpp's
|
||||
// NotifyAndFree frees whatever the notice does), so PipeSlotPeek never saw a leak.
|
||||
// What leaked was the SERVER's twin: a framebuffer has NO wire delete opcode at all
|
||||
// (BRIEF-P4A D-I2), so `object_death` is its only death delivery, and without a
|
||||
// consumer here the server's record stayed Live for the life of the session.
|
||||
void OnMagmaFrontendStateObjectDestroyed(MG_Pipe::MGPipeKind kind, Uint64 lifetimeId) {
|
||||
// Monolith has no record to emit and no server twin to tell.
|
||||
if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return;
|
||||
// Raised ON the apply thread: the server role destroying a frontend object it
|
||||
// created itself (Magma's hidden blit / depth-mipmap resources). The client's
|
||||
// allocator is a client-thread surface (CONTRACT-P5C §3.1) and EmitObjectDeathRecord
|
||||
// probes it, so this arm stays silent exactly as Espryt's does.
|
||||
if (MG_Remote::Server::ServerLoop::OnApplyThread()) return;
|
||||
(void)MG_Remote::Client::EmitObjectDeathRecord(kind, lifetimeId);
|
||||
}
|
||||
|
||||
const MG_State::GLState::StateObjectDeathOps g_magmaStateObjectDeathOps = {
|
||||
.OnDestroyed = OnMagmaFrontendStateObjectDestroyed,
|
||||
};
|
||||
} // namespace
|
||||
|
||||
void InstallStateObjectDeathOps() {
|
||||
// Installed from BackendObject_DirectVulkan::Initialize(), i.e. step 1 of
|
||||
// InitServerRoleCommon (ServerLoop::CreateBackend calls Initialize), which is before
|
||||
// the client session starts and therefore before any frontend object can die.
|
||||
// Unconditional, exactly as Espryt's ResolveEsprytSlotTablesArm is: only one backend
|
||||
// is live at a time, and InstallClientWireTables' own "whoever installed first keeps
|
||||
// the notice" guard is what keeps the remote client from stomping this.
|
||||
MG_State::GLState::SetStateObjectDeathOps(&g_magmaStateObjectDeathOps);
|
||||
}
|
||||
|
||||
Bool StateObjectDeathOpsInstalled() {
|
||||
return MG_State::GLState::GetStateObjectDeathOps() == &g_magmaStateObjectDeathOps;
|
||||
}
|
||||
#endif // MOBILEGL_BUILD_DISAGGREGATED && MOBILEGL_PIPE_PUSH
|
||||
|
||||
Uint64 GetRendererGeneration() {
|
||||
return g_rendererGeneration.load(std::memory_order_acquire);
|
||||
}
|
||||
|
||||
@@ -23,6 +23,18 @@ namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
Uint64 GetRendererGeneration();
|
||||
void BumpRendererGeneration();
|
||||
|
||||
#if MOBILEGL_BUILD_DISAGGREGATED && MOBILEGL_PIPE_PUSH
|
||||
// P7 wave 2 package C (CONTRACT-P7 §5.5). Magma's StateObjectDeathOps table, installed
|
||||
// from BackendObject_DirectVulkan::Initialize() so that GetStateObjectDeathOps() is
|
||||
// non-null on every Magma role that owns a backend. See the definition for what the one
|
||||
// arm does and for the inproc gap it closes. Push-only and disaggregated-only: a pull
|
||||
// build declares no notice at all (StateObjectDeathNotice.h) and has nothing to emit.
|
||||
void InstallStateObjectDeathOps();
|
||||
// True when the notice currently installed is this backend's. Read by MG_Test so the
|
||||
// install can be asserted without a death having to happen first.
|
||||
Bool StateObjectDeathOpsInstalled();
|
||||
#endif
|
||||
|
||||
// Drops every cached program-resource reflection entry (CPU-side strings/vectors
|
||||
// only, no Vulkan handles). Called at EGL teardown next to the renderer reset;
|
||||
// safe because GL calls are serialized in this codebase, and any still-live
|
||||
|
||||
@@ -166,9 +166,17 @@ namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
// WHAT IT DOES NOT COVER, AND WHY NO REPRODUCER OF THIS SHAPE CAN [fix-aba review v1,
|
||||
// MAJOR 1]. It does NOT exercise the GENERATION half of {slot, gen}:
|
||||
//
|
||||
// * this mint has no death notification - nothing in MG_Backend/DirectVulkan consumes
|
||||
// NotifyStateObjectDestroyed - so a slot returns to the free list only through
|
||||
// OnFrameBoundary's age sweep (kSweepInterval 256, kRetireAgeBoundaries 1024, below);
|
||||
// * THIS MINT still has no death notification, and that is now a narrower statement than
|
||||
// it was. P7 wave 2 package C gives DirectVulkan a StateObjectDeathOps table
|
||||
// (DirectVulkan.cpp's g_magmaStateObjectDeathOps, CONTRACT-P7 §5.5), so
|
||||
// NotifyStateObjectDestroyed DOES have a consumer on this backend - but that consumer's
|
||||
// one job is to emit the `object_death` RECORD so the server drops its twin, and it
|
||||
// never touches these per-renderer identity tables: they are the SERVER-side renderer's
|
||||
// own, the notice is raised on the CLIENT thread, and an Acquire/retire from the wrong
|
||||
// side is precisely the cross-role read CONTRACT-P5C §3.1 forbids. So a slot here still
|
||||
// returns to the free list only through OnFrameBoundary's age sweep (kSweepInterval
|
||||
// 256, kRetireAgeBoundaries 1024, below), and everything the two bullets below derive
|
||||
// from that is unchanged;
|
||||
// * HandleRecycleScenario issues five frame boundaries, so the free list is empty when
|
||||
// the replacement VAO acquires and it gets a BRAND-NEW slot at Gen 1 (measured:
|
||||
// redVao slot=2 gen=1, greenVao slot=3 gen=1). The knob-off FRESH verdict there is
|
||||
|
||||
@@ -13,8 +13,19 @@
|
||||
// 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.
|
||||
// failed on (DirectVulkan, which at P3a installed no StateObjectDeathOps) rather than only on
|
||||
// the one where a backend-owned free happened to exist.
|
||||
//
|
||||
// P7 wave 2 package C: DirectVulkan now installs a table of its own (CONTRACT-P7 §5.5), and
|
||||
// the sentence above is history rather than the current state - but the reason this header
|
||||
// exists is UNCHANGED and is worth spelling out, because the obvious reading of that news is
|
||||
// wrong. Magma's table does not free a client slot and never could: the free is P4a's, it
|
||||
// runs from the frontend object's own destructor on the client thread whatever backend is
|
||||
// live (MG_Impl/Pipe/PipeFill.cpp's NotifyAndFree), and Magma's table is the EMIT arm that
|
||||
// tells the SERVER to drop its twin. So the allocator is still the only observable of the
|
||||
// client half, and a case that reads `deaths` alone would be blind to a slot that never came
|
||||
// back. The two readings belong together, which is what CtWireScenario's framebuffer case now
|
||||
// does.
|
||||
//
|
||||
// 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
|
||||
|
||||
@@ -25,6 +25,7 @@
|
||||
// half of that control is TheDirectApplierResetCallOnTheGLThreadIsRoleViolation below, and
|
||||
// the guard's two non-Fatal arms are pinned in PipeWireCodecTest's CtWireFatals suite.
|
||||
|
||||
#include "../Harness/PipeSlotPeek.h"
|
||||
#include "../Harness/PipeStatsWindow.h"
|
||||
#include "../Harness/ScenarioFixture.h"
|
||||
#include "../Harness/SplitRuntimePeek.h"
|
||||
@@ -147,13 +148,16 @@ namespace {
|
||||
|
||||
TEST_F(CtWireScenario, TextureDeathCrossesAndTheRecycledSlotAnswersTheNewObject) {
|
||||
if (!Ready()) return;
|
||||
// The object_death producer is Espryt-side (OnFrontendStateObjectDestroyed,
|
||||
// CONTRACT-P5C.md §5.2); Magma installs no StateObjectDeathOps (P7), so under
|
||||
// DirectVulkan there is no death record to watch and the case has nothing to prove.
|
||||
if (HeadlessGL::Get().BackendName() != "DirectGLES") {
|
||||
GTEST_SKIP() << "object_death is produced by the DirectGLES death-notice ops; "
|
||||
"DirectVulkan has none until P7";
|
||||
}
|
||||
// P7 wave 2 package C: THE BACKEND GUARD IS GONE, and both halves of why it was here
|
||||
// are answered rather than waived. The producer used to be Espryt-side only
|
||||
// (OnFrontendStateObjectDestroyed, CONTRACT-P5C.md §5.2), so a DirectVulkan run had
|
||||
// no death record to watch; Magma now installs its own StateObjectDeathOps
|
||||
// (DirectVulkan.cpp, CONTRACT-P7 §5.5) and emits the same `object_death`. Package L
|
||||
// measured that the SPAWN arm already passed once the WireTables install condition
|
||||
// went backend-agnostic and that the INPROC arm failed here with `deaths` 0 vs 0
|
||||
// (notes/p7/magma-two-process-first-run.md §6); the table is what closes the inproc
|
||||
// half, and this case is the gate on it. It is also the RED-ONCE for that table: with
|
||||
// the install site short-circuited, both cases red on the two EXPECT_GT below.
|
||||
const GLubyte red[4] = {255, 0, 0, 255};
|
||||
const GLubyte green[4] = {0, 255, 0, 255};
|
||||
|
||||
@@ -204,14 +208,13 @@ namespace {
|
||||
|
||||
TEST_F(CtWireScenario, FramebufferDeathCrossesAndTheRecycledSlotAnswersTheNewObject) {
|
||||
if (!Ready()) return;
|
||||
// Same producer reason as the texture case above: object_death is emitted by the
|
||||
// DirectGLES death-notice ops; DirectVulkan has none until P7.
|
||||
if (HeadlessGL::Get().BackendName() != "DirectGLES") {
|
||||
GTEST_SKIP() << "object_death is produced by the DirectGLES death-notice ops; "
|
||||
"DirectVulkan has none until P7";
|
||||
}
|
||||
// Backend-agnostic for the texture case's reason (P7 wave 2 package C): both
|
||||
// backends now install a StateObjectDeathOps table and both emit `object_death`.
|
||||
// Framebuffer is the kind object_death EXISTS for: it has no other wire delete
|
||||
// opcode (CONTRACT-P5C.md §5.2). The renderbuffer goes along so the FBO has storage.
|
||||
// opcode (CONTRACT-P5C.md §5.2) - which is exactly why the missing Magma table was
|
||||
// a SERVER-side twin leak and not merely a missing counter: with no consumer for
|
||||
// the notice, nothing on the wire ever told the server this framebuffer had died.
|
||||
// The renderbuffer goes along so the FBO has storage.
|
||||
GLuint fbo = 0, renderbuffer = 0;
|
||||
glGenFramebuffers(1, &fbo);
|
||||
glGenRenderbuffers(1, &renderbuffer);
|
||||
@@ -234,6 +237,21 @@ namespace {
|
||||
const auto beforeCounters = ServerCounters();
|
||||
ASSERT_TRUE(beforeCounters.valid) << "server object-death telemetry missing";
|
||||
const MobileGL::Uint64 deathsBefore = beforeCounters.deaths;
|
||||
// THE CLIENT'S OWN ACCOUNTING, read beside the server's (P7 wave 2 package C,
|
||||
// CONTRACT-P7 §5.5's PipeSlotPeek clause). The two numbers answer DIFFERENT halves of
|
||||
// one death and neither implies the other, which is exactly why both are read here:
|
||||
// * `deaths` below says the SERVER was told - the half the missing Magma table
|
||||
// broke, and the only delivery a framebuffer has (no wire delete opcode at all);
|
||||
// * this one says the CLIENT returned the slot. That half has been backend-neutral
|
||||
// since P4a (PipeFill.cpp's NotifyAndFree frees whatever the notice does), so it
|
||||
// is a CONTROL rather than the subject: if it ever regressed, a green `deaths`
|
||||
// would be measuring an object that never let go of its handle.
|
||||
// `false` means the allocator is out of reach (a pull build, or Android's hidden
|
||||
// symbols), and the peek is then simply not asserted - "could not look" is not
|
||||
// "did not leak", so nothing is concluded from it either way.
|
||||
unsigned fboSlotsBefore = 0;
|
||||
const bool slotsReadable =
|
||||
PeekPipeSlotLiveCount(PipeSlotKind::Framebuffer, &fboSlotsBefore);
|
||||
glDeleteFramebuffers(1, &fbo);
|
||||
// THE FENCE (MOBILEGL_IPC_BATCH_WAITS, default on): object_death is a kCtxObject
|
||||
// value-class record - published WITHOUT waiting for its own apply (production-safe:
|
||||
@@ -245,7 +263,16 @@ namespace {
|
||||
ASSERT_TRUE(afterCounters.valid) << "server object-death telemetry missing";
|
||||
EXPECT_GT(afterCounters.deaths, deathsBefore)
|
||||
<< "the framebuffer's death produced no object_death record - and no other "
|
||||
"opcode can carry it";
|
||||
"opcode can carry it. Backend "
|
||||
<< Gl().BackendName();
|
||||
if (slotsReadable) {
|
||||
unsigned fboSlotsAfter = 0;
|
||||
ASSERT_TRUE(PeekPipeSlotLiveCount(PipeSlotKind::Framebuffer, &fboSlotsAfter));
|
||||
EXPECT_LT(fboSlotsAfter, fboSlotsBefore)
|
||||
<< "the dead framebuffer kept its client slot: live count " << fboSlotsBefore
|
||||
<< " -> " << fboSlotsAfter << " across the delete. Backend "
|
||||
<< Gl().BackendName();
|
||||
}
|
||||
glDeleteRenderbuffers(1, &renderbuffer);
|
||||
|
||||
// Recycle: a new FBO and a new backing store, cleared to a different colour. A stale
|
||||
|
||||
@@ -1394,13 +1394,21 @@ void main() { oColor = texture(uTex, vUv); }
|
||||
// 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
|
||||
// StateObjectDeathOps consumer - so under Magma, which installed none
|
||||
// (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.
|
||||
//
|
||||
// P7 WAVE 2 PACKAGE C DOES NOT RETIRE THIS CASE, and the reason is the whole point of
|
||||
// C-1. Magma now installs a StateObjectDeathOps table (CONTRACT-P7 §5.5), but that
|
||||
// table's one arm EMITS the `object_death` record so the SERVER drops its twin - it
|
||||
// frees no client slot and could not: the free is the client's own, from the frontend
|
||||
// destructor, on the client thread. So this case still measures the only thing that
|
||||
// ever protected the allocator, and it would still be the case that goes red if that
|
||||
// free were ever moved back behind a backend table.
|
||||
//
|
||||
// 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.
|
||||
|
||||
@@ -139,8 +139,9 @@ namespace MobileGL::MG_Pipe {
|
||||
|
||||
// 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
|
||||
// StateObjectDeathOps table; under a backend that installed none - DirectVulkan/Magma at
|
||||
// P3a, which keeps its own age-reclaimed identity table on purpose and which P7 wave 2
|
||||
// package C gave an EMIT-only table that still frees no slot - 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
|
||||
|
||||
@@ -757,13 +757,21 @@ namespace MobileGL::MG_Remote::Client {
|
||||
// That is what "keep inproc's existing backend dispatcher" meant, and it is also
|
||||
// what keeps this from stomping a backend that installs ops of its own later.
|
||||
//
|
||||
// WHAT THIS DOES *NOT* DO IS GIVE MAGMA A DEATH TABLE. Magma installs no
|
||||
// StateObjectDeathOps at all (wave 2 package C mirrors g_glesStateObjectDeathOps for
|
||||
// it); until it does, the client-side emitter installed here still asks
|
||||
// MGPipeSlots().FindByLifetimeId for a handle, gets a null one for an object Magma
|
||||
// never routed, and emits nothing. The count under Magma may therefore still be 0 -
|
||||
// but it is now 0 because no object crossed, which the lane can see and attribute,
|
||||
// instead of 0 because nobody was listening, which it could not.
|
||||
// MAGMA NOW HAS A DEATH TABLE OF ITS OWN (P7 wave 2 package C, CONTRACT-P7 §5.5:
|
||||
// DirectVulkan.cpp's g_magmaStateObjectDeathOps), and the two installs do NOT race,
|
||||
// which is worth stating because the global is a bare last-writer-wins pointer with
|
||||
// no stacking. They cannot meet: Magma's is installed from
|
||||
// BackendObject_DirectVulkan::Initialize(), i.e. only in a process that OWNS a
|
||||
// DirectVulkan backend, and the arm here runs only under `Transport == Spawn`, which
|
||||
// is by construction the process that owns NO backend at all (InitSplitRoles builds a
|
||||
// BackendObject_Remote there). So the spawn client keeps this emitter, the inproc and
|
||||
// server-side roles keep Magma's, and `GetStateObjectDeathOps() == nullptr` below
|
||||
// keeps meaning what P6.5 meant by it.
|
||||
//
|
||||
// MEASURED, both directions (package C's red-once): with Magma's install
|
||||
// short-circuited, the two CtWireScenario death cases red on the INPROC arm alone
|
||||
// (`deaths` 0 vs 0) and stay green on spawn and tcp - which is exactly the split of
|
||||
// responsibility this comment claims.
|
||||
if (MG_Config::Transport == MG_Config::TransportMode::Spawn &&
|
||||
MG_State::GLState::GetStateObjectDeathOps() == nullptr) {
|
||||
MG_State::GLState::SetStateObjectDeathOps(&kClientStateObjectDeathOps);
|
||||
|
||||
@@ -50,9 +50,12 @@ namespace MobileGL::MG_State::GLState {
|
||||
// ALL FOUR OF THOSE GO THROUGH ONE HELPER, and it is the client's rather than a
|
||||
// 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,
|
||||
// under a backend that installs none - which DirectVulkan/Magma did, 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. P7 wave 2 package C gave Magma a table (CONTRACT-P7 §5.5) and the
|
||||
// argument is UNCHANGED, because that table emits the wire death record and frees no
|
||||
// slot: the client's helper is still the only path any of the four take. 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
|
||||
|
||||
@@ -0,0 +1,80 @@
|
||||
# P7 wave 2 package C — Magma object death, resident sub-data, the reflection archive, the vertex-layout split
|
||||
|
||||
Base: `p7/magma-c` off `feat/disaggregated @ a686131e` (pipe HEAD at branch time).
|
||||
Build: `build-split` = `-DMOBILEGL_BUILD_DISAGGREGATED=ON -DMOBILEGL_BUILD_DISAGGREGATED_INPROC=ON
|
||||
-DMOBILEGL_PIPE_PUSH=ON -DMOBILEGL_BUILD_INTEGRATION_TEST=ON`, ICD pinned to
|
||||
`/usr/share/vulkan/icd.d/lvp_icd.json` (the same lavapipe ICD `~/w7/pipe/build-split` pins; the
|
||||
worktree tool leaves `MOBILEGL_ITEST_VK_ICD` empty and CMake warns about it).
|
||||
|
||||
Contract rows: CONTRACT-P7 §5.5 (`StateObjectDeathOps`), §5.4 (OQ-10), §5.3 (OQ-8), §3.2
|
||||
(`vertex-layout`, `vertex-format-conversion`). Discipline: rule I (no `std::abort()` on a Magma
|
||||
wire arm), rule J / R-16 (every fix red once on a two-process arm first).
|
||||
|
||||
---
|
||||
|
||||
## Slice 1 — Magma `StateObjectDeathOps` (§5.5)
|
||||
|
||||
**What landed.** `MG_Backend/DirectVulkan/DirectVulkan.cpp` gains
|
||||
`g_magmaStateObjectDeathOps` and `InstallStateObjectDeathOps()`, installed from
|
||||
`BackendObject_DirectVulkan::Initialize()` — i.e. from `ServerLoop::CreateBackend`, step 1 of
|
||||
`InitServerRoleCommon`, which both Magma server roles (inproc and the spawn/TCP child) run and
|
||||
which is before any client session exists. Guarded `#if MOBILEGL_BUILD_DISAGGREGATED &&
|
||||
MOBILEGL_PIPE_PUSH`, so the pull build compiles nothing new (G1).
|
||||
|
||||
**The one arm, and why it is only one.** Espryt's `g_glesStateObjectDeathOps` has two jobs: emit
|
||||
the wire `object_death` record off the apply thread, and otherwise destroy a backend twin keyed
|
||||
by lifetime id. Magma has no lifetime-id-keyed twin at all — its two client-minted kinds
|
||||
(`VertexElementsCso`, `Buffer`) are keyed on `{slot, gen}` by `MagmaPipeIdentityTables` and the
|
||||
other six are still reached from their frontend objects — so the Magma table is exactly the EMIT
|
||||
arm and nothing else.
|
||||
|
||||
**Correction to the plan's wording (worth the integrator's attention).** Plan §1.3 and the task
|
||||
brief describe the gap as a client SLOT leak provable with `PipeSlotPeek`. It is not, and has not
|
||||
been since P4a: `MG_Impl/Pipe/PipeFill.cpp`'s `NotifyAndFree` frees the client slot
|
||||
unconditionally and backend-neutrally, so `PipeSlotPeek` never saw a Magma leak and does not see
|
||||
a change now. What leaked was the **server's twin**. A framebuffer has no wire delete opcode at
|
||||
all (BRIEF-P4A D-I2), so `object_death` is its only death delivery; with no consumer installed
|
||||
under Magma inproc, the server's record stayed `Live` for the life of the session. The
|
||||
`CtWireScenario` framebuffer case now reads BOTH numbers — the server's `deaths` tally and the
|
||||
client's `PipeSlotKind::Framebuffer` live count — so a future regression in either half is
|
||||
attributable.
|
||||
|
||||
**Cleanup of the Magma special cases.** The two executable ones are the `CtWireScenario` skip
|
||||
guards (`:153` / `:209`), both deleted. Everything else the audit counted is comment text, and it
|
||||
is reworded rather than deleted because the underlying argument survives: `PipeSlotPeek.h`,
|
||||
`MagmaPipeArms.h` (the age-sweep-instead-of-death-notice paragraph — still true, because the new
|
||||
table never touches the per-renderer identity tables), `PipeMutation.h`, `PipeFill.cpp`,
|
||||
`VertexArrayObject.cpp`, `DirectGLES/Managers.cpp`, `HandleRecycleScenario.cpp`,
|
||||
`WireTables.cpp`.
|
||||
|
||||
**The install-order question, answered rather than assumed.** `SetStateObjectDeathOps` is a bare
|
||||
last-writer-wins pointer with no stacking, so two installers could in principle race.
|
||||
They cannot meet: Magma's runs only in a process that OWNS a DirectVulkan backend, and
|
||||
`InstallClientWireTables`' arm runs only under `Transport == Spawn`, which is by construction the
|
||||
process that owns no backend (`InitSplitRoles` builds a `BackendObject_Remote` there). The
|
||||
red-once below measures exactly that split of responsibility.
|
||||
|
||||
**Proof (green).** `ctest -R "DirectVulkan\.(Split|Spawn|Tcp)\.Ct\..*Death"` — 6/6 pass
|
||||
(`integration-magma-all-{split,spawn,tcp}`), plus the two `TcpServer` fixtures. DirectGLES
|
||||
control `ctest -R "DirectGLES\.(Split|Spawn|Tcp)\.Ct\..*Death"` — 6/6 pass, unchanged.
|
||||
|
||||
**RED-ONCE (executed).** `BackendObject_DirectVulkan::Initialize()`'s call changed to
|
||||
`if (false) InstallStateObjectDeathOps();`, rebuilt, lane re-run:
|
||||
|
||||
```
|
||||
1/8 DirectVulkan.Split.Ct.CtWireScenario.TextureDeath... ***Failed
|
||||
2/8 DirectVulkan.Spawn.Ct.CtWireScenario.TextureDeath... Passed
|
||||
4/8 DirectVulkan.Tcp.Ct.CtWireScenario.TextureDeath... Passed
|
||||
5/8 DirectVulkan.Split.Ct.CtWireScenario.FramebufferDeath... ***Failed
|
||||
6/8 DirectVulkan.Spawn.Ct.CtWireScenario.FramebufferDeath... Passed
|
||||
7/8 DirectVulkan.Tcp.Ct.CtWireScenario.FramebufferDeath... Passed
|
||||
75% tests passed, 2 tests failed out of 8
|
||||
|
||||
CtWireScenario.cpp:264: Failure
|
||||
Expected: (afterCounters.deaths) > (deathsBefore), actual: 0 vs 0
|
||||
```
|
||||
|
||||
Restored and rebuilt: 8/8 green again. This is precisely the shape package L's temporary probe
|
||||
predicted (`notes/p7/magma-two-process-first-run.md` §6): spawn and tcp were already carried by
|
||||
`InstallClientWireTables`' emitter, and **inproc was the only hole** — which is the hole this
|
||||
table closes.
|
||||
Reference in New Issue
Block a user