diff --git a/MobileGL/MG_Remote/CapsCodec.cpp b/MobileGL/MG_Remote/CapsCodec.cpp index 322ee6d0..28d86678 100644 --- a/MobileGL/MG_Remote/CapsCodec.cpp +++ b/MobileGL/MG_Remote/CapsCodec.cpp @@ -6,31 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P5 package w1: MGPCaps's two blob serializers, the pair MGPipeTypes.h:134-136 defers to -// this phase by name ("Their serializers land with the transport (P5)"). -// -// THE FORMAT, and every part of it is a refusal rather than a guess. It is -// ProgramArtifactsCodec's shape on purpose - that codec is the tree's one worked example of a -// wire format that has survived a real transport, so copying it is cheaper than being -// original and much cheaper than being wrong: -// -// * a VERSION word first, and the two TABLE DIMENSIONS second, so a peer whose -// TextureInternalFormat enum grew is a mismatch AT READ TIME rather than a silent shear -// that reads one format's capabilities as another's; -// * length-prefixed everything, with the count checked against the bytes that REMAIN before -// a single element is reserved, so a corrupt count cannot become a four-billion-element -// resize; -// * SPARSE for all three capability tables. The dense form is -// 2 * targets * formats * 8 bytes plus the sample-count lists - with 13 targets and 77 -// internal formats that is ~16 KiB of mostly zeroes, PER CONTEXT AND PER CAPS -// INVALIDATION, because R-12 makes a re-arriving snapshot the invalidation rather than a -// once-per-process cost. The tables are overwhelmingly empty, so what crosses is -// (index, value) pairs and the decoder Clear()s first; -// * little-endian by memcpy of fixed-width scalars, which is what every other MobileGL wire -// struct already assumes and what the ABI fingerprint below makes checkable; -// * every decoder returns FALSE on truncation, a bad version, a dimension mismatch, an -// out-of-range index or TRAILING BYTES THE FORMAT DOES NOT ACCOUNT FOR. These bytes -// arrive over a wire and the outputs are left in a defined, default state on a refusal. +// MGPCaps's two blob serializers: the format capability cache and the renderer info. Each rule +// of the format refuses rather than guesses. A version word and the two table dimensions come +// first, so a peer whose format enum grew fails at read time instead of shearing one format's +// caps onto another. Every count is length-prefixed and checked against the remaining bytes +// before anything is reserved. The three capability tables cross sparse as (index, value) pairs: +// dense they are ~16 KiB of mostly zeroes, resent with every snapshot. Scalars are little-endian +// by memcpy. Decoders return false on truncation, a bad version or dimension, an out-of-range +// index or trailing bytes, and leave the outputs in a default state. #include "CapsCodec.h" diff --git a/MobileGL/MG_Remote/CapsCodec.h b/MobileGL/MG_Remote/CapsCodec.h index 3c687a8a..26e91414 100644 --- a/MobileGL/MG_Remote/CapsCodec.h +++ b/MobileGL/MG_Remote/CapsCodec.h @@ -32,33 +32,14 @@ namespace MobileGL::MG_Remote { - // ---- CallMask's layout (c0's ruling, extending R-8) --------------------------------- - // - // R-8 requires the client's liveness gates - MGPipeResourceSubsystemEnabled() and - // P4aFamilyHasItsConsumer() - to answer from MGPCaps::CallMask instead of from - // MGPipeGetResourceOps(), because that op table is the SERVER's registration: under - // inproc a client reading it is right by accident, and under spawn it is null and five - // whole record families silently emit nothing. - // - // But CallMask as declared carries only the nine MGPCapBit FEATURE bits - // (MGPipeTypes.h:108-124) and has no per-family bit at all, so "read the CallMask" was - // not yet an implementable instruction. It is now: - // - // bits 0..8 MGPCapBit, unchanged. kCapViewportArray (0), kCapNeedsHostIndexBytes (7) - // and kCapNeedsHostUboBytes (8) are RESERVED (P8-F): never published, and - // their positions are never reused, which keeps every MGHostSpan off the wire. - // bits 9..31 reserved for further MGPCapBits. - // bits 32..47 THE CONSUMER MASK: bit (32 + n) means "the server has a consumer for - // MGPipe subsystem bit n" - i.e. the server's own subsystem mask, shifted. - // Sixteen bits covers bits 0..12 allocated through P4a with room to P8. + // ---- CallMask's layout --------------------------------------------------------------- + // The client's consumer gates (MGPipeResourceSubsystemEnabled, P4aFamilyHasItsConsumer) + // read CallMask, never MGPipeGetResourceOps(): that op table is the server's registration. + // bits 0..31 MGPCapBit. Reserved bits (kCapViewportArray, kCapNeedsHost*Bytes) are + // never published and their positions are never reused. + // bits 32..47 consumer mask: bit (32 + n) = the server consumes MGPipe subsystem bit n. // bits 48..63 reserved. - // - // protocol.fbs:94's `tableSlotMask: ulong` is DELETED rather than renamed (R-8 offered - // either). Two reasons, and the second is decisive: ARCHITECTURE.md §3.3 says CallMask - // REPLACES "is this table slot null" as the capability probe, so a field whose comment is - // "which GLFunctionsTable slots the peer registered" re-introduces exactly what it - // replaced; and GLFunctionsTable has SIXTY-NINE function-pointer slots - // (BackendObject.h:117-292), so a 64-bit mask cannot address it and never could. + // There is no table-slot mask: CallMask replaces the null-slot probe (ARCHITECTURE.md §3.3). inline constexpr Uint32 kMGCapsConsumerBitShift = 32; inline constexpr Uint64 kMGCapsConsumerMask = 0xFFFFull << kMGCapsConsumerBitShift; diff --git a/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp b/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp index 14403a47..dae3fbb7 100644 --- a/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp +++ b/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp @@ -33,30 +33,12 @@ namespace MobileGL::MG_Remote::Client { // ---- the EGL seam ------------------------------------------------------------------ // - // THE EGL VIRTUALS CALL v1's TWELVE FORWARDERS AND NOTHING ELSE (the swap interval's - // since P10 through the emit table's slot, not an override here). c1 round 1 built - // its own trampolines over ServerLoop's control channel and ServerLoop::Backend(), - // which ran the right driver call on the right thread and was still wrong, because - // three of the twelve do MORE than forward: - // - // ServerMakeEGLCurrent re-publishes the caps snapshot after the server's own - // InitCapabilities has run (R-12 arm (a)) - // ServerInitCapabilities the same, on the explicit path - // ServerSetWindowHandle hands the surface to the server's backend - // - // Calling `Backend()->MakeEGLCurrent(...)` skips the republish, so the client's mirror - // keeps the snapshot Accept() sent BEFORE any context existed - every limit, every - // advertised extension and the compile-env fingerprint read off an empty backend, with - // InitCapabilities below happily reporting success because a snapshot did arrive once. - // That is the failure this seam exists to make impossible: there is no second route to - // the server's EGL, so there is no route that can skip what the forwarder does. - // - // The forwarders pack a SurfaceControlFrame and block on mgl-srv-apply themselves (P5f - // fc; the channel was a function-pointer mailbox before that) and run INLINE when the - // caller is already on that thread, so this file needs no frame, no per-call args - // struct, and no null-backend check of its own - the dispatch's null-backend arm is the - // one place that answers "the bring-up did not complete", and it answers `false` rather - // than crashing or succeeding locally. + // The EGL virtuals call the Server* forwarders (ServerLoop.h) and nothing else, because + // some do more than forward: ServerMakeEGLCurrent and ServerInitCapabilities republish + // the caps snapshot after the server's InitCapabilities, and ServerSetWindowHandle hands + // the surface to the server's backend; calling the backend directly would leave the caps + // mirror on the pre-context snapshot. The forwarders block on mgl-srv-apply (inline when + // already on it) and answer false when the bring-up did not complete. // CapsMirror's adoption hook. A free function because the hook is a raw function // pointer (ID-8: this can fire on a path that must not allocate), and it reaches the diff --git a/MobileGL/MG_Remote/Client/BackendObject_Remote.h b/MobileGL/MG_Remote/Client/BackendObject_Remote.h index abf7e137..18d82409 100644 --- a/MobileGL/MG_Remote/Client/BackendObject_Remote.h +++ b/MobileGL/MG_Remote/Client/BackendObject_Remote.h @@ -6,45 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// THE CLIENT ROLE'S BackendObject. Owner: package c1. -// -// MG_Backend::Init() installs one of these into pActiveBackendObject when the transport -// resolved (the hook is v1's; the object is c1's, table 3). It is NOT a backend: it owns no -// context, no driver and no GL state. It is the frontend's single answer to four questions - -// "what can the device do", "what backend is it", "what table do I call", and the nine EGL -// lifecycle calls - and each of the four is answered somewhere that is not here. -// -// THE THREE TRAPS THE SCOUT FOUND, AND WHERE EACH IS PAID: -// -// 1. GetRendererInfo() RETURNS A REFERENCE (BackendObject.h:590), so the object cannot -// synthesise one per call. CapsMirror owns the storage; this class forwards. And -// LogBackendInfo() reads it at MG_Backend/Init.cpp:21, DURING MG_Backend::Init(), before -// any surface exists - so the mirror answers a placeholder and P5 accepts one imprecise -// startup log line. MG_Backend::Init() is NOT restructured (scout-caps-reply §1.2 (a)). -// -// 2. GetFormatCapabilities() IS NOT VIRTUAL (BackendObject.h:594). It returns the base class's -// own m_formatCapabilities member, so there is no accessor to override: the cache has to be -// PUSHED into that member, and the only moment this object can know to is when a snapshot -// lands. CapsMirror's adoption hook is that moment. -// -// 3. GetBackendType() MUST ANSWER THE SERVER'S BACKEND. There is no "Remote" enumerator and -// there must not be one: GL_Framebuffer.cpp:47, GL_Texture.cpp:6536 and CompileEnv.cpp:122 -// switch on this value, and a value they do not know takes a WRONG ARM rather than failing. -// -// THE EGL VIRTUALS ARE BOTH FORWARDED AND KEPT. The base class runs a real state machine - -// surface registration, per-thread current-context bookkeeping, the lazy InitCapabilities latch, -// and SwapEGLBuffers' route into GetBackendFunctions().Present() - and the client needs all of -// it, because Present is a class-B emitter reached through exactly that route (the verb census's -// trap 3: Present has zero MG_Impl call sites). So each override does BOTH: it runs the real -// EGL work on the apply thread, through ServerLoop's control-frame channel (P5f fc; v1's -// function-pointer mailbox before it), and then lets the -// base class keep the client-side books. -// -// SetEGLSwapInterval IS NOT OVERRIDDEN (P10 B). It used to be, because the base implementation -// calls GetBackendFunctions().SetSwapInterval (BackendObject.cpp:402) and that slot was the last -// class-C Fatal in the emit table. The slot is now the forwarder the override called -// (Server::ServerSetEGLSwapInterval, EmitTables.cpp), so the base's route IS the forward and a -// second spelling of it here would only be a second place for the two to drift apart. +// The client role's BackendObject: no context, driver or GL state. Caps come from CapsMirror, +// the function table is the remote emit table, and EGL work is forwarded to the server. +// Trap 1: GetRendererInfo() returns a reference, so CapsMirror owns it (a placeholder until the +// first snapshot). Trap 2: GetFormatCapabilities() is not virtual, so each adopted snapshot is +// pushed into m_formatCapabilities. Trap 3: GetBackendType() answers the server's backend; there +// is no Remote value, because callers switch on it and an unknown value takes a wrong arm. +// Each EGL override runs the real work on the server, then lets the base class keep the client's +// books (so SwapEGLBuffers reaches Present). SetEGLSwapInterval's table slot already forwards. #pragma once #include diff --git a/MobileGL/MG_Remote/Client/CapsMirror.cpp b/MobileGL/MG_Remote/Client/CapsMirror.cpp index 72bd14fe..cb3c3bc4 100644 --- a/MobileGL/MG_Remote/Client/CapsMirror.cpp +++ b/MobileGL/MG_Remote/Client/CapsMirror.cpp @@ -6,27 +6,12 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P5 package c1. -// -// EVERY ACCESSOR ANSWERS BEFORE THE FIRST SNAPSHOT, AND THAT IS THE RULING, NOT A WEAKENING. -// c0's stubs aborted on all but Valid()/Generation() so that nothing could answer from a zeroed -// mirror by accident. But scout-caps-reply §1.2's option (a) - the one the brief adopts - says -// LogBackendInfo() reads GetRendererInfo() at MG_Backend/Init.cpp:21, during MG_Backend::Init(), -// BEFORE any surface exists, and that P5 accepts one imprecise startup log line rather than -// restructure MG_Backend::Init(). An abort there is not a stricter mirror; it is a process that -// cannot start. So the mirror answers a placeholder and SAYS SO, once per accessor: a wrong -// number that announced itself is auditable, and the announcement is what a case asserts on. -// -// 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. +// The read accessors answer before the first snapshot instead of aborting, so an early read +// such as LogBackendInfo's startup logging cannot keep the process from starting. They return +// the default-constructed members, which Adopt() replaces in place (the accessors return +// references, so the storage outlives every caller), and a read of the placeholder logs a +// warning. ServerConsumes() is the exception and never answers from a placeholder; see +// RequireFirstSnapshot. #include "CapsMirror.h" @@ -175,23 +160,12 @@ namespace MobileGL::MG_Remote::Client { } Bool CapsMirror::ServerConsumes(Uint64 subsystemBit) const { - // THE ONLY LEGAL CLIENT-SIDE SPELLING (R-8). Never MGPipeGetResourceOps(): that is the - // SERVER's registration, a process-wide global, right by accident under inproc and null - // under spawn - and a null read there silently stops five record families while the - // client goes on clearing its dirty flags, which is ID-39's 66 lost uploads with a wire - // 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 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. + // The only client-side source of this answer. MGPipeGetResourceOps() is the server's + // registration: right by accident inproc and null under spawn, where reading it would + // silently stop whole record families while the client clears its dirty flags. The bit + // layout is read through CapsCodec.h's helper so both sides share one spelling. A + // placeholder mirror does not answer: the client has no pull path, so withholding would + // drop records. The decision waits for a real snapshot or fails naming the family. if (m_generation == 0) { RequireFirstSnapshot(subsystemBit); } diff --git a/MobileGL/MG_Remote/Client/CapsMirror.h b/MobileGL/MG_Remote/Client/CapsMirror.h index b15f669f..b6583618 100644 --- a/MobileGL/MG_Remote/Client/CapsMirror.h +++ b/MobileGL/MG_Remote/Client/CapsMirror.h @@ -6,34 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// The client's copy of the server's capabilities. Owner: package c1. Signatures by c0. -// -// WHY A MIRROR AND NOT A ROUND TRIP. There are 56 client-side caps read points -// (40 GetDynamicParameters + 7 GetRendererInfo + 4 GetFormatCapabilities + 3 GetBackendType + -// 2 GetBackendAPIVersionString), and several of them - GL_Getter.cpp:2400 and -// ShaderTranspiler/CompileEnv.cpp:120-124 - bind a reference and then read many members, so a -// partial snapshot is not an option. Every one of the 56 must be answerable locally, with no -// record on the wire. -// -// GetRendererInfo() RETURNS A REFERENCE (BackendObject.h:590), so the mirror must OWN a -// RendererInfo instance to hand back - including before the first snapshot arrives, because -// LogBackendInfo() reads it at MG_Backend/Init.cpp:21, during MG_Backend::Init(), long before -// any context exists. Ruling (scout-caps-reply §1.2 option (a)): the mirror answers with a -// placeholder until the first snapshot, P5 accepts one inaccurate startup log line, and -// MG_Backend::Init() is NOT restructured. -// -// GetFormatCapabilities() is NON-VIRTUAL (BackendObject.h:594), so a remote backend object -// cannot override the accessor: it must FILL BackendObject::m_formatCapabilities from this -// mirror instead. -// -// INVALIDATION IS RE-ARRIVAL (R-12). DirectGLES has no OnCapsInvalidated producer at all - it -// re-runs UpdateAdvertisedCapabilityExtensions + UpdateDynamicBackendParameters at -// BackendObject_DirectGLES.cpp:865-871 and tells the frontend nothing, which is correct in -// monolith and a silent bug under split. Rather than add a DirectGLES-side callback (a -// dev-shaped backend edit), the SERVER re-sends the whole snapshot on every InitCapabilities -// re-run and the CLIENT treats a second arrival as the invalidation. Generation() is what a -// client-side memo keys on, and it is also the re-open signal for the server-context-death -// case that MGPipeCallbacks has no eleventh slot for (MGPipeCallbacks.h:56-58). +// The client's copy of the server's capabilities. Every client-side caps read is answered +// locally, with no record on the wire, and several readers bind a reference and read many +// members, so the mirror holds the whole snapshot. The accessors return references, so the +// mirror owns the storage; before the first snapshot it answers a placeholder rather than abort. +// GetFormatCapabilities() is non-virtual, so the remote backend fills +// BackendObject::m_formatCapabilities from here (SetCapsAdoptedHook). +// Invalidation is re-arrival: the server re-sends the whole snapshot on every InitCapabilities +// run, and Generation() is what a client-side memo keys on. #pragma once #include @@ -86,25 +66,13 @@ 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. + // ServerConsumes() decides whether a record family reaches the wire, so it never answers + // from the placeholder: a zero mask would drop the family's records while the emitter + // clears their dirty flags, and the client has no pull path to fall back to. This waits, + // bounded by kFirstSnapshotWaitMs, for a handshake in flight on another thread, else it is + // Fatal{CapsBeforeFirstSnapshot, ""}. It never waits on the thread running the + // handshake, which would deadlock; MobileGL::Initialize brings the split roles up before + // MG_State::Init() so no frontend object asks that early. void RequireFirstSnapshot(Uint64 subsystemBit) const; MG_Pipe::MGPCaps m_caps{}; diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index a8513f83..1d58c0c0 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -167,22 +167,13 @@ namespace MobileGL::MG_Remote::Client { ::std::atomic g_bringUpsInFlight{0}; thread_local Bool tl_bringingUpHere = false; - // ---- P7 CI: THE COLD-START BUDGET OF A SURFACE-CONTROL REPLY ----------------------- + // ---- cold-start budget of a surface-control reply ---------------------------------- // - // A spawned (or tcp) server brings its NATIVE backend up lazily, INSIDE the first - // surface creation (Espryt: eglInitialize under CreatePbufferSurface/CreateWindowSurface) - // or the first MakeCurrent (Magma: the Vulkan instance and device) - ServerLoop.cpp's - // ApplySurfaceControlFrame. On a workstation that is ~100 ms; on a loaded CI runner it - // took longer than the 5 s steady-state bound, and the retrace-split spawn legs died on - // "no SurfaceReply for CreatePbufferSurface seq 2" / "for MakeCurrent seq 3" with the - // server still alive and about to answer (11 of 77 legs in run 35671704873, 17 of 77 in - // 35706183230, a different set each run). The server was never wedged; the client simply - // asked a cold software rasteriser for a warm one's latency. - // - // So until the session's first MakeCurrent is answered ok, those three ops wait the - // cold-start budget; every other op, and all three once the backend is warm, keep the - // steady bound. The budget is never shorter than the steady bound, and its expiry is - // the same non-fatal, NAMED answer (D5b) - a longer wait, not a different contract. + // The server brings its native backend up lazily inside the first surface creation + // (Espryt's eglInitialize) or the first MakeCurrent (Magma's Vulkan instance and device; + // ServerLoop.cpp's ApplySurfaceControlFrame). On a loaded CI runner that outlasts the + // steady bound, so until a MakeCurrent is answered ok these three ops wait + // max(steady, ColdStartMs). Expiry is the same non-fatal named answer as any timeout. Bool MayBringTheBackendUp(Server::SurfaceControlOp kind) { return kind == Server::SurfaceControlOp::CreatePbufferSurface || kind == Server::SurfaceControlOp::CreateWindowSurface || @@ -393,25 +384,13 @@ namespace MobileGL::MG_Remote::Client { return "?"; } - // ---- c1: the reverse channel's reading end --------------------------------------- + // ---- the reverse channel's reading end ------------------------------------------- // - // R-12's three: OnBufferWriteback (#3), OnGpuWritten (#2), OnSurfaceChanged (#7). - // Drained BY THE GL THREAD BETWEEN VERBS, which under the barrier means immediately - // after appliedSeq reaches this record - the one moment at which the apply thread is - // known not to be inside the applier. - // - // THE BYTES LIVE IN THE RING ITSELF, so Drained() is called only after every payload - // pointer popped here has been consumed: retiring earlier is R-11's violation one level - // down (EventRing.h:168-171 says so in as many words). - // - // THE CONSUMERS ARE CALLED BY NAME, NEVER THROUGH gMGPipeCallbacks (CONTRACT-P5C - // §4.1): with an active transport the global table's reverse entries are the SERVER - // session's producer callbacks, so routing the drain through it would call the - // producer back from the GL thread - the layer-2 violation the contract names. - // - // THE SURFACE-CHANGED CONSUMER IS WHERE R3's OWNERSHIP ALWAYS WAS: the backend - // posts the MGPSurfaceInfo, and the allocate/format writes against - // pDefaultFramebufferInfo happen HERE, on the GL thread, against client memory. + // The GL thread drains SEG_EVENT between verbs and inside client waits. The payloads live + // in the ring, so Drained() runs only after every popped payload has been consumed. + // Consumers are called by name, never through gMGPipeCallbacks: with an active transport + // its reverse entries are the server session's producer callbacks. The surface-changed + // consumer applies the server's MGPSurfaceInfo to pDefaultFramebufferInfo in client memory. void ApplySurfaceChangedToClient(const MG_Pipe::MGPSurfaceInfo& info) { auto& defaultFBOInfo = MG_Impl::GLImpl::FramebufferImpl::pDefaultFramebufferInfo; if (!defaultFBOInfo) return; @@ -1955,25 +1934,14 @@ namespace MobileGL::MG_Remote::Client { } } - // ---- 9. AND ONLY NOW THE THIRTY-SEVEN WIRE EMITTERS (R-17). This is the line that - // arms `integration-split`: the 21 `DirectGLES.Split.*` entries skip on - // `ClientSession::Active() == nullptr`, and every one of the four arming facts is true - // at exactly this point and at no earlier one. - // - // IT IS LAST, AND EACH OF THE FOUR REASONS IS A DIFFERENT FAILURE: - // - after Hello/Welcome (step 2-4), or an emitter would publish into a ring the peer - // has not mapped; - // - after the first CapsSnapshot (step 7), because R-8's liveness gates read the caps - // mirror and a PLACEHOLDER mirror consumes nothing - a record emitted before it - // would go to a server this client has not been told consumes that family; - // - after ServerLoop::Start (step 8), because EmitAndWait BLOCKS on appliedSeq and - // with no apply thread nothing advances it: the first resource_create would spend - // the whole kBarrierTimeoutMs in the barrier and then Fatal{BarrierTimeout}; - // - on THIS thread, the one that called MG_Backend::Init(), because it is the GL - // thread and table 3 makes gPipeInputs its to touch while the barrier holds. - // The publication is safe without a fence because the apply thread never reads these - // tables - the server decodes straight into MGPipeApply* - and this thread wrote them - // before it can reach any GL entry point. + // ---- 9. Install the wire emitters last; each earlier step is a precondition: + // - after Hello/Welcome, or an emitter would publish into a ring the peer has not mapped; + // - after the first CapsSnapshot: liveness gates read the caps mirror, and a placeholder + // mirror says the server consumes nothing; + // - after the inproc ServerLoop::Start, or EmitAndWait would wait on an idle appliedSeq; + // - on the GL thread that called MG_Backend::Init(), the one that may write gPipeInputs. + // No fence is needed: the apply thread never reads these tables, and this thread wrote + // them before it can reach any GL entry point. InstallClientWireTables(); return MOBILEGL_OK; } @@ -2307,23 +2275,13 @@ namespace MobileGL::MG_Remote::Client { // will not read this one (resource_subdata's buffer half, §2.5 - the only such caller). // The server posts either way, so the wire is unchanged and the row keeps its flag. - - // ---- THE P5e WAIT RULE (CONTRACT-P5E §2.3) --------------------------------------- + // ---- run-ahead wait rule --------------------------------------------------------- // - // IT STANDS BEFORE THE BATCH SKIP AND REPLACES IT WHOLE, rather than adding a clause to - // it. The batch skip is a rule about CALL CLASSES ("a set/CSO/object record carries its - // whole value in the payload"); the wait class is a rule about WHAT AN APPLY READS, and - // it is generated from the same PipeCalls.def row the class comes from - // (MGPipeWaitClassFor, c0e). Two rules layered would mean a row could be skipped by the - // class rule and waited by the column, and the column is the one the SERVER's - // MGPipeBarriered reads - so the two halves of the wire would disagree about whether - // the client is parked behind this record, which is the single fact §4.4's exemptions - // and §3.3's detector both rest on. - // - // WITH RUN-AHEAD DISARMED NOTHING BELOW RUNS and the file's original shape continues - - // Magma, a monolith transport, MOBILEGL_IPC_RUN_AHEAD=0 and every build before the - // integration commit take the old path byte for byte, which is what makes RUN_AHEAD=0 - // a pure wait-rule A/B on identical server code (§5.8). + // With run-ahead armed, the WaitClass column (MGPipeWaitClassFor) alone decides whether a + // record waits, and the batch skip below is disarmed. The server's MGPipeBarriered reads + // the same column, so a second skip rule would let the two sides disagree about whether + // the client is parked behind the record (ARCHITECTURE.md §11.6.1). With run-ahead + // disarmed this block is skipped, so MOBILEGL_IPC_RUN_AHEAD=0 is a pure wait-rule A/B. if (RunAheadArmed() && !ownsReplySlot) { MG_Pipe::MGPipeWaitClass waitClass = MG_Pipe::MGPipeWaitClassFor(op); // §2.5's named relaxation, and the ONLY place a static column answer is overridden: @@ -2357,41 +2315,13 @@ namespace MobileGL::MG_Remote::Client { } } - // MOBILEGL_IPC_BATCH_WAITS (default 1): a value-class record is published WITHOUT - // waiting for its own apply. The barrier's one load-bearing reader is the backend - // sync that pulls the object-class fields (CONTRACT-P5 table 2's remaining rows), and - // those reads happen only at the kCtxVerb applies - draw / clear / blit / dispatch / - // readback / XFB - plus the screen and query classes, and EVERY ONE OF THOSE STILL - // WAITS below (as does every reply-slot row, whose answer the client consumes). So - // the pulled fields are exactly as fresh at every read point as the per-record - // barrier made them: between two waits the residual fill may move a field, but no - // apply is reading it in between, and the next pull-verb's fill runs after the - // previous pull-verb's apply was waited - the R-1 fence posts are untouched, only - // the number of round trips between them changes. Set / CSO / object records carry - // their whole value in the payload and are consumed at the next (waited) sync, which - // the in-order ring guarantees precedes that sync's apply. - // - // WITH ONE NAMED EXCEPTION, FOUND BY ITS OWN FATAL (P5d round 3): generate_mipmap is - // catalogued kCtxObject, but its sink is the backend's GenerateMipmap, which reads - // gPipeInputs->GetActiveTextureUnit() and the unit's binding slot - residual inputs whose - // freshness is the CLIENT's verb serial. Published without a wait, the record sits in - // the ring while this thread runs the next verb's MGPipeValidateForVerb, which moves - // that serial and withdraws the server stamp; the apply thread then reads the unit - // stale and aborts Fatal{UnmigratedPipeInput, "GetActiveTextureUnit@"} - // (F1WireScenario.GenerateMipmap*Pixels, reproduced on the gate tree once the round-3 - // spin no longer read the clock). So it waits, exactly as the four stamping verbs - // do. The rule this encodes: a record may skip its wait only if its apply reads - // nothing the residual fill writes - the value carried in the payload is the whole - // input. No other kCtxObject row has a backend body behind it that reads gPipeInputs - // (set_texture_params, resource_subdata, resource_readback and get_texture_image - // own reply slots and wait anyway; the resource_* transfers and object_death resolve - // by handle). - // - // AND IT IS DISARMED WHOLE UNDER RUN-AHEAD (P5e §2.3): the wait class above is the - // rule there, and a second skip rule reading a different column is how the two halves - // of the wire end up disagreeing about whether the client is parked. Written as a - // conjunct here rather than left to "the classes happen not to overlap", because the - // classes overlapping is one PipeCalls.def row away at any time. + // MOBILEGL_IPC_BATCH_WAITS (default 1), lockstep only: a state, CSO or object record is + // published without waiting for its apply. Its payload is its whole input, and the backend + // reads pulled fields only in verb, screen and query applies, which still wait, so each + // read sees the values the per-record barrier would give. A record may skip its wait only + // if its apply reads nothing the residual fill writes: generate_mipmap is an object record + // but GenerateMipmap reads the active texture unit from gPipeInputs, so it waits. + // Disarmed under run-ahead, where the WaitClass rule above is the only rule. if (!ownsReplySlot && !RunAheadArmed() && MG_Config::Ipc.BatchWaits != 0 && op != MG_Pipe::MGPWireOp::GenerateMipmap) { const MG_Pipe::MGPipeCallClass callClass = MG_Pipe::MGPipeCallClassFor(op); @@ -2770,29 +2700,11 @@ namespace MobileGL::MG_Remote::Client { // // This is where a fill left standing for an unbarriered record lands, by name. if (ClientSessionInstance().RunAheadArmed()) { - // ---- P5e (ra2): THE EXEMPTION IS A FACT NOW, NOT A CLAIM ------------------------ - // - // This arm used to read `if (isBarrieredFill) return;`, i.e. the caller's own - // sentence "this touch is the residual fill of a record this thread is about to park - // behind" was accepted as the whole argument. It is an argument about the FUTURE. - // The order at the validate point is fill, then emit, then park, so at the instant - // of the write the apply thread is still draining the unbarriered records the client - // ran ahead of - and the measured consequence was the GL thread bumping - // CurrentVerbSerial, withdrawing MGPipeServerClearVerbBoundary's flag and renaming - // m_currentVerb underneath a record the applier was inside, which surfaced as - // Fatal{UnmigratedPipeInput, "@"} on the apply thread. - // Because the exemption was unconditional, this guard could not fire on any of it: - // that is the answer to "does RefusePipeInputsTouchWhileApplierOwnsIt fire, and if - // not, why not". - // - // So the sentence still exempts the fill, but only once it is TRUE of this instant. - // The fill sites establish it by taking §2.5's forced wait first - // (QuiesceApplierBeforeFill, PipeFill.cpp); removing that wait is the red-once and - // lands here, by name, on the first barriered verb behind a run-ahead backlog. - // - // ApplyThreadIsInsideApplier() is an acquire load of the flag PipeApplier raises for - // the whole of ApplyOne - stamp, decode and LeaveApplier included - so "not inside" - // is exactly "no record is being applied" and not merely "not decoding". + // A barriered fill is exempt only if the applier is idle at this instant: the validate + // point runs fill, emit, park, so the applier may still be applying records the client + // ran ahead of. The fill sites wait for it first (QuiesceApplierBeforeFill), and + // ApplyThreadIsInsideApplier() covers the whole of PipeApplier's ApplyOne, so "not + // inside" means no record is being applied (ARCHITECTURE.md §9.2.2). if (isBarrieredFill && !ApplyThreadIsInsideApplier()) return; if (Server::ServerLoop::OnApplyThread()) return; // the applier owns the block // P15: with two blocks the GL thread's fill writes the client block, which the @@ -3305,22 +3217,10 @@ namespace MobileGL::MG_Remote::Client { Transport::LogRoleMemory(phase, SampleMemory()); } - // R-10's AND R-9's numbers IN EVERY SPLIT PRIVATE LOG, not only in the lanes that set - // MOBILEGL_PIPE_STATS=1. - // - // WHY IT IS HERE AND NOT ONLY ON THE STATS LINE. `MGPipe stats:` is an opt-in channel: two - // ctest entries out of 21 set MOBILEGL_PIPE_STATS, and neither the retrace lanes nor the - // 19 ordinary split entries do. R-10's proof obligation is about THE PHASE, not about the - // two counting lanes - "no record on the reduced path comes near half the ring" has to be - // readable from any split run that happened, which is what ID-53's per-entry private log - // is for. One line per session teardown costs nothing and cannot be missed. - // - // IT IS ALSO WHERE THE PROOF FAILS SOFTLY. A record ABOVE the cap already aborts on the - // spot with Fatal{RingOverrun} (PipeWireCodec.cpp), so this line's job is the other half: - // a maximum that is merely CLOSE to the cap is not a crash and would otherwise be - // invisible until the day a workload crossed it. The percentage is printed for exactly - // that reason, and R-10 names the integrator as the person who decides between early - // chunking and a bigger default ring when it climbs. + // Logs the wire ledger once per session teardown at INFO, so every split run's log shows the + // largest record against the ring's record cap whatever MOBILEGL_PIPE_STATS says. A record + // above the cap already aborts with Fatal{RingOverrun} (PipeWireCodec.cpp); the percentage + // shows a maximum creeping toward it, which calls for earlier chunking or a bigger ring. void ClientSession::LogWireLedger() const { const Uint64 maxRecord = m_encoder.MaxRecordBytesSeen(); const Uint64 cap = m_encoder.MaxRecordBytesCap(); diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index f621c761..c9188aab 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -6,31 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// The client half of a session: the rings, the handshake, the verb barrier. Owner: package s1 -// (construction and handshake) with c1 (the barrier and the reply read). Signatures by c0. -// -// INPROC USES ShmSegment AND THE RING, NOT new[] AND NOT InProcessTransport's deques. That is -// half of what "inproc runs the same G3 codec as spawn" means: InProcessTransport -// (InProcessTransport.cpp:38-97) is two deque> plus two condvar doorbells, it -// touches neither a ring nor a codec, and building the session on top of it instead of on top -// of the ring would make the whole phase unfalsifiable. The transport supplies the two -// DOORBELLS and the control plane; the records go through SEG_CMD. -// -// THE VERB BARRIER (R-1). After emitting a verb the client blocks until -// RingControl::appliedSeq >= the seq it just got back from the encoder. It is not caution: 31 -// of the 63 PipeInputs fields are still filled by the client's residual pass out of a live -// GLContext, so an unbarriered queue lets the server read a FUTURE value of them. Two -// consequences that must be stated because both are load-bearing: -// - while the barrier holds, at most one of {GL thread, apply thread} is runnable, which is -// what makes a single process-wide gPipeInputs legal (table 3); -// - the barrier is a RETIRING object, not a design. It opens family by family as table 2's -// BARRIER-PULLED column empties, and each later package reports how many rows it left. -// -// THE BARRIER'S WAIT IS ALSO THE REPLY'S WAIT (R-3/R-5). The reply slot id IS the record seq, -// so "wait for appliedSeq >= mySeq" and "wait for my answer" are one wait and the four Bool -// acceptance returns, ReadPixels' pixels and MapPersistent's decline cost ZERO extra round -// trips. The client MUST NOT re-derive any of those four answers locally - that is the c0f/c0g -// defect P4a paid two contract corrections for, and "always accept" is ID-39's 66 lost uploads. +// The client half of a session: the rings, the handshake and the record wait rule +// (ARCHITECTURE.md §11.6). Inproc uses the same shared segments, ring and codec as spawn; +// InProcessTransport supplies only the two doorbells and the control plane, so the inproc lane +// exercises the real data path. The reply slot id is the record seq, so waiting for +// appliedSeq >= seq is also the wait for the answer and a reply costs no extra round trip. +// An answer the server may decline is read from its reply, never assumed: a client that assumed +// "accepted" would clear its dirty flags for records the server dropped. Rows that skip the +// answer (willReadReply=false) do so only where the client already knows it or nobody uses it. #pragma once #include "../Transport/ILink.h" @@ -73,26 +56,14 @@ namespace MobileGL::MG_Remote::Client { // Null until Start() succeeds; MG_Backend::Init() is the only caller of Start(). static ClientSession* Active(); - // P6 `dl` (CONTRACT-P6 5.3). THE DEVICE-LOST LATCH, and the two words that matter are - // SESSION-SCOPED and LATCH: it is set at most once per session and never cleared while - // that session lives. What recovers is the PROCESS, not the session: the application's - // next eglCreateContext / eglCreate*Surface / eglInitialize ends the lost session and - // dials a fresh one (RecoverAfterDeviceLoss), and every context built before stays lost. - // - // SET FROM Doorbell::PeerHungUp() OR THE SERVER'S OWN SessionFault, NEVER FROM A - // TIMEOUT. 5.4's whole point is that slow and dead are different states and must be told - // apart without a threshold: under P5e run-ahead a server one frame behind is the - // INTENDED steady state, so a latch armed by a deadline would fire on a healthy session - // under load. The descriptor (or the server, naming its fault) answers instead. - // - // WHAT THE CALLING THREAD SEES, which is more than the latch: the thread's GL state may - // belong to a session that has already been replaced (a context from before a recovered - // loss, still current on some thread), and for that thread the device is lost for good - - // nothing it does may reach the new session. ScopedWireMute below answers true as well; - // ScopedCurrentSessionWork lifts the thread's own staleness (not the latch). - // - // The static form answers false whenever no session is active, so monolith and the pull - // build get the honest answer with no branch of their own. + // Whether the device is lost for the calling thread. The latch is per session: set at + // most once, never cleared while that session lives. Recovery replaces the session + // (RecoverAfterDeviceLoss) and every context built before stays lost (ARCHITECTURE.md + // §15.3). It is set only when the peer hangs up or the server sends a SessionFault, never + // from a timeout: under run-ahead a server a frame behind is healthy. Also true inside + // ScopedWireMute and on a thread whose context belongs to a replaced session; + // ScopedCurrentSessionWork lifts the thread's staleness, not the latch. Otherwise false + // when no session is active (monolith). static Bool DeviceLost(); // The latch alone: is the ACTIVE session lost. What the EGL layer asks before recovering. static Bool SessionLatchedLost(); @@ -184,32 +155,13 @@ namespace MobileGL::MG_Remote::Client { // ran monolith and went green" failure, and it must be loud. MobileGLResult Start(MG_Config::TransportMode mode, const String& endpoint); - // Start()'s second half: the handshake and everything after it, over a transport pair - // the CALLER made with InProcessTransport::CreatePair. Start() refuses every mode but - // `inproc`, makes the pair, and calls this; it is public for exactly one reason. The - // Welcome guard below (envelope->msg_as_Welcome() == nullptr, ID-46 finding 7) can only - // be reached by a frame that arrives on the server->client direction BEFORE the - // server's own Welcome, and Start() builds that pair itself, so no control could put - // one there. SessionHandshakeTest does it through here, and the null-union Welcome must - // come back as MOBILEGL_ERR_PROTOCOL_MISMATCH with the guard's own line. Not a second - // way to start a session: MG_Backend::Init() calls Start(), and nothing else may call - // this with a pair it did not just create. - // P6 `sm`: the spawn client's half. `transport` is this process's end of - // the control socket to an already-running server process; the two bell - // fds are this side's ends of the two bell pairs (ServerSpawn.h). - // - // It is the SAME handshake StartOverTransportPair runs, with three - // differences and no others: the server half happens in another process - // so there is no local Accept to call, the four segments arrive as - // descriptors over SCM_RIGHTS instead of being dup'ed from a local - // owner, and the bells are sockets rather than condvars. Everything - // below the attach - producer, reply pool, event ring, segment table, - // encoder - is byte for byte the inproc path, which is the property - // that makes "P6 is a transport swap" true of THIS function even though - // it is not true of the phase. - // `cp`: the remote control sink ServerLoop calls when the server is - // another process. Encode, send, wait for the SurfaceReply that carries - // OUR seq back. + // The sink ServerLoop calls when the server is another process: it encodes the frame, + // sends it and waits for the SurfaceReply carrying its seq. + // StartOverTransportPair (below) is Start()'s second half, over a pair the caller just + // made with InProcessTransport::CreatePair. It is public only so SessionHandshakeTest can + // put a frame ahead of the server's Welcome; production reaches it only through Start(). + // StartSpawned and StartOverSocket run the same handshake against a server in another + // process. static MobileGLResult RemoteControlSinkThunk(void* user, Server::SurfaceControlFrame& frame); MobileGLResult RunRemoteSurfaceControlFrame(Server::SurfaceControlFrame& frame); @@ -321,23 +273,14 @@ namespace MobileGL::MG_Remote::Client { // DECLINED rather than a hang. static void SetT0OfferFaultForTest(Int32 delayMs); - // `willReadReply` (P12) and `wantReply` ARE TWO QUESTIONS AND MUST STAY TWO. - // - // wantReply does THIS CALL read the answer (it blocks on the barrier and takes the - // reply slot before returning); - // willReadReply will the answer be read AT ALL - now, or later by somebody else. - // - // The create window is the case that makes the difference load-bearing: it emits with - // wantReply=false and its answers are read afterwards by the window's drain, so a record - // it emits must still be ANSWERED. Only willReadReply=false tells the server it may skip - // the answer (Transport::kRecNoReply), and only three sites may say it: the parameter row - // for a confirmed object, the respecify row's fire-and-forget half, and the sub-data row - // - each of which answers its own caller locally and reads nothing from the wire. - // - // THE TWO ILLEGAL COMBINATIONS ARE FATAL BY NAME rather than ignored, because both would - // fail silently in opposite directions: wantReply && !willReadReply is a caller blocking - // on an answer it has just told the server not to send, and !willReadReply on a row with - // no reply slot is a caller discarding an answer that never existed. + // EmitAndWaitTails' `wantReply` and `willReadReply` are separate questions: + // wantReply this call blocks and reads the answer before it returns; + // willReadReply someone reads the answer at all, now or later (the create window emits + // with wantReply=false and its drain reads the answers afterwards). + // Only willReadReply=false lets the server skip the answer (Transport::kRecNoReply); the + // rows in WireTables.cpp that pass it each answer their caller locally. wantReply with + // !willReadReply, and !willReadReply on a row with no reply slot, are Fatal by name: the + // first would wait for an answer never sent, the second discards one never posted. // MOBILEGL_IPC_VERB_BARRIER. False is the R-1 negative control and is EXPECTED to be // red; it must be run once and the way it goes red recorded. @@ -431,34 +374,13 @@ namespace MobileGL::MG_Remote::Client { static Bool InBarrierWait(); static Bool ApplyThreadIsInsideApplier(); - // P5c (gt, CONTRACT-P5C §6 layer 2 / audit A1): the sentence above's second half, - // wired. A GL-thread touch of gPipeInputs while the apply thread is inside the - // applier and THIS thread is not in a barrier wait is Fatal{RoleViolation, - // "gPipeInputs"} - the barrier is the only thing that makes one process-wide - // gPipeInputs legal (CONTRACT-P5 table 3), and a touch in that window races the - // applier's own reads of it. The residual fill and the verify harness are legal by - // timing, not by exemption: they run before the record is published, which under an - // armed barrier is a moment the flag is provably down (the applier drops it before - // appliedSeq advances past the record the client last waited on). No-op when no armed - // split session is live, when the caller IS the apply thread, and when the barrier is - // disarmed - MOBILEGL_IPC_VERB_BARRIER=0 is R-1's negative control and its red belongs - // to Fatal{BarrierViolation}, not to this check. - // - // P5e (ra, CONTRACT-P5E §3.5) RE-DERIVES IT UNDER RUN-AHEAD, and `isBarrieredFill` is - // what the caller says about ITSELF: "this touch is the residual fill of a record this - // thread is about to park behind". With run-ahead armed the block is server-role - // memory for every other touch. The MOBILEGL_IPC_BATCH_WAITS early return goes with - // it: batching is about how many round trips a lockstep client pays, and it says - // nothing about a client that pays none. - // - // P5e (ra2) KEEPS THE ARGUMENT AND STOPS TAKING IT ON TRUST. "About to park behind it" - // is a claim about the future; the write is now. Between the fill and the park the - // apply thread is still draining the unbarriered records the client ran ahead of, so an - // unconditional exemption made this guard unfireable on exactly the class it was - // written for - measured as the apply thread aborting on a field whose stamp the GL - // thread had just withdrawn. So the rule is "not while the applier is inside" again, - // for barriered fills too; what changed is that the fill sites MAKE it true first, with - // §2.5's forced wait (PipeFill.cpp's QuiesceApplierBeforeFill), instead of asserting it. + // Fatal{RoleViolation, "gPipeInputs"} when a GL-thread touch of gPipeInputs could race the + // applier's reads (ARCHITECTURE.md §9.2.2). Under run-ahead only a barriered fill + // (`isBarrieredFill`) made while the applier is not inside a record is legal, so fill + // sites wait first (QuiesceApplierBeforeFill); with distinct client and server blocks a + // barriered fill is always legal. Under lockstep it checks only with + // MOBILEGL_IPC_BATCH_WAITS=0 and an armed barrier, and fires when the applier is inside + // and this thread is not in a barrier wait. Never fires on the apply thread. static void RefusePipeInputsTouchWhileApplierOwnsIt(const char* surface, Bool isBarrieredFill = false); diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index ad353379..bbb90f4b 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -6,26 +6,12 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P5 package c1: the 71-slot emit table. -// -// THE PARTITION IS CONTRACT-P5.md §7's AND IS NOT RE-DERIVED HERE (R-15, ID-12): -// class A 2 slots answered locally from the caps mirror, never emitted, never Fatal -// class B 54 slots emitted; class C 15 slots name their unmigrated verb. -// The three counts are static_asserted to sum to kRemoteEmitSlotCount below, so a slot that -// changes class without changing the arithmetic is a build break rather than a behaviour -// change nobody reviewed. -// -// P5b MOVES SLOTS FROM C TO B, ONE PACKAGE AT A TIME (docs/archive/contracts/CONTRACT-P5B.md §7). The three -// numbers above are the partition AT THE P5b CONTRACT COMMIT and they are the ones the contract -// states; the arithmetic below is what the tree currently has, and the per-package ownership -// assertions say which package moved which slot. On this head t2 has landed: class B is 5 + 6 -// and class C is 58. -// -// THE PRE-VERB HOOKS RUN BEFORE THE RECORD, NEVER AFTER (b1, ID-18). PushPersistentMapsBeforeVerb -// publishes the bytes an application wrote through a coherent map with no API call at all, and -// MarkGpuWritesForDraw builds the conservative GPU-write set the client now owns. Both describe -// the work the record is ABOUT TO START, so a hook deferred past its own record is the C-1 -// regression re-committed at the transport layer. +// The remote emit table: one slot per function pointer of GlobalBackendFunctionsTable +// (kRemoteEmitSlotCount). Class A slots are answered locally from the caps mirror, class B slots +// emit a record, and class C slots abort naming their unmigrated verb. The class counts are +// static_asserted to partition the table, so a slot that changes class without updating the +// counts breaks the build. Pre-verb hooks (VerbPort.h) run before the record, never after: they +// publish coherent-map bytes and the GPU-write set for the work the record is about to start. #include "EmitTables.h" #include @@ -163,22 +149,10 @@ namespace MobileGL::MG_Remote::Client { ClientSession& session = RequireSession("Present"); BeforeReadOnlyVerb(); - // ---- R-10's and R-9's readings, published BEFORE the present record ------------ - // - // THE FRAME BOUNDARY IS THE RIGHT PLACE and the per-record path is the wrong one: - // the encoder keeps all five as run totals, so this is five relaxed stores per - // frame rather than five per record. Guarded by Enabled() like every other counting - // site in the tree, so the cost with MOBILEGL_PIPE_STATS unset is a global load and - // a predicted branch. - // - // AND IT IS BEFORE THE EmitAndWait BELOW, WHICH IS NOT A DETAIL. PipeStats::OnPresent - // is called by the SERVER's Present - i.e. from inside the apply of the very record - // this function is about to emit - so a publish placed after it lands one frame - // late, and the FIRST summary line of every run then reads `maxrec=0 maxcap=0`. - // Measured that way once: a zero that means "not published yet" is printed in the - // same shape as a zero that means "nothing crossed", and the second one is a real - // defect (an emit table that fell through to the driver). The Present record is 24 - // bytes and cannot be the maximum, so nothing is lost by reading one record early. + // Publish the encoder's run totals at the frame boundary (five stores per frame, not + // per record) and before the present record: PipeStats::OnPresent runs in the server's + // apply of that record, so publishing after it would make each run's first summary + // line read zero. The present record is too small to be the maximum. if (MG_Util::PipeStats::Enabled()) { const Wire::PipeWireEncoder& encoder = session.Encoder(); using MG_Util::PipeStats::Gauge; @@ -398,32 +372,11 @@ namespace MobileGL::MG_Remote::Client { Uint64 replyBytes = 0; session.EmitAndWait(op, payload, bytes, nullptr, 0, &result, sizeof(result), &status, &replyBytes); - // A DECLINED SYNC REPLY IS AN ANSWER, NOT A CORRUPTION (P12). ReplySink opens the - // three-value space with "DECLINED IS A REAL ANSWER, not a failure" - // (PipeWireCodec.h:457-460) and the escape rows are held to it the other way round - - // "ERROR IS NOT A DECLINE", an escape may not fold 2 into false - // (WireTables.cpp:446-450). This site folded 1 AND 2 into one abort. - // - // BOTH VALUES OF 1 ARE REACHABLE WITHOUT A BYTE OF CORRUPTION: - // - the SERVER declines these two rows by design whenever it cannot answer them - - // PostReply(op, seq, ok ? kStatusOk : kStatusDeclined, ok ? &result : nullptr, - // ok ? sizeof(result) : 0) at PipeWireCodec.cpp:1820-1830, which is a legitimate - // zero-length answer to a kWaitReply row (R-5); - // - the CLIENT produces the same status itself once the session stops carrying - // records: the device-lost latch (ClientSession.cpp:1837) and a doorbell that died - // during teardown (ClientSession.cpp:2025, where the comment says outright that - // "the verb did not happen" is reported as DECLINED and not as ERROR). - // - // AND THE NO-OP IS THE ONE THIS FRONTEND ALREADY GIVES for a fence it cannot wait on: - // GL_Sync.cpp:103-106 answers GL_ALREADY_SIGNALED for a sync with no backend handle, - // PipeApplier.cpp:175-177 answers GL_ALREADY_SIGNALED for a fence with no native - // object (a fence that provably did no work), and ClientWaitSync() in DirectGLES.cpp does the same - // for an absent sync. A lost device is that same class of fact. - // - // WHAT ABORTING HERE COSTS, MEASURED: after a clean device loss Minecraft 26.2's next - // glClientWaitSync - the chunk-upload fences - reached this line with DECLINED and died - // of Fatal{ProtocolCorruption, "Fence.reply"}, rc=134, instead of the no-op the design - // promises ("GL calls become no-ops", design/08-runtime-and-platform.md:33). + // A declined reply is an answer, not a corruption: the server declines these rows when + // it cannot answer them, and the client reports a verb that did not happen (the + // device-lost latch, a doorbell that died during teardown) as declined. It gets the + // frontend's answer for a fence it cannot wait on, signaled, so GL calls after a device + // loss are no-ops (ARCHITECTURE.md §15.3) rather than aborts. if (status == Wire::ReplySink::kStatusDeclined) { MGLOG_E_ONCE("MG_Remote client: the peer DECLINED a %s reply - the verb did not " "happen (a lost device, or a teardown); answering the no-op the " @@ -503,31 +456,14 @@ namespace MobileGL::MG_Remote::Client { #include "QueryEmit.inc" - // CLASS C - each remaining slot names its first blocker. + // Class C - each remaining slot names its first blocker. // ============================================================================= // - // PARTITIONED BY THE P5b PACKAGE THAT OWNS THE FLIP (docs/archive/contracts/CONTRACT-P5B.md, - // docs/archive/notes/p5b/BRIEF-P5B.md), so that four packages migrating in parallel edit four - // DISJOINT lists and four DISJOINT counts rather than one list and one number. To flip a - // slot a package (1) removes its X row from ITS list, (2) assigns the real emitter in - // BuildRemoteEmitTable's class-B block, (3) raises ITS kEmittedSlots* by one. The - // per-package ownership assertions below then still hold, the totals stay arithmetic, - // and a slot that changes class without changing the arithmetic is a build break. The - // X-macro shape is kept so the DEFINITION and the ASSIGNMENT cannot drift apart. Three - // slots carried a body the macro cannot (DispatchCompute, DispatchComputeIndirect, - // SetSwapInterval) and were written out by hand below; all three are class B now. - // - // d1 indexed / instanced / multi-draw / indirect draws -> draw_vbo (59), its - // kDrawIsIndirect tail and its kDrawHasUserIndices span - // i1 image bind, compute, barriers, copy-image, storage block -> bind_shader_image - // (72), launch_grid (60), memory_barrier (61), resource_copy_region (53), - // set_storage_block_binding (75) - // t2 the XFB spans and object bind, the patch parameter -> begin/end/pause/resume_ - // stream_output (62..65), bind_stream_output (74), patch_parameter (73) - // f1 the clear family, the framebuffer-sourced copies, mips -> clear (57), - // copy_framebuffer_to_texture (76), generate_mipmap (54) - // tail the wave-3 remainder nothing measured: queries, syncs, the texture readbacks, - // the DSA blit, the swap interval (census-classC.md "static cross") + // One X-macro list per slot group (D1 draws, I1 image/compute, T2 XFB and patch + // parameter, F1 clear/copy/mipmap, TAIL queries/syncs/readbacks), each paired with its + // kEmittedSlots* count so the ownership static_asserts below hold. A slot moves to class B + // by removing its row, assigning its emitter in BuildRemoteEmitTable and raising its + // group's count. The X-macro keeps a stub's definition and assignment in step. // d1 v1 flipped all nineteen (DrawElements, DrawElementsBaseVertex, MultiDrawArrays, // MultiDrawElements, MultiDrawElementsBaseVertex, MultiDrawElementsIndirect, diff --git a/MobileGL/MG_Remote/Client/EmitTables.h b/MobileGL/MG_Remote/Client/EmitTables.h index 48ead139..4501fcec 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.h +++ b/MobileGL/MG_Remote/Client/EmitTables.h @@ -6,56 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// The client's emitting function table. Owner: package c1. Signatures by c0. -// -// MG_Backend/Init.cpp:44 assigns gBackendFunctionsTable from the active backend object, and -// 91 MG_Impl/GLImpl sites call through it directly. So a BackendObject_Remote has to return a -// COMPLETE table, and "complete" is a bigger number than R-4's headline: -// -// GLFunctionsTable (BackendObject.h:117-292) = 69 function pointers -// + Bool PrefersCpuXfbPrimitiveAccounting -// GlobalBackendFunctionsTable (BackendObject.h:293-299) = the above, + Present, + SetSwapInterval -// = 71 function pointers in total -// -// R-4's rule, restated over all 71: NO SLOT MAY BE NULL, and no slot may fall through to a -// driver. A null slot is 91 potential null calls; a pass-through slot is a split lane quietly -// running monolith and going green, which is the one outcome every gate in this phase exists -// to prevent. A verb P5 does not implement gets a slot that raises -// Fatal{UnmigratedVerb, ""} - the same shape as MGPipeInputPoisonFatal, live at every -// log level, MGLOG_F + std::abort. -// -// WHICH SLOTS GET A REAL EMITTER IS DECIDED BY THE VERB CENSUS (R-4), not guessed here: -// docs/archive/notes/p5/verb-census.md. CONTRACT-P5.md §7 carries the resulting THREE-CLASS SPLIT and -// it is not to be re-derived: -// -// A. ANSWERED LOCALLY from the caps mirror, never emitted and never Fatal (R-15) - two -// slots, GetIntegeri_v and IsTimerQuerySupported, plus the Bool member -// PrefersCpuXfbPrimitiveAccounting, which is not a slot. GetIntegeri_v is the one that -// would otherwise sink the phase: it is reached by the FIRST glCompileShader of every -// context (CompileEnv.cpp:134-138 <- Core.cpp:39), not by any verb, so a Fatal there -// aborts every scenario before it draws anything. -// B. EMITTED in P5 - five slots: Clear, DrawArrays, ReadPixels, BlitFramebuffer, Present. -// Present has ZERO MG_Impl call sites: it is reached through EGLImpl.cpp:178 -> -// BackendObject.cpp:396, so mirroring GLImpl will not find it. -// C. Fatal{UnmigratedVerb} - the remaining 64, SetSwapInterval and GetGpuTimestampNs among -// them. (That was P5's partition. P5b..P10 moved every one of them to B; the last, -// SetSwapInterval, is a forward to the server's control channel since P10, and class C is -// empty. UnmigratedVerbFatal below remains the named refusal of a class-B emitter's -// unrepresentable SHAPE - "+".) -// -// AND THE RULE R-4 WOULD OTHERWISE BREAK. 41 of the 69 slots are null-checked at their call -// site, and several of those checks are CAPABILITY PROBES, not safety checks - BeginOcclusionQuery -// (GL_Query.cpp:481, :785), BeginXfbPrimitivesQuery (:534), SubDataResident. With no null slot -// in this table every one of them answers "supported" and the fallback behind it silently -// disappears. A null check on a slot may not survive into the client: it becomes a caps-mirror -// read, which is what ARCHITECTURE.md §3.3 means by "CallMask replaces 'is this table slot null'". -// -// NOTE the asymmetry this table does not resolve, AND WHERE IT IS RESOLVED (R-17): the -// resource, CSO, framebuffer, texture, sampler and program families do NOT come through here. -// They were emitted from MG_Impl/Pipe/* by 40 direct calls to the 37 MGPipeApply* entry -// points; those call sites now go through the two generated tables -// (MG_Pipe/PipeRoute.h -> MG_Remote/Client/WireTables.cpp). This table covers only the verbs - -// the draws, clears, blits, readbacks, queries, fences and present. +// The client's emit table for the remote role: GlobalBackendFunctionsTable (the 69 +// GLFunctionsTable slots plus Present and SetSwapInterval), installed by MG_Backend::Init as +// gBackendFunctionsTable. No slot may be null or fall through to a local driver: each emits a +// verb record, answers from the caps mirror (GetIntegeri_v, reached by every context's first +// glCompileShader, and IsTimerQuerySupported), or forwards to the server (SetSwapInterval). +// UnmigratedVerbFatal names a call shape an emitter cannot represent. Sites that probed a slot +// for null read a cap bit instead (SlotCaps.h). Resource, CSO, texture, sampler and program +// records bypass this table and go through the routed tables (MG_Pipe/PipeRoute.h). #pragma once #include diff --git a/MobileGL/MG_Remote/Client/QueryEmit.inc b/MobileGL/MG_Remote/Client/QueryEmit.inc index e79eff2d..3c0ad989 100644 --- a/MobileGL/MG_Remote/Client/QueryEmit.inc +++ b/MobileGL/MG_Remote/Client/QueryEmit.inc @@ -35,23 +35,11 @@ Int32 status = Wire::ReplySink::kStatusError; Uint64 returned = 0; session.EmitAndWait(op, request, size, nullptr, 0, &result, sizeof(result), &status, &returned); - // A DECLINED QUERY IS AN ANSWER TOO (P12), for ReadFenceReply's reason and on the - // same three-value space (PipeWireCodec.h:457-460). The server declines these three - // rows by design whenever it cannot answer them - QueryAvailable :1841-1847, - // QueryResult :1848-1854 and QueryTimestamp :1855-1859 all post kStatusDeclined with - // a zero-length payload - and the client produces the same status once the session - // stops carrying records (ClientSession.cpp:1837 is the device-lost latch, :2025 a - // doorbell that died during teardown). `returned != sizeof(result)` is guaranteed for - // such an answer, so the guard below used to abort on every one of them. - // - // THE ZEROED RESULT IS THE NO-OP EACH ROW ALREADY HAS, and it is what a query has - // when there is no answer YET rather than no answer ever: - // - QueryAvailable -> 0 -> false, "the result is not available" (the same answer an - // unfinished query gives - GL_QUERY_RESULT_AVAILABLE exists to be told that); - // - QueryResult -> Produced = 0 -> the caller returns false with *value untouched - // (:70 spells the healthy analogue of the same answer); - // - QueryTimestamp -> 0, no timestamp to report. - // ERROR stays fatal below, and so does an OK reply that arrived short. + // A declined reply is an answer, as in ReadFenceReply: the server declines these rows + // when it cannot answer, and the client reports a verb that did not happen (device + // lost, teardown) as declined. The zeroed result is each row's "no answer yet" value: + // QueryAvailable false, QueryResult Produced = 0 (the caller returns false, *value + // untouched), QueryTimestamp 0. An error or a short OK reply stays fatal. if (status == Wire::ReplySink::kStatusDeclined) { MGLOG_E_ONCE("MG_Remote client: the peer DECLINED a %s reply - the verb did not " "happen (a lost device, or a teardown); answering the row's " diff --git a/MobileGL/MG_Remote/Client/ServerProbe.h b/MobileGL/MG_Remote/Client/ServerProbe.h index dc7ec2e9..8f65985d 100644 --- a/MobileGL/MG_Remote/Client/ServerProbe.h +++ b/MobileGL/MG_Remote/Client/ServerProbe.h @@ -6,22 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// IS THE SERVER THIS PROCESS WOULD DIAL THERE AT ALL. -// -// Installed as the system's EGL/GLX vendor, this library is asked first by every GL process on the -// machine, and the session bring-up behind its first display waits up to 20 s for a server that -// may simply not be running. The loaders that ask (EGL and GLX dispatchers, the GBM loader) all -// move on to the next implementation when one declines - so the honest, cheap answer when nobody -// is listening is to decline before that wait starts, and this is the question that decides it. -// -// Only a CONNECT dial can be probed: MOBILEGL_TRANSPORT=spawn with MOBILEGL_IPC_CONTROL naming -// `unix:`, `unix:@` or `tcp://host:port`. Everything else - monolith, inproc, a -// fork control that launches its own server, an fd: pair somebody already connected - has no -// remote to be absent and answers NotApplicable. Both settings are read the way the configuration -// loader reads them (environment over the system configuration files), because the probe runs -// before MobileGL initializes. -// -// MOBILEGL_IPC_PROBE_TIMEOUT_MS bounds one probe (default 250, 1..60000); 0 turns probing off. +// Whether the server this process would dial is listening. As the system's EGL/GLX vendor this +// library is asked first by every GL process, and session bring-up would wait up to 20 s for an +// absent server; the EGL/GLX dispatchers and the GBM loader move on when a vendor declines, so +// the library declines early instead. Only a connect dial is probed (MOBILEGL_TRANSPORT=spawn +// with MOBILEGL_IPC_CONTROL = unix:, unix:@ or tcp://host:port); anything else +// answers NotApplicable. Settings are read as the config loader reads them (environment over +// system files) because the probe runs before MobileGL initializes. +// MOBILEGL_IPC_PROBE_TIMEOUT_MS bounds one probe (default 250, range 1..60000); 0 disables it. #pragma once diff --git a/MobileGL/MG_Remote/Client/SlotCaps.h b/MobileGL/MG_Remote/Client/SlotCaps.h index 0411c4d4..5142c8d6 100644 --- a/MobileGL/MG_Remote/Client/SlotCaps.h +++ b/MobileGL/MG_Remote/Client/SlotCaps.h @@ -6,97 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// THE 41 NULL CHECKS. Owner: package c1 (CONTRACT-P5.md §7, ID-14). -// -// FORTY-ONE OF THE SIXTY-NINE GLFunctionsTable SLOTS ARE NULL-CHECKED AT THEIR CALL SITE, AND -// SEVERAL OF THOSE CHECKS ARE CAPABILITY PROBES RATHER THAN SAFETY CHECKS. R-4 forbids a null -// slot in the client's emit table - so in that table every one of those probes would answer -// "supported", and whatever fallback sits behind it would silently disappear. That is not a -// theoretical risk: it is how a split lane produces a plausible picture for the wrong reason. -// ARCHITECTURE.md §3.3 already said what replaces the probe - "CallMask replaces 'is this table -// slot null' as the implicit capability probe" - and this header is the concrete list. -// -// TWO SPELLINGS, AND WHICH ONE A SITE TAKES IS DECIDED BY THE SLOT'S CLASS, NOT BY TASTE: -// -// MGL_BACKEND_SLOT_CAP(Slot, CapBit) -// The capability has a published bit. Under split the answer is the SERVER's bit; -// under monolith it is exactly today's null check, character for character. -// The three cases CONTRACT-P5.md §7 names by hand are all of this shape: -// GL_Query.cpp:481 / :558 / :785 BeginOcclusionQuery -> kCapOcclusionQuery -// GL_Query.cpp:534 BeginXfbPrimitivesQuery -> kCapXfbPrimitivesQuery -// PipeFill.cpp SubDataResident -> kCapResidentSubData -// (the third is not a GLFunctionsTable slot but an op-table one, so it reads the bit -// directly in PipeFill.cpp rather than through this header). -// -// MGL_BACKEND_SLOT_LOCAL(Slot) -// The capability has NO published bit and the slot is CONTRACT-P5.md §7 class C - -// Fatal{UnmigratedVerb} in the client's table. Under split the honest answer is -// "absent", which is precisely what the monolith nullptr meant, so every fallback the -// site already has survives instead of being replaced by an abort. Under monolith it is -// again today's null check. -// -// WHY "ABSENT" AND NOT "LET IT FATAL". A Fatal is loud, and for the 28 UNGUARDED slots it is -// strictly better than today (a null there is already an immediate crash with no diagnostic). -// But a guarded site is guarded because the frontend has a real answer for the absent case - -// always-signaled syncs, a CPU readback, CPU primitive accounting - and turning that answer -// into an abort is a behaviour change nobody asked for, in the direction that stops a lane -// dead. The class-C slot IS absent from this client; saying so is the accurate answer, not a -// weakening of R-4. -// -// THE TEST THAT DECIDES WHETHER A GUARDED SITE IS CONVERTED AT ALL, and it is the half of the -// walk the contract leaves to whoever does it: A PROBE MOVES ONLY WHERE "ABSENT" IS A CORRECT -// AND SUFFICIENT ANSWER. Where the fallback behind the probe produces a RIGHT result, "absent" -// is the accurate description of a client that does not have the slot, and converting keeps -// the lane running. Where the fallback produces a SILENTLY WRONG result, converting would -// manufacture exactly the defect R-4 exists to prevent, and the probe is left alone so the -// class-C slot Fatals by name instead. -// -// Converted, because the fallback is right: -// FenceSync used this fallback through P5. P5b §9 moved it to class B: the frontend -// now calls the emitter and the server preserves the optional/native-null fallback. -// GetTexImage/GetTextureImage now emit owned texture readback replies. Their -// split path must never use the frontend shadow after GPU writes. -// GL_Getter.cpp x2 GetGpuTimestampNs and GL_Query.cpp's query probes now use their -// timer/occlusion/primitive capability bits. An advertised path emits real query -// records; only an unavailable native capability reports COUNTER_BITS 0. -// -// NOT converted, deliberately, because "absent" would be wrong rather than quiet: -// GL_Drawing.cpp:1274/:1371/:1420/:1435/:1641/:1673 the transform-feedback span family. The -// frontend reads a null EndTransformFeedback as "this backend does NOT own the capture, -// so reorder the captured records for it" (FixupGsStripCaptureOrder, :1290). Under split -// the server's backend DOES own it, so answering "absent" would reorder a capture that -// was already in GL order - a silently corrupt buffer. Transform feedback is off P5's -// reduced path (BRIEF §4's exclusion list) and its slots are class C, so the first XFB -// call aborts by name, which is the outcome R-4 asks for. -// GL_Drawing.cpp:844 PatchParameteri. "Absent" means the patch size is never set and every -// tessellation draw silently uses the previous one. Class C; it aborts by name. -// -// P5b t2 CLOSED THE FIRST OF THOSE TWO, AND THE OTHER SIX SITES NEEDED NOTHING (CONTRACT-P5B.md -// §2 t2, §6.5). The six SPAN sites - :1274 Begin, :1371 End, :1420 Pause, :1435 Resume, :1641 -// Delete, :1673 Bind - are guards over a slot that is now class B in the client's table, so they -// simply call the emitter; their `if (const auto f = ...)` shape is left exactly as it was, -// because under split the slot is non-null and under monolith nothing moved. THE PROBE AT :1290 -// IS THE ONE THAT HAD TO CHANGE, and it is the reason this header said the XFB span family would -// be the first to need a bit: it is not a guard on a call, it is a QUESTION ABOUT THE BACKEND -// asked of a table that under split belongs to the client. It now reads -// MGL_BACKEND_SLOT_CAP(EndTransformFeedback, kCapBackendOwnsXfbCapture), the bit the server sets -// from ITS table in MG_Backend/Init.cpp's InitSplitRoles. Espryt registers the slot and answers -// yes; Magma registers no XFB slot, answers no, and the client keeps reordering for it exactly -// as it does under monolith. GL_Drawing.cpp:844's PatchParameteri stays a plain guard for the -// same reason as the six: the slot is class B now, so "absent" never arises. -// -// DeleteTransformFeedback now emits DeleteStreamOutput with the object's lifetime -// identity; its ordinary slot guard reaches that emitter under a transport. -// -// WHAT THIS HEADER DELIBERATELY DOES NOT DO. It does not touch the 28 unguarded slots: those -// have no probe to convert, and calling one reaches Fatal{UnmigratedVerb, ""} by name, -// which is R-4's intent. And it does not invent a cap bit - a new MGPCapBit is an -// MGPipeTypes.h edit and that file is c0's, so a family that needs one goes through the -// integrator (the XFB span family is the first that will). It did: P5b's contract granted t2 -// exactly that one bit, kCapBackendOwnsXfbCapture (CONTRACT-P5B.md §6.5, §8), and t2 added it. -// -// G1: in a build without MOBILEGL_BUILD_DISAGGREGATED both macros expand to the null check the -// site already had, so the pull build's code generation is unchanged. +// Capability probes over GLFunctionsTable slots. A call site that tests a slot for null to learn +// whether the backend supports something cannot do so under a transport, where the client's +// emit table has no null slot; it reads a capability bit instead (ARCHITECTURE.md §3.3). +// MGL_BACKEND_SLOT_CAP(Slot, CapBit): split reads the server's CapBit; monolith tests the slot. +// MGL_BACKEND_SLOT_LOCAL(Slot): no published bit; split answers "absent" so the site's fallback +// runs. Use it only where that fallback is correct, not silently wrong (the XFB capture-order +// probe reads kCapBackendOwnsXfbCapture for that reason). Monolith tests the slot. +// Without MOBILEGL_BUILD_DISAGGREGATED both expand to the plain null check. #pragma once #include diff --git a/MobileGL/MG_Remote/Client/WireTables.cpp b/MobileGL/MG_Remote/Client/WireTables.cpp index 2b63cb70..41a560cc 100644 --- a/MobileGL/MG_Remote/Client/WireTables.cpp +++ b/MobileGL/MG_Remote/Client/WireTables.cpp @@ -6,27 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// The ENCODE TWIN of gMGPipeWireRecordApply: thirty-eight emitters that turn a table call into -// a wire record. Owner: package c1 (P5 ruling R-17). See WireTables.h for the install order -// and MG_Pipe/PipeRoute.h for what R-17 actually cost. -// -// EVERY EMITTER IS THE SAME FOUR STEPS, and the macros below exist so that a reader can check -// thirty-eight rows against PipeTables.inc in one pass instead of reading thirty-eight bodies: -// -// 1. require a session - a slot that fell through to a driver this role does not have is -// the failure R-4 exists to prevent, and there is no fall-through here either; -// 2. stage the blob, if the row has one, and name the SEG_STAGE run in the payload's own -// MGPBlobRef - which is why the payload is COPIED: the table hands it over const, and -// the blobref is the one field the client must write after the caller is done with it; -// 3. EmitAndWait, which is the barrier's wait and the reply's wait at once (R-3/R-5); -// 4. post the answer, for the rows that have one, into MG_Pipe's reply mailbox. -// -// WHAT IS DELIBERATELY NOT HERE. b1's `PushPersistentMapsBeforeVerb` / `MarkGpuWritesFor*` are -// NOT called from these thirty-eight. They are pre-VERB hooks and these are not verbs: they -// are the resource, CSO and state records that a verb is later drawn against. The five class-B -// verbs in EmitTables.cpp call them, once each, immediately before their record, which is the -// ordering b1's B-1 fix depends on. Calling them here as well would push a persistent map -// before every `set_dynamic_state` - hundreds of times a frame, and each one a real record. +// The encode twin of gMGPipeWireRecordApply: the emitters that turn a routed MGPipe table call +// into a wire record (install order: WireTables.h). On the server role's own thread an emitter +// runs the server's code directly; otherwise it requires a session, stages its blob (the payload +// is copied because its MGPBlobRef must name the SEG_STAGE run), emits through EmitAndWait and +// posts any answer into MG_Pipe's reply mailbox. The macros keep the regular rows checkable +// against PipeTables.inc at a glance. These are state and object records, not verbs, so they +// never call PushPersistentMapsBeforeVerb / MarkGpuWritesFor*: those are pre-verb hooks +// (MG_Impl/Pipe/Verb/VerbPort.cpp) and must not run before every set_dynamic_state. #include "WireTables.h" #include @@ -68,34 +55,12 @@ namespace MobileGL::MG_Remote::Client { using MG_Pipe::MGPWireOp; - // TABLE 3's ROLE SPLIT, AS A RUNTIME CHECK. gMGPipeScreen / gMGPipeContext are PROCESS - // globals and under `inproc` the server role is a thread in this same process, so the - // apply thread running the server's own backend - the EGL bring-up, InitCapabilities, - // the applier - reaches these very emitters. A record published there would be waited - // for by the thread that is supposed to apply it: `Fatal{BarrierTimeout, - // "ResourceRespecify"}` from `mgl-srv-apply` one barrier budget into bring-up, which is - // exactly how this was found. - // - // THE ANSWER IS NOT "SUPPRESS THE RECORD" - it is "run the server's own code", because - // on that thread this process IS the server and the applier is one call away. It is - // the same thing PipeWireCodec does on the decode side, where every arm calls - // MGPipeApply* directly and never goes through a table. - // - // Under `spawn` (P6) the predicate is constantly false in the client process and - // constantly true in the server's, so this costs one atomic load and changes nothing. - // - // NOT in the anonymous namespace, because M5 needs it from MG_Impl/Pipe/PipeFill.cpp too - // (its split-only respecify/flush branches must not run on the apply thread). Declared in - // WireTables.h. - // - // AND IT STAYS OUT OF LINE (P5d round 3, package D). OnApplyThread() became an inline - // relaxed load plus a thread-pointer compare in ServerLoop.h, so the body this forwards to - // is now smaller than the call that reaches it - but inlining THIS would mean including a - // server header from WireTables.h, and the declaration above exists precisely so PipeFill - // does not have to. One PLT hop per verb is the price of that boundary; the four-part - // predicate behind it, which was the measured cost (2.66% self on the client thread at - // head 56a77348), is gone either way. The "one atomic load" the paragraph above promised - // is now literally what it costs. + // True on the server role's own apply thread. Under inproc the server's backend bring-up and + // applier run in this process and reach these emitters; a record published there would wait + // for the very thread meant to apply it (Fatal{BarrierTimeout} during bring-up). So on that + // thread an emitter runs the server's code directly, as the decoder does. Under spawn the + // answer is constant per process. Declared in WireTables.h because PipeFill.cpp and + // EGLImpl.cpp need it too; kept out of line so that header includes no server header. Bool RunsAsTheServerRole() { return Server::ServerLoop::OnApplyThread(); } namespace { @@ -322,102 +287,21 @@ namespace MobileGL::MG_Remote::Client { // "always accept" is ID-39's 66 lost DirectVulkan uploads and "accept if we emitted" is // the same bug wearing a counter. - // ---- P12: the CREATE WINDOW, AND WHY IT IS OFF ---------------------------------------- - // - // THE IDEA. The create row is the last per-record round trip on the load path (4,536 of one - // frame's 4,543 reply waits) and the one row whose answer is not optional: PublishCreate - // latches "the applier holds this handle" on it, and that latch is what every converted row - // reads. So this row would not STOP reading its answer - it would DEFER it. Up to - // MOBILEGL_IPC_CREATE_WINDOW creates go out fire-and-forget, the caller reads a provisional - // accept (so the latch is taken), and the answers are collected IN ORDER: the oldest blocks - // when the window is full, and one that comes back DECLINED un-latches its object - // (MGPipeNoteHandleUnpublished), which sends every later row for that object back down the - // blocking path. - // - // IT DOES NOT WORK, AND NOT FOR A REASON THAT TUNING REACHES. MEASURED on the device (the - // same frame, the same binary, the same 101,421,396 B staged, axe aikjs4s8cys8ba7l): - // - // MOBILEGL_IPC_CREATE_WINDOW=4 by_op 2 = 4,536 wall 28.64 s - // MOBILEGL_IPC_CREATE_WINDOW=1 by_op 2 = 4,536 wall 28.64 s - // (the handoff's own pre-window baseline was 31.3 s on this device) - // - // by_op 2 counts the creates that took the BLOCKING path, and it did not move, so no create - // deferred. A temporary counter in the row said why: pushed=24, blocked=4,476, took=20, - // readfail=8,951, pending pinned at 4. The drain READ the window's answers back and failed - // every time. - // - // ---- AND THE DIAGNOSIS BELOW WAS HALF WRONG; READ THIS FIRST ---------------------- - // - // The arithmetic that follows is real and the no-reply bit it motivated WORKS (measured: - // the server skipped 50,001 answers and wrote 4,521, cutting a frame's replies by 92%). - // IT DID NOT FIX THE WINDOW: the drain still read back nothing (readfail 8,951 -> 8,872, - // -0.9%). The binding constraint is not how fast the bounded buffer is overwritten but - // that the drain is STRICTLY FIFO AND HAS NO WAY OUT OF A FAILED READ: it retries the - // same head for ever. Measured on the device (P12miss): `want=310` on every miss while - // `replySeq` climbed from 1,504 to 102,915 - one answer lost to a bounded buffer wedges - // the window permanently, and with 4,536 deferred answers that is a certainty rather - // than a risk. docs/archive/notes/p12/CREATE-WINDOW-MEASURED.md section 8 has the numbers and the - // shape of the fix (abandon the head: un-latch the object, add it to the suspect list, - // and pop - the DECLINE path reused for "unknown"). - // - // WHY, IN THE PROTOCOL'S OWN ARITHMETIC. An answer is readable only until its reply-pool - // slot is reused, and the pool is kDefaultReplySlotCount = 8 slots deep. The frame posts a - // reply for EVERY reply-owning record - not only for the rows that wait, because the server - // cannot know which those are (ClientSession.cpp:1897-1900: "The server posts either way") - // - and that is 55,903 replies in this one frame (op 4 ResourceDestroy is NOT counted: its - // catalogue flags are kNone, so PostReply would Fatal on it and it never answers): - // - // create 4,536 + respecify 20,633 + params 18,056 + sub-data 12,671 + 3,770 + 6 + 1 - // - // 8 slots against 55,903 replies is a pool that churns 6,988 times per frame, i.e. 12.3 - // replies between two creates: an answer posted at create N is overwritten after EIGHT - // replies, which is 0.65 of a create. There is no window depth for which the deferred - // answer still exists when the client comes back for it - the deferral is not slow, it is - // UNREADABLE. (A 1-deep "window" is the blocking path, which is what the row does.) - // - // AND THE FAILURE IS NOT NEUTRAL: with the window on, the provisional accept latches an - // object the applier may have refused, and the un-latch that would correct it lives in the - // drain - the same drain that cannot read. A refusal therefore becomes an object the client - // believes is published, which is exactly the silent divergence the suspect list exists to - // prevent. So the default is 1: the row is the pre-window shape unless someone sets - // MOBILEGL_IPC_CREATE_WINDOW explicitly, and the machinery below is kept as the record of - // what was tried and what it cost (a fix would have to make a deferred answer outlive - // 12.3 intervening replies, which means a deeper pool, or NOT sending the ~51,000 answers - // nobody will read - and that second one needs its predicate stated exactly, because - // "fire-and-forget" is the WRONG one: the window's own creates are fire-and-forget and - // the drain above reads them, so a bit keyed on wantReply disables the mechanism it is - // meant to enable. The bit has to mean "this answer will never be read" - which is true - // of respecify/params/sub-data and false of a windowed create. See - // docs/archive/notes/p12/CREATE-WINDOW-MEASURED.md section 7, reviewed). - // - // The gates that pin it are in MG_Test/Wire/RemoteClientControls.inc, and they still hold: - // the window is a coherent mechanism, it is simply unusable on a frame with this much - // other reply traffic. - // (THE NARRATIVE ABOVE PREDATES RETENTION and ends at "the default is 1". It is kept as - // the record of what was tried; what made the window work is the declared-answer store, - // and the default is now 10 - docs/Performance/ipc-create-window.md carries the device numbers.) + // ---- the create window (MOBILEGL_IPC_CREATE_WINDOW) ------------------------------------ + // resource_create's answer cannot be dropped: an accepted create latches "the applier + // holds this handle" (PublishCreate), and every converted row reads that latch. The window + // defers the answer instead: up to that many creates (at most kCreateWindowMax) go out + // without waiting, the caller reads a provisional accept, and answers are collected in + // order; a declined one un-latches its object (MGPipeNoteHandleUnpublished) and marks it + // suspect, so its later creates block. It runs only on a link that keeps declared answers + // until read (ILink::RetainsReplies). Measurements: docs/Performance/ipc-create-window.md. constexpr Uint32 kCreateWindowMax = 12; - // THE POOL ARGUMENT THAT USED TO PIN THIS AT 4 IS GONE, AND WHY IT IS GONE IS THE ONE - // THING TO RE-CHECK BEFORE RAISING IT AGAIN. It read: the pool is kDefaultReplySlotCount - // slots addressed seq % count, so an answer is readable only while its slot has not been - // reused, and a window owning half of them would starve the rows that take their answers - // immediately - the change the handoff named as the one worth pinning, "someone raises - // kCreateWindowMax to 8 and steps on the pool". That is true of a link whose answers live - // in a SLOT POOL, and the window does not run on one: CreateWindowEffective() is gated on - // ILink::RetainsReplies, which is false for ShmLink, so on the pool-addressed transport - // the window is 1 and holds nothing. On the link where it does run - StreamLink - an - // answer is kept in the declared-answer store until its reader takes it, so depth costs - // STORE ENTRIES, not pool slots. - // - // SO THE SAME RULE IS RE-DERIVED AGAINST THE RIGHT OBJECT: at most half of StreamLink's - // kRetainedCap = 24, the other half being the one blocking row in flight and whatever a - // retired window has stranded. The store's overflow is a named Fatal and never an - // eviction, so exceeding this costs a run rather than a wrong picture - and the store - // logs its own peak as it crosses, which is what a future raise must be argued from. - // - // MEASURED, device load frame, three interleaved runs per arm: the curve had NOT flattened - // at the old cap (1->2 saved 10.29 s, 2->3 3.74 s, 3->4 1.83 s), which is why this - // constant moved at all. + // kCreateWindowMax is at most half of StreamLink's declared-answer store + // (kRetainedAnswersMax): the other half is for the one blocking row in flight and for + // answers a retired window left behind. The window never runs on the slot-pool link + // (ShmLink retains no replies), so depth costs store entries, not reply slots. Store + // overflow is a named Fatal, never an eviction, and the store logs its peak: argue any + // raise from that peak (docs/Performance/ipc-create-window.md). static_assert(kCreateWindowMax <= Transport::StreamLink::kRetainedAnswersMax / 2, "the create window may hold at most half of StreamLink's declared-answer " "store: the other half belongs to the one blocking row in flight and to " @@ -609,28 +493,10 @@ namespace MobileGL::MG_Remote::Client { if (status == 1) ++g_declined; if (status == 0) ClearCreateSuspect(payload->Resource); } - // ---- P12: WHY THERE IS NO RESIDENCY BUDGET HERE ANY MORE ------------------------------- - // - // There was one, and it was measured: it counted the reply-owning records this client had - // emitted since the oldest window entry was pushed and took that entry back when the count - // approached the reply buffer's depth. On the device it fired on essentially EVERY create - // (budget=4,451 of 4,501 calls) and the drain still read nothing back (taken=63, - // readfail=8,872, by_op 2 = 9,071, wall 27.9 s). The clock was wrong, structurally: - // - // * the window only gets control WHEN A CREATE ARRIVES, and between two creates this - // client emits on the order of a thousand records - at the FIRST failed read the reply - // ring already spanned [1175..1504] while the head was seq 310; - // * what displaces a deferred answer is not what this client emits, it is WHAT THE - // SERVER ANSWERS, and the server is behind: its late answers to the earlier creates - // are what fills a bounded buffer first; - // * so the check fired too late by construction. Tightening it only removed the - // deferral entirely, which is what pushed=65 was. - // - // Retention replaced it, and retention is where the bound now lives: a link that declares - // RetainsReplies keeps an answer until its reader takes it (StreamLink's declared-answer - // store, whose capacity is tied to the window's own depth), and the window does not defer - // at all on a link that does not. That removes the only thing the budget was for. - // ReplyPostings() stays as a measurement. + // The window keeps no client-side residency budget. On a link that reuses reply slots, + // what displaces a deferred answer is the server's reply traffic, not this client's + // emissions, so a budget counted here fires too late. The window defers only where the + // link retains answers, and the store bounds it (docs/Performance/ipc-create-window.md). void Wire_ResourceCreate(const MG_Pipe::MGPResourceDesc* payload, MG_Pipe::MGPReplySlot* reply) { if (RunsAsTheServerRole()) { MG_Pipe::MGPipeMonolithScreen().ResourceCreate(payload, reply); return; } ClientSession& session = RequireSession("ResourceCreate"); @@ -732,27 +598,12 @@ namespace MobileGL::MG_Remote::Client { ++g_emitted; } - // A PARAMETER CHANGE FOR AN OBJECT WHOSE EXISTENCE IS CONFIRMED OWES NO ANSWER (P12). - // - // The acceptance this row used to wait for says one thing: "the applier holds a record for - // this handle". That is a fact about the SERVER's table, and the client already keeps its - // own copy of exactly that fact - MGPipeHandleIsPublished, set when the object's create was - // ACCEPTED and never cleared while the record lives (the applier keeps texture records - // across a make-current: MGPipeApplierReset clears only MGPipeWorkingState). So the wait - // is asking the server a question the client has already answered. - // - // WHAT REPLACES IT, on the device run's numbers: this row paid 18,056 of one frame's - // 43,232 round trips at ~5 ms each, for records carrying a few hundred bytes. The object's - // ONE confirmation is its create, paid at its first use (the emitter sends it proactively - // when the latch is clear - EmitTextureParams' HealUnpublishedRecord); every parameter - // record after that is emitted fire-and-forget and the caller reads a PROVISIONAL accept, - // exactly as resource_subdata has done since item B. - // - // AND WHEN THE LATCH IS CLEAR THIS ROW STILL WAITS, which is the whole fallback: an object - // whose create was refused stays unpublished, so its next parameter record takes the real - // answer and the emitter's existing one-retry heal runs. A refusal can only reach here - // for an object the client thinks it never published, or for a bug - the two cases that - // were already the only ones the per-record wait could name. + // set_texture_params owes no answer once the texture is published. MGPipeHandleIsPublished + // is the client's copy of "the applier holds this handle": set when the create is accepted + // and kept while the record lives (MGPipeApplierReset keeps texture records). A published + // texture's record goes fire-and-forget with a provisional accept. An unpublished one still + // waits, so a refusal reaches the caller as a refusal. EmitTextureParams pays the + // confirming create at the texture's first use (HealUnpublishedRecord). void Wire_SetTextureParams(const MG_Pipe::MGPTextureParams* payload, MG_Pipe::MGPReplySlot* reply) { if (RunsAsTheServerRole()) { MG_Pipe::MGPipeMonolithContext().SetTextureParams(payload, reply); return; } ClientSession& session = RequireSession("SetTextureParams"); @@ -878,22 +729,12 @@ namespace MobileGL::MG_Remote::Client { } else { MG_Pipe::MGPipeClearRespecifiedLevel(record); } - // P12: WHETHER THIS RECORD OWES AN ANSWER IS ONE PREDICATE, and it lives beside the - // route (PipeRoute.h, MGPipeResourceRespecifyWantsItsReply) so the emitter and the wire - // cannot disagree about it. The two halves are argued there; the short version is that the - // buffer half's Bool is discarded at both call sites, and the texture half's is owed - // until the object's create has been accepted - the confirmation it pays at first use. - // - // WHAT A PROVISIONAL ACCEPT DOES TO THE TEXTURE HALF, stated here because it is a - // deliberate trade rather than an oversight: the emitter's acceptance mirror - // (entry.LastDesc) now advances on an answer nobody waited for. ID-18 M3 - // (TextureEmit.h:885-888) forbids exactly that for a record whose verdict is UNKNOWN - - // "the next identical call is suppressed against a record that was never stored" - and - // what makes it safe here is that the object is PUBLISHED: the client is not guessing, - // it is reading a latch the server set by accepting this handle's create. The residue - // is the one case where the server loses a record it had, which today has no production - // path (PipeApply.cpp:1593's only callers are tests), and whose guard is the - // invalidation notice the route's notes call ①. + // Whether this record waits is MGPipeResourceRespecifyWantsItsReply, shared with the + // route (ARCHITECTURE.md §13.1.1). On the texture half a provisional accept advances + // the emitter's acceptance mirror (entry.LastDesc, TextureEmit.h) without a verdict. + // That is safe only because the texture is published: the server accepted its create + // and keeps its record. The one exception, the server dropping a live record + // (MGPipeApplierReleaseObjectRecords), has no production caller. const Bool wantsReply = MG_Pipe::MGPipeResourceRespecifyWantsItsReply(record); Int32 status = Wire::ReplySink::kStatusError; if (!wantsReply) { @@ -1284,46 +1125,14 @@ namespace MobileGL::MG_Remote::Client { void InstallClientWireTables() { using namespace MG_Pipe; - // DirectGLES normally installs this notice while creating its handle - // backend. An independent client never creates that backend: without a - // client emitter, NotifyAndFree silently drops object_death before - // freeing the slot. Keep inproc's existing backend dispatcher, and give - // the remote client the same wire delivery without a local twin table. - // - // P7 PACKAGE L: THE BACKEND TEST IS GONE, AND ITS ABSENCE IS THE POINT. P6.5 wrote - // `ActiveBackendType == DirectGLES` because DirectGLES was the only backend with a - // two-process lane, so the condition read as "the case this can happen in" rather than - // as a policy. Under a Magma spawn or tcp client it is a silent hole of exactly the - // shape P6.5 closed for Espryt: CtWireScenario's two death cases would report - // ObjectDeaths=0 and PASS NOTHING, because no notice was ever installed and - // NotifyAndFree would drop every object_death before freeing the slot - a green that - // means "the mechanism is absent". MEASURED on this tree: with the install site - // disabled, both DirectGLES spawn death cases red on CtWireScenario.cpp:187/:246 with - // `Expected: (afterCounters.deaths) > (deathsBefore), actual: 0 vs 0`. - // - // THE REMAINING TWO CONDITIONS STILL CARRY P6.5's INTENT, both halves of it: - // * Transport == Spawn: this is the REMOTE client, the one with no local twin table. - // Under inproc the server role is a thread in this process and its backend's own - // dispatcher is the right one. - // * GetStateObjectDeathOps() == nullptr: whoever installed first keeps the notice. - // That is what "keep inproc's existing backend dispatcher" meant, and it is also - // what keeps this from stomping a backend that installs ops of its own later. - // - // MAGMA NOW HAS A DEATH TABLE OF ITS OWN (P7 wave 2 package C, CONTRACT-P7 §5.5: - // DirectVulkan.cpp's g_magmaStateObjectDeathOps), and the two installs do NOT race, - // which is worth stating because the global is a bare last-writer-wins pointer with - // no stacking. They cannot meet: Magma's is installed from - // BackendObject_DirectVulkan::Initialize(), i.e. only in a process that OWNS a - // DirectVulkan backend, and the arm here runs only under `Transport == Spawn`, which - // is by construction the process that owns NO backend at all (InitSplitRoles builds a - // BackendObject_Remote there). So the spawn client keeps this emitter, the inproc and - // server-side roles keep Magma's, and `GetStateObjectDeathOps() == nullptr` below - // keeps meaning what P6.5 meant by it. - // - // MEASURED, both directions (package C's red-once): with Magma's install - // short-circuited, the two CtWireScenario death cases red on the INPROC arm alone - // (`deaths` 0 vs 0) and stay green on spawn and tcp - which is exactly the split of - // responsibility this comment claims. + // A remote client has no backend, so nothing else installs a state-object death notice + // and NotifyAndFree would free slots without emitting object_death. Install the wire + // emitter only under Transport == Spawn (inproc keeps its backend's dispatcher) and only + // when no ops are installed (the first installer keeps the notice). Magma's own ops + // (DirectVulkan.cpp) cannot collide with these through the last-writer-wins global: Magma + // installs only in a process that owns a DirectVulkan backend, and a spawn client owns + // none (InitSplitRoles builds a BackendObject_Remote). CtWireScenario's two death cases + // cover both installs. if (MG_Config::Transport == MG_Config::TransportMode::Spawn && MG_State::GLState::GetStateObjectDeathOps() == nullptr) { MG_State::GLState::SetStateObjectDeathOps(&kClientStateObjectDeathOps); diff --git a/MobileGL/MG_Remote/Client/WireTables.h b/MobileGL/MG_Remote/Client/WireTables.h index 77e3345b..da6a6bd1 100644 --- a/MobileGL/MG_Remote/Client/WireTables.h +++ b/MobileGL/MG_Remote/Client/WireTables.h @@ -6,31 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// THE CLIENT ARM OF R-17's ROUTING: the encode twin of `gMGPipeWireRecordApply`. Owner: c1. -// -// Thirty-eight thin emitters over `ClientSession::EmitAndWait`, installed over the two -// generated tables and the escape table that `MG_Pipe/PipeRoute.h` declares, so that under -// split every resource, CSO, texture and program record leaves the GL thread as a WIRE RECORD -// instead of executing synchronously against a context the apply thread now owns. -// -// WHAT ARMS `integration-split`. The 21 `DirectGLES.Split.*` entries skip on -// `ClientSession::Active() == nullptr`. `Install()` below is called from the END of a -// successful `ClientSession::Start()`, on the GL thread (the thread that called -// `MG_Backend::Init()`), AFTER the Hello/Welcome handshake, after the first CapsSnapshot has -// been adopted, and after the in-process server role's apply thread has been started. That -// order is not a preference: -// - after the handshake, because an emitter that published before Welcome would be writing -// into a ring the peer has not mapped; -// - after the first snapshot, because R-8's liveness gates read the caps mirror and a -// placeholder mirror consumes nothing, so a record emitted before it would be emitted to a -// server this client has not yet been told consumes that family; -// - after the apply thread exists, because `EmitAndWait` BLOCKS on `appliedSeq` and nothing -// would advance it - a barrier wait with no applier is the 30-second Fatal, not a hang; -// - on the GL thread, because that is the only thread that may touch `gPipeInputs` while the -// barrier holds (table 3), and installing from the apply thread would publish the table to -// the GL thread with no synchronisation at all. -// `Uninstall()` runs at the TOP of `Stop()`, before the rings go away, so the last thing any -// straggling GL-thread call reaches is a named refusal rather than a dangling session. +// The client arm of MGPipe routing, the encode twin of gMGPipeWireRecordApply: thin emitters over +// ClientSession::EmitAndWait, installed into the generated tables and the escape table that +// MG_Pipe/PipeRoute.h declares, so every resource, CSO, texture and program record leaves the GL +// thread as a wire record instead of running against a context the apply thread owns. +// InstallClientWireTables() runs last in ClientSession::Start, on the GL thread: after the +// handshake (the peer has mapped the rings), after the first caps snapshot (consumer gates read +// it) and after the apply thread starts (EmitAndWait blocks on appliedSeq). Uninstall runs first +// in Stop, so a straggling routed call is refused by name instead of reaching freed rings. #pragma once #include @@ -130,23 +113,14 @@ namespace MobileGL::MG_Remote::Client { // ForgetEndedSession). void ForgetWireTablesOfEndedSession(); - // ---- P14 S1: the EGL context lifecycle's control-plane half ----------------------------- + // ---- the EGL context lifecycle's control-plane half ------------------------------------- // - // NOT ring records and NOT routed rows: eglCreateContext/eglDestroyContext are the EGL - // lifecycle, which the catalogue deliberately keeps off the record plane (PipeCalls.def's - // "the eight EGL lifecycle entry points stay virtual functions on pActiveBackendObject"), - // so they cross as two SurfaceOp kinds through the same blocking frame channel the nine - // surface forwarders use. Called by MG_Impl's EGLImpl on the app thread, in the same order - // as the EGL calls themselves - which is the ordering the server's context table needs. - // - // THREE ANSWERS, NOT TWO, for the reason ObjectDeathEmit below has three: "the server said - // no" and "there is no wire for the question to cross" are different facts and the caller's - // answer to each is different. NoWire is the bring-up window before ClientSession::Start and - // the server role's own thread - in both, the pre-S1 shape (the client keeps its own EGL - // books and no server table exists) is the correct one and the context must be KEPT; Refused - // is a server that answered, and eglCreateContext turns it into EGL_NO_CONTEXT, because a - // context whose token the server did not table is one the first bind_context after it would - // be refused for. + // eglCreateContext / eglDestroyContext are not ring records: they cross as SurfaceOp frames + // on the blocking control channel the surface forwarders use, sent by EGLImpl on the app + // thread in EGL call order, which is the order the server's context table needs. NoWire + // (before ClientSession::Start, or on the server role's own thread) means keep the context on + // the client's books alone. Refused is a server answer: eglCreateContext then returns + // EGL_NO_CONTEXT, because the server would refuse that context's first bind_context. enum class ContextFrameEmit : Uint8 { Sent, NoWire, Refused }; ContextFrameEmit SendCreateContextFrame(Uint64 clientContextToken, Uint64 shareGroupToken, Uint32 flags); // A destroy has the same three answers; the client's EGL book is dropped either way (that is diff --git a/MobileGL/MG_Remote/FatalFunnel.h b/MobileGL/MG_Remote/FatalFunnel.h index b542bc39..addf7c02 100644 --- a/MobileGL/MG_Remote/FatalFunnel.h +++ b/MobileGL/MG_Remote/FatalFunnel.h @@ -6,24 +6,12 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P6 `dl` (CONTRACT-P6 5.2). THE ONE PLACE A SESSION DIES. -// -// Every `Fatal{...}` under MG_Remote/ used to be an MGLOG_F followed by its own std::abort(). -// a6 counted ~92 of them. Scattered, they gave the funnel's three benefits to nobody: there was -// no single place to publish a SessionFault to the peer before dying, no single telemetry point, -// and no way for a census to be sure a new death was named. This is that place. -// -// THE MESSAGE STRING IS PASSED VERBATIM, family word and all. The contract requires the log -// lines, the CI greps and every red-once to be byte-identical, so SessionFail does not compose -// the line from the family - it takes the SAME string the site used to hand MGLOG_F and forwards -// it unchanged. The family enum rides ALONGSIDE, redundantly, so the funnel knows the coarse wire -// FatalCode without parsing the string; a test asserts the enum's name appears in the string, so -// the redundancy cannot rot into disagreement. -// -// NOT in WireLog.h: that header is deliberately dependency-light (ITransport.h's rule, the -// include-graph purity gate), and this needs FatalFamily.h, which pulls the protocol header. The -// header-layer sites that must stay pure keep using WireLogFatal, whose own abort routes through -// the same publisher in WireLog.cpp. +// The one place a session dies. SessionFail logs the line, publishes a SessionFault to the peer, +// counts it and aborts, so every Fatal under MG_Remote/ is named and counted. The message is +// forwarded verbatim, family word included, because log greps and the fatal census match it; the +// MGFatalFamily argument gives the wire FatalCode (ARCHITECTURE.md §15.3.1) without parsing the +// string, and a test asserts the family's name appears in it. Header-only Transport code, which +// must not include FatalFamily.h, uses WireLogFatal, whose abort reaches the same publisher. #pragma once @@ -48,35 +36,14 @@ namespace MobileGL::MG_Remote { // latched fault (SessionLatch below) counts here too: it ends the session just as surely. ::std::uint64_t SessionFaultCount(); - // ---- PH-1 (3), ID-P7-1: THE PER-SESSION LATCH --------------------------------------- - // - // WHAT IT IS. At a site whose input bytes the PEER wrote, and which already sits in a - // function that can say "no" (Bool / MobileGLResult), a named fault no longer has to kill the - // process: SessionLatch logs the SAME line SessionFail would (family word and all, so every - // `Fatal{` grep, the census and the SessionFault frame read exactly as before), publishes the - // SessionFault to the peer, records the FIRST fault (family + line), and RETURNS false. The - // site returns failure, the decoder declines, ServerLoop::DrainRing stops applying at its - // next check, the apply thread leaves its loop, and ServerMain::RunSession closes the session - // cleanly and exits kSessionLatchedExitCode - a named, counted end the supervisor reaps and - // then serves the next connection, exactly as P6.5's fork-per-session already does for a - // crash (ID-P7-1: "下一连接照常服务" is the supervisor's, not the latch's). - // - // WHERE IT IS ARMED, AND WHY ONLY THERE. ArmSessionLatch() is called by exactly one site: - // ServerMain::RunSession, i.e. a served session - the spawn / TCP session child (with or - // without --serve), whose process IS the session, and (P14 S2) each session the in-process - // display server runs, which arms ITS OWN latch domain so a fault there ends that session and - // no other. Unarmed, SessionLatch IS SessionFail - same line, same abort - and - // that is the answer everywhere else, deliberately: - // * INPROC keeps Fatal. CONTRACT-P5's R-2 arms (rules A/B/C, the four honesty arms) are - // Fatal{ProtocolCorruption} in the formal wording, and under inproc the client and the - // server share one process: a latched-but-alive server would leave the client's GL thread - // parked on a barrier in the SAME process with no EOF to wake it, and there is no - // supervisor to serve a "next connection" - the process is the session. So the latch - // changes nothing inproc and every death test stays a death test. - // * THE CLIENT never arms it: a client-side arm of a shared helper (the encoder's honesty - // pass) is a local bug, not a peer's bytes. - // The census (scripts/ci/fatal_census.py rule 2) checks SessionLatch calls the way it checks - // SessionFail calls: the string must carry its `Fatal{Word` and the word must have a .def row. + // ---- per-session latch --------------------------------------------------------------- + // At a site whose input bytes the peer wrote and that can already return failure, an armed + // SessionLatch logs SessionFail's line, publishes the SessionFault, records the first fault + // and returns false; DrainRing stops applying and RunSession ends the session with + // kSessionLatchedExitCode. Only ServerMain::RunSession arms it, in the session's latch domain. + // Unarmed it is SessionFail: inproc keeps every Fatal, since a latched server would leave the + // client's GL thread parked in the same process, and the client never arms it. + // scripts/ci/fatal_census.py checks SessionLatch strings like SessionFail's. void ArmSessionLatch(); bool SessionLatchArmed(); // P12 (D5): THE IN-PROCESS DISPLAY SERVER RUNS SESSIONS ONE AFTER ANOTHER IN ONE PROCESS, so @@ -101,22 +68,14 @@ namespace MobileGL::MG_Remote { // Every fault this armed process latched or declined after the first (>= 1 once latched). ::std::uint64_t SessionLatchCount(); - // ---- P14 S2: ONE LATCH DOMAIN PER SERVED SESSION ------------------------------------ - // - // THE LATCH IS PER SESSION NOW, and that is a correctness requirement of serving several in - // one process, not a tidy-up. Its three readers sit on a session's own threads - the apply - // loop's park, DrainRing's per-pop check, ServerMain's control loop - so a latch raised by - // session A used to stop session B's apply thread mid-ring and end B's control loop: one - // peer's bad bytes took down its neighbours. ResetSessionLatch had the mirror problem: the - // reap of a finished session cleared the latch a live one was standing on. - // - // A DOMAIN is the latch state, and a THREAD IS BOUND TO ONE. SessionRuntime opens a domain per - // served session and ThreadSessionScope binds both of that session's threads to it. A thread - // that has bound nothing uses DOMAIN 0, the process default - which is exactly the old - // behaviour, and is what the client, the inproc one-process shape and every unit case get. - // - // 0 IS RESERVED FOR THE PROCESS DEFAULT and FatalLatchDomainOpen never returns it (0 on - // exhaustion, which degrades to "shares the default" rather than to a crash). + // ---- one latch domain per served session --------------------------------------------- + // The latch is per session so one peer's bad bytes cannot stop a neighbour in the same + // process: its readers (the apply loop, DrainRing, ServerMain's control loop) run on the + // session's own threads. SessionRuntime opens a domain per served session and + // ThreadSessionScope binds both of its threads to it; an unbound thread uses domain 0, the + // process default (client, inproc, unit tests). FatalLatchDomainOpen returns 0 only when + // domains run out, and the session then shares the default. + // See docs/Disaggregated/design/11-state-ownership.md. inline constexpr int kMaxLatchDomains = 64; int FatalLatchDomainOpen(); // The domain's state goes; no thread may still be bound to it (the session's apply thread is diff --git a/MobileGL/MG_Remote/Handshake.h b/MobileGL/MG_Remote/Handshake.h index 552476d6..76101d44 100644 --- a/MobileGL/MG_Remote/Handshake.h +++ b/MobileGL/MG_Remote/Handshake.h @@ -49,23 +49,14 @@ namespace MobileGL::MG_Remote { return true; } - // PH-7 (1)(2), ID-P7-3. THE TOKEN POLICY, ONCE, FOR BOTH SERVER-SIDE SITES. - // - // ServerSession::Accept and ServerMain::RunSession each had their own reading of it (see - // Transport/AuthToken.h for what the three readings were). They now call this, so the answer - // to "is this peer allowed to open a session" is one function with one comparison, and it is - // the constant-time one. - // - // THE FOUR STATES, stated here because they were implicit and therefore different at each - // site: - // in-process - there is no peer; the "connection" is this process. Never authenticated. - // no token - not an empty credential: the LISTEN policy has already confined this - // server to loopback (SocketTransport.cpp's ListenTcp), so there is nothing - // left for a token to add. A peer that sends one anyway is not refused for - // it; ServerMain used to do that and no document ever said so. + // The one token check every server-side site uses (ServerSession::Accept, ServerMain's + // session and pre-auth paths, PairAcceptor), with a constant-time comparison: + // in-process - no peer; never authenticated. + // no token set - the listen policy already confines the server to loopback + // (SocketTransport.cpp's ListenTcp); a token the peer sends is not refused. // token, match - welcomed. - // token, mismatch - `Refuse{Authentication}`, detail "token mismatch", and the value is - // not in the line. A missing token field is a mismatch, not a separate case. + // token, mismatch - Refuse{Authentication}, detail "token mismatch", the value never logged. + // A missing token field is a mismatch. inline MobileGLResult AuthenticatePeerToken(Transport::ITransport& transport, const ::flatbuffers::String* presented) { if (transport.Role() == Transport::TransportRole::InProcess) return MOBILEGL_OK; diff --git a/MobileGL/MG_Remote/Protocol/SurfaceOpCodec.h b/MobileGL/MG_Remote/Protocol/SurfaceOpCodec.h index 23401742..ab7dd9a2 100644 --- a/MobileGL/MG_Remote/Protocol/SurfaceOpCodec.h +++ b/MobileGL/MG_Remote/Protocol/SurfaceOpCodec.h @@ -6,38 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P5f, package fc: the codec between the inproc SurfaceControlFrame and the wire's -// ::MobileGL::Wire::SurfaceOp / ::MobileGL::Wire::SurfaceReply (protocol.fbs). -// -// The frame is the inproc payload (Server/SurfaceControlFrame.h); this is the ONLY place the -// frame meets the schema. Nothing in the inproc hot path encodes - the frame crosses the one-slot -// channel by value - but the codec is what makes "the same struct encodes under spawn" a tested -// claim rather than a plan, and it owns the two schema seams that are NOT integer translations: -// -// * SurfaceControlOp <-> ::MobileGL::Wire::SurfaceOpKind: numerically aligned for the ten framed ops and -// PINNED by static_asserts in the .cpp, but mapped through an explicit table. The four -// inproc-only kinds have no wire kind and refuse to encode by name. -// * MG_Backend::WindowBackend <-> ::MobileGL::Wire::WindowKind: the two enums do not agree (WindowKind's -// Surfaceless/Pbuffer are surface shapes, not window backends; WindowBackend's MetalLayer had -// no wire enumerator until fc added it), so the mapping is a table with named failures, never -// a cast (f0-egl census, schema gap 3). -// -// ERROR SEMANTICS. Decode is pure: it answers a SurfaceWireError. The FATALS live at the server -// entry point, ServerApplyWireSurfaceOp, because "a wire op the server cannot honour" is a -// contract violation, not a data problem: AndroidNativeWindowArrived dies -// Fatal{UnmigratedSurface, "AndroidNativeWindow:use-MOBILEGL_IPC_SURFACE=server"} (the ANativeWindow* means nothing in the -// server's process, permanently - a windowed client uses MOBILEGL_IPC_SURFACE=server) and everything else dies -// Fatal{ProtocolCorruption, "SurfaceOp"}. P6's control pump is the intended caller; P5f's unit -// tests drive it directly. MetalLayerArrived likewise dies -// Fatal{UnmigratedSurface, "MetalLayer:use-MOBILEGL_IPC_SURFACE=server"}: CAMetalLayer* is also process-local. -// -// P12 (on-screen server window). WindowKind::ServerOwned is the one window kind whose window is -// NOT the client's: the server substitutes its own (ServerLoop.cpp's ServerOwned arm). The codec -// maps it onto the frame-local Server::kServerOwnedWindowBackend and writes/reads nativeToken 0, -// always - so the client's window value never reaches the wire. A ServerOwned op with a non-zero -// token is a peer's corrupt bytes (Fatal{ProtocolCorruption, "SurfaceOp.nativeToken"}, latched); -// SetWindowHandle naming it is a named refusal (SurfaceRefusal::ServerOwnedOnSetWindowHandle), -// answered and not latched. +// The codec between the in-process SurfaceControlFrame and the wire's SurfaceOp / SurfaceReply +// (protocol.fbs); the only place the frame meets the schema. Both enum seams are explicit tables +// with named failures, never casts: SurfaceControlOp <-> SurfaceOpKind (in-process-only kinds +// refuse to encode) and WindowBackend <-> WindowKind (Surfaceless/Pbuffer are surface shapes). +// Decode is pure and returns a SurfaceWireError; the Fatals are in ServerApplyWireSurfaceOp: a +// native window or Metal layer pointer is process-local (Fatal{UnmigratedSurface}; use +// MOBILEGL_IPC_SURFACE=server). WindowKind::ServerOwned always carries nativeToken 0; a non-zero +// token is a latched ProtocolCorruption, and ServerOwned on SetWindowHandle a named refusal. #pragma once #include diff --git a/MobileGL/MG_Remote/Protocol/mg_protocol_base.h b/MobileGL/MG_Remote/Protocol/mg_protocol_base.h index 016761c7..693bf6a1 100644 --- a/MobileGL/MG_Remote/Protocol/mg_protocol_base.h +++ b/MobileGL/MG_Remote/Protocol/mg_protocol_base.h @@ -6,32 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// Shared vocabulary of the MG_Remote wire contracts (transport, framing, ring, -// shm). Inherited from the earlier `Feat/CS-Delta-IPC` branch -// (MobileGL/Protocol/mg_protocol_base.h) and cut down to what plan B's -// transport actually needs: result codes, byte spans, a shm region reference -// and the id typedefs. -// -// Deliberately NOT inherited: MobileGLObjectKind / MobileGLObjectScope / -// MobileGLObjectHandle. Plan B does not put GL object identity on the wire at -// all - the frontend allocates {slot, generation} handles in MG_Pipe -// (PLAN-B.md section 4.2.1) and those are the only identity the backend ever -// sees, so a second object-identity vocabulary here would be a drift surface -// with no reader. -// -// This header must stay: -// - pure C (compilable from C and C++, no MG C++ types, no exceptions/RTTI), -// - dependency-free (only //), -// - append-only within an ABI major (see versioning rules below). -// -// Versioning rules (contract-wide): -// - Every versioned struct starts with uint32_t structSize. -// - Appending fields at the tail is a MINOR bump; receivers must ignore -// bytes beyond the structSize they know. -// - Changing/removing/reordering existing fields is a MAJOR bump. -// - A major mismatch is a hard, structured failure, never an exception. -// (Plan B keeps the structSize-first discipline as the answer to risk B-R10, -// PLAN-B.md section 14.2.) +// Shared vocabulary of the MG_Remote wire contracts (transport, framing, ring, shm): result +// codes, byte spans, a shm region reference and id typedefs. No GL object identity lives here; +// the wire's only object identity is MG_Pipe's {slot, gen} handle (ARCHITECTURE.md §2.1). +// Keep this header pure C, dependent only on //, and +// append-only within an ABI major. Every versioned struct starts with uint32_t structSize. +// Appending fields is a minor bump and receivers ignore bytes past the structSize they know; +// changing, removing or reordering fields is a major bump, and a major mismatch is a hard, +// structured failure. #ifndef MOBILEGL_REMOTE_PROTOCOL_BASE_H #define MOBILEGL_REMOTE_PROTOCOL_BASE_H @@ -51,27 +33,13 @@ extern "C" { #define MOBILEGL_PROTOCOL_ABI_MAJOR 1 #define MOBILEGL_PROTOCOL_ABI_MINOR 0 -// The CONTROL schema's revision (protocol.fbs), mixed into wireFingerprint. The fingerprint's -// other inputs are struct sizes and codec digests of the DATA plane; nothing moved it when a -// control message changed shape, so a peer built before a control-schema change agreed with one -// built after it and the difference surfaced as whatever the first misread field did. Bump this -// with every protocol.fbs change a peer must agree on. -// -// THE PIN. This is a hand-bumped integer, and the bump is the only thing that moves the -// fingerprint - so a protocol.fbs edit that forgot it would put old and new peers back in -// silent agreement. scripts/ci/protocol_revision_pin.py holds {revision -> sha256(protocol.fbs)} -// (protocol_revision_pins.json) and refuses a schema whose digest is not the current revision's; -// it runs as ProtocolSchema.RevisionPinsDigest (label unit) and in CI's flatc-check job. After a -// bump: `python3 scripts/ci/protocol_revision_pin.py --write` records the new row. -// 1 P7 wave 2-F, PH-7 (4): Welcome.dataNonce and the DataBind message. -// 2 P7 (p7/spawnhang): the SurfaceProgress message. -// 3 P12 (on-screen server window): WindowKind.ServerOwned, the SurfaceRefusal enum and -// SurfaceReply.width/height/refusal. -// 4 P11 PAIR: the PairBind message - a socket client's control and aux connections pair by -// the nonce each presents first, not by arrival order. -// 5 P11 B2: LinkTerms.adoptTier - the client's persistent-map adoption tier ask (T0 or T2). -// 6 P14 S1 (docs/Disaggregated/design/11-state-ownership.md): SurfaceOpKind.CreateContext / -// DestroyContext, plus SurfaceOp.shareGroupToken and SurfaceOp.contextFlags. +// The control schema's revision (protocol.fbs), mixed into wireFingerprint; the fingerprint's +// other inputs cover only the data plane. Bump it with every protocol.fbs change a peer must +// agree on, or old and new peers handshake and then misread a field. +// scripts/ci/protocol_revision_pin.py pins each revision to sha256(protocol.fbs) +// (protocol_revision_pins.json) and fails a schema whose digest is not the current revision's; +// it runs as the ProtocolSchema.RevisionPinsDigest test and in CI's flatc-check job. After a +// bump, record the new row with `python3 scripts/ci/protocol_revision_pin.py --write`. #define MOBILEGL_PROTOCOL_CONTROL_REVISION 6 #define MOBILEGL_ABI_VERSION(major, minor) (((uint32_t)(major) << 16) | (uint32_t)(minor)) diff --git a/MobileGL/MG_Remote/Server/AdoptInbox.h b/MobileGL/MG_Remote/Server/AdoptInbox.h index 85ded246..cbd9d090 100644 --- a/MobileGL/MG_Remote/Server/AdoptInbox.h +++ b/MobileGL/MG_Remote/Server/AdoptInbox.h @@ -6,22 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P11 B2 (docs/archive/contracts/CONTRACT-P11.md B2): the server's end of T0's descriptor channel. -// -// WHO READS THE FD, AND WHY IT CANNOT DEADLOCK THE APPLY THREAD. The APPLY THREAD reads it, -// inline, when it decodes the map_persistent record whose seq the Offer names - nothing else reads -// a session's aux socket after the handshake, so there is no second reader to race. The wait -// cannot deadlock because of the client's order: it queues the Offer (ShareFd returns once the -// kernel - or the in-process queue - holds it) BEFORE it publishes the record, so when the apply -// thread sees the record the descriptor is already there, and the wait depends on nothing the -// client has still to do. A client emits at most one T0 map_persistent at a time (the row blocks -// on its reply), so at most one Offer is ever outstanding and the aux queue (10 datagrams on an -// Android kernel) cannot fill. The wait is still bounded (AdoptT0::kOfferWaitMs): an Offer that -// never comes is a named DECLINED, never a hang. -// -// Offers are matched by seq, not by arrival order. One that arrives for a LATER record is kept -// for it; one for an EARLIER record (its record already timed out and was declined) is closed -// and counted as stale. +// The server's end of the aux-socket descriptor channel: T0 map_persistent Offers and shared-image +// descriptors, matched by record seq and told apart by their sideband's magic. Only the apply +// thread reads it, inline, when it decodes the record the descriptor belongs to. It cannot +// deadlock: the client queues the descriptor before it publishes the record, and a T0 +// map_persistent blocks on its reply, so at most one Offer is outstanding (an Android aux queue +// holds 10). The wait is still bounded (AdoptT0::kOfferWaitMs); a missing Offer is a named +// DECLINED. A descriptor for a later record is kept for it; one for an earlier, already declined +// record is closed and counted as stale. #pragma once diff --git a/MobileGL/MG_Remote/Server/ApplyThreadPolicy.h b/MobileGL/MG_Remote/Server/ApplyThreadPolicy.h index 171daaf0..8f7b8900 100644 --- a/MobileGL/MG_Remote/Server/ApplyThreadPolicy.h +++ b/MobileGL/MG_Remote/Server/ApplyThreadPolicy.h @@ -6,36 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P11 B1 (CONTRACT-P11 B1): WHERE THE APPLY THREAD RUNS AND HOW LONG IT SPINS, CHOSEN PER SESSION -// FROM WHO THE CLIENT IS AND WHICH DATA PLANE IT USES. -// -// `auto` used to mean one thing for every session: the apply thread pinned to every big core -// (DetectBigCoreMask, 0xc0 on the workload device) and spinning MOBILEGL_IPC_SPIN_US before each -// park. That is right for inproc and the same-app fork spawn (A4's measured shapes), and for a client -// on the stream plane, whose records an io thread feeds to the apply thread. It is wrong for a -// client that DIALLED IN from another process over SHARED SEGMENTS - an app_process-launched -// program (the fd: endpoint), a Termux one, a root or same-app one on `unix:`: the apply thread -// spins on the ring that client writes directly, the client's GL thread lands on the same two prime -// cores, and on the Redmi rd12 ran 23-26 fps over shared memory against 45-54 over tcp, 118 when the -// two were placed apart by hand (B0-CROSS-APP.md, HSPIKE.md). So `auto` now reads the peer's Hello: -// -// peer, plane `auto` resolves to -// in-process (No) every big core, the configured spin - unchanged -// forked (Fork) every big core, the configured spin - unchanged -// dialled in (Connect), stream every big core, the configured spin - unchanged -// dialled in, shared segments ONE core, ReservedApplyCore's: the lowest of several prime cores, -// a big core beside a lone prime core, or none - never the only -// prime core - and kConnectedPeerSpinUs unless MOBILEGL_IPC_SPIN_US -// -// The lowest prime core, not the highest, by measurement (CONTRACT-P11 B1): the scheduler puts a -// foreground app's hot GL thread on the HIGHEST prime core (cpu7 in 43 of 44 samples); pinning the -// apply thread there shared that core with it (rd12 6.5 fps), pinning it to the lowest gave 120. A -// client launched through ExternalClientHelper is also kept off the reserved core (the supervisor -// announces it, the broker hands it to the helper), so a client the scheduler would otherwise place -// on it - an adb-shell one - is not starved either. -// -// An explicit mask or `off` wins for every peer, exactly as before: the operator's word is the -// policy. A pure function, so the unit suite can hold every row without a thread or a cpufreq tree. +// Where the apply thread runs and how long it spins before parking, chosen per session from the +// peer (the Hello's DialMode) and the data plane. With MOBILEGL_IPC_SERVER_AFFINITY=auto an +// in-process, forked or stream-plane peer gets every big core and the configured spin; a peer +// that dialled in over shared segments gets at most one core (ReservedApplyCore), never the only +// prime core, and kConnectedPeerSpinUs unless MOBILEGL_IPC_SPIN_US is set, because it writes the +// ring the apply thread spins on and its GL thread wants a prime core. An explicit mask or `off` +// wins for every peer. Pure functions, so the unit suite covers every row; the measurements are +// in ARCHITECTURE.md §14.1. #pragma once diff --git a/MobileGL/MG_Remote/Server/DisplayServerJni.cpp b/MobileGL/MG_Remote/Server/DisplayServerJni.cpp index ee953abb..b9ca75e0 100644 --- a/MobileGL/MG_Remote/Server/DisplayServerJni.cpp +++ b/MobileGL/MG_Remote/Server/DisplayServerJni.cpp @@ -6,35 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P12 (on-screen server window), D3/D5/D6/D7. THE JNI GLUE OF THE IN-PROCESS DISPLAY SERVER. -// -// The APK's MobileGLDisplayActivity (android:process=":mglwin") owns a SurfaceView and runs -// the TCP server on a thread of its own process, so a headless client (MOBILEGL_IPC_SURFACE=server) -// renders straight onto that SurfaceView. This file is the whole native half of that Activity: -// -// nativeInstallDisplay ServerDisplayInstance().Install(): ANativeWindow acquire/release plus the -// geometry up-call below. Before the server serves. -// nativeServe mobilegl_server_serve_inprocess(listen) on the CALLING (Java) thread; -// returns its exit code when the server stops or fails. -// nativeStop mobilegl_server_stop_inprocess(): non-blocking, safe on the UI thread. -// nativeAttachWindow surfaceCreated / surfaceChanged: ANativeWindow_fromSurface, Attach (which -// takes its OWN reference), then this call's reference is released. -// nativeDetachWindow surfaceDestroyed: ServerDisplay::Detach, which BLOCKS (bounded) until the -// session rendering into the window has released its backend surface on the -// apply thread. Returns the result's name for the Activity's log. -// nativeUninstallDisplay after the server returned: the display is gone. -// -// THE GEOMETRY UP-CALL. ServerDisplay calls RequestGeometry on the APPLY THREAD, from AcquireFor, -// during a ServerOwned CreateWindowSurface, with the size the client asked for. That thread is a -// native thread the JVM does not know, so it is attached for the call and detached after it (a -// native thread that exits while still attached aborts the runtime). The Java side only POSTS the -// request to the UI thread (SurfaceHolder.setFixedSize / setSizeFromLayout must run there) and -// returns: this must not block and must not call back into ServerDisplay. The class and method are -// resolved here, on the Java thread that installs the display, because a native thread's FindClass -// would search the system class loader, not the app's. -// -// Compiled only for Android with MOBILEGL_BUILD_DISAGGREGATED (CMakeLists.txt, beside -// DriverPostJni.cpp); the guard keeps it inert anywhere else. +// The JNI glue of the in-process display server: the native half of MobileGLDisplayActivity +// (process ":mglwin"), which owns a SurfaceView and runs the TCP server in its own process so a +// headless client (MOBILEGL_IPC_SURFACE=server) renders onto it. nativeInstallDisplay and +// nativeUninstallDisplay bracket nativeServe (blocking, on the calling Java thread); nativeStop +// does not block; nativeDetachWindow blocks (bounded) until the session releases the window. +// RequestGeometry runs on the apply thread, attached to the JVM for the call only, and must not +// block or call back into ServerDisplay (Java only posts to the UI thread). Its class and method +// are resolved at install: a native thread's FindClass would miss the app's classes. #ifdef __ANDROID__ #include "InProcessServer.h" diff --git a/MobileGL/MG_Remote/Server/InProcessServer.h b/MobileGL/MG_Remote/Server/InProcessServer.h index 0eda5e4a..d39aa9c5 100644 --- a/MobileGL/MG_Remote/Server/InProcessServer.h +++ b/MobileGL/MG_Remote/Server/InProcessServer.h @@ -6,27 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P12 (on-screen server window), D5. THE IN-PROCESS DISPLAY SERVER'S ENTRY POINTS. -// -// The on-screen server cannot be the exec'd supervisor that forks a child per session: the window -// it renders into (the display Activity's SurfaceView) is an ANativeWindow in the APP's process, and -// neither exec nor fork carries one. So the Activity's own process serves: a thread it starts calls -// mobilegl_server_serve_inprocess, which is `libMobileGLServer.so tcp://... --serve` with THREADS -// instead of fork - the same pre-auth, the same Busy for an authenticated second Hello while a -// session is live, one session at a time, each session a thread of this process, the latch reset -// between sessions and the backend pinned for the process lifetime. Crash isolation is what it -// gives up: a session's abort (an unarmed SessionFail) takes this process - the Activity - with it. -// -// PRECONDITIONS, the same as mobilegl_server_main's, set in the environment BEFORE libMobileGL is -// loaded (Os.setenv from Java): MOBILEGL_IPC_DIAL=no (else 64), none of MOBILEGL_TRANSPORT / -// MOBILEGL_IPC_SERVER_PATH / MOBILEGL_IPC_RING_MB / MOBILEGL_IPC_STAGE_MB (else 65), -// MOBILEGL_IPC_ROLE=server (forced if missing, as the supervisor does), MOBILEGL_IPC_TOKEN (required -// for a non-loopback listen; >= the minimum length), optionally MOBILEGL_BACKEND_TYPE (the pin), -// MOBILEGL_IPC_LOG_FORWARD, MOBILEGL_LOG_FILE_PATH. -// -// THE DISPLAY is separate: the JNI glue installs ServerDisplayInstance() (ServerDisplay.h) and -// attaches/detaches the window from the SurfaceHolder callbacks. A server with no display installed -// serves offscreen sessions only and refuses a ServerOwned surface by name. +// Entry points of the in-process display server (ARCHITECTURE.md §15.2). Its window lives in the +// app's process and survives neither fork nor exec, so a thread of that process serves, with a +// thread per session instead of a fork: tcp:// runs the --serve supervisor (pre-auth, one session, +// Busy), a unix endpoint up to MOBILEGL_IPC_INPROC_MAX_SESSIONS at once. Latches reset between +// sessions, the backend is pinned for the process, and there is no crash isolation. +// Set before libMobileGL loads: MOBILEGL_IPC_DIAL=no (else 64); none of MOBILEGL_TRANSPORT, +// MOBILEGL_IPC_SERVER_PATH, _RING_MB or _STAGE_MB (else 65); MOBILEGL_IPC_TOKEN for a non-loopback +// listen. The JNI glue installs ServerDisplayInstance(); without one, ServerOwned is refused. #pragma once diff --git a/MobileGL/MG_Remote/Server/PairAcceptor.h b/MobileGL/MG_Remote/Server/PairAcceptor.h index bc3cfbf0..3c1b6e71 100644 --- a/MobileGL/MG_Remote/Server/PairAcceptor.h +++ b/MobileGL/MG_Remote/Server/PairAcceptor.h @@ -6,38 +6,13 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P11 PAIR. A SOCKET CLIENT'S TWO CONNECTIONS ARE PAIRED BY WHO THEY ARE, NOT BY WHEN THEY ARRIVED. -// -// SocketTransport::ConnectTo opens two connections to one listener - control, then aux (the -// SCM_RIGHTS socket; on tcp:// the data connection). The server used to take "the next two accepted -// connections, first is control" (SocketTransport::AcceptPair). That was a protocol only while -// nothing else could reach the listener, and on the device it was not (B0-CROSS-APP.md F2): the -// server app's readiness probe - one connect() and close() - became the next client's control -// connection, that session read the probe's socket and ended ("control peer closed before sending -// a first frame"), and the client got no Welcome. Two clients connecting at the same moment could -// just as well swap halves. -// -// Now each connection's first frame is a PairBind (protocol.fbs, control revision 4): the client's -// nonce, the same on both, and which half it is. This acceptor keeps every accepted connection in a -// small set until it has presented one, and hands out a control and an aux connection only when -// their nonces match. What it does with everything else: -// - a connection that closes having sent nothing (a readiness probe) is closed, quietly; -// - a first frame that is not a PairBind is refused by name. A Hello is a client from before -// PairBind: it is asked what the session would ask, in the session's order - the token, then the -// wire (Handshake.h) - so it hears Refuse{WireFingerprint} as it would from any session, and one -// that passes both is still refused, because it cannot be paired; -// - a connection that has not presented a PairBind, or whose partner has not presented the same -// nonce, within the budget is dropped - a control connection with a refusal, an aux connection -// (whose client reads descriptors there, not frames) by closing it - and nobody else's -// connection is touched; -// - past kPairMaxUnpaired connections the oldest one that has not identified itself (else the -// oldest) is dropped the same way. -// -// It never reads a byte past a PairBind (FirstFrameAssembler): on control the Hello follows it, and -// the session reads that itself; on aux the descriptors follow it. -// -// Stateful because pairs complete out of order: with A-control, B-control, B-aux, A-aux, B is -// handed out first and A-control has to still be here for the call that hands out A. +// Pairs a socket client's two connections (control, then aux: the SCM_RIGHTS socket, or the data +// connection on tcp://) by nonce rather than arrival order, so a readiness probe or two clients +// connecting at once cannot be mispaired. Each connection's first frame is a PairBind with the +// client's nonce and its half; nothing past it is read. A connection that closes having sent +// nothing is closed quietly; a Hello is checked token then wire, as a session would, then refused; +// one unpaired after the budget, or the oldest unidentified one past kPairMaxUnpaired, is dropped +// without touching others. Stateful because pairs complete out of order. #pragma once diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index 9ec0fcfb..d73d7892 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -206,22 +206,14 @@ namespace MobileGL::MG_Remote::Server { m_unreportedFences.clear(); } - // P10 (CONTRACT-P10.md §1): WHICH OF THE CLIENT'S FENCES HAS THE GPU FINISHED. - // - // The client answers a poll of a fence it knows is signaled without asking - that is the - // whole of package A - and it learns what is signaled only from here. Walked in CREATION - // order and stopped at the first one still pending: one context's GPU work completes in - // the order it was submitted, so a later fence cannot be done while an earlier one is not, - // and the steady-state cost is one status query per drain batch. (Were a backend ever to - // finish them out of order, the later fence would only be reported late - an answer the - // client then gets by its own round trip - never early.) - // - // `flush`: the apply thread is about to go idle. A fence whose commands the driver still - // holds never signals on its own, and an idle server submits nothing more, so the first - // pending fence is asked through ClientWaitSync(GL_SYNC_FLUSH_COMMANDS_BIT, 0) - the one - // backend-neutral way both backends take to submit what they hold - instead of the plain - // status query. Only when idle: mid-stream the next records flush soon enough, and a - // submit per batch would cost Magma its command-buffer batching. + // Reports which of the client's fences the GPU has finished; the client answers a poll of a + // reported fence without a round trip. Walked in creation order and stopped at the first + // pending one: one context's work completes in submission order, so this costs one status + // query per drain batch, and an out-of-order finish would be reported late, never early. + // `flush`: the apply thread is about to go idle, and a fence whose commands the driver still + // holds never signals on its own, so the first pending fence is queried with + // ClientWaitSync(GL_SYNC_FLUSH_COMMANDS_BIT, 0). Only when idle: a submit per batch would + // break Magma's command-buffer batching. Uint32 ServerVerbSink::ReportSignaledFences(Bool flush) { if (m_unreportedFences.empty()) return 0; ServerSession* session = ServerSession::Active(); @@ -719,44 +711,14 @@ namespace MobileGL::MG_Remote::Server { // dropped before this function returns, and s1's SessionConsumer::ApplyOne publishes // appliedSeq only after that - so by the time the client is runnable the flag is down. const Client::ClientSession::ScopedApplierEntry insideApplier; - // P5e (id), CONTRACT-P5E §2.1 / §4.4: STAMP WHETHER THE CLIENT IS PARKED BEHIND THIS - // RECORD, before anything can ask. It is the input to the allocator guard's exemption - // (SlotAllocator.cpp) and to every frontend-keyed twin member that survives as monolith - // glue (SlotTables.h), and it has to be up before DecodeAndApply because the sinks those - // reach are exactly the askers. MGPipeBarriered answers true for every record until ra - // lands the wait rule, so this line changes nothing this phase and is the line ra - // rebases onto rather than adds. - // - // AND THE STAMP IS `true` UNTIL ra LANDS THE CLIENT'S HALF (ID-103). The predicate - // describes what the client WILL do once EmitAndWaitTails follows the wait classes; - // today it still blocks after every record, so a `false` stamp here would withdraw the - // §4.4 exemptions from a probe the client's own wait still makes safe - a refusal with - // no defect behind it. The predicate is computed on every record all the same, so it is - // exercised for the whole phase rather than first run on the day it starts deciding. - // - // AND THE SERVER'S OWN CAPABILITY IS THE SECOND CONJUNCT (P5e gl, ID-111). The constant - // above says what the CLIENT will do once ra's wait rule is live; it says nothing about - // which server this is. kCapRunAheadApply is never published on DirectVulkan (ID-90, - // MGPipeRunAheadCapBitsFor), yet draw_vbo / blit / clear / launch_grid are all - // kWaitNone - so a stamp that read the build constant alone would, on the day it flips, - // have a MAGMA server mark every draw record UNBARRIERED while its client is still - // lockstep. CountBarrierPull (PipeInputs.cpp) is an unconditional Fatal on an - // unbarriered pull, no knob involved, and Magma's residual fill is its ONLY source for - // the seven pointer-backed fields: Magma would die on its first draw and take - // MagmaP7AllocatorDebtScope's exemption with it. - // - // BOTH PREDICATES ARE COMPUTED UNCONDITIONALLY, as arguments rather than as the arms of - // a short-circuit, which is ID-103's reason extended to the capability probe: they are - // exercised on every record for the whole phase rather than first running on the day - // they start deciding. - // - // PH-1 (3): BUT NOT BEFORE THE RECORD HAS BEEN ADMITTED. MGPipeBarriered reads payload - // fields (a DrawVbo's MGPDrawInfo::Flags) and the stamps index per-opcode tables, so a - // record whose ring header is shorter than its own type - or names no opcode - would be - // read past its end here, before DecodeAndApply's pre-gate could refuse it. In an armed - // session child the pre-gate therefore runs FIRST: a refused record latches by name and - // is declined with nothing stamped (ServerLoopTest's short-record case reads the stamp). - // Unarmed it admits everything and the generated gate keeps its death, as before. + // Stamp whether the client is parked behind this record before anything can ask: the + // barrier-pull check (PipeInputs.cpp) and the deferred-destroy queue (PipeFill.cpp) read + // it from inside DecodeAndApply. It is MGPipeBarriered's answer only when this server + // publishes run-ahead; otherwise its client waits after every record, so every record is + // stamped barriered. Both inputs are computed on every record so they stay exercised. + // Admission runs first: MGPipeBarriered reads payload fields and the stamps index + // per-opcode tables, so in a session that latches faults a short or unknown record is + // declined before anything is stamped. if (!m_decoder.AdmitOrDecline(record)) return false; const Bool wireSaysBarriered = MG_Pipe::MGPipeBarriered( static_cast(record.kind), record.payload, MG_Pipe::MGPipeApplier()); @@ -779,23 +741,10 @@ namespace MobileGL::MG_Remote::Server { // before it aborts on the FIRST field inside SyncRenderState. StampVerbBoundary(static_cast(record.kind)); const Bool applied = m_decoder.DecodeAndApply(record); - // AND THE CLEAR IS INSIDE ApplyOne, NOT AFTER THE DRAIN BATCH. That is not tidiness, - // it is the barrier invariant. s1's SessionConsumer::ApplyOne publishes appliedSeq the - // instant this returns, and publishing appliedSeq is what makes the CLIENT runnable - // again (R-1: the barrier waits on exactly that watermark). A clear that ran after the - // batch would therefore be a second writer of gPipeInputs while the client is already - // touching it - the one thing table 3 says may not be introduced before the barrier - // retires - and the first version of this file had it there. It was caught by - // AClearRecordCrossesAndIsStampedAsAVerbBoundary failing INTERMITTENTLY, which is what - // a race looks like from the outside. - // - // THE COST, STATED: a record that is NOT a verb boundary now applies with the flag - // disarmed, so a sticky forward pulled from inside such a record's applier is not - // counted in `rsp`. Closing that needs an "enter the applier" entry point beside - // MGPipeServerStampVerbBoundary that arms the flag WITHOUT re-stamping - re-stamping on - // a non-verb op is what p1 forbids outright - and PipeInputs.cpp is p1's file. Left for - // the integrator to sequence; it makes `rsp` larger, never smaller, so the number this - // phase reports is a floor. + // LeaveApplier runs here, inside ApplyOne, and not after the drain batch: + // SessionConsumer::ApplyOne publishes appliedSeq as soon as this returns, which makes the + // client runnable, so a later clear would write gPipeInputs while the client uses it. + // How a record that is not a verb boundary is read-checked: ARCHITECTURE.md §9.2.1. LeaveApplier(); return applied; } diff --git a/MobileGL/MG_Remote/Server/PipeApplier.h b/MobileGL/MG_Remote/Server/PipeApplier.h index 6691c188..3c6476a2 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.h +++ b/MobileGL/MG_Remote/Server/PipeApplier.h @@ -6,36 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// The server's applier bridge. Owner: package v1, with p1 for the stamp rule. Signatures by c0. -// -// IT IS A BRIDGE, NOT AN APPLIER. The applier already exists and is not edited by this phase: -// MG_Pipe/PipeApply.{h,cpp}, 37 MGPipeApply* free functions. This class owns the three things -// that only exist once records arrive over a wire rather than by direct call: -// -// 1. THE VERB STAMP. This is the phase's prerequisite, and it is not in the ROADMAP row. -// MGPipeApplyAccess deliberately does not stamp the poison generations -// (PipeInputs.h:612-618): "a stamp says the filler published this for THIS verb, which is -// the walk's statement, not the applier's". Under split the filler is in another role, so -// NOTHING stamps, every FilledGen[] stays 0, MGPipeInputFieldIsFresh returns false for -// everything, and a pure server aborts on the FIRST read inside SyncRenderState with -// Fatal{UnmigratedPipeInput, "GetRenderStateParameters@"} - before reaching any -// interesting case. So: the server stamps at the verb boundary. p1 defines what is -// stamped and for which verb; v1 places the call. Neither half works alone. -// -// 2. ACCEPTANCE. Four applier entry points return Bool - ResourceCreate, ResourceRespecify, -// ResourceSubData, SetTextureParams - and MapPersistent returns void*. Those returns are -// what the CLIENT gates destructive state changes on (clearing per-level dirty flags, -// latching parameters, adopting a pointer). They go back through the reply slot, id = -// record seq (R-3), and are collected in the barrier's existing wait (R-5). The client -// may not recompute any of them. -// -// 3. R-11, THE BORROWED-POINTER RULE. A SEG_STAGE run is valid from publish until retiredSeq -// passes the record naming it. NO APPLIER ENTRY POINT MAY HOLD A POINTER PAST ITS RETURN. -// The tree has exactly one violation and it is named: GLESBufferResource::hostBytes -// (Managers.h:839), written by Ops_H_SubData (Managers.cpp:1980-1983) and Ops_H_FlushRange -// (:2035), read by six later drains (:2000, :2062, :2080, :2111, :2741, :2843). Under split -// those two must copy into server-owned storage. MOBILEGL_IPC_AUDIT=1's 0xDD fill (R-2.5) -// is the mechanical control that says whether they did. +// The server's applier bridge. The appliers are MG_Pipe/PipeApply's MGPipeApply* functions; this +// class adds what exists only when records arrive over a wire: +// - the verb stamp: the server stamps PipeInputs itself at each verb boundary +// (StampVerbBoundary); the client's stamps cannot stand in for it (ARCHITECTURE.md §9.2.1); +// - acceptance: the Bool appliers and MapPersistent answer through the record's reply slot, and +// the client gates destructive state changes on that answer, never on its own guess; +// - borrowed pointers: a SEG_STAGE run is valid until retiredSeq passes its record, so no applier +// keeps a pointer past its return; MOBILEGL_IPC_AUDIT=1 poisons retired bytes to prove it. #pragma once #include @@ -63,22 +41,12 @@ namespace MobileGL::MG_Remote::Server { // action under test is the probe itself). Bool MGPipeServerPublishesRunAhead(); - // ---- the barriered stamp, AS A PURE FUNCTION (ID-111) --------------------------------- - // - // ApplyOne stamps "is the client parked behind this record" before anything can ask - // (CONTRACT-P5E §2.1 / §4.4). Until the P5e integration commit the answer is `true` for - // every record whatever the wire says, because the client still blocks after each one - // (ID-103) - but the day the constant flips, the answer must ALSO ask whether this server - // published the run-ahead bit at all. It is `wireSaysBarriered` only when BOTH halves of - // the run-ahead arm are true; Magma publishes no bit 10, so a Magma server keeps stamping - // `true` and its lockstep client keeps being safe. Without the second conjunct a Magma - // server would stamp every draw_vbo / blit / clear / launch_grid UNBARRIERED (they are all - // kWaitNone) while its client is still parked, and CountBarrierPull aborts unconditionally - // on an unbarriered pull - Magma dies on its first draw, no knob involved. - // - // IT IS A PURE FUNCTION for MGPipeRunAheadCapBitsFor's reason: "Magma never runs ahead" is - // then something a unit case can hold AT THE POST-FLIP VALUE of the constant, rather than a - // claim that first becomes testable on the day the integration commit throws the switch. + // The barriered stamp, as a pure function: ApplyOne stamps whether the client is parked + // behind this record before anything can ask. The wire's answer is used only when the + // client's wait rule is in force and this server publishes kCapRunAheadApply; otherwise every + // record is barriered. A server without that bit has a lockstep client, and stamping its + // kWaitNone draws unbarriered while that client is parked would make CountBarrierPull abort. + // Pure so a unit test can pin "no run-ahead bit, never runs ahead" for any input. constexpr Bool MGPipeApplierStampsBarriered(Bool clientWaitRuleLanded, Bool serverPublishesRunAhead, Bool wireSaysBarriered) { diff --git a/MobileGL/MG_Remote/Server/PreAuthGate.h b/MobileGL/MG_Remote/Server/PreAuthGate.h index 539879c2..47aea203 100644 --- a/MobileGL/MG_Remote/Server/PreAuthGate.h +++ b/MobileGL/MG_Remote/Server/PreAuthGate.h @@ -6,49 +6,13 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// PH-7 (5), CONTRACT-P7 §12, docs/archive/notes/p7/ph-f.md §6.4. THE TCP SUPERVISOR AUTHENTICATES BEFORE IT -// FORKS, AND AN UNAUTHENTICATED PEER'S WORK IS BOUNDED. -// -// Two exposures were on the tree before wave 2-F and were recorded by its review: -// (a) the supervisor forked a session child for every accepted TCP connection and let the child -// read the Hello - so any peer that could reach the port cost the server a fork, and a -// silent one held that child for up to 10 s; -// (b) the child ran ValidatePeerHandshake before AuthenticatePeerToken, so an unauthenticated -// peer that sent a Hello with a wrong fingerprint was answered with Refuse{WireFingerprint} -// carrying THIS build's wire fingerprint (and Refuse{BuildFingerprint} its build stamp). -// -// The supervisor now reads every new connection's FIRST FRAME itself, without blocking on any one -// peer, under a short deadline; a Hello is authenticated there, token first, and only an -// authenticated Hello is ever forked for. The pieces that decide are here, free of I/O policy and -// of the supervisor's loop, so they are unit-testable (PreAuthGateTest): -// -// PreAuthKnobs the four knobs, read once from the environment; -// ClassifyFirstFrame what a complete first frame IS - the fuzz arm 1 malformations each land -// on a named shape here, and the identifier is asked before the verifier; -// FirstFrameAssembler reads EXACTLY one frame off a non-blocking socket, a few bytes at a time, -// and never a byte past it: a DataBind is followed on its connection by -// StreamLink's own frames, which belong to the session child; -// AuthBackoff failures per peer address, and the exponentially growing window during -// which that address is refused at accept, before a byte of it is read. -// -// What a failure IS, for the backoff: a Hello whose token did not match, a first frame that is not -// a well-formed control frame, a connection that let the deadline pass, and a connection that HELD -// its pending slot for kPreAuthProbeGraceMs or longer and then closed (or was displaced, below) -// without completing a frame. What it is NOT: a connection that closed within kPreAuthProbeGraceMs -// of its accept without sending a byte (a port probe costs the server an accept and nothing else), -// a DataBind that names no live session (a stale data connection of a session that just ended is a -// legitimate client's), and a refusal the backoff itself issued (counting those would let anybody -// behind a shared address keep that address locked out for ever). -// -// Fix round (the f2-auth critic's reproduction). The first cut counted no silent close at all, and -// the pending slots were one pool for everybody: one address could open MOBILEGL_IPC_PREAUTH_MAX -// silent connections, close them just before the deadline, open them again, and every Hello from -// anywhere was refused Busy for as long as it liked - no failure counted, no backoff ever. Two -// rules close that: a silent connection that held its slot past the grace is a failure (so the -// holder is backed off after MOBILEGL_IPC_AUTH_BACKOFF_AFTER of them), and a full queue is SHARED -// (ChoosePendingToDisplace): a newcomer from an address that holds fewer slots than the busiest -// address displaces the busiest address's oldest pending connection, so no one address can keep -// another out. +// Pre-authentication for the TCP supervisor. It reads each new connection's first frame itself, +// without blocking on any one peer and under a short deadline, checks the token first, and forks +// a session child only for an authenticated Hello. The deciding pieces are free of I/O policy so +// PreAuthGateTest can drive them: PreAuthKnobs (the environment knobs), ClassifyFirstFrame, +// FirstFrameAssembler (reads exactly one frame and never a byte past it: a DataBind's connection +// goes on with StreamLink frames that belong to the session child) and AuthBackoff. What counts +// as a failure and how the pending queue is shared: ARCHITECTURE.md §15.1.1. #pragma once diff --git a/MobileGL/MG_Remote/Server/ServerDisplay.h b/MobileGL/MG_Remote/Server/ServerDisplay.h index a3525d9a..043772a2 100644 --- a/MobileGL/MG_Remote/Server/ServerDisplay.h +++ b/MobileGL/MG_Remote/Server/ServerDisplay.h @@ -6,54 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P12 (on-screen server window), D3/D6. THE SERVER'S OWN WINDOW, AS ONE PROCESS-WIDE HOLDER. -// -// WHAT IT IS FOR. A client that sets MOBILEGL_IPC_SURFACE=server is headless: it asks the server to -// create its window surface on the SERVER's window (WindowKind::ServerOwned) and learns the -// surface's geometry back from the server. That window belongs to the in-process display server - -// on Android the display Activity's SurfaceView, handed in through JNI (the next stage's glue) - -// and this class is where it is held between the UI thread that owns the window's lifecycle -// (surfaceCreated / surfaceChanged / surfaceDestroyed) and the apply thread that renders into it. -// -// WHO HAS A DISPLAY. Nobody, until Install(): the Linux host server, the exec'd supervisor and -// every session child it forks never call it, so HasDisplay() is false there and a ServerOwned -// request is refused by name (ServerLoop.cpp's ServerOwned arm, SurfaceRefusal::NoServerDisplay). -// Only the in-process display server installs one. -// -// THE WINDOW IS OPAQUE HERE. It is a `void*` with three hooks - acquire and release (Android: -// ANativeWindow_acquire / ANativeWindow_release) and a geometry request (the native -> Java -// up-call: SurfaceHolder.setFixedSize(w, h) for w, h > 0, setSizeFromLayout() for 0/0). That is -// what lets the host unit tests drive attach / wait / detach with a fake handle, and it keeps this -// file free of any platform header. -// -// THE LEASE. The apply thread that substitutes the window into a backend takes a LEASE on it -// (AcquireFor) and names a hook that asks it to let go. Detach() - the surfaceDestroyed path - -// calls that hook and then BLOCKS, bounded, until the holder has released its backend surface and -// ended the lease (EndLease); only then is the window's reference released. That ordering is the -// GLSurfaceView contract: after surfaceDestroyed returns nothing may render into the window. A -// holder that does not answer in time does not get the window pulled out from under it - the -// reference is kept and released when the lease finally ends - and Detach returns TimedOut, by -// name, so the UI thread is never held longer than its bound. -// -// A LEASE THAT IS ENDING IS WAITED FOR, NOT REFUSED. A holder whose session is over - it latched a -// fault (a device loss among them) or is stopping - says so (MarkLeaseEnding) before its client can -// learn of it; its lease still lasts until its backend has let go of the window. A client that loses -// its session re-creates its window surface on a fresh session at once, typically while the old -// session's apply thread is still tearing its backend down: that request waits for the old lease -// to end, bounded by its own timeout, instead of being refused as LeasedElsewhere. A holder whose -// session goes on keeps the window exclusively, as before. -// -// SUSPEND, NOT END (screen off / lock). A holder whose backend can let go of the window and keep -// everything else - its contexts, objects and device - ends its lease with SuspendLease instead of -// EndLease and names a second hook: the next Attach of a NEW window calls it, so the holder can -// lease that window (AcquireFor) and rebuild its surface on it. The registration lasts until that -// AcquireFor succeeds or the holder cancels it (CancelResume, at the session's end). -// -// THREADS. Every method is thread-safe. The hooks are called from the thread that called the -// method: acquire from Attach's caller, release from Detach's / EndLease's caller, requestGeometry -// from the apply thread (AcquireFor), the lost hook from Detach's caller and the attached hook from -// Attach's (or SuspendLease's) caller, both WITH THIS OBJECT'S LOCK HELD - so they must not block and -// must not call back into this object (ServerLoop's set an atomic and ring its doorbell). +// The server's own on-screen window, held process-wide between the UI thread that owns its +// lifecycle (Attach / Detach on surfaceCreated, surfaceChanged, surfaceDestroyed) and the apply +// thread that renders into it under a lease (AcquireFor / EndLease). Only the in-process display +// server calls Install(); elsewhere a ServerOwned surface is refused (NoServerDisplay). The window +// is an opaque `void*` driven through ServerDisplayHooks, so no platform header is needed here. +// Nothing may render into the window after surfaceDestroyed returns, so Detach asks the holder to +// let go and waits, bounded, for EndLease; on timeout it returns TimedOut and EndLease releases +// the window later. A lease marked ending is waited for, not refused. See ARCHITECTURE.md §15.2. #pragma once #include diff --git a/MobileGL/MG_Remote/Server/ServerLoop.h b/MobileGL/MG_Remote/Server/ServerLoop.h index 632b4288..9ede2e81 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.h +++ b/MobileGL/MG_Remote/Server/ServerLoop.h @@ -6,66 +6,13 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// The apply thread and the server's private backend object. Owner: package v1 - the highest -// risk item in P5. Signatures by c0. -// -// WHY THE THREAD IS THE POINT. DirectGLES has 16 IsBackendContextCurrentOnThisThread() guards -// (DirectGLES.cpp:12034..12428) and Managers.cpp has 16 CanTouchGLNow() guards (:1494..:3428); -// every one of them DEGRADES when the answer is false - fences become always-signaled, queries -// return null handles, Present creates no frame fence so the buffer pool's recycle watermark -// never advances, and the two persistent-map acquisitions (Managers.cpp:1494, :2170) DECLINE, -// which would make PersistentCoherentMapScenario unreachable. Making the apply thread the -// context owner for life turns all 32 of those answers true on the server and removes the -// whole degradation class at once. It is also exactly the shape P6's spawned server inherits. -// -// P5 BUILDS ONE THREAD, NOT TWO. No mgl-srv-io: inproc's control plane is in the same process. -// P6 splits it. -// -// PARKING AND SHUTDOWN. The thread parks on Doorbell::Wait(consumerParked, ready, spinUs, -// kWaitForever) and shuts down when Wait returns false with Dead() set. TWO things can un-park a -// kWaitForever waiter, not one, and the difference is the whole of review m-4: for the STOP case a -// plain Doorbell::Notify() is sufficient, because m_stopRequested is in the park predicate and -// Stop() publishes it BEFORE it rings (Doorbell.h re-tests ready() after every Park return); -// Doorbell::Kill() (Doorbell.h, CondVarDoorbell::Kill) is load-bearing only for a predicate -// that has NOTHING to see, which is why the redcheck's park-predicate-loses-control entry - the one that drops the -// control flag FROM the predicate - is the case that actually times out. Kill BEFORE join; join -// before the client frees any emitter-owned Vector; and the join must be bounded (that test uses -// 5 s) so a regression is a red test and not a hung CI job. -// -// THE EGL OWNERSHIP MOVE, AS MEASURED (ID-54; review v2 item 10 and N-3). The native -// eglMakeCurrent for a surface runs ONCE PER CONTEXT LIFETIME on this thread and the context is -// then held for life. That "once" lives in TWO layers, because one is not enough: -// DirectGLES::MakeCurrent always calls native eglMakeCurrent and rewrites the owner (codex C7), -// and it is reached twice per bring-up - once from InitPbufferSurface/InitWindowSurface when the -// surface is CREATED, and once more from the client's first eglMakeCurrent. So (1) -// ServerMakeEGLCurrent classifies the request against the tuple it last bound -// (ClassifyEglMakeCurrent): an identical repeat is a no-op, and the R-12 republish decision for -// it is "nothing to republish" (ID-67: the client's mirror generation must not move); a different -// tuple is a real forwarded bind AND a caps republish; a client release-current is RECORDED -// (ClientReleaseCount) and NOT forwarded. And (2) BackendObject_DirectGLES::MakeEGLCurrent, under -// an active transport only, skips the native call when the requested draw surface is the one -// already natively current on this thread (IsBackendContextCurrentOnThisThread, which is EGL -// ground truth) AND the virtual tuple is the one that bind was for - or the bind is the surface's -// own creation, which the first tuple adopts; a different virtual context onto the same surface -// binds natively again, because MakeCurrent's invalidations describe the frontend context that -// changed (ID-67). Measured at the EGL function -// table by ServerLoopTest's C7 control on a real llvmpipe context: surface creation + two -// identical make-currents + a client release + a bind after the release = ONE native -// eglMakeCurrent, ZERO native releases, and the apply thread still the owner afterwards. So -// DirectGLES.cpp's six cache invalidations run once per context lifetime rather than once per -// client make-current, and the 16 IsBackendContextCurrentOnThisThread() / 16 CanTouchGLNow() -// sites answer TRUE on the server. The tuple is FORGOTTEN (N-3) on every event after which the -// native context it names may be gone - ReleaseEGLResources, ReleaseEGLSurface of the surface it -// names, a surface (re)creation, backend destruction - so a recycled handle value after a destroy -// is a real bind again and never a silent no-context-current. The client's nine EGL virtuals -// become BLOCKING control requests executed here. ReleaseEGLResources and -// ~BackendObject_DirectGLES MUST be blocking: MobileGL::Destroy() (MobileGL/Init.cpp:68) -// otherwise walks on while the server still holds the context. -// -// THE FALLBACK IS PRE-DECLARED, NOT INVENTED UNDER PRESSURE (R-1). If the context migration is -// still not running ClearThenReadPixels at the end of v1's fourth working day, the integrator - -// not the package - declares `inproc-inline`: the client thread drains the ring itself, no -// thread is created, no context migrates, and a second package picks up the thread arm. +// The apply thread (mgl-srv-apply) and the server's private backend object. The apply thread +// owns the backend's native EGL context for the context's whole life, so the backend's +// IsBackendContextCurrentOnThisThread() / CanTouchGLNow() guards answer true on the server +// instead of degrading. The client's EGL calls reach it as blocking control requests (the +// Server* forwarders below); ARCHITECTURE.md §15.5 has the make-current dedup and teardown rules. +// Stop() publishes m_stopRequested, which is in the park predicate, before it rings, so a plain +// Notify un-parks the thread; the join is bounded so a lost wakeup fails instead of hanging. #pragma once #include @@ -85,60 +32,14 @@ namespace MobileGL::MG_Remote::Server { - // --------------------------------------------------------------------------------- - // THE APPLY-THREAD IDENTITY, AS ONE INTEGER (P5d round 3, package D) - // --------------------------------------------------------------------------------- - // - // OnApplyThread() is the predicate under EVERY role guard on both hot paths - - // RefuseLegacyBufferArmFromApplyThread in BufferObject's accessors, MG_Record::PersistentMapTracker:: - // OnServerRole, ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt, RunsAsTheServerRole, - // MGPipeRefuseAllocatorFromApplyThread, RefuseLegacyTextureArmFromApplyThread. simpleperf at - // head 56a77348 (FCL + Minecraft, VD12, Adreno 830) measured it at 2.66% self / 3.64% - // inclusive ON THE CLIENT THREAD, for an answer that is "no" every single time. - // - // The old body paid five times over for that "no": ServerLoopInstance()'s function-static - // guard byte, an acquire load of m_running, an acquire load of an atomic, - // std::this_thread::get_id(), and a thread::id comparison that is an out-of-line - // pthread_equal through the PLT (@plt was 6.1% of the apply thread). None of it carries - // information the guard needs. A live thread's identity is already sitting in a - // register-cheap place - the thread pointer, TPIDR_EL0 on aarch64 - so the loop publishes - // ITS identity as a plain integer here and every caller compares one relaxed load against - // one register read, inline, with no call and no singleton. - // - // ZERO MEANS "NO APPLY THREAD IS RUNNING", which is exactly the answer m_running == false - // used to give: the key is published as the apply thread's FIRST act (ApplyThreadMain) and - // cleared in the same m_controlMutex critical section that clears m_running on the way out - // (ApplyThreadMain's C2 exit block), so "key != 0" and "m_running" move together and the - // window in which a live-but-exiting apply thread answers true is byte-for-byte the old - // one. The ServerLoop object still OWNS the key - nothing but ApplyThreadMain writes it - - // it simply no longer has to be reached through the singleton to be read. - // P14 S2: AND THE ANSWER IS PER THREAD NOW, NOT PER PROCESS. One word cannot hold N apply - // threads: two sessions' threads would each publish their own key and the second store would - // make the FIRST thread answer "no" to every role guard it owns - which is not a slower guard - // but a dead one (a draw would take the client arm on the apply thread, and - // RunSurfaceControlFrame would lose its re-entrancy shortcut and deadlock the EGL teardown). - // The question is "am I an apply thread", a fact about the CALLER, so it is asked in the - // caller's own TLS - one constant-initialized word, no guard, no atomic: - // - // t_onApplyThread the calling thread IS an apply thread. - // g_applyThreadKey HOW MANY are running in this process. Kept because - // MG_Backend/DirectGLES/Managers.cpp (outside this package's file set) - // reads this name `!= 0` - "an apply thread is running somewhere" - which - // is the count's zero-ness exactly. It is no longer a key and nothing on - // the hot paths reads it. - // - // ZERO MEANS "NO APPLY THREAD IS RUNNING" still: t_onApplyThread is raised as the apply - // thread's FIRST act and lowered in the same m_controlMutex critical section that clears - // m_running on the way out, so the window in which a live-but-exiting apply thread answers - // true is byte-for-byte the old one. - // - // WHAT WENT WITH IT, so nobody re-derives it: the aarch64 TPIDR_EL0 fast path, the - // std::thread::id fallback with its no-padding static_assert, the MOBILEGL_DETAIL_ - // APPLY_THREAD_POINTER build knob and the relaxed-load argument about a recycled TLS block. - // They existed to make "is the process's one apply thread mine" cheap; the question is now - // "is the caller an apply thread", whose cheap answer is the caller's own word - and a word - // cannot be recycled, because the thread that inherits a dead apply thread's TLS block is - // not an apply thread until it runs ApplyThreadMain. + // The apply-thread identity. OnApplyThread() sits under every role guard on the per-draw hot + // paths, so it is one inline read of the caller's own TLS word: no call, atomic or singleton. + // It is a per-thread fact because a process runs one apply thread per served session. + // t_onApplyThread the calling thread is an apply thread. ApplyThreadMain raises it before + // it applies anything and lowers it in the m_controlMutex section that + // clears m_running. + // g_applyThreadKey how many apply threads this process runs; Managers.cpp reads `!= 0` as + // "an apply thread is running somewhere". No hot path reads it. namespace Detail { // A fresh thread's TLS word is zero-initialized, so a thread that inherits the exited // apply thread's TLS block - the recycling case the old relaxed-load argument was about - @@ -152,27 +53,13 @@ namespace MobileGL::MG_Remote::Server { inline std::atomic g_applyThreadKey{0}; } // namespace Detail - // --------------------------------------------------------------------------------- - // P12 (on-screen server window), D4: ONE SURFACE MODE PER SESSION - // --------------------------------------------------------------------------------- - // - // "Only one of the two rendering paths is active at a time, decided at context creation": a - // session's mode latches at its FIRST successful surface creation - a ServerOwned window makes - // it on-screen, a pbuffer makes it offscreen - and a later surface of the OTHER kind in the same - // session is refused by name (SurfaceRefusal::SurfaceModeMismatch: reply ok=false, logged, the - // session NOT latched). Windows the client names itself (the X11/Win32/None tokens CONTRACT-P6 - // D8 has yet to close) are neither kind and neither latch nor are refused: nothing about them - // changes here. Reset by ServerLoop::Start, i.e. per session. - // - // P14 S2: AND THE LATCH IS PER SESSION, WHICH IS ALL THIS SLICE NEEDS. The mode is a member of - // the loop and a served session owns its loop, so two sessions in one process latch - // independently and neither's first surface decides the other's - the semantics above are - // otherwise unchanged (design/11-state-ownership.md §4.5: the latch stays at the session layer - // and is never hoisted to the process). A mode DECLARED before a session's first surface would - // have to travel on the wire - the Hello, or a SurfaceOp the client sends - because one - // process's MOBILEGL_IPC_SURFACE cannot speak for the several sessions a unified server runs; - // that is P14d's. Reading that env here instead refused a dialled-in ServerOwned client with - // SurfaceModeMismatch before the display was ever consulted (ServerOwnedSurfaceTest). + // One surface mode per session. It latches at the session's first successful surface + // creation: a ServerOwned window makes it on-screen, a pbuffer offscreen. A later surface of + // the other kind is refused with SurfaceModeMismatch (reply ok=false, nothing latched). + // Client-named windows (X11/Win32/None) and the surfaceless stand-in pbuffer neither latch + // nor are refused. The mode is a loop member reset by ServerLoop::Start, so each session + // latches on its own; it is not read from MOBILEGL_IPC_SURFACE, because one process's + // environment cannot speak for every session a unified server runs. enum class SessionSurfaceMode : Uint8 { None, OnScreen, Offscreen }; const char* SessionSurfaceModeName(SessionSurfaceMode mode); // The decision, pure, so a unit case can drive all six pairs without a backend. @@ -221,30 +108,14 @@ namespace MobileGL::MG_Remote::Server { // two files, and it is why no thread-keyed shim is needed for MOBILEGL_BUILD_DISAGGREGATED_INPROC. MG_Backend::BackendObject* Backend(); - // Run one blocking control request on the apply thread and wait for it. This is how all - // nine EGL lifecycle virtuals cross; it is deliberately NOT a queue of async messages, - // because every one of them has a return value the caller acts on immediately. - // - // THE REQUEST IS A VALUE FRAME (P5f, package fc). It used to be a raw function pointer - // plus a void* to a stack-local Args struct - neither has a meaning across a process - // boundary, which is the whole of P5c audit row G4. Now the slot carries one - // SurfaceControlFrame BY VALUE: the op's identity is the frame's `kind`, its arguments - // are scalars, and its reply fields come back in the same value when the wait returns. - // What did NOT move: the blocking handshake, the one slot, and the apply-thread - // ownership. Under spawn the same frame is what SurfaceOpCodec encodes into a - // Wire::SurfaceOp - this signature is the seam both transports share. - // P6 `cp`: where a control frame goes when the server is ANOTHER PROCESS. - // - // All twelve Server* forwarders funnel through RunSurfaceControlFrame, so - // this is ONE seam rather than twelve (a6 row A4-13 counted them). The - // client installs a sink at handshake; with none installed the behaviour - // is exactly what it was - post into the one-slot mailbox, or run inline - // when we are already on the apply thread. - // - // A HOOK RATHER THAN A CALL INTO ClientSession, deliberately: the server - // half calling the client half is four of the six symbols a6 found - // MG_Remote owes itself (a6-link-experiment §5), and this is the one - // place that would have added a fifth. + // Runs one blocking control request on the apply thread and waits for it; every Server* + // EGL forwarder funnels through here. Blocking rather than async, because each caller + // acts on the reply at once. The frame crosses by value: `kind` names the op and the + // reply fields come back in the same frame; under spawn SurfaceOpCodec encodes it as a + // Wire::SurfaceOp. When the server is another process the client installs a + // RemoteControlSink at handshake; with none, the frame goes to the one-slot mailbox, or + // runs inline when already on the apply thread. A hook rather than a call into + // ClientSession, so the server half does not depend on the client half. using RemoteControlSink = MobileGLResult (*)(void* user, SurfaceControlFrame& frame); void SetRemoteControlSink(RemoteControlSink sink, void* user); @@ -388,31 +259,14 @@ namespace MobileGL::MG_Remote::Server { void ForgetCurrentTuple(); void ForgetCurrentTupleIfItNames(EGLSurface surface); - // Part of the apply thread's park predicate: a posted control request must be able to - // un-park a thread waiting on kWaitForever, which a Notify alone cannot do. - // - // IT READS ONE ATOMIC AND TAKES NO LOCK (P5d round 3, package T item 1). It used to - // take m_controlMutex on every call, and the idle poll calls it on every iteration - - // once inside the `ready` lambda and once more through PumpControlRequest - which - // simpleperf at head 56a77348 measured as ~49% of the whole apply thread's cycles - // (mutex::lock 12.4 + mutex::unlock 13.6 + the two pthread_* halves, 58-60% of them - // with ApplyThreadMain as the DIRECT caller). An idle poll on an empty ring must cost - // loads, not a futex-backed critical section. m_controlPosted is that load; the - // mailbox's own fields stay under the mutex and the handshake below is unchanged. - // - // PUBLIC because ServerLoopTest reads it: the clear side of the shadow has a failure - // mode - a shadow left set makes `ready` permanently true and the loop never parks - // again - that no other public surface can see. - // - // READ THE PREDICATE LITERALLY: "a request is POSTED AND NOT YET TAKEN", which is - // narrower than the name's "a control request is in flight". The shadow is cleared - // when the pump takes the work, not when the work returns, so for the whole duration - // of `work(user)` this answers false although m_controlPending is still true and the - // poster is still blocked in RunSurfaceControlFrame. That is deliberate - it is a PARK - // PREDICATE, and the thread that would act on it is the one running the work - but it - // means this is not a liveness query and must not be used as one. The only in-tree - // reader outside the test is the apply thread's own `ready` lambda, which by - // construction cannot be inside that window. + // Part of the apply thread's park predicate: a posted control request must un-park a + // thread waiting on kWaitForever, which a Notify alone cannot do. It reads only + // m_controlPosted, without the lock, because the idle poll calls it on every iteration and + // a mutex here cost about half the apply thread's cycles. It means "posted and not yet + // taken": the pump clears it when it takes the work, so it reads false while the work runs + // and the poster still waits. Not a liveness query; the only reader is the apply thread's + // `ready` lambda. Public so ServerLoopTest can catch a flag left set, which would stop the + // loop from ever parking. Bool ControlIsPending() const; // ---- P12 (on-screen server window), D3/D6 ------------------------------------------- @@ -672,43 +526,14 @@ namespace MobileGL::MG_Remote::Server { // The supervisor announces it at start-up; the server app's broker relays it to the helper. Uint64 ReservedApplyCoreForThisProcess(const char** outRule); - // --------------------------------------------------------------------------------- - // THE EGL OWNERSHIP MOVE - the part that can sink the phase, expressed as twelve calls - // --------------------------------------------------------------------------------- - // - // Under split the app thread must never reach the driver's eglMakeCurrent. Today it does: - // EGLImpl.cpp:284 -> BackendObject_DirectGLES.cpp:963 -> DirectGLES.cpp:11925, and the - // owner slot g_backendContextOwnerThread (DirectGLES.cpp:11865) is stamped with whatever - // thread got there. So BackendObject_Remote's nine EGL virtuals - package c1's - call these - // twelve, each of which is a BLOCKING control request that runs the SERVER's backend object - // on mgl-srv-apply. The native eglMakeCurrent then runs once per context lifetime on that - // thread (the surface's own creation binds; the client's make-currents onto that surface - // are deduped at both layers, header block above) and a client release is never forwarded, - // so g_backendContextOwnerThread is written once per context lifetime, DirectGLES.cpp's six - // cache invalidations run once per context lifetime rather than once per client - // make-current, the per-frame EGL re-verification stamp holds, and the 16 - // IsBackendContextCurrentOnThisThread() sites plus the 16 CanTouchGLNow() sites answer TRUE - // on the server instead of silently degrading. Measured, not assumed: ServerLoopTest's C7 - // control counts the driver's eglMakeCurrent calls at the EGL function table. - // - // TWO OF THEM MUST BLOCK OR THE PROCESS TEARS ITS OWN CONTEXT DOWN UNDER ITSELF: - // ReleaseEGLResources (reached from EGLImpl.cpp:326, which for DirectGLES runs - // DestroyEGLContext) and ~BackendObject_DirectGLES (reached from - // pActiveBackendObject.reset() at MobileGL/Init.cpp:68). Both are blocking here - the first - // by being one of these calls, the second because ServerLoop::Stop() destroys the private - // backend ON the apply thread before that thread exits and Stop() itself waits. - // - // WHY THEY ARE FREE FUNCTIONS AND NOT MEMBERS: c1 needs exactly this surface and nothing - // else of the server, so the seam between the two packages is a list of twelve signatures - // rather than a class with a lifecycle. - // - // P5f (package fc): each of the twelve packs its arguments into a SurfaceControlFrame and - // posts it through RunSurfaceControlFrame - the wire-shaped value channel that replaced the - // function-pointer mailbox (P5c audit row G4). Nine of them map onto the ten wire ops of - // protocol.fbs's SurfaceOp; ServerSwapEGLBuffers, ServerInitCapabilities and - // ServerInitWindowSurface ride the same channel as inproc-only kinds (present travels as a - // record, capabilities are answered by the CapsSnapshot frame, and the third has no caller - - // f0-egl's census, findings F8/§4.2). + // The EGL control forwarders. BackendObject_Remote's EGL virtuals call these, so the app + // thread never reaches the driver's eglMakeCurrent: each packs its arguments into a + // SurfaceControlFrame and blocks in RunSurfaceControlFrame while mgl-srv-apply runs it against + // the server's backend (ARCHITECTURE.md §15.5). ServerReleaseEGLResources must block, and so + // must ~BackendObject_DirectGLES, which ServerLoop::Stop() runs on the apply thread before it + // exits; otherwise teardown runs on while the server still holds the context. Free functions, + // because the client needs only these calls. SwapBuffers and InitWindowSurface are + // inproc-only frame kinds with no wire op: present travels as a record. Bool ServerInitializeEGLDisplay(EGLDisplay dpy, EGLint* major, EGLint* minor); Bool ServerCreateEGLWindowSurface(EGLSurface surface, const MG_Backend::WindowHandle& handle); Bool ServerResizeEGLWindowSurface(EGLSurface surface, Uint32 width, Uint32 height); diff --git a/MobileGL/MG_Remote/Server/ServerMain.cpp b/MobileGL/MG_Remote/Server/ServerMain.cpp index a4fef99e..52217bd2 100644 --- a/MobileGL/MG_Remote/Server/ServerMain.cpp +++ b/MobileGL/MG_Remote/Server/ServerMain.cpp @@ -361,26 +361,14 @@ void DelaySessionHandshakeForTest() { std::this_thread::sleep_for(std::chrono::milliseconds(std::min(ms, 60000))); } -// Called only after fork (or, in the single-session shape, instead of it): the supervisor never -// creates a backend or EGL context. `dataSource` is set on TCP (PH-7 (4)) and empty on a unix -// endpoint, whose second connection is the SCM_RIGHTS socket PairAcceptor already paired. -// `sourceFd` is the descriptor that source reads from - the child's end of the hand-off -// socketpair under --serve, the listener itself in the single-session shape - and -1 when there -// is none; it is let go of the moment Accept returns, and on every exit before that -// (ExitBeforeAccept, ID-P7-44). -// -// `helloFrame` is the first frame when somebody has already read and authenticated it - the TCP -// supervisor always has (PH-7 (5)) - and empty when this function reads it itself, within -// `helloWaitMs`. -// -// P12 (D5): IT RETURNS THE SESSION'S EXIT CODE, and never exits. A forked child and a -// single-session process pass the code straight to ExitSessionProcess - so every one of those shapes -// ends with the `_exit` code it always had - while `inProcess` (the in-process display server's -// session thread) returns it to the supervisor thread and the process lives on. What a process exit -// used to clean up for free, `inProcess` cleans up by hand on every way out: the backend (the loop's -// Stop drops one no thread owns), the session singleton, the stats window, the log forwarder and the -// progress sink, both of which point at `control`. The latch is reset by the supervisor between -// sessions (ResetSessionLatch). +// Runs one served session; the supervisor never creates a backend or EGL context. `dataSource` is +// set on TCP and empty on a unix endpoint, whose aux connection PairAcceptor already paired. +// `sourceFd` (the hand-off socketpair end under --serve, the listener in the single-session shape, +// or -1) is released when Accept returns and on every earlier exit (LeaveBeforeAccept). +// `helloFrame` holds the first frame if the caller already read and authenticated it; otherwise it +// is read here within `helloWaitMs`. Returns the session's exit code and never exits; with +// `inProcess` (a session thread of the in-process display server) every way out cleans up by hand +// what a process exit would, and the supervisor resets the latch between sessions. int RunSession(std::unique_ptr control, std::vector helloFrame, Server::ServerSession::DataConnectionSource dataSource, int sourceFd, DataSource sourceKind, std::uint32_t helloWaitMs, bool inProcess = false) { @@ -481,23 +469,13 @@ int RunSession(std::unique_ptr control, std::vector(); Server::Detail::ThreadSessionScope threadScope(*runtime); ArmSessionLatch(); @@ -705,30 +683,12 @@ int RunSession(std::unique_ptr control, std::vector buffer(64 * 1024); - // PH-6 (ID-P7-2): THE CONTROL WAIT IS BOUNDED, SO A SESSION CAN END FROM THE SERVER'S SIDE. - // It used to be kWaitForever, which left exactly one way out: the peer closing its control - // connection. A peer that forfeited the reverse channel is by definition one that stopped - // reading, and it may well keep that connection open - so the apply thread would stop and - // this child would sit here, holding the supervisor's one session slot, answering every - // later client Busy. Now each quiet interval asks whether the apply thread is still there; - // when it is not (forfeit, or a dead data bell), the session takes the ordinary exit below: - // Stop, Close, _exit(0). (PH-6 used the supervisor's 250 ms accept poll as the interval; the - // PH-1 note below says why the slice is now 100 ms.) - // - // ASKED AT THE TOP OF EVERY TURN, NOT ONLY WHEN A WAIT TIMES OUT (PH-6 fix round). A peer that - // forfeited the reverse channel but keeps TALKING - a LogFlush or a surface op more often than - // every 250 ms, each of which this loop answers - never let ReceiveFrame time out, so the - // question was never asked and the session, with the supervisor's one slot, lived as long as - // the peer kept chattering. - // - // PH-1 (3): the same wait is also how a LATCH raised on the apply thread (the decoder, the - // applier, DrainRing) ends this loop within one slice while the peer is silent. ReceiveFrame - // keeps a partial frame in its reassembler across a timeout, so slicing loses no bytes; a - // latched session is then closed below with kSessionLatchedExitCode. The latch is asked - // BEFORE the apply thread's liveness because a latch also stops that thread (it leaves its - // loop and clears m_running), and the session must be named by its latch, not as a stopped - // apply thread. One slice serves both: the latch's 100 ms (PH-6 had 250 ms; the shorter wait - // only asks the question more often). + // The control wait is sliced so a session can end from the server's side. Each turn first asks + // whether the session latched (a fault on the apply thread, which also stops that thread, so + // the latch is asked first and names the end), then whether the apply thread still runs (it + // stops on a forfeited reverse channel or a dead data bell). Both are asked every turn, not + // only on timeout, so a peer that keeps sending frames cannot hold the supervisor's session + // slot. ReceiveFrame keeps a partial frame across a timeout, so slicing loses no bytes. constexpr std::uint32_t kControlSliceMs = 100; for (;;) { if (SessionLatched()) break; @@ -765,23 +725,10 @@ int RunSession(std::unique_ptr control, std::vector(control->BufferedBytes())); break; } - // THE FILE IDENTIFIER, ASKED FIRST AND SAID OUT LOUD (P7 wave 0). - // - // The plan asked for CtrlEnvelopeBufferHasIdentifier here because ServerSession:: - // ParseEnvelope (:109) asks it and this loop did not. MEASURED, THE PREMISE IS WRONG: - // the generated VerifyCtrlEnvelopeBuffer is - // `verifier.VerifyBuffer(CtrlEnvelopeIdentifier())` - // (protocol_generated.h:2378), so a wrong identifier was ALREADY refused - and so is - // ParseEnvelope's second call. Recorded for the integrator: the gap that row names does - // not exist. - // - // What DID exist is that both refusals were silent. A peer whose post-Welcome frame was - // garbage got its session ended with no line in the log at all, which is the half of a - // fuzz arm that matters - an arm that cannot tell "refused, by name" from "the child - // went away" measures nothing. So the identifier is now asked FIRST, because it is the - // specific diagnosis ("that is not our schema") where the verifier's is the general one, - // and both say so. Asking it first needs the length guard the verifier would otherwise - // have provided: BufferHasIdentifier reads bytes [4, 8). + // The identifier is checked before the verifier, which would also reject a wrong one, so + // the log names the specific cause; every refusal here is logged, because a silent end + // cannot be told apart from a crashed child. BufferHasIdentifier reads bytes [4, 8), + // hence the length guard. if (size < 8) { WireLogError("MG_Remote server: control frame is %llu bytes, too short to carry a " "CtrlEnvelope root and identifier; ending the session", @@ -926,37 +873,11 @@ void LogSupervisorSummary(unsigned long sessionsFaulted) { static_cast(::getpid()), sessionsFaulted); } -// PH-7 (5), ID-P7-3, ph-f.md §6.4, CONTRACT-P7 §12. THE TCP SUPERVISOR: AUTHENTICATE, THEN FORK. -// -// Until this package the --serve loop accepted a TCP connection and forked a session child for -// it before a byte had been read; the child read the Hello (10 s budget), checked the wire and -// only then the token. So anybody who could reach the port cost the server a fork per connection -// and a child held for up to 10 s, and an unauthenticated Hello with a wrong fingerprint was -// answered with this build's fingerprint. Now: -// -// - every new connection is PENDING until its first frame is complete. The supervisor reads it -// itself, a few bytes per wakeup, from all pending connections at once (one poll loop - no -// unauthenticated peer can make it wait for another; the one wait left is up to 50 ms for an -// exiting session before an AUTHENTICATED Hello is told Busy), and never a byte past the frame; -// - a pending connection has MOBILEGL_IPC_PREAUTH_MS (default 2000) to complete it, and at most -// MOBILEGL_IPC_PREAUTH_MAX (default 8) are pending at once. The queue is SHARED between -// addresses: when it is full a newcomer displaces the oldest connection of the address holding -// the most slots, and is itself refused Busy only when its own address already holds as many; -// - a Hello is AUTHENTICATED HERE, token first; an unauthenticated one is answered -// Refuse{Authentication} and nothing else. Only an authenticated Hello is forked for, and the -// child is handed the frame it would otherwise have read; -// - a failure (wrong token, malformed first frame, deadline, a silent connection that held its -// slot past kPreAuthProbeGraceMs before it closed or was displaced) is counted against the -// peer's address; after MOBILEGL_IPC_AUTH_BACKOFF_AFTER (default 5) of them that address is refused -// at accept, before a byte is read, for MOBILEGL_IPC_AUTH_BACKOFF_MS doubling per further -// failure up to a minute. An authenticated Hello from the address clears it; -// - a DataBind is routed exactly as PH-7 (4) routed it: to the live session's child over the -// hand-off with the nonce as sideband (the child compares), or refused by name when there is -// no live session - here now, instead of in a child forked to say so. -// -// What is unchanged: one session at a time (an AUTHENTICATED second client is Busy), the child's -// own ValidatePeerHandshake / Accept / BindDataConnection, the hand-off and its refusals, the -// reap lines and sessionsFaulted. +// The TCP supervisor reads every new connection's first frame itself without blocking on any +// one peer, authenticates a Hello (token first) and only then hands the session off (fork or +// thread); it routes DataBinds to the live session and backs off failing addresses +// (ARCHITECTURE.md §15.1.1, PreAuthGate.h). One session at a time: an authenticated second Hello +// is answered Busy. class TcpSupervisor { public: using Clock = std::chrono::steady_clock; diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index 2a8a58b4..be135d65 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/MobileGL/MG_Remote/Server/ServerSession.cpp @@ -46,25 +46,12 @@ 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. + // MOBILEGL_TEST_DELAY_FIRST_CAPS_MS: a test-only knob (disaggregated builds, inert unless + // set) that delays the server's first CapsSnapshot only; re-sends are never delayed. Read + // once per process. Across processes it makes the snapshot genuinely late and exercises the + // client's wait for it; under inproc Accept runs inside ClientSession::Start, so it only + // slows Start, showing the first snapshot cannot be late relative to session start there + // (the window before Start is closed by ordering in MobileGL::Initialize). void DelayFirstCapsSnapshotForTest() { #if MOBILEGL_BUILD_DISAGGREGATED static Bool spent = false; @@ -228,28 +215,14 @@ namespace MobileGL::MG_Remote::Server { eventName, static_cast(payloadBytes)); } - // ---- P5e (ra), CONTRACT-P5E §2.6: the ring stops being a Fatal and becomes a queue -- - // - // THE FATAL'S PREMISE DIES WITH THE LOCKSTEP. "A full ring is a producer burst no - // workload has" was true because the client drained at EVERY verb: the ring was empty - // whenever the server started a record, so filling 256 KiB inside one apply meant a - // defect. A run-ahead client drains at its waits, which on the steady path are one - // present apart, so a full ring is now an ordinary backlog and aborting on it would - // turn the reverse channel's capacity into a workload limit. - // - // SO THE PRODUCER BLOCKS. It publishes what is already in the ring and rings the - // client (a client that has not yet noticed cannot drain), then parks on its own bell - // until the client's Drained() clears the latch and rings back. The deadlock argument - // is the contract's, and it needs BOTH halves: the server blocks only here, the client - // blocks only on watermarks the server advances, and every client park breaks on - // `eventRingFull` as well (SessionProducer::WaitFor*OrEventBacklog) - so a parked - // client is always wakeable by the server that is waiting for it. - // - // ONE NAMED REFUSAL SURVIVES, the impossible case: a single event larger than the ring - // can EVER hold (no amount of draining helps). The two BUSY refusals that used to sit - // beside it - a wait that ran out of patience, and two rounds against a ring the record - // fits - are the PEER's behaviour and became the forfeit latch below (PH-6, ID-P7-2). - // A server that does not publish kCapRunAheadApply keeps P5C's Fatal unchanged. + // With run-ahead the client drains SEG_EVENT only at its waits, so a full event ring is an + // ordinary backlog: the producer publishes what it has, rings the client and parks on its + // own bell until the client's Drained() clears the latch and rings back. No deadlock: the + // server blocks only here, and every client park also breaks on `eventRingFull` + // (SessionProducer::WaitFor*OrEventBacklog). One event larger than the ring can ever hold + // is still Fatal; a client that does not drain forfeits the reverse channel + // (ServerSession::ForfeitReverseChannel). A server without kCapRunAheadApply keeps the + // overflow Fatal. See ARCHITECTURE.md §11.7. Bool ServerPublishesRunAhead(const ServerSession& session) { if (!session.CallMaskIsSet()) return false; return (session.CallMask() & static_cast(MG_Pipe::kCapRunAheadApply)) != 0; diff --git a/MobileGL/MG_Remote/Server/ServerSession.h b/MobileGL/MG_Remote/Server/ServerSession.h index 6e4d2f65..24905bd1 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.h +++ b/MobileGL/MG_Remote/Server/ServerSession.h @@ -6,38 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// The server half of a session: the consumer side of the rings, the handshake reply, the caps -// snapshot. Owner: package s1. Signatures by c0. -// -// The four segment sizes are already pinned by ProtocolSmokeTest.cpp:72 and are not up for -// re-derivation here: SEG_CMD 8 MiB, SEG_STAGE 32 MiB, SEG_REPLY 8 MiB, SEG_EVENT 256 KiB. -// MOBILEGL_IPC_RING_MB and MOBILEGL_IPC_STAGE_MB move the first two; the ring caps ONE record -// at half its size (R-10). See SessionRings.h's header for why the ring inside SEG_CMD is 4 MiB -// rather than 8 - the control page takes the head and the capacity must be a power of two - -// and why that deviation from CONTRACT-P5 §5's arithmetic is in the safe direction. -// -// THE TWO DOORBELL ACCESSORS ARE ON THE CONCRETE CLASS, NOT ON ITransport -// (InProcessTransport.h:64-68). P5 decides this now rather than letting P6 discover it: the -// SESSION owns the pair and hands out references, so ITransport stays the dumb control-plane -// interface its header says it is and SocketTransport does not have to grow two accessors it -// has no natural home for. Discovering this in P6 would mean re-laying one package's call sites. -// -// WHO CREATES THE SEGMENTS: this side. Welcome announces all four SegmentRefs and Welcome is -// server -> client, so the server allocates and the client attaches. "Client-owned" in -// protocol.fbs's comments is about who WRITES a segment, not who allocates it. -// -// WHAT Accept() DOES, IN ORDER. The ABI assertion is FIRST, before a single record is decoded -// and before any segment exists, because its whole purpose is to refuse to interpret the peer's -// bytes at all: -// 1. receive Hello; -// 2. compare abiMajor/abiMinor and CapsAbiFingerprint() - Fatal{AbiMismatch} on a difference, -// never a downgrade; -// 3. create and map the four segments, initialise both control pages, clear the reply pool; -// 4. send Welcome with the four SegmentRefs; -// 5. publish the first CapsSnapshot, IF a backend has been handed over (SetBackend). Without -// one the session is accepted and the snapshot is deferred with a loud line: the caps -// blob codecs are w1's and the server's private BackendObject is v1's, and a session that -// refused to exist until both landed would block every other package's bring-up. +// The server half of a session: the consumer side of the rings, the doorbell pair (handed out +// here, not by ITransport, which stays control-plane only), the handshake reply and the caps +// snapshot. The server allocates the segments and the client attaches; "client-owned" in +// protocol.fbs means who writes a segment. Accept, in order: receive Hello; authenticate the peer +// and refuse any ABI, fingerprint or link-terms mismatch before any shared memory exists or any +// record is decoded; create the segments; send Welcome; bind the stream data connection, if any; +// install the segment resolver and the reverse-channel callbacks; publish the first CapsSnapshot +// if SetBackend was called, else defer it with a warning. #pragma once #include "../Transport/ILink.h" @@ -146,30 +122,14 @@ namespace MobileGL::MG_Remote::Server { // PublishCapsSnapshot yourself. void SetBackend(MG_Backend::BackendObject* backend); - // MGPCaps::CallMask's two halves. BOTH ARE MANDATORY AND NEITHER HAS A DEFAULT. - // - // The first version of this file derived a default for each. That was the phase's - // marquee defect committed from the server's side: the consumer default read - // MGPipeGetResourceOps(), a PROCESS-WIDE global (PipeApply.cpp), so under inproc the - // server answered with whatever the client half of the same process had registered, - // and under spawn it collapsed to P2's 0x7f. Either way CapsMirror::ServerConsumes - // then answers a client-side liveness gate with a guess: the client stops emitting - // five P4a families, CLEARS ITS DIRTY FLAGS ON ACCEPTANCE ANYWAY, and the lane goes - // green with the uploads lost - ID-39's 66 lost uploads, reflected. R-8's whole point - // is that a client-side gate may never be answered by a server-side fact; a - // server-side gate answered by a PROCESS-wide fact is the same defect one level down. - // - // So there is no derivation at all. An unset mask is a programming error and - // CallMask() is a named Fatal on one - loud at the first snapshot instead of silent - // for a phase. Flagging it in a report was not a mechanism; this is. - // - // BITS 0..8, THE MGPCapBit FEATURE BITS. What each one answers belongs to the package - // that owns the question (kCapTimerQuery to the query family, kCapResidentSubData to - // b1, and so on). `SetCapabilityBits(0)` is a legitimate and explicit answer - "this - // server offers no optional capability" - and is the right call while those packages - // land. kCapNeedsHostIndexBytes / kCapNeedsHostUboBytes / kCapViewportArray are RESERVED - // (P8-F; CONTRACT-P5 table 0 first held the two host-byte bits at 0): the first two are - // the only things that ask for an MGHostSpan, and 0 keeps every one of them off the wire. + // MGPCaps::CallMask's two halves; both are mandatory and neither has a default. The client + // decides from the consumer bits which record families to emit and still clears its dirty + // flags on acceptance, so a guessed mask (a process-wide registration, a fixed value) loses + // uploads silently; CallMask() is Fatal{UnsetCallMask} until both setters have run. This + // one takes the MGPCapBit feature bits, each answered by the subsystem that owns it; + // SetCapabilityBits(0) is an explicit "no optional capability". kCapViewportArray and the + // two host-byte bits (kCapNeedsHost*Bytes, the only thing that asks for an MGHostSpan) are + // reserved and stay 0. void SetCapabilityBits(Uint64 capBits); // BITS 32..47, THE CONSUMER MASK (R-8 / C-4): which MGPipe subsystems this server has @@ -236,24 +196,13 @@ namespace MobileGL::MG_Remote::Server { Bool EventRingHasRoom() const; Transport::ITransport* Control_Plane(); - // ---- PH-6 (ID-P7-2): THE REVERSE CHANNEL IS FORFEITED, NOT FATAL ------------------- - // - // A run-ahead producer that finds SEG_EVENT full parks until the client drains room - // (CONTRACT-P5E §2.6). What a client that does not drain used to cost was two Fatals - // inside ReserveEventOrBlock - "waited 30000 ms", twice, and "a drained ring that still - // refuses a record it fits is a corrupt cursor set", which a peer that drains one slot at - // a time reaches in milliseconds. Both are the PEER's behaviour, not a defect of this - // process, so neither may abort it. - // - // What replaces them is a LATCH on this session. The first event that cannot be placed - // within MOBILEGL_IPC_EVENT_WAIT_MS raises it (ForfeitReverseChannel, which logs - // `ReverseChannelForfeit{}` once); that event and every later one is a counted - // drop (CountDrop on the shared page, ForfeitDrops() here) into scratch the producer may - // fill and forget; the apply loop's park predicate sees the latch and takes the ordinary - // stop path; ServerMain's control loop sees the apply thread gone and ends the session. - // The process exits 0 and the supervisor welcomes the next connection. The latch is - // set, read and reset on the apply thread's side of Accept/Close only, and read with - // acquire elsewhere (the control loop, tests). + // Reverse-channel forfeit: a client that does not drain SEG_EVENT is the peer's fault and + // must not abort this process. The first event that cannot be placed within + // MOBILEGL_IPC_EVENT_WAIT_MS raises this session's latch (ForfeitReverseChannel logs + // `ReverseChannelForfeit{}` once); that event and every later one is a counted drop + // into scratch; the apply loop sees the latch and stops, and ServerMain ends the session + // with status 0, not as a fault. The latch is written only on the apply thread (reset by + // Accept) and read with acquire elsewhere. See ARCHITECTURE.md §11.7. Bool ReverseChannelForfeited() const { return m_reverseChannelForfeit.load(std::memory_order_acquire); } diff --git a/MobileGL/MG_Remote/Server/ServerSpawn.cpp b/MobileGL/MG_Remote/Server/ServerSpawn.cpp index 1212b302..b88c9c31 100644 --- a/MobileGL/MG_Remote/Server/ServerSpawn.cpp +++ b/MobileGL/MG_Remote/Server/ServerSpawn.cpp @@ -77,34 +77,13 @@ namespace MobileGL::MG_Remote::Server { return std::string(); } - // Every MOBILEGL_TRANSPORT and MOBILEGL_IPC_* goes. Built BEFORE fork: - // between fork and execve only async-signal-safe calls are allowed, and - // allocating a vector of strings is not one of them. - // - // EXCEPT A KNOB THE SERVER READS, named here one by one. P7 wave 4 M2's - // MOBILEGL_IPC_WIRE_DEFERRED_MB bounds the server's own orphaned wire buffer stores; - // nothing in the client reads it, so scrubbing it did not make the child safer - it made - // the knob mean something under inproc and silently nothing under spawn, the worst of - // the three states (the tcp server reads its supervisor's environment, as for every - // knob). It names no endpoint, no role and no segment size, so neither anti-recursion - // catch has anything to say about it. - // - // M2 round 2 (review): three more the server reads and that were silently defaulting in - // the spawned child for the same reason - MOBILEGL_IPC_SPIN_US (the doorbell spin budget, - // read by BOTH roles, so the child should spin as the parent was told to), - // MOBILEGL_IPC_SERVER_AFFINITY (the apply thread's CPU mask, read only by the server) and - // MOBILEGL_IPC_AUDIT (the server's 0xDD fill of retired stage bytes). Each is a tuning or - // audit value, none names an endpoint, a role, a path or a segment size, and ServerMain's - // catch (b) keeps verifying the four it verifies (TRANSPORT, SERVER_PATH, RING_MB, - // STAGE_MB). Everything else under the prefix still goes. - // - // PH-6 (ID-P7-2) adds MOBILEGL_IPC_EVENT_WAIT_MS on the same argument: it is the server's - // patience with a client that has stopped draining SEG_EVENT, nothing on the client reads - // it, and scrubbing it would make a spawned server wait the default whatever the lane set. - // - // P11 B2 adds MOBILEGL_IPC_ALLOW_ADOPT_T0, the server's own T0 allow switch: only the server - // reads it, and scrubbing it would make "allow switch off" unreachable on the spawn arm. - // MOBILEGL_IPC_ADOPT_TIER itself still goes - the client's ask travels in its Hello. + // Scrubs MOBILEGL_TRANSPORT and every MOBILEGL_IPC_* from the child's environment; built + // before fork, because allocating is not async-signal-safe between fork and execve. + // kServerOwned lists the exceptions, knobs the server itself reads (wire + // deferred budget, spin budget, apply-thread affinity, stage audit fill, event wait, T0 + // allow switch): scrubbing them would silently default them under spawn. None names an + // endpoint, role, path or segment size, so ServerMain's environment catch is unaffected. + // MOBILEGL_IPC_ADOPT_TIER is scrubbed: the client's ask travels in its Hello. bool ShouldScrub(const char* entry) { static constexpr const char* kServerOwned[] = { "MOBILEGL_IPC_WIRE_DEFERRED_MB=", "MOBILEGL_IPC_SPIN_US=", "MOBILEGL_IPC_SERVER_AFFINITY=", @@ -149,26 +128,11 @@ namespace MobileGL::MG_Remote::Server { // §3.1 is the whole argument for why those are two axes. env.storage.emplace_back("MOBILEGL_IPC_ROLE=server"); env.storage.emplace_back("MOBILEGL_IPC_DIAL=no"); - // LD_LIBRARY_PATH, BECAUSE THE SERVER IS EXEC'd AND NOT dlopen'd. - // - // It links libMobileGL.so, and an exec'd process gets a FRESH linker - // namespace with the default search path - not the one the app built - // for its own libraries. An APK's lib// is not on that path, and - // the failure is total and immediate: - // - // CANNOT LINK EXECUTABLE ".../libMobileGLServer.so": - // library "libMobileGL.so" not found: needed by main executable - // - // It is invisible on desktop, where CMake gives build-tree targets an - // absolute RPATH of their own - so the WSL server had been resolving - // its dependency through a path no phone has. - // - // $ORIGIN WOULD BE THE TIDIER ANSWER and the target asks for it, but - // the NDK toolchain drops RPATH from the link: measured with readelf - // on the APK's own copy, which carries NEEDED libMobileGL.so and no - // RUNPATH at all. So the launcher states it instead - it is the one - // component that has already resolved where the image lives, and a - // pair that is not co-located is not a pair. + // LD_LIBRARY_PATH: the server is exec'd, not dlopen'd, so the linker uses its default + // search path, which lacks the APK's lib//, and libMobileGL.so is not found. + // Desktop builds hide this behind CMake's absolute build-tree RPATH. $ORIGIN would be + // tidier, but the NDK link drops RPATH (the APK's copy has no RUNPATH), so the + // launcher, which already resolved the image's directory, states it. if (!imageDir.empty()) { env.storage.emplace_back("LD_LIBRARY_PATH=" + imageDir); } diff --git a/MobileGL/MG_Remote/Server/ServerSpawn.h b/MobileGL/MG_Remote/Server/ServerSpawn.h index 0a8e48cf..5bc0191f 100644 --- a/MobileGL/MG_Remote/Server/ServerSpawn.h +++ b/MobileGL/MG_Remote/Server/ServerSpawn.h @@ -6,35 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// LAUNCHING a server process, and the process discipline around it. -// Package `sm` (CONTRACT-P6.md §3). -// -// IT LAUNCHES; IT DOES NOT COUPLE. The two processes are independent: this -// hands the child an endpoint NAME and nothing else - no inherited descriptors, -// no shared memory, no handshake. The child listens on that name and the client -// connects to it, exactly as it would to a server someone started by hand or -// that an Android Service started minutes earlier. -// -// An earlier shape of this file forked the server and handed it fds 3..6. -// Inheritance works and is less code, but it can only ever produce a server -// that is a CHILD OF ITS CLIENT - which the end state cannot use, because there -// the server is a standing application and the client is elsewhere, possibly on -// another kernel. So fork/execve survives here only as A WAY TO START A -// PROCESS, which is all it ever needed to be, and every fd it used to carry is -// gone. -// -// ANTI-RECURSION IS STILL TWO INDEPENDENT CATCHES, and CONTRACT-P6 §3.1 is why -// neither is the original ARCHITECTURE.md §15.1 rule. That rule says the child hard-sets -// Transport to Monolith; a6 measured that 226 live lines test -// `Transport != Monolith` to select the SERVER arm, 148 of them in MG_Backend, -// so obeying it flips the whole backend to frontend glue in the one process -// with no frontend. Instead: -// (a) STRUCTURAL: MOBILEGL_IPC_DIAL=no, a separate axis from which role the -// process plays; -// (b) ENVIRONMENTAL: MOBILEGL_TRANSPORT and every MOBILEGL_IPC_* removed -// from the child's envp. -// The child refuses with exit 64 for (a) and 65 for (b), so the two safeties -// are distinguishable - two that cannot be told apart are one. +// Launches a server process. The child gets an endpoint name and nothing else (no inherited +// descriptors, no shared memory) and listens on it like a server started by hand or by an Android +// Service, so a spawned server never depends on being its client's child. +// Anti-recursion is two independent catches (ARCHITECTURE.md §15.1), not forcing Transport to +// Monolith, which would turn off the server arms selected by `Transport != Monolith`: +// (a) MOBILEGL_IPC_DIAL=no, separate from the process's role; (b) MOBILEGL_TRANSPORT and the +// MOBILEGL_IPC_* knobs scrubbed from the child's environment. The child exits 64 for (a) and 65 +// for (b), so the two are distinguishable. #pragma once diff --git a/MobileGL/MG_Remote/Server/SessionRuntime.h b/MobileGL/MG_Remote/Server/SessionRuntime.h index a8e05893..4bdfed83 100644 --- a/MobileGL/MG_Remote/Server/SessionRuntime.h +++ b/MobileGL/MG_Remote/Server/SessionRuntime.h @@ -6,30 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P14 S2 (docs/Disaggregated/design/11-state-ownership.md): ONE SERVED SESSION'S PROCESS-LOCAL -// STATE, and the routing that lets the rest of the tree keep spelling the singletons. -// -// WHAT THE SINGLETONS WERE. ServerSessionInstance()/ServerLoopInstance() were the session, so one -// session per process was a property of every file that named them. This slice gives each served -// session its own {ServerSession, ServerLoop, PipeInputs} triple, held here, and resolves the two -// singleton accessors through the CALLING THREAD's session - which is what makes the ~900 sites -// that spell gPipeInputs, gPipeInputs, ServerSession::Active() or ServerLoopInstance() find the right -// one without one line of any of them moving. -// -// WHO IS A SESSION THREAD. Two threads per session, both created by the session itself: -// * the CONTROL thread - ServerMain::RunSession's own thread, which reads the Hello, drives the -// blocking control pump and ends the session (a std::thread per session in the supervisors, a -// forked process in the `--serve` worker shape); -// * the APPLY thread - ServerLoop's mgl-srv-apply, which applies the ring and owns the backend. -// ThreadSessionScope is installed on both, first thing. A thread with no scope resolves to the -// process-wide defaults, which is exactly the old behaviour and is what the CLIENT process, the -// inproc one-process client+server shape and every unit case keep getting. -// -// THE INPROC CLIENT+SERVER SHAPE IS DELIBERATELY UNTOUCHED. ClientSession::Start calls -// ServerLoopInstance().Start(ServerSessionInstance()) and never opens a scope, so its applier and -// its app thread still share the one process-wide PipeInputs block - the verb barrier's premise -// (one block two roles alternate on) is a property of that shape and this file must not break it. -// Only ServerMain::RunSession, which serves a session with its own transport, opens a scope. +// One served session's process-local state ({ServerSession, ServerLoop, PipeInputs}) and the +// per-thread routing behind the singletons: on a thread inside a ThreadSessionScope, +// ServerSessionInstance(), ServerLoopInstance() and gPipeInputs resolve to that thread's session; +// elsewhere they resolve to the process-wide defaults. ServerMain::RunSession opens the scope on +// its control thread and ServerLoop on the session's apply thread. The in-process client+server +// shape (ClientSession::Start) opens none, so its app thread and applier keep sharing the one +// PipeInputs block the verb barrier relies on; keep it that way. +// See docs/Disaggregated/design/11-state-ownership.md. #pragma once #include diff --git a/MobileGL/MG_Remote/Server/SurfaceControlFrame.h b/MobileGL/MG_Remote/Server/SurfaceControlFrame.h index e9a0787e..3d8ab0d0 100644 --- a/MobileGL/MG_Remote/Server/SurfaceControlFrame.h +++ b/MobileGL/MG_Remote/Server/SurfaceControlFrame.h @@ -6,34 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P5f, package fc: the EGL/surface control plane, framed. -// -// WHAT THIS REPLACES. The twelve Server* forwarders used to cross to the apply thread through a -// one-slot mailbox carrying a raw FUNCTION POINTER plus a void* to a stack-local Args struct -// (P5c audit row G4). Both halves are meaningless across a process boundary, which is the whole -// reason P5f exists. The frame below is the seam f0-egl's census prescribed: every forwarder's -// arguments converge into one pure-value struct; inproc the struct crosses the (retained, -// blocking, one-slot) channel by value; under spawn the SAME struct is what SurfaceOpCodec -// encodes into a Wire::SurfaceOp. The thread model does not move - only the payload's shape. -// -// THE FRAME CARRIES NO POINTERS, and that is enforced by the exhaustive member check below: -// adding a member requires updating its structured binding; pointer types fail the assertion. -// The three places the old Args structs held client addresses map onto values: -// -// ServerInitializeEGLDisplay's major/minor out-pointers -> the reply fields eglMajor/eglMinor; -// ServerCreateEGLWindowSurface / ServerSetWindowHandle's const WindowHandle* -> windowBackend + -// nativeToken + width/height (WindowHandle::Handle as an integer); -// every EGL handle (display/surface/context) -> a Uint64 token. Inproc the token IS the handle -// value bit-cast (same process, same driver, and the N-3 tuple bookkeeping compares them); -// under spawn the client mints dense tokens instead (P6-CONTRACT-DRAFT table 0) - the field -// width is already the token's, so nothing here changes shape on that day. -// -// PROCESS-LOCAL WINDOW OBJECTS ARE REFUSED BY NAME on the wire: ANativeWindow* and -// CAMetalLayer* mean nothing in the server process. SurfaceOpCodec names their refusals -// AndroidNativeWindow:use-MOBILEGL_IPC_SURFACE=server and MetalLayer:use-MOBILEGL_IPC_SURFACE=server, -// and they are permanent (Rule H): a client's window never crosses, a windowed client uses -// MOBILEGL_IPC_SURFACE=server, which gives the SERVER a window of its own (kServerOwnedWindowBackend -// below, WindowKind::ServerOwned on the wire). +// The EGL/surface control plane as a value frame. Every Server* forwarder packs its arguments +// into one SurfaceControlFrame: inproc it crosses the blocking one-slot channel to the apply +// thread by value, and under spawn SurfaceOpCodec encodes the same struct as a Wire::SurfaceOp. +// The frame holds no pointers (the exhaustive member check below enforces it): out-pointers +// become reply fields, a WindowHandle becomes windowBackend + nativeToken + size, and EGL +// handles travel as Uint64 tokens (inproc, the handle value itself). Process-local windows +// (ANativeWindow*, CAMetalLayer*) never cross: SurfaceOpCodec refuses them by name, and a +// windowed client uses MOBILEGL_IPC_SURFACE=server (kServerOwnedWindowBackend). #pragma once #include diff --git a/MobileGL/MG_Remote/Transport/AdoptT0.h b/MobileGL/MG_Remote/Transport/AdoptT0.h index 4b51a7fc..36106dc2 100644 --- a/MobileGL/MG_Remote/Transport/AdoptT0.h +++ b/MobileGL/MG_Remote/Transport/AdoptT0.h @@ -6,24 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P11 B2 (docs/archive/contracts/CONTRACT-P11.md B2): T0, the zero-copy adoption of a persistently mapped -// store. The CLIENT allocates the store as an AHardwareBuffer (AHB), keeps a CPU lock on it for -// the store's life and uses that pointer as the adopted mapping; the SERVER imports the same -// pages as the store's resident twin (Espryt glBufferStorageExternalEXT, Magma -// VK_ANDROID_external_memory_android_hardware_buffer). -// -// HOW THE HANDLE CROSSES, AND WHY IT IS ORDERED WITH THE RECORDS THAT USE THE STORE. The adoption -// rides the data-ring map_persistent record (not a control op, which the apply thread could -// overtake under run-ahead). Its AHB travels over the session's aux socket as ONE descriptor: a -// fresh socketpair end (the "hop") on which AHardwareBuffer_sendHandleToUnixSocket already wrote -// the handle, with an Offer in the sideband naming the record's seq. The hop keeps the aux channel -// uniform (one fd + sideband per message, FdPassing's shape on every route) and the AHB's own -// message format off it. Step 1 of B2 proved both this and the direct form on the Redmi across -// every route (shell, another app, same app, spawned child; 64 KiB to 128 MiB). -// -// Every T0 map_persistent is preceded by exactly one Offer for its seq - with the handle, or with -// flags 0 when this client could not allocate one - so the server always has something to wait -// for and never waits for nothing. +// T0: zero-copy adoption of a persistently mapped buffer store (ARCHITECTURE.md §12). The client +// allocates the store as an AHardwareBuffer and keeps it CPU-locked for the store's life as its +// mapping; the server imports the same pages as the store's resident copy. The handle travels +// with the data-ring map_persistent record, not as a control op, which the apply thread could +// overtake under run-ahead: one aux-socket descriptor (a socketpair end the handle was written +// on) plus an Offer naming the record's seq. Every T0 map_persistent is preceded by exactly one +// Offer for its seq, with the handle or with flags 0 when the client could not allocate one, so +// the server never waits for an Offer that is not coming. #pragma once diff --git a/MobileGL/MG_Remote/Transport/AdoptTier.h b/MobileGL/MG_Remote/Transport/AdoptTier.h index 22b7feb2..4fc3f364 100644 --- a/MobileGL/MG_Remote/Transport/AdoptTier.h +++ b/MobileGL/MG_Remote/Transport/AdoptTier.h @@ -6,33 +6,13 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P11 A1 + B2 (docs/archive/contracts/CONTRACT-P11.md §1 and B2): THE PERSISTENT-MAP ADOPTION TIER IS ASKED -// AT THE HANDSHAKE, BY THE SIDE THAT KNOWS THE DATA PLANE, BEFORE THE FIRST map_persistent. -// -// Tier names follow docs/Disaggregated/design/07: T0 = the client allocates an AHardwareBuffer -// and the server imports it; T1 = the server exports an opaque fd (CLOSED, ID-P11-1); T2 = the -// resource owner declines and the client keeps the shadow and pushes. -// -// MOBILEGL_IPC_ADOPT_TIER IS THE SWITCH (ruling ID-P11-14). UNSET - the default since 2026-09-29, -// the user's decision - asks for T0 and falls back to T2 QUIETLY (MGLOG_D only, still counted), -// so a tcp session or a host session is not one line longer; 0 asks for T0 BY NAME (the table -// below); 1 is refused by name and runs T2; 2 is T2 byte for byte. T0 that this session cannot use -// is never a Fatal - it is T2 plus one named line (knob 0) or one D line (unset): -// -// data plane side knob 0 knob 1 -// Stream (tcp) either `Refuse{AdoptTierOnStream, "T0"}`, T2 `Refuse{AdoptTierOnStream, "T1"}`, T2 -// (A1, unchanged: a stream shares no memory, so no tier but T2 exists) -// SharedSegments client asks T0 in its Hello (LinkTerms. `Refuse{AdoptTierClosed, "T1"}`, asks T2 -// adoptTier = 0); whether it runs T0 is -// read at the first map_persistent from -// kCapAdoptT0 (ClientSession::AdoptT0) -// SharedSegments server its own knob is not consulted: the client's ask decides, and the -// server grants it at its first native bind (ServerSession:: -// SettleAdoptT0AtBind) or names why not -// -// In MG_Remote/Transport (the link-ratchet's SERVER partition) because both roles call it: -// ServerSession::Accept and ClientSession's two Start halves. The at-use check the server's -// applier and codec ask on the T2 path (Client::AdoptTierIsEmulate) reads the settlement back. +// The persistent-map adoption tier is settled at the handshake, before the first map_persistent, +// by the side that knows the data plane. T0: the client allocates an AHardwareBuffer and the +// server imports it; T1: a server-exported fd (closed); T2: decline, the client keeps the shadow +// and pushes. A T0 the session cannot use is never Fatal: it is T2 plus one line. The rules per +// data plane and knob value are in ARCHITECTURE.md §12.1. Lives in Transport because both roles +// call it (ServerSession::Accept, ClientSession::Start); Client::AdoptTierIsEmulate reads the +// settlement back on the T2 path. #pragma once diff --git a/MobileGL/MG_Remote/Transport/AuthToken.h b/MobileGL/MG_Remote/Transport/AuthToken.h index 9edc364c..81090698 100644 --- a/MobileGL/MG_Remote/Transport/AuthToken.h +++ b/MobileGL/MG_Remote/Transport/AuthToken.h @@ -6,36 +6,12 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// PH-7 (1)(2)(3), ID-P7-3. THE ONE PLACE THAT KNOWS WHAT `MOBILEGL_IPC_TOKEN` MEANS. -// -// It was three places, and the three did not agree: -// -// SocketTransport.cpp's ListenTcp asked only "is it non-empty", and a one-byte token -// therefore unlocked a `tcp://0.0.0.0` listen. -// ServerSession.cpp compared with `std::strcmp` under "the role is not InProcess AND a token -// is configured". -// ServerMain.cpp compared with `std::string::operator==` under "the control connection is -// TCP", which additionally demanded that a peer present an EMPTY token to a server that had -// configured none - a rule the other site did not have and no document states. -// -// Two of the three compared byte-at-a-time and returned at the first differing byte. That is the -// textbook remote timing oracle: an attacker who can retry learns the token one byte at a time -// instead of guessing 2^128. Whether it is exploitable over a LAN at these timings is beside the -// point; the comparison costs the same either way, and a security property nobody has to -// re-derive is worth more than a measurement. -// -// WHY A MINIMUM LENGTH IS A LISTEN-TIME REFUSAL AND NOT A WARNING. Ph's threat model (ID-P7-3) -// is "anybody who can reach the port has a remote code path". The answer to it is the token, so -// a token an attacker can enumerate is not an answer - it is the appearance of one, which is -// worse than no token at all, because `no token` is already handled: it confines the listener to -// loopback. A four-byte token, before this file, did not confine anything. 16 bytes is the -// floor, it is checked where the listener is opened, and a short token FAILS THE LISTEN rather -// than silently degrading to loopback-only: an operator who set a token and got a loopback -// server would have no way to tell that from success, which is the exact silence Ph exists to -// remove. -// -// The token is never logged, at any level, on any path. The refusals name the policy, not the -// secret (CONTRACT-P65: "错令牌返回 Refuse，不打印令牌值"). +// The one definition of what MOBILEGL_IPC_TOKEN means; listeners and handshake checks use these +// helpers (ARCHITECTURE.md §15.1). No token confines the listener to loopback. A configured +// token shorter than kMinimumAuthTokenBytes fails the listen rather than falling back to +// loopback, since an operator who set a token could not tell that fallback from success. +// Comparisons are constant time: a byte-at-a-time compare lets a peer that can retry learn the +// token one byte at a time. The token is never logged; refusals name the policy, not the secret. #pragma once diff --git a/MobileGL/MG_Remote/Transport/Doorbell.h b/MobileGL/MG_Remote/Transport/Doorbell.h index 8d281305..cc6ef442 100644 --- a/MobileGL/MG_Remote/Transport/Doorbell.h +++ b/MobileGL/MG_Remote/Transport/Doorbell.h @@ -6,50 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// The bidirectional doorbell: spin briefly, then park. -// -// Both directions exist, and that is the point (inherited design, earlier plan -// section 6.2a): -// - client -> server: the consumer spins, sets consumerParked, then blocks; -// the producer rings only when consumerParked is set. -// - server -> client: the client spins MOBILEGL_IPC_SPIN_US (default 50us), -// sets producerParked, then blocks; the server rings after advancing any -// watermark, only when producerParked is set. -// Without the second direction every client wait - present credit, a blocking -// kNeedsAck request, a full ring - degenerates into a cross-process spin on -// one shared cache line: up to a whole frame of a big core at full clock on a -// phone, fighting the GPU and the game's JVM for it. MobileGL has no affinity -// control anywhere in the tree, so it cannot even be pushed to a little core. -// -// Two implementations, no platform-specific wakeup primitive (no futex, no -// eventfd, no named event): -// - CondVarDoorbell for `inproc` (one process, two threads), -// - SocketDoorbell for `spawn` (one byte on a socket; POSIX only). -// -// The lost-wakeup window is closed by two seq_cst FENCES, not by the ordering -// of the park flag's own load and store: -// - the waiter sets the flag, executes std::atomic_thread_fence(seq_cst), -// and THEN re-tests the condition (Doorbell::Wait); -// - the notifier publishes its watermark, executes the same fence, and THEN -// reads the flag (NotifyIfParked). -// Both fences sit in the single seq_cst total order, so one precedes the -// other, and [atomics.order] then forces at least one side to observe the -// other's store. The flag's own accesses may be relaxed: they are not what -// closes the window. -// -// A seq_cst store paired with a seq_cst load would NOT be enough, which is -// why the fences are here and why neither may be removed. That Dekker -// argument needs all FOUR accesses in the total order, and the other two are -// not: the watermark publish is a release store (RingProducer::Publish) and -// the condition re-test is an acquire load. On x86 the gap is concrete rather -// than theoretical - a release store is a plain MOV that can still sit in the -// store buffer while the load of the park flag, also a plain MOV, reads 0, so -// the notifier skips the ring and the waiter parks on a stale watermark -// forever. (ARMv8 survives it only because STLR->LDAR is RCsc, i.e. by luck.) -// -// The other half of the contract is ordering between the caller and the -// fence: NotifyIfParked must be called AFTER the watermark is published. A -// fence only orders what precedes it. +// The bidirectional doorbell: a waiter spins, sets its park flag, then blocks, and the other side +// rings only when that flag is set (client -> server on consumerParked; server -> client on +// producerParked, after any watermark advance). Without the server -> client direction every client +// wait would be a long cross-process spin on one shared cache line. No platform wakeup primitive +// (futex, eventfd, named event): CondVarDoorbell in process, SocketDoorbell across processes. +// Two seq_cst fences close the lost-wakeup window: Doorbell::Wait's (after setting the flag, before +// re-testing) and NotifyIfParked's (after publishing, before reading the flag). Neither may be +// removed, and NotifyIfParked must follow the publish; why: ARCHITECTURE.md §11.3.1. #pragma once @@ -83,45 +47,14 @@ namespace MobileGL::MG_Remote::Transport { #endif } - // ----------------------------------------------------------------------- - // THE SPIN BUDGET IS COUNTED IN ITERATIONS, NOT IN CLOCK READS. - // - // WHY IT CHANGED (P5d round 3, package T item 3). Wait() used to spin until - // steady_clock passed `start + spinUs`, reading the clock once per 64 spins. - // On the workload device a steady_clock read is a real syscall (no vDSO), and - // simpleperf at head 56a77348 measured the GL thread paying - // steady_clock::now() 2.34 self / 5.91 inclusive plus __kernel_clock_gettime - // 2.37 and clock_gettime 1.94 - about 6.6% of the thread spent asking what - // time it is, at 900+ waits per frame, for a duration that is a HINT. The - // common case (WaitForApplied, server not yet there) paid at least two of - // those reads per wait even when the spin succeeded on its second iteration. - // - // WHAT REPLACES IT. Once per process the loop measures itself: how many - // `load(acquire)` + CpuRelax() iterations fit in a microsecond. Wait() then - // spins `itersPerUs * spinUs` iterations and reads no clock at all; only the - // park phase - which is about to block, so a syscall there is noise - computes - // a deadline. - // - // THE OVERSHOOT BOUND, STATED HONESTLY. The old loop overshot by at most one - // batch of 64 yields. This one overshoots by whatever the calibration was - // wrong by, and the calibration is a single reading taken on one core at one - // frequency: - // - a later DVFS drop or a migration to a little core stretches the spin by - // the frequency ratio (on the workload device's big/little spread that is - // bounded by roughly 3-4x, so a 50 us budget can become ~200 us); - // - a preemption inside the spin adds one scheduler slice, exactly as before; - // - the calibration probe does not run the caller's `ready()`, which is one - // or two acquire loads, so a real iteration is dearer than a probed one and - // the spin ends EARLY rather than late by that factor; - // - the clamps below cap the pathological directions: a probe that was - // preempted cannot produce a budget smaller than kMinSpinItersPerUs, and a - // probe that saw a broken clock cannot produce one larger than - // kMaxSpinItersPerUs. - // None of it is a correctness bound: `ready()` is re-tested every iteration - // and the deadline, when there is one, is still measured against the real - // clock in the park phase. The cost of being wrong is CPU burnt before a park - // (too long) or a park that a few more spins would have avoided (too short). - // ----------------------------------------------------------------------- + // The spin budget is counted in iterations, not clock reads: CalibrateSpinItersPerUs measures + // once per process how many acquire-load + CpuRelax() iterations fit in a microsecond, and Wait + // spins itersPerUs * spinUs without reading a clock (a steady_clock read can be a syscall, and + // Wait runs hundreds of times per frame). Only the park phase computes a deadline. The budget + // is a hint, not a correctness bound: `ready()` is re-tested every iteration; a DVFS drop or a + // move to a little core stretches the spin by the frequency ratio (roughly 3-4x); the probe + // omits `ready()`, so real spins end early; the clamps in CalibrateSpinItersPerUs bound bad + // readings. namespace Detail { // 0 until the first Wait calibrates it. Plain relaxed atomic rather than a // function-local static: a static's guard variable is a load and a branch on @@ -182,29 +115,12 @@ namespace MobileGL::MG_Remote::Transport { // peer process, which is what makes the whole latch a no-op under inproc and monolith. virtual bool PeerHungUp() const { return false; } - // How many Wait() calls on this bell exhausted their spin budget and - // really blocked. THE WAIT LEDGER'S RAW READING: the pair (waits, parks) - // is what says whether the spin budget is sized for the workload, and it - // is the one number a wait counter cannot give - a wait that spun and a - // wait that blocked cost three orders of magnitude apart. - // - // ONCE PER Wait, NOT ONCE PER Park: a wait that blocks, wakes on a - // remembered Notify, re-tests false and blocks again is ONE wait that - // paid the park, and a `chunkMs == 0` Park is a poll that does not block - // at all. Both are counted out at the site below, so parks <= waits holds - // for a single waiter. - // - // PER BELL, THOUGH, NOT PER WAIT PATH. A bell belongs to an ENDPOINT and - // every waiter on that endpoint shares it: the client's self bell carries - // SessionProducer's barrier waits AND the encoder's SEG_STAGE retirement - // wait (PipeWireCodec's m_stageRetirementBell, which is the same object). - // A ledger that prints parks as the subset of ITS OWN waits must pass a - // `parkTally` to Wait rather than read this; this counter answers "parks - // on this bell", which is what a test on a private bell wants. - // - // Read from any thread; the counter is monotone and relaxed, so a reader - // may see a value one behind, which is exactly what a per-frame gauge - // wants. + // How many Wait() calls on this bell exhausted their spin budget and really blocked, + // counted once per Wait (re-parks and zero-length polls are not counted), so parks <= waits + // for one waiter. The count is per bell, and a bell is shared by every waiter on its + // endpoint (the client's self bell also carries PipeWireCodec's SEG_STAGE retirement wait); + // a caller that needs parks as a subset of its own waits passes `parkTally` to Wait + // instead. Monotone and relaxed: a reader on another thread may see a value one behind. std::uint64_t ParkEntries() const { return m_parkEntries.load(std::memory_order_relaxed); } // Spin `spinUs`, then park until `ready()` or the deadline. @@ -221,24 +137,9 @@ namespace MobileGL::MG_Remote::Transport { return true; } - // THE SPIN PHASE READS NO CLOCK AT ALL. It used to read one per 64 - // iterations plus one before the loop - see the SpinItersPerUs block - // above for the measurement that made even that too dear, and for the - // overshoot bound the iteration budget buys instead. The duration is - // still a hint; what changed is that asking for it is free. - // - // spinUs == 0 NOW MEANS ZERO ITERATIONS, AND THAT IS A DELIBERATE CHANGE - // rather than a side effect. The old batched loop tested - // `(++spinBatch & 63) != 0 || now < spinEnd`, so a caller that asked for - // no spin still ran 63 iterations before the first clock read could end - // the loop. That was an artifact of the batch size, never a promise, and - // multiplying a budget the caller deliberately set to zero would be the - // wrong way to keep it. Two callers pass 0: PipeWireCodec's SEG_STAGE - // retirement wait (rare, already slow, and it wants the park) and any - // lane that sets MOBILEGL_IPC_SPIN_US=0 as a no-spin control, which now - // really gets no spin - which is the point of such a control. The - // short-circuit also keeps a zero-spin caller from paying the one-time - // calibration probe for a reading it will never multiply by. + // The spin phase reads no clock (see SpinItersPerUs). spinUs == 0 means no spin at all + // and skips the calibration probe: PipeWireCodec's SEG_STAGE retirement wait passes 0 + // because it wants the park, and MOBILEGL_IPC_SPIN_US=0 is a no-spin control. const std::uint64_t budget = spinUs == 0 ? 0ull : static_cast(SpinItersPerUs()) * @@ -294,27 +195,12 @@ namespace MobileGL::MG_Remote::Transport { std::chrono::duration_cast(deadline - now).count(); chunkMs = remaining <= 0 ? 0 : static_cast(remaining); } - // COUNTED HERE, at the one place a waiter stops spinning and - // really blocks (P5d round 3, package T item 4). Unconditional - // rather than behind the stats latch: it is one relaxed add on a - // path that is about to enter a condition variable or a poll(), - // and a counter that only exists when MOBILEGL_PIPE_STATS is set - // could not be read by a test that wants to prove the spin budget - // avoided the park. - // - // TWO THINGS ARE COUNTED OUT, because this number is published as - // the SUBSET of the waits that blocked, and a subset that can - // exceed its own denominator is worse than no number at all: - // - RE-PARKS. One Wait can traverse this loop many times: a - // CondVarDoorbell remembers a Notify delivered while nobody - // was waiting, so Park returns at once, `ready()` is still - // false (the peer had already drained what it published) and - // the wait parks again. That is one wait that paid a park, not - // two, so only the first is counted. - // - POLLS. `chunkMs == 0` is the last sub-millisecond of a - // finite timeout rounding down; Park(0) does not block, and - // counting it would charge a park per iteration of a tight - // re-test loop that never left the CPU. + // Counted here, where a waiter really blocks, and unconditionally (one relaxed + // add before a condvar wait or poll), so a test can read it without + // MOBILEGL_PIPE_STATS. Only the first blocking Park of a Wait counts: a + // CondVarDoorbell can return at once on a remembered Notify and re-park, and + // chunkMs == 0 is a non-blocking poll at the end of a finite timeout. That keeps + // parks a subset of waits. if (chunkMs != 0 && !countedPark) { countedPark = true; m_parkEntries.fetch_add(1, std::memory_order_relaxed); @@ -438,23 +324,13 @@ namespace MobileGL::MG_Remote::Transport { int Fd() const { return m_fd; } - // P6 `dl` (CONTRACT-P6 D5c). THE DESCRIPTOR THAT ANSWERS "IS THE PEER STILL THERE", - // which for this object can never be the one it parks on. - // - // The client parks on clientBell[0] and rings ITSELF through clientBell[1], so it holds - // BOTH ends of that socketpair. EOF arrives only when every writer closes and the client - // is one of them, so that descriptor cannot hang up no matter what happens to the server. - // A bell that can wake itself is a bell that cannot hear a death. Measured: a server that - // died on its first Clear left the client waiting out the full 120 s barrier and - // reporting Fatal{BarrierTimeout} - the wrong diagnosis, two minutes late. - // - // The witness is a descriptor whose FAR END ONLY THE PEER HOLDS - in practice the control - // socket. It is NOT OWNED here and is NEVER READ FROM: Park adds it to the poll set with - // `events` asking for hangup ALONE, so a control reply sitting unread in its queue cannot - // wake the bell and cannot be consumed from under SocketTransport's reassembler. What it - // contributes is exactly one fact, and only once: the peer is gone. - // - // -1 disables it, which is what every bell that has no peer socket uses. + // The death witness: a descriptor whose far end only the peer holds (in practice the + // control socket). The park descriptor cannot report the peer's death when this side also + // holds the notify end of the same socketpair (the client rings itself through + // clientBell[1]), so without a witness a dead server surfaces only as a barrier timeout. + // Not owned and never read: Park polls it for hangup only, so unread control replies + // neither wake the bell nor get taken from SocketTransport's reassembler. A hangup sets + // PeerHungUp(). -1 disables it. void SetDeathWitness(int fd) { m_witnessFd = fd; } private: diff --git a/MobileGL/MG_Remote/Transport/EventRing.h b/MobileGL/MG_Remote/Transport/EventRing.h index 9d771cdd..0978e68c 100644 --- a/MobileGL/MG_Remote/Transport/EventRing.h +++ b/MobileGL/MG_Remote/Transport/EventRing.h @@ -6,30 +6,13 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// SEG_EVENT: the server -> client reverse channel. Owner: package s1. -// -// WHAT P5 OWES HERE AND NOTHING MORE (BRIEF R-12, and s1's brief): it must be -// able to CARRY OnBufferWriteback, OnGpuWritten and OnSurfaceChanged. The other -// seven MGPipeCallbacks members are off P5's reduced path, and THE OVERFLOW -// POLICY IS P9's - what is here is the mechanism (a full ring latches -// eventRingFull, a lossy post that is dropped counts in eventDropped) and not a -// policy that decides between them. -// -// IT IS A SECOND RingControl, NOT A THIRD CURSOR SET. RingControl carries two -// cursor triples (SEG_CMD and SEG_STAGE) and adding a third would resize the -// shared page that Ring.h static_asserts at exactly 4096 bytes. So SEG_EVENT -// gets its OWN control page at its own head and drives it with the Cmd cursor -// set - the same RingProducer/RingConsumer code, in the opposite direction. The -// two EVENT FLAGS still live in the SEG_CMD page, because that is where Ring.h -// declares them and where the client's own waits already look. -// -// THE PAYLOAD SHAPES ARE FIXED-WIDTH AND LIVE HERE, not in MG_Pipe's headers. -// Nothing under Transport/ may reach MobileGL/Includes.h (WireLog.h:9-24, and -// the purity gate's `wire-header` probe), so MGPipeHandle / MGPRange / -// MGPSurfaceInfo cannot be named in this file. The wire shapes below mirror them -// field for field and the SESSIONS assert the two agree, which is the same -// discipline the rest of the wire is held to: a wire struct is fixed-width, and -// the conversion happens where the frontend types are legal. +// SEG_EVENT: the server-to-client reverse channel for MGPipeCallbacks events; the overflow rules +// (eventRingFull, eventDropped) are ARCHITECTURE.md §11.7. It has its own RingControl page, +// driven with the Cmd cursor set by the same producer/consumer code in the opposite direction, +// because a third cursor set would grow RingControl past its fixed 4 KiB page. The two event +// flags stay in the SEG_CMD page, where Ring.h declares them. Nothing under Transport/ may +// include MobileGL/Includes.h, so the payloads below are fixed-width mirrors of MGPipeHandle, +// MGPRange and MGPSurfaceInfo, converted by the sessions where the frontend types are legal. #pragma once diff --git a/MobileGL/MG_Remote/Transport/FdPassing.h b/MobileGL/MG_Remote/Transport/FdPassing.h index 66989e52..d9b1b3db 100644 --- a/MobileGL/MG_Remote/Transport/FdPassing.h +++ b/MobileGL/MG_Remote/Transport/FdPassing.h @@ -6,24 +6,12 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// SCM_RIGHTS descriptor passing over an AF_UNIX socket pair. POSIX only. -// -// This is the FIRST transport commit, deliberately (inherited design, plan -// section 8.1, "SCM_RIGHTS must be implemented in the first transport -// commit"). The earlier branch pushed it to a later phase and hardcoded -// `out->fd = -1` in its offer poll, so on the only platform that matters its -// data plane could never move a byte: every segment announcement resolved to -// "no descriptor". A transport whose shm cannot cross the process boundary is -// not a transport. -// -// Channel shape: a dedicated AF_UNIX SOCK_DGRAM socketpair, NOT the control -// byte stream. Two reasons: -// - SOCK_DGRAM preserves message boundaries on every POSIX (SOCK_SEQPACKET -// does not exist on macOS), so one sendmsg is exactly one recvmsg and the -// ancillary data can never be split away from its payload; -// - ancillary data attached to a byte stream binds to whichever ordinary -// byte happens to be at the front of the reader's buffer, which is -// unmanageable once frames are being reassembled. +// SCM_RIGHTS descriptor passing over an AF_UNIX socket pair (POSIX only); without it the shared +// segments cannot cross the process boundary. The channel is a dedicated SOCK_DGRAM socketpair, +// not the control byte stream: datagrams keep message boundaries on every POSIX (macOS has no +// SOCK_SEQPACKET), so one sendmsg is one recvmsg and ancillary data stays with its payload; on a +// byte stream it would bind to whichever byte is at the front of the reader's buffer, which the +// frame reassembler cannot manage. #pragma once diff --git a/MobileGL/MG_Remote/Transport/Framing.h b/MobileGL/MG_Remote/Transport/Framing.h index 7de27b7f..dac18aa7 100644 --- a/MobileGL/MG_Remote/Transport/Framing.h +++ b/MobileGL/MG_Remote/Transport/Framing.h @@ -6,26 +6,13 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// Control-channel wire framing: [u32 magic 'MGLF'][u32 payloadLength][payload]. -// Length excludes the 8-byte header and is capped at 64 MiB. -// -// Two defects of the earlier branch's codec are fixed here, and both are the -// reason this file is not a copy of it: -// -// 1. Its Feed() unconditionally returned OK and its header peek merely -// returned false on a bad magic or an oversized length. A corrupt or -// desynchronized stream therefore turned into a silent, permanent hang - -// the reader kept waiting for a message that could never be parsed, with -// no error anywhere. Here a violation latches a failed state, is logged at -// ERROR, and every later call returns MOBILEGL_ERR_PROTOCOL_MISMATCH. -// -// 2. Its receive path failed the call and consumed the message when the -// caller's buffer was too small, wedging the stream. Here -// MOBILEGL_ERR_BUFFER_TOO_SMALL reports the required size and KEEPS the -// message queued. -// -// The reader is a plain byte-stream reassembler: it never assumes a read() -// returned a whole frame. +// Control-channel wire framing: [u32 magic 'MGLF'][u32 payloadLength][payload]. The length +// excludes the 8-byte header and is capped at 64 MiB. +// A bad magic or an oversized length latches the reader failed, is logged at ERROR, and makes +// every later call return MOBILEGL_ERR_PROTOCOL_MISMATCH, so a desynchronized stream fails rather +// than hangs. A buffer too small for the next message gets MOBILEGL_ERR_BUFFER_TOO_SMALL with the +// required size and the message stays queued. The reader is a byte-stream reassembler: it never +// assumes a read() returned a whole frame. #pragma once diff --git a/MobileGL/MG_Remote/Transport/ILink.h b/MobileGL/MG_Remote/Transport/ILink.h index 84c432b0..2094be49 100644 --- a/MobileGL/MG_Remote/Transport/ILink.h +++ b/MobileGL/MG_Remote/Transport/ILink.h @@ -6,64 +6,13 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// The DATA-plane seam. Declared by package c6, implemented by package lk. -// Authority: CONTRACT-P6.md §8. -// -// WHY THIS EXISTS AND WHY IT IS NOT ITransport. ITransport.h:16-20 says in its -// own header that everything on the hot path bypasses it: it carries the -// handshake, surface ops, resync, aux requests and fatals, and that scope is -// already correct for a socket. Below it there was NO interface of any kind - -// Ring.h, ReplySlot.h and EventRing.h each take a raw `void* base` plus a -// capacity, so "swap the transport" had nothing to swap. This header is that -// missing thing, and ITransport is neither extended nor changed by it. -// -// NOTHING INCLUDES THIS FILE IN c6. That is the whole of c6's behaviour -// guarantee: the seam is declared, StreamLink.h is declared against it (which -// is the adequacy proof - see below), and not one call site moves. Package lk -// implements ShmLink by relocating today's SessionSegments / Ring / -// ReplySlotPool / EventRing behind it, line for line, and only then rewrites -// the call sites. -// -// THE SEAM IS SETUP-TIME, NOT HOT-PATH. No method here is called per record, -// per publish, or inside a spin predicate. The producer takes a LinkArena once -// at attach and reserves through it non-virtually; every wait predicate reads -// LinkProgress through a raw pointer taken once. Three grounds, the second -// decisive (CONTRACT-P6.md §8.2): -// -// 1. Two virtuals per record at the 4000 records/frame SEG_CMD is sized -// against is ~8000 indirect calls/frame, about 0.5% of the measured -// 15.72 ms client frame - affordable, but the only lane that could -// measure it has +/-4% dispersion, EIGHT TIMES the effect. A gate that -// can only ever print "within noise" is ID-124's failure mode one level -// down. -// 2. Doorbell::Wait is a non-virtual template whose spin loop calls ready() -// up to SpinItersPerUs() * spinUs times; at 916 waits/frame with 99.8% -// resolved inside the spin, the predicate runs 1e5-1e6 times per frame -// and reads the watermarks straight off the shared page. That is the -// 31.58%-of-the-GL-thread path. A virtual there is not a rounding -// question. -// 3. The encode path already depends on four inline accessors around its one -// Reserve; virtualising Reserve alone forces either four more virtuals or -// moving R-10's wrap counters behind the seam, and those are deliberately -// encoder-owned. -// -// So lk's gate is an objdump call-count of the encode path, not a wall clock: -// after lk that path has no more out-of-line calls than before. That is -// falsifiable; "within noise" is not. -// -// WHAT THIS HEADER DELIBERATELY DOES NOT DO. -// - It does not define Watermark::Reached or any other predicate that -// SessionRings.h already defines. One definition, one owner. -// - It does not relocate the bodies of RingProducer::Reserve or -// RingConsumer::Pop. Those log (MGLOG_E / MGLOG_D) and use Align8 and -// ; no header under Transport/ pulls the logger in, and this one -// does not start. They stay out of line and move to ShmLink.cpp in lk. -// - It declares no function it does not define. Every entry point here is -// pure virtual or a POD accessor. -// - It does not regroup RingControl. That regroup is real and necessary -// (CONTRACT-P6.md §8.3) but its rename blast radius is 109 production -// sites, five of them behind a non-Android #if, so it is lk's with a -// published per-file itemisation - not a side effect of declaring a seam. +// The data-plane seam: record reservation and commit, watermarks, replies, events and span +// resolution, implemented by ShmLink and StreamLink (ARCHITECTURE.md §11.9). ITransport carries +// only the control plane and is not part of it. +// Reserve, pop and the wait predicates never dispatch through it: callers cache the ring +// endpoints (CommandsOut/CommandsIn) and the LinkProgress pointer once at attach. The spin +// predicate runs up to 1e5-1e6 times a frame, where a virtual call is a measurable cost. +// Watermark predicates such as Watermark::Reached stay in SessionRings.h. #pragma once @@ -94,44 +43,13 @@ namespace MobileGL::MG_Remote::Transport { }; // ---- the progress block ------------------------------------------------- - // - // The watermarks every wait predicate reads. On ShmLink this aliases the - // shared control page and the loads are cross-process acquire loads; on - // StreamLink it is process-local and a reader thread stores into it from - // decoded progress messages. - // - // THE RULES SURVIVE THE MECHANISM VERBATIM, and that is the point of naming - // the block rather than the page: late-never-early (Ring.h's "BATCHING MAY ONLY MAKE A WATERMARK LATE"), `>=` - // never `==` (SessionRings.h's Watermark::Reached), monotone with a - // backwards move Fatal (Ring.cpp's AdvanceMonotonic). A stream link changes - // only HOW a value arrives, never what it means. - // - // WHY THESE FOUR AND NOT THE WHOLE PAGE, AND WHY THE EVENT FLAGS ARE NOT - // HERE. RingControl's watermark cache line HELD, until lk regrouped it, {appliedSeq, - // submittedSeq, retiredSeq, completedFrameSerial, presentAckSerial}, and - // submittedSeq is PRODUCER-written (Ring.h's "THE FIVE WATERMARKS" block, - // "Advanced by the PRODUCER after it publishes") while the other four are consumer-written - so the - // line has MIXED WRITERS as it stands and no single-writer block can be - // aliased over it. lk's regroup moves submittedSeq out to the producer's - // own line, beside cmdHead, which is the other thing the producer writes; - // what is left IS this struct, and it IS a named member of RingControl, - // pinned there by per-field offsetof static_asserts (Ring.h). - // - // THE TWO EVENT FLAGS ARE DELIBERATELY EXCLUDED, and an earlier draft of - // this header had them in and was wrong. eventRingFull has TWO writers: - // the server latches it with store(1) when SEG_EVENT fills - // (EventRing.h:140) and the CLIENT clears it with an exchange(0) on EVERY - // DRAIN (EventRing.h:200). Promoting it here would put a per-drain RMW by - // the client on the same cache line as appliedSeq - which the server - // release-stores every 64 records and the client's spin predicate reads - // 1e5-1e6 times per frame. That is new false sharing on the hottest line - // in the system, and it would land in exactly the numbers lk's gate has to - // hold constant. They stay in the doorbell/generation group, where mixed - // writers already live beside consumerParked/producerParked, and cross the - // seam through EventFlags() instead. - // - // It was free to regroup while both peers are the same binary; a wire break the day a second build exists, - // which is why it is booked now rather than later. + // The watermarks every wait predicate reads: on ShmLink an alias of the shared control page, + // on StreamLink process-local, stored by a reader thread from progress messages. Both keep + // the same rules: a watermark may arrive late, never early (Ring.h), compare with `>=` + // (Watermark::Reached), and a backwards move is Fatal (AdvanceMonotonic). It holds only + // consumer-written fields, so it is one RingControl member on its own line (ARCHITECTURE.md + // §11.2). The event flags stay out: the client clears eventRingFull on every drain, an RMW + // that would share the cache line the client's spin predicate reads for appliedSeq. struct LinkProgress { std::atomic appliedSeq; // records applied std::atomic retiredSeq; // GPU finished diff --git a/MobileGL/MG_Remote/Transport/ITransport.h b/MobileGL/MG_Remote/Transport/ITransport.h index 50e64950..3c90b874 100644 --- a/MobileGL/MG_Remote/Transport/ITransport.h +++ b/MobileGL/MG_Remote/Transport/ITransport.h @@ -6,27 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// The control-plane transport interface. -// -// It is deliberately dumb: complete messages in, complete messages out, plus -// the one thing shared memory cannot do without help - handing a file -// descriptor to the peer. No session routing, no seq accounting, no -// serialization; those live above, in the protocol layer. -// -// Everything on the hot path bypasses this interface entirely: records go into -// the SEG_CMD ring (Ring.h) and the peer is woken through a Doorbell -// (Doorbell.h). ITransport carries the handshake, surface ops, resync, aux -// requests and fatals - the rare, variable-length, must-evolve traffic that -// plan section 7.1 assigns to FlatBuffers tables. -// -// This header stays dependency-light on purpose (mg_protocol_base.h plus the -// standard library): it is included by both roles and by the eventual -// server-side binary, and nothing about a byte pipe needs the GL frontend's -// umbrella header. -// -// Threading: one instance is not internally synchronized for send; callers -// serialize sends. ReceiveFrame/ReceiveFd may be called from one dedicated -// reader thread concurrently with sends from another. +// The control-plane transport interface: complete messages in, complete messages out, plus +// handing a file descriptor to the peer. No session routing, seq accounting or serialization; +// those live in the protocol layer. Hot-path records bypass it (Ring.h, Doorbell.h); it carries +// the handshake, surface ops, aux requests and fatals (ARCHITECTURE.md §11.4). +// Dependency-light (mg_protocol_base.h and the standard library): both roles include it, and a +// byte pipe needs nothing from the GL frontend's umbrella header. +// Threading: callers serialize sends; one reader thread may call ReceiveFrame/ReceiveFd +// concurrently with sends from another. #pragma once @@ -60,23 +47,12 @@ namespace MobileGL::MG_Remote::Transport { // Receives the next complete message. // - // MOBILEGL_OK - copied into `buffer`, *outSize is - // the message size, message consumed. - // MOBILEGL_ERR_BUFFER_TOO_SMALL - `buffer` is too small. *outSize is - // the size required and THE MESSAGE - // STAYS QUEUED: call again with a - // buffer of at least that size and it - // is still there. - // MOBILEGL_ERR_TIMEOUT - nothing arrived within timeoutMs - // (0 = non-blocking poll). - // MOBILEGL_ERR_TRANSPORT_CLOSED - peer gone, nothing left buffered. - // MOBILEGL_ERR_PROTOCOL_MISMATCH- framing violated; the transport is - // latched failed and never recovers. - // - // The buffer-too-small half of that contract is the whole point of - // having one: the earlier branch's transport failed the call AND - // dropped the message, which wedges the stream permanently the first - // time a message is bigger than the reader's guess. + // MOBILEGL_OK - copied into `buffer`, *outSize is its size, consumed. + // MOBILEGL_ERR_BUFFER_TOO_SMALL - *outSize is the size required; the message stays + // queued for a call with a large enough buffer. + // MOBILEGL_ERR_TIMEOUT - nothing arrived within timeoutMs (0 = poll). + // MOBILEGL_ERR_TRANSPORT_CLOSED - peer gone, nothing left buffered. + // MOBILEGL_ERR_PROTOCOL_MISMATCH - framing violated; the transport stays failed. virtual MobileGLResult ReceiveFrame(MobileGLMutableByteSpan buffer, std::uint64_t* outSize, std::uint32_t timeoutMs) = 0; diff --git a/MobileGL/MG_Remote/Transport/InProcessTransport.h b/MobileGL/MG_Remote/Transport/InProcessTransport.h index efa8c005..cb08ec36 100644 --- a/MobileGL/MG_Remote/Transport/InProcessTransport.h +++ b/MobileGL/MG_Remote/Transport/InProcessTransport.h @@ -6,30 +6,13 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// The `inproc` transport: two in-memory message queues and a pair of condvar -// doorbells, one connected endpoint at each end. -// -// It is not a test double. `inproc` is a delivery mode of its own - the server -// side is the monolith's own render thread, which is the single largest CPU -// lever this project has, and it is also the CI form of the split build. What -// it does NOT exercise is serialization of the byte stream, so the framing -// codec is covered separately by FramingTest. -// -// It is built by MOBILEGL_BUILD_DISAGGREGATED, the one option this skeleton -// adds, and selected at RUNTIME (plan appendix B: MOBILEGL_TRANSPORT = -// monolith / inproc / spawn / ...). The plan also reserves a separate -// MOBILEGL_BUILD_DISAGGREGATED_INPROC option for the role-isolation shim that -// a single-process CI build will need; that option does not exist yet, and -// nothing here depends on it. -// -// Messages are queued whole, so no framing bytes are involved; the size cap is -// still enforced so that a payload which would be illegal on a socket is -// illegal here too and does not pass CI only to fail after the switch to -// `spawn`. -// -// Descriptor passing is a plain dup(): both ends are the same process, so -// there is nothing to transfer, but the API stays identical so callers can be -// written once. +// The `inproc` transport: two in-memory message queues and a pair of condvar doorbells, one +// connected endpoint at each end. Not a test double: inproc is a delivery mode of its own and +// the CI form of the split build. It carries only the control plane; records use the same +// shared-segment rings as spawn. Messages are queued whole, so framing is not exercised here +// (FramingTest covers it), but the size cap is enforced so a payload illegal on a socket fails +// here too. Descriptor passing is a plain dup(), since both ends are one process, behind the +// same API as the socket transport. Selected at run time by MOBILEGL_TRANSPORT=inproc. #pragma once diff --git a/MobileGL/MG_Remote/Transport/LogForward.h b/MobileGL/MG_Remote/Transport/LogForward.h index 958bbff1..a1223437 100644 --- a/MobileGL/MG_Remote/Transport/LogForward.h +++ b/MobileGL/MG_Remote/Transport/LogForward.h @@ -6,65 +6,13 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P9 W3. THE SERVER'S LOG LINES ON THE CONTROL PLANE, GRADED, AND NEVER A REASON TO STOP APPLYING. -// -// On TCP the server forwards every line it logs to its client as a LogLine control frame, and the -// client files it under `.server.log` (ControlInbox.cpp). Until this file the forward was a -// blocking SocketTransport::SendFrame made by the LOGGING THREAD, inside the logger: a client that -// stopped reading its control connection - a GL thread parked in a debugger, a ControlInbox whose -// bounded queue filled, a peer that simply stopped - filled the socket, and the next MGLOG on the -// apply thread never returned. Every record behind it waited; a forfeit that would have ended the -// session by name (ServerSession::ForfeitReverseChannel) was itself a log line and never finished -// being said. Design/04 §8.1 had written the policy down from the start (<= WARN lossy, >= ERROR -// lossless and rate limited), and LogLine.level has been on the wire for it since P6; nothing -// used either, and every line went out as Info. -// -// THE SHAPE. A LogForwardChannel owns a queue and ONE sender thread; the forwarder the logger -// calls (Offer) only enqueues, so the logging thread never touches the socket. The sender is the -// only thread that writes log frames, and it writes them in the order they were offered. -// -// THE POLICY, per severity (MOBILEGL_LOG_LEVEL_*): -// * DEBUG / INFO / WARN are LOSSY. A line that would take the queue's lossy bytes past -// LogForwardLimits::lossyQueueBytes is dropped and COUNTED. The drop is not silent to the -// client either: when the gap closes - the next line at or below WARN that fits, a LogFlush -// ack, or Close - the channel puts a WARN line of its own in the stream, `LogForward{Dropped} - -// N line(s) at or below WARN (B bytes) were not forwarded, from