[MG_Remote, MG_Test, scripts] (P7): the latch comments say the check before every pop narrows the control-thread window to check-to-pop and the session ends at the next check

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) {

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) {

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;",