From ea9fc7fa71e785d2f5fa05c20c0260be6a1bfd4e Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 21 Sep 2026 12:23:41 -0400 Subject: [PATCH] [MG_Remote] (P6/cp): a real trace retraces across two processes, pixel-for-pixel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenRA.DirectGLES.SPAWN is green: ssim=1.000000, mismatchPixels=0, zero Fatal{ lines, against a server in a process the client started by name. The inproc SPLIT arm stays beside it as the control, so "did the transport break it" and "did the phase break it" remain separately answerable. Five defects stood between the handshake and that picture. Each one was silent in its own way, and none of them looked like what it was. 1. THE DOORBELL COULD NOT WAKE ITSELF. SocketDoorbell's one-fd form sends on the end it polls, so a write is delivered to the PEER - correct for waking the other process, useless for waking a thread in this one. The server's control pump posts into the apply thread's mailbox and must ring the bell that thread parks on. Symptom: every eglInitialize crossed, landed in the mailbox, and timed out at 5 s. The two-fd form (park on [0], notify on [1]) is restored here, and the bell handover goes from two descriptors to THREE - the client needs the same split for the same reason. Seven offers now. 2. InitCapabilities HAD NO WIRE KIND. CONTRACT-P6 §2 predicted this exactly ("the request has a live client caller and the codec refuses to encode it"). SurfaceOpKind gains 11, append-only; the decode range's upper bound moves with it, or the new tag reads as out-of-range and surfaces as Fatal{ProtocolCorruption} rather than the missing row it is. The op's name loses its InprocOnly suffix, which stopped being true here. 3. THE CHILD LOADED NO CONFIG AT ALL. mobilegl_server_main calls InitServerRoleForSpawn directly, and MG_ConfigLoader::Init is a step of MobileGL::Initialize - so every MG_Config value in the server was its static default. MOBILEGL_BACKEND_TYPE survives the launcher's scrub on purpose and was read by nobody: a DirectVulkan session would have brought up a DirectGLES server and then disagreed with its client about which backend the capability mask describes. 4. AND THEREFORE Transport READ AS Monolith, which CONTRACT-P6 §3.1 (D1) forbids in as many words. a6 measured the cost: 226 live lines test `Transport != Monolith` to select the SERVER arm, 148 in MG_Backend. Monolith flips all of them to frontend glue in the one process with no frontend. Observed precisely as predicted - UnitTexturesByHandle() answered false, Clear took the legacy arm that dereferences borrowed FRONTEND slot pointers, and the apply thread took a SIGSEGV inside SyncNeccessaryTextures on the first Clear record of the trace. It is pinned in-process rather than through the environment because the environment is where it must NOT be: LooksLikeAClientEnvironment exits 65 on a surviving MOBILEGL_TRANSPORT, and that check is anti-recursion catch (b). Dial, not Transport, is that axis. 5. THE LOG WAS CORRUPT, NOT MERELY INTERLEAVED. Two processes opened the same MOBILEGL_LOG_FILE_PATH with "w" and each carried its OWN file offset, so they overwrote each other's bytes - half-lines like `ote client: CapsMirror generation 1 adopted`, and the server's whole startup block vanishing under the client's later writes. This cost real diagnosis time twice, because a missing line read as a code path not taken. O_APPEND now; the client still truncates once so a fresh run is not a pile of old ones, and the server never does - it would delete the very startup lines a failed session needs. run_trace_case.cmake's Fatal{ census reads exactly this file, so it was also reading a lie. The retrace runner's split assertions become transport-aware: one row per transport it admits, and a transport with no row is a FATAL rather than a fall-through, because the fall-through is the accident the block exists to prevent. The spawn row carries a SECOND sentence and it is the stronger half - ConfigLoader's marker proves the value RESOLVED, while `spawn ARMED - the server role runs in pid N` is written only after the launch, the connect and the handshake all succeeded, and carries a pid that is not this process's. inproc resolves its own marker while running the server role on a thread here; nothing on that path can write this one. G14: one test name added (TheEleventhFramedOpIsInitCapabilitiesAndItCrosses- TheWire), none removed. The ten-op table keeps its name and its scope rather than being renamed to eleven. Two unit failures predate this change and are environmental in this tree: PipeCatalogue.FrontendNeverTakesAnApplierAddress (the frontend source root resolves empty) and SessionHandshakeTest.TheAbiFingerprintChangesWhenAnyOf- ItsInputsDoes (GIT_COMMIT_HASH_SHORT is empty here, which a6 already recorded as red in this configuration). Co-Authored-By: Claude Opus 5 --- MobileGL/ConfigLoader.cpp | 30 ++++++- MobileGL/MG_Backend/Init.cpp | 34 +++++++ MobileGL/MG_Remote/Client/ClientSession.cpp | 90 +++++++++++++++++-- MobileGL/MG_Remote/Client/ClientSession.h | 10 +++ .../MG_Remote/Protocol/SurfaceOpCodec.cpp | 12 ++- .../Protocol/generated/protocol_generated.h | 13 +-- MobileGL/MG_Remote/Protocol/protocol.fbs | 6 ++ MobileGL/MG_Remote/Server/ServerLoop.cpp | 4 +- MobileGL/MG_Remote/Server/ServerMain.cpp | 73 +++++++++++++-- .../MG_Remote/Server/SurfaceControlFrame.cpp | 6 +- .../MG_Remote/Server/SurfaceControlFrame.h | 14 ++- MobileGL/MG_Remote/Transport/Doorbell.cpp | 38 +++++++- MobileGL/MG_Remote/Transport/Doorbell.h | 20 ++++- .../MG_Test/Wire/SurfaceControlFrameTest.cpp | 38 +++++++- MobileGL/MG_Util/Debug/Log.cpp | 29 +++++- tools/trace_replay/CMakeLists.txt | 16 ++++ tools/trace_replay/run_trace_case.cmake | 57 ++++++++++-- 17 files changed, 443 insertions(+), 47 deletions(-) diff --git a/MobileGL/ConfigLoader.cpp b/MobileGL/ConfigLoader.cpp index a9686a22..5ea497ba 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -336,10 +336,32 @@ namespace MobileGL::MG_ConfigLoader { } // The three P6 forms. Recognised precisely, so the diagnostic can say "not yet" // rather than "unknown", which are different bugs on the operator's side. - if (lowered == "spawn" || lowered.compare(0, 5, "unix:") == 0 || - lowered.compare(0, 5, "pipe:") == 0) { - MGLOG_E("Config: MOBILEGL_TRANSPORT='%s' names a transport P6 implements and P5 does " - "not; staying on monolith. This run is NOT a split run.", + // P6 `sm`/`cp`: spawn is IMPLEMENTED. It launches a server process on a + // rendezvous of its own and connects to it - the two processes are + // independent, so this is `Dial = Connect` with a launcher attached + // rather than the fork-coupled shape ARCHITECTURE.md:488 described. + if (lowered == "spawn") { + MG_Config::Transport = MG_Config::TransportMode::Spawn; + // THE SAME SHAPE AS THE inproc LINE ABOVE, AND FOR THE SAME REASON. + // run_trace_case.cmake asserts on a distinctive sentence from this + // function rather than on `MOBILEGL_TRANSPORT=spawn` alone, because + // ConfigLoader logs an `Accepted env variable: KEY=VALUE` line for + // every MOBILEGL_* in EVERY build including the pull one - so the + // bare KEY=VALUE is satisfied by a monolith library that parsed + // nothing (review M-5). A spawn arm with no line of its own could + // only have been asserted with the inproc marker, which spawn never + // emits: the arm would have been red for a missing sentence rather + // than green for a working transport. + MGLOG_I("Config: MOBILEGL_TRANSPORT=spawn - the MGPipe record stream crosses a real " + "ring to an apply thread in ANOTHER PROCESS"); + return; + } + if (lowered.compare(0, 5, "unix:") == 0 || lowered.compare(0, 5, "pipe:") == 0) { + // unix: and pipe: remain P6+ - connecting to an endpoint SOMEBODY + // ELSE is listening on needs the server lifecycle to be somebody + // else's too, which is P12's. Named, never a silent degrade. + MGLOG_E("Config: MOBILEGL_TRANSPORT='%s' names a transport P6 does not implement yet; " + "staying on monolith. This run is NOT a split run.", value.c_str()); MG_Config::Transport = MG_Config::TransportMode::Monolith; return; diff --git a/MobileGL/MG_Backend/Init.cpp b/MobileGL/MG_Backend/Init.cpp index 71937ee1..70413581 100644 --- a/MobileGL/MG_Backend/Init.cpp +++ b/MobileGL/MG_Backend/Init.cpp @@ -226,6 +226,26 @@ namespace MobileGL::MG_Backend { // the client half here. Bool InitSplitRoles() { using namespace MobileGL::MG_Remote; + + // SPAWN: THE SERVER ROLE IS NOT IN THIS PROCESS. Steps 1 and 2 - + // the backend and the two CallMask halves - happen in the server, + // which ran InitServerRoleForSpawn before it accepted us. Running + // them here as well would build a second BackendObject in the one + // process that must not have one, and would publish a capability + // mask computed from the WRONG backend's function table. + if (MG_Config::Transport == MG_Config::TransportMode::Spawn) { + const MobileGLResult started = Client::ClientSessionInstance().StartSpawned(); + if (started != MOBILEGL_OK) { + MGLOG_E("MG_Remote: the spawn session failed to start (rc=%d); MobileGL will " + "NOT fall back to monolith - a lane named split that ran monolith is " + "the one failure this phase is built to make impossible", + static_cast(started)); + return false; + } + pActiveBackendObject = MakeUnique(); + return true; + } + if (!InitServerRoleCommon()) { return false; } @@ -308,6 +328,19 @@ namespace MobileGL::MG_Backend { MGLOG_W("Failed to initialize MobileGL backend libraries for the remote object"); return; } + // P6: BOTH CHECKS BELOW ARE STATEMENTS ABOUT THE SERVER'S PROCESS, and under + // spawn that is not this one. Their own wording says so - "a mask is a + // statement about THIS BACKEND", "the table the SERVER'S BACKEND registered at + // step 1" - and in a spawn client there is no backend and never was a table, so + // MGPipeGetResourceOps() is null by construction and the first check fires on a + // correct session. It did, on the first spawn retrace ever run. + // + // The checks are NOT weakened: the server runs the step-2 one itself, inside + // InitServerRoleForSpawn, against its own freshly registered table - which is the + // process where the question has an answer. What is lost under spawn is the + // step-5 re-run's ability to catch a table REPLACED between accept and here, and + // that is a question about one address space; it cannot be asked across two. + if (MG_Config::Transport != MG_Config::TransportMode::Spawn) { // m-6, re-worded per review v2 N-8. The honesty cross-check runs a SECOND time, now // that step 4's pActiveBackendObject (the client's BackendObject_Remote) exists and // its Initialize() has run inside InitSpecificBackendLibs. What the re-run CAN catch @@ -329,6 +362,7 @@ namespace MobileGL::MG_Backend { static_cast(g_resourceOpsAtStep2)); std::abort(); } + } LogBackendInfo(); return; } diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index 0b8d952d..54a4262c 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -57,6 +57,9 @@ namespace MobileGL::MG_Remote::Client { // is gone is a different fact from a peer that is slow, and only the // doorbell death latch may say which. constexpr Uint32 kRemoteControlTimeoutMs = 5000; + // The server has to create a backend before it binds, and a cold + // software rasteriser is not fast. Bounded, and a NAMED refusal at the end. + constexpr Uint32 kSpawnConnectTimeoutMs = 20000; // Teardown's drain. Also bounded, and for the same reason - table 3's order is // "publish and wait for the server to drain and acknowledge", and a wait with no // deadline there turns a lost record into a hung process exit. @@ -614,6 +617,55 @@ namespace MobileGL::MG_Remote::Client { /*startApplyThreadHere=*/true); } + MobileGLResult ClientSession::StartSpawned() { + if (m_started) { + return MOBILEGL_ERR_INVALID_ARGUMENT; + } + // A rendezvous nobody else can collide with. Independent processes need + // a NAME; making it per-process-per-instant is what stops two runs on + // one machine - which `ctest -j` produces by construction - finding each + // other's server. + const std::string endpoint = std::string("/tmp/mgl-") + + std::to_string(static_cast(::getpid())) + "-" + + std::to_string(reinterpret_cast(this)) + + ".sock"; + + const MobileGLResult launched = + Server::LaunchServer(std::string(MG_Config::Ipc.ServerPath.c_str()), endpoint, + &m_spawned); + if (launched != MOBILEGL_OK) { + return launched; + } + + std::unique_ptr socket; + // The bounded connect retry is what tolerates the server still binding - + // and it is the CLIENT's, not the launcher's. An exhausted budget is a + // NAMED refusal; there is no monolith fallback from here. + const MobileGLResult connected = + Transport::SocketTransport::ConnectTo(endpoint, kSpawnConnectTimeoutMs, socket); + if (connected != MOBILEGL_OK) { + int exitCode = 0; + Server::ReapServer(m_spawned, 2000, &exitCode); + MGLOG_E("MG_Remote client: could not reach the server we launched at \"%s\" " + "(rc=%d, server exit=%d)", + endpoint.c_str(), static_cast(connected), exitCode); + return connected; + } + // THE ONE FACT A MONOLITH OR inproc RUN CANNOT PRODUCE: a pid that is + // not ours. The ConfigLoader marker proves the VALUE resolved; this + // proves the SERVER ROLE LEFT THIS PROCESS, which is the whole claim the + // SPAWN arm of the retrace matrix makes. run_trace_case.cmake requires + // BOTH, so a spawn arm cannot go green on a transport that resolved and + // then quietly ran the server role locally. + const MobileGLResult started = StartOverSocket(std::move(socket)); + if (started == MOBILEGL_OK) { + MGLOG_I("MG_Remote client: spawn ARMED - the server role runs in pid %d, reached at " + "\"%s\"", + m_spawned.pid, endpoint.c_str()); + } + return started; + } + MobileGLResult ClientSession::StartOverSocket( std::unique_ptr transport) { if (m_started) { @@ -702,8 +754,9 @@ namespace MobileGL::MG_Remote::Client { // The slot travels in the sideband rather than being inferred from arrival // order, so a reordered or dropped offer is a NAMED mismatch instead of two // segments quietly swapped. - // Six offers: four segments, then the two bells (slots 4 and 5). - int fds[6] = {-1, -1, -1, -1, -1, -1}; + // Seven offers: four segments, then THREE bell descriptors (slots 4..6). + // Three because each side parks on one and rings another; see ServerMain. + int fds[7] = {-1, -1, -1, -1, -1, -1, -1}; { struct Sideband { Uint32 slot; @@ -717,7 +770,7 @@ namespace MobileGL::MG_Remote::Client { } } }; - for (int received = 0; received < 6; ++received) { + for (int received = 0; received < 7; ++received) { Sideband sideband{}; Uint64 sidebandSize = 0; int fd = -1; @@ -742,7 +795,7 @@ namespace MobileGL::MG_Remote::Client { } std::memcpy(&sideband, scratch.data(), sizeof(Sideband)); const bool isSegment = sideband.slot < 4; - if (sideband.slot >= 6 || fds[sideband.slot] >= 0 || + if (sideband.slot >= 7 || fds[sideband.slot] >= 0 || (isSegment && sideband.bytes != announced[sideband.slot])) { MGLOG_E("MG_Remote client: segment offer names slot %u with %llu bytes; " "Welcome announced a different size or the slot is already filled", @@ -770,8 +823,14 @@ namespace MobileGL::MG_Remote::Client { // WHICH BELL IS WHICH IS THE SESSION'S KNOWLEDGE, exactly as under // inproc: slot 4 is the pair end whose WRITE wakes the server, slot 5 // is the one this side POLLS. - m_socketPeerBell = MakeUnique(fds[4], 1, true); - m_socketSelfBell = MakeUnique(fds[5], 1, true); + // PARK ON ONE END, RING THE OTHER - the same split the server makes, + // and for the same reason: a single-fd bell rings the PEER, so it + // cannot wake its own waiter. + // fds[4] ring the SERVER (ring-only, we never park on it) + // fds[5] our park end + // fds[6] ring OURSELVES + m_socketPeerBell = MakeUnique(-1, fds[4], 1, true); + m_socketSelfBell = MakeUnique(fds[5], fds[6], 1, true); } // `cp`: from here the twelve EGL forwarders cross the socket instead of @@ -1116,6 +1175,25 @@ namespace MobileGL::MG_Remote::Client { // process that no longer has a session. During the whole span above, the raised flag made // any routed call abort by name instead. ReinstallMonolithAfterTeardown(); + // P6: the server we launched. Closing the transport is what it sees as + // EOF and EOF is its whole exit condition, so the order is transport + // first, reap second - and the reap is BOUNDED, because a server that + // will not go is a different fact from one that is slow and only the + // exit code can say which. + if (m_spawned.pid >= 0) { + if (m_socketTransport) { + m_socketTransport->Shutdown(); + } + int exitCode = 0; + if (Server::ReapServer(m_spawned, 3000, &exitCode) != MOBILEGL_OK) { + MGLOG_W("MG_Remote client: the spawned server (pid %d) did not exit within 3 s", + m_spawned.pid); + } else if (exitCode != 0) { + MGLOG_W("MG_Remote client: the spawned server exited %d", exitCode); + } + } + Server::ServerLoopInstance().SetRemoteControlSink(nullptr, nullptr); + } Wire::PipeWireEncoder& ClientSession::Encoder() { return m_encoder; } diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index f823fb96..194df5df 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -38,6 +38,7 @@ #include #include +#include "../Server/ServerSpawn.h" #include "../Server/SurfaceControlFrame.h" #include "../Transport/Doorbell.h" #include "../Transport/EventRing.h" @@ -108,6 +109,12 @@ namespace MobileGL::MG_Remote::Client { // RUNNING server process. The two bells arrive over it, with the four // segments, as SCM_RIGHTS offers - nothing here is inherited, because // the two processes were started independently. + // P6: launch a server process and connect to it. The two are + // INDEPENDENT - the launcher hands over a rendezvous name and nothing + // else - so this is the same code path a client would use against a + // server that was already running when it started. + MobileGLResult StartSpawned(); + MobileGLResult StartOverSocket(std::unique_ptr transport); MobileGLResult StartOverTransportPair(std::unique_ptr clientEnd, @@ -391,6 +398,9 @@ namespace MobileGL::MG_Remote::Client { std::mutex m_remoteControlMutex; Uint64 m_remoteControlSeq = 0; + // The server we launched, if we launched one. Reaped in Stop(). + Server::LaunchedServer m_spawned; + std::unique_ptr m_socketTransport; std::unique_ptr m_socketSelfBell; std::unique_ptr m_socketPeerBell; diff --git a/MobileGL/MG_Remote/Protocol/SurfaceOpCodec.cpp b/MobileGL/MG_Remote/Protocol/SurfaceOpCodec.cpp index 1d0bbd0a..a3aa2847 100644 --- a/MobileGL/MG_Remote/Protocol/SurfaceOpCodec.cpp +++ b/MobileGL/MG_Remote/Protocol/SurfaceOpCodec.cpp @@ -47,9 +47,13 @@ namespace MobileGL::MG_Remote { static_cast(::MobileGL::Wire::SurfaceOpKind::ReleaseResources)); static_assert(static_cast(SurfaceControlOp::SetWindowHandle) == static_cast(::MobileGL::Wire::SurfaceOpKind::SetWindowHandle)); + // cp's one schema addition, on the same append-only terms. + static_assert(static_cast(SurfaceControlOp::InitCapabilities) == + static_cast(::MobileGL::Wire::SurfaceOpKind::InitCapabilities)); static_assert(static_cast(::MobileGL::Wire::SurfaceOpKind::SetSwapInterval) == 8 && static_cast(::MobileGL::Wire::SurfaceOpKind::ReleaseResources) == 9 && - static_cast(::MobileGL::Wire::SurfaceOpKind::SetWindowHandle) == 10); + static_cast(::MobileGL::Wire::SurfaceOpKind::SetWindowHandle) == 10 && + static_cast(::MobileGL::Wire::SurfaceOpKind::InitCapabilities) == 11); static_assert(static_cast(::MobileGL::Wire::WindowKind::MetalLayer) == 6); bool OpNeedsAWindowBackend(SurfaceControlOp op) { @@ -80,8 +84,12 @@ namespace MobileGL::MG_Remote { } bool SurfaceControlOpForWireKind(::MobileGL::Wire::SurfaceOpKind kind, SurfaceControlOp* out) { + // THE UPPER BOUND IS THE LAST APPENDED KIND, and it moves with every + // append or the new tag decodes as out-of-range - which reads as + // Fatal{ProtocolCorruption} at ServerApplyWireSurfaceOp rather than as + // the missing row it is. if (::flatbuffers::IsOutRange(kind, ::MobileGL::Wire::SurfaceOpKind::InitializeDisplay, - ::MobileGL::Wire::SurfaceOpKind::SetWindowHandle)) { + ::MobileGL::Wire::SurfaceOpKind::InitCapabilities)) { return false; } const SurfaceControlOp op = static_cast(static_cast(kind)); diff --git a/MobileGL/MG_Remote/Protocol/generated/protocol_generated.h b/MobileGL/MG_Remote/Protocol/generated/protocol_generated.h index 87b6e287..a209a7d1 100644 --- a/MobileGL/MG_Remote/Protocol/generated/protocol_generated.h +++ b/MobileGL/MG_Remote/Protocol/generated/protocol_generated.h @@ -125,11 +125,12 @@ enum class SurfaceOpKind : uint8_t { SetSwapInterval = 8, ReleaseResources = 9, SetWindowHandle = 10, + InitCapabilities = 11, MIN = None, - MAX = SetWindowHandle + MAX = InitCapabilities }; -inline const SurfaceOpKind (&EnumValuesSurfaceOpKind())[11] { +inline const SurfaceOpKind (&EnumValuesSurfaceOpKind())[12] { static const SurfaceOpKind values[] = { SurfaceOpKind::None, SurfaceOpKind::InitializeDisplay, @@ -141,13 +142,14 @@ inline const SurfaceOpKind (&EnumValuesSurfaceOpKind())[11] { SurfaceOpKind::ReleaseCurrent, SurfaceOpKind::SetSwapInterval, SurfaceOpKind::ReleaseResources, - SurfaceOpKind::SetWindowHandle + SurfaceOpKind::SetWindowHandle, + SurfaceOpKind::InitCapabilities }; return values; } inline const char * const *EnumNamesSurfaceOpKind() { - static const char * const names[12] = { + static const char * const names[13] = { "None", "InitializeDisplay", "CreateWindowSurface", @@ -159,13 +161,14 @@ inline const char * const *EnumNamesSurfaceOpKind() { "SetSwapInterval", "ReleaseResources", "SetWindowHandle", + "InitCapabilities", nullptr }; return names; } inline const char *EnumNameSurfaceOpKind(SurfaceOpKind e) { - if (::flatbuffers::IsOutRange(e, SurfaceOpKind::None, SurfaceOpKind::SetWindowHandle)) return ""; + if (::flatbuffers::IsOutRange(e, SurfaceOpKind::None, SurfaceOpKind::InitCapabilities)) return ""; const size_t index = static_cast(e); return EnumNamesSurfaceOpKind()[index]; } diff --git a/MobileGL/MG_Remote/Protocol/protocol.fbs b/MobileGL/MG_Remote/Protocol/protocol.fbs index 9fb8d2bc..fe5429dc 100644 --- a/MobileGL/MG_Remote/Protocol/protocol.fbs +++ b/MobileGL/MG_Remote/Protocol/protocol.fbs @@ -176,6 +176,12 @@ enum SurfaceOpKind : ubyte { SetSwapInterval = 8, ReleaseResources = 9, SetWindowHandle = 10, + // P6 (package cp) appended this one. The draft held that InitCapabilities needs + // no wire form because "the CapsSnapshot answers it" - but a CapsSnapshot is the + // ANSWER and this is the REQUEST, and a spawn client has a live caller for it + // (Client/BackendObject_Remote.cpp). Without the tag the codec refused it by + // name and every spawned eglMakeCurrent failed with InprocOnlyOpOnTheWire. + InitCapabilities = 11, } enum WindowKind : ubyte { diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index 615e788b..a021c9ef 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -1030,7 +1030,7 @@ namespace MobileGL::MG_Remote::Server { backend->SetWindowHandle(UnpackWindowHandle(frame)); frame.ok = true; return MOBILEGL_OK; - case SurfaceControlOp::InitCapabilitiesInprocOnly: + case SurfaceControlOp::InitCapabilities: // Inproc-only (f0-egl §4.2): on the wire the ANSWER to this op is the CapsSnapshot // frame itself, so there is no SurfaceOp kind for it. The frame channel still has to // carry it inproc because InitCapabilities runs GL queries and therefore belongs to @@ -1172,7 +1172,7 @@ namespace MobileGL::MG_Remote::Server { Bool ServerInitCapabilities() { SurfaceControlFrame frame; - frame.kind = SurfaceControlOp::InitCapabilitiesInprocOnly; + frame.kind = SurfaceControlOp::InitCapabilities; const MobileGLResult rc = ServerLoopInstance().RunSurfaceControlFrame(frame); return rc == MOBILEGL_OK && frame.ok; } diff --git a/MobileGL/MG_Remote/Server/ServerMain.cpp b/MobileGL/MG_Remote/Server/ServerMain.cpp index c79350f5..efa3c31b 100644 --- a/MobileGL/MG_Remote/Server/ServerMain.cpp +++ b/MobileGL/MG_Remote/Server/ServerMain.cpp @@ -43,7 +43,9 @@ #include "ServerSession.h" #include "SurfaceControlFrame.h" +#include // MG_Config::Transport - D1 pins it to Spawn in this process #include +#include // MG_ConfigLoader::Init - the child loads its own config (step 1.5) #include #include @@ -147,6 +149,40 @@ extern "C" __attribute__((visibility("default"))) int mobilegl_server_main(int a } WireLogError("MG_Remote server: pid=%d listening on \"%s\"", selfPid, endpoint.c_str()); + // ---- 1.5. CONFIG, AND THEN D1. + // + // TWO SEPARATE DEFECTS LIVED HERE, and both were silent. + // + // (a) NOTHING LOADED THE CONFIG AT ALL. mobilegl_server_main calls + // InitServerRoleForSpawn directly rather than MobileGL::Initialize, and + // MG_ConfigLoader::Init is a step of the latter - so every MG_Config + // value in this process was its STATIC DEFAULT. MOBILEGL_BACKEND_TYPE + // survives the launcher's scrub on purpose, and it was being read by + // nobody: a DirectVulkan session would have brought up a DirectGLES + // server and then disagreed with its client about which backend the + // capability mask describes. + // + // (b) THE CHILD RESOLVED Transport == Monolith. CONTRACT-P6 §3.1 (D1) says + // it must read as Spawn, and a6 measured the cost of getting it wrong: + // 226 live lines test `Transport != Monolith` to select the SERVER arm, + // 148 of them in MG_Backend. Monolith flips all of them to frontend glue + // in the one process that has no frontend. Observed, exactly as + // predicted: TextureImpl::UnitTexturesByHandle() answered false, Clear + // took the legacy arm that dereferences borrowed FRONTEND slot pointers, + // and the apply thread took a SIGSEGV inside SyncNeccessaryTextures on + // the first Clear record of the OpenRA retrace. + // + // IT IS SET HERE AND NOT THROUGH THE ENVIRONMENT because the environment is + // where it must NOT appear: LooksLikeAClientEnvironment above exits 65 if + // MOBILEGL_TRANSPORT survived into this process, and that check is + // anti-recursion catch (b). D1's own answer is that Dial - not Transport - + // is the anti-recursion axis, so Transport is free to say what is true. + MobileGL::MG_ConfigLoader::Init(); + MobileGL::MG_Config::Transport = MobileGL::MG_Config::TransportMode::Spawn; + WireLogError("MG_Remote server: pid=%d config loaded, backend=%d, Transport pinned to Spawn " + "(D1: this process IS the server arm)", + selfPid, static_cast(MobileGL::MG_Config::ActiveBackendType)); + // ---- 2. the backend, through the SAME entry point the inproc server role // uses (MG_Backend/ServerRole.h). No EGL yet: the context is created on the // apply thread when the client's first eglMakeCurrent crosses. @@ -188,8 +224,21 @@ extern "C" __attribute__((visibility("default"))) int mobilegl_server_main(int a std::fflush(nullptr); ::_exit(74); } - SocketDoorbell selfBell(serverBell[0], /*code=*/1, /*ownsFd=*/true); - SocketDoorbell peerBell(clientBell[0], /*code=*/1, /*ownsFd=*/true); + // PARK AND NOTIFY ON DIFFERENT DESCRIPTORS, and this is not a refinement - + // the single-fd form CANNOT WAKE ITSELF. send() on one end of a socketpair + // is delivered to the OTHER end, so a one-fd bell built on serverBell[0] + // writes to serverBell[1] and polls serverBell[0]: correct for waking a + // PEER, useless for waking a thread in this process. The server needs + // exactly that, because the control pump posts into the apply thread's + // mailbox and then has to ring the bell that thread is parked on. Measured + // symptom when it was wrong: every eglInitialize crossed, landed in the + // mailbox, and timed out at 5 s because nothing ever woke mgl-srv-apply. + SocketDoorbell selfBell(/*parkFd=*/serverBell[0], /*notifyFd=*/serverBell[1], /*code=*/1, + /*ownsFds=*/true); + // RINGS ONLY. The client parks on clientBell[0]; writing clientBell[1] is + // what reaches it. parkFd = -1 says in the type that this side never waits. + SocketDoorbell peerBell(/*parkFd=*/-1, /*notifyFd=*/clientBell[1], /*code=*/1, + /*ownsFds=*/true); Server::ServerSession& session = Server::ServerSessionInstance(); session.SetExternalDoorbells(&selfBell, &peerBell); @@ -237,10 +286,15 @@ extern "C" __attribute__((visibility("default"))) int mobilegl_server_main(int a ::_exit(69); } } - // Slots 4 and 5 are the bells. The client gets the OTHER end of each pair: - // it writes to the server's bell and polls its own. - const int bellFds[2] = {serverBell[1], clientBell[1]}; - for (std::uint32_t index = 0; index < 2; ++index) { + // SLOTS 4, 5 AND 6 ARE THE BELLS - three, not two, and the third is the + // one the two-fd form above forces: + // 4 serverBell[1] the client's handle to RING THE SERVER + // 5 clientBell[0] the client's PARK end + // 6 clientBell[1] the client's handle to ring ITSELF, for the same + // reason the server needs serverBell[1] + // Sharing DUPs, so the descriptors these bells own stay valid here. + const int bellFds[3] = {serverBell[1], clientBell[0], clientBell[1]}; + for (std::uint32_t index = 0; index < 3; ++index) { Sideband sideband{4 + index, 0}; if (control->ShareFd(bellFds[index], MobileGLByteSpan{&sideband, sizeof(sideband)}) != MOBILEGL_OK) { @@ -250,8 +304,9 @@ extern "C" __attribute__((visibility("default"))) int mobilegl_server_main(int a ::_exit(69); } } - ::close(serverBell[1]); - ::close(clientBell[1]); + // clientBell[0] is the CLIENT's park end and no bell here owns it; the + // other three belong to selfBell and peerBell and are closed with them. + ::close(clientBell[0]); } // ---- 7. the apply thread. It holds the native context for its whole life, @@ -264,7 +319,7 @@ extern "C" __attribute__((visibility("default"))) int mobilegl_server_main(int a } WireLogError("MG_Remote server: pid=%d transport=spawn role=server ready " - "(backend up, six descriptors handed over, apply thread running)", + "(backend up, seven descriptors handed over, apply thread running)", selfPid); // ---- 8. the control pump. EOF is the whole exit condition: no timeout may diff --git a/MobileGL/MG_Remote/Server/SurfaceControlFrame.cpp b/MobileGL/MG_Remote/Server/SurfaceControlFrame.cpp index 4bcfd882..01bba314 100644 --- a/MobileGL/MG_Remote/Server/SurfaceControlFrame.cpp +++ b/MobileGL/MG_Remote/Server/SurfaceControlFrame.cpp @@ -23,7 +23,7 @@ namespace MobileGL::MG_Remote::Server { case SurfaceControlOp::SetSwapInterval: return "SetSwapInterval"; case SurfaceControlOp::ReleaseResources: return "ReleaseResources"; case SurfaceControlOp::SetWindowHandle: return "SetWindowHandle"; - case SurfaceControlOp::InitCapabilitiesInprocOnly: return "InitCapabilitiesInprocOnly"; + case SurfaceControlOp::InitCapabilities: return "InitCapabilities"; case SurfaceControlOp::SwapBuffersInprocOnly: return "SwapBuffersInprocOnly"; case SurfaceControlOp::InitWindowSurfaceInprocOnly: return "InitWindowSurfaceInprocOnly"; case SurfaceControlOp::ProbeForTesting: return "ProbeForTesting"; @@ -43,6 +43,10 @@ namespace MobileGL::MG_Remote::Server { case SurfaceControlOp::SetSwapInterval: case SurfaceControlOp::ReleaseResources: case SurfaceControlOp::SetWindowHandle: + // cp: the request half of InitCapabilities. Its ANSWER is a CapsSnapshot + // frame, which is why this row was missing; the request still has to + // cross, because under spawn the apply thread is in another process. + case SurfaceControlOp::InitCapabilities: return true; default: return false; diff --git a/MobileGL/MG_Remote/Server/SurfaceControlFrame.h b/MobileGL/MG_Remote/Server/SurfaceControlFrame.h index 7aef9b82..42d33d9e 100644 --- a/MobileGL/MG_Remote/Server/SurfaceControlFrame.h +++ b/MobileGL/MG_Remote/Server/SurfaceControlFrame.h @@ -55,12 +55,18 @@ namespace MobileGL::MG_Remote::Server { SetSwapInterval = 8, ReleaseResources = 9, SetWindowHandle = 10, + // P6 (cp) MOVED THIS ONE ONTO THE WIRE, and the old name was a lie the + // moment spawn existed. f0-egl reasoned that the wire's ANSWER to + // InitCapabilities is the CapsSnapshot frame, which is true and beside + // the point: the REQUEST still has to reach the apply thread, and under + // spawn that thread is in another process. Measured before the fix - + // every spawned eglMakeCurrent died at "surface op kind 11 cannot cross + // the wire (InprocOnlyOpOnTheWire)", one call after the backend came up. + InitCapabilities = 11, // INPROC-ONLY kinds: legal inside this process's frame channel, NEVER encodable onto the // wire (SurfaceOpCodec refuses them by name). f0-egl's census found no production caller - // for the two dead forwarders and a CapsSnapshot answer for the third; they ride the same - // frame channel because the function-pointer mailbox is GONE, not because they are wire - // ops. - InitCapabilitiesInprocOnly = 11, // the wire's answer is the CapsSnapshot frame itself + // for the two dead forwarders; they ride the same frame channel because the + // function-pointer mailbox is GONE, not because they are wire ops. SwapBuffersInprocOnly = 12, // present travels as a record (the class-B Present emitter) InitWindowSurfaceInprocOnly = 13,// a client-side no-op; kept for the inproc test lane ProbeForTesting = 14, // MG_Test's arbitrary-work seam through the same channel diff --git a/MobileGL/MG_Remote/Transport/Doorbell.cpp b/MobileGL/MG_Remote/Transport/Doorbell.cpp index 8ebb36dd..606f7f29 100644 --- a/MobileGL/MG_Remote/Transport/Doorbell.cpp +++ b/MobileGL/MG_Remote/Transport/Doorbell.cpp @@ -212,7 +212,7 @@ namespace MobileGL::MG_Remote::Transport { // ----------------------------------------------------------------------- SocketDoorbell::SocketDoorbell(int fd, std::uint8_t code, bool ownsFd) - : m_fd(fd), m_code(code), m_ownsFd(ownsFd) { + : m_fd(fd), m_notifyFd(fd), m_code(code), m_ownsFd(ownsFd) { #if defined(SO_NOSIGPIPE) // The per-socket form of MSG_NOSIGNAL, on the platforms that lack the per-call one: // a Notify to a hung-up peer must come back as EPIPE, not as a fatal signal. @@ -223,19 +223,44 @@ namespace MobileGL::MG_Remote::Transport { #endif } + // The two-fd form: park on one descriptor, ring another. See the header for + // why the server needs it and the cross-process form cannot provide it. + SocketDoorbell::SocketDoorbell(int parkFd, int notifyFd, std::uint8_t code, bool ownsFds) + : m_fd(parkFd), m_notifyFd(notifyFd), m_code(code), m_ownsFd(ownsFds) { +#if defined(SO_NOSIGPIPE) + for (int fd : {m_fd, m_notifyFd}) { + if (fd >= 0) { + const int one = 1; + (void)::setsockopt(fd, SOL_SOCKET, SO_NOSIGPIPE, &one, sizeof(one)); + } + } +#endif + } + SocketDoorbell::~SocketDoorbell() { - if (m_ownsFd && m_fd >= 0) { + if (!m_ownsFd) { + return; + } + if (m_fd >= 0) { ::close(m_fd); } + // Only when they are different: the single-fd form has m_notifyFd == m_fd + // and closing it twice is a double close, which on a busy process closes + // somebody else's descriptor rather than failing. + if (m_notifyFd >= 0 && m_notifyFd != m_fd) { + ::close(m_notifyFd); + } } void SocketDoorbell::Notify() { - if (m_fd < 0) { + // The NOTIFY descriptor, not the park one: a ring-only bell has no park + // fd at all and must still be able to ring. + if (m_notifyFd < 0) { return; } const std::uint8_t byte = m_code; for (;;) { - const ssize_t written = ::send(m_fd, &byte, 1, MSG_DONTWAIT | MSG_NOSIGNAL); + const ssize_t written = ::send(m_notifyFd, &byte, 1, MSG_DONTWAIT | MSG_NOSIGNAL); if (written == 1) { return; } @@ -259,6 +284,11 @@ namespace MobileGL::MG_Remote::Transport { } bool SocketDoorbell::Park(std::uint32_t timeoutMs) { + // A ring-only bell was never a waiter. Saying so immediately is the + // honest answer; blocking forever on -1 would be a hang with no cause. + if (m_fd < 0) { + return false; + } if (m_fd < 0 || m_dead) { return false; } diff --git a/MobileGL/MG_Remote/Transport/Doorbell.h b/MobileGL/MG_Remote/Transport/Doorbell.h index 1f31552b..43f1dc1b 100644 --- a/MobileGL/MG_Remote/Transport/Doorbell.h +++ b/MobileGL/MG_Remote/Transport/Doorbell.h @@ -397,6 +397,23 @@ namespace MobileGL::MG_Remote::Transport { // When `ownsFd` the descriptor is closed with this object. `code` is // the byte written by Notify. SocketDoorbell(int fd, std::uint8_t code, bool ownsFd); + + // P6: PARK AND NOTIFY ON DIFFERENT DESCRIPTORS. + // + // The single-fd form above is the cross-process one: send() on this end + // is delivered to the PEER's end, so the peer's own SocketDoorbell is + // what receives it. That is correct, and it is also why the single-fd + // form cannot wake ITSELF - and the server needs exactly that, because + // its control pump posts into the apply thread's mailbox and must ring + // the bell that thread is parked on. Under inproc CondVarDoorbell has no + // such split: notify and wait are the same object. + // + // So: `parkFd` is polled, `notifyFd` is written. For a socketpair where + // this side holds both ends they are [0] and [1], and a peer that must + // also be able to ring it gets a DUP of [1]. `parkFd` may be -1 for a + // bell this side only ever RINGS - Park then returns immediately, which + // is the honest answer for an object that was never a waiter. + SocketDoorbell(int parkFd, int notifyFd, std::uint8_t code, bool ownsFds); ~SocketDoorbell() override; void Notify() override; @@ -413,7 +430,8 @@ namespace MobileGL::MG_Remote::Transport { // afterwards. std::uint64_t Drain(); - int m_fd; + int m_fd; // polled + int m_notifyFd; // written; equals m_fd in the single-fd (cross-process) form std::uint8_t m_code; bool m_ownsFd; bool m_dead = false; diff --git a/MobileGL/MG_Test/Wire/SurfaceControlFrameTest.cpp b/MobileGL/MG_Test/Wire/SurfaceControlFrameTest.cpp index 5f935d5c..528d171c 100644 --- a/MobileGL/MG_Test/Wire/SurfaceControlFrameTest.cpp +++ b/MobileGL/MG_Test/Wire/SurfaceControlFrameTest.cpp @@ -137,8 +137,11 @@ TEST(SurfaceControlFrameTest, ExactlyTheTenFramedOpsHaveWireKindsAndTheValuesAre EXPECT_EQ(static_cast(::MobileGL::Wire::SurfaceOpKind::SetWindowHandle), 10u); // The inproc-only kinds refuse the wire by name, both directions. + // InitCapabilities WAS in this list and cp took it out; the eleventh op has a + // test of its own below rather than a line here, because this test's name is + // a statement about ten and G14 does not let a name be traded for another. const SurfaceControlOp inprocOnly[] = { - SurfaceControlOp::None, SurfaceControlOp::InitCapabilitiesInprocOnly, + SurfaceControlOp::None, SurfaceControlOp::SwapBuffersInprocOnly, SurfaceControlOp::InitWindowSurfaceInprocOnly, SurfaceControlOp::ProbeForTesting, }; @@ -153,6 +156,37 @@ TEST(SurfaceControlFrameTest, ExactlyTheTenFramedOpsHaveWireKindsAndTheValuesAre << "a wire kind the schema does not define must not decode into an op"; } +TEST(SurfaceControlFrameTest, TheEleventhFramedOpIsInitCapabilitiesAndItCrossesTheWire) { + // cp. The op above this one in the enum, SetWindowHandle, was fc's last + // append; this is P6's. It gets a test rather than a row in the ten-op table + // because the failure it guards is specific and was REAL: with no wire kind + // the codec answered InprocOnlyOpOnTheWire, BackendObject_Remote's + // InitCapabilities call failed, MakeEGLCurrent reported "InitCapabilities + // failed", and the OpenRA spawn retrace died one call after the server's + // backend had come up green. Nothing about that read as a missing enum row. + ::MobileGL::Wire::SurfaceOpKind kind; + ASSERT_TRUE(WireKindForSurfaceControlOp(SurfaceControlOp::InitCapabilities, &kind)) + << "InitCapabilities lost its wire kind; a spawn client cannot ask for caps"; + EXPECT_EQ(static_cast(kind), 11u) << "wire ABI: append-only, and 11 is taken"; + EXPECT_EQ(static_cast(SurfaceControlOp::InitCapabilities), 11u); + + SurfaceControlOp back; + ASSERT_TRUE(SurfaceControlOpForWireKind(kind, &back)) + << "the decode range's upper bound did not move with the append, so the new tag " + "reads as out-of-range - which surfaces as Fatal{ProtocolCorruption}, not as the " + "missing row it is"; + EXPECT_EQ(back, SurfaceControlOp::InitCapabilities); + + // And it really encodes. WireKindForSurfaceControlOp answering yes is not the + // same fact as the encoder accepting the frame: the refusal the client hit + // came out of EncodeSurfaceOpFrame. + flatbuffers::FlatBufferBuilder builder(256); + SurfaceControlFrame frame; + frame.kind = SurfaceControlOp::InitCapabilities; + frame.seq = 7; + EXPECT_EQ(EncodeSurfaceOpFrame(frame, &builder), SurfaceWireError::None); +} + TEST(SurfaceControlFrameTest, WindowBackendAndWireWindowKindMapExplicitlyBothWays) { using MG_Backend::WindowBackend; const struct { @@ -269,8 +303,6 @@ TEST(SurfaceControlFrameTest, MalformedAndInprocOnlyFramesRefuseToEncode) { EXPECT_EQ(EncodeSurfaceOpFrame(frame, &builder), SurfaceWireError::InprocOnlyOpOnTheWire); frame.kind = SurfaceControlOp::SwapBuffersInprocOnly; EXPECT_EQ(EncodeSurfaceOpFrame(frame, &builder), SurfaceWireError::InprocOnlyOpOnTheWire); - frame.kind = SurfaceControlOp::InitCapabilitiesInprocOnly; - EXPECT_EQ(EncodeSurfaceOpFrame(frame, &builder), SurfaceWireError::InprocOnlyOpOnTheWire); frame.kind = SurfaceControlOp::InitWindowSurfaceInprocOnly; EXPECT_EQ(EncodeSurfaceOpFrame(frame, &builder), SurfaceWireError::InprocOnlyOpOnTheWire); diff --git a/MobileGL/MG_Util/Debug/Log.cpp b/MobileGL/MG_Util/Debug/Log.cpp index c0801875..7b90a488 100644 --- a/MobileGL/MG_Util/Debug/Log.cpp +++ b/MobileGL/MG_Util/Debug/Log.cpp @@ -8,6 +8,8 @@ #include "../../Includes.h" +#include // std::strcmp, for the spawned-server role check in InitFile + namespace MobileGL { namespace MG_Util::Debug { static FILE* s_logFile = nullptr; @@ -70,7 +72,32 @@ namespace MobileGL { logPath = MOBILEGL_LOG_FILE_PATH; } if (logPath && *logPath) { - s_logFile = std::fopen(logPath, "w"); + // "a", NOT "w", WHENEVER A SECOND PROCESS SHARES THIS PATH. + // + // Under transport=spawn the server is a separate process + // writing the same MOBILEGL_LOG_FILE_PATH, and two "w" + // handles each carry their OWN file offset: both start at 0 + // and overwrite each other's bytes. The result is not + // interleaved, it is CORRUPT - observed as half-lines like + // `ote client: CapsMirror generation 1 adopted` while + // diagnosing the spawn retrace, and as the server's whole + // startup block vanishing under the client's later writes. + // O_APPEND makes each write atomic at the end of the file, + // which is what makes the log a usable census across both + // processes - and run_trace_case.cmake's Fatal{ count reads + // exactly this file. + // + // THE CLIENT STILL TRUNCATES ONCE, so a fresh run does not + // read as a growing pile of old ones. The server never does: + // it is always started by a client that has already opened + // this path, and a truncate from the far side would delete + // the very startup lines a failed session needs. + const char* role = std::getenv("MOBILEGL_IPC_ROLE"); + const bool isSpawnedServer = role != nullptr && std::strcmp(role, "server") == 0; + if (!isSpawnedServer) { + if (FILE* truncate = std::fopen(logPath, "w")) std::fclose(truncate); + } + s_logFile = std::fopen(logPath, "a"); } } #endif diff --git a/tools/trace_replay/CMakeLists.txt b/tools/trace_replay/CMakeLists.txt index 81229cb2..d5a6fae0 100644 --- a/tools/trace_replay/CMakeLists.txt +++ b/tools/trace_replay/CMakeLists.txt @@ -465,6 +465,22 @@ function(add_trace_replay_split_test CASE_NAME) endif() add_trace_replay_test(${CASE_NAME} DirectGLES VARIANT SPLIT TRANSPORT inproc IPC_SERVER_PATH "${CMAKE_BINARY_DIR}/libMobileGLServer.so" ${ARGN}) + + # --- P6: the SPAWN arm, beside the inproc one rather than instead of it ------------------ + # + # TWO ARMS, NOT ONE RENAMED. The inproc arm is the CONTROL: it is the same + # library, the same codec, the same applier and the same picture, with the + # only difference being which process the server role runs in. A spawn arm + # that had replaced it would have left "did the transport break it?" and + # "did the phase break it?" answerable only together. + # + # G14 is satisfied by construction - a name is added, none is removed - and + # the VARIANT gives it its own output and artifact directories, which is the + # half the comment above says is easy to forget: both arms write + # output/mobilegl.log, and that log is the ONLY valid refusal census because + # the console sink is compiled out of these configurations. + add_trace_replay_test(${CASE_NAME} DirectGLES VARIANT SPAWN TRANSPORT spawn + IPC_SERVER_PATH "${CMAKE_BINARY_DIR}/libMobileGLServer.so" ${ARGN}) endfunction() set(MOBILEGL_TRACE_CASES_CMAKE "${CMAKE_CURRENT_BINARY_DIR}/trace_cases.cmake") diff --git a/tools/trace_replay/run_trace_case.cmake b/tools/trace_replay/run_trace_case.cmake index f684c8e3..358de6cd 100644 --- a/tools/trace_replay/run_trace_case.cmake +++ b/tools/trace_replay/run_trace_case.cmake @@ -262,7 +262,34 @@ if(DEFINED ENV{MOBILEGL_TRANSPORT} AND NOT "$ENV{MOBILEGL_TRANSPORT}" STREQUAL " # DEBUG to debug a split failure. The sentence below exists only in # ConfigLoader::InitTransport's InProcess arm, which exists only under # MOBILEGL_BUILD_DISAGGREGATED. - set(split_expected_marker "MOBILEGL_TRANSPORT=inproc - the MGPipe record stream") + # + # ONE ROW PER TRANSPORT THIS RUNNER ADMITS. A transport with no row is a + # FATAL below rather than a fall-through, because the fall-through is the + # exact accident this whole block exists to prevent: an arm that named a + # transport nobody asserted on and went green having proved nothing. + # + # THE SPAWN ROW CARRIES A SECOND SENTENCE, and it is the stronger half. + # The ConfigLoader marker proves the VALUE RESOLVED - it is a statement + # about a parser. `spawn ARMED - the server role runs in pid N` is + # written by ClientSession::StartSpawned only after the connect and the + # handshake succeeded, and it carries a pid that is not this process's. + # inproc resolves its own marker while running the server role on a + # thread HERE; nothing on that path can write this one. + set(split_expected_second "") + if("$ENV{MOBILEGL_TRANSPORT}" STREQUAL "inproc") + set(split_expected_marker "MOBILEGL_TRANSPORT=inproc - the MGPipe record stream") + elseif("$ENV{MOBILEGL_TRANSPORT}" STREQUAL "spawn") + set(split_expected_marker "MOBILEGL_TRANSPORT=spawn - the MGPipe record stream") + set(split_expected_second "spawn ARMED - the server role runs in pid ") + else() + message(FATAL_ERROR + "MOBILEGL_TRANSPORT=$ENV{MOBILEGL_TRANSPORT} is set for ${split_case} and this " + "runner has no marker for it, so the run could not be checked. ConfigLoader " + "recognises monolith|inproc|spawn|unix:|pipe: and REFUSES the last " + "two BY NAME, staying on monolith - a run that asked for one of those would " + "have replayed monolith. Add a row above when the transport lands; do not let " + "an unasserted transport reach a green.") + endif() string(FIND "${split_log}" "${split_expected_marker}" split_armed_at) if(split_armed_at EQUAL -1) # Say which transport was actually asked for. ConfigLoader REFUSES spawn / unix: / @@ -270,11 +297,14 @@ if(DEFINED ENV{MOBILEGL_TRANSPORT} AND NOT "$ENV{MOBILEGL_TRANSPORT}" STREQUAL " # has a different diagnosis from one that set inproc against a monolith library, and # the old message named `inproc` either way. set(split_refusal "") - if(NOT "$ENV{MOBILEGL_TRANSPORT}" STREQUAL "inproc") + if("$ENV{MOBILEGL_TRANSPORT}" STREQUAL "spawn") set(split_refusal - " NOTE: this run asked for '$ENV{MOBILEGL_TRANSPORT}', which P5 does not " - "implement - ConfigLoader recognises spawn / unix: / pipe: and REFUSES them by " - "name, staying on monolith. Only 'inproc' can resolve in P5.") + " NOTE: this run asked for 'spawn'. The server is a SEPARATE PROCESS here, so " + "the two ways to get this far with no marker are a library that has no spawn " + "arm (pre-P6) and a launcher that never resolved the transport at all. " + "MOBILEGL_IPC_SERVER_PATH must point at a libMobileGLServer.so built from this " + "same tree; LaunchServer REFUSES an unresolvable image BY NAME and never falls " + "back to running the server role locally.") endif() message(FATAL_ERROR "MOBILEGL_TRANSPORT=$ENV{MOBILEGL_TRANSPORT} is set for ${split_case} and the library " @@ -288,6 +318,23 @@ if(DEFINED ENV{MOBILEGL_TRANSPORT} AND NOT "$ENV{MOBILEGL_TRANSPORT}" STREQUAL " "admits INFO (at WARN or above the line is compiled out and this reds for no " "defect).${split_refusal}") endif() + # ... and the half that a same-process run cannot satisfy. Checked + # SEPARATELY from the marker above so the two failures read differently: + # no marker means the transport never resolved, while marker-but-no-pid + # means it resolved and the session never came up across the boundary. + if(NOT "${split_expected_second}" STREQUAL "") + string(FIND "${split_log}" "${split_expected_second}" split_second_at) + if(split_second_at EQUAL -1) + message(FATAL_ERROR + "${split_case}: ${mobilegl_log} reports MOBILEGL_TRANSPORT=" + "$ENV{MOBILEGL_TRANSPORT} resolved, but carries no " + "\"${split_expected_second}\". That sentence is written by " + "ClientSession::StartSpawned AFTER the launch, the connect and the handshake " + "all succeeded, and the pid in it is the SERVER's - it is the only evidence " + "in this log that the server role left this process. Without it the run " + "resolved a transport and then replayed something else.") + endif() + endif() # The refusal census. Recorded on every split run, pass or fail. file(STRINGS "${mobilegl_log}" split_fatals REGEX "Fatal\\{") list(LENGTH split_fatals split_fatal_count)