From bbfd4fef81261891561562fc449442be5370c866 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 08:22:26 -0400 Subject: [PATCH] [MG_Test, MG_Impl, MG_Pipe] (Disaggregated): P5e ra - the wait rule's cases, and the two constants are one switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fifteen cases, and every one of them arms run-ahead through the production latch (publish kCapRunAheadApply into the caps mirror before Start) rather than through a test-only setter - ID-102's lesson: a red-once whose probe is a branch beside the production path is a red-once that proves the branch. - RemoteRunAhead: the knob alone does not arm it; VERB_BARRIER=0 disarms it even with the cap bit; a credit-1 client pays at every present after the first and a credit-3 client at none of its first three; an unbarriered verb fills nothing and is allowed; a 3 MB event burst behind an unwaited sequence flow-controls instead of aborting and arrives LOSSLESS; only the texture half of resource_subdata wants its reply. - RemoteGuards: a gPipeInputs touch outside a barriered fill under run-ahead is Fatal{RoleViolation, "gPipeInputs"} by name and the barriered fill is allowed; a residual pull under an UNBARRIERED record is Fatal WITHOUT the strict knob while the same pull under a barriered one still only counts (ruling 4, in executable form); a deferred destroy from an unbarriered apply is the finding and from a barriered one is still served (ruling 13). - SessionTest: a full SEG_EVENT wakes a client parked on appliedSeq AND one parked on the present credit, without touching either watermark; the drain clears the latch and RINGS, once, and is silent when the latch was already down. - The event burst's drainer starts 50 ms late on purpose. A drainer that starts at once can keep a 256 KiB ring from ever being full, and a red-once that depends on the scheduler losing a race is not a red-once. - ClientVerbIsBarriered now takes a nullable context: the validate point asks the question before it knows whether there is anything to fill, so the answer is about the RECORD. A null context has no open XFB span, and treating it as "barriered" instead made the fill red-once unreachable in any fixture without a live GLContext. - kMGPipeP5eClientWaitRuleLanded flips WITH kMGPipeP5eRunAheadReady, in the integration commit - an amendment to ID-103's wording ("the same commit that lands the wait rule"). The argument ID-103 makes is about whether the CLIENT is parked: RunAheadArmed() needs the server's caps bit, so between ra landing and the bit being published the client still blocks after every record, and a `false` stamp there would withdraw §4.4's exemptions from probes that wait still makes safe. Two spellings, one switch. --- MobileGL/MG_Impl/Pipe/PipeFill.cpp | 17 +- MobileGL/MG_Pipe/PipeApply.h | 16 +- MobileGL/MG_Test/Wire/RemoteClientTest.cpp | 372 +++++++++++++++++++++ MobileGL/MG_Test/Wire/SessionTest.cpp | 116 +++++++ 4 files changed, 513 insertions(+), 8 deletions(-) diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index 34d23700..da761dd3 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -387,11 +387,19 @@ namespace MobileGL::MG_Pipe { // AN OP WITH NO VERB ROW ANSWERS BARRIERED. A verb the join does not know is one this // file cannot reason about, and the safe answer - fill it, wait for it - is also the // pre-P5e answer. - Bool ClientVerbIsBarriered(MGPipeVerb verb, GLContext& ctx) { + // `ctx` MAY BE NULL, and that is not a convenience: the validate point asks this + // question BEFORE it has decided whether there is anything to fill, so that the answer + // is about the RECORD and not about the state of the block. A null context has no open + // transform-feedback span, so clause 2 is false for it - and the verb is a no-op below + // either way. + Bool ClientVerbIsBarriered(MGPipeVerb verb, GLContext* ctx) { const MGPWireOp op = WireOpForVerb(verb); if (op == MGPWireOp::kOpCount) return true; if (MGPipeWaitClassFor(op) != kWaitNone) return true; - if (MGPipeCallClassFor(op) == kCtxVerb && ctx.IsTransformFeedbackActive()) return true; + if (MGPipeCallClassFor(op) == kCtxVerb && ctx != nullptr && + ctx->IsTransformFeedbackActive()) { + return true; + } return false; } @@ -3191,9 +3199,8 @@ namespace MobileGL::MG_Pipe { // process rather than a value the wire owes it. MGPipeFillAccess::SetIdentity(inputs, ctx); #if MOBILEGL_BUILD_DISAGGREGATED - // §2.1's predicate, client-side (see ClientVerbIsBarriered). A null context makes the - // verb a no-op below anyway, so it answers barriered and the old path runs whole. - const Bool barriered = !runAhead || ctx == nullptr || ClientVerbIsBarriered(verb, *ctx); + // §2.1's predicate, client-side (see ClientVerbIsBarriered). + const Bool barriered = !runAhead || ClientVerbIsBarriered(verb, ctx); // THE ONE LINE THE RED-ONCE FLIPS: `= true` here is "fill for every verb", the // pre-P5e behaviour, and it turns the guard below into the abort §3.5 names. const Bool fillOwed = barriered; diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index 232ef337..165851e3 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -917,9 +917,19 @@ namespace MobileGL::MG_Pipe { // EmitAndWaitTails the client still blocks after every record, so the answer the server // must stamp is `true` whatever the wire says - the two named exemption scopes (§4.4) are // legal exactly while the client is parked, and it is parked behind all of them today. - // Package ra flips this to true in the same commit that lands the wait rule; the sink's - // stamp then follows the predicate. Computing the predicate anyway (PipeApplier::ApplyOne) - // is deliberate: it keeps the function exercised on every record for the whole phase. + // IT FLIPS WITH kMGPipeP5eRunAheadReady, IN THE INTEGRATION COMMIT, AND NOT WHEN ra LANDS + // (ra's amendment to ID-103's wording, which said "the same commit that lands the wait + // rule"). The argument ID-103 makes is about whether the CLIENT is parked, not about + // whether ra's code exists: RunAheadArmed() is a conjunction whose third term is the + // server's caps bit, so between ra landing and the caps bit being published the client + // still blocks after every record - and stamping `false` there would withdraw §4.4's + // exemptions from probes that client's own wait still makes safe, which is exactly the + // "refusal with no defect behind it" ID-103 refused. The two constants are therefore ONE + // switch with two spellings, and the integration commit throws both. + // + // Computing the predicate anyway (PipeApplier::ApplyOne) is deliberate: it keeps the + // function exercised on every record for the whole phase rather than first run on the day + // it starts deciding. inline constexpr Bool kMGPipeP5eClientWaitRuleLanded = false; // P5c (rv): the two serials the PipeInputs texture-shutter accessors answer with under a diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index fc6699ea..50961b30 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -47,6 +47,7 @@ #include #include #include +#include #include #include #include @@ -2062,8 +2063,379 @@ TEST(RemoteGuards, ClientPipeInputsFillWithTheApplierIdleIsAllowed) { }); ExpectChildSuccess(child); } + +// =========================================================================================== +// P5e (ra) - the wait rule, the present credit and gPipeInputs' ownership +// (MG_Remote/CONTRACT-P5E.md §1, §2.3, §2.4, §3.1, §3.3, §3.5) +// =========================================================================================== +// +// EVERY CASE BELOW ARMS RUN-AHEAD BY PUBLISHING THE CAP BIT, because that is the only thing +// that arms it: RunAheadArmed() is a conjunction whose third term is the SERVER's own +// statement that it applies an unbarriered record without reading client memory, and in a +// tree where kMGPipeP5eRunAheadReady is still false no production session sets it. Driving the +// arm through the caps mirror rather than through a test-only setter is deliberate (ID-102's +// lesson): what these cases exercise is then the production latch and not a branch beside it. +// +// NONE OF THEM DRAWS. The draw path is the family packages' to migrate; until they land, a +// draw under run-ahead would abort inside the backend for a reason that has nothing to do with +// what is being asserted here. +void StartRunAheadSession() { + ::alarm(15); + MG_Config::Transport = MG_Config::TransportMode::InProcess; + MG_Config::Ipc.RunAhead = 1; + Srv::ServerSessionInstance().SetCapabilityBits( + static_cast(MG_Pipe::kCapRunAheadApply)); + Srv::ServerSessionInstance().SetConsumedSubsystems(kMGPipeSubsystemsMigratedAtP4a); + // THE MIRROR IS ADOPTED BEFORE Start, AND THAT IS THE LATCH'S RULE MADE VISIBLE. This + // process has no server backend, so PublishCapsSnapshot is deferred and the handshake + // carries no snapshot at all - and §1 says a later snapshot may only turn run-ahead OFF, + // never on. So the arm has to be true at the FIRST adoption, which for a fixture means + // before the session takes its latch. A test that adopted afterwards and found the client + // still lockstep would be reading the contract's own rule as a failure. + { + MG_Pipe::MGPCaps caps{}; + caps.CallMask = static_cast(MG_Pipe::kCapRunAheadApply) | + MG_Remote::MGCapsConsumerBits(kMGPipeSubsystemsMigratedAtP4a); + CapsMirrorInstance().Adopt(caps, MG_Backend::FormatCapabilityCache{}, RendererInfo{}, + String{"4.6"}, BackendType::DirectGLES); + } + if (ClientSessionInstance().Start(MG_Config::TransportMode::InProcess, {}) != MOBILEGL_OK) { + ::_exit(81); + } + if (!ClientSessionInstance().RunAheadArmed()) ::_exit(83); // the latch never took +} + +// THE SERVER HALF OF §2.4, SUBSTITUTED - and it is substituted because this fixture has no +// backend object at all, so ServerVerbSink::OnPresent DECLINES before it can reach +// ServerSession::ReturnPresentCredit ("present arrived with no backend object"). What the +// cases below assert is therefore the CLIENT's half of the credit: that it pays, when, and in +// whose id space. That the server returns exactly one credit per swap, after Present() has +// returned, is PipeApplier::OnPresent's own three lines and the device exit's `credit-waits` +// reading; a peer here cannot pin it without a backend to swap on. +struct PresentCreditPeer : Codec::WireVerbSink { + std::atomic lastSerial{0}; + Bool OnPresent(const MGPPresent& present) override { + lastSerial.store(present.FrameSerial); + Srv::ServerSessionInstance().ReturnPresentCredit(present.FrameSerial); + return true; + } + void Install() { + if (Srv::ServerLoopInstance().RunOnApplyThread( + [](void* self) { + auto& decoder = Srv::ServerSessionInstance().Applier().*PeerMember(DecoderTag{}); + decoder.SetVerbSink(static_cast(self)); + return MOBILEGL_OK; + }, + this) != MOBILEGL_OK) { + ::_exit(82); + } + } +}; + +// §1: the conjunction, and the half of it that is not the client's to decide. The cap bit is +// absent here, so the knob alone changes nothing - which is the whole reason the knob is +// parsed on every arm rather than only where it means something (Config.h). +TEST(RemoteRunAhead, TheKnobAloneDoesNotArmRunAheadWithoutTheServersCapBit) { + const auto child = RunInChild([] { + MG_Config::Ipc.RunAhead = 1; + StartControlSession(); // SetCapabilityBits(0) + if (ClientSessionInstance().RunAheadArmed()) ::_exit(101); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + +// §1 again, from the other side: MOBILEGL_IPC_VERB_BARRIER=0 is the LOCKSTEP arm's negative +// control and it disarms run-ahead outright, because the barrier is the first term of the +// conjunction. Under run-ahead its documented red is the first barriered row's stale pull - +// that one is a scenario, not a unit case; what is pinned here is that the two controls do not +// silently compose into a third mode nobody designed. +TEST(RemoteRunAhead, ClearingTheVerbBarrierDisarmsRunAheadEvenWithTheCapBit) { + const auto child = RunInChild([] { + MG_Config::Ipc.RunAhead = 1; + MG_Config::Ipc.VerbBarrier = 0; + ::alarm(15); + MG_Config::Transport = MG_Config::TransportMode::InProcess; + Srv::ServerSessionInstance().SetCapabilityBits( + static_cast(MG_Pipe::kCapRunAheadApply)); + Srv::ServerSessionInstance().SetConsumedSubsystems(kMGPipeSubsystemsMigratedAtP4a); + MG_Pipe::MGPCaps caps{}; + caps.CallMask = static_cast(MG_Pipe::kCapRunAheadApply) | + MG_Remote::MGCapsConsumerBits(kMGPipeSubsystemsMigratedAtP4a); + CapsMirrorInstance().Adopt(caps, MG_Backend::FormatCapabilityCache{}, RendererInfo{}, + String{"4.6"}, BackendType::DirectGLES); + if (ClientSessionInstance().Start(MG_Config::TransportMode::InProcess, {}) != MOBILEGL_OK) { + ::_exit(81); + } + if (ClientSessionInstance().RunAheadArmed()) ::_exit(101); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + +// §2.4's RED-ONCE, BY COUNT. With a credit of 1 the client may have one present in flight, so +// the second and third presents each pay the credit before they encode - `credit-waits` is 2 +// after three presents and the present-ack watermark has reached 2. Delete the +// WaitForPresentAck arm from ClientSession::AcquirePresentCredit and the counter stays 0 while +// all three presents publish: red by count, which is the only way a wait that is usually +// already satisfied can be tested at all. +// +// The serial is asserted too, because the counter alone would survive a credit paid against +// the wrong id space (the pre-P5e FrameSerial was 0 and the server stamped its own). +TEST(RemoteRunAhead, ACreditOneClientPaysTheCreditAtEveryPresentAfterTheFirst) { + const auto child = RunInChild([] { + MG_Config::Ipc.PresentCredit = 1; + StartRunAheadSession(); + PresentCreditPeer peer; + peer.Install(); + ClientSession& session = ClientSessionInstance(); + if (session.PresentCreditWaits() != 0) ::_exit(101); + RemoteEmitTable().Present(); + if (session.PresentCreditWaits() != 0) ::_exit(102); // the first one is free + RemoteEmitTable().Present(); + RemoteEmitTable().Present(); + if (session.PresentCreditWaits() != 2) ::_exit(103); + // ONE CREDIT PER SWAP, in the client's own 1-based space: the server returned the + // second present's credit, so the watermark is at least 2. + if (session.Control() == nullptr || + session.Control()->presentAckSerial.load() < 2u) { + ::_exit(104); + } + session.Stop(); + }); + ExpectChildSuccess(child); +} + +// §2.4's other half: with the credit raised the client stops paying until it is that far +// ahead. It is the control that keeps the case above from passing because of an unconditional +// counter rather than because of the credit. +TEST(RemoteRunAhead, ACreditThreeClientPaysNothingForItsFirstThreePresents) { + const auto child = RunInChild([] { + MG_Config::Ipc.PresentCredit = 3; + StartRunAheadSession(); + PresentCreditPeer peer; + peer.Install(); + ClientSession& session = ClientSessionInstance(); + RemoteEmitTable().Present(); + RemoteEmitTable().Present(); + RemoteEmitTable().Present(); + if (session.PresentCreditWaits() != 0) ::_exit(101); + RemoteEmitTable().Present(); + if (session.PresentCreditWaits() != 1) ::_exit(102); + session.Stop(); + }); + ExpectChildSuccess(child); +} + +// §3.1 / §3.5's RED-ONCE. `Clear` is a kWaitNone row, so under run-ahead the client publishes +// it and moves on - and therefore does not fill gPipeInputs for it. This case is the GREEN +// half: the validate point runs the tracker walk and the emitters and touches the block not at +// all, so nothing aborts. +// +// THE RED: in MGPipeValidateForVerb change `const Bool fillOwed = barriered;` to `= true` - +// which is exactly "leave one CopyField in an unbarriered fill" - and the guard below it fires +// Fatal{RoleViolation, "gPipeInputs"} on this very call. +TEST(RemoteRunAhead, AnUnbarrieredVerbDoesNotFillPipeInputsAndIsAllowed) { + const auto child = RunInChild([] { + StartRunAheadSession(); + MGPipeValidateForVerb(MGPipeVerb::Clear); + MGPipeLeaveVerb(); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + +// §3.5, the aborting arm, driven at the guard itself: a GL-thread touch that is NOT the +// residual fill of a barriered record. Under lockstep this same call is a no-op (the apply +// thread is not inside the applier and MOBILEGL_IPC_BATCH_WAITS is 1); under run-ahead there +// is no such window to be outside of, because the client never parks for an unbarriered +// record - so the answer stops depending on timing and becomes the rule. +TEST(RemoteGuards, ClientPipeInputsTouchOutsideABarrieredFillUnderRunAheadIsFatalByName) { + const auto child = RunInChild([] { + StartRunAheadSession(); + ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt("ra-test-surface", + /*isBarrieredFill=*/false); + ClientSessionInstance().Stop(); + }); + ExpectNamedAbort(child, "Fatal{RoleViolation, \"gPipeInputs\"}"); +} + +// ... and the control: the barriered fill says so and is let through. Without this the case +// above would pass just as well against "abort unconditionally". +TEST(RemoteGuards, ClientPipeInputsTouchInsideABarrieredFillUnderRunAheadIsAllowed) { + const auto child = RunInChild([] { + StartRunAheadSession(); + ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt("ra-test-surface", + /*isBarrieredFill=*/true); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + +// §3.3's RED-ONCE, and the one that turns the strict lane into a gate. The probe is the +// sticky forward itself, run on the apply thread inside a server-stamped verb with the +// current record marked UNBARRIERED - which is the state the sink puts that thread in for +// every kWaitNone row once the wait rule is live. MOBILEGL_IPC_STRICT_ERRORS is deliberately +// NOT set: the point is that the knob has stopped being the deciding input, because a row the +// client never filled has no value to count. +// +// THE RED: put CountBarrierPull's first line back to `if (MG_Config::Ipc.StrictErrors)` only +// and this case stops aborting - and, on the lane, every scenario that still pulls a row goes +// from a named abort to a wrong picture with a number beside it. +TEST(RemoteGuards, AResidualPullUnderAnUnbarrieredRecordIsFatalWithoutTheStrictKnob) { + const auto child = RunInChild([] { + MG_Config::Ipc.StrictErrors = false; + StartRunAheadSession(); + Srv::ServerLoopInstance().RunOnApplyThread( + +[](void*) -> MobileGLResult { + MG_Pipe::MGPipeServerStampVerbBoundary(MGPipeVerb::Clear); + MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(false); + MG_Pipe::MGPipeStickyForwardPull(MGPipeInputField::GetProgramObject); + return MOBILEGL_OK; + }, + nullptr); + ClientSessionInstance().Stop(); + }); + ExpectNamedAbort(child, "Fatal{UnmigratedPipeInput, \"GetProgramObject@Clear\"}"); +} + +// The control: the SAME probe inside a BARRIERED record is P5C's counted residual pull, and +// with the strict knob off it counts and carries on. Ruling 4 is this line - a barriered +// record keeps P5C's semantics exactly, because the client is parked in its own wait. +TEST(RemoteGuards, AResidualPullUnderABarrieredRecordStillOnlyCounts) { + const auto child = RunInChild([] { + MG_Config::Ipc.StrictErrors = false; + StartRunAheadSession(); + Srv::ServerLoopInstance().RunOnApplyThread( + +[](void*) -> MobileGLResult { + MG_Pipe::MGPipeServerStampVerbBoundary(MGPipeVerb::Clear); + MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(true); + const Uint64 before = MG_Pipe::MGPipeResidualPullCount(); + MG_Pipe::MGPipeStickyForwardPull(MGPipeInputField::GetProgramObject); + if (MG_Pipe::MGPipeResidualPullCount() != before + 1) ::_exit(101); + return MOBILEGL_OK; + }, + nullptr); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + +// §2.6's RED-ONCE, and the one the brief names: ~300 KiB of kEventGpuWritten published from +// the apply thread behind a sequence nobody waited for. SEG_EVENT is 256 KiB, so this is +// several ringfuls; under P5C's rule the FIRST Reserve that failed was Fatal{EventRingOverflow} +// and this case aborts by that name. With flow control the producer publishes, rings, parks on +// the latch and retries, and the burst completes. +// +// THE DRAIN RUNS ON A THREAD OF ITS OWN, and that is forced by the fixture rather than chosen: +// RunOnApplyThread is synchronous, so the thread that posted the burst cannot also be the +// thread that empties the ring. In production it is the GL thread draining at its own waits +// (§2.6's deadlock argument); here it is a drainer beside the poster, which puts the producer +// in exactly the state that argument describes. +TEST(RemoteRunAhead, AnEventBurstBehindAnUnwaitedSequenceFlowControlsInsteadOfAborting) { + const auto child = RunInChild([] { + StartRunAheadSession(); + std::atomic stop{false}; + std::atomic delivered{0}; + std::thread drainer([&stop, &delivered] { + // THE FIRST DRAIN IS DELAYED ON PURPOSE. A drainer that starts immediately can + // keep a 256 KiB ring from ever being full, and a red-once that depends on the + // scheduler losing a race is not a red-once. 50 ms is four orders of magnitude + // more than the burst needs to fill the ring, so the producer is provably parked + // on the latch before anything empties it. + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + while (!stop.load(std::memory_order_acquire)) { + delivered.fetch_add(ClientSessionInstance().DrainPublishedEvents()); + std::this_thread::yield(); + } + delivered.fetch_add(ClientSessionInstance().DrainPublishedEvents()); + }); + // 3000 records of 64 ranges each is ~3 MB against a 256 KiB ring - twelve ringfuls, + // with a drainer racing it - so the full latch is hit many times over rather than + // maybe once. That margin is what makes the red-once (restore the Fatal) reliable: + // a burst that merely might fill the ring would be a red-once that merely might be red. + Srv::ServerLoopInstance().RunOnApplyThread( + +[](void*) -> MobileGLResult { + MG_Pipe::MGPRange ranges[64]; + for (Uint32 r = 0; r < 64; ++r) ranges[r] = MG_Pipe::MGPRange{r * 64ull, 64ull}; + for (Uint32 i = 0; i < 3000; ++i) { + MG_Pipe::gMGPipeCallbacks.OnGpuWritten(MG_Pipe::MGPipeHandle{7, 1}, 64, ranges); + } + return MOBILEGL_OK; + }, + nullptr); + stop.store(true, std::memory_order_release); + drainer.join(); + // LOSSLESS, which is the property flow control had to preserve: every one of the 3000 + // records crossed. A drop policy would have been the other way to survive a full ring, + // and P5C's §4.4 refuses it - a writeback or a GPU-write mark that did not arrive is a + // stale buffer, not a missing statistic. + if (delivered.load() < 3000u) ::_exit(101); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + +// §2.7 / ruling 13: the deferred-destroy queue stays, and an enqueue from an unbarriered +// apply is the finding. Under strict it is the abort; this is the arm the lane owns. +TEST(RemoteGuards, ADeferredDestroyFromAnUnbarrieredApplyIsFatalUnderStrict) { + const auto child = RunInChild([] { + MG_Config::Ipc.StrictErrors = true; + StartRunAheadSession(); + Srv::ServerLoopInstance().RunOnApplyThread( + +[](void*) -> MobileGLResult { + MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(false); + (void)MG_Pipe::MGPipeDeferDestroyAndFreeIfOnApplyThread(MG_Pipe::MGPipeKind::Texture, + 4242); + return MOBILEGL_OK; + }, + nullptr); + ClientSessionInstance().Stop(); + }); + ExpectNamedAbort(child, "Fatal{RoleViolation, \"deferred-destroy\"}"); +} + +// The control, and the whole of ruling 13: a BARRIERED apply may still be a last owner (its +// fill's O-class rows, XFB's pinned targets), so the queue takes the death and the GL thread +// replays it. Deleting the queue would have made this an allocator touch from the server role. +TEST(RemoteGuards, ADeferredDestroyFromABarrieredApplyIsStillServed) { + const auto child = RunInChild([] { + MG_Config::Ipc.StrictErrors = true; + StartRunAheadSession(); + Srv::ServerLoopInstance().RunOnApplyThread( + +[](void*) -> MobileGLResult { + MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(true); + if (!MG_Pipe::MGPipeDeferDestroyAndFreeIfOnApplyThread(MG_Pipe::MGPipeKind::Texture, + 4243)) { + ::_exit(101); + } + return MOBILEGL_OK; + }, + nullptr); + MG_Pipe::MGPipeDrainDeferredDestroys(); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} #endif +// §2.5 / ruling 15 (ID-93): which half of resource_subdata wants its answer. One predicate, +// read by MGPipeRouteResourceSubData and by Wire_ResourceSubData - a second spelling of it is +// how the client comes to wait for an answer the emitter told the server not to bother with. +// The buffer target's whole packed field is 0 (MGPipeTypes.h asserts it beside the packer), +// which is what makes the test a comparison and not a mask. +TEST(RemoteRunAhead, OnlyTheTextureHalfOfResourceSubDataWantsItsReply) { + MG_Pipe::MGPSubData buffer{}; + buffer.Target = MG_Pipe::MGPipePackSubDataTarget(MG_Pipe::kMGPipeResourceTargetBuffer, 0u); + EXPECT_FALSE(MG_Pipe::MGPipeSubDataWantsItsReply(buffer)); + + MG_Pipe::MGPSubData texture{}; + texture.Target = MG_Pipe::MGPipePackSubDataTarget( + static_cast(MG_Pipe::MGPipeResourceTarget::Tex2D), + static_cast(TextureUploadTarget::Texture2D)); + EXPECT_TRUE(MG_Pipe::MGPipeSubDataWantsItsReply(texture)); +} + int main(int argc, char** argv) { namespace fs = std::filesystem; diff --git a/MobileGL/MG_Test/Wire/SessionTest.cpp b/MobileGL/MG_Test/Wire/SessionTest.cpp index 2d33969d..668f69e7 100644 --- a/MobileGL/MG_Test/Wire/SessionTest.cpp +++ b/MobileGL/MG_Test/Wire/SessionTest.cpp @@ -550,6 +550,122 @@ TEST(SessionTest, PresentAckSerialIsWaitedOnWithGreaterOrEqualAndWakesThroughThe EXPECT_EQ(session->Control().producerParked.load(), 0u); } +// --------------------------------------------------------------------------- +// P5e (ra) - SEG_EVENT flow control (MG_Remote/CONTRACT-P5E.md §2.6) +// --------------------------------------------------------------------------- +// +// THE DEADLOCK THE CONTRACT ARGUES AWAY HAS TWO HALVES AND THIS IS THE FIRST. A run-ahead +// server that cannot Reserve on SEG_EVENT stops producing and therefore stops applying, so +// appliedSeq stops moving - and the client parked on appliedSeq is holding the only drain +// there is. A waiter that could ONLY be woken by the watermark would wait for a number nobody +// is going to publish. +// +// Red-once: point the case at WaitForApplied instead of WaitForAppliedOrEventBacklog and the +// join below never returns - the 5 s deadline fires and the wait reads TimedOut. +TEST(SessionTest, AFullEventRingWakesAClientParkedOnAppliedSeq) { + auto session = std::make_shared(); + ASSERT_TRUE(session->Build(TestSizes())); + + std::atomic released{false}; + std::atomic result{SessionWait::TimedOut}; + std::thread parked([session, &released, &result] { + result.store(session->producer.WaitForAppliedOrEventBacklog(4, 5000)); + released.store(true, std::memory_order_release); + }); + + while (session->Control().producerParked.load() == 0 && !released.load()) { + std::this_thread::yield(); + } + // NOT the watermark. The server latches "SEG_EVENT is full and I stopped" and rings; the + // client's business is to drain, not to conclude that its record was applied. + session->Control().eventRingFull.store(1, std::memory_order_release); + session->consumer.NotifyClient(); + + parked.join(); + EXPECT_TRUE(released.load()); + EXPECT_EQ(result.load(), SessionWait::Reached); + // AND THE WATERMARK IS UNTOUCHED, which is what makes the caller's re-read the right way + // to tell the two wakeups apart: a third SessionWait value would have turned every + // existing `== Reached` site into a bug. + EXPECT_EQ(session->Control().appliedSeq.load(), 0u); + EXPECT_TRUE(session->producer.EventRingIsFull()); +} + +// The same for the present credit, because a credit-paced client parks there for a whole frame +// at a time and is the likeliest waiter to be holding the drain when the ring fills. +TEST(SessionTest, AFullEventRingWakesAClientParkedOnThePresentCredit) { + auto session = std::make_shared(); + ASSERT_TRUE(session->Build(TestSizes())); + + std::atomic released{false}; + std::atomic result{SessionWait::TimedOut}; + std::thread parked([session, &released, &result] { + result.store(session->producer.WaitForPresentAckOrEventBacklog(2, 5000)); + released.store(true, std::memory_order_release); + }); + + while (session->Control().producerParked.load() == 0 && !released.load()) { + std::this_thread::yield(); + } + session->Control().eventRingFull.store(1, std::memory_order_release); + session->consumer.NotifyClient(); + + parked.join(); + EXPECT_TRUE(released.load()); + EXPECT_EQ(result.load(), SessionWait::Reached); + EXPECT_EQ(session->Control().presentAckSerial.load(), 0u); +} + +// THE SECOND HALF: the drain that empties the ring must RING, not merely clear. A cleared flag +// with no bell is the lost wakeup the forward direction's publish-then-ring order exists to +// prevent, and the server parked on this flag is inside a producer that cannot return without +// placing its event. +// +// THE BELL IS COUNTED RATHER THAN WAITED ON, and that is the difference between a test and a +// coincidence: a real doorbell REMEMBERS a notify delivered while nobody was parked and can +// return from a park for reasons of its own, so a thread that happened to wake proves nothing +// about who rang. This one records every Notify(). +// +// Red-once: drop the NotifyIfParked from EventRingConsumer::Drained() and `rings` stays 0. +namespace { + struct CountingBell : Doorbell { + std::atomic rings{0}; + void Notify() override { rings.fetch_add(1, std::memory_order_relaxed); } + bool Park(std::uint32_t) override { return false; } + void Reset() override {} + }; +} // namespace + +TEST(SessionTest, DrainingAFullEventRingClearsTheLatchAndRingsTheServer) { + auto session = std::make_shared(); + ASSERT_TRUE(session->Build(TestSizes())); + + CountingBell serverBell; + // The bell the CLIENT rings is the one the SERVER parks on - which is which is the + // session's knowledge and not the transport's (contract §3.9), so the fixture spells the + // pair the same way ClientSession::Start does. + session->eventIn.SetServerDoorbell(&serverBell); + + // A drain with the latch DOWN is the steady state - every wait exit drains - and it must + // not ring: a store to a shared cache line per drain is what the doorbell design exists to + // avoid (Doorbell.h's bidirectional argument). + session->eventIn.Drained(); + EXPECT_EQ(serverBell.rings.load(), 0u); + + // Now the server is parked on the latch, which is what the flag means. + session->Control().eventRingFull.store(1, std::memory_order_release); + session->Control().consumerParked.store(1, std::memory_order_release); + session->eventIn.Drained(); + + EXPECT_EQ(session->Control().eventRingFull.load(), 0u); + EXPECT_EQ(serverBell.rings.load(), 1u); + + // ONE-SHOT: the latch is down again, so the next drain is silent. Without the exchange - + // a plain store plus a read - a steady stream of drains would ring every time. + session->eventIn.Drained(); + EXPECT_EQ(serverBell.rings.load(), 1u); +} + // --------------------------------------------------------------------------- // kRecPad (R-9's last sentence, and the one with no other detector) // ---------------------------------------------------------------------------