swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-11-07 18:13:23 +09:00
Code Issues Packages Projects Releases Wiki Activity

[MG_Remote, MG_Test, scripts] (P7): DrainRing checks the session latch immediately before every pop, so a latch the control thread stores between two records stops the next one

Browse Source
This commit is contained in:
swung0x48
2026-09-23 17:04:13 -04:00
parent 1052b13a1a
commit 03bcf5e7fb
6 changed files with 145 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Remote/FatalFunnel.h
+1 -1
View File
@@ -77,7 +77,7 @@ namespace MobileGL::MG_Remote {
// SessionFail calls: the string must carry its `Fatal{Word` and the word must have a .def row.
void ArmSessionLatch();
bool SessionLatchArmed();
// True once the first fault has latched. One acquire load: DrainRing asks it per record.
// True once the first fault has latched. One acquire load: DrainRing asks it before every pop.
bool SessionLatched();
// The first latched fault - its family and its line, verbatim. Meaningful only once
// SessionLatched() is true; the line's storage lives for the process.
MobileGL/MG_Remote/Server/ServerLoop.cpp
+34 -17
View File
@@ -657,25 +657,37 @@ namespace MobileGL::MG_Remote::Server {
// allocation; `applied == 0` then skips the retire block below entirely. Keep it that
// way: anything added here runs ~900 times a frame doing nothing.
//
// PH-1 (3), ID-P7-1: THE LATCH CHECK, TWICE, AND EACH ONE HAS A JOB THE OTHER CANNOT DO.
// Once a named fault has latched (an armed spawn / TCP session child only - unarmed this is
// one acquire load of a flag nobody sets), no further record is applied: the rest of the
// ring is declined unread, the apply thread leaves its loop, and RunSession closes the
// session by name.
// * HERE, at the top: a drain ENTERED latched pops nothing. That is the apply thread's
// exit-path drain after it left on the latch, and any drain after RunSession's thread
// latched a control op. ServerLoopTest's latched-batch case goes red without it (the
// exit-path drain applies the record behind the latched one).
// * AFTER EACH RECORD, at the bottom of the loop: a record that latched is the last one
// this drain pops, even when the peer published more behind it in the same batch
// (PeerLatchTest's two-record case and ServerLoopTest's latched-batch case).
// A check at the loop's head instead would only repeat this one on the first iteration.
if (SessionLatched()) return 0;
// PH-1 (3), ID-P7-1: THE LATCH CHECK, ONCE, IMMEDIATELY BEFORE EVERY POP. Once a named
// fault has latched (an armed spawn / TCP session child only - unarmed this is one acquire
// load of a flag nobody sets), no further record is applied: the rest of the ring is
// declined unread, the apply thread leaves its loop, and RunSession closes the session by
// name. The one check does three jobs, and each has a case that goes red without it:
// * a drain ENTERED latched pops nothing - the apply thread's exit-path drain after it
// left on the latch, and any drain after RunSession's thread latched a control op
// (ServerLoopTest's latched-batch case: the exit-path drain applies the record behind
// the latched one);
// * a record that latched is the last one this drain pops, even when the peer published
// more behind it in the same batch (PeerLatchTest's two-record case, ServerLoopTest's
// 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.
//
// 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.
ServerSession& session = *m_session;
Transport::SessionConsumer& consumer = session.Consumer();
PipeApplier& applier = session.Applier();
Uint64 applied = 0;
for (;;) {
if (SessionLatched()) break;
bool corrupt = false;
const bool popped = consumer.ApplyOne(
[this, &applier](const Transport::RingRecordView& record) {
@@ -708,12 +720,17 @@ namespace MobileGL::MG_Remote::Server {
}
if (!popped) break;
++applied;
if (SessionLatched()) break; // PH-1 (3): the per-record check the top of this function describes
// PH-6: the record whose event forfeited the reverse channel is the last one this
// session applies. Checked only AFTER a popped record, so the empty-ring answer above
// still costs its two loads and nothing else. (The latch check sits directly under
// `++applied` because ph_latch_sites.py's MECHANICS row pins exactly that shape.)
// still costs its two loads and nothing else. (A record that LATCHED is stopped by the
// loop-head check on the next iteration - ph_latch_sites.py's MECHANICS row pins that
// 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.
if (const auto hook = m_betweenRecordsHook.load(std::memory_order_acquire)) hook();
}
if (applied != 0) {
// THE TWO TALLIES MUST AGREE, AND THAT IS WHAT MAKES R-9's BATCHING BAN CHECKABLE.
MobileGL/MG_Remote/Server/ServerLoop.h
+8
View File
@@ -310,6 +310,13 @@ namespace MobileGL::MG_Remote::Server {
void SetBeforeRetireHookForTesting(void (*hook)()) {
m_beforeRetireHook.store(hook, std::memory_order_release);
}
// Scheduling point only, same discipline: DrainRing runs the hook on the apply thread
// BETWEEN two records - after a popped record's own checks, before the next pop's latch
// check - and never on an empty-ring poll. ServerLoopLatchTest latches from a second
// thread there, which is the interleaving codex closeout finding 6 named.
void SetBetweenRecordsHookForTesting(void (*hook)()) {
m_betweenRecordsHook.store(hook, std::memory_order_release);
}
// C7 / ID-54 diagnostics, read by ServerLoopTest's C7 and N-3 controls. NativeBindCount is
// how many times ApplyMakeCurrent FORWARDED a bind to the backend (a tuple it did not
@@ -459,6 +466,7 @@ namespace MobileGL::MG_Remote::Server {
// that reached the blocking Park. Relaxed everywhere: it is a gauge.
std::atomic<Uint64> m_parkBlocks{0};
std::atomic<void (*)()> m_beforeRetireHook{nullptr};
std::atomic<void (*)()> m_betweenRecordsHook{nullptr};
// C7 / ID-54: the (dpy, draw, read, ctx) currently bound on the apply thread. Written and
// read ONLY on the apply thread inside ApplyMakeCurrent, so it needs no lock; the two
MobileGL/MG_Test/Wire/PeerLatchTest.cpp
+5 -6
View File
@@ -33,7 +33,7 @@
//
// THE DECLINE-AND-CLOSE HALF has its own cases below the table (they are not sites, so they are
// not rows; scripts/ci/ph_latch_sites.py's MECHANICS list names them): a latched record is the
// last one its batch applies (DrainRing's two latch checks), and a latch raised on the apply
// last one its batch applies (DrainRing's pre-pop latch check), and a latch raised on the apply
// thread closes the session while the peer is still connected and silent (RunSession's sliced
// control wait). ServerLoopTest holds the in-process half of the same checks.
//
@@ -394,7 +394,7 @@ namespace {
// SEVERAL IMAGES, ONE PUBLISH: each gets its own filler (its ring position is taken right
// after that filler is encoded, so a wrap pad between two of them cannot shift it), all are
// overwritten, and one PublishAndNotify makes them visible to the server together - the shape
// of a peer that batches, which is what DrainRing's per-record latch check is for.
// of a peer that batches, which is what DrainRing's pre-pop latch check stops mid-batch.
// `mutateHeader` applies to the FIRST image's header; `outSeq` is the LAST record's seq.
bool PublishImages(Client::ClientSession& client, const std::vector<std::vector<Uint8>>& images,
Uint64* outSeq,
@@ -1681,10 +1681,9 @@ namespace {
}
} // namespace
// DrainRing's two latch checks, over the wire: the record behind the latched one in the same batch
// is never applied - its own named fault never reaches the log - and the session closes having
// latched exactly one. Red with either check deleted: the top one (the apply thread's exit-path
// drain then applies the second record) or the per-record one (the same drain does).
// DrainRing's pre-pop latch check, over the wire: the record behind the latched one in the same
// batch is never applied - its own named fault never reaches the log - and the session closes
// having latched exactly one. Red with the check deleted (the same drain applies the second record).
TEST(PeerLatchTest, ALatchedRecordIsTheLastRecordItsBatchApplies) {
Supervisor sup;
ASSERT_TRUE(Launch("batch", /*tcp=*/false, &sup));
MobileGL/MG_Test/Wire/ServerLoopTest.cpp
+83 -4
View File
@@ -2912,12 +2912,79 @@ namespace {
fixture.Stop();
::_exit(failed);
}
// A LATCH FROM ANOTHER THREAD, BETWEEN TWO RECORDS OF ONE BATCH (codex closeout finding 6).
// 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.
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
kBetweenWrongFault = 4, // not exactly one fault latched, or it is not the control thread's
kBetweenWatermark = 8, // appliedSeq moved past the first record
kBetweenHookCalls = 16, // the hook did not run exactly once (the case did not interleave)
};
std::atomic<int> g_betweenRecordsCalls{0};
void LatchFromAControlThreadOnTheFirstCall() {
if (g_betweenRecordsCalls.fetch_add(1, std::memory_order_acq_rel) != 0) return;
std::thread control([] {
(void)MobileGL::MG_Remote::SessionLatch(
MobileGL::MG_Remote::MGFatalFamily::ProtocolCorruption,
"MGPipe: Fatal{ProtocolCorruption, \"unit.between-records\"} - latched by a control "
"thread after the apply thread's checks on the first record");
});
control.join();
}
[[noreturn]] void LatchFromAnotherThreadBetweenTwoRecordsAndExit() {
Remote::ArmSessionLatch();
ServerFixture fixture;
if (!fixture.Handshake() || !fixture.StartLoop()) ::_exit(64);
if (!fixture.WaitUntilTrulyParked()) ::_exit(65);
Server::ServerLoop& loop = Server::ServerLoopInstance();
loop.SetBetweenRecordsHookForTesting(&LatchFromAControlThreadOnTheFirstCall);
// Three LEGAL draws under ONE publish (the draw ARecordShorterThanItsType... proves applies
// in an armed child without latching), so nothing but the control thread's latch can stop
// the batch.
MG_Pipe::MGPDrawInfo info{};
info.Mode = 0x0004; // GL_TRIANGLES
info.InstanceCount = 1;
info.MinIndex = ~0u;
info.MaxIndex = ~0u;
info.NumDraws = 1;
const MG_Pipe::MGPDrawRange range{0, 3, 0};
Uint64 last = Codec::kInvalidSeq;
for (int i = 0; i < 3; ++i) {
last = fixture.encoder.EncodeRecord(MG_Pipe::MGPWireOp::DrawVbo, &info, sizeof(info), &range,
sizeof(range));
if (last == Codec::kInvalidSeq) ::_exit(66);
}
fixture.encoder.Publish();
fixture.producer.PublishAndNotify(last);
int failed = 0;
if (!PollUntil([&] { return !loop.Running(); }, 3000)) failed |= kBetweenThreadStayed;
if (loop.DrainedRecords() != 1) failed |= kBetweenDrainedMore;
if (Remote::SessionLatchCount() != 1 ||
std::strstr(Remote::SessionLatchedLine(), "\"unit.between-records\"") == nullptr) {
failed |= kBetweenWrongFault;
}
if (fixture.session->Consumer().AppliedSeq() != 1) failed |= kBetweenWatermark;
if (g_betweenRecordsCalls.load(std::memory_order_acquire) != 1) failed |= kBetweenHookCalls;
loop.SetBetweenRecordsHookForTesting(nullptr);
fixture.Stop();
::_exit(failed);
}
} // namespace
// DrainRing's two latch checks and the apply thread's own exit, in process. Red with the top
// check deleted (the exit-path drain applies the null-handle record: bits 2|4|16), with the
// per-record check deleted (the same drain applies both records behind it: bits 2|4|16), and with
// ApplyThreadMain's latched break deleted (bit 1: the thread spins on a ring it will not drain).
// DrainRing's pre-pop latch check and the apply thread's own exit, in process. Red with the check
// deleted (the drain applies both records behind the latched one: bits 2|4|16 - the exit-path drain
// would too), and with ApplyThreadMain's latched break deleted (bit 1: the thread spins on a ring it
// will not drain).
TEST(ServerLoopLatchTest, ALatchedRecordEndsItsBatchAndTheApplyThreadLeavesWithoutAStop) {
EXPECT_EXIT(RunALatchedBatchAndExit(), ::testing::ExitedWithCode(0), ".*")
<< "bits: 1 = the apply thread was still running 3 s after the latch; 2 = a record behind the "
@@ -2959,6 +3026,18 @@ TEST(ServerLoopLatchTest, ARecordShorterThanItsTypeIsLatchedBeforeTheApplierStam
<< "bits: 1 = the Clear verb was stamped from a record shorter than MGPClear; 2 = the latched "
"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
// 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) {
EXPECT_EXIT(LatchFromAnotherThreadBetweenTwoRecordsAndExit(), ::testing::ExitedWithCode(0), ".*")
<< "bits: 1 = the apply thread was still running 3 s after the latch; 2 = a record behind the "
"latch point reached the applier; 4 = not exactly the control thread's fault latched; 8 = "
"appliedSeq moved past the first record; 16 = the between-records hook did not run exactly "
"once (64+ = setup)";
}
#endif
int main(int argc, char** argv) {
scripts/ci/ph_latch_sites.py
+14 -8
View File
@@ -23,7 +23,7 @@ behind popcount(Mask) == Count, so only the mismatch is reachable).
covering row in turn and requires its group to turn up unmapped.
THE DECLINE-AND-CLOSE HALF IS NOT A SITE, so it is not a row: MECHANICS below names each check
that makes a latched session decline the rest of its work and close (DrainRing's two latch checks,
that makes a latched session decline the rest of its work and close (DrainRing's pre-pop latch check,
the apply thread's own exit, the control pump's and SurfaceOpCodec's latched answers, RunSession's
sliced wait, the pre-gate ahead of the applier's stamp, SessionLatch's own bookkeeping), the code
that IS the check, and the cases that go red without it. The script requires both to exist.
@@ -75,14 +75,20 @@ WIRE_TESTS = ROOT / "MobileGL" / "MG_Test" / "Wire"
# (source, what the check does, a regex that IS the check, [(test file, case name)]) - each case goes
# red with the check deleted (the red-once is recorded in the package's NOTE).
MECHANICS = [
(REMOTE / "Server" / "ServerLoop.cpp", "DrainRing: a drain ENTERED latched pops nothing (the exit-path drain)",
r"Uint64 ServerLoop::DrainRing\(\) \{.*?if \(SessionLatched\(\)\) return 0;",
# ONE check, the first statement of DrainRing's loop, immediately before every pop. It replaced
# the two the F2 latch package shipped (one at the function's top, one under `++applied;`):
# codex closeout finding 6 showed a latch that another thread stores after the per-record check
# 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).
(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;",
[("ServerLoopTest.cpp", "ALatchedRecordEndsItsBatchAndTheApplyThreadLeavesWithoutAStop"),
("PeerLatchTest.cpp", "ALatchedRecordIsTheLastRecordItsBatchApplies")]),
(REMOTE / "Server" / "ServerLoop.cpp", "DrainRing: a record that latched is the last one its batch applies",
r"\+\+applied;\s*if \(SessionLatched\(\)\) break;",
[("ServerLoopTest.cpp", "ALatchedRecordEndsItsBatchAndTheApplyThreadLeavesWithoutAStop"),
("PeerLatchTest.cpp", "ALatchedRecordIsTheLastRecordItsBatchApplies")]),
("PeerLatchTest.cpp", "ALatchedRecordIsTheLastRecordItsBatchApplies"),
("ServerLoopTest.cpp", "ALatchFromAnotherThreadBetweenTwoRecordsStopsTheNextPop")]),
(REMOTE / "Server" / "ServerLoop.cpp", "ApplyThreadMain: the apply thread leaves its loop on the latch, no Stop()",
r"if \(SessionLatched\(\)\) \{\s*MGLOG_E\(\"MG_Remote server: mgl-srv-apply leaves its loop",
[("ServerLoopTest.cpp", "ALatchedRecordEndsItsBatchAndTheApplyThreadLeavesWithoutAStop")]),