swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-08 20:28:32 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix, Test] (Espryt): deliver a death notice to every holder of a kind, retire the twin table's last sweep, and pin the bring-up call site under the armless knob pair

Browse Source 
- BackendSlotTable links every instance into a per-type holder list; OnFrontendObjectDestroyed resolves the handle once, drops the twin in each holder by handle and frees the slot once, last - a notice delivered to one registry left the fixture's saved copy holding a dead twin, and its driver storage, for the life of the process (review v4 minor 2)
- ReclaimDeadSlots and CollectGarbageNow leave the slot table: nothing called them, so the header's "backstop" was a claim without a caller; the registry's pre-P2 CollectGarbageNow is a no-op on the handle arm (minor 1)
- HandleOf no longer memoises a null answer, which a second holder's acquire could never refresh (minor 8)
- EnsureProcessTeardownSentinel is armed by the slot table's first insertion, as D13 says; the registry arms it only on the legacy arm (minor 9)
- new SanityTest cases: OneDeathNoticeDropsTheTwinInEveryHolderOfTheKind, ASavedCopyOfARealRegistryDropsTheTwinOnTheSameNotice, ANegativeLookupIsNotCachedAcrossAnotherHoldersAcquire, and EglBringUpUnderTheArmlessKnobPairReturnsInsteadOfStopping, which runs InitPbufferSurface under the pair in a forked child and fails naming both knobs if InitDisplayAndContext ever stops there again (minor 4)
- AnArmlessKnobCombinationStopsInsteadOfSkippingTheLane writes a per-process, per-case log path and restores MOBILEGL_LOG_FILE_PATH and MG_Config::Features through RAII guards on every exit path (minor 5)
This commit is contained in:
swung0x48
2026-09-07 23:18:09 -04:00
parent c9dd173201
commit df784c6752
4 changed files with 602 additions and 179 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectGLES/Managers.cpp
+8 -1
View File
@@ -178,9 +178,16 @@ namespace MobileGL::MG_Backend::DirectGLES {
// for all six - which is why the notice carries the kind rather than there being six
// ops tables.
//
// Each arm below names the registry GLOBAL of its kind, but DestroyByLifetimeId is
// static: it is answered by every table of that kind that exists at the moment - the
// global's own and any by-value copy a fixture or a context reset is holding - and
// the slot goes back once, after all of them have let go (SlotTables.h, the holder
// list). Naming one instance here is a spelling, not a choice of holder.
//
// 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.
// is about to hand every GPU object back anyway. That twin is a deliberate leak, not
// garbage for a later collection - there is none on this arm.
void OnFrontendStateObjectDestroyed(MG_Pipe::MGPipeKind kind, Uint64 lifetimeId) {
if (InProcessTeardown()) return;
switch (kind) {
MobileGL/MG_Backend/DirectGLES/Managers.h
+30 -16
View File
@@ -315,20 +315,27 @@ namespace MobileGL::MG_Backend::DirectGLES {
using BackendMap = UnorderedMap<StateObject*, Entry>;
using iterator = typename BackendMap::iterator;
using const_iterator = typename BackendMap::const_iterator;
#if MOBILEGL_PIPE_PUSH
using SlotTable = BackendSlotTable<StateObject, BackendObject, kKind>;
#endif
BackendPtr& GetOrCreate(const StatePtr& stateObj) {
MOBILEGL_ASSERT(stateObj != nullptr, "State object must not be null");
// Twin creation is the moment a driver-owned id starts needing a guarded
// destructor; cold path, so the once-guard costs nothing per draw. It is armed
// here, at the first insertion, on BOTH arms - a destructor hook on the table
// itself is wrong for the reason spelled out above InProcessTeardown().
EnsureProcessTeardownSentinel();
#if MOBILEGL_PIPE_PUSH
if (EsprytSlotTablesEnabled()) {
// The slot table arms the teardown sentinel itself, at its own first
// insertion (D13; SlotTables.h) - so a table used outside a registry arms
// it too, which is right: it is the twin, not the registry, that owns the
// driver id a guarded destructor exists for.
return m_slotTable.GetOrCreate(stateObj);
}
#endif
// Twin creation is the moment a driver-owned id starts needing a guarded
// destructor; cold path, so the once-guard costs nothing per draw. It is armed
// here, at the first insertion - a destructor hook on the table itself is wrong
// for the reason spelled out above InProcessTeardown().
EnsureProcessTeardownSentinel();
// Sweep BEFORE the entry reference below exists: the map is open-addressed and an
// erase relocates the rest of the probe cluster, so collecting once that reference
// is taken would invalidate it. The sweep is therefore owed from an earlier call
@@ -417,15 +424,20 @@ 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
// P2 step e2. STATIC, because a death notice is about an object and not about a
// registry instance: it is answered by EVERY table of this kind that exists - this
// registry's own, and any by-value copy of it a fixture or a context reset is holding
// (SlotTables.h explains the holder list and why one holder was a leak).
//
// 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) {
static Bool DestroyByLifetimeId(Uint64 lifetimeId) {
if (EsprytSlotTablesEnabled()) {
return m_slotTable.DestroyByLifetimeId(lifetimeId);
return SlotTable::OnFrontendObjectDestroyed(lifetimeId);
}
return false;
}
@@ -454,13 +466,13 @@ namespace MobileGL::MG_Backend::DirectGLES {
// The seven DirectGLES.cpp call sites drive the LEGACY arm and nothing else. On the
// handle arm death is announced by the frontend object's destructor
// (MG_State/GLState/StateObjectDeathNotice.h), so there is no garbage to collect on a
// tick and this is the predicted branch plus a return - which is how ROADMAP.md:18's
// "delete the GC" is delivered without deleting the legacy arm's own collector while
// that arm is still compiled beside it.
// tick, the slot table has no collector to forward to, and this is the predicted
// branch plus a return - which is how ROADMAP.md:18's "delete the GC" is delivered
// without deleting the legacy arm's own collector while that arm is still compiled
// beside it.
void CollectGarbageIfNeeded() {
#if MOBILEGL_PIPE_PUSH
if (EsprytSlotTablesEnabled()) {
m_slotTable.CollectGarbageIfNeeded();
return;
}
#endif
@@ -474,10 +486,12 @@ namespace MobileGL::MG_Backend::DirectGLES {
#endif
}
// Pre-P2 API, kept for the legacy arm. On the handle arm there is nothing it could
// collect: a twin leaves with its object's death notice, and a notice dropped during
// process teardown is a deliberate leak (SlotTables.h), not garbage awaiting a call.
void CollectGarbageNow() {
#if MOBILEGL_PIPE_PUSH
if (EsprytSlotTablesEnabled()) {
m_slotTable.CollectGarbageNow();
return;
}
#endif
MobileGL/MG_Backend/DirectGLES/SlotTables.h
+199 -101
View File
@@ -28,8 +28,8 @@
// lose their reason to exist.
// * The lookup stops being a hash probe into an open-addressed map and becomes one bounds
// check plus one array index, so a returned BackendPtr* is NOT invalidated by the next Find
// or sweep on the table. That kills the hazard Managers.h documents at length, and with it
// the by-value copy plus second Find that SyncTextureObjectToBackend paid to survive it.
// on the table. That kills the hazard Managers.h documents at length, and with it the
// by-value copy plus second Find that SyncTextureObjectToBackend paid to survive it.
// * 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.
//
@@ -37,22 +37,35 @@
// deliverable ROADMAP.md:18 spells "GC" in and the one D13 makes a precondition of the switch-
// over. All six re-keyed object classes raise MG_State::GLState::NotifyStateObjectDestroyed()
// from their destructor (BufferBackendOps' shape, one entry point for six kinds), the backend
// consumes it in Managers.cpp, and DestroyByLifetimeId() below drops the twin and returns the
// slot at the moment the frontend object's last SharedPtr goes. So:
// * there is NO draw-path tick and NO creation tick on this arm. CollectGarbageIfNeeded() is
// an empty call, and the seven call sites in DirectGLES.cpp drive the LEGACY registry only;
// consumes it in Managers.cpp, and OnFrontendObjectDestroyed() below drops the twin in EVERY
// table of the kind and returns the slot, at the moment the frontend object's last SharedPtr
// goes. So:
// * there is NO draw-path tick, NO creation tick and NO sweep of any kind on this arm. The
// seven CollectGarbageIfNeeded call sites in DirectGLES.cpp drive the LEGACY registry only;
// * a twin, and the driver storage it owns, is freed when the application lets go of the
// object rather than up to 64 creations or 1024 draw ticks later. That is what
// Managers.h's "dead gigabytes" note asked for.
//
// The weak_ptr per entry survives, and only for what it is honest about:
// * ForEachLive() hands the callee a STRONG reference to the frontend object, which the one
// direct-iteration site (ScopedDetachedTextureFramebufferAttachments) needs; and
// * ReclaimDeadSlots() is kept as the body of the EXPLICIT CollectGarbageNow(), i.e. a
// collection someone asks for, never a periodic one. It is the backstop for the one case
// the notice cannot cover: a destructor that runs after exit() has begun, where
// InProcessTeardown() drops the notice because a twin destructor must not call the driver.
// It is never an identity test - that is what Gen is for.
// EVERY HOLDER OF THE KIND, not one. Two live tables of one kind is a real configuration - the
// ScopedDirectGLESTextureBindings fixture keeps a by-value copy of the Texture registry for the
// length of a test, and a context reset does the same in reverse - and the slot allocator
// erases its lifetimeId -> slot mapping on Free, so a notice delivered to one holder and
// resolved again by the next would find nothing to resolve. Every table therefore links itself
// into a per-table-type list at construction and out at destruction, and one notice resolves
// the handle ONCE, drops the twin in each holder BY HANDLE, and frees the slot once, last. No
// holder can be left naming a live entry for a dead object, and there is nothing a sweep could
// still find. (The list is per table TYPE; the kind is the type's template parameter, and each
// of the six kinds has exactly one table type in this backend. Magma's subsystem-4 table mints
// out of its own per-renderer allocator, not MGPipeSlots(), so it is not a holder here.)
//
// The weak_ptr per entry survives for exactly one reason: ForEachLive() hands the callee a
// STRONG reference to the frontend object, which the one direct-iteration site
// (ScopedDetachedTextureFramebufferAttachments) needs. It is never an identity test - that is
// what Gen is for - and it is never read to decide whether an entry is dead: a destructor that
// runs after exit() has begun has its notice dropped by InProcessTeardown(), and that twin is
// then a DELIBERATE leak (the process is exiting, the driver reclaims the object, and a twin
// destructor must not call into a driver that may already be unloaded), not something to be
// collected later.
//
// P3+ DEBT, recorded rather than hidden: this header is under MG_Backend/ and it MINTS
// handles (MGPipeSlots().Acquire below) off a frontend SharedPtr's GetLifetimeId().
@@ -66,6 +79,11 @@ namespace MobileGL::MG_Backend::DirectGLES {
#if MOBILEGL_PIPE_PUSH
// Declared in Managers.h as well; repeated here because this header is included from it
// before that declaration, and the table below is the arming site on this arm (D13: "the
// arming site moves to the slot table's first insertion").
void EnsureProcessTeardownSentinel();
// What the two knobs add up to. Split out as a PURE function of them so a test can drive
// every combination without needing a process per combination.
enum class EsprytSlotArmVerdict {
@@ -90,6 +108,11 @@ namespace MobileGL::MG_Backend::DirectGLES {
// D14/D18 A/B is driven with, which is what ROADMAP.md:7 forbids. So bring-up only
// DIAGNOSES; the stop is raised by ResolveEsprytSlotTablesArm() at the first twin lookup,
// which happens in the test body where the harness reports it as a failure.
//
// The CALL SITE (InitDisplayAndContext in DirectGLES.cpp) is pinned by
// DirectGLESSlotTable.EglBringUpUnderTheArmlessKnobPairReturnsInsteadOfStopping, which runs
// the real bring-up entry point under the pair in a forked child: edit that site back to
// ResolveEsprytSlotTablesArm() and the case fails naming both knobs.
void DiagnoseEsprytSlotArm();
// Reads the config, logs, installs the death-notice consumer, and STOPS when the operator
@@ -101,11 +124,11 @@ namespace MobileGL::MG_Backend::DirectGLES {
// True when this process runs the {slot, gen} arm. Fixed for the life of the process: the
// two arms hold their twins in different containers, so flipping mid-run would strand them.
//
// INLINE on purpose. Every Find / GetOrCreate / HandleOf / ForEachLive / CollectGarbage*
// on the twin tables consults it, i.e. it is on the per-draw path several times per draw.
// As an out-of-line function in Managers.cpp (no LTO in any shipped configuration) that was
// a call through the PLT per lookup; here the caller sees a guard-variable load and a
// perfectly-predicted branch, and the arm dispatch folds into the caller.
// INLINE on purpose. Every Find / GetOrCreate / HandleOf / ForEachLive on the twin tables
// consults it, i.e. it is on the per-draw path several times per draw. As an out-of-line
// function in Managers.cpp (no LTO in any shipped configuration) that was a call through
// the PLT per lookup; here the caller sees a guard-variable load and a perfectly-predicted
// branch, and the arm dispatch folds into the caller.
inline Bool EsprytSlotTablesEnabled() {
static const Bool enabled = ResolveEsprytSlotTablesArm();
return enabled;
@@ -120,9 +143,10 @@ namespace MobileGL::MG_Backend::DirectGLES {
struct Entry {
BackendPtr backend;
// LIVENESS ONLY. Never compared against another object to decide identity - that is
// what Gen is for - and never dereferenced for its address. Read by
// ReclaimDeadSlots(), and locked by ForEachLive() so the callee holds a strong ref.
// LIVENESS ONLY, and only for ForEachLive(), which locks it so the callee holds a
// strong ref. Never compared against another object to decide identity - that is
// what Gen is for - never dereferenced for its address, and never read to decide
// whether the slot is dead: death is announced, not discovered.
StateWeakPtr stateRef;
// The generation this entry's twin was built for. An entry whose Gen no longer
// matches the allocator's is a twin of the slot's PREVIOUS owner.
@@ -130,6 +154,49 @@ namespace MobileGL::MG_Backend::DirectGLES {
Bool Live = false;
};
// Every constructor links the table into the per-type holder list and the destructor
// unlinks it, so a by-value copy (the ScopedDirectGLESTextureBindings fixture's saved
// registry) is a holder for exactly as long as it exists. Copy and move carry the
// ENTRIES and the memo; the links are the table's own and are never copied.
BackendSlotTable() { LinkHolder(); }
BackendSlotTable(const BackendSlotTable& other):
m_slots(other.m_slots),
m_nullTwin(other.m_nullTwin),
m_memoLifetimeId(other.m_memoLifetimeId),
m_memoHandle(other.m_memoHandle) {
LinkHolder();
}
BackendSlotTable(BackendSlotTable&& other) noexcept:
m_slots(std::move(other.m_slots)),
m_nullTwin(std::move(other.m_nullTwin)),
m_memoLifetimeId(other.m_memoLifetimeId),
m_memoHandle(other.m_memoHandle) {
other.m_slots.clear();
other.ForgetHandle();
LinkHolder();
}
BackendSlotTable& operator=(const BackendSlotTable& other) {
if (this != &other) {
m_slots = other.m_slots;
m_nullTwin = other.m_nullTwin;
m_memoLifetimeId = other.m_memoLifetimeId;
m_memoHandle = other.m_memoHandle;
}
return *this;
}
BackendSlotTable& operator=(BackendSlotTable&& other) noexcept {
if (this != &other) {
m_slots = std::move(other.m_slots);
m_nullTwin = std::move(other.m_nullTwin);
m_memoLifetimeId = other.m_memoLifetimeId;
m_memoHandle = other.m_memoHandle;
other.m_slots.clear();
other.ForgetHandle();
}
return *this;
}
~BackendSlotTable() { UnlinkHolder(); }
// Resolve-or-create. The handle comes from the client allocator keyed on the frontend
// object's lifetime id, so two calls for the same live object always land on the same
// slot, and a successor object at the same heap address never does.
@@ -144,11 +211,17 @@ namespace MobileGL::MG_Backend::DirectGLES {
// entry, so a second null call was handed the same twin the first one got.
// Resetting here instead would have destroyed it - an arm difference in the one
// path that documents relying on this. One per-table parking slot, never live,
// never swept, never handed a handle, because a null object has no identity and
// never handed a handle, because a null object has no identity and
// therefore cannot have a {slot, gen}.
return m_nullTwin;
}
// D13: the teardown sentinel is armed by the slot table's first insertion. Twin
// creation is the moment a driver-owned id starts needing a guarded destructor;
// this is the cold path, so the once-guard costs nothing per draw. On the legacy
// arm StateBackendObjectRegistry::GetOrCreate arms it itself.
EnsureProcessTeardownSentinel();
const MG_Pipe::MGPipeHandle handle =
MG_Pipe::MGPipeSlots().Acquire(kKind, stateObj->GetLifetimeId());
MOBILEGL_ASSERT(!MG_Pipe::MGPipeHandleIsNull(handle),
@@ -173,9 +246,9 @@ namespace MobileGL::MG_Backend::DirectGLES {
}
// Null when no live twin of this object exists. Unlike the registry's Find this NEVER
// mutates the table, so the returned pointer survives any later Find or sweep on it;
// only a GetOrCreate that grows the vector can move it, and callers that hold one
// across a possible insertion still copy the BackendPtr out.
// mutates the table, so the returned pointer survives any later Find on it; only a
// GetOrCreate that grows the vector can move it, and callers that hold one across a
// possible insertion still copy the BackendPtr out.
BackendPtr* Find(StateObject* stateObj) {
if (stateObj == nullptr) return nullptr;
return FindByHandle(HandleOf(stateObj));
@@ -195,91 +268,62 @@ namespace MobileGL::MG_Backend::DirectGLES {
// The handle this object's twin is keyed on, or the null handle. This is what a backend
// memo stores instead of a raw pointer, a GL name or a bare lifetime id.
//
// A NULL answer is never memoised. The memo is per table and the allocator is per
// kind, so with two holders of one kind the OTHER table can be the one that acquires;
// a cached "no handle" here would then outlive the twin's creation over there, and
// nothing on this table's own acquire path would ever refresh it. A miss costs the
// allocator probe it always cost; a hit is refreshed the moment anyone acquires.
MG_Pipe::MGPipeHandle HandleOf(const StateObject* stateObj) const {
if (stateObj == nullptr) return MG_Pipe::kMGPipeNullHandle;
const Uint64 lifetimeId = stateObj->GetLifetimeId();
if (lifetimeId == m_memoLifetimeId) return m_memoHandle;
const MG_Pipe::MGPipeHandle handle =
MG_Pipe::MGPipeSlots().FindByLifetimeId(kKind, lifetimeId);
RememberHandle(lifetimeId, handle);
if (!MG_Pipe::MGPipeHandleIsNull(handle)) RememberHandle(lifetimeId, handle);
return handle;
}
// Drop the twin of every slot whose frontend object is gone and return the slot to the
// allocator. Freeing is what makes the NEXT handout of that slot bump Gen.
void ReclaimDeadSlots() {
if (m_isCollecting) return;
m_isCollecting = true;
for (SizeT slot = 0; slot < m_slots.size(); ++slot) {
Uint32 gen = 0;
// The twin's destructor is a driver call and could, in principle, re-enter
// GetOrCreate on this table and resize m_slots. So NOTHING that outlives the
// destructor may be a reference into m_slots: the twin is moved out into a
// local, the entry is finished with, and only then is the local released.
BackendPtr dead;
{
Entry& entry = m_slots[slot];
if (!entry.Live || !entry.stateRef.expired()) continue;
gen = entry.Gen;
dead = std::move(entry.backend);
entry.backend.reset();
entry.stateRef.reset();
entry.Live = false;
}
MG_Pipe::MGPipeSlots().Free(
kKind, MG_Pipe::MGPipeHandle{static_cast<Uint32>(slot), gen});
if (m_memoHandle.Slot == static_cast<Uint32>(slot)) ForgetHandle();
dead.reset();
}
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.
// P2 step e2's backend half. The frontend object with this lifetime id has just been
// DESTROYED: resolve its handle ONCE, drop its twin in EVERY table of this type, and
// return the slot to the allocator - in that order, because the allocator forgets the
// lifetime id on Free and a holder told second could no longer resolve it.
//
// 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) {
// The slot is returned whether or not any holder still had a twin at it: the lifetime
// id is dead and MG_State never hands one out twice, so nothing can acquire it again,
// and a slot minted for it that no table holds (a table reset with `= {}` drops its
// entries without freeing) would otherwise stay allocated for the life of the process.
//
// STATIC, and deliberately so: a notice is about an object, not about a table, and
// "which table holds it" is exactly the question that produced the two-holder leak.
// Returns whether the object had a slot of this kind, i.e. whether anything was freed;
// a second call for the same id answers false because the allocator no longer maps it.
static Bool OnFrontendObjectDestroyed(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;
for (BackendSlotTable* holder = s_firstHolder; holder != nullptr;) {
// The successor is read BEFORE the release: ReleaseTwinAt runs the twin's
// destructor, which is a driver call, and nothing that outlives it may be a
// reference into this holder.
BackendSlotTable* const next = holder->m_nextHolder;
holder->ReleaseTwinAt(handle);
holder = next;
}
if (m_memoHandle.Slot == handle.Slot) ForgetHandle();
MG_Pipe::MGPipeSlots().Free(kKind, handle);
dead.reset();
return true;
}
// Deliberately EMPTY, and this is the P2 deliverable rather than an omission: on this
// arm death is announced, so there is nothing for a periodic sweep to discover. The
// seven DirectGLES.cpp call sites keep their spelling because they are the legacy
// registry's driver and that arm is still compiled beside this one; on this arm they
// cost the predicted branch in StateBackendObjectRegistry and return.
void CollectGarbageIfNeeded() {}
// An EXPLICIT collection - someone asked, so it runs. Not a driver: nothing calls this
// on a tick. It is the backstop for a notice that could not be delivered (see the
// InProcessTeardown() note in the file header) and the tests' way of forcing the
// liveness sweep without waiting for one.
void CollectGarbageNow() { ReclaimDeadSlots(); }
// How many tables of this type exist right now. For the tests that pin the holder
// list; nothing on a shipping path asks.
static Uint32 HolderCount() {
Uint32 count = 0;
for (const BackendSlotTable* holder = s_firstHolder; holder != nullptr;
holder = holder->m_nextHolder) {
++count;
}
return count;
}
// fn(const StatePtr& state, const BackendPtr& twin) over every live, still-owned entry.
// Replaces the registry's begin()/end(), whose iterator exposed the raw frontend
@@ -291,8 +335,8 @@ namespace MobileGL::MG_Backend::DirectGLES {
// Index loop and a COPIED twin, not a range-for over references: fn is arbitrary
// backend code, and a nested GetOrCreate on this table would resize m_slots and
// invalidate both the iterator and any reference into the vector that outlives the
// call. ReclaimDeadSlots walks by index for the same reason. The one caller today
// happens not to insert; that is not a property the walk should depend on.
// call. The one caller today happens not to insert; that is not a property the
// walk should depend on.
for (SizeT slot = 0; slot < m_slots.size(); ++slot) {
const Entry& entry = m_slots[slot];
if (!entry.Live || !entry.backend) continue;
@@ -312,6 +356,32 @@ namespace MobileGL::MG_Backend::DirectGLES {
}
private:
// Drop the twin at `handle` if THIS table holds it. Frees nothing: the slot belongs to
// the kind, not to the table, and OnFrontendObjectDestroyed returns it once, after
// every holder has let go.
Bool ReleaseTwinAt(MG_Pipe::MGPipeHandle handle) {
// Forget the memo whenever it names this slot, even if this table has no entry
// there: a memo can be a handle learned from the allocator for an object another
// holder twinned, and it must not survive the slot's next handout.
if (m_memoHandle.Slot == handle.Slot) ForgetHandle();
if (handle.Slot >= m_slots.size()) return false;
// The twin's destructor is a driver call and could, in principle, re-enter
// GetOrCreate on this table and resize m_slots. So NOTHING that outlives the
// destructor may be a reference into m_slots: the twin is moved out into a local,
// the entry is finished with, and only then is the local released.
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;
}
dead.reset();
return true;
}
Entry& EntryAt(Uint32 slot) {
if (slot >= m_slots.size()) m_slots.resize(static_cast<SizeT>(slot) + 1);
return m_slots[slot];
@@ -326,10 +396,36 @@ namespace MobileGL::MG_Backend::DirectGLES {
m_memoHandle = MG_Pipe::kMGPipeNullHandle;
}
// The holder list: intrusive and doubly linked, so registering and unregistering are
// two pointer writes with no allocation, and its head is a constant-initialised
// static - which is what lets the process-lifetime registry globals in Managers.cpp
// link themselves in from their own constructors with no initialisation-order
// question to answer. Single-threaded, like every table it links (the tables live and
// die on the context thread, as the notice they answer does).
void LinkHolder() {
m_prevHolder = nullptr;
m_nextHolder = s_firstHolder;
if (s_firstHolder != nullptr) s_firstHolder->m_prevHolder = this;
s_firstHolder = this;
}
void UnlinkHolder() {
if (m_prevHolder != nullptr) {
m_prevHolder->m_nextHolder = m_nextHolder;
} else {
s_firstHolder = m_nextHolder;
}
if (m_nextHolder != nullptr) m_nextHolder->m_prevHolder = m_prevHolder;
m_prevHolder = nullptr;
m_nextHolder = nullptr;
}
static inline BackendSlotTable* s_firstHolder = nullptr;
BackendSlotTable* m_prevHolder = nullptr;
BackendSlotTable* m_nextHolder = nullptr;
// Indexed by MGPipeHandle::Slot; [0] is the reserved slot and is never live.
Vector<Entry> m_slots;
Bool m_isCollecting = false;
// Handed back by GetOrCreate for a null state object. Never live, never swept.
// Handed back by GetOrCreate for a null state object. Never live, never handed a handle.
BackendPtr m_nullTwin;
// ONE-entry resolution memo, lifetimeId -> handle. It exists because without it every
@@ -345,13 +441,15 @@ namespace MobileGL::MG_Backend::DirectGLES {
// a per-unit memo and a direct-mapped 6-slot array there). Making the memo per-unit /
// per-target is the fix, and G11 - the device-side gate that would price it - is owed.
//
// It cannot serve a stale answer, by two independent arguments:
// It cannot serve a stale answer, by three independent arguments:
// * the key is a lifetime id, which MG_State never hands out twice, so a recycled
// heap address cannot hit this memo the way it could hit an address-keyed one; and
// heap address cannot hit this memo the way it could hit an address-keyed one;
// * a null answer is never stored, so another holder's acquire cannot be hidden by
// a "no handle" this table remembered earlier; and
// * even a hit for a slot that has since been freed and re-handed is caught, because
// the caller resolves the handle through FindByHandle, which compares Gen.
// Cleared anyway when the sweep frees the memoised slot. 0 is never a live lifetime id
// (MG_State's counters start at 1), so a zeroed memo is a guaranteed miss.
// Cleared anyway when a death notice names the memoised slot. 0 is never a live
// lifetime id (MG_State's counters start at 1), so a zeroed memo is a guaranteed miss.
mutable Uint64 m_memoLifetimeId = 0;
mutable MG_Pipe::MGPipeHandle m_memoHandle = MG_Pipe::kMGPipeNullHandle;
};
MobileGL/MG_Test/SanityTest.cpp
+365 -61
View File
@@ -36,6 +36,7 @@
#include <MG_Util/ShaderTranspiler/CompileEnv.h>
#include <MG_Util/ShaderTranspiler/ShaderSourceProcessor.h>
#include <MG_Util/Debug/Log.h>
#include <MG_Util/BackendLoaders/OpenGL/Loader.h>
#include <MG_Util/Types.h>
#include <Config.h>
#include <MG_Pipe/MGPipe.h>
@@ -46,8 +47,14 @@
#include <MG_State/GLState/TextureState/TextureObject2D.h>
#include <MG_State/GLState/VertexArrayState/VertexArrayObject.h>
#include <csignal>
#include <chrono>
#include <limits>
#include <set>
#if defined(_WIN32)
#include <process.h>
#else
#include <unistd.h>
#endif
#if MOBILEGL_PIPE_PUSH
namespace {
@@ -3215,6 +3222,86 @@ namespace {
BackendSlotTable<FakeStateObject, FakeBackendObject, MobileGL::MG_Pipe::MGPipeKind::Query>;
using FakeSharedKindSlotTable = MobileGL::MG_Backend::DirectGLES::
BackendSlotTable<FakeStateObject, FakeBackendObject, MobileGL::MG_Pipe::MGPipeKind::Fence>;
// 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.
std::filesystem::path UniqueScratchLogPath(const char* stem) {
static int counter = 0;
#if defined(_WIN32)
const long pid = static_cast<long>(_getpid());
#else
const long pid = static_cast<long>(::getpid());
#endif
const auto ticks = std::chrono::steady_clock::now().time_since_epoch().count();
return std::filesystem::temp_directory_path() /
(std::string(stem) + "-" + std::to_string(pid) + "-" + std::to_string(++counter) +
"-" + std::to_string(ticks) + ".log");
}
// Points MobileGL's file log at `path` for the life of the guard and puts back whatever the
// operator had - the previous MOBILEGL_LOG_FILE_PATH, or none - on EVERY exit path,
// including a failed ASSERT. The log is closed on both sides of the switch, because Log.cpp
// reads the variable only when it opens the file.
struct ScopedLogFileRedirect {
explicit ScopedLogFileRedirect(const std::filesystem::path& path): m_path(path) {
if (const char* previous = std::getenv("MOBILEGL_LOG_FILE_PATH")) {
m_hadPrevious = true;
m_previous = previous;
}
std::filesystem::remove(m_path);
MobileGL::MG_Util::Debug::Close();
SetEnvVar("MOBILEGL_LOG_FILE_PATH", m_path.string().c_str());
}
~ScopedLogFileRedirect() {
MobileGL::MG_Util::Debug::Close();
if (m_hadPrevious) {
SetEnvVar("MOBILEGL_LOG_FILE_PATH", m_previous.c_str());
} else {
UnsetEnvVar("MOBILEGL_LOG_FILE_PATH");
}
std::error_code ignored;
std::filesystem::remove(m_path, ignored);
}
ScopedLogFileRedirect(const ScopedLogFileRedirect&) = delete;
ScopedLogFileRedirect& operator=(const ScopedLogFileRedirect&) = delete;
// Everything written so far. Closes the log first so the last line is on disk.
std::string Contents() const {
MobileGL::MG_Util::Debug::Close();
std::ifstream logFile(m_path);
if (!logFile.good()) return {};
return std::string(std::istreambuf_iterator<char>(logFile), std::istreambuf_iterator<char>());
}
private:
std::filesystem::path m_path;
bool m_hadPrevious = false;
std::string m_previous;
};
// Sets the two knobs into the combination that leaves no twin-table arm at all -
// kMGPipeSubsystemEsprytSlots clear and PipeLegacyMemos false, which is what
// MOBILEGL_PIPE_PUSH=0 MOBILEGL_PIPE_LEGACY_MEMOS=0 in the environment produces - and
// restores MG_Config::Features on every exit path.
struct ScopedArmlessKnobPair {
ScopedArmlessKnobPair():
m_savedPush(MobileGL::MG_Config::Features.PipePush),
m_savedLegacy(MobileGL::MG_Config::Features.PipeLegacyMemos) {
MobileGL::MG_Config::Features.PipePush =
m_savedPush & ~MobileGL::MG_Pipe::kMGPipeSubsystemEsprytSlots;
MobileGL::MG_Config::Features.PipeLegacyMemos = false;
}
~ScopedArmlessKnobPair() {
MobileGL::MG_Config::Features.PipePush = m_savedPush;
MobileGL::MG_Config::Features.PipeLegacyMemos = m_savedLegacy;
}
ScopedArmlessKnobPair(const ScopedArmlessKnobPair&) = delete;
ScopedArmlessKnobPair& operator=(const ScopedArmlessKnobPair&) = delete;
private:
MobileGL::Uint64 m_savedPush;
MobileGL::Bool m_savedLegacy;
};
} // namespace
// The property the whole slice exists for. The pre-P2 registry keyed twins on the frontend heap
@@ -3233,9 +3320,11 @@ TEST(DirectGLESSlotTable, ARecycledSlotIsANewHandleAndTheStaleOneResolvesToNothi
ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(firstHandle));
EXPECT_EQ(table.LiveCount(), 1u);
// The frontend object dies and the slot is reclaimed - which is the only moment Gen moves.
// The frontend object dies and announces it (FakeStateObject is not one of the six re-keyed
// classes, so the notice its destructor would raise is raised by hand), and the slot is
// reclaimed - which is the only moment Gen moves.
first.reset();
table.CollectGarbageNow();
EXPECT_TRUE(FakeSlotTable::OnFrontendObjectDestroyed(0xA1u));
EXPECT_EQ(table.LiveCount(), 0u);
EXPECT_EQ(table.FindByHandle(firstHandle), nullptr)
<< "a handle whose object is gone still resolved to a twin";
@@ -3254,6 +3343,9 @@ TEST(DirectGLESSlotTable, ARecycledSlotIsANewHandleAndTheStaleOneResolvesToNothi
EXPECT_EQ(table.FindByHandle(firstHandle), nullptr);
ASSERT_NE(table.FindByHandle(secondHandle), nullptr);
EXPECT_EQ((*table.FindByHandle(secondHandle))->marker, 2);
second.reset();
EXPECT_TRUE(FakeSlotTable::OnFrontendObjectDestroyed(0xA2u));
}
// Gen moves on reuse and ONLY on reuse: a live object that is looked up again, or respecified,
@@ -3263,16 +3355,12 @@ TEST(DirectGLESSlotTable, RepeatedLookupsOfALiveObjectKeepOneHandle) {
FakeSlotTable table;
auto object = MakeShared<FakeStateObject>(0xB1u);
// HandleOf caches whatever the allocator answered, INCLUDING the null handle - an object
// that is bound but never synced has no twin, and re-probing the hash for it every draw is
// exactly what the memo exists to avoid. What makes that safe is that GetOrCreate refreshes
// the memo, so a cached "no handle" can never outlive the twin's creation. Nothing pinned
// that; this does.
// An object that is bound but never synced has no twin and no handle; asking is a miss.
EXPECT_TRUE(MG_Pipe::MGPipeHandleIsNull(table.HandleOf(object.get())));
table.GetOrCreate(object) = MakeShared<FakeBackendObject>();
const MG_Pipe::MGPipeHandle handle = table.HandleOf(object.get());
ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(handle))
<< "the memo went on answering the null handle it cached before the twin existed";
<< "the memo went on answering a null handle after the twin was created";
for (int i = 0; i < 8; ++i) {
auto* slot = table.GetOrCreate(object) ? table.Find(object.get()) : nullptr;
@@ -3280,11 +3368,46 @@ TEST(DirectGLESSlotTable, RepeatedLookupsOfALiveObjectKeepOneHandle) {
EXPECT_TRUE(table.HandleOf(object.get()) == handle) << "handle moved on lookup " << i;
}
EXPECT_EQ(table.LiveCount(), 1u);
object.reset();
EXPECT_TRUE(FakeSlotTable::OnFrontendObjectDestroyed(0xB1u));
}
// The round-4 review's minor 8: HandleOf used to memoise a NULL answer, on the argument that
// GetOrCreate refreshes the memo. That holds for ONE table. The allocator is per kind and the
// memo is per table, so once a second holder of the kind can be the one that acquires, the first
// table's cached "no handle" outlives the twin's creation and nothing on its own acquire path
// ever corrects it - every Find through it is a miss for a twin that exists. This is that
// configuration, and it must resolve.
TEST(DirectGLESSlotTable, ANegativeLookupIsNotCachedAcrossAnotherHoldersAcquire) {
using namespace MobileGL;
FakeSharedKindSlotTable first;
FakeSharedKindSlotTable second;
auto object = MakeShared<FakeStateObject>(0xB2u);
// `first` asks before anyone has acquired: a miss, which must NOT be remembered.
EXPECT_TRUE(MG_Pipe::MGPipeHandleIsNull(first.HandleOf(object.get())));
// `second` is the holder that acquires.
second.GetOrCreate(object) = MakeShared<FakeBackendObject>();
const MG_Pipe::MGPipeHandle handle = second.HandleOf(object.get());
ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(handle));
// `first` never acquired, so nothing on its own path refreshed its memo; it must still
// answer the handle the kind now has for this object.
EXPECT_TRUE(first.HandleOf(object.get()) == handle)
<< "the first holder kept answering the null handle it cached before the second holder "
"acquired, so every lookup through it misses a twin that exists";
object.reset();
EXPECT_TRUE(FakeSharedKindSlotTable::OnFrontendObjectDestroyed(0xB2u));
}
// The lookup does not mutate the table, which is what lets SyncTextureObjectToBackend stop paying
// a by-value copy plus a second Find to survive the registry's erase-inside-Find. A dead entry
// stays put until the sweep, and a live entry's pointer is unaffected by looking up anything else.
// a by-value copy plus a second Find to survive the registry's erase-inside-Find. An entry whose
// object has gone stays put until the death notice arrives, and a live entry's pointer is
// unaffected by looking up anything else.
TEST(DirectGLESSlotTable, FindNeverMutatesTheTable) {
using namespace MobileGL;
@@ -3298,35 +3421,58 @@ TEST(DirectGLESSlotTable, FindNeverMutatesTheTable) {
ASSERT_NE(keptSlot, nullptr);
const FakeBackendObject* keptTwin = keptSlot->get();
// The object goes but its notice is deliberately withheld for a moment, so that whatever
// changes between here and the notice is Find's doing. The registry's Find would have
// erased the expired entry here and relocated the rest of the probe cluster, invalidating
// keptSlot. This one answers what it answers and touches nothing.
doomed.reset();
// The registry's Find would have erased the expired entry here and relocated the rest of the
// probe cluster, invalidating keptSlot. This one answers null and touches nothing.
EXPECT_EQ(table.Find(kept.get()), keptSlot);
EXPECT_EQ(table.LiveCount(), 2u) << "Find reclaimed a slot; only the sweep may do that";
EXPECT_EQ(table.LiveCount(), 2u) << "Find reclaimed a slot; only a death notice may do that";
EXPECT_EQ(keptSlot->get(), keptTwin);
table.CollectGarbageNow();
EXPECT_TRUE(FakeSlotTable::OnFrontendObjectDestroyed(0xC2u));
EXPECT_EQ(table.LiveCount(), 1u);
EXPECT_EQ(table.Find(kept.get())->get(), keptTwin);
kept.reset();
EXPECT_TRUE(FakeSlotTable::OnFrontendObjectDestroyed(0xC1u));
}
// ScopedDirectGLESTextureBindings saves a whole twin table by value, resets it with `= {}` and
// restores it. The slot table has to keep that shape or the fixture stops isolating anything.
// restores it. The slot table has to keep that shape or the fixture stops isolating anything -
// and the saved copy is a HOLDER for as long as it exists, so a death announced while it is
// held reaches it too (the round-4 review's minor 2, in the fixture's own shape).
TEST(DirectGLESSlotTable, AWholeTableSavesResetsAndRestores) {
using namespace MobileGL;
const Uint32 holdersBefore = FakeSlotTable::HolderCount();
FakeSlotTable table;
auto object = MakeShared<FakeStateObject>(0xD1u);
table.GetOrCreate(object) = MakeShared<FakeBackendObject>();
(*table.Find(object.get()))->marker = 7;
const MG_Pipe::MGPipeHandle handle = table.HandleOf(object.get());
const FakeSlotTable saved = table;
EXPECT_EQ(FakeSlotTable::HolderCount(), holdersBefore + 2u)
<< "the by-value copy did not register as a holder";
table = {};
EXPECT_EQ(FakeSlotTable::HolderCount(), holdersBefore + 2u)
<< "the reset changed the holder count - a temporary's registration leaked or the "
"table's own was lost";
EXPECT_EQ(table.Find(object.get()), nullptr) << "the reset left the twin reachable";
table = saved;
ASSERT_NE(table.Find(object.get()), nullptr);
EXPECT_EQ((*table.Find(object.get()))->marker, 7);
// The object dies while BOTH the working table and the saved copy hold its twin. One
// notice, and neither may keep a live entry.
object.reset();
EXPECT_TRUE(FakeSlotTable::OnFrontendObjectDestroyed(0xD1u));
EXPECT_EQ(table.FindByHandle(handle), nullptr);
EXPECT_EQ(table.LiveCount(), 0u);
EXPECT_EQ(saved.LiveCount(), 0u)
<< "the saved copy kept the dead object's twin - the notice reached one holder only";
}
// The whole point of routing every twin through the client allocator: a table that keeps its own
@@ -3365,27 +3511,76 @@ TEST(DirectGLESSlotTable, TwoTablesOfTheSameKindShareOneSlotAndKeepTheirOwnTwin)
EXPECT_EQ((*a.FindByHandle(handle))->marker, 1);
EXPECT_EQ((*b.FindByHandle(handle))->marker, 2);
// TWO HOLDERS OF ONE SLOT is a real configuration, not a test artefact, and this is where
// the sharp edge is: whichever holder sweeps (or is told of the death) first returns the
// slot to the allocator while the other still names it. The allocator makes that SAFE -
// Free is generation-guarded and idempotent, and FindByHandle's Gen compare turns the
// other holder's now-stale handle into a miss - but not free: if the object is still alive
// the next resolution re-Acquires it onto a NEW slot, orphaning the first table's entry
// until its own sweep. Two such configurations exist or are landing: the
// TWO HOLDERS OF ONE SLOT is a real configuration, not a test artefact (the
// ScopedDirectGLESTextureBindings fixture above holds a second live table of kind Texture
// for the length of a test, and package D's subsystem 4 re-keys VaoDrawMemo out of this
// same per-kind allocator. Flagged for the integrator rather than defended here, because
// the fix (a refcounted slot-ownership token) belongs with whoever owns both holders.
//
// The cleanup below is what keeps that out of the OTHER cases: object first, then both
// tables, so the slot is back on the free list and this kind's LiveCount is where it was.
// for the length of a test), and the death of the object is what makes it sharp: the
// allocator forgets the lifetime id on Free, so a notice delivered to ONE holder leaves
// the other with a live entry - and its twin's driver storage - that nothing can resolve
// and nothing sweeps. OneDeathNoticeDropsTheTwinInEveryHolderOfTheKind below is the case
// for that; here the cleanup only has to leave the kind's LiveCount where it was.
object.reset();
a.CollectGarbageNow();
b.CollectGarbageNow();
EXPECT_TRUE(FakeSharedKindSlotTable::OnFrontendObjectDestroyed(0xE1u));
EXPECT_EQ(slots.LiveCount(kKind), liveBefore)
<< "the shared slot outlived both holders and the object";
}
// The round-4 review's minor 2, closed: one notice, EVERY holder. Before this the dispatcher
// told one table per kind and DestroyByLifetimeId freed only a slot THIS table held, so with
// the sweep retired the second holder kept a live entry, and the twin, for the life of the
// process. Three holders here - two independent tables and a by-value copy, which is exactly
// what the fixture makes - and a fourth that never twinned the object and must be untouched.
TEST(DirectGLESSlotTable, OneDeathNoticeDropsTheTwinInEveryHolderOfTheKind) {
using namespace MobileGL;
constexpr MG_Pipe::MGPipeKind kKind = MG_Pipe::MGPipeKind::Fence;
auto& slots = MG_Pipe::MGPipeSlots();
const Uint32 liveBefore = slots.LiveCount(kKind);
const Uint32 holdersBefore = FakeSharedKindSlotTable::HolderCount();
FakeSharedKindSlotTable a;
FakeSharedKindSlotTable b;
FakeSharedKindSlotTable bystander;
auto object = MakeShared<FakeStateObject>(0xE2u);
auto uninvolved = MakeShared<FakeStateObject>(0xE3u);
a.GetOrCreate(object) = MakeShared<FakeBackendObject>();
b.GetOrCreate(object) = MakeShared<FakeBackendObject>();
bystander.GetOrCreate(uninvolved) = MakeShared<FakeBackendObject>();
const FakeSharedKindSlotTable copyOfA = a; // the fixture's saved registry
EXPECT_EQ(FakeSharedKindSlotTable::HolderCount(), holdersBefore + 4u);
const MG_Pipe::MGPipeHandle handle = a.HandleOf(object.get());
ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(handle));
// Observers on the twins, so "dropped" means destroyed and not merely unreachable.
const std::weak_ptr<FakeBackendObject> twinA = *a.FindByHandle(handle);
const std::weak_ptr<FakeBackendObject> twinB = *b.FindByHandle(handle);
EXPECT_EQ(slots.LiveCount(kKind), liveBefore + 2u);
// ONE notice. The object is still alive so that nothing but the notice can be at work.
EXPECT_TRUE(FakeSharedKindSlotTable::OnFrontendObjectDestroyed(object->GetLifetimeId()));
EXPECT_EQ(a.FindByHandle(handle), nullptr) << "holder a kept the twin";
EXPECT_EQ(b.FindByHandle(handle), nullptr) << "holder b kept the twin";
EXPECT_EQ(copyOfA.LiveCount(), 0u) << "the by-value copy kept the twin";
EXPECT_EQ(a.LiveCount(), 0u);
EXPECT_EQ(b.LiveCount(), 0u);
EXPECT_TRUE(twinA.expired()) << "a's twin is unreachable but still allocated";
EXPECT_TRUE(twinB.expired()) << "b's twin is unreachable but still allocated";
EXPECT_EQ(slots.LiveCount(kKind), liveBefore + 1u) << "the slot was not returned exactly once";
EXPECT_FALSE(FakeSharedKindSlotTable::OnFrontendObjectDestroyed(object->GetLifetimeId()))
<< "a second notice for the same object found a slot to free";
// The holder that never twinned the object is exactly as it was.
EXPECT_EQ(bystander.LiveCount(), 1u);
ASSERT_NE(bystander.Find(uninvolved.get()), nullptr);
object.reset();
uninvolved.reset();
EXPECT_TRUE(FakeSharedKindSlotTable::OnFrontendObjectDestroyed(0xE3u));
EXPECT_EQ(bystander.LiveCount(), 0u);
EXPECT_EQ(slots.LiveCount(kKind), liveBefore);
}
// The sweep and both of its drivers are RETIRED on this arm (ROADMAP.md:18's "delete the GC"),
// and this is the property that replaces them. The registry this table replaces learned of a
// death only by finding an expired weak_ptr, so it needed a 1024-call draw tick AND a
@@ -3412,7 +3607,7 @@ TEST(DirectGLESSlotTable, AnnouncedDeathKeepsObjectChurnFromAccumulatingWithoutA
// classes really do fire it is EveryReKeyedObjectClassAnnouncesItsOwnDeath below, and
// that the registries answer it per kind is
// EverySwitchedOverKindResolvesItsTwinThroughTheHandleArm.
EXPECT_TRUE(table.DestroyByLifetimeId(object->GetLifetimeId()));
EXPECT_TRUE(FakeSlotTable::OnFrontendObjectDestroyed(object->GetLifetimeId()));
}
EXPECT_EQ(peakLive, 1u)
@@ -3472,20 +3667,34 @@ TEST(DirectGLESSlotTable, AnAnnouncedDeathReturnsTheSlotWithoutASweep) {
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()));
// The object is STILL ALIVE here, which is the point: there is no weak_ptr test anywhere
// that could fire, so anything that changes is the notice's doing and nothing else's.
EXPECT_TRUE(FakeSlotTable::OnFrontendObjectDestroyed(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()));
// Idempotent: the allocator no longer maps the id, so a repeated notice frees nothing and
// says so - and it says so for every holder, since the notice is about the object and not
// about a table.
EXPECT_FALSE(FakeSlotTable::OnFrontendObjectDestroyed(object->GetLifetimeId()));
// A slot minted for an object that NO table holds any more - the fixture's `table = {}`
// drops entries without freeing - still goes back when the object dies: the id is dead and
// cannot be acquired again, so keeping the slot would be the process-lifetime leak the
// review named.
auto orphaned = MakeShared<FakeStateObject>(0x1E2A0002ull);
{
FakeSlotTable transient;
transient.GetOrCreate(orphaned) = MakeShared<FakeBackendObject>();
}
EXPECT_EQ(slots.LiveCount(MG_Pipe::MGPipeKind::Query), liveBefore + 1u);
orphaned.reset();
EXPECT_TRUE(FakeSlotTable::OnFrontendObjectDestroyed(0x1E2A0002ull))
<< "a slot no holder had an entry for was left allocated";
EXPECT_EQ(slots.LiveCount(MG_Pipe::MGPipeKind::Query), liveBefore);
}
// The firing side of e2, on ALL SIX re-keyed object classes - the round-3 review's MAJOR 3.
@@ -3670,6 +3879,39 @@ TEST(DirectGLESSlotTable, EverySwitchedOverKindResolvesItsTwinThroughTheHandleAr
[] { return MakeShared<VertexArrayObject>(0u); });
}
// The two-holder fix, end to end through the REAL Texture registry and the REAL destructor:
// a by-value copy of TextureImpl::g_backendTextureObjects - which is precisely what
// ScopedDirectGLESTextureBindings keeps in `previousRegistry` for the length of a test - must
// drop the twin on the same notice the registry global does, with no sweep and no second call.
TEST(DirectGLESSlotTable, ASavedCopyOfARealRegistryDropsTheTwinOnTheSameNotice) {
using namespace MobileGL;
using namespace MobileGL::MG_Backend::DirectGLES;
using namespace MobileGL::MG_State::GLState;
if (!EsprytSlotTablesEnabled()) {
GTEST_SKIP() << "the legacy arm keys twins on the frontend heap address and cannot "
"answer a death notice";
}
auto& registry = TextureImpl::g_backendTextureObjects;
SharedPtr<ITextureObject> texture = MakeShared<TextureObject2D>(0u);
auto& twin = registry.GetOrCreate(texture);
(void)twin;
const MG_Pipe::MGPipeHandle handle = registry.HandleOf(texture.get());
ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(handle));
// The fixture's shape: copy the registry while the twin is live.
auto saved = registry;
ASSERT_NE(saved.FindByHandle(handle), nullptr) << "the copy did not carry the live entry";
// The real destructor raises the real notice; nothing else runs.
texture.reset();
EXPECT_EQ(registry.FindByHandle(handle), nullptr) << "the registry global kept the twin";
EXPECT_EQ(saved.FindByHandle(handle), nullptr)
<< "the saved copy kept the dead texture's twin - the dispatcher told one holder only";
EXPECT_FALSE(registry.DestroyByLifetimeId(0)) << "sanity: a null id frees nothing";
}
// The gate on MAJOR 1 of the round-3 review. Commit d89fb684 raised
// Fatal{PipeLegacyMemosDisabled} from inside InitDisplayAndContext(), i.e. from inside EGL
// bring-up - and the integration harness pre-flights EGL bring-up in a FORKED CHILD, converting
@@ -3697,20 +3939,18 @@ TEST(DirectGLESSlotTable, AnArmlessKnobCombinationStopsInsteadOfSkippingTheLane)
EXPECT_EQ(MG_Backend::DirectGLES::ClassifyEsprytSlotArm(false, true), EsprytSlotArmVerdict::Legacy);
EXPECT_EQ(MG_Backend::DirectGLES::ClassifyEsprytSlotArm(false, false), EsprytSlotArmVerdict::NoArm);
const Uint64 savedPush = MG_Config::Features.PipePush;
const Bool savedLegacy = MG_Config::Features.PipeLegacyMemos;
MG_Config::Features.PipePush = savedPush & ~MG_Pipe::kMGPipeSubsystemEsprytSlots;
MG_Config::Features.PipeLegacyMemos = false;
// Both guards restore on every exit path - a failed ASSERT included - so no later case in
// this binary runs on a mutated config or without the operator's file log, and the log
// path is unique per process and per case (the round-4 review's minor 5).
const ScopedArmlessKnobPair knobs;
ASSERT_EQ(MG_Backend::DirectGLES::CurrentEsprytSlotArmVerdict(), EsprytSlotArmVerdict::NoArm);
const fs::path logPath = fs::temp_directory_path() / "mobilegl-espryt-armless-knobs.log";
fs::remove(logPath);
MG_Util::Debug::Close();
SetEnvVar("MOBILEGL_LOG_FILE_PATH", logPath.string().c_str());
const ScopedLogFileRedirect log(UniqueScratchLogPath("mobilegl-espryt-armless-knobs"));
// Bring-up's half of the split. It must NAME the knobs and it must RETURN: this call is the
// one InitDisplayAndContext() makes, and it runs inside the harness's forked pre-flight
// child. If it ever stops again, this line takes the whole binary down and the case is red.
// (That the call SITE still makes this call and not the stopping one is
// EglBringUpUnderTheArmlessKnobPairReturnsInsteadOfStopping below.)
MG_Backend::DirectGLES::DiagnoseEsprytSlotArm();
#if !defined(_WIN32)
@@ -3721,19 +3961,9 @@ TEST(DirectGLESSlotTable, AnArmlessKnobCombinationStopsInsteadOfSkippingTheLane)
::testing::KilledBySignal(SIGABRT), "");
#endif
MG_Util::Debug::Close();
UnsetEnvVar("MOBILEGL_LOG_FILE_PATH");
MG_Config::Features.PipePush = savedPush;
MG_Config::Features.PipeLegacyMemos = savedLegacy;
std::string contents;
{
std::ifstream logFile(logPath);
ASSERT_TRUE(logFile.good()) << "neither the diagnosis nor the fatal wrote a line an "
"operator could read";
contents.assign(std::istreambuf_iterator<char>(logFile), std::istreambuf_iterator<char>());
}
fs::remove(logPath);
const std::string contents = log.Contents();
ASSERT_FALSE(contents.empty()) << "neither the diagnosis nor the fatal wrote a line an "
"operator could read";
EXPECT_NE(contents.find("PipeLegacyMemosDisabled"), std::string::npos) << contents;
EXPECT_NE(contents.find("MOBILEGL_PIPE_PUSH"), std::string::npos) << contents;
@@ -3746,6 +3976,64 @@ TEST(DirectGLESSlotTable, AnArmlessKnobCombinationStopsInsteadOfSkippingTheLane)
#endif // MOBILEGL_PIPE_LEGACY_MEMOS
}
// The round-4 review's minor 4: the case above pins the two FUNCTIONS, and nothing failed if
// InitDisplayAndContext() (DirectGLES.cpp) was edited back to call the stopping one - which is
// exactly the regression that produced the round-3 major. This pins the CALL SITE, by running
// the real bring-up entry point under the armless pair.
//
// No display is needed: InitDisplayAndContext's twin-arm call is its first statement after
// the context teardown, ahead of eglGetDisplay, so an EGL table whose eglGetDisplay answers
// EGL_NO_DISPLAY takes bring-up through that call and straight back out with `false`. The
// child must then EXIT with the code below. If the site stops again it dies of SIGABRT
// instead, and the death test fails - naming both knobs - rather than skipping: in the
// integration harness that same abort is what turned into a green lane that ran nothing.
//
// Not skipped in a build without the legacy arm either: there the verdict is Handles and
// bring-up has nothing to diagnose, but it must still return, and this says so.
TEST(DirectGLESSlotTable, EglBringUpUnderTheArmlessKnobPairReturnsInsteadOfStopping) {
#if defined(_WIN32)
GTEST_SKIP() << "needs a forked death test";
#else
using namespace MobileGL;
constexpr int kBringUpReturnedFalse = 0x51;
constexpr int kBringUpReturnedTrue = 0x52;
// The redirect is set up in the PARENT: the child inherits the environment and writes the
// file, the parent reads it once the child has gone, and the guard restores the operator's
// log path either way.
const ScopedLogFileRedirect log(UniqueScratchLogPath("mobilegl-espryt-armless-bringup"));
EXPECT_EXIT(
{
const ScopedArmlessKnobPair knobs;
MG_External::EGLFunctionsTable egl{};
egl.eglGetDisplay = +[](EGLNativeDisplayType) -> EGLDisplay { return EGL_NO_DISPLAY; };
MG_Backend::DirectGLES::SetEGLFuncsTable(egl);
const Bool ok = MG_Backend::DirectGLES::InitPbufferSurface(1, 1);
MG_Util::Debug::Close();
std::exit(ok ? kBringUpReturnedTrue : kBringUpReturnedFalse);
},
::testing::ExitedWithCode(kBringUpReturnedFalse), "")
<< "EGL bring-up under MOBILEGL_PIPE_PUSH with kMGPipeSubsystemEsprytSlots clear and "
"MOBILEGL_PIPE_LEGACY_MEMOS=0 did not RETURN: InitDisplayAndContext() is stopping "
"on the armless knob pair again instead of diagnosing it, and the integration "
"harness's forked pre-flight turns that stop into a lane that skips every scenario";
#if MOBILEGL_PIPE_LEGACY_MEMOS
// And it diagnosed, by name, on the way through - the operator is told which two knobs
// they set before the first draw - without a Fatal{} anywhere in bring-up.
const std::string contents = log.Contents();
EXPECT_NE(contents.find("PipeLegacyMemosDisabled"), std::string::npos)
<< "bring-up returned but did not diagnose the armless pair: " << contents;
EXPECT_NE(contents.find("MOBILEGL_PIPE_PUSH"), std::string::npos) << contents;
EXPECT_NE(contents.find("MOBILEGL_PIPE_LEGACY_MEMOS=0"), std::string::npos) << contents;
EXPECT_EQ(contents.find("Fatal{"), std::string::npos)
<< "bring-up wrote a Fatal{} - the stop is back inside EGL bring-up: " << contents;
#endif
#endif
}
#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
@@ -3801,4 +4089,20 @@ TEST(DirectGLESSlotTable, AnArmlessKnobCombinationStopsInsteadOfSkippingTheLane)
TEST(DirectGLESSlotTable, EverySwitchedOverKindResolvesItsTwinThroughTheHandleArm) {
GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH";
}
TEST(DirectGLESSlotTable, ANegativeLookupIsNotCachedAcrossAnotherHoldersAcquire) {
GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH";
}
TEST(DirectGLESSlotTable, OneDeathNoticeDropsTheTwinInEveryHolderOfTheKind) {
GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH";
}
TEST(DirectGLESSlotTable, ASavedCopyOfARealRegistryDropsTheTwinOnTheSameNotice) {
GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH";
}
TEST(DirectGLESSlotTable, EglBringUpUnderTheArmlessKnobPairReturnsInsteadOfStopping) {
GTEST_SKIP() << "the {slot, gen} twin table is compiled only under MOBILEGL_PIPE_PUSH";
}
#endif // MOBILEGL_PIPE_PUSH