diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e3f848ef..18897431 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1562,7 +1562,8 @@ jobs: # --no-tests=error is half the gate, exactly as in integration-verify: the integration-split # entries exist only when the library was configured with -DMOBILEGL_BUILD_DISAGGREGATED=ON, # so a build that lost the option matches nothing and reds here instead of reporting a green - # run of nothing. The other half is the transport-resolution check below. + # run of nothing. The other half is in-test: the split fixture fails an armed entry whose + # process did not resolve the transport its registration claims. # # An entry whose owning package (c1 client, s1 session, v1 server) has not landed SKIPS with # the reason and never goes green - MG_IntegrationTest/CMakeLists.txt probes MG_Remote for @@ -1592,34 +1593,17 @@ jobs: python3 ../scripts/ci/junit_tally.py "${RUNNER_TEMP}/tcp-baseline.xml" --require-tcp-ran --require-run-ahead --discovery "${RUNNER_TEMP}/tcp-discovery.json" python3 ../scripts/ci/junit_tally.py "${RUNNER_TEMP}/tcp-baseline.xml" --require-entry-passed TcpLane.SupervisorProtocolControls --require-entry-passed TcpLane.EventForfeitPeer --require-entry-passed TcpLane.ControlFrameFuzz - # THE RUNTIME HALF OF "THIS IS REALLY A SPLIT BUILD". The build-level nm check in - # build-linux-split proves the library CARRIES MG_Remote; this proves the transport - # RESOLVED in a process of this lane. ConfigLoader::InitTransport logs one INFO line when it - # selects InProcess, and the DirectGLES.Split.PersistentMapArm. entry retains its private - # counting log (all Split entries now have private paths). It runs right after the lane, so - # it reads the lane's own run rather than a later step's rerun of the same entry. - # The line is written during bring-up, before any scenario decides to skip, so this - # check is live from the day the lanes land rather than from the day they stop skipping. - - name: Transport and default-knob resolution check + # Whether each entry ran the transport it claims is asserted in-test: the split fixture + # (Harness/ScenarioFixture.h) fails an armed entry whose process resolved another transport + # or whose server is not where the claim says, and junit_tally's --require-*-ran above + # require each arm to have passed entries. This step proves the IPC defaults the lanes + # rely on, from every passed entry's own client log; it runs right after the lane, so it + # reads the lane's own run rather than a later step's rerun of the same entry. + - name: Default-knob resolution check id: resolution + if: env.MOBILEGL_CI_DISAGGREGATED == 'ON' working-directory: ${{ env.SPLIT_BUILD_DIR }} run: | - # P6: the sink writes one file per ROLE from the base MOBILEGL_LOG_FILE_PATH; the - # transport-resolution line is ConfigLoader's, on the CLIENT thread, so it is in the - # .client half. The base name itself names no file now. - log=MobileGL/MG_IntegrationTest/persistent-map-arm-split-DirectGLES.client.log - if [ ! -f "${log}" ]; then - echo "::error::${log} does not exist: the DirectGLES.Split.PersistentMapArm. entry never ran (or the per-role split renamed it), so nothing in this job establishes that MOBILEGL_TRANSPORT ever resolved to inproc in a live process" - exit 1 - fi - # THE DISTINCTIVE PART OF THE INFO LINE, not the bare KEY=VALUE (review finding M-5): - # ConfigLoader logs `Config: Accepted env variable: MOBILEGL_TRANSPORT=inproc` for the - # env dump too, unconditionally and in a PULL build, and at DEBUG that line is live. - if ! grep -q "MOBILEGL_TRANSPORT=inproc - the MGPipe record stream" "${log}"; then - echo "::error::${log} carries no transport-resolution line. ConfigLoader::InitTransport logs it at INFO when it selects InProcess, and that code exists only in a MOBILEGL_BUILD_DISAGGREGATED build - so this lane ran a monolith library while claiming to be the split lane. (A bare MOBILEGL_TRANSPORT=inproc substring is NOT accepted: the env dump prints one in every build.)" - exit 1 - fi - echo "the split lane resolved MOBILEGL_TRANSPORT=inproc" # The IPC knobs every passed entry resolved, from its own client log: the defaults the # lane relies on (run-ahead and the role-split state on, strict off) unless the entry # pins its own. Proven defaults make Transport A/B below a repeat of the integration @@ -1628,7 +1612,7 @@ jobs: rc=0 python3 "${GITHUB_WORKSPACE}/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py" ipc-config \ "${RUNNER_TEMP}/split-lane.json" "${RUNNER_TEMP}/split-baseline.xml" "${RUNNER_TEMP}/split-ipc.json" \ - strict=0 role-split-state=1 run-ahead=1 --assumed role-split-state,run-ahead || rc=$? + strict=0 role-split-state=1 run-ahead=1 --assumed strict,role-split-state,run-ahead || rc=$? if [ "${rc}" -eq 0 ]; then echo "defaults-on=true" >> "${GITHUB_OUTPUT}" elif [ "${rc}" -eq 3 ]; then diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index e903c3cd..dcff47de 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -3697,4 +3697,11 @@ if (MOBILEGL_BUILD_DISAGGREGATED) add_test(NAME SplitLogPaths.ResultAccountingControls COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/Harness/test_split_log_paths.py) set_tests_properties(SplitLogPaths.ResultAccountingControls PROPERTIES LABELS "unit") + + # The split fixture's transport-claim rule on canned inputs, so the rule itself can go red + # off a GPU: no GL, no library, just the pure function the fixture calls. + add_executable(SplitLaneHarnessTest Harness/SplitLaneHarnessTest.cpp) + target_link_libraries(SplitLaneHarnessTest PRIVATE GTest::gtest_main) + add_test(NAME SplitLaneHarness.TransportClaimRule COMMAND SplitLaneHarnessTest) + set_tests_properties(SplitLaneHarness.TransportClaimRule PROPERTIES LABELS "unit") endif() diff --git a/MobileGL/MG_IntegrationTest/Harness/ScenarioFixture.h b/MobileGL/MG_IntegrationTest/Harness/ScenarioFixture.h index faa3dc0d..b47eecd3 100644 --- a/MobileGL/MG_IntegrationTest/Harness/ScenarioFixture.h +++ b/MobileGL/MG_IntegrationTest/Harness/ScenarioFixture.h @@ -158,11 +158,21 @@ namespace MGITest { // entries had no runtime evidence of anything, and their green meant only "the // same GL workload passed"). const SplitRuntimeState state = PeekSplitRuntime(); + // The transport the registration claims must be the one this process resolved, + // served where it says: a thread here for inproc, a process of its own for spawn + // and tcp. A lane that ran some other transport fails here, by name. + const std::string claimMismatch = SplitLane::TransportClaimMismatch( + SplitLane::MarkerValue("MOBILEGL_TRANSPORT"), SplitLane::MarkerValue("MOBILEGL_IPC_CONTROL"), + state.transportName, state.peerServerPid, state.selfPid); + if (!claimMismatch.empty()) { + FAIL() << claimMismatch; + } m_splitAssertionsArmed = true; m_emitSeqAtSetUp = state.emitSeq; RecordProperty("split_transport", state.transportName); RecordProperty("split_implemented_verbs", static_cast(state.implementedVerbs)); RecordProperty("split_emit_seq_at_setup", static_cast(state.emitSeq)); + RecordProperty("split_peer_server_pid", std::to_string(state.peerServerPid)); #if defined(__linux__) if (std::getenv("MGITEST_TCP_LANE")) { // Connect must never silently launch a local server. The diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitLane.h b/MobileGL/MG_IntegrationTest/Harness/SplitLane.h index ea80ceea..e21bc6ca 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitLane.h +++ b/MobileGL/MG_IntegrationTest/Harness/SplitLane.h @@ -84,6 +84,36 @@ namespace MGITest::SplitLane { return SplitRuntimeSkipReason(); } + // The transport a registration claims, against what this process resolved and which process + // serves it; empty when they agree. claimedTransport and claimedControl are the entry's own + // MOBILEGL_TRANSPORT and MOBILEGL_IPC_CONTROL; the rest comes from PeekSplitRuntime(). In + // process, the server role is a thread here and states this process's pid; spawn and tcp + // need a server process of their own. A registration that claims no transport is not judged, + // and an unknown own pid (0) leaves only the comparison with it unjudged. + inline std::string TransportClaimMismatch(const std::string& claimedTransport, const std::string& claimedControl, + const std::string& resolvedName, unsigned long long peerPid, + unsigned long long selfPid) { + const bool tcp = claimedControl.rfind("tcp://", 0) == 0; + const bool selfKnown = selfPid != 0; + const std::string pids = + " (server pid " + std::to_string(peerPid) + ", this process " + std::to_string(selfPid) + ")"; + if (claimedTransport == "inproc" && !tcp) { + if (resolvedName != "inproc" || peerPid == 0 || (selfKnown && peerPid != selfPid)) { + return "this registration claims MOBILEGL_TRANSPORT=inproc, but the process resolved '" + resolvedName + + "' with its server role in another process or none" + pids; + } + return {}; + } + if (claimedTransport == "spawn" || tcp) { + const std::string claim = tcp ? "tcp (MOBILEGL_IPC_CONTROL=" + claimedControl + ")" : "spawn"; + if (resolvedName != "spawn" || peerPid == 0 || (selfKnown && peerPid == selfPid)) { + return "this registration claims " + claim + ", but the process resolved '" + resolvedName + + "' without a server process of its own" + pids; + } + } + return {}; + } + // "adopted", "emulated", or empty when the lane declared nothing. inline std::string DeclaredPersistentMapArm() { return MarkerValue("MGITEST_PERSISTENT_MAP_ARM"); diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitLaneHarnessTest.cpp b/MobileGL/MG_IntegrationTest/Harness/SplitLaneHarnessTest.cpp new file mode 100644 index 00000000..62776956 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Harness/SplitLaneHarnessTest.cpp @@ -0,0 +1,64 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Harness/SplitLaneHarnessTest.cpp +// Copyright (c) 2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// SplitLane::TransportClaimMismatch on canned inputs: each claim must be refused when the process +// resolved another transport or the wrong process serves it, and accepted when both agree. + +#include "SplitLane.h" + +#include + +namespace { + + using MGITest::SplitLane::TransportClaimMismatch; + + constexpr unsigned long long kSelf = 4242; + constexpr unsigned long long kServer = 4343; + + TEST(SplitLaneHarness, AnInprocClaimNeedsTheServerThreadHere) { + EXPECT_EQ(TransportClaimMismatch("inproc", "", "inproc", kSelf, kSelf), ""); + EXPECT_NE(TransportClaimMismatch("inproc", "", "spawn", kServer, kSelf), ""); + EXPECT_NE(TransportClaimMismatch("inproc", "", "inproc", kServer, kSelf), ""); + EXPECT_NE(TransportClaimMismatch("inproc", "", "inproc", 0, kSelf), ""); + EXPECT_NE(TransportClaimMismatch("inproc", "", "monolith", 0, kSelf), ""); + } + + TEST(SplitLaneHarness, ASpawnClaimNeedsAServerProcessOfItsOwn) { + EXPECT_EQ(TransportClaimMismatch("spawn", "", "spawn", kServer, kSelf), ""); + EXPECT_NE(TransportClaimMismatch("spawn", "", "inproc", kSelf, kSelf), ""); + EXPECT_NE(TransportClaimMismatch("spawn", "", "spawn", kSelf, kSelf), ""); + EXPECT_NE(TransportClaimMismatch("spawn", "", "spawn", 0, kSelf), ""); + } + + TEST(SplitLaneHarness, ATcpClaimIsASpawnSessionToAnotherProcess) { + EXPECT_EQ(TransportClaimMismatch("spawn", "tcp://127.0.0.1:30613", "spawn", kServer, kSelf), ""); + EXPECT_NE(TransportClaimMismatch("spawn", "tcp://127.0.0.1:30613", "spawn", kSelf, kSelf), ""); + EXPECT_NE(TransportClaimMismatch("inproc", "tcp://127.0.0.1:30613", "inproc", kSelf, kSelf), ""); + } + + TEST(SplitLaneHarness, NoClaimIsNotJudged) { + EXPECT_EQ(TransportClaimMismatch("", "", "monolith", 0, kSelf), ""); + EXPECT_EQ(TransportClaimMismatch("monolith", "", "monolith", 0, kSelf), ""); + } + + TEST(SplitLaneHarness, AnUnknownOwnPidStillJudgesTheTransportAndTheServer) { + EXPECT_EQ(TransportClaimMismatch("inproc", "", "inproc", kSelf, 0), ""); + EXPECT_NE(TransportClaimMismatch("inproc", "", "spawn", kServer, 0), ""); + EXPECT_NE(TransportClaimMismatch("inproc", "", "inproc", 0, 0), ""); + EXPECT_EQ(TransportClaimMismatch("spawn", "", "spawn", kServer, 0), ""); + EXPECT_NE(TransportClaimMismatch("spawn", "", "inproc", kSelf, 0), ""); + EXPECT_NE(TransportClaimMismatch("spawn", "", "spawn", 0, 0), ""); + } + + TEST(SplitLaneHarness, TheRefusalNamesBothPids) { + const std::string message = TransportClaimMismatch("spawn", "", "spawn", kSelf, kSelf); + EXPECT_NE(message.find("server pid 4242"), std::string::npos) << message; + EXPECT_NE(message.find("this process 4242"), std::string::npos) << message; + } + +} // namespace diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp index 13fa77c7..addaf3df 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp +++ b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp @@ -25,6 +25,11 @@ #include #include #include +#if defined(_WIN32) +#include +#else +#include +#endif #define MGITEST_SPLIT_RUNTIME_PEEK_LIVE 1 #endif // P13 W5: THE RECORD-ARM HALF OF THE PEEK, which needs no transport. A library built with the @@ -141,6 +146,9 @@ namespace MGITest { case TransportMode::InProcess: state.transportName = "inproc"; break; + case TransportMode::Spawn: + state.transportName = "spawn"; + break; default: state.transportName = "non-monolith"; break; @@ -154,11 +162,17 @@ namespace MGITest { MobileGL::MG_Remote::Client::ClientSession* session = MobileGL::MG_Remote::Client::ClientSession::Active(); state.sessionActive = session != nullptr; state.implementedVerbs = MobileGL::MG_Remote::Client::ImplementedVerbCount(); +#if defined(_WIN32) + state.selfPid = static_cast(::_getpid()); +#else + state.selfPid = static_cast(::getpid()); +#endif if (session != nullptr) { // Encoder() returns the member; EmitSeq() returns m_emitSeq. Neither is a stub, and // neither emits anything - this is a read. const MobileGL::MG_Remote::Wire::PipeWireEncoder& encoder = session->Encoder(); state.emitSeq = encoder.EmitSeq(); + state.peerServerPid = session->PeerServerPid(); state.runAheadArmed = session->RunAheadArmed(); // P7 wave 2 package C, OQ-10: read through the SAME accessor the frontend's own // MGPipeResourceOpsHaveSubDataResident reads (MG_Impl/Pipe/PipeFill.cpp), so a diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h index 8424abf7..1977ca27 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h +++ b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h @@ -74,6 +74,11 @@ namespace MGITest { unsigned int totalVerbSlots = 0; // The encoder's highest produced record ordinal, or 0 when there is no session. unsigned long long emitSeq = 0; + // ClientSession::PeerServerPid(), the pid the server stated in its Welcome (0 without a + // session), and this process's own (0 when unknown), so a lane can tell a server thread + // here from a server process elsewhere. + unsigned long long peerServerPid = 0; + unsigned long long selfPid = 0; // Real session state/watermarks. Only the client thread reads this snapshot; // the scheduling observer below reads producerParked separately. bool runAheadArmed = false; diff --git a/docs/ci/ci-cleanup-log.md b/docs/ci/ci-cleanup-log.md index 10f7a72b..5c751d50 100644 --- a/docs/ci/ci-cleanup-log.md +++ b/docs/ci/ci-cleanup-log.md @@ -47,12 +47,13 @@ them wrong. | 2026-10-17 | the pre-flip arm of `Strict split lane` (red-by-design while `kMGPipeP5eRunAheadReady` was false) and the "GREEN with rows outstanding" rule of `Dual-block census lane` | Test / `integration-split-strict` | dead: the constant is true, and `dualblock-expected-fatals.txt` has no rows, so neither branch could run | the hard-green arm and its two-sided marker census, and the empty-census rule (rows == 0 requires ctest success), both kept | | 2026-10-17 | `scripts/ci/tcp_log_forwarding_control.py` | none | no caller outside the archived planning notes | none needed | | 2026-10-17 | the compile of `build-linux-split` on the disaggregation branches (when its guard proves the configurations equal) | Test / `build-linux-split` | it built the library `build-linux` builds there, from a configuration that differs only by the benchmarks (211 paired runs never disagreed); it delayed every split consumer by up to 9 minutes | `build_fingerprint.py` guards in both build jobs (cache, compile commands, link statements, CTest files, toolchain; benchmarks excluded); the consumers take `build-linux`'s artifacts and red if `build-linux`'s own guard disagrees; the link ratchet runs in `build-linux`; on any difference `build-linux-split` builds as before, and dev is unchanged | -| 2026-10-17 | `Transport A/B (monolith vs inproc)` (skip-census lanes `arm-monolith`, `arm-inproc` now optional) | Test / `integration-split` | with `build-linux` reused it repeats the integration job's two legs on the same library (79 paired runs, no one-sided red) | the integration legs; `Transport and default-knob resolution check` now reads 'Split lanes' own run and proves, from every passed entry's client log, strict=0, role-split-state=1 and run-ahead=1 unless pinned (`split_log_paths.py ipc-config`); A/B still runs when a default differs or the build is not reused | +| 2026-10-17 | `Transport A/B (monolith vs inproc)` (skip-census lanes `arm-monolith`, `arm-inproc` now optional) | Test / `integration-split` | with `build-linux` reused it repeats the integration job's two legs on the same library (79 paired runs, no one-sided red) | the integration legs; `Default-knob resolution check` (disaggregation branches) reads 'Split lanes' own run and proves, from every passed entry's client log, strict=0, role-split-state=1 and run-ahead=1 unless pinned (`split_log_paths.py ipc-config`); A/B still runs when a default differs or the build is not reused | | 2026-10-17 | `Unit tests on the split runtime` (skip-census lane `unit-split` now optional) | Test / `integration-split` | with `build-linux` reused the test job runs the same unit lane on the same build (182 paired runs; the one recurring one-sided red was a fixed test race) | the test job, which now carries `MOBILEGL_ITEST_REQUIRE_GPU=1` and `libegl-mesa0` on the three branches; the strict unit step is unchanged | | 2026-10-17 | the integration-gpu entries that pin their own `MOBILEGL_TRANSPORT` (804 inproc, 1446 spawn, 131 monolith) and the 12 `TcpServer` fixture entries only they need, in the inproc-acceptance leg (both passes) | Test / `integration` (inproc-acceptance) | their pin overrides the only variable the two legs differ in, so they ran identically in both (173 paired runs; the one one-sided red was a port bind fixed by cde6512b) | the monolith-control leg, which runs them on every branch; `transport_pinned.py` proves per run that the trimmed selection lost exactly that population (else the leg runs everything) and that the monolith-control leg ran every one; skip census: the monolith-control lanes list their own names and the inproc-acceptance lanes drop the pinned ones | | 2026-10-17 | `Strict split lane` on the disaggregation branches (dev keeps it) | Test / `integration-split-strict` | with the role-split state defaulting on, its environment equals the dual-block census lane's, whose entries are a superset (31 paired runs after the flip, never red only in the strict lane) | the dual-block step's first run leaves the knob at its default, proves per entry from its client log that strict, the dual block and run-ahead resolved on (`ipc-config`), writes the integration-split part of its JUnit as `strict-split.xml` (every name exactly once, `junit_tally.py --select-from`), and makes the strict lane's checks on it (hard green, two-sided marker census, StrictArming ran); a default that changed reruns the census with the knob set | | 2026-10-17 | `Magma split lane (strict, dual-block)` on the disaggregation branches when the dual-block step proved its defaults | Test / `integration-split-strict` | the dual-block census run covers the `integration-magma-split` label in the same environment | the dual-block census run | | 2026-10-17 | the 75 retrace-split inproc rows other than OpenRA's, when `build-linux-split` reuses `build-linux` | Test / `retrace-split` | the retrace job's inproc-acceptance rows replay the same cases on the same library, strict and with the dual block and run-ahead on; no one-sided pair since 10-05 | the inproc-acceptance rows, which now prove per leg from the client log that they run with strict=1, role-split-state=1 and run-ahead=1 (`MGTRACE_EXPECT_IPC`) and keep a refusal census; every remaining retrace-split leg proves its defaults (strict=0, role-split-state=1, run-ahead=1) and a changed default reds every leg with the rows to restore; `trace_cases.py --self-test-split-matrix` (disaggregation branches) proves every dropped row is an acceptance row; OpenRA's inproc rows stay for the wire evidence | +| 2026-10-17 | `Transport resolution check`'s grep of one entry's log for the inproc resolution sentence | Test / `integration-split` | it checked one entry (`DirectGLES.Split.PersistentMapArm.`) after the fact | in-test: the split fixture (`Harness/ScenarioFixture.h`) now fails every armed Split/Spawn/Tcp entry whose process did not resolve the transport its registration claims, or whose server is not where the claim says (a thread here for inproc, a process of its own for spawn and tcp), from the running process (`SplitRuntimePeek`, `ClientSession::PeerServerPid()`); `SplitLaneHarness.TransportClaimRule` (unit) proves the rule red on canned inputs; the step keeps the IPC default proof as `Default-knob resolution check` | ## Kept after investigation diff --git a/scripts/data/skip_census_baseline.json b/scripts/data/skip_census_baseline.json index 81558ae6..509aa7fd 100644 --- a/scripts/data/skip_census_baseline.json +++ b/scripts/data/skip_census_baseline.json @@ -11274,6 +11274,7 @@ "SplitBufferSet.TheLastBlockIsTheRemainderAndNotAWholeBlock", "SplitBufferSet.ThePushCutsTheMappedSpanIntoBlocksAndMovesPmap", "SplitBufferSet.UnderSplitTheWritebackClearsThePendingFlagAndNotTheRequest", + "SplitLaneHarness.TransportClaimRule", "SplitLogPaths.ResultAccountingControls", "SplitReadWriteImageUniformsTest.ADeclarationQualifiedBothWaysIsLeftCompletelyAlone", "SplitReadWriteImageUniformsTest.ANameThatIsAPrefixOfAnotherIsNotClobbered",