6 Commits

Include renames

Author	SHA1	Message	Date
swung0x48	901d48a678	[Fix] (MGPipe, Metrics, Config, CI): exchange the per-frame stats instead of racing a store, carry the three uncarried table entries, drop the inline host span from a buffer range, spell the buffer subdata range, and close the small gate holes ... - S-1 PipeStats::OnPresent read each frame accumulator and then store(0)'d it; a Bump from a staging thread landing in between was lost from the Tracy plot and from every frame. Each accumulator is now exchange(0, relaxed) and the exchanged value is what is plotted, so every add lands in exactly one frame. - T-3 FdPassing without MSG_CMSG_CLOEXEC (macOS, BSD) handed back descriptors that survived exec; every received fd now gets FD_CLOEXEC by hand under !MSG_CMSG_CLOEXEC. MSG_NOSIGNAL is defined to 0 where the platform lacks it (FdPassing.cpp, Doorbell.cpp) and SO_NOSIGPIPE is set on the socketpair and on a SocketDoorbell's descriptor where it exists, so a write to a hung-up peer is EPIPE rather than a fatal signal. - T-4 the missing-flatbuffers fallback wrote OFF into the cache with FORCE, so a plain re-configure after `git submodule update` stayed OFF silently. It is a normal-variable set now, shadowing the cache for that configure only; verified by hiding flatbuffers.h, configuring with ON (warning, transport off, cache still ON) and re-configuring plainly with the header back (transport ON). - P-2 three LIVE GLFunctionsTable entries had no carrier: GetGpuTimestampNs (glGetInteger64v(GL_TIMESTAMP), a synchronous server answer), QueryCounterTimestamp (glQueryCounter, a one-shot stamp, not a begin/end pair) and WaitSync (the GPU-side wait FenceWait's client wait does not express). QueryTimestamp (MGPTimestampRequest, kCtxQuery, kReplySlot), QueryCounter (MGPQueryDesc with Kind = GL_TIMESTAMP, kCtxQuery) and FenceWaitServer (MGPFenceWait, kScreen) are APPENDED at the end of PipeCalls.def because the opcode is the position: SetSwapInterval stays 68, the three take 69-71, and PipeCatalogue.LateArrivalsAreAppendedWithoutRenumbering pins that. Header counts 71 (screen 11, query 8); the seven generators regenerated. - P-3 MGPBufferRange inlined a 32-byte MGHostSpan into every range of every class - dead space on every SSBO, atomic-counter and XFB range, and D-B8 says not to freeze the named-UBO payload before the stage-ubo-named numbers exist. The range is 24 bytes now; the host spans are an optional second var-tail behind the ranges, announced by MGPShaderBuffers::HostSpanCount (0 or Count), with set_shader_buffers keeping its kVarTail|kHostSpan flags. PipeCatalogue.BufferRangeCarriesNoInlineHostSpan pins the sizes, the flags and the comparator's view of the count. - P-4 QueryEnvUint64 parsed with base 0 (a leading zero meant octal: MOBILEGL_PIPE_PUSH=010 read as 8) and accepted -1 as every bit set; it is decimal or explicit 0x now and a '-' anywhere is refused with the warning (smoke through the integration binary: -1 and 12abc warn, 010 and 0x10 parse). The CI stdio gate's alternation now also catches fprintf(stdout, puts( and std::cout/cerr; it is green over MG_Backend and MG_State. MGPSubData states how the buffer half expresses [offset, size): UnionBox.X / UnionBox.W with Target == Buffer, Y = Z = 0, H = D = 1, one record bounded at a 2^31-1 offset and 2^32-1 size beyond which the emitter splits (the same rule the ring's half-capacity bound already imposes); MGPipeSetSubDataBufferRange / MGPipeSubDataBufferOffset / Size are the only spelling and PipeCatalogue.SubDataBufferRangeRidesInTheUnionBox pins the encoding and its bounds. gen_pipe.py now refuses, in both modes, a call payload named in PipeCalls.def with no field list in PipeFields.def (the four memcmp-fallback member types are the documented exception); shown by dropping P(MGPSwapInterval), which exits 1 naming the payload. - The MGPPixelPackState size assertion compared sizeof against itself; it asserts the literal 28 PixelStoreParameters measures. - Verified: ctest -L unit green in both the default and the split configuration, gen_pipe.py --check clean with the generated files committed, nm --defined-only of the default libMobileGL.so has no MG_Remote symbol, and the full integration-gpu suite passes (the *IsActuallyArmedWhenTheEnvironmentPinsItOn family trips under -j 8 as documented and passes serially).	2026-09-05 21:19:00 -04:00
swung0x48	1154f9a00d	[Fix] (MG_Remote, Transport): give the inproc doorbell a death state so Shutdown can join a parked waiter, and bound a ring record at half the capacity so a refusal can never look like backpressure ... - T-1: InProcessChannel::Close rang each CondVarDoorbell once and claimed that unparks a peer mid-frame. It does not. Doorbell::Wait consumes the one ring, re-tests a condition nothing published, finds the bell alive (CondVarDoorbell never overrode Dead(); Doorbell.cpp had no death state at all) and with kWaitForever parks again for good - so Shutdown could never join a server thread sitting in the design's own steady state (spun, set consumerParked, blocked; plan section 8.1 inheriting the earlier plan's 6.2a). CondVarDoorbell now carries an atomic death latch: Kill() sets it under the mutex and notify_all's, Dead() reports it, Park returns false at once on a dead bell (and the wait predicate includes it, so a Kill cannot slip between the test and the wait), and Close kills both bells instead of ringing them. Same shape as SocketDoorbell's EOF latch; Notify stays the ordinary wakeup. - T-2: RingProducer::Reserve refused only total > capacity, but a record with capacity/2 < total <= capacity is unplaceable at every head offset where neither the space to the wrap boundary nor the space before it holds it - even in an EMPTY ring, because a wrap pad costs spaceToEnd bytes on top of the record. Concretely: head offset 16 of an empty 256-byte ring, a 248-byte record; FreeBytes() says 256, Reserve says nullptr, forever, and a producer waiting for FreeBytes() >= 248 stalls with nothing logged. The bound is now capacity/2, which is exact rather than conservative (worst case 2*total-8 <= capacity-8), exposed as MaxRecordBytes() for the emitter to chunk against; the minimum ring is two headers so the smallest record still fits the bound. Ring.h states Capacity()/2 as the chunking bound and the G3 header comment in gen_pipe.py now states the chunking rule plan section 8.2 asks G3 to define (PipeWire.inc regenerated). - Tests, each shown red with only the fix site reverted and green with it: InProcessTransportTest.ShutdownUnparksAWaiterWithNoDeadline (bounded join through a shared_ptr-owned waiter: 5 s red instead of a hung job; reverted it hangs and fails at 5051 ms), RingTest.RecordLargerThanHalfTheRingIsRefused (reverted, the 248-byte record is accepted), RingTest.RecordPlaceabilityDoesNotDependOnTheHeadOffset (the offset-0 vs offset-16 negative control), RingTest.HalfCapacityRecordFitsAtEveryHeadOffset (the positive half: the maximal record at all 32 head offsets of a 256-byte ring) and RingTest.RejectsARingTooSmallForTheSmallestRecord.	2026-09-05 21:19:00 -04:00
swung0x48	c1a7ffac94	[Fix] (MG_Remote, Transport): give descriptor offers their own condition variable, cap the ring at what a 32-bit size field can describe, and keep the frontend umbrella out of Framing.h ... - InProcessChannel::Direction served two different predicates from one condition_variable signalled with notify_one, so a SendFrame wakeup could be delivered to a thread blocked in ReceiveFd, which re-tests its own predicate and goes back to sleep - leaving a queued message undelivered until some unrelated later event. ITransport narrows the contract to one dedicated reader thread, but that is a comment, not a mechanism, and the first caller that splits its reader should not have to discover this. Offers now have their own fdCv, and Close() notifies both. - RingProducer/RingConsumer accepted any power-of-two capacity while RingRecordHeader::size is 32-bit by wire contract (plan section 8.1 -> PLAN.md section 6.3: 8-byte RecHeader). At 4 GiB or more a record's size - or a wrap filler's, which is sized by the distance to the boundary - would be truncated on the way in, and the consumer would then bounds-check the truncated value against the real one. kMaxRingCapacity rejects that at construction, the same class of guard as the power-of-two and smaller-than-a-header checks beside it. Unreachable today (SEG_CMD 8 MiB, SEG_STAGE 32 MiB), which is the point of catching it now. - Framing.h included MG_Util/Debug/Log.h, which includes Includes.h, the GL frontend's umbrella header: 661 headers by `clang++ -H`. It is the one header under Transport/ that broke the rule ITransport.h states for this layer ("nothing about a byte pipe needs the GL frontend's umbrella header"), which matters when the server-side binary links this and when the include-graph purity gate of plan section 10.3 (gate A, asserted on -H output rather than on symbols) lands. Its three error paths now call WireLogError, declared in a new dependency-free WireLog.h whose .cpp owns the umbrella. Framing.h is down to 134 headers, none of them MG_State, Includes.h or Log.h. - Two documentation corrections. ITransport::Shutdown documented a one-sided "releases the endpoint" while InProcessTransport::Shutdown closes both directions - which is what closing a socket does, so the spawn transport will behave the same way; the interface now says whole-connection teardown, and keeps the promise that queued messages stay readable until drained. InProcessTransport.h cited a CMake option MOBILEGL_BUILD_DISAGGREGATED_INPROC that grep finds nowhere: plan appendix B reserves it for the role-isolation shim, this skeleton does not add it, and the delivery mode is a runtime choice (MOBILEGL_TRANSPORT), not a build one. - Evidence: both configurations reconfigured and rebuilt; nm --defined-only on the OFF build still reports 0 MG_Remote symbols and the ldd dependency set is byte-identical to the OFF link (plan section 10.3, the two surviving byte-level equalities). Negative controls: removing the capacity ceiling makes RingTest.RejectsACapacityTheRecordHeaderCannotDescribe fail on both roles; collapsing fdCv back into cv makes InProcessTransportTest.AFrameWakeupIsNotEatenByAWaiterOnDescriptors fail at 3950ms against its 2000ms bound.	2026-09-05 20:16:50 -04:00
swung0x48	bdd4bed431	[Fix] (MG_Remote, Transport): close the doorbell's lost-wakeup window with two seq_cst fences and stop a hung-up peer turning every park into a spin ... - The header claimed the park flag's own seq_cst store and load closed the lost-wakeup window. They cannot: that Dekker argument needs all FOUR accesses in the seq_cst total order, and the other two are not in it - the watermark publish is a release store (RingProducer::Publish) and the condition re-test is an acquire load. There was no atomic_thread_fence anywhere under MG_Remote. On x86 a release store and a seq_cst load are both plain MOVs, so the notifier can read parked==0 while its head store still sits in the store buffer, and the waiter then parks on a stale watermark forever; ARMv8 survived only because STLR->LDAR is RCsc, which is luck, not the design. Doorbell::Wait now fences after announcing and NotifyIfParked fences before reading the flag - the pairing the standard actually guarantees ([atomics.order]) - and the header says so, including the other half of the contract: publish the watermark BEFORE ringing, because a fence only orders what precedes it. This is the claim the whole P5/P6 wait discipline (present credit, kNeedsAck blocking requests, full-ring escalation) will be built on, and its failure mode is a silent cross-process hang. Inherited design, plan section 8.1 (PLAN.md section 6.2/6.2a: bidirectional doorbell, MOBILEGL_IPC_SPIN_US default 50us, condvar for inproc). - SocketDoorbell::Park treated any poll() return > 0 as a wakeup and never looked at revents. Measured on this machine: an AF_UNIX SOCK_STREAM socketpair whose peer has closed returns revents=POLLIN|POLLHUP with recv()==0 immediately and forever. Park therefore returned true, Wait stored parked=0, found its condition still false and re-parked - so a waiter with kWaitForever burned a big core at full clock with no bound. That is exactly the pathology the bidirectional doorbell exists to prevent (a whole 16.6ms frame of a big core on a phone, competing with the GPU and the game's JVM), reached from the other side. Park now branches on revents, a new Drain() latches death on EOF (and on ECONNRESET/EPIPE from Notify), and the new Doorbell::Dead() lets Wait give up instead of re-parking on a descriptor that can never deliver another wakeup. - Same commit, same defect class: `fd` is documented as one end of an AF_UNIX socket pair, not "a socket or pipe end". Notify uses send(MSG_DONTWAIT| MSG_NOSIGNAL) and Park uses poll()+recv(), which a pipe end refuses with ENOTSOCK, and a SOCK_DGRAM pair reports no readiness at all when the peer closes (measured), so the spawn transport wants SOCK_STREAM. - Evidence: build-linux-split rebuilt clean; the wire suite is green (47/47). Negative control - reinstating the revents-blind Park makes FdPassingTest.SocketDoorbellStopsParkingWhenThePeerHangsUp fail on both Park assertions, and restoring this code turns it green again.	2026-09-05 20:16:49 -04:00
swung0x48	bfa087d0f7	[Feat] (MG_Remote, Transport): land the transport skeleton behind MOBILEGL_BUILD_DISAGGREGATED - framing, the SPSC ring, doorbells, shm segments and SCM_RIGHTS ... - New CMake option MOBILEGL_BUILD_DISAGGREGATED (default OFF, plan B appendix B). ON appends the MG_Remote sources to SOURCE_FILES, puts 3rdparty/flatbuffers/include on the include path and defines MOBILEGL_BUILD_DISAGGREGATED=1. OFF compiles nothing from MG_Remote and adds no include path and no library, which is one of the two byte-level equalities plan B section 10.3 keeps: measured `nm --defined-only build-linux/libMobileGL.so | grep -ic MG_Remote` = 0 with the option OFF and 93 with it ON, with an identical ldd set in both configurations. - The option forces itself OFF with message(WARNING) when 3rdparty/flatbuffers/include/flatbuffers/flatbuffers.h is missing, so a checkout without the submodule still configures and builds rather than failing a hundred lines later on a missing header. - ITransport.h: SendFrame / ReceiveFrame / PeekFrameSize / ShareFd / ReceiveFd / Shutdown. ReceiveFrame's contract is the fix for a defect of the earlier branch: a destination buffer smaller than the pending message returns MOBILEGL_ERR_BUFFER_TOO_SMALL with the required size and KEEPS the message queued. The earlier transport failed the call and popped the message anyway, which wedges the stream permanently the first time a reader guesses a size wrong. - Framing.h: [u32 'MGLF'][u32 len][payload], 64 MiB cap, validated on read. A bad magic or an oversized length latches the reader dead, is logged at ERROR and makes every later call return MOBILEGL_ERR_PROTOCOL_MISMATCH. This is the second inherited defect: Feat/CS-Delta-IPC's Framing.h:41-45 Feed() always returned OK and its header peek merely returned false, so a desynchronized stream became a silent permanent hang; its LocalSocketTransport.cpp:232-236 then allocated on the peer-supplied length with no cap. The magic is also byte-ordered so it reads "MGLF" on the wire, where the earlier constant spelled "FLGM". - Ring.h/.cpp: RingControl exactly as the inherited design (plan B section 8.1 -> earlier section 6.2): two independent cursor triples (cmd and stage, each {head, appliedTail, retiredTail}), appliedSeq / submittedSeq / retiredSeq / completedFrameSerial / presentAckSerial, serverEpoch, ringGeneration, consumerParked, producerParked, eventRingFull, eventDropped. One 4096-byte page with each contended group on its own cache line, pinned by static_assert on size and alignment and by an offset test. - The SPSC pair uses monotonic byte cursors and a power-of-two mask, so a torn read can never look like a valid earlier position. A record never straddles the wrap: the producer emits a kPad filler to the boundary, which is always a multiple of 8 and therefore always has room for a header. The producer reclaims against retiredTail rather than appliedTail, so the day the server borrows a ring slot into the GPU timeline (kRecBorrowSlot) it does not silently degrade to early recycling. HardDrainRing bumps ringGeneration only when the ring is quiesced, and leaves the cursors monotonic so cached offsets are recognisably stale. - The consumer bounds-checks every record header before dispatch (8-aligned, at least a header, no larger than what the producer published, contiguous inside the mapping) and reports corruption to the caller instead of dispatching into undefined behaviour. That is the runtime half of the earlier section 6.3 discipline: SEG_CMD is written by another process, so a compile-time static_assert on record sizes proves nothing about what is in the mapping. - Doorbell.h/.cpp: spin then park, both directions (earlier section 6.2a). CondVarDoorbell for inproc, SocketDoorbell for spawn (one byte, codes 0x01 ring-advanced and 0x02 watermark-advanced) - no futex, eventfd or named event anywhere. The lost-wakeup window is closed by ordering: the waiter stores its park flag seq_cst and then re-tests the condition, NotifyIfParked loads the same flag seq_cst after the watermark is published, so one of the two always sees the other. kDefaultSpinUs is 50, the MOBILEGL_IPC_SPIN_US default. - ShmSegment.{h,cpp} + ShmSegmentPosix.cpp: memfd_create by raw syscall on desktop Linux (the glibc wrapper is too recent to rely on), ASharedMemory_create on Android (API 26; libc's memfd_create wrapper is API 30, above MobileGL's floor), shm_open + immediate shm_unlink as the fallback. Adopt() refuses a descriptor whose fstat size is smaller than the size the peer announced, so a short segment cannot turn every later offset into an out-of-bounds map. ShmSegmentWin32.cpp (CreateFileMappingW in Local\) is compile-guarded and untested - this project's Windows machine is not a correctness gate. - The Android path is compile-verified, not just written: an arm64-v8a NDK build with the option ON links, ShmSegmentPosix.cpp.o carries an undefined ASharedMemory_create, and ShmSegmentWin32.cpp.o is empty there. That build is also what caught the missing <cstddef> in ShmSegment.h and Framing.h, where std::size_t / std::ptrdiff_t only resolved through a transitive include on the host sysroot. - FdPassing.{h,cpp}: SCM_RIGHTS in the FIRST transport commit, as plan B section 8.1 demands, over a dedicated AF_UNIX SOCK_DGRAM socketpair rather than the control byte stream (message boundaries survive on every POSIX - SOCK_SEQPACKET does not exist on macOS - and ancillary data can never be split from its payload). The third inherited defect this replaces: Feat/CS-Delta-IPC deferred fd passing and hardcoded `out->fd = -1` in LocalSocketTransport.cpp:296, so on the only platform that matters its data plane could not move one byte between processes. MSG_CTRUNC, an unexpected descriptor count and a malformed sideband header all close every descriptor received before failing, and a too-small sideband buffer is refused before the recvmsg so a datagram is never half-consumed. - InProcessTransport.{h,cpp}: two in-memory queues plus the condvar doorbell pair. It keeps the frame size cap so nothing that passes in inproc becomes illegal after the switch to spawn, hands descriptors over with dup() under the same ownership rule as SCM_RIGHTS, and lets a peer's queued messages be drained after Shutdown - usually the last one says why it is going away. - Not done here on purpose: the MOBILEGL_IPC_* environment variables are parsed in ConfigLoader.cpp, which belongs to another P0 work package running in parallel; this commit only exposes the constants (kDefaultSpinUs, kMaxFramePayloadSize, FdPassing::kMaxSidebandBytes) so that plumbing has something to set.	2026-09-05 20:16:49 -04:00
swung0x48	a1e22c26ab	[Build] (MG_Remote, Protocol): pin the flatbuffers submodule, add the control-plane schema and commit its generated header ... - 3rdparty/flatbuffers submodule pinned to the latest release tag v25.12.19 (7e163021). The runtime is header-only, so only 3rdparty/flatbuffers/include is ever used and no library is linked; CMake never calls add_subdirectory on it and flatc is not in the build graph (plan B section 8.1, inheriting the earlier plan's section 7.1). - MobileGL/MG_Remote/Protocol/protocol.fbs carries the CONTROL PLANE only: SegmentRef, Hello, Welcome, CapsSnapshot, DefaultFramebufferInfo, SurfaceOp, SurfaceReply, ResyncRequest, ResyncDone, AuxRequest, Fatal, LogLine, union CtrlMsg and the CtrlEnvelope root with a file_identifier. Hot-path records are FlatBuffers structs generated from MG_Pipe/PipeCalls.def in a later package and are deliberately absent here, so record numbering never churns. - Two deviations from the earlier plan's section 7.1 sketch, both deliberate: (a) ProgramReflection is not a union member - plan B ships program artifacts inside the create_shader_state CSO blob (section 8.2), and union tags are wire values that may only ever be appended, so reserving a tag for a message that may never exist is worse than appending one later; (b) maxComputeWorkGroupCount/Size are vectors, not [int:3] - fixed-size arrays are legal only in FlatBuffers structs, never in tables. - scripts/gen_protocol.py resolves flatc as MOBILEGL_FLATC_EXECUTABLE, otherwise builds the pinned flatc ONCE into <repo>/../flatc-build (override with MOBILEGL_FLATC_BUILD_DIR), outside the project build graph. A flatc found on PATH is deliberately refused and a version mismatch against the pinned runtime is a hard error: the generated header static_asserts FLATBUFFERS_VERSION, so a stray flatc either fails to compile or churns the committed file on every machine. The earlier branch did the opposite - Protocol/CMakeLists.txt:22-38 add_subdirectory'd the FlatBuffers tree with FLATBUFFERS_BUILD_FLATC=ON whenever MOBILEGL_FLATC_EXECUTABLE was unset, which is exactly the NDK trap it claimed to avoid (cross-compile an arm64 flatc, then run it on the host). - protocol_generated.h is committed with the project source header prepended by the generator, so regeneration is byte-identical: verified by running gen_protocol.py twice and by perturbing the file and regenerating it back. - Reuse from Feat/CS-Delta-IPC: MobileGL/Protocol/mg_protocol_base.h, kept as the shared C vocabulary (result codes, byte spans, shm region, id typedefs) and keeping the structSize-first versioning discipline that section 14.2 calls the answer to risk B-R10. Dropped from it: MobileGLObjectKind / MobileGLObjectScope / MobileGLObjectHandle - plan B never puts GL object identity on the wire (the frontend allocates {slot, generation} handles in MG_Pipe, section 4.2.1), so a second identity vocabulary would be a drift surface with no reader. Added MOBILEGL_ERR_BUFFER_TOO_SMALL as an append-only code for the receive contract.	2026-09-05 20:16:49 -04:00