diff --git a/MobileGL/Config.h b/MobileGL/Config.h index 5a98476b..0075a232 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -456,6 +456,20 @@ namespace MobileGL::MG_Config { // The of `unix:` / the of `pipe:`. Empty for the other three modes. extern String TransportEndpoint; + // P7 F1. DID **THIS PROCESS'S OWN CONFIGURATION** ASK FOR A SPLIT TRANSPORT. + // + // `Transport` alone cannot answer that, and the difference is exactly the one the F1 gate + // turns on. A unit fixture assigns `Transport` by hand to put the code under test on its + // split arm (ServerLoopTest's main does, and says why) while building no client at all; a + // real run gets it from MG_ConfigLoader::Init(), and that run WILL bring a client half up. + // Only the second may be held to "never decide a record family's fate from a placeholder + // caps mirror" - in the first there is no handshake, no mirror to adopt and nothing the + // rule could mean. + // + // Written once, by InitTransport(), and never cleared: a process does not stop having been + // configured. Read by MG_Remote::Client::CapsMirror::RequireFirstSnapshot. + extern Bool SplitTransportRequestedByConfig; + // The MOBILEGL_IPC_* family. A separate table rather than more FeaturesTable members, // for the G1 reason above and because every field here is meaningless without the // transport: a build that cannot reach the MG_Remote code cannot honour one of them. diff --git a/MobileGL/ConfigLoader.cpp b/MobileGL/ConfigLoader.cpp index 60a68c2a..3826c87e 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -32,6 +32,7 @@ namespace MobileGL::MG_Config { // and there is nothing to define. TransportMode Transport = TransportMode::Monolith; String TransportEndpoint; + Bool SplitTransportRequestedByConfig = false; IpcTable Ipc; #endif } // namespace MobileGL::MG_Config @@ -330,6 +331,10 @@ namespace MobileGL::MG_ConfigLoader { } if (lowered == "inproc") { MG_Config::Transport = MG_Config::TransportMode::InProcess; + // P7 F1: see Config.h. Set beside the mode, on the two arms that actually resolve + // one, and NOT on the two that name a transport and then stay monolith - a process + // that was refused its transport brings no client half up either. + MG_Config::SplitTransportRequestedByConfig = true; MGLOG_I("Config: MOBILEGL_TRANSPORT=inproc - the MGPipe record stream crosses a real " "ring to an apply thread"); return; @@ -342,6 +347,7 @@ namespace MobileGL::MG_ConfigLoader { // rather than the fork-coupled shape ARCHITECTURE.md:488 described. if (lowered == "spawn") { MG_Config::Transport = MG_Config::TransportMode::Spawn; + MG_Config::SplitTransportRequestedByConfig = true; // P7 F1, as above. // 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 diff --git a/MobileGL/Init.cpp b/MobileGL/Init.cpp index 2436d792..aef7e23e 100644 --- a/MobileGL/Init.cpp +++ b/MobileGL/Init.cpp @@ -121,9 +121,50 @@ namespace MobileGL { // boundary counters latch their enable flag here, so every counting site in the // two backends is a load of an already-settled global for the rest of the run. MG_Util::PipeStats::Init(); +#if MOBILEGL_BUILD_DISAGGREGATED + // F1 (P7 wave 2). THE WIRE COMES UP BEFORE THE FIRST FRONTEND OBJECT IS BORN, and the + // ORDER is the fix - not a wait, and not a friendlier default mask. + // + // MG_State::Init() constructs GLContext, and GLContext's TextureState constructor + // materialises one default texture object per target (GL 3.3 core 3.8, + // TextureState.cpp:62-71). TextureObjectBase's constructor emits resource_create for + // every one of them (TextureState/TextureObject.cpp:168-169), and that emission is + // gated, through PipeFill's FamilyIsLive, on + // CapsMirror::ServerConsumes(kMGPipeSubsystemTextureResources). In the old order the + // mirror was still GENERATION 0 when it was asked - it had no snapshot because the + // session had not been started yet, MG_Backend::Init() being what starts it - so it + // answered from the placeholder, every one of those creates was withheld, and the log + // said so once and then never again: + // + // MG_Remote client: the server does not consume MGPipe subsystem 0x400 - this + // family emits NOTHING and the legacy pull path runs for it (R-8). callMask=0x0, + // caps generation 0 + // + // That is not a race and it never was one: the line reproduces byte-for-byte on + // lavapipe and on an Adreno 830, because the two Init calls are two statements in one + // thread. And "the legacy pull path runs for it" is false under split - the client has + // no pull path to fall back to - so the records are simply lost, and only a later + // respecify's self-healing create (TextureEmit.h:872) ever brings one back. + // + // A BOUNDED WAIT INSIDE ServerConsumes CANNOT FIX THIS AND WOULD DEADLOCK: during + // MG_State::Init() no handshake is in flight, and the one that will land is started + // LATER BY THIS SAME THREAD. The only correct answer is to have already brought it up, + // which is what this hook does. ServerConsumes now REFUSES to answer from a + // placeholder (CapsMirror.cpp), so a regression of this order is a named Fatal rather + // than a whole family of silently dropped records. + // + // In a build without MOBILEGL_BUILD_DISAGGREGATED this statement does not exist, so + // the pull build gains no symbol, no branch and no byte - which is what G1 measures. + const Bool splitBackendIsUp = MG_Backend::InitSplitRolesBeforeState(); +#endif MG_State::Init(); MGLOG_D("MG_State initialized"); - MG_Backend::Init(); +#if MOBILEGL_BUILD_DISAGGREGATED + if (!splitBackendIsUp) +#endif + { + MG_Backend::Init(); + } MGLOG_D("MG_Backend initialized"); MG_Impl::Init(); MGLOG_D("MG_Impl initialized"); diff --git a/MobileGL/MG_Backend/BackendObjects.h b/MobileGL/MG_Backend/BackendObjects.h index 8d71c674..0bc43b11 100644 --- a/MobileGL/MG_Backend/BackendObjects.h +++ b/MobileGL/MG_Backend/BackendObjects.h @@ -39,5 +39,21 @@ namespace MobileGL::MG_Backend { // client-minted and needs no backend call, which is P10's. CONTRACT-P5 4 flags this as a // KNOWN OPEN ITEM and asks v1 to record which way it went: P5 keeps today's order. void ShutdownSplitRoles(); + + // F1 (P7 wave 2). Init()'s SPLIT ARM, run BEFORE MG_State::Init() rather than after it. + // + // Returns true when this process took the split path - whether the bring-up succeeded or + // was refused by name - and the caller must then NOT call Init() a second time. Returns + // false under MG_Config::TransportMode::Monolith, where there is nothing to bring up and + // Init() keeps its own place after MG_State::Init(). + // + // WHY IT EXISTS AT ALL, rather than MobileGL::Initialize simply calling Init() earlier: + // the monolith arm of Init() must keep running after MG_State::Init(), because + // BackendObject_DirectGLES::Initialize() is the tree's own order for it and moving it is a + // pull-build change G1 forbids. So the split arm - and only the split arm - moves. + // + // It exists ONLY in a split build, for the reason InitServerRoleForSpawn states: a + // definition guarded only on the inside still emits a symbol, and G1 measured that. + Bool InitSplitRolesBeforeState(); #endif } // namespace MobileGL::MG_Backend diff --git a/MobileGL/MG_Backend/Init.cpp b/MobileGL/MG_Backend/Init.cpp index d0ebe69f..047f79a3 100644 --- a/MobileGL/MG_Backend/Init.cpp +++ b/MobileGL/MG_Backend/Init.cpp @@ -276,55 +276,16 @@ namespace MobileGL::MG_Backend { pActiveBackendObject = MakeUnique(); return true; } - } // namespace -#endif -#if MOBILEGL_BUILD_DISAGGREGATED - void ShutdownSplitRoles() { - if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return; - // ClientSession::Stop IS table 3's whole order and it is idempotent: publish and wait - // for the server to drain (bounded - a lost record must be a red lane, not a hung - // exit), Doorbell::Kill through the transport's Shutdown, ServerLoop::Stop's bounded - // join - which also destroys the server's private BackendObject ON the apply thread - // while it still owns the context - the transport, and only THEN anything an emitter - // owns. A var-tail still named by an unapplied record is a use-after-free the join is - // what prevents, which is why the order is not a preference. - MG_Remote::Client::ClientSessionInstance().Stop(); - // M-6: ClientSession::Stop's !m_started arm (a Start that FAILED after - // ServerSession::Accept - a refused Accept, an invalid cmd/reply ring) tears down only - // the client half and never stops the apply thread or drops the server's private backend, - // which ServerLoop::CreateBackend already built and which holds the process-wide - // g_resourceOps. So call ServerLoop::Stop() here unconditionally. It is idempotent: on the - // started path ClientSession::Stop already joined the thread, so this hits Stop's - // !joinable arm, which resets a backend that never ran a thread and is otherwise a no-op. - // Without this an early Start failure leaves BackendObject_DirectGLES permanently alive - // and every later split bring-up in the process fails at CreateBackend's m_backend!=null - // guard. - MG_Remote::Server::ServerLoopInstance().Stop(); - } -#endif + // F1 (P7 wave 2). THE WHOLE SPLIT ARM OF Init(), AS ONE BODY, so it can run from + // either of the two places that may reach it - MobileGL::Initialize's early hook, + // before MG_State::Init(), and Init() itself for a direct caller such as a unit + // fixture - without a second copy. Two copies of a bring-up order is how the two + // orders come to differ on the day one of them is wrong. + Bool g_splitBackendBroughtUp = false; -#if MOBILEGL_BUILD_DISAGGREGATED - // The spawn server's entry into the SAME bring-up the inproc server role - // runs (ServerRole.h). A thin forwarder on purpose: the body stays in the - // anonymous namespace beside InitSplitRoles so the two cannot drift. - // - // THE WHOLE FUNCTION IS INSIDE THE GUARD, not just its body. A version with - // the guard inside still DEFINES the symbol in a pull build, and G1 caught - // it: 2 symbols added, 1 removed, .text +16 bytes. The pull build's identity - // is byte-for-byte, and "it returns false there" is not the same as "it is - // not there". - Bool InitServerRoleForSpawn() { return InitServerRoleCommon(); } -#endif - - void Init() { - MGLOG_D("Initializing MobileGL Backend..."); - -#if MOBILEGL_BUILD_DISAGGREGATED - // THE SINGLE HOOK. In a build without MOBILEGL_BUILD_DISAGGREGATED, MG_Config::Transport - // is a `constexpr Monolith` (Config.h) and this whole statement is discarded, so the - // pull build gains no symbol, no branch and no byte - which is what G1 measures. - if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + void InitSplitBackend() { + g_splitBackendBroughtUp = true; if (!InitSplitRoles()) { // NOT a fallback to the switch. pActiveBackendObject stays null and the next GL // call fails loudly, which is the only honest outcome: the operator asked for a @@ -373,6 +334,75 @@ namespace MobileGL::MG_Backend { } } LogBackendInfo(); + } + } // namespace +#endif + +#if MOBILEGL_BUILD_DISAGGREGATED + // F1 (P7 wave 2). See BackendObjects.h for why this exists and MobileGL/Init.cpp for the + // defect the order closes. + Bool InitSplitRolesBeforeState() { + if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return false; + InitSplitBackend(); + return true; + } +#endif + +#if MOBILEGL_BUILD_DISAGGREGATED + void ShutdownSplitRoles() { + g_splitBackendBroughtUp = false; + if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return; + // ClientSession::Stop IS table 3's whole order and it is idempotent: publish and wait + // for the server to drain (bounded - a lost record must be a red lane, not a hung + // exit), Doorbell::Kill through the transport's Shutdown, ServerLoop::Stop's bounded + // join - which also destroys the server's private BackendObject ON the apply thread + // while it still owns the context - the transport, and only THEN anything an emitter + // owns. A var-tail still named by an unapplied record is a use-after-free the join is + // what prevents, which is why the order is not a preference. + MG_Remote::Client::ClientSessionInstance().Stop(); + // M-6: ClientSession::Stop's !m_started arm (a Start that FAILED after + // ServerSession::Accept - a refused Accept, an invalid cmd/reply ring) tears down only + // the client half and never stops the apply thread or drops the server's private backend, + // which ServerLoop::CreateBackend already built and which holds the process-wide + // g_resourceOps. So call ServerLoop::Stop() here unconditionally. It is idempotent: on the + // started path ClientSession::Stop already joined the thread, so this hits Stop's + // !joinable arm, which resets a backend that never ran a thread and is otherwise a no-op. + // Without this an early Start failure leaves BackendObject_DirectGLES permanently alive + // and every later split bring-up in the process fails at CreateBackend's m_backend!=null + // guard. + MG_Remote::Server::ServerLoopInstance().Stop(); + } +#endif + +#if MOBILEGL_BUILD_DISAGGREGATED + // The spawn server's entry into the SAME bring-up the inproc server role + // runs (ServerRole.h). A thin forwarder on purpose: the body stays in the + // anonymous namespace beside InitSplitRoles so the two cannot drift. + // + // THE WHOLE FUNCTION IS INSIDE THE GUARD, not just its body. A version with + // the guard inside still DEFINES the symbol in a pull build, and G1 caught + // it: 2 symbols added, 1 removed, .text +16 bytes. The pull build's identity + // is byte-for-byte, and "it returns false there" is not the same as "it is + // not there". + Bool InitServerRoleForSpawn() { return InitServerRoleCommon(); } +#endif + + void Init() { + MGLOG_D("Initializing MobileGL Backend..."); + +#if MOBILEGL_BUILD_DISAGGREGATED + // THE SINGLE HOOK. In a build without MOBILEGL_BUILD_DISAGGREGATED, MG_Config::Transport + // is a `constexpr Monolith` (Config.h) and this whole statement is discarded, so the + // pull build gains no symbol, no branch and no byte - which is what G1 measures. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + // F1: MobileGL::Initialize now brings the split roles up BEFORE MG_State::Init(), + // through InitSplitRolesBeforeState() below, so by the time Init() is reached on + // that path the work is done. A DIRECT caller - a unit fixture that calls + // MG_Backend::Init() itself - still gets the whole bring-up here. The flag rather + // than `pActiveBackendObject == nullptr`, because a bring-up that FAILED leaves + // that pointer null too and retrying it would refuse at CreateBackend's + // m_backend != nullptr guard with a second, misleading line. + if (!g_splitBackendBroughtUp) InitSplitBackend(); return; } #endif diff --git a/MobileGL/MG_Remote/Client/CapsMirror.cpp b/MobileGL/MG_Remote/Client/CapsMirror.cpp index a5c5ecad..2301e012 100644 --- a/MobileGL/MG_Remote/Client/CapsMirror.cpp +++ b/MobileGL/MG_Remote/Client/CapsMirror.cpp @@ -20,19 +20,35 @@ // THE PLACEHOLDER IS A REAL OBJECT, NOT A TEMPORARY. GetRendererInfo() and // GetDynamicParameters() return const references, so the storage has to outlive every caller; // the members below are it, default-constructed, and Adopt() replaces them in place. +// +// P7 F1 NARROWS "EVERY ACCESSOR" BY EXACTLY ONE. The ruling above is about the READ accessors +// and it stands for them: an imprecise startup log line is cheaper than restructuring +// MG_Backend::Init(). ServerConsumes() is not a read, it is a DECISION about whether a whole +// record family reaches the wire at all, and P5's "a placeholder consumes nothing, which is the +// safe direction because the legacy pull path runs" was simply untrue under split - the client +// has no pull path. It now refuses to answer from a placeholder; see RequireFirstSnapshot. #include "CapsMirror.h" #include "../CapsCodec.h" +#include "../FatalFunnel.h" +#include // MG_Config::SplitTransportRequestedByConfig +#include // MGPipeServerArm, D1c's role predicate #include #include +#include + namespace MobileGL::MG_Remote::Client { namespace { CapsAdoptedHook g_capsAdoptedHook = nullptr; + // F1. See CapsMirror.h for the contract of each of these three. + CapsFirstSnapshotWait g_firstSnapshotWait = nullptr; + ::std::atomic g_publishedGeneration{0}; + // R-8's negative control, counted at the funnel. Not atomics: every reader of this // mirror is the GL thread, by the same argument that makes one gPipeInputs legal. Uint64 g_consumerRefusals = 0; @@ -67,6 +83,9 @@ namespace MobileGL::MG_Remote::Client { m_apiVersion = apiVersion; m_backend = backend; ++m_generation; + // F1: published BEFORE the log line and before the hook, because a thread parked in + // RequireFirstSnapshot is waiting on exactly this store and has nothing else to read. + g_publishedGeneration.store(m_generation, ::std::memory_order_release); // R-12: RE-ARRIVAL IS THE INVALIDATION, and this is the one place that acts on it. // GLContext::GetCompileEnv() memoises on the raw pActiveBackendObject pointer @@ -130,10 +149,19 @@ namespace MobileGL::MG_Remote::Client { // in between. Folded through CapsCodec.h's helper rather than shifted here, because two // spellings of one bit layout is how the two sides come to disagree about it. // - // A PLACEHOLDER MIRROR CONSUMES NOTHING, and that is the safe direction: with no - // snapshot the mask is 0, every family answers "no consumer", the client emits nothing - // and the LEGACY PULL PATH runs untouched. The unsafe direction - emitting to a server - // that has no consumer - is the one that loses uploads. + // A PLACEHOLDER MIRROR ANSWERS NOTHING (F1, P7 wave 2). P5 wrote the opposite here - + // "a placeholder mirror consumes nothing, and that is the safe direction" - on the + // reasoning that withholding leaves the legacy pull path running. That reasoning is + // false on the client under split: there is no legacy pull path on this side of the + // wire, so withholding is not a fallback, it is a DROP. Measured: MG_State::Init() + // built GLContext's eleven default texture objects before MG_Backend::Init() had + // started the session, all eleven resource_creates were withheld against callMask=0 at + // caps generation 0, and the whole event was one WARN line in a log nobody diffs. + // + // The decision now waits for a real snapshot, or dies naming the family that asked. + if (m_generation == 0) { + RequireFirstSnapshot(subsystemBit); + } const Bool consumes = MGCapsServerConsumes(m_caps.CallMask, subsystemBit); if (!consumes) { ++g_consumerRefusals; @@ -151,6 +179,72 @@ namespace MobileGL::MG_Remote::Client { return consumes; } + void CapsMirror::RequireFirstSnapshot(Uint64 subsystemBit) const { + // THE SERVER ARM IS NOT A CLIENT WITH A LATE SNAPSHOT (D1c, PipeInputs.h). On the apply + // thread, and in a spawn server process, the CLIENT's caps mirror is not merely + // un-adopted - it is the wrong object to ask, and it will never be adopted, because + // there is no client half here to adopt it. That arm answers through + // RunsAsTheServerRole()'s monolith rows in WireTables.cpp and through the server + // session's own mask in PipeApply.cpp; the placeholder's "no consumer" is what keeps a + // server-role birth off a wire it has no business writing to, and it is correct there. + // Only the CLIENT may not decide a family's fate from a placeholder. + if (MG_Pipe::MGPipeServerArm()) return; + + // AND NEITHER IS A PROCESS THAT NEVER ASKED FOR A TRANSPORT. MG_Config::Transport can + // be assigned by hand - unit fixtures do it to put the code under test on its split + // arm while building no client at all (ServerLoopTest's main says so in as many words) + // - and holding those to a rule about a handshake they will never perform would be + // holding them to nothing. The gate is armed by MG_ConfigLoader::Init having RESOLVED a + // split transport, which is the same decision that makes MG_Backend::Init bring a + // client half up. See Config.h. + if (!MG_Config::SplitTransportRequestedByConfig) return; + + // The wait is real only when somebody else is fetching the snapshot. ClientSession's + // hook answers false when no bring-up is in flight and when the CALLING thread is the + // one performing it - both of which mean nothing can land while this call blocks. + if (g_firstSnapshotWait != nullptr && g_firstSnapshotWait(kFirstSnapshotWaitMs) && + m_generation != 0) { + return; + } + // NOT MGLOG_W AND CARRY ON. The whole defect this closes is that carrying on was + // indistinguishable, in every downstream artefact, from a server that genuinely + // consumes nothing: same absence of records, same cleared dirty flags, same green + // lane. A death names the family, publishes a SessionFault to a peer if one is + // connected, and lands in the census like every other one. + SessionFail(MGFatalFamily::CapsBeforeFirstSnapshot, + "MG_Remote client: Fatal{CapsBeforeFirstSnapshot, \"%s\"} - the liveness gate " + "for MGPipe subsystem 0x%llx asked a caps mirror that is still a PLACEHOLDER " + "(generation 0, callMask=0). Answering it would withhold that family's whole " + "record stream on a mask the server never published, and there is no legacy " + "pull path on the client to take over. The session must be started - and its " + "first CapsSnapshot adopted - before any frontend object of this family is " + "born; MobileGL::Initialize does that before MG_State::Init()", + CapsSubsystemName(subsystemBit), + static_cast(subsystemBit)); + } + + void SetCapsFirstSnapshotWait(CapsFirstSnapshotWait wait) { g_firstSnapshotWait = wait; } + + Uint64 PublishedCapsGeneration() { + return g_publishedGeneration.load(::std::memory_order_acquire); + } + + const char* CapsSubsystemName(Uint64 subsystemBit) { + switch (subsystemBit) { + case MG_Pipe::kMGPipeSubsystemResources: return "Resources"; + case MG_Pipe::kMGPipeSubsystemVertexInput: return "VertexInput"; + case MG_Pipe::kMGPipeSubsystemFramebuffer: return "Framebuffer"; + case MG_Pipe::kMGPipeSubsystemTextureResources: return "TextureResources"; + case MG_Pipe::kMGPipeSubsystemSamplers: return "Samplers"; + case MG_Pipe::kMGPipeSubsystemPrograms: return "Programs"; + case MG_Pipe::kMGPipeSubsystemBufferBindings: return "BufferBindings"; + default: break; + } + // A bit with no word yet, or a mask of several. Named by its value rather than by a + // guess: G14 lets the list grow, never lets a wrong word into a Fatal. + return "unnamed-subsystem"; + } + Uint64 ConsumerRefusals() { return g_consumerRefusals; } Uint64 LastRefusedSubsystem() { return g_lastRefusedSubsystem; } void ResetConsumerRefusalsForTest() { @@ -159,6 +253,12 @@ namespace MobileGL::MG_Remote::Client { g_refusedSubsystemsLogged = 0; } + void ResetCapsMirrorForTest() { + CapsMirrorInstance() = CapsMirror{}; + g_publishedGeneration.store(0, ::std::memory_order_release); + ResetConsumerRefusalsForTest(); + } + Bool CapsMirror::PrefersCpuXfbPrimitiveAccounting() const { return HasCap(MG_Pipe::kCapCpuXfbPrimitiveAccounting); } diff --git a/MobileGL/MG_Remote/Client/CapsMirror.h b/MobileGL/MG_Remote/Client/CapsMirror.h index f961cabe..b15f669f 100644 --- a/MobileGL/MG_Remote/Client/CapsMirror.h +++ b/MobileGL/MG_Remote/Client/CapsMirror.h @@ -75,6 +75,8 @@ namespace MobileGL::MG_Remote::Client { // of the answer on the client under split: MGPipeGetResourceOps() is the SERVER's // registration and is null in the client process, which would silently disable the // whole push path in the one mode that matters. + // + // F1 (P7 wave 2): IT NEVER ANSWERS FROM A PLACEHOLDER ANY MORE. See RequireFirstSnapshot. Bool ServerConsumes(Uint64 subsystemBit) const; // GLFunctionsTable::PrefersCpuXfbPrimitiveAccounting (BackendObject.h:274) does NOT @@ -84,6 +86,27 @@ namespace MobileGL::MG_Remote::Client { Bool PrefersCpuXfbPrimitiveAccounting() const; private: + // F1 (P7 wave 2). THE PLACEHOLDER MAY NOT ANSWER "THIS FAMILY HAS NO CONSUMER". + // + // The placeholder rule above is right for the READ accessors - one imprecise startup + // log line is cheaper than restructuring MG_Backend::Init() - and it was catastrophic + // for this one, which is not a read but a DECISION: answering it from a zeroed mask + // says "the server consumes nothing, emit nothing, run the legacy pull path", and + // under split there IS no legacy pull path on the client. Every record the caller + // would have emitted is simply lost, the emitter clears its dirty flags on the + // acceptance it never asked for, and the one R-8 warning that says so is emitted once + // per family for the life of the process. + // + // So: generation 0 is not an answer. This waits, bounded, for a handshake that is + // genuinely in flight on ANOTHER thread, and otherwise dies by name - + // Fatal{CapsBeforeFirstSnapshot, ""} - naming the family that asked. It does + // NOT wait when the calling thread is the one performing the handshake: that is the + // shape MobileGL::Initialize used to produce (MG_State::Init() before + // MG_Backend::Init()), and waiting for a snapshot this thread has not yet gone to + // fetch is how a deadlock is spelled. The ORDER is the fix; this is the gate that + // makes a regression of it loud instead of silent. + void RequireFirstSnapshot(Uint64 subsystemBit) const; + MG_Pipe::MGPCaps m_caps{}; MG_Backend::FormatCapabilityCache m_formats{}; RendererInfo m_renderer{}; @@ -121,4 +144,42 @@ namespace MobileGL::MG_Remote::Client { Uint64 LastRefusedSubsystem(); void ResetConsumerRefusalsForTest(); + // F1's cases need the ONE state a running process can never get back: a mirror that has + // adopted nothing. A fork child inherits its parent's adopted instance, so without this a + // case about the placeholder would be a case about whatever the previous case adopted. + // Test-only by name, like ResetConsumerRefusalsForTest above it; nothing in the library + // calls either. + void ResetCapsMirrorForTest(); + + // ---- F1 (P7 wave 2) -------------------------------------------------------------------- + // + // THE BOUNDED WAIT'S ONE HONEST SHAPE. ServerConsumes cannot pump a control plane itself - + // CapsMirror deliberately knows nothing about ClientSession - so the session installs the + // wait and the mirror calls it. The contract is narrow on purpose: + // + // * it returns FALSE IMMEDIATELY when no session bring-up is in flight, or when the + // CALLING thread is the one performing it. Both mean the snapshot cannot arrive while + // this call blocks, and a wait there would burn the budget and then die anyway; + // * otherwise it waits up to `timeoutMs` for the first Adopt() and reports whether one + // landed. + // + // Installed once, by ClientSession's constructor. A raw function pointer for ID-8's + // reason, like SetCapsAdoptedHook beside it. + using CapsFirstSnapshotWait = Bool (*)(Uint32 timeoutMs); + void SetCapsFirstSnapshotWait(CapsFirstSnapshotWait wait); + + // The generation, readable from a thread that does not own the mirror. m_generation is + // GL-thread state by the same argument that makes one gPipeInputs legal; this is the one + // fact a WAITING thread has to see, so it is published separately and atomically. + Uint64 PublishedCapsGeneration(); + + // `kMGPipeSubsystem*` -> the word the Fatal and the R-8 warning print. Names only grow + // (G14); an unknown bit prints as a hex literal rather than as a guess. + const char* CapsSubsystemName(Uint64 subsystemBit); + + // How long RequireFirstSnapshot gives a handshake that IS in flight. Bounded and small: + // an in-process Adopt lands in microseconds and a socket one in single-digit milliseconds, + // so anything past this is a peer that is not coming. + inline constexpr Uint32 kFirstSnapshotWaitMs = 250; + } // namespace MobileGL::MG_Remote::Client diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index 144b65e3..81154114 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -34,8 +34,10 @@ #include #include +#include #include #include +#include #include #include @@ -87,6 +89,34 @@ namespace MobileGL::MG_Remote::Client { // deadline there turns a lost record into a hung process exit. constexpr Uint32 kDrainTimeoutMs = 5000; + // ---- F1 (P7 wave 2): WHO IS FETCHING THE FIRST SNAPSHOT -------------------------- + // + // CapsMirror::RequireFirstSnapshot needs one fact this file owns and no other does: + // is a bring-up in flight, and is it on THIS thread. The counter answers the first; + // the thread-local answers the second, and it is the one that matters, because the + // defect F1 closes had the emitter and the handshake on the same thread in sequence - + // a wait there would burn its whole budget and change nothing. + // + // Not guarded by a mutex: the counter is only ever compared against zero and the + // thread-local is private to its thread by construction. + ::std::atomic g_bringUpsInFlight{0}; + thread_local Bool tl_bringingUpHere = false; + + struct BringUpScope { + BringUpScope() { + g_bringUpsInFlight.fetch_add(1, ::std::memory_order_acq_rel); + m_outer = tl_bringingUpHere; + tl_bringingUpHere = true; + } + ~BringUpScope() { + tl_bringingUpHere = m_outer; + g_bringUpsInFlight.fetch_sub(1, ::std::memory_order_acq_rel); + } + BringUpScope(const BringUpScope&) = delete; + BringUpScope& operator=(const BringUpScope&) = delete; + Bool m_outer = false; + }; + MobileGLResult ReceiveEnvelope(Transport::ITransport& transport, std::vector& out, Uint32 timeoutMs) { Uint64 size = 0; @@ -500,9 +530,44 @@ namespace MobileGL::MG_Remote::Client { return *instance; } + namespace { + // F1 (P7 wave 2). The wait CapsMirror::RequireFirstSnapshot calls. Its whole job is to + // distinguish "the snapshot is on its way and somebody else is fetching it" from "no + // snapshot can arrive while you block", and to answer the second immediately. + // + // IT DOES NOT PUMP. Pumping a half-built session from a thread that does not own it is + // how two readers end up splitting one control frame; the thread that IS bringing the + // session up already pumps, in FinishStartup step 7, and this one only has to see the + // generation it publishes. + Bool AwaitFirstCapsSnapshot(Uint32 timeoutMs) { + if (PublishedCapsGeneration() != 0) return true; + // The two immediate falses. Nothing is fetching, or the fetcher is the caller - + // which is the exact shape MobileGL::Initialize produced before F1 moved the + // split bring-up above MG_State::Init(). + if (tl_bringingUpHere) return false; + if (g_bringUpsInFlight.load(::std::memory_order_acquire) == 0) return false; + const auto deadline = + std::chrono::steady_clock::now() + std::chrono::milliseconds(timeoutMs); + while (std::chrono::steady_clock::now() < deadline) { + if (PublishedCapsGeneration() != 0) return true; + if (g_bringUpsInFlight.load(::std::memory_order_acquire) == 0) break; + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } + return PublishedCapsGeneration() != 0; + } + } // namespace + + Bool BringUpInFlight() { + return g_bringUpsInFlight.load(::std::memory_order_acquire) != 0; + } + ClientSession::ClientSession() { m_link = Transport::CreateSharedLink(Transport::TransportRoleTag::ClientProducer); m_cmd = &m_link->CommandsOut(); m_events = &m_link->EventsIn(); + // F1: installed from the constructor rather than from Start, because the question it + // answers is asked by frontend object births that may precede any Start at all - and + // the honest answer to those is the Fatal, not a silent false. + SetCapsFirstSnapshotWait(&AwaitFirstCapsSnapshot); } ClientSession::~ClientSession() { @@ -514,6 +579,12 @@ namespace MobileGL::MG_Remote::Client { Bool ClientSession::Started() const { return m_started; } MobileGLResult ClientSession::Start(MG_Config::TransportMode mode, const String& endpoint) { + // F1: the bring-up window opens HERE and not at FinishStartup, because under inproc the + // server's first CapsSnapshot is produced inside ServerSession::Accept - step 3 below - + // and a window that started after it would not contain the one moment a reader on + // another thread has to be able to wait through. Re-entrant: StartOverTransportPair + // opens a nested scope and BringUpScope nests. + const BringUpScope bringUp; if (m_started) { return MOBILEGL_ERR_INVALID_ARGUMENT; } @@ -540,6 +611,7 @@ namespace MobileGL::MG_Remote::Client { MobileGLResult ClientSession::StartOverTransportPair( std::unique_ptr clientEnd, std::unique_ptr serverEnd) { + const BringUpScope bringUp; // F1, and see Start() for why it opens this early. if (m_started) { return MOBILEGL_ERR_INVALID_ARGUMENT; } @@ -687,6 +759,7 @@ namespace MobileGL::MG_Remote::Client { } MobileGLResult ClientSession::StartSpawned() { + const BringUpScope bringUp; // F1, and see Start() for why it opens this early. if (m_started) { return MOBILEGL_ERR_INVALID_ARGUMENT; } @@ -771,6 +844,7 @@ namespace MobileGL::MG_Remote::Client { MobileGLResult ClientSession::StartOverSocket( std::unique_ptr transport) { + const BringUpScope bringUp; // F1, and see Start() for why it opens this early. if (m_started) { return MOBILEGL_ERR_INVALID_ARGUMENT; } @@ -1218,6 +1292,10 @@ namespace MobileGL::MG_Remote::Client { MobileGLResult ClientSession::FinishStartup(Transport::Doorbell* peerBell, Transport::Doorbell* selfBell, bool startApplyThreadHere) { + // F1: every startup path - inproc, spawn, unix, tcp - reaches step 7 through here, so + // this is the one place that has to declare "a first snapshot is being fetched, on + // this thread". CapsMirror reads both halves of that (see AwaitFirstCapsSnapshot). + const BringUpScope bringUp; m_link->InitializeEndpoints(); // NO STAGE RING. SEG_STAGE is package w1's encoder-local LINEAR ALLOCATOR: // a staged byte run carries no RingRecordHeader, nothing consumes SEG_STAGE, diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index 899b627c..edb5e51d 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -474,4 +474,13 @@ namespace MobileGL::MG_Remote::Client { // tidy teardown. MG_Backend::Init() calls Start() on this one. ClientSession& ClientSessionInstance(); + // F1 (P7 wave 2). Is a session bring-up running anywhere in this process right now. + // + // CapsMirror asks a narrower question through its own hook (SetCapsFirstSnapshotWait) and + // this is the half a TEST needs: a case that wants to read the mirror DURING a handshake + // has to know when the window has opened, and inferring it from a log line or a sleep is + // how such a case starts passing for the wrong reason. True from the first line of Start / + // StartSpawned / StartOverSocket / StartOverTransportPair until FinishStartup returns. + Bool BringUpInFlight(); + } // namespace MobileGL::MG_Remote::Client diff --git a/MobileGL/MG_Remote/FatalFamilies.def b/MobileGL/MG_Remote/FatalFamilies.def index 7ef2b19f..fc41ae4f 100644 --- a/MobileGL/MG_Remote/FatalFamilies.def +++ b/MobileGL/MG_Remote/FatalFamilies.def @@ -67,6 +67,7 @@ X(UnmigratedEmulation, ProtocolCorruption, "an emulation path the applier has no migrated arm for was reached") \ X(RoleViolation, ProtocolCorruption, "a call ran on the wrong role/thread - an invariant the two roles must hold") \ X(UnsetCallMask, ProtocolCorruption, "the CallMask was consulted before either half was published") \ + X(CapsBeforeFirstSnapshot, ProtocolCorruption, "a record family's liveness was asked of a placeholder caps mirror (F1)") \ X(ConsumerMaskLie, ProtocolCorruption, "the server published a consumer bit for a family whose op table is absent") \ X(ClientTablesUninstalled, ProtocolCorruption, "the applier ran before the client installed its tables") \ X(NoClientSession, ProtocolCorruption, "a path that needs the client session ran without one") \ diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index 7a8cc8b5..0350d3c8 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/MobileGL/MG_Remote/Server/ServerSession.cpp @@ -24,8 +24,10 @@ #include #include +#include #include #include +#include #include // CONTRACT-P6 4.3: the one value in the handshake that a same-process session cannot fake. @@ -38,6 +40,42 @@ namespace MobileGL::MG_Remote::Server { namespace { + // F1 (P7 wave 2). THE ONLY KNOB THAT CAN MAKE THE FIRST CapsSnapshot LATE. + // + // MOBILEGL_TEST_DELAY_FIRST_CAPS_MS delays the server's FIRST publication, and only + // it - a re-send (R-12) is never delayed, because the question is about the window + // before the client has any mask at all. Read once, on the first Accept, so a lane + // that does not set it pays one getenv per session and nothing else. + // + // WHAT IT PROVES, AND WHERE. Under spawn / unix: / tcp:// the publication happens in + // the SERVER's process, so the delay is a genuinely late snapshot from the client's + // point of view and it exercises the client's step-7 wait (ClientSession.cpp:1284). + // Under inproc there is no second process: Accept runs inside ClientSession::Start on + // the client's own thread, so the delay simply makes Start slower - which is itself + // the proof that under inproc the first snapshot CANNOT be late relative to session + // start, and therefore that the only window in which the client could read a + // placeholder is the one BEFORE Start is called. That window is F1's defect, and + // MobileGL::Initialize closes it by ordering rather than by waiting. + // + // It is a TEST knob: it exists only in a disaggregated build and does nothing at all + // unless it is set, so no production path can reach a delay it did not ask for. + void DelayFirstCapsSnapshotForTest() { +#if MOBILEGL_BUILD_DISAGGREGATED + static Bool spent = false; + if (spent) return; + spent = true; + const char* text = std::getenv("MOBILEGL_TEST_DELAY_FIRST_CAPS_MS"); + if (text == nullptr || *text == '\0') return; + const long ms = std::strtol(text, nullptr, 10); + if (ms <= 0) return; + MGLOG_W("MG_Remote server: MOBILEGL_TEST_DELAY_FIRST_CAPS_MS=%ld - the FIRST " + "CapsSnapshot is held back by that many milliseconds. This is F1's " + "test-only lever and must never be set in a measured run", + ms); + std::this_thread::sleep_for(std::chrono::milliseconds(ms)); +#endif + } + // File-local on purpose: two callers, both handshake facts. A process-id helper on a // public header invites uses that are not. std::uint32_t SelfProcessId() { @@ -615,6 +653,7 @@ namespace MobileGL::MG_Remote::Server { // ---- 6. the first CapsSnapshot, if there is a backend to take it from. if (m_backend != nullptr) { + DelayFirstCapsSnapshotForTest(); const MobileGLResult published = PublishCapsSnapshot(); if (published != MOBILEGL_OK) { return published; diff --git a/scripts/ci/fatal_census_baseline.json b/scripts/ci/fatal_census_baseline.json index b358b76a..fcd7e00e 100644 --- a/scripts/ci/fatal_census_baseline.json +++ b/scripts/ci/fatal_census_baseline.json @@ -30,6 +30,7 @@ "BarrierTimeout", "BarrierViolation", "BlobMissing", + "CapsBeforeFirstSnapshot", "ClientTablesUninstalled", "ConsumerMaskLie", "EventRingOverflow",