diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index 10a9128a..2902ee72 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -671,14 +671,14 @@ namespace MobileGL::MG_Remote::Server { // latched-batch case); // * a latch ANOTHER thread stores between two records - RunSession's control thread // latching a malformed SurfaceOp while this drain is mid-batch - stops the next pop - // (codex closeout finding 6; ServerLoopLatchTest's between-records case). The first - // version checked at the top of the function and after each record instead, and a - // latch that landed after the per-record check was still followed by one more pop and - // apply before anything looked again. - // What remains is the window between this load and the pop itself, and it is not a gap - // in the guarantee but its linearization point: a latch stored after this load is ordered - // after the record the load admitted, exactly as a latch stored while that record runs is. - // Closing it would take a lock the control thread shares with every pop. + // when it is stored before this load (codex closeout finding 6; ServerLoopLatchTest's + // between-records case). The first version checked at the top of the function and after + // each record, and made one more load (the PH-6 forfeit check) before the next pop; + // checking HERE only NARROWS that control/apply window to this load-to-pop span, it + // does not close it. + // A latch stored inside that span still lets the one record already admitted be popped and + // applied; the session then ends at the next check, one record later. Closing the span would + // take a lock the control thread shares with every pop. // // The idle poll's cost is unchanged: this load replaces the function-top check the first // version made, and the empty-ring answer is still it plus Pop's cmdHead load. @@ -727,9 +727,11 @@ namespace MobileGL::MG_Remote::Server { // check as the first statement of this loop.) if (session.ReverseChannelForfeited()) break; // TEST-ONLY scheduling point BETWEEN two records: after this record's own checks and - // before the next pop's latch check, which is where a latch from RunSession's control - // thread used to slip one more record through. Only after a popped record, so the idle - // poll never pays for it; null outside ServerLoopLatchTest. + // before the next pop's latch check. A control-thread latch stored here is seen by that + // check (the first version let it through); one stored after the check and before the + // pop still is not - the window is narrowed to check-to-pop, not closed, and the session + // ends at the check after that record. Only after a popped record, so the idle poll never + // pays for it; null outside ServerLoopLatchTest. if (const auto hook = m_betweenRecordsHook.load(std::memory_order_acquire)) hook(); } if (applied != 0) { diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index 18d1a887..8c84248b 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -2917,8 +2917,11 @@ namespace { // RunSession's control thread latches a malformed SurfaceOp while the apply thread is mid-batch; // the interleaving the finding names is "just after the apply thread's per-record check". The // loop's between-records hook is that point, made deterministic: on its FIRST call - after the - // first record's own checks, before the second pop - it starts a thread that latches, and joins - // it, so the latch is stored by another thread and complete before the apply thread moves on. + // first record's own checks, before the second pop's latch check - it starts a thread that + // latches, and joins it, so the latch is stored by another thread and complete before the apply + // thread moves on. This pins the NARROWED window only: DrainRing's check-before-pop sees a latch + // stored here, but one stored between that check and the pop still lets that record through + // (the window is check-to-pop, not closed), and the session then ends at the next check. enum : int { kBetweenThreadStayed = 1, // the apply thread was still running 3 s after the latch, no Stop() kBetweenDrainedMore = 2, // a record behind the latch point reached the applier @@ -3027,8 +3030,8 @@ TEST(ServerLoopLatchTest, ARecordShorterThanItsTypeIsLatchedBeforeTheApplierStam "line is not record.Minimum; 4 = not exactly two records reached the applier (64+ = setup)"; } -// Codex closeout finding 6: a latch ANOTHER thread stores between two records of one batch stops the -// next pop. Red with DrainRing's pre-pop check put back to the F2 shape (a check at the function's +// Codex closeout finding 6: a latch ANOTHER thread stores between two records of one batch, before the +// next pop's check, stops that pop (the check-to-pop span itself stays open - narrowed, not closed). Red with DrainRing's pre-pop check put back to the F2 shape (a check at the function's // top and one under `++applied;`, the hook where it is now): the latch lands after the first // record's checks, the drain pops and applies the second draw, and only then looks again (bits 2|8). TEST(ServerLoopLatchTest, ALatchFromAnotherThreadBetweenTwoRecordsStopsTheNextPop) { diff --git a/scripts/ci/ph_latch_sites.py b/scripts/ci/ph_latch_sites.py index 596ff582..a6807e48 100644 --- a/scripts/ci/ph_latch_sites.py +++ b/scripts/ci/ph_latch_sites.py @@ -81,8 +81,9 @@ MECHANICS = [ # still let one more record be popped and applied. The one check does all three jobs - a drain # ENTERED latched pops nothing (the exit-path drain), a record that latched is the last one its # batch applies, and a latch from RunSession's control thread between two records stops the - # next pop - and each case below goes red with it deleted. The pattern may not leave DrainRing's - # body (`\n }\n` closes a member function in ServerLoop.cpp). + # next pop when stored before this check (it NARROWS the window to check-to-pop, not closes it; + # the session ends at the next check) - and each case below goes red with it deleted. The + # pattern may not leave DrainRing's body (`\n }\n` closes a member function in ServerLoop.cpp). (REMOTE / "Server" / "ServerLoop.cpp", "DrainRing: no record is popped once the session latched (checked immediately before every pop)", r"Uint64 ServerLoop::DrainRing\(\) \{(?:(?!\n \}\n).)*?for \(;;\) \{\s*if \(SessionLatched\(\)\) break;",