mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-18 00:58:30 +09:00
[Fix, Test] (MG_Pipe, clientfb, clientsp): retire every emitter's entry at the object's death - the six death helpers freed the slot and told no emitter, so a dead-but-unrecycled texture handle still resolved to the freed ITextureObject* (the allocator's generation moves only at the next hand-out) and the drain list kept the level: glTexImage2D; glDeleteTextures; <any verb> called a virtual on freed memory from the next validate point (final review C-2); the helpers now forward to the texture, renderbuffer, framebuffer, sampler-view and shader-CSO emitters between the wire delete and the free, ResolveTexture refuses a dead slot loudly on IsLive, the sticky-mask producers stamp the generation they write under, and the delete-then-use sequence is pinned for every kind, for a recycled slot, and under MALLOC_PERTURB_ on both backends
This commit is contained in:
@@ -389,6 +389,26 @@ namespace MobileGL::MG_Pipe {
|
||||
|
||||
// ---- what a unit case reads. The emitter builds INTO these and hands the applier the
|
||||
// same objects, so "what was emitted" costs no copy. ----
|
||||
// ---- the death half (P4a final review C-2) ----
|
||||
//
|
||||
// Called by the contract's death helper before the slot is freed (there is no wire
|
||||
// delete for this kind, D-I2, so this is the only client-side thing a framebuffer's
|
||||
// death has to do). The per-object Named latch is the entry: a recycled handle's Gen
|
||||
// already refuses the stale latch, so this is hygiene rather than a fix - the rule
|
||||
// (ID-8) is that whatever mints a handle retires everything it keeps under it at the
|
||||
// death, and every P4a kind takes the same shape. Gen-keyed for a late notice.
|
||||
void NoteFramebufferDied(MGPipeHandle handle) {
|
||||
const SizeT slot = handle.Slot;
|
||||
if (MGPipeHandleIsNull(handle) || slot >= m_named.size()) return;
|
||||
if (m_named[slot].Gen == handle.Gen) m_named[slot] = NamedEntry{};
|
||||
}
|
||||
// "Does this emitter hold a Named-record latch for this handle at its generation."
|
||||
Bool NamedRecordIsLatched(MGPipeHandle handle) const {
|
||||
const SizeT slot = handle.Slot;
|
||||
if (MGPipeHandleIsNull(handle) || slot >= m_named.size()) return false;
|
||||
return m_named[slot].Has && m_named[slot].Gen == handle.Gen;
|
||||
}
|
||||
|
||||
const MGPFramebufferState& LastDraw() const { return m_lastDraw; }
|
||||
const MGPFramebufferState& LastRead() const { return m_lastRead; }
|
||||
const MGPFramebufferState& LastNamed() const { return m_lastNamed; }
|
||||
|
||||
@@ -1370,11 +1370,24 @@ namespace MobileGL::MG_Pipe {
|
||||
}
|
||||
} // namespace
|
||||
|
||||
// THE EMITTER IS TOLD BETWEEN THE WIRE DELETE AND THE FREE (P4a final review C-2), for
|
||||
// every kind that keeps client state under a handle: a texture's drain entries, pointer,
|
||||
// cache reference and latches; a renderbuffer's entry; a framebuffer's Named latch; a
|
||||
// sampler view's and a shader CSO's record memo. Before this the six helpers freed the slot
|
||||
// and told nobody, so the texture emitter kept the freed ITextureObject* and the level on
|
||||
// the drain list, and `glTexImage2D; glDeleteTextures; <verb>` called a virtual on freed
|
||||
// memory from the next validate point. The forward is the P3a shape
|
||||
// (MGPipeEmitVertexElementsDestroyAndFree's emitter.NoteRecordDestroyed) applied to the
|
||||
// five P4a kinds that have an entry to retire; the content-addressed sampler CSO keeps
|
||||
// none per object (its death is the cache's LRU, ID-17). Unconditional in a push build,
|
||||
// like the mints: the entries exist whether or not the family bit is set.
|
||||
Bool MGPipeEmitSamplerViewCsoDestroyAndFree(Uint64 lifetimeId) {
|
||||
const MGPipeHandle handle =
|
||||
MGPipeSlots().FindByLifetimeId(MGPipeKind::SamplerViewCso, lifetimeId);
|
||||
const Bool published =
|
||||
EmitDeleteIfPublished(MGPipeKind::SamplerViewCso, handle, &MGPipeApplyDeleteSamplerView);
|
||||
ForwardWhenWired<kMGPipeWiredSamplerSubsystem>(
|
||||
MGPipeSamplerEmitterInstance(), [&](auto& emitter) { emitter.NoteRecordDestroyed(handle); });
|
||||
// THE NOTICE IS RAISED FOR THIS KIND TOO, and the reason it once was not is wrong:
|
||||
// NotifyStateObjectDestroyed takes a KIND and a lifetime id, not an object
|
||||
// (StateObjectDeathNotice.h - one entry point for every kind rather than one ops table
|
||||
@@ -1394,6 +1407,11 @@ namespace MobileGL::MG_Pipe {
|
||||
const MGPipeHandle handle = MGPipeSlots().FindByLifetimeId(MGPipeKind::Texture, lifetimeId);
|
||||
const Bool published =
|
||||
EmitDeleteIfPublished(MGPipeKind::Texture, handle, &MGPipeApplyResourceDestroy);
|
||||
// The emitter retires its entry while the handle still resolves (C-2): the drain list
|
||||
// drops the dead texture's levels, the raw pointer goes, the built-in sampler's cache
|
||||
// reference is given back, the latches and the sticky mask are cleared.
|
||||
ForwardWhenWired<kMGPipeWiredTextureSubsystem>(
|
||||
MGPipeTextureEmitterInstance(), [&](auto& emitter) { emitter.NoteTextureDied(handle); });
|
||||
NotifyAndFree(MGPipeKind::Texture, lifetimeId, handle);
|
||||
// THE SAMPLER VIEW DIES WITH ITS TEXTURE, because it is minted off the same lifetime
|
||||
// id: one SamplerViewCso per ITextureObject (D-F2), re-issued on the same handle
|
||||
@@ -1430,6 +1448,8 @@ namespace MobileGL::MG_Pipe {
|
||||
MGPipeSlots().FindByLifetimeId(MGPipeKind::Renderbuffer, lifetimeId);
|
||||
const Bool published =
|
||||
EmitDeleteIfPublished(MGPipeKind::Renderbuffer, handle, &MGPipeApplyResourceDestroy);
|
||||
ForwardWhenWired<kMGPipeWiredTextureSubsystem>(
|
||||
MGPipeTextureEmitterInstance(), [&](auto& emitter) { emitter.NoteRenderbufferDied(handle); });
|
||||
NotifyAndFree(MGPipeKind::Renderbuffer, lifetimeId, handle);
|
||||
return published;
|
||||
}
|
||||
@@ -1454,6 +1474,8 @@ namespace MobileGL::MG_Pipe {
|
||||
// whatever it owed", which for a framebuffer is the death notice this just raised.
|
||||
const MGPipeHandle handle =
|
||||
MGPipeSlots().FindByLifetimeId(MGPipeKind::Framebuffer, lifetimeId);
|
||||
ForwardWhenWired<kMGPipeWiredFramebufferSubsystem>(
|
||||
MGPipeFramebufferEmitterInstance(), [&](auto& emitter) { emitter.NoteFramebufferDied(handle); });
|
||||
NotifyAndFree(MGPipeKind::Framebuffer, lifetimeId, handle);
|
||||
return false;
|
||||
}
|
||||
@@ -1463,6 +1485,11 @@ namespace MobileGL::MG_Pipe {
|
||||
MGPipeSlots().FindByLifetimeId(MGPipeKind::SamplerCso, lifetimeId);
|
||||
const Bool published =
|
||||
EmitDeleteIfPublished(MGPipeKind::SamplerCso, handle, &MGPipeApplyDeleteSamplerState);
|
||||
// NOTHING TO RETIRE IN AN EMITTER FOR THIS KIND, stated rather than implied: a sampler
|
||||
// CSO is content-addressed and belongs to a value, so no emitter keeps an entry under
|
||||
// a SamplerObject's handle - the cache's entries are keyed by value and reference
|
||||
// count, and the death of a bound sampler object releases its unit's reference at the
|
||||
// next bind_sampler_states pass (SamplerEmit.h's reconciliation).
|
||||
NotifyAndFree(MGPipeKind::SamplerCso, lifetimeId, handle);
|
||||
return published;
|
||||
}
|
||||
@@ -1479,6 +1506,8 @@ namespace MobileGL::MG_Pipe {
|
||||
MGPipeSlots().FindByLifetimeId(MGPipeKind::ShaderCso, lifetimeId);
|
||||
const Bool published =
|
||||
EmitDeleteIfPublished(MGPipeKind::ShaderCso, handle, &MGPipeApplyDeleteShaderState);
|
||||
ForwardWhenWired<kMGPipeWiredProgramSubsystem>(
|
||||
MGPipeProgramEmitterInstance(), [&](auto& emitter) { emitter.NoteRecordDestroyed(handle); });
|
||||
NotifyAndFree(MGPipeKind::ShaderCso, lifetimeId, handle);
|
||||
return published;
|
||||
}
|
||||
|
||||
@@ -324,14 +324,12 @@ namespace MobileGL::MG_Pipe {
|
||||
|
||||
// The memo's other half, and the bound-mirror clearing beside it.
|
||||
//
|
||||
// NO PRODUCTION CALLER TODAY, stated rather than implied: since c0b the death path
|
||||
// reads the contract's latch and never asks an emitter. It is kept because the memo
|
||||
// above needs a way to be told, and because everything it clears SELF-HEALS if it is
|
||||
// not called - the slot's Gen moves on reuse, so `RecordGen == handle.Gen` refuses a
|
||||
// stale record latch, and the three bound mirrors below hold a handle whose generation
|
||||
// can never be handed out again, so the next EmitShaderState compares against a
|
||||
// different handle and re-binds. Clearing them here is the cheaper answer, not the
|
||||
// load-bearing one.
|
||||
// THE CALLER IS THE CONTRACT's DEATH HELPER (P4a final review C-2): the death path
|
||||
// reads the contract's latch for the wire delete and then forwards here, before the
|
||||
// slot is freed, so a dead handle no longer reads as published in this memo between
|
||||
// the death and the recycle and the three bound mirrors never name a dead program.
|
||||
// Gen-keyed, so a late notice for a slot already handed out again clears nothing of
|
||||
// the successor's.
|
||||
void NoteRecordDestroyed(MGPipeHandle handle) {
|
||||
if (MGPipeHandleIsNull(handle)) return;
|
||||
Vector<Latch>& table = TableOf(handle);
|
||||
|
||||
@@ -986,11 +986,11 @@ namespace MobileGL::MG_Pipe {
|
||||
}
|
||||
|
||||
// The memo's other half, for a caller that knows the applier has dropped this record.
|
||||
// NO PRODUCTION CALLER TODAY, and that is stated rather than implied: the death path
|
||||
// goes through the contract's latch, not through here. It is kept because the memo
|
||||
// above needs a way to be told, and because leaving the latch standing SELF-HEALS
|
||||
// anyway - the slot's Gen moves on reuse, so the `RecordGen == handle.Gen` test in
|
||||
// RecordIsPublished and in AcquireSamplerView already refuses a stale entry.
|
||||
// THE CALLER IS THE CONTRACT's DEATH HELPER (P4a final review C-2): the texture's
|
||||
// helper drops the sampler view minted off the texture's lifetime id and forwards here
|
||||
// before the slot is freed, so a dead handle no longer reads as published in this memo
|
||||
// between the death and the recycle. Gen-keyed, so a late notice for a slot already
|
||||
// handed out again clears nothing of the successor's.
|
||||
void NoteRecordDestroyed(MGPipeHandle handle) {
|
||||
if (MGPipeHandleIsNull(handle)) return;
|
||||
const SizeT slot = handle.Slot;
|
||||
|
||||
@@ -441,17 +441,79 @@ namespace MobileGL::MG_Pipe {
|
||||
|
||||
// The texture a handle names, or null. A RAW pointer is exact here for
|
||||
// MGPipeResourceTracker::Resolve's reason: the entry exists only between the create the
|
||||
// constructor emits and the destroy the destructor emits, and the Gen compare is what
|
||||
// refuses a stale handle rather than resolving it to whatever now occupies the slot.
|
||||
// constructor emits and the destroy the destructor emits - and since the final review's
|
||||
// C-2 that sentence is ESTABLISHED rather than assumed: the contract's death helper
|
||||
// forwards to NoteTextureDied below before it frees the slot, so a dead handle finds a
|
||||
// null pointer here. The Gen compare refuses a RECYCLED handle rather than resolving it
|
||||
// to whatever now occupies the slot.
|
||||
//
|
||||
// A DEAD SLOT IS REFUSED, LOUDLY. The allocator's generation moves only at the NEXT
|
||||
// hand-out, so between a death and a recycle a dead handle compares equal to the slot's
|
||||
// generation - which is why the guard is IsLive and not GenOfSlot (the review's C-2:
|
||||
// that compare guarded a recycled slot and never a dead one, and the drain then called
|
||||
// a virtual on the freed object once per verb). Reaching this arm at all means a death
|
||||
// path skipped the emitter, which is a seam defect and not traffic: it is counted and
|
||||
// logged once, and the answer is null.
|
||||
ITextureObject* ResolveTexture(MGPipeHandle handle) const {
|
||||
const SizeT slot = handle.Slot;
|
||||
if (MGPipeHandleIsNull(handle) || slot >= m_textures.size()) return nullptr;
|
||||
const Entry& entry = m_textures[slot];
|
||||
if (entry.Texture == nullptr || entry.Gen != handle.Gen) return nullptr;
|
||||
if (MGPipeSlots().GenOfSlot(MGPipeKind::Texture, handle.Slot) != handle.Gen) return nullptr;
|
||||
if (!MGPipeSlots().IsLive(MGPipeKind::Texture, handle)) {
|
||||
++m_deadResolves;
|
||||
MGLOG_E_ONCE("MGPipe: texture handle {slot=%u, gen=%u} is dead but the emitter still holds its "
|
||||
"object - the death path did not retire the entry; refused rather than resolved",
|
||||
handle.Slot, handle.Gen);
|
||||
return nullptr;
|
||||
}
|
||||
return entry.Texture;
|
||||
}
|
||||
|
||||
// ---- the death half (P4a final review C-2) ----
|
||||
//
|
||||
// CALLED BY THE CONTRACT'S DEATH HELPER, after the wire delete went out and BEFORE the
|
||||
// slot is freed (ID-8's order: delete, notice, free - this sits between the first two).
|
||||
// v2 had no such door: the helper freed the slot, the emitter kept the freed
|
||||
// ITextureObject* and the level on the drain list, and `glTexImage2D; glDeleteTextures;
|
||||
// <any verb>` walked freed memory at the next validate point - a SIGABRT ("pure virtual
|
||||
// method called") at the shipping mask. Everything the entry owns goes here: the drain
|
||||
// entries (nothing is owed for a dead texture - its record is gone with the wire delete),
|
||||
// the built-in sampler's cache reference (ID-17: one per entry, released at the death
|
||||
// and no longer at the recycle), the latches and the sticky mask. RetireIfRecycled stays
|
||||
// as the belt for a slot whose death this emitter was never told about.
|
||||
//
|
||||
// Keyed on the GENERATION so a late notice for a slot that has already been handed out
|
||||
// again cannot retire the successor's entry.
|
||||
void NoteTextureDied(MGPipeHandle handle) {
|
||||
const SizeT slot = handle.Slot;
|
||||
if (MGPipeHandleIsNull(handle) || slot >= m_textures.size()) return;
|
||||
Entry& entry = m_textures[slot];
|
||||
if (entry.Gen != handle.Gen) return;
|
||||
if (!entry.DrainKeys.empty()) {
|
||||
// A death inside the drain cannot happen (no SharedPtr drops there), but if one
|
||||
// ever did the loop below is iterating m_drain: the null pointer the reset
|
||||
// leaves is what makes EmitOneLevel answer "nothing owed" and drop the entry.
|
||||
if (!m_draining) {
|
||||
SizeT kept = 0;
|
||||
for (SizeT i = 0; i < m_drain.size(); ++i) {
|
||||
if (m_drain[i].Handle == handle) continue;
|
||||
m_drain[kept++] = m_drain[i];
|
||||
}
|
||||
m_drain.resize(kept);
|
||||
}
|
||||
entry.DrainKeys.clear();
|
||||
}
|
||||
MGPipeSamplerCsoCacheInstance().Release(entry.BuiltinSampler);
|
||||
entry = Entry{};
|
||||
}
|
||||
void NoteRenderbufferDied(MGPipeHandle handle) {
|
||||
const SizeT slot = handle.Slot;
|
||||
if (MGPipeHandleIsNull(handle) || slot >= m_renderbuffers.size()) return;
|
||||
Entry& entry = m_renderbuffers[slot];
|
||||
if (entry.Gen != handle.Gen) return;
|
||||
entry = Entry{};
|
||||
}
|
||||
|
||||
// ---- the sticky bind mask (D-A4) ----
|
||||
//
|
||||
// ORed, never cleared, and emitted on BOTH resource_create and every
|
||||
@@ -474,6 +536,11 @@ namespace MobileGL::MG_Pipe {
|
||||
void NoteTextureBoundAs(MGPipeHandle handle, Uint16 bit) {
|
||||
if (MGPipeHandleIsNull(handle)) return;
|
||||
Entry& entry = EntryFor(m_textures, handle);
|
||||
// The entry is stamped with the generation it is written under, and a predecessor's
|
||||
// entry on a recycled slot is retired first (the same door AcquireTexture takes): a
|
||||
// texture born while the family bit was clear has no create to have done it.
|
||||
RetireIfRecycled(entry, handle);
|
||||
entry.Gen = handle.Gen;
|
||||
const Uint16 before = entry.BindMask;
|
||||
const Uint16 now = static_cast<Uint16>(before | bit);
|
||||
if (now == before) return;
|
||||
@@ -491,6 +558,8 @@ namespace MobileGL::MG_Pipe {
|
||||
void NoteRenderbufferBoundAs(MGPipeHandle handle, Uint16 bit) {
|
||||
if (MGPipeHandleIsNull(handle)) return;
|
||||
Entry& entry = EntryFor(m_renderbuffers, handle);
|
||||
RetireIfRecycled(entry, handle);
|
||||
entry.Gen = handle.Gen;
|
||||
const Uint16 before = entry.BindMask;
|
||||
const Uint16 now = static_cast<Uint16>(before | bit);
|
||||
if (now == before) return;
|
||||
@@ -706,9 +775,9 @@ namespace MobileGL::MG_Pipe {
|
||||
// what stops the LRU pulling a handle out from under a standing MGPTextureParams
|
||||
// record: the applier deliberately does not resolve BuiltinSampler, and an eviction
|
||||
// is not a parameter change, so nothing would refuse and nothing would re-emit. The
|
||||
// previous handle is released when the content moves it, and the last one when the
|
||||
// slot is recycled (RetireIfRecycled) - which is the only moment this package can
|
||||
// see a texture die, the death helper being A's.
|
||||
// previous handle is released when the content moves it, and the last one at the
|
||||
// texture's death (NoteTextureDied, reached from the contract's death helper) - or
|
||||
// at the recycle, as the belt, for a death this emitter was not told about.
|
||||
MGPipeSamplerCsoCache& cache = MGPipeSamplerCsoCacheInstance();
|
||||
Uint64 samplerBytes = 0;
|
||||
const MGPipeHandle builtinSampler =
|
||||
@@ -879,6 +948,9 @@ namespace MobileGL::MG_Pipe {
|
||||
// itself returns no byte count - it is not emitted from the validate point's payload
|
||||
// histogram - so this is where the cache's answer lands.
|
||||
Uint64 SamplerCsoPayloadBytes() const { return m_samplerCsoPayloadBytes; }
|
||||
// Dead handles that still held an object when resolved: a death path that skipped the
|
||||
// emitter. 0 on a healthy tree; a case that drives every death path asserts it.
|
||||
Uint64 DeadResolveCount() const { return m_deadResolves; }
|
||||
MGPipeHandle BuiltinSamplerOf(MGPipeHandle handle) const {
|
||||
const SizeT slot = handle.Slot;
|
||||
if (MGPipeHandleIsNull(handle) || slot >= m_textures.size()) return kMGPipeNullHandle;
|
||||
@@ -901,6 +973,7 @@ namespace MobileGL::MG_Pipe {
|
||||
m_creates = m_respecifies = m_paramSets = m_subDatas = 0;
|
||||
m_refusedSubDatas = 0;
|
||||
m_samplerCsoPayloadBytes = 0;
|
||||
m_deadResolves = 0;
|
||||
}
|
||||
|
||||
// A unit fixture's per-case reset; the library never calls it. See
|
||||
@@ -968,7 +1041,8 @@ namespace MobileGL::MG_Pipe {
|
||||
}
|
||||
static Uint16 MaskOf(const Vector<Entry>& table, MGPipeHandle handle) {
|
||||
const SizeT slot = handle.Slot;
|
||||
return slot < table.size() ? table[slot].BindMask : Uint16{0};
|
||||
if (slot >= table.size() || table[slot].Gen != handle.Gen) return Uint16{0};
|
||||
return table[slot].BindMask;
|
||||
}
|
||||
// A SLOT THE ALLOCATOR HAS HANDED OUT AGAIN CARRIES ITS PREDECESSOR'S ENTRY, and every
|
||||
// field in it is a lie about the new object (m4). The sticky BindMask is the one that
|
||||
@@ -977,10 +1051,10 @@ namespace MobileGL::MG_Pipe {
|
||||
// the dead one's mask and its first descriptor said so. The generation is what
|
||||
// distinguishes them and the reset is here because AcquireTexture is the one door.
|
||||
//
|
||||
// IT IS ALSO THE ONLY MOMENT THIS PACKAGE CAN SEE A TEXTURE DIE. The death helper is
|
||||
// A's (MGPipeEmitTextureDestroyAndFree) and does not forward to this emitter, so the
|
||||
// built-in sampler's cache reference is dropped here - bounded by the number of live
|
||||
// texture slots rather than unbounded, which is the shape ID-17 rule 3 names.
|
||||
// IT IS THE BELT, NOT THE PATH (final review C-2): the death helper forwards to
|
||||
// NoteTextureDied, which retires the entry - drain entries, cache reference, latches,
|
||||
// mask - at the death itself. This stays for a slot whose death this emitter was never
|
||||
// told about, and drops the same reference if one is still standing.
|
||||
void RetireIfRecycled(Entry& entry, MGPipeHandle handle) {
|
||||
if (entry.Gen == handle.Gen) return;
|
||||
MGPipeSamplerCsoCacheInstance().Release(entry.BuiltinSampler);
|
||||
@@ -1200,6 +1274,7 @@ namespace MobileGL::MG_Pipe {
|
||||
Uint64 m_subDatas = 0;
|
||||
Uint64 m_refusedSubDatas = 0;
|
||||
Uint64 m_samplerCsoPayloadBytes = 0;
|
||||
mutable Uint64 m_deadResolves = 0;
|
||||
};
|
||||
|
||||
inline MGPipeTextureEmitter& MGPipeTextureEmitterInstance() {
|
||||
@@ -1234,7 +1309,10 @@ namespace MobileGL::MG_Pipe {
|
||||
// keeping the inline definition here would not have compiled at all;
|
||||
// * step 1 of the death order is inside MGPipeEmitTextureDestroyAndFree /
|
||||
// ...RenderbufferDestroyAndFree, which read MGPipeHandleIsPublished and emit the
|
||||
// resource_destroy themselves, so the destructors call ONE helper and not two;
|
||||
// resource_destroy themselves, so the destructors call ONE helper and not two - and
|
||||
// since the final review's C-2 the helper then forwards to NoteTextureDied /
|
||||
// NoteRenderbufferDied above, so no ITextureObject* survives its object in this table
|
||||
// and no dead level survives on the drain list;
|
||||
// * the publication latch is PipeFill.cpp's {kind, slot, gen} table, written by
|
||||
// PublishCreate above and read by those helpers.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user