diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index 06732475..622bab8e 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -30,19 +30,21 @@ namespace MobileGL::MG_Remote::Server { - namespace { - // P5e (ra, CONTRACT-P5E §1 / §6): DOES THIS SERVER PUBLISH kCapRunAheadApply? The - // question is asked of the server's OWN CallMask and not of a build constant, because - // "the client may run ahead" is exactly what that bit says and Magma never sets it. - // A session with no CallMask yet (the bring-up window, a fixture that never called - // SetCapabilityBits) answers false: no client can have latched run-ahead against a - // snapshot that was never published. - Bool ServerPublishesRunAhead() { - const ServerSession* session = ServerSession::Active(); - if (session == nullptr || !session->CallMaskIsSet()) return false; - return (session->CallMask() & static_cast(MG_Pipe::kCapRunAheadApply)) != 0; - } - } // namespace + // P5e (ra, CONTRACT-P5E §1 / §6): DOES THIS SERVER PUBLISH kCapRunAheadApply? The + // question is asked of the server's OWN CallMask and not of a build constant, because + // "the client may run ahead" is exactly what that bit says and Magma never sets it. + // A session with no CallMask yet (the bring-up window, a fixture that never called + // SetCapabilityBits) answers false: no client can have latched run-ahead against a + // snapshot that was never published. + // + // P5e (gl, ID-111): PROMOTED OUT OF THE ANONYMOUS NAMESPACE, unchanged in body. It is now + // the second conjunct of the barriered stamp as well as Present's frame-serial gate, and + // the red-once has to be able to assert what it answers for the session it built. + Bool MGPipeServerPublishesRunAhead() { + const ServerSession* session = ServerSession::Active(); + if (session == nullptr || !session->CallMaskIsSet()) return false; + return (session->CallMask() & static_cast(MG_Pipe::kCapRunAheadApply)) != 0; + } ReplyPool::ReplyPool(void* base, Uint64 sizeBytes, Uint32 slotCount, Uint32 slotBytes) : m_base(static_cast(base)), m_size(sizeBytes), m_slots(slotCount), m_slotBytes(slotBytes) {} @@ -311,7 +313,7 @@ namespace MobileGL::MG_Remote::Server { // peer that has not been re-built - and on a server that PUBLISHES the run-ahead cap it // is a protocol fault, because such a client is pacing on this answer and a 0 would // acknowledge a frame nobody asked about. - if (present.FrameSerial == 0 && ServerPublishesRunAhead()) { + if (present.FrameSerial == 0 && MGPipeServerPublishesRunAhead()) { MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"Present.FrameSerial\"} - a run-ahead " "server was handed present serial 0. The client mints this 1-based and " "waits on it for its credit (CONTRACT-P5E §2.4); returning a credit for " @@ -1176,10 +1178,27 @@ namespace MobileGL::MG_Remote::Server { // §4.4 exemptions from a probe the client's own wait still makes safe - a refusal with // no defect behind it. The predicate is computed on every record all the same, so it is // exercised for the whole phase rather than first run on the day it starts deciding. + // + // AND THE SERVER'S OWN CAPABILITY IS THE SECOND CONJUNCT (P5e gl, ID-111). The constant + // above says what the CLIENT will do once ra's wait rule is live; it says nothing about + // which server this is. kCapRunAheadApply is never published on DirectVulkan (ID-90, + // MGPipeRunAheadCapBitsFor), yet draw_vbo / blit / clear / launch_grid are all + // kWaitNone - so a stamp that read the build constant alone would, on the day it flips, + // have a MAGMA server mark every draw record UNBARRIERED while its client is still + // lockstep. CountBarrierPull (PipeInputs.cpp) is an unconditional Fatal on an + // unbarriered pull, no knob involved, and Magma's residual fill is its ONLY source for + // the seven pointer-backed fields: Magma would die on its first draw and take + // MagmaP7AllocatorDebtScope's exemption with it. + // + // BOTH PREDICATES ARE COMPUTED UNCONDITIONALLY, as arguments rather than as the arms of + // a short-circuit, which is ID-103's reason extended to the capability probe: they are + // exercised on every record for the whole phase rather than first running on the day + // they start deciding. const Bool wireSaysBarriered = MG_Pipe::MGPipeBarriered( static_cast(record.kind), record.payload, MG_Pipe::MGPipeApplier()); MG_Pipe::MGPipeApplierSetCurrentRecordBarriered( - MG_Pipe::kMGPipeP5eClientWaitRuleLanded ? wireSaysBarriered : true); + MGPipeApplierStampsBarriered(MG_Pipe::kMGPipeP5eClientWaitRuleLanded, + MGPipeServerPublishesRunAhead(), wireSaysBarriered)); // ORDER IS THE CONTRACT'S: stamp, then apply. The stamp is what makes any server-side // read of gPipeInputs legal at all (PipeApplier.h's block 1), so a record applied // before it aborts on the FIRST field inside SyncRenderState. diff --git a/MobileGL/MG_Remote/Server/PipeApplier.h b/MobileGL/MG_Remote/Server/PipeApplier.h index e8c3d3e1..a9c5e178 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.h +++ b/MobileGL/MG_Remote/Server/PipeApplier.h @@ -48,6 +48,39 @@ namespace MobileGL::MG_Remote::Server { + // P5e (gl, ID-111): DOES THIS SERVER PUBLISH kCapRunAheadApply? Asked of the server's OWN + // CallMask and not of a build constant - that bit IS the sentence "the client may run ahead + // of this apply", and Magma never sets it (MGPipeRunAheadCapBitsFor returns kCapNone for + // every backend but DirectGLES, ID-90). A session with no CallMask yet answers false: no + // client can have latched run-ahead against a snapshot that was never published. + // + // PUBLIC because the red-once has to be able to say "the session I built lacks bit 10" in + // its own words rather than inferring it from the stamp it is testing (ID-102's rule: the + // action under test is the probe itself). + Bool MGPipeServerPublishesRunAhead(); + + // ---- the barriered stamp, AS A PURE FUNCTION (ID-111) --------------------------------- + // + // ApplyOne stamps "is the client parked behind this record" before anything can ask + // (CONTRACT-P5E §2.1 / §4.4). Until the P5e integration commit the answer is `true` for + // every record whatever the wire says, because the client still blocks after each one + // (ID-103) - but the day the constant flips, the answer must ALSO ask whether this server + // published the run-ahead bit at all. It is `wireSaysBarriered` only when BOTH halves of + // the run-ahead arm are true; Magma publishes no bit 10, so a Magma server keeps stamping + // `true` and its lockstep client keeps being safe. Without the second conjunct a Magma + // server would stamp every draw_vbo / blit / clear / launch_grid UNBARRIERED (they are all + // kWaitNone) while its client is still parked, and CountBarrierPull aborts unconditionally + // on an unbarriered pull - Magma dies on its first draw, no knob involved. + // + // IT IS A PURE FUNCTION for MGPipeRunAheadCapBitsFor's reason: "Magma never runs ahead" is + // then something a unit case can hold AT THE POST-FLIP VALUE of the constant, rather than a + // claim that first becomes testable on the day the integration commit throws the switch. + constexpr Bool MGPipeApplierStampsBarriered(Bool clientWaitRuleLanded, + Bool serverPublishesRunAhead, + Bool wireSaysBarriered) { + return (clientWaitRuleLanded && serverPublishesRunAhead) ? wireSaysBarriered : true; + } + // Writes answers into SEG_REPLY at seq % slots, stamping the seq back into the slot header // so a wrong-slot read is detectable rather than plausible (table 0's slot header row: // {Uint64 Seq; Int32 Status; Uint32 Size;}). diff --git a/MobileGL/MG_Test/Wire/RemoteClientControls.inc b/MobileGL/MG_Test/Wire/RemoteClientControls.inc index b04edfbd..d5158b32 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientControls.inc +++ b/MobileGL/MG_Test/Wire/RemoteClientControls.inc @@ -659,4 +659,61 @@ TEST(RemoteClientControls, StagingWaitsForRetirementAfterThePriorRecordApplied) }); ExpectChildSuccess(child); } + +// P5e (gl) RED-ONCE, ID-111. THE SERVER-SIDE MIRROR OF THE CLIENT LATCH - the line that makes +// ID-90 ("Magma keeps the lockstep for the whole of P5e") true in code rather than in prose. +// +// The control session calls SetCapabilityBits(0), so its CallMask IS set and does NOT carry +// bit 10. That is a Magma server's answer for ever (MGPipeRunAheadCapBitsFor returns kCapNone +// off DirectGLES) and an Espryt server's answer for the whole bring-up window. A DrawVbo then +// crosses for real, so what is under test is the stamp PipeApplier::ApplyOne writes and not a +// test-only branch beside it. +// +// FOUR ASSERTIONS, because each one alone can be true for the wrong reason: +// 1. the session really lacks the bit, said in its own words rather than inferred from the +// stamp being tested (ID-102: the action under test is the probe itself); +// 2. the wire really answers UNBARRIERED for this record - draw_vbo is kWaitNone - so the +// `wireSaysBarriered` arm is the one a flipped constant would take; +// 3. the record that actually crossed was stamped barriered; +// 4. and the stamp is STILL barriered at the POST-FLIP value of the wait-rule constant. +// +// (4) IS THE ONE THAT MATTERS AND IT IS STATED AT `true` RATHER THAN READ FROM THE CONSTANT. +// kMGPipeP5eClientWaitRuleLanded is false today, so (3) alone would pass for a server that had +// no capability conjunct at all - and would keep passing until the integration commit, at which +// point Magma SIGSEGVs on its first draw with the lane still green. Asking the pure function +// for the post-flip answer is how the claim gets tested before the day it starts deciding. +// +// THE RED: delete `serverPublishesRunAhead &&` from MGPipeApplierStampsBarriered +// (MG_Remote/Server/PipeApplier.h) and (4) flips to false - the child exits 124. +TEST(RemoteClientControls, ADrawOnASessionWithoutTheRunAheadBitStaysStampedBarriered) { + const auto child = RunInChild([] { + StartControlSession(); + DrawPeer peer; + peer.Install(); + MGPDrawInfo info{}; + info.NumDraws = 1; + MGPDrawRange range{}; + if (ClientSessionInstance().EmitAndWait(MGPWireOp::DrawVbo, &info, sizeof(info), &range, + sizeof(range), nullptr, 0, nullptr) == 0) + ::_exit(120); + // Read the stamp on the thread that owns it: it is a thread_local describing "which + // record is the apply thread inside right now", and ApplyOne leaves it standing. + if (Srv::ServerLoopInstance().RunOnApplyThread([](void*) { + if (Srv::MGPipeServerPublishesRunAhead()) ::_exit(121); + MGPDrawInfo probe{}; + probe.NumDraws = 1; + if (MGPipeBarriered(MGPWireOp::DrawVbo, &probe, MGPipeApplier())) ::_exit(122); + if (!MGPipeApplierCurrentRecordIsBarriered()) ::_exit(123); + if (!Srv::MGPipeApplierStampsBarriered(/*clientWaitRuleLanded=*/true, + Srv::MGPipeServerPublishesRunAhead(), + /*wireSaysBarriered=*/false)) + ::_exit(124); + return MOBILEGL_OK; + }, + nullptr) != MOBILEGL_OK) + ::_exit(125); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} #endif