[MG_Pipe, MG_Backend, MG_Impl] (P6/sm, P6/st): four guards that were compiled in and could never arm, and the crash behind one of them

CONTRACT-P6 3.2 (D1c), 3.3 (D13) and 3.4 (D10).

Every guard below asked a question a SERVER PROCESS answers wrongly - "is a
ClientSession active" (never, there) or "am I on the apply thread" (every
thread is, there) - so all four were present, compiled, and permanently
disarmed in the one shape they exist for. That is worse than absent: it reads
as coverage.

D13 FIRST, THEN THE WIDENING. MGPipeRoleSplitActive() meant "the REHEARSAL
knob is on AND the transport is real", so widening it in place would have left
a narrow name describing a wide predicate. It is renamed to
MGPipeRoleSplitRehearsalActive() across its 12 sites - including the
DualBlockScenario assertion message that embeds it - and the wider predicate
is introduced BESIDE it.

MGPipeBlocksAreDistinct() IS TRUE FOR TWO DIFFERENT REASONS, which is the
whole point: under the rehearsal the two blocks are distinct objects in one
process; under SPAWN they are distinct because they are in different ADDRESS
SPACES, and no knob is involved. The spawn retrace runs with
role-split-state=0 - measured off the lane's own log - so every guard gated on
the rehearsal was off there.

THE CRASH. MGPipeServerBlockNoteIdentity early-returned on the rehearsal, so a
spawn server's gPipeInputs.ContextIdentity() stayed nullptr for the life of the
process. DirectGLES' framebuffer binding-slot memo compares that identity FIRST
and with NO generation, against a cache variable that also starts nullptr - so
nullptr == nullptr reads as a CACHE HIT and the function returns a slot pointer
nothing ever filled. `sm` widens the gate; `st` lands the named Fatal
PipeInputs.h had already asked for, so a reopening says which memo and which
identity instead of corrupting memory three frames later. Guarded on
MOBILEGL_PIPE_PUSH: in a pull build that identity is the live GLContext's
address and is never null, so the test would be dead code and G1 measures that.

DISJUNCTS, NOT SUBSTITUTIONS - and this was measured the hard way. The first
attempt REPLACED OnApplyThread() with MGPipeServerArm() and
ClientSession::Active() with MGPipeSessionLive(). 14 RemoteGuards and
RemoteClientControls cases went green by omission: a ServerLoop fixture with no
client session has only the original facts, so swapping them out disarmed every
test that drives the apply thread directly. The originals stay; the new
predicates are added beside them.

AND THE PROBE HAD TO BE THE RIGHT ONE. Installing
ClientSession::ApplyThreadIsInsideApplier as the apply-thread probe made every
GL-thread call a role violation whenever the applier happened to be busy - the
inproc retrace died on Fatal{RoleViolation, "MGPipeSlots"} raised from the GL
thread. "Am I the apply thread" and "is the apply thread inside the applier"
are different questions; the second is R-1's mutual-exclusion probe and is
about another thread entirely. ServerLoop::OnApplyThread is the one the guards
mean.

Three of D1c's six predicates land here, the three the four guards need. The
other three (MGPipeSplitActive, MGPipeShouldDial, MGPipeBackendIsLocal) have no
D10 site and are not added as dead code; their call sites are ContextEpoch and
ActiveXfbState, which this commit does not touch.

D14 holds: the non-disaggregated build gets constexpr false definitions, not
runtime-false predicates, so every consult folds at compile time. G1
re-measured - 0 symbol diff, .text unchanged at 0xa52203.

Two test names added, both falsifiable by construction: the spawn case asserts
the blocks are distinct with the rehearsal knob OFF, and the monolith case
asserts the predicate is not simply a constant. Unit suite 2353 tests, one
pre-existing environmental failure. All six OpenRA arms green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-21 19:38:19 -04:00
co-authored by Claude Opus 5
parent 570ebde9bc
commit 743d1facba
11 changed files with 230 additions and 22 deletions
+22 -3
View File
@@ -1164,6 +1164,21 @@ namespace MobileGL::MG_Remote::Client {
// them down in the mirror order (m_started = false, then g_active = nullptr).
m_started = true;
g_active = this;
// D1c: THE SESSION-LIVE AND APPLY-THREAD FACTS, stated where they become true.
//
// MGPipeApplierReset's layer-2 guard used to ask ClientSession::Active() != nullptr,
// which no SERVER process can ever answer yes to - so the guard was compiled in and
// permanently disarmed there. MGPipeSessionLive() is the same question asked in a way
// both roles can answer, and the probe hands MG_Backend the one thread fact it cannot
// derive: whether the CALLING thread is inside the applier.
MG_Pipe::MGPipeSetSessionLive(true);
// ServerLoop::OnApplyThread, NOT ApplyThreadIsInsideApplier - two different questions,
// and installing the wrong one turned every GL-thread call into a role violation while
// the applier happened to be busy. Measured: the inproc retrace died on
// Fatal{RoleViolation, "MGPipeSlots"} raised from the GL thread. "Am I the apply
// thread" is what the guards ask; "is the apply thread inside the applier" is R-1's
// mutual-exclusion probe and is about a DIFFERENT thread.
MG_Pipe::MGPipeSetApplyThreadProbe(&Server::ServerLoop::OnApplyThread);
LogMemory("handshake");
// ---- 8. and only now the apply thread. It is package v1's ServerLoop: it names the
@@ -1298,6 +1313,10 @@ namespace MobileGL::MG_Remote::Client {
m_presentsSent = 0;
if (g_active == this) {
g_active = nullptr;
// Mirror order, and the probe goes with it: a dangling function pointer into a
// torn-down session is a worse answer than "there is no applier".
MG_Pipe::MGPipeSetSessionLive(false);
MG_Pipe::MGPipeSetApplyThreadProbe(nullptr);
}
// AND ONLY NOW the monolith adapters go back (codex 4): every ring an emitter would have
// used is freed above, so from here a routed call - an at-exit ~BufferObject delete - runs
@@ -1803,7 +1822,7 @@ namespace MobileGL::MG_Remote::Client {
// its own diagnostic; the client writes a different block and waits through appliedSeq.
Bool ClientSession::InBarrierWait() { return g_inBarrierWait; }
Bool ClientSession::ApplyThreadIsInsideApplier() {
if (MG_Pipe::MGPipeRoleSplitActive()) return g_roleInsideApplier;
if (MG_Pipe::MGPipeRoleSplitRehearsalActive()) return g_roleInsideApplier;
return g_applyThreadInsideApplier.load(std::memory_order_acquire);
}
@@ -1900,12 +1919,12 @@ namespace MobileGL::MG_Remote::Client {
}
void ClientSession::NoteApplyThreadEnteredApplier() {
g_roleInsideApplier = true;
if (!MG_Pipe::MGPipeRoleSplitActive())
if (!MG_Pipe::MGPipeRoleSplitRehearsalActive())
g_applyThreadInsideApplier.store(true, std::memory_order_release);
}
void ClientSession::NoteApplyThreadLeftApplier() {
g_roleInsideApplier = false;
if (!MG_Pipe::MGPipeRoleSplitActive())
if (!MG_Pipe::MGPipeRoleSplitRehearsalActive())
g_applyThreadInsideApplier.store(false, std::memory_order_release);
}