From 817e1c40cf88f95797774fa58c63d9a23cb7b787 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 11 Sep 2026 14:13:07 -0400 Subject: [PATCH 01/10] [Feat] (MG_Remote, Transport): the ring-owning session pair over ShmSegment - four segments, the five watermarks with their first real writers, the SEG_REPLY slot pool and the SEG_EVENT reverse channel, with the doorbell pair taken from the transport and the ring capacity derived as the largest power of two left after the control page --- MobileGL/MG_Remote/Transport/EventRing.h | 205 +++++++++++ MobileGL/MG_Remote/Transport/ReplySlot.h | 251 +++++++++++++ MobileGL/MG_Remote/Transport/Ring.cpp | 255 ++++++++++++++ MobileGL/MG_Remote/Transport/RoleMemory.h | 85 +++++ MobileGL/MG_Remote/Transport/SessionRings.h | 369 ++++++++++++++++++++ MobileGL/MG_Remote/Transport/ShmSegment.cpp | 309 ++++++++++++++++ 6 files changed, 1474 insertions(+) create mode 100644 MobileGL/MG_Remote/Transport/EventRing.h create mode 100644 MobileGL/MG_Remote/Transport/ReplySlot.h create mode 100644 MobileGL/MG_Remote/Transport/RoleMemory.h create mode 100644 MobileGL/MG_Remote/Transport/SessionRings.h diff --git a/MobileGL/MG_Remote/Transport/EventRing.h b/MobileGL/MG_Remote/Transport/EventRing.h new file mode 100644 index 00000000..bca91fda --- /dev/null +++ b/MobileGL/MG_Remote/Transport/EventRing.h @@ -0,0 +1,205 @@ +// MobileGL - MobileGL/MG_Remote/Transport/EventRing.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// 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. + +#pragma once + +#include "Doorbell.h" +#include "Ring.h" + +#include +#include + +namespace MobileGL::MG_Remote::Transport { + + // Record kinds on SEG_EVENT. 0 is kRingPadRecordKind and can never be an + // event, which is why the list starts at 1. + enum EventKind : std::uint16_t { + kEventNone = 0, + kEventBufferWriteback = 1, // MGPipeCallbacks::OnBufferWriteback + kEventGpuWritten = 2, // MGPipeCallbacks::OnGpuWritten + kEventSurfaceChanged = 3, // MGPipeCallbacks::OnSurfaceChanged + }; + + // The 8-byte {slot, gen} pair, mirrored (MGPipeHandles.h:54-65). + struct EventHandle { + std::uint32_t Slot; + std::uint32_t Gen; + }; + static_assert(sizeof(EventHandle) == 8, "the handle is the 8-byte {slot, gen} pair"); + + // MGPRange, mirrored (MGPipeTypes.h:63-67). + struct EventRange { + std::uint64_t Offset; + std::uint64_t Size; + }; + static_assert(sizeof(EventRange) == 16, "MGPRange is 16 bytes on the wire"); + + // OnBufferWriteback(res, offset, MGPBlobRef bytes). The bytes follow this + // head INSIDE THE RECORD: the blobref the client hands the frontend names + // SEG_EVENT and the in-segment offset of those inline bytes, which is what + // makes "the destination is the client's shadow" (contract table 1 row 22) + // reachable without a second segment. `Size` is therefore the record's own + // tail length and is cross-checked against it. + struct EventBufferWritebackHead { + EventHandle Resource; + std::uint64_t Offset; // destination offset inside the resource + std::uint64_t Size; // inline byte count that follows + }; + static_assert(sizeof(EventBufferWritebackHead) == 24, "wire shape"); + + // OnGpuWritten(res, rangeCount, ranges). EventRange[RangeCount] follows. + struct EventGpuWrittenHead { + EventHandle Resource; + std::uint32_t RangeCount; + std::uint32_t Pad0; + }; + static_assert(sizeof(EventGpuWrittenHead) == 16, "wire shape"); + + // OnSurfaceChanged(const MGPSurfaceInfo*), mirrored (MGPipeTypes.h:1394-1401). + struct EventSurfaceChangedHead { + std::uint32_t Width; + std::uint32_t Height; + std::uint32_t InternalFormat; + std::uint16_t Samples; + std::uint16_t Layers; + std::uint8_t IsDefault; + std::uint8_t Pad0[7]; + }; + static_assert(sizeof(EventSurfaceChangedHead) == 24, "MGPSurfaceInfo is 24 bytes on the wire"); + + // The server's end. One producer: the apply thread, by construction. + class EventRingProducer { + public: + EventRingProducer() = default; + + // `eventControl` is SEG_EVENT's own control page; `cmdControl` is the + // SEG_CMD page, because Ring.h declares eventRingFull / eventDropped + // there and the client's waits already look at it. + EventRingProducer(RingControl* eventControl, RingControl* cmdControl, void* base, + std::uint64_t capacityBytes) + : m_cmdControl(cmdControl), + m_producer(eventControl, base, capacityBytes, RingCursorSet::Cmd) {} + + bool Valid() const { return m_producer.Valid() && m_cmdControl != nullptr; } + + // Reserves one event record. nullptr means the ring is full: the caller + // decides, and the two flags are how it says which decision it took. + // THIS FUNCTION DOES NOT DECIDE - that is P9's. + void* Reserve(EventKind kind, std::uint64_t payloadBytes) { + void* slot = m_producer.Reserve(static_cast(kind), kRecNone, payloadBytes); + if (slot == nullptr && m_cmdControl != nullptr) { + // "SEG_EVENT full, server stopped applying" - Ring.h:123. Latched + // here, cleared by the consumer once it has drained. + m_cmdControl->eventRingFull.store(1, std::memory_order_release); + } + return slot; + } + + // For a LOSSY event the caller could not place. Lossless events must + // never call this; they wait for the client to drain instead. + void CountDrop() { + if (m_cmdControl != nullptr) { + m_cmdControl->eventDropped.fetch_add(1, std::memory_order_relaxed); + } + } + + // Publish, THEN ring - the same order as the forward direction, and for + // the same reason (Doorbell.h:186-193: the fence only orders what + // precedes it, so ringing first reopens the lost-wakeup window). + void PublishAndNotify(Doorbell& clientBell, std::atomic& producerParked) { + m_producer.Publish(); + NotifyIfParked(clientBell, producerParked); + } + + RingProducer& Ring() { return m_producer; } + + private: + RingControl* m_cmdControl = nullptr; + RingProducer m_producer; + }; + + // The client's end. One consumer: the GL thread, which drains between verbs. + class EventRingConsumer { + public: + EventRingConsumer() = default; + + EventRingConsumer(RingControl* eventControl, RingControl* cmdControl, void* base, + std::uint64_t capacityBytes, const void* segmentBase) + : m_cmdControl(cmdControl), + m_consumer(eventControl, base, capacityBytes, RingCursorSet::Cmd), + m_segmentBase(static_cast(segmentBase)) {} + + bool Valid() const { return m_consumer.Valid() && m_cmdControl != nullptr; } + + bool Pop(RingRecordView& out, bool* outCorrupt = nullptr) { + return m_consumer.Pop(out, outCorrupt); + } + + // Release the bytes and clear the full latch. Only after the caller has + // finished with every payload pointer it popped: a writeback's bytes live + // in the ring itself, so retiring early is the R-11 violation one level + // down. + void Drained() { + m_consumer.PublishRetired(); + if (m_cmdControl != nullptr) { + m_cmdControl->eventRingFull.store(0, std::memory_order_release); + } + } + + // Byte offset of `payload` inside SEG_EVENT, which is what an + // OnBufferWriteback MGPBlobRef must carry (Seg = kSegEvent, Offset = + // this, Size = the head's Size). Never a host address - R-2's rule B. + std::uint64_t OffsetInSegment(const void* payload) const { + return static_cast(static_cast(payload) - + m_segmentBase); + } + + std::uint64_t DroppedEvents() const { + return m_cmdControl == nullptr + ? 0 + : m_cmdControl->eventDropped.load(std::memory_order_relaxed); + } + bool RingIsFull() const { + return m_cmdControl != nullptr && + m_cmdControl->eventRingFull.load(std::memory_order_acquire) != 0; + } + + RingConsumer& Ring() { return m_consumer; } + + private: + RingControl* m_cmdControl = nullptr; + RingConsumer m_consumer; + const std::uint8_t* m_segmentBase = nullptr; + }; + +} // namespace MobileGL::MG_Remote::Transport diff --git a/MobileGL/MG_Remote/Transport/ReplySlot.h b/MobileGL/MG_Remote/Transport/ReplySlot.h new file mode 100644 index 00000000..068dbb72 --- /dev/null +++ b/MobileGL/MG_Remote/Transport/ReplySlot.h @@ -0,0 +1,251 @@ +// MobileGL - MobileGL/MG_Remote/Transport/ReplySlot.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// SEG_REPLY: the slot pool the server writes a kReplySlot answer into, and the +// client reads it back out of. Owner: package s1. +// +// THE ID IS THE RECORD SEQUENCE NUMBER (P5 R-3). There is no second id space and +// no allocator: the ten kReplySlot calls carry no MGPReplySlot in their payloads +// (that is why the id has to be DERIVED rather than carried), the wire has no +// per-record seq field (ARCHITECTURE.md:124), so the record's ordinal IS its +// reply-slot id. The slot is addressed `seq % slotCount` and the server STAMPS +// THE SEQ BACK INTO THE SLOT HEADER, which is what makes a wrong-slot read +// detectable rather than merely plausible. Seq is 1-based; 0 means "no record". +// +// THE SLOT HEADER IS CONTRACT-P5 TABLE 0's ROW, verbatim: +// { Uint64 Seq; Int32 Status; Uint32 Size; } // 16 bytes, then the payload +// Status: 0 = OK, 1 = DECLINED, 2 = ERROR +// +// DECLINED IS A REAL ANSWER, NOT A FAILURE. It is how MapPersistent says nullptr +// (R-6) and how the four Bool acceptance entry points - ResourceCreate, +// ResourceRespecify, ResourceSubData, SetTextureParams - say false (R-5). A +// client that folds DECLINED into "the call failed" re-creates ID-39's 66 lost +// uploads from the other side, and a client that folds it into OK accepts a +// pointer the server never handed out. +// +// A REPLY LARGER THAN ONE SLOT IS FATAL, NOT CHUNKED. P5's only large answer is +// ReadPixels, and the client knows its size before it emits the record, so an +// overflow means the two sides disagree about the frame rather than that the +// pool is too small. Chunking is P8's; growing the pool is an operator's. +// +// ORDERING. The client only looks at a slot after it has seen +// RingControl::appliedSeq >= its own seq with an ACQUIRE load, and the server +// advances appliedSeq with a RELEASE store AFTER posting the reply +// (PipeApplier::ApplyOne's order: decode -> stamp -> apply -> post -> advance). +// That pair is what publishes the slot's bytes; the fences below are the +// belt-and-braces for a caller - a unit test, or P9's async pool - that reads a +// slot without going through appliedSeq first. + +#pragma once + +#include "WireLog.h" + +#include +#include +#include +#include + +namespace MobileGL::MG_Remote::Transport { + + // CONTRACT-P5 table 0, "reply slot header". + struct ReplySlotHeader { + std::uint64_t Seq; // the record ordinal, stamped back for self-check + std::int32_t Status; // ReplyStatus + std::uint32_t Size; // payload bytes following this header + }; + static_assert(sizeof(ReplySlotHeader) == 16, "the reply slot header is 16 bytes on the wire"); + static_assert(alignof(ReplySlotHeader) == 8, "the reply slot header must not gain padding"); + + enum ReplyStatus : std::int32_t { + kReplyStatusOk = 0, + // Not an error. MapPersistent's nullptr and the four Bool acceptance + // returns' `false` both arrive as this. + kReplyStatusDeclined = 1, + kReplyStatusError = 2, + }; + + // Eight slots of a 8 MiB SEG_REPLY is 1 MiB per answer. + // + // Why eight and not sixty-four: while the verb barrier holds (R-1) the client + // blocks at every verb boundary, so the in-flight depth is exactly ONE and + // every extra slot buys nothing but a smaller maximum answer. The trade is + // the other way round - fewer slots, bigger replies - and P5's only large + // answer is a blocking ReadPixels. 1 MiB covers a 512x512 RGBA8 read. P9, + // which is what makes the pool asynchronous, re-chooses this geometry with + // real depth to size it against. + inline constexpr std::uint32_t kDefaultReplySlotCount = 8; + + // Slot 0 exists and is used: seq is 1-based, so seq % slotCount hits slot 0 + // on seq == slotCount, not on "no record". + class ReplySlotPool { + public: + ReplySlotPool() = default; + + // `base`/`sizeBytes` are SEG_REPLY's mapping. `slotCount` must be a power + // of two - the addressing is a mask, and a non-power-of-two modulus on + // the apply thread is a division in the reply path of every blocking + // call. Anything else leaves Valid() false rather than half-working. + ReplySlotPool(void* base, std::uint64_t sizeBytes, std::uint32_t slotCount) { + if (base == nullptr || slotCount == 0 || (slotCount & (slotCount - 1)) != 0) { + WireLogError("MG_Remote reply pool: rejected, slotCount %u must be a non-zero power " + "of two over a non-null mapping", + static_cast(slotCount)); + return; + } + const std::uint64_t slotBytes = sizeBytes / slotCount; + if (slotBytes <= sizeof(ReplySlotHeader)) { + WireLogError("MG_Remote reply pool: rejected, %llu bytes over %u slots leaves no " + "room for a payload past the %llu byte slot header", + static_cast(sizeBytes), + static_cast(slotCount), + static_cast(sizeof(ReplySlotHeader))); + return; + } + m_base = static_cast(base); + m_size = sizeBytes; + m_slots = slotCount; + m_mask = slotCount - 1; + // Truncated to 32 bits deliberately: the header's Size field is + // 32-bit, so a slot no 32-bit count could describe would let a + // legal-looking Size name bytes past the slot. + m_slotBytes = slotBytes > 0xFFFFFFFFull ? 0xFFFFFFFFu + : static_cast(slotBytes); + } + + bool Valid() const { return m_base != nullptr; } + std::uint32_t SlotCount() const { return m_slots; } + std::uint32_t SlotBytes() const { return m_slotBytes; } + // What a single answer may carry. The client checks against this BEFORE + // it emits a ReadPixels, which is the whole reason a fixed slot size is + // legitimate rather than a guess. + std::uint32_t MaxReplyBytes() const { + return m_slotBytes == 0 ? 0u + : m_slotBytes - static_cast(sizeof(ReplySlotHeader)); + } + + // Zeroes every header, so a stale seq from a previous session cannot be + // mistaken for this session's answer. Called on the server side at Accept. + void Clear() { + if (m_base == nullptr) { + return; + } + for (std::uint32_t slot = 0; slot < m_slots; ++slot) { + ReplySlotHeader header{}; + std::memcpy(m_base + static_cast(slot) * m_slotBytes, &header, + sizeof(header)); + } + } + + // Server side. `size` bytes of `bytes` become the answer for `seq`. + // An answer larger than one slot is FATAL, never truncated and never + // chunked - see the file header. + void Post(std::uint64_t seq, std::int32_t status, const void* bytes, std::uint64_t size) { + if (m_base == nullptr) { + WireLogError("MG_Remote reply pool: Post(seq=%llu) on an unconfigured pool", + static_cast(seq)); + std::abort(); + } + if (seq == 0) { + WireLogError("MG_Remote reply pool: seq 0 is \"no record\" and can never name a " + "slot (R-3: seq is 1-based)"); + std::abort(); + } + if (size > MaxReplyBytes()) { + WireLogError("MG_Remote reply pool: Fatal{ProtocolCorruption} - a %llu byte answer " + "for seq %llu does not fit a %u byte slot (payload cap %u). P5 does " + "not chunk replies: the client knows an answer's size before it emits " + "the record, so this means the two sides disagree about the frame", + static_cast(size), + static_cast(seq), static_cast(m_slotBytes), + static_cast(MaxReplyBytes())); + std::abort(); + } + std::uint8_t* slot = SlotAt(seq); + if (size != 0 && bytes != nullptr) { + std::memcpy(slot + sizeof(ReplySlotHeader), bytes, static_cast(size)); + } + ReplySlotHeader header{}; + header.Seq = seq; + header.Status = status; + header.Size = static_cast(size); + // The payload must be visible before the stamp that says it is there. + std::atomic_thread_fence(std::memory_order_release); + std::memcpy(slot, &header, sizeof(header)); + } + + // Client side. Returns false when the slot does not carry THIS seq - the + // self-check the stamp exists for. `outBytes` may be null for an answer + // with no payload (every DECLINE, and the four Bool acceptances). + // + // A payload larger than the caller's buffer is a caller bug rather than a + // wire fault (the caller sized it from the call it made), so it returns + // false with *outSize set to what was there, the ReceiveFrame shape. + bool Read(std::uint64_t seq, void* outBytes, std::uint64_t outCapacity, + std::int32_t* outStatus, std::uint64_t* outSize) const { + if (outStatus != nullptr) { + *outStatus = kReplyStatusError; + } + if (outSize != nullptr) { + *outSize = 0; + } + if (m_base == nullptr || seq == 0) { + return false; + } + const std::uint8_t* slot = SlotAt(seq); + ReplySlotHeader header{}; + std::memcpy(&header, slot, sizeof(header)); + std::atomic_thread_fence(std::memory_order_acquire); + if (header.Seq != seq) { + // Not "retry": under the verb barrier the answer is already + // there by the time appliedSeq passed this record, so a stamp + // that disagrees is a drifted sequence space (R-9's kRecPad + // rule) or a wrong-slot read, and both are faults. + WireLogError("MG_Remote reply pool: slot %llu carries seq %llu, not %llu - the two " + "sides' sequence spaces have drifted (a counted kRecPad, R-9) or the " + "addressing disagrees", + static_cast(seq & m_mask), + static_cast(header.Seq), + static_cast(seq)); + return false; + } + if (header.Size > MaxReplyBytes()) { + WireLogError("MG_Remote reply pool: slot for seq %llu declares %u payload bytes in " + "a %u byte slot", + static_cast(seq), header.Size, + static_cast(m_slotBytes)); + return false; + } + if (outStatus != nullptr) { + *outStatus = header.Status; + } + if (outSize != nullptr) { + *outSize = header.Size; + } + if (header.Size == 0) { + return true; + } + if (outBytes == nullptr || outCapacity < header.Size) { + return false; + } + std::memcpy(outBytes, slot + sizeof(ReplySlotHeader), header.Size); + return true; + } + + private: + std::uint8_t* SlotAt(std::uint64_t seq) const { + return m_base + (seq & m_mask) * static_cast(m_slotBytes); + } + + std::uint8_t* m_base = nullptr; + std::uint64_t m_size = 0; + std::uint32_t m_slots = 0; + std::uint32_t m_mask = 0; + std::uint32_t m_slotBytes = 0; + }; + +} // namespace MobileGL::MG_Remote::Transport diff --git a/MobileGL/MG_Remote/Transport/Ring.cpp b/MobileGL/MG_Remote/Transport/Ring.cpp index defee42d..b54f4635 100644 --- a/MobileGL/MG_Remote/Transport/Ring.cpp +++ b/MobileGL/MG_Remote/Transport/Ring.cpp @@ -8,6 +8,8 @@ #include "Ring.h" +#include "SessionRings.h" + #include #include @@ -303,4 +305,257 @@ namespace MobileGL::MG_Remote::Transport { } } + // ======================================================================= + // P5: the five watermarks, the two session endpoints, and the ABI mixer. + // ======================================================================= + + std::uint64_t LargestPowerOfTwoAtMost(std::uint64_t bytes) { + if (bytes == 0) { + return 0; + } + std::uint64_t value = 1; + while (value <= (bytes >> 1)) { + value <<= 1; + } + return value; + } + + std::uint64_t RingCapacityForSegment(std::uint64_t segmentBytes) { + if (segmentBytes <= sizeof(RingControl)) { + return 0; + } + const std::uint64_t usable = LargestPowerOfTwoAtMost(segmentBytes - sizeof(RingControl)); + return usable < kMinRingCapacity ? 0 : usable; + } + + namespace Watermark { + + namespace { + // A watermark may be published LATE but never EARLY, and it may never + // move BACKWARDS. Backwards is the half that is mechanically + // detectable from inside, so it is refused loudly here; "early" can + // only be caught at the call site, which is why every advance below + // has exactly one caller and a named unit case. + void AdvanceMonotonic(std::atomic& watermark, std::uint64_t to, + const char* name) { + const std::uint64_t current = watermark.load(std::memory_order_relaxed); + if (to < current) { + MGLOG_E("MG_Remote watermark: refusing to move %s backwards, %llu -> %llu; a " + "waiter that already resumed on the higher value cannot be un-resumed", + name, static_cast(current), + static_cast(to)); + return; + } + if (to == current) { + return; + } + // Release: everything the advance is a statement ABOUT - the + // record that was applied, the staged bytes that were drained, + // the reply that was posted - must be visible to the acquiring + // waiter before the number that says it happened. + watermark.store(to, std::memory_order_release); + } + } // namespace + + void AdvanceSubmitted(RingControl& control, std::uint64_t seq) { + AdvanceMonotonic(control.submittedSeq, seq, "submittedSeq"); + } + + void AdvanceApplied(RingControl& control, std::uint64_t seq) { + AdvanceMonotonic(control.appliedSeq, seq, "appliedSeq"); + } + + void AdvanceRetired(RingControl& control, std::uint64_t seq) { + // retiredSeq may never overtake appliedSeq: the staging allocator + // reclaims behind it, so a retire ahead of the apply hands live bytes + // back to the producer. Clamped rather than refused, because a + // caller that retires "everything applied" is the normal shape. + const std::uint64_t applied = control.appliedSeq.load(std::memory_order_acquire); + AdvanceMonotonic(control.retiredSeq, seq > applied ? applied : seq, "retiredSeq"); + } + + void AdvanceCompletedFrame(RingControl& control, std::uint64_t serial) { + AdvanceMonotonic(control.completedFrameSerial, serial, "completedFrameSerial"); + } + + void AdvancePresentAck(RingControl& control, std::uint64_t serial) { + AdvanceMonotonic(control.presentAckSerial, serial, "presentAckSerial"); + } + + } // namespace Watermark + + // ----------------------------------------------------------------------- + // SessionProducer + // ----------------------------------------------------------------------- + + void SessionProducer::Attach(RingControl* control, RingProducer* cmd, RingProducer* stage, + Doorbell* peerBell, Doorbell* selfBell, std::uint32_t spinUs) { + m_control = control; + m_cmd = cmd; + m_stage = stage; + m_peerBell = peerBell; + m_selfBell = selfBell; + m_spinUs = spinUs; + } + + void SessionProducer::Detach() { + m_control = nullptr; + m_cmd = nullptr; + m_stage = nullptr; + m_peerBell = nullptr; + m_selfBell = nullptr; + } + + void SessionProducer::PublishAndNotify(std::uint64_t submittedSeq) { + if (!Valid()) { + return; + } + // 1. the records themselves. + m_cmd->Publish(); + if (m_stage != nullptr) { + m_stage->Publish(); + } + // 2. the diagnostic watermark, after the bytes it describes. + Watermark::AdvanceSubmitted(*m_control, submittedSeq); + // 3. and only now the bell. Publish-then-ring, never ring-then-publish. + if (m_peerBell != nullptr) { + NotifyIfParked(*m_peerBell, m_control->consumerParked); + } + } + + template + SessionWait SessionProducer::Park(Ready&& ready, std::uint32_t timeoutMs) { + if (!Valid() || m_selfBell == nullptr) { + return SessionWait::TimedOut; + } + if (m_selfBell->Wait(m_control->producerParked, ready, m_spinUs, timeoutMs)) { + return SessionWait::Reached; + } + // Wait == false && Dead() is "the session was shut down", and it is the + // only thing that returns from a kWaitForever park. Anything else is the + // deadline. + return m_selfBell->Dead() ? SessionWait::ShutDown : SessionWait::TimedOut; + } + + SessionWait SessionProducer::WaitForApplied(std::uint64_t seq, std::uint32_t timeoutMs) { + if (!Valid()) { + return SessionWait::TimedOut; + } + RingControl* control = m_control; + return Park([control, seq] { return Watermark::Reached(control->appliedSeq, seq); }, + timeoutMs); + } + + SessionWait SessionProducer::WaitForPresentAck(std::uint64_t serial, std::uint32_t timeoutMs) { + if (!Valid()) { + return SessionWait::TimedOut; + } + RingControl* control = m_control; + return Park([control, serial] { return Watermark::Reached(control->presentAckSerial, serial); }, + timeoutMs); + } + + SessionWait SessionProducer::WaitForCmdSpace(std::uint64_t bytes, std::uint32_t timeoutMs) { + if (!Valid()) { + return SessionWait::TimedOut; + } + RingProducer* cmd = m_cmd; + return Park([cmd, bytes] { return cmd->FreeBytes() >= bytes; }, timeoutMs); + } + + SessionWait SessionProducer::WaitForStageSpace(std::uint64_t bytes, std::uint32_t timeoutMs) { + if (!Valid() || m_stage == nullptr) { + return SessionWait::TimedOut; + } + RingProducer* stage = m_stage; + return Park([stage, bytes] { return stage->FreeBytes() >= bytes; }, timeoutMs); + } + + // ----------------------------------------------------------------------- + // SessionConsumer + // ----------------------------------------------------------------------- + + void SessionConsumer::Attach(RingControl* control, RingConsumer* cmd, Doorbell* peerBell, + Doorbell* selfBell, std::uint32_t spinUs) { + m_control = control; + m_cmd = cmd; + m_peerBell = peerBell; + m_selfBell = selfBell; + m_spinUs = spinUs; + m_appliedSeq = control == nullptr ? 0 : control->appliedSeq.load(std::memory_order_acquire); + } + + void SessionConsumer::Detach() { + m_control = nullptr; + m_cmd = nullptr; + m_peerBell = nullptr; + m_selfBell = nullptr; + } + + SessionWait SessionConsumer::WaitForWork(std::uint32_t timeoutMs) { + if (!Valid() || m_selfBell == nullptr) { + return SessionWait::TimedOut; + } + RingControl* control = m_control; + RingConsumer* cmd = m_cmd; + const bool woke = m_selfBell->Wait( + control->consumerParked, + [control, cmd] { + return control->cmdHead.load(std::memory_order_acquire) != cmd->LocalTail(); + }, + m_spinUs, timeoutMs); + if (woke) { + return SessionWait::Reached; + } + return m_selfBell->Dead() ? SessionWait::ShutDown : SessionWait::TimedOut; + } + + void SessionConsumer::RetireThrough(std::uint64_t seq) { + if (!Valid()) { + return; + } + Watermark::AdvanceRetired(*m_control, seq); + m_cmd->PublishRetired(); + NotifyClient(); + } + + void SessionConsumer::NotifyClient() { + if (m_control != nullptr && m_peerBell != nullptr) { + NotifyIfParked(*m_peerBell, m_control->producerParked); + } + } + + // ----------------------------------------------------------------------- + // The ABI fingerprint's mixer + // ----------------------------------------------------------------------- + + std::uint64_t MixAbiFingerprint(std::uint64_t dynamicParamsSize, std::uint64_t capsSize, + std::uint64_t functionTableSize, std::uint32_t abiVersion, + const char* buildStamp) { + // FNV-1a over the four numbers and the stamp. Not a hash with any + // security property and not meant to be one: it has to (a) change when + // ANY input changes and (b) be computable identically in two processes + // built from one source tree, which rules out anything seeded at runtime. + std::uint64_t hash = 1469598103934665603ull; + const auto mix = [&hash](std::uint64_t value) { + for (int byte = 0; byte < 8; ++byte) { + hash ^= static_cast((value >> (byte * 8)) & 0xFF); + hash *= 1099511628211ull; + } + }; + mix(dynamicParamsSize); + mix(capsSize); + mix(functionTableSize); + mix(abiVersion); + if (buildStamp != nullptr) { + for (const char* c = buildStamp; *c != '\0'; ++c) { + hash ^= static_cast(static_cast(*c)); + hash *= 1099511628211ull; + } + } + // 0 is reserved for "not stated": a peer that forgot to fill the field + // must not accidentally agree with one that did. + return hash == 0 ? 1ull : hash; + } + } // namespace MobileGL::MG_Remote::Transport diff --git a/MobileGL/MG_Remote/Transport/RoleMemory.h b/MobileGL/MG_Remote/Transport/RoleMemory.h new file mode 100644 index 00000000..fcbe30ad --- /dev/null +++ b/MobileGL/MG_Remote/Transport/RoleMemory.h @@ -0,0 +1,85 @@ +// MobileGL - MobileGL/MG_Remote/Transport/RoleMemory.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// Peak-RSS accounting for the two roles. Owner: package s1; CONSUMER: package t1, +// which puts the numbers in MEASUREMENTS. +// +// WHY BOTH HALVES ARE NEEDED, AND WHY NEITHER ALONE IS THE ANSWER. +// +// VmHWM is the kernel's own high-water mark of resident set size, in +// /proc/self/status. It is the only number that cannot be argued with - it +// counts what the process actually touched, including the pages the allocator +// never gave back. But under `inproc` BOTH ROLES ARE ONE PROCESS, so a single +// VmHWM cannot be split between them and reporting it as "the client's" would +// be a lie that only becomes visible in P6. +// +// The segment ledger is the other half: every ShmSegment this process mapped, +// by kind and by role. It is exact, it IS separable by role, and under `spawn` +// it is the part that appears in both processes at once (one mapping, two +// address spaces, one set of physical pages) - which is precisely the number a +// naive "sum the two VmHWMs" double-counts. +// +// So the pair is the measurement: VmHWM for what the process really cost, the +// ledger for how much of it is shared mapping that a second process will not pay +// for again. t1 reports both, per role, and the split's memory claim is +// (client VmHWM + server VmHWM - shared ledger), never either half on its own. +// +// A SAMPLE IS A SYSCALL AND A PARSE. Take it at phase boundaries - after the +// handshake, after the first frame, at teardown - never per record. + +#pragma once + +#include + +namespace MobileGL::MG_Remote::Transport { + + enum class MemoryRole : std::uint32_t { + Client = 0, + Server = 1, + kMemoryRoleCount = 2, + }; + + // Resident-set high-water mark of THIS PROCESS in bytes, from + // /proc/self/status's VmHWM line. 0 when the platform has no such file + // (Windows, and Android's /proc is readable but the caller should still + // treat 0 as "not measured" rather than "measured zero"). + std::uint64_t ProcessPeakRssBytes(); + + // Current resident set (VmRSS), same source and same 0 convention. Sampled + // beside the peak so a phase that never grew the peak is distinguishable + // from one that was not sampled. + std::uint64_t ProcessCurrentRssBytes(); + + // The ledger. ShmSegment does NOT update it itself: a segment is also created + // by tests and by P6's adopt path, and a ledger that counted those would stop + // meaning "this session's footprint". The SESSION books its own segments. + void LedgerAddSegment(MemoryRole role, std::uint64_t bytes); + void LedgerRemoveSegment(MemoryRole role, std::uint64_t bytes); + std::uint64_t LedgerMappedBytes(MemoryRole role); + // Every role's mapped bytes. Under inproc the two roles map THE SAME pages, + // so this over-counts on purpose: the two per-role numbers are what t1 + // subtracts with, and a single total that silently deduplicated them would + // hide exactly the spawn-vs-inproc difference the measurement is for. + std::uint64_t LedgerMappedBytesAllRoles(); + + // One sample, both halves, for one role. + struct RoleMemorySample { + std::uint64_t PeakRssBytes = 0; + std::uint64_t CurrentRssBytes = 0; + std::uint64_t MappedSegmentBytes = 0; // this role's ledger + MemoryRole Role = MemoryRole::Client; + }; + + RoleMemorySample SampleRoleMemory(MemoryRole role); + + // Emits one line at ERROR level (the wire layer's only level - WireLog.h) so + // t1's harness can grep it out of a lane log without a new log sink. + // `phase` is a short tag: "handshake", "first-frame", "teardown". + void LogRoleMemory(const char* phase, const RoleMemorySample& sample); + +} // namespace MobileGL::MG_Remote::Transport diff --git a/MobileGL/MG_Remote/Transport/SessionRings.h b/MobileGL/MG_Remote/Transport/SessionRings.h new file mode 100644 index 00000000..a52429e3 --- /dev/null +++ b/MobileGL/MG_Remote/Transport/SessionRings.h @@ -0,0 +1,369 @@ +// MobileGL - MobileGL/MG_Remote/Transport/SessionRings.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// The ring-owning half of a session: the four segments, the two ring endpoints, +// and the five watermarks. Owner: package s1. +// +// This is the object the P5 gate is really about. ROADMAP.md:21 asks that +// `inproc` run THE SAME G3 codec as `spawn`, and InProcessTransport cannot +// deliver that no matter how it is edited: it is two deque> plus +// two condvar doorbells (InProcessTransport.cpp:38-97), it owns THE BELLS BUT +// NOT THE RING, and it runs no codec at all. So the rings live here, above +// ITransport, in one implementation both delivery modes use - and the transport +// supplies the control plane and the two bells, which is exactly what its own +// header says it is for (ITransport.h:16-20: "everything on the hot path +// bypasses this interface entirely"). +// +// INPROC USES ShmSegment TOO, NOT new[]. In one address space a heap allocation +// would work and would be faster to write. It is refused deliberately: it is half +// of what makes "the same code path" true rather than nominal. A `new` here means +// the mapping, the alignment, the size rounding, the read-only peer view and the +// lifetime are all exercised for the first time in P6, on the day the second +// process appears - which is the shape of every "it was green in CI" failure this +// phase is trying not to repeat. +// +// --------------------------------------------------------------------------- +// THE RING CAPACITY IS HALF THE SEGMENT, AND THAT IS ARITHMETIC, NOT A CHOICE. +// +// Ring.h:11-13 puts RingControl at the HEAD of SEG_CMD, and RingProducer requires +// a POWER-OF-TWO capacity (Ring.cpp:89-103, the mask is the indexing). A segment +// of 8 MiB therefore has 8 MiB - 4096 bytes left for records, and the largest +// power of two that fits is 4 MiB. A record may be at most half the ring +// (RingProducer::MaxRecordBytes), so the real cap on one record is 2 MiB. +// +// CONTRACT-P5 §5 and Config.h's MOBILEGL_IPC_RING_MB comment both say "8 MiB caps +// one record at 4 MiB". That arithmetic assumed the whole segment is ring bytes +// and did not subtract the control page. The number here is HALF of theirs, and +// the deviation is deliberately in the SAFE direction: R-10's obligation is to +// PROVE no record ever approaches the cap, and a lower cap makes that proof fire +// earlier and louder rather than later and silently. The alternatives were both +// worse - announcing SegmentRef.sizeBytes as 4096 + 8 MiB breaks the four sizes +// ProtocolSmokeTest.cpp:72 pins, and moving RingControl out of SEG_CMD needs a +// fifth SegmentRef that Welcome does not have. +// +// SEG_STAGE has no control page of its own: RingControl carries TWO cursor +// triples (Ring.h:101-109) and the stage triple is the second. So SEG_STAGE's +// capacity is its whole segment, and 32 MiB is already a power of two. +// --------------------------------------------------------------------------- + +#pragma once + +#include "Doorbell.h" +#include "EventRing.h" +#include "ReplySlot.h" +#include "Ring.h" +#include "RoleMemory.h" +#include "ShmSegment.h" + +#include +#include + +namespace MobileGL::MG_Remote::Transport { + + // The four sizes are CONTRACT-P5's and are pinned by ProtocolSmokeTest.cpp:72. + // MOBILEGL_IPC_RING_MB / MOBILEGL_IPC_STAGE_MB move the first two. + struct SessionSegmentSizes { + std::uint64_t CmdBytes = 8ull * 1024 * 1024; + std::uint64_t StageBytes = 32ull * 1024 * 1024; + std::uint64_t ReplyBytes = 8ull * 1024 * 1024; + std::uint64_t EventBytes = 256ull * 1024; + std::uint32_t ReplySlotCount = kDefaultReplySlotCount; + }; + + // Largest power of two <= `bytes`, or 0 when there is none. The ring's + // indexing is a mask, so this is what any segment's usable ring area is. + std::uint64_t LargestPowerOfTwoAtMost(std::uint64_t bytes); + + // Usable ring capacity of a segment that carries a RingControl page at its + // head. See the header block above for why this is half the segment. + std::uint64_t RingCapacityForSegment(std::uint64_t segmentBytes); + + enum class SessionSegmentSlot : std::uint32_t { + Cmd = 0, + Stage = 1, + Reply = 2, + Event = 3, + kSessionSegmentCount = 4, + }; + + // The four ShmSegments of one session, created and mapped read/write. + // + // WHO CREATES THEM: the SERVER, because Welcome announces all four + // (protocol.fbs's Welcome table) and Welcome is server -> client. "Client + // owned" in the schema's comments is about who WRITES a segment, not who + // allocates it. Under `inproc` the client then attaches to the same mapping + // (AttachInProcess); under `spawn` it will adopt the fds the server passed by + // SCM_RIGHTS, which is P6's and is why Adopt is on ShmSegment already. + class SessionSegments { + public: + SessionSegments() = default; + ~SessionSegments(); + + SessionSegments(const SessionSegments&) = delete; + SessionSegments& operator=(const SessionSegments&) = delete; + + // Creates and maps all four, initialises BOTH control pages (SEG_CMD's + // and SEG_EVENT's), and books the mapping in `role`'s ledger. + MobileGLResult Create(const SessionSegmentSizes& sizes, MemoryRole role); + + // The inproc peer's view: the SAME mapping, booked under the OTHER role. + // It does not re-init the control pages - there is one shared page and + // re-initialising it would zero the owner's cursors under it. + MobileGLResult AttachInProcess(SessionSegments& owner, MemoryRole role); + + void Close(); + bool Valid() const { return m_valid; } + + RingControl* CmdControl() const { return m_cmdControl; } + void* CmdRingBase() const { return m_cmdRingBase; } + std::uint64_t CmdRingCapacity() const { return m_cmdRingCapacity; } + + void* StageBase() const { return m_stageBase; } + std::uint64_t StageCapacity() const { return m_stageCapacity; } + + void* ReplyBase() const { return m_replyBase; } + std::uint64_t ReplyBytes() const { return m_replyBytes; } + std::uint32_t ReplySlotCount() const { return m_replySlotCount; } + + RingControl* EventControl() const { return m_eventControl; } + void* EventSegmentBase() const { return m_eventSegmentBase; } + void* EventRingBase() const { return m_eventRingBase; } + std::uint64_t EventRingCapacity() const { return m_eventRingCapacity; } + + // For Welcome's four SegmentRefs. The announced size is the MAPPING size, + // which is what a peer must map - not the ring capacity inside it. + std::uint64_t AnnouncedSize(SessionSegmentSlot slot) const; + const char* AnnouncedName(SessionSegmentSlot slot) const; + int DescriptorFor(SessionSegmentSlot slot) const; // POSIX; -1 elsewhere + + std::uint64_t MappedBytes() const { return m_mappedBytes; } + + private: + void DeriveViews(); + + ShmSegment m_owned[4]; // empty on an attached (peer) view + ShmSegment* m_segments[4] = {nullptr, nullptr, nullptr, nullptr}; + + RingControl* m_cmdControl = nullptr; + void* m_cmdRingBase = nullptr; + std::uint64_t m_cmdRingCapacity = 0; + void* m_stageBase = nullptr; + std::uint64_t m_stageCapacity = 0; + void* m_replyBase = nullptr; + std::uint64_t m_replyBytes = 0; + std::uint32_t m_replySlotCount = kDefaultReplySlotCount; + RingControl* m_eventControl = nullptr; + void* m_eventSegmentBase = nullptr; + void* m_eventRingBase = nullptr; + std::uint64_t m_eventRingCapacity = 0; + + std::uint64_t m_mappedBytes = 0; + MemoryRole m_role = MemoryRole::Client; + bool m_valid = false; + bool m_owns = false; + bool m_booked = false; + }; + + // ----------------------------------------------------------------------- + // The five watermarks (R-9). Every write and every wait goes through here, + // so the rules in Ring.h's header have exactly one implementation. + // ----------------------------------------------------------------------- + // + // THE ONE RULE THAT MATTERS: a watermark may be published LATE but NEVER + // EARLY. Late costs a waiter some latency; early makes every waiter a silent + // use of work that has not happened, and there is no checksum anywhere on + // this ring that would catch it. So the advances below REFUSE to move a + // watermark backwards (that is the detectable half) and the callers are + // responsible for never calling them before the work is done (that is the + // half only a call-site review and R-9's unit cases can enforce). + namespace Watermark { + + // Producer, after Publish. Nobody waits on it - it is the answer to "how + // far ahead of the server is the client right now". + void AdvanceSubmitted(RingControl& control, std::uint64_t seq); + // Consumer, ONCE PER APPLIED RECORD. P5 forbids the 64-record batching + // this ring was designed for: the verb barrier and every reply wait read + // it. kRecPad does not count - RingConsumer::Pop skips fillers, so the + // rule is kept by counting Pops rather than bytes. + void AdvanceApplied(RingControl& control, std::uint64_t seq); + // Consumer, once the SEG_STAGE bytes a record referenced are finished + // with. The staging allocator reclaims behind it and nothing else may. + void AdvanceRetired(RingControl& control, std::uint64_t seq); + // Server, when a present completes. Trails appliedSeq by the GPU's own + // depth; never conflate the two. + void AdvanceCompletedFrame(RingControl& control, std::uint64_t serial); + // Server, when it returns a present credit. The only back-pressure that + // bounds latency rather than bytes. + void AdvancePresentAck(RingControl& control, std::uint64_t serial); + + // Every wait is >=, never ==: both sides advance in jumps, and an + // equality waiter misses its wakeup and hangs until the next coincidence. + inline bool Reached(const std::atomic& watermark, std::uint64_t target) { + return watermark.load(std::memory_order_acquire) >= target; + } + + } // namespace Watermark + + enum class SessionWait : std::uint32_t { + Reached = 0, + // The doorbell died: the peer shut the session down. The ONLY thing that + // can un-park a waiter on kWaitForever (Doorbell.h:211-221), and the + // reason a bounded join is possible at all. + ShutDown = 1, + TimedOut = 2, + }; + + // ----------------------------------------------------------------------- + // The client's end of the rings. + // + // THE TWO DOORBELL ACCESSORS LIVE ON THE SESSION, NOT ON ITransport + // (contract §3.9, the ruling s1 is asked to make now rather than let P6 + // discover). The session takes the two references InProcessTransport hands + // out and is the only thing that knows which is which; ITransport stays the + // dumb control-plane interface its header claims to be, and P6's + // SocketTransport does not grow two accessors it has no natural home for. + // ----------------------------------------------------------------------- + class SessionProducer { + public: + SessionProducer() = default; + + // `peerBell` is the bell the SERVER parks on and this side rings; + // `selfBell` is this side's own. InProcessTransport::PeerDoorbell() and + // SelfDoorbell() are exactly that pair, from the client endpoint. + void Attach(RingControl* control, RingProducer* cmd, RingProducer* stage, Doorbell* peerBell, + Doorbell* selfBell, std::uint32_t spinUs); + void Detach(); + bool Valid() const { return m_control != nullptr && m_cmd != nullptr; } + + // Publish the command ring's head, record submittedSeq, THEN ring - in + // that order and never any other. Doorbell.h:186-193: the fence only + // orders what precedes it, so ringing before publishing reopens the very + // lost-wakeup window the fences exist to close. RingTest.cpp:446 pins the + // call order; this is the one place production code performs it. + void PublishAndNotify(std::uint64_t submittedSeq); + + // The verb barrier's wait, AND the reply's wait: they are the same wait + // (R-3/R-5), which is why a blocking ReadPixels, MapPersistent's decline + // and the four Bool acceptances cost ZERO extra round trips. + SessionWait WaitForApplied(std::uint64_t seq, std::uint32_t timeoutMs); + // Present throttle. + SessionWait WaitForPresentAck(std::uint64_t serial, std::uint32_t timeoutMs); + // Back-pressure when Reserve returned nullptr. NEVER call this when + // FreeBytes() is already >= the record: Ring.h:226-233 - a nullptr with + // enough free bytes can only mean "too big, chunk", and waiting on it + // stalls forever. + SessionWait WaitForCmdSpace(std::uint64_t bytes, std::uint32_t timeoutMs); + SessionWait WaitForStageSpace(std::uint64_t bytes, std::uint32_t timeoutMs); + + RingControl* Control() const { return m_control; } + RingProducer* Cmd() const { return m_cmd; } + RingProducer* Stage() const { return m_stage; } + Doorbell* PeerDoorbell() const { return m_peerBell; } + Doorbell* SelfDoorbell() const { return m_selfBell; } + std::uint32_t SpinUs() const { return m_spinUs; } + + private: + template + SessionWait Park(Ready&& ready, std::uint32_t timeoutMs); + + RingControl* m_control = nullptr; + RingProducer* m_cmd = nullptr; + RingProducer* m_stage = nullptr; + Doorbell* m_peerBell = nullptr; + Doorbell* m_selfBell = nullptr; + std::uint32_t m_spinUs = kDefaultSpinUs; + }; + + // ----------------------------------------------------------------------- + // The server's end of the rings: the apply thread's loop, minus the applier. + // ----------------------------------------------------------------------- + class SessionConsumer { + public: + SessionConsumer() = default; + + void Attach(RingControl* control, RingConsumer* cmd, Doorbell* peerBell, Doorbell* selfBell, + std::uint32_t spinUs); + void Detach(); + bool Valid() const { return m_control != nullptr && m_cmd != nullptr; } + + // Park until a record is waiting, the session is shut down, or the + // deadline passes. Pass kWaitForever for the steady state; a dead bell is + // what ends it, which is why Doorbell::Kill() is load-bearing for the + // join (InProcessTransportTest.cpp:344 pins the shape). + SessionWait WaitForWork(std::uint32_t timeoutMs); + + // Pop ONE record and hand it to `apply`. Returns false when the ring is + // empty. On a corrupt header it returns false and sets *outCorrupt, which + // the CALLER escalates to Fatal{ProtocolCorruption} rather than retrying. + // + // This is the only place appliedSeq is advanced, and it advances it by + // EXACTLY ONE per record - never a batch (R-9). kRecPad cannot reach + // `apply`: RingConsumer::Pop skips fillers before returning, so a filler + // is never counted here and the two sides' sequence spaces cannot drift. + // Order: apply -> appliedSeq -> PublishApplied -> ring the client. + template + bool ApplyOne(Apply&& apply, bool* outCorrupt = nullptr) { + if (outCorrupt != nullptr) { + *outCorrupt = false; + } + if (!Valid()) { + return false; + } + RingRecordView view{}; + if (!m_cmd->Pop(view, outCorrupt)) { + return false; + } + apply(view); + ++m_appliedSeq; + Watermark::AdvanceApplied(*m_control, m_appliedSeq); + m_cmd->PublishApplied(); + NotifyClient(); + return true; + } + + // Records without kRecBorrowSlot retire as soon as they are applied; a + // borrowed slot retires on completedFrameSerial, which is why this is a + // separate call and not folded into ApplyOne. + void RetireThrough(std::uint64_t seq); + + // Ring the client's bell, but only when it said it is parked: a store to + // a shared cache line otherwise burns a big core for a whole frame on a + // phone (Doorbell.h:13-22). + void NotifyClient(); + + std::uint64_t AppliedSeq() const { return m_appliedSeq; } + RingControl* Control() const { return m_control; } + RingConsumer* Cmd() const { return m_cmd; } + Doorbell* PeerDoorbell() const { return m_peerBell; } + Doorbell* SelfDoorbell() const { return m_selfBell; } + std::uint32_t SpinUs() const { return m_spinUs; } + + private: + RingControl* m_control = nullptr; + RingConsumer* m_cmd = nullptr; + Doorbell* m_peerBell = nullptr; + Doorbell* m_selfBell = nullptr; + std::uint32_t m_spinUs = kDefaultSpinUs; + std::uint64_t m_appliedSeq = 0; + }; + + // ----------------------------------------------------------------------- + // The ABI fingerprint's mixer. + // + // It lives under Transport/ rather than in CapsCodec.cpp so that it can be + // tested without the GL frontend's umbrella header, and so that the SIZES it + // mixes are the caller's - CapsCodec.cpp passes the three real sizeofs, a + // unit test passes made-up ones and can then prove a one-byte difference + // changes the answer. A fingerprint that cannot be shown to change is + // indistinguishable from one that is never compared. + // ----------------------------------------------------------------------- + std::uint64_t MixAbiFingerprint(std::uint64_t dynamicParamsSize, std::uint64_t capsSize, + std::uint64_t functionTableSize, std::uint32_t abiVersion, + const char* buildStamp); + +} // namespace MobileGL::MG_Remote::Transport diff --git a/MobileGL/MG_Remote/Transport/ShmSegment.cpp b/MobileGL/MG_Remote/Transport/ShmSegment.cpp index 298062dd..54a7f715 100644 --- a/MobileGL/MG_Remote/Transport/ShmSegment.cpp +++ b/MobileGL/MG_Remote/Transport/ShmSegment.cpp @@ -8,9 +8,20 @@ // Platform-independent half of ShmSegment. The create/map/close bodies live in // ShmSegmentPosix.cpp and ShmSegmentWin32.cpp. +// +// P5 adds two things that are about a SET of segments rather than about one: +// SessionSegments (the four a session owns, and the ring geometry derived from +// them) and the role memory ledger that t1 reports against. #include "ShmSegment.h" +#include "RoleMemory.h" +#include "SessionRings.h" + +#include + +#include +#include #include #include @@ -46,4 +57,302 @@ namespace MobileGL::MG_Remote::Transport { bool ShmSegment::Valid() const { return m_size != 0 && (m_fd >= 0 || m_nativeHandle != nullptr); } + // ======================================================================= + // P5: the role memory ledger and the VmHWM sample + // ======================================================================= + + namespace { + constexpr std::size_t kMemoryRoleCount = static_cast(MemoryRole::kMemoryRoleCount); + + std::atomic& LedgerSlot(MemoryRole role) { + static std::atomic ledger[kMemoryRoleCount]; + const std::size_t index = static_cast(role); + return ledger[index < kMemoryRoleCount ? index : 0]; + } + + const char* RoleName(MemoryRole role) { + return role == MemoryRole::Server ? "server" : "client"; + } + + // One pass over /proc/self/status for a "VmHWM:" / "VmRSS:" line. The + // values are in kB and the unit suffix is part of the line, so it is + // parsed rather than assumed. + std::uint64_t ProcStatusBytes(const char* key) { +#if defined(__linux__) || defined(__ANDROID__) + std::FILE* file = std::fopen("/proc/self/status", "re"); + if (file == nullptr) { + return 0; + } + const std::size_t keyLength = std::strlen(key); + char line[256]; + std::uint64_t bytes = 0; + while (std::fgets(line, sizeof(line), file) != nullptr) { + if (std::strncmp(line, key, keyLength) != 0) { + continue; + } + unsigned long long kilobytes = 0; + // The format is ":\t kB". Anything else is a + // kernel this code has not seen, and 0 ("not measured") is the + // honest answer for it. + if (std::sscanf(line + keyLength, ": %llu kB", &kilobytes) == 1) { + bytes = static_cast(kilobytes) * 1024ull; + } + break; + } + std::fclose(file); + return bytes; +#else + (void)key; + return 0; +#endif + } + } // namespace + + std::uint64_t ProcessPeakRssBytes() { return ProcStatusBytes("VmHWM"); } + + std::uint64_t ProcessCurrentRssBytes() { return ProcStatusBytes("VmRSS"); } + + void LedgerAddSegment(MemoryRole role, std::uint64_t bytes) { + LedgerSlot(role).fetch_add(bytes, std::memory_order_relaxed); + } + + void LedgerRemoveSegment(MemoryRole role, std::uint64_t bytes) { + std::atomic& slot = LedgerSlot(role); + const std::uint64_t current = slot.load(std::memory_order_relaxed); + // Clamped rather than wrapped: a double-unbook would otherwise report a + // role holding sixteen exabytes, which is a number nobody reads as a bug. + slot.store(bytes > current ? 0 : current - bytes, std::memory_order_relaxed); + } + + std::uint64_t LedgerMappedBytes(MemoryRole role) { + return LedgerSlot(role).load(std::memory_order_relaxed); + } + + std::uint64_t LedgerMappedBytesAllRoles() { + std::uint64_t total = 0; + for (std::size_t index = 0; index < kMemoryRoleCount; ++index) { + total += LedgerSlot(static_cast(index)).load(std::memory_order_relaxed); + } + return total; + } + + RoleMemorySample SampleRoleMemory(MemoryRole role) { + RoleMemorySample sample; + sample.Role = role; + sample.PeakRssBytes = ProcessPeakRssBytes(); + sample.CurrentRssBytes = ProcessCurrentRssBytes(); + sample.MappedSegmentBytes = LedgerMappedBytes(role); + return sample; + } + + void LogRoleMemory(const char* phase, const RoleMemorySample& sample) { + // INFO, not DEBUG: t1 greps this out of a lane log and MGLOG_D is compiled out at the + // INFO level every P5 lane builds at. It is a handful of lines per session - the + // handshake, the first frame and teardown - so it is not per-frame noise either. + // + // VmHWM is the PROCESS's, so under inproc both roles report the same + // number and only the ledger differs. The line says so rather than + // leaving a reader to work out why two roles have one peak. + MGLOG_I("MG_Remote memory[%s/%s]: peakRss=%llu currentRss=%llu roleMapped=%llu " + "allRolesMapped=%llu (peakRss is the PROCESS's; under inproc both roles share it)", + phase == nullptr ? "?" : phase, RoleName(sample.Role), + static_cast(sample.PeakRssBytes), + static_cast(sample.CurrentRssBytes), + static_cast(sample.MappedSegmentBytes), + static_cast(LedgerMappedBytesAllRoles())); + } + + // ======================================================================= + // P5: SessionSegments + // ======================================================================= + + namespace { + constexpr std::size_t kSlotCount = + static_cast(SessionSegmentSlot::kSessionSegmentCount); + + std::size_t SlotIndex(SessionSegmentSlot slot) { + const std::size_t index = static_cast(slot); + return index < kSlotCount ? index : 0; + } + } // namespace + + SessionSegments::~SessionSegments() { Close(); } + + MobileGLResult SessionSegments::Create(const SessionSegmentSizes& sizes, MemoryRole role) { + Close(); + + struct Spec { + const char* name; + std::uint64_t bytes; + }; + const Spec specs[kSlotCount] = { + {"mgl-cmd", sizes.CmdBytes}, + {"mgl-stage", sizes.StageBytes}, + {"mgl-reply", sizes.ReplyBytes}, + {"mgl-event", sizes.EventBytes}, + }; + + for (std::size_t index = 0; index < kSlotCount; ++index) { + const MobileGLResult created = + ShmSegment::Create(specs[index].name, specs[index].bytes, m_owned[index]); + if (created != MOBILEGL_OK) { + MGLOG_E("MG_Remote session: could not create segment %s of %llu bytes (rc=%d)", + specs[index].name, static_cast(specs[index].bytes), + static_cast(created)); + Close(); + return created; + } + // Read/write on both roles under inproc: they are the same mapping. + // P6's read-only peer view is a property of the ADOPT path, not of + // this one, and pretending otherwise here would give the inproc lane + // a protection the spawn lane does not reproduce. + const MobileGLResult mapped = m_owned[index].Map(false); + if (mapped != MOBILEGL_OK) { + MGLOG_E("MG_Remote session: could not map segment %s (rc=%d)", specs[index].name, + static_cast(mapped)); + Close(); + return mapped; + } + } + + m_owns = true; + m_replySlotCount = sizes.ReplySlotCount; + DeriveViews(); + if (!m_valid) { + Close(); + return MOBILEGL_ERR_INVALID_ARGUMENT; + } + + // Both control pages, zeroed with their generations at 1. The owner does + // this exactly once; the inproc peer must NOT, or it would zero the + // cursors out from under whoever is already using them. + InitRingControl(*m_cmdControl); + InitRingControl(*m_eventControl); + + m_role = role; + LedgerAddSegment(role, m_mappedBytes); + m_booked = true; + return MOBILEGL_OK; + } + + MobileGLResult SessionSegments::AttachInProcess(SessionSegments& owner, MemoryRole role) { + Close(); + if (!owner.Valid()) { + return MOBILEGL_ERR_NOT_INITIALIZED; + } + for (std::size_t index = 0; index < kSlotCount; ++index) { + m_segments[index] = owner.m_segments[index]; + } + m_owns = false; + m_replySlotCount = owner.m_replySlotCount; + DeriveViews(); + if (!m_valid) { + Close(); + return MOBILEGL_ERR_INVALID_ARGUMENT; + } + m_role = role; + // Booked under this role as well, and that double-counting is the point: + // the pages are shared under inproc and are NOT shared under spawn, so + // the per-role numbers are what t1 subtracts with. + LedgerAddSegment(role, m_mappedBytes); + m_booked = true; + return MOBILEGL_OK; + } + + void SessionSegments::DeriveViews() { + m_valid = false; + if (m_owns) { + for (std::size_t index = 0; index < kSlotCount; ++index) { + m_segments[index] = &m_owned[index]; + } + } + for (std::size_t index = 0; index < kSlotCount; ++index) { + if (m_segments[index] == nullptr || m_segments[index]->Data() == nullptr) { + return; + } + } + + auto* cmdBase = static_cast(m_segments[0]->Data()); + m_cmdControl = reinterpret_cast(cmdBase); + m_cmdRingBase = cmdBase + sizeof(RingControl); + m_cmdRingCapacity = RingCapacityForSegment(m_segments[0]->Size()); + + // SEG_STAGE carries no control page of its own: RingControl holds TWO + // cursor triples and the stage triple is the second (Ring.h:106-109). + m_stageBase = m_segments[1]->Data(); + m_stageCapacity = LargestPowerOfTwoAtMost(m_segments[1]->Size()); + + m_replyBase = m_segments[2]->Data(); + m_replyBytes = m_segments[2]->Size(); + + auto* eventBase = static_cast(m_segments[3]->Data()); + m_eventSegmentBase = eventBase; + m_eventControl = reinterpret_cast(eventBase); + m_eventRingBase = eventBase + sizeof(RingControl); + m_eventRingCapacity = RingCapacityForSegment(m_segments[3]->Size()); + + m_mappedBytes = 0; + for (std::size_t index = 0; index < kSlotCount; ++index) { + m_mappedBytes += m_segments[index]->Size(); + } + + if (m_cmdRingCapacity == 0 || m_stageCapacity == 0 || m_eventRingCapacity == 0 || + m_replyBytes == 0) { + MGLOG_E("MG_Remote session: segment sizes leave no usable ring (cmd cap=%llu stage " + "cap=%llu event cap=%llu reply=%llu). A ring is the largest POWER OF TWO that " + "fits after the 4096 byte control page, so a segment must be strictly larger " + "than one page plus the smallest ring", + static_cast(m_cmdRingCapacity), + static_cast(m_stageCapacity), + static_cast(m_eventRingCapacity), + static_cast(m_replyBytes)); + return; + } + m_valid = true; + } + + void SessionSegments::Close() { + if (m_booked) { + LedgerRemoveSegment(m_role, m_mappedBytes); + m_booked = false; + } + if (m_owns) { + for (ShmSegment& segment : m_owned) { + segment.Close(); + } + } + for (std::size_t index = 0; index < kSlotCount; ++index) { + m_segments[index] = nullptr; + } + m_cmdControl = nullptr; + m_cmdRingBase = nullptr; + m_cmdRingCapacity = 0; + m_stageBase = nullptr; + m_stageCapacity = 0; + m_replyBase = nullptr; + m_replyBytes = 0; + m_eventControl = nullptr; + m_eventSegmentBase = nullptr; + m_eventRingBase = nullptr; + m_eventRingCapacity = 0; + m_mappedBytes = 0; + m_owns = false; + m_valid = false; + } + + std::uint64_t SessionSegments::AnnouncedSize(SessionSegmentSlot slot) const { + const ShmSegment* segment = m_segments[SlotIndex(slot)]; + return segment == nullptr ? 0 : segment->Size(); + } + + const char* SessionSegments::AnnouncedName(SessionSegmentSlot slot) const { + const ShmSegment* segment = m_segments[SlotIndex(slot)]; + return segment == nullptr ? "" : segment->Name(); + } + + int SessionSegments::DescriptorFor(SessionSegmentSlot slot) const { + const ShmSegment* segment = m_segments[SlotIndex(slot)]; + return segment == nullptr ? -1 : segment->Fd(); + } + } // namespace MobileGL::MG_Remote::Transport From 7c25130d5a546512ab35467f3d0085bc98763bc1 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 11 Sep 2026 14:13:20 -0400 Subject: [PATCH 02/10] [Feat] (MG_Remote, Protocol): delete CapsSnapshot's four redundant trailing fields and give CallMask, the server's backend type and the ABI fingerprint the carriers they never had - tableSlotMask could not address the 69-slot table its comment named and the two compute limits already ride inside dynamicParameters --- .../Protocol/generated/protocol_generated.h | 138 +++++++++++------- MobileGL/MG_Remote/Protocol/protocol.fbs | 48 +++++- 2 files changed, 128 insertions(+), 58 deletions(-) diff --git a/MobileGL/MG_Remote/Protocol/generated/protocol_generated.h b/MobileGL/MG_Remote/Protocol/generated/protocol_generated.h index eb91a6a5..8324e1be 100644 --- a/MobileGL/MG_Remote/Protocol/generated/protocol_generated.h +++ b/MobileGL/MG_Remote/Protocol/generated/protocol_generated.h @@ -532,7 +532,8 @@ struct Hello FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { VT_BUILDFINGERPRINT = 8, VT_BACKENDTYPE = 10, VT_PID = 12, - VT_CONFIGBLOB = 14 + VT_CONFIGBLOB = 14, + VT_ABIFINGERPRINT = 16 }; uint32_t abiMajor() const { return GetField(VT_ABIMAJOR, 0); @@ -552,6 +553,9 @@ struct Hello FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { const ::flatbuffers::Vector *configBlob() const { return GetPointer *>(VT_CONFIGBLOB); } + uint64_t abiFingerprint() const { + return GetField(VT_ABIFINGERPRINT, 0); + } template bool Verify(::flatbuffers::VerifierTemplate &verifier) const { return VerifyTableStart(verifier) && @@ -563,6 +567,7 @@ struct Hello FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { VerifyField(verifier, VT_PID, 4) && VerifyOffset(verifier, VT_CONFIGBLOB) && verifier.VerifyVector(configBlob()) && + VerifyField(verifier, VT_ABIFINGERPRINT, 8) && verifier.EndTable(); } }; @@ -589,6 +594,9 @@ struct HelloBuilder { void add_configBlob(::flatbuffers::Offset<::flatbuffers::Vector> configBlob) { fbb_.AddOffset(Hello::VT_CONFIGBLOB, configBlob); } + void add_abiFingerprint(uint64_t abiFingerprint) { + fbb_.AddElement(Hello::VT_ABIFINGERPRINT, abiFingerprint, 0); + } explicit HelloBuilder(::flatbuffers::FlatBufferBuilder &_fbb) : fbb_(_fbb) { start_ = fbb_.StartTable(); @@ -607,8 +615,10 @@ inline ::flatbuffers::Offset CreateHello( ::flatbuffers::Offset<::flatbuffers::String> buildFingerprint = 0, uint32_t backendType = 0, uint32_t pid = 0, - ::flatbuffers::Offset<::flatbuffers::Vector> configBlob = 0) { + ::flatbuffers::Offset<::flatbuffers::Vector> configBlob = 0, + uint64_t abiFingerprint = 0) { HelloBuilder builder_(_fbb); + builder_.add_abiFingerprint(abiFingerprint); builder_.add_configBlob(configBlob); builder_.add_pid(pid); builder_.add_backendType(backendType); @@ -630,7 +640,8 @@ inline ::flatbuffers::Offset CreateHelloDirect( const char *buildFingerprint = nullptr, uint32_t backendType = 0, uint32_t pid = 0, - const std::vector *configBlob = nullptr) { + const std::vector *configBlob = nullptr, + uint64_t abiFingerprint = 0) { auto buildFingerprint__ = buildFingerprint ? _fbb.CreateString(buildFingerprint) : 0; auto configBlob__ = configBlob ? _fbb.CreateVector(*configBlob) : 0; return MobileGL::Wire::CreateHello( @@ -640,7 +651,8 @@ inline ::flatbuffers::Offset CreateHelloDirect( buildFingerprint__, backendType, pid, - configBlob__); + configBlob__, + abiFingerprint); } struct Welcome FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { @@ -653,7 +665,9 @@ struct Welcome FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { VT_CMDRING = 10, VT_STAGERING = 12, VT_REPLYPOOL = 14, - VT_EVENTRING = 16 + VT_EVENTRING = 16, + VT_BUILDFINGERPRINT = 18, + VT_ABIFINGERPRINT = 20 }; uint32_t abiMajor() const { return GetField(VT_ABIMAJOR, 0); @@ -676,6 +690,12 @@ struct Welcome FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { const MobileGL::Wire::SegmentRef *eventRing() const { return GetPointer(VT_EVENTRING); } + const ::flatbuffers::String *buildFingerprint() const { + return GetPointer(VT_BUILDFINGERPRINT); + } + uint64_t abiFingerprint() const { + return GetField(VT_ABIFINGERPRINT, 0); + } template bool Verify(::flatbuffers::VerifierTemplate &verifier) const { return VerifyTableStart(verifier) && @@ -690,6 +710,9 @@ struct Welcome FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { verifier.VerifyTable(replyPool()) && VerifyOffset(verifier, VT_EVENTRING) && verifier.VerifyTable(eventRing()) && + VerifyOffset(verifier, VT_BUILDFINGERPRINT) && + verifier.VerifyString(buildFingerprint()) && + VerifyField(verifier, VT_ABIFINGERPRINT, 8) && verifier.EndTable(); } }; @@ -719,6 +742,12 @@ struct WelcomeBuilder { void add_eventRing(::flatbuffers::Offset eventRing) { fbb_.AddOffset(Welcome::VT_EVENTRING, eventRing); } + void add_buildFingerprint(::flatbuffers::Offset<::flatbuffers::String> buildFingerprint) { + fbb_.AddOffset(Welcome::VT_BUILDFINGERPRINT, buildFingerprint); + } + void add_abiFingerprint(uint64_t abiFingerprint) { + fbb_.AddElement(Welcome::VT_ABIFINGERPRINT, abiFingerprint, 0); + } explicit WelcomeBuilder(::flatbuffers::FlatBufferBuilder &_fbb) : fbb_(_fbb) { start_ = fbb_.StartTable(); @@ -738,8 +767,12 @@ inline ::flatbuffers::Offset CreateWelcome( ::flatbuffers::Offset cmdRing = 0, ::flatbuffers::Offset stageRing = 0, ::flatbuffers::Offset replyPool = 0, - ::flatbuffers::Offset eventRing = 0) { + ::flatbuffers::Offset eventRing = 0, + ::flatbuffers::Offset<::flatbuffers::String> buildFingerprint = 0, + uint64_t abiFingerprint = 0) { WelcomeBuilder builder_(_fbb); + builder_.add_abiFingerprint(abiFingerprint); + builder_.add_buildFingerprint(buildFingerprint); builder_.add_eventRing(eventRing); builder_.add_replyPool(replyPool); builder_.add_stageRing(stageRing); @@ -755,6 +788,31 @@ struct Welcome::Traits { static auto constexpr Create = CreateWelcome; }; +inline ::flatbuffers::Offset CreateWelcomeDirect( + ::flatbuffers::FlatBufferBuilder &_fbb, + uint32_t abiMajor = 0, + uint32_t abiMinor = 0, + uint32_t serverPid = 0, + ::flatbuffers::Offset cmdRing = 0, + ::flatbuffers::Offset stageRing = 0, + ::flatbuffers::Offset replyPool = 0, + ::flatbuffers::Offset eventRing = 0, + const char *buildFingerprint = nullptr, + uint64_t abiFingerprint = 0) { + auto buildFingerprint__ = buildFingerprint ? _fbb.CreateString(buildFingerprint) : 0; + return MobileGL::Wire::CreateWelcome( + _fbb, + abiMajor, + abiMinor, + serverPid, + cmdRing, + stageRing, + replyPool, + eventRing, + buildFingerprint__, + abiFingerprint); +} + struct CapsSnapshot FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { typedef CapsSnapshotBuilder Builder; struct Traits; @@ -764,10 +822,8 @@ struct CapsSnapshot FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { VT_FORMATCAPS = 8, VT_EXTENSIONS = 10, VT_APIVERSION = 12, - VT_MAXCOMPUTEWORKGROUPCOUNT = 14, - VT_MAXCOMPUTEWORKGROUPSIZE = 16, - VT_TABLESLOTMASK = 18, - VT_PREFERSCPUXFBPRIMITIVEACCOUNTING = 20 + VT_CALLMASK = 14, + VT_BACKENDTYPE = 16 }; const ::flatbuffers::Vector *dynamicParameters() const { return GetPointer *>(VT_DYNAMICPARAMETERS); @@ -784,17 +840,11 @@ struct CapsSnapshot FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { const ::flatbuffers::String *apiVersion() const { return GetPointer(VT_APIVERSION); } - const ::flatbuffers::Vector *maxComputeWorkGroupCount() const { - return GetPointer *>(VT_MAXCOMPUTEWORKGROUPCOUNT); + uint64_t callMask() const { + return GetField(VT_CALLMASK, 0); } - const ::flatbuffers::Vector *maxComputeWorkGroupSize() const { - return GetPointer *>(VT_MAXCOMPUTEWORKGROUPSIZE); - } - uint64_t tableSlotMask() const { - return GetField(VT_TABLESLOTMASK, 0); - } - bool prefersCpuXfbPrimitiveAccounting() const { - return GetField(VT_PREFERSCPUXFBPRIMITIVEACCOUNTING, 0) != 0; + uint32_t backendType() const { + return GetField(VT_BACKENDTYPE, 0); } template bool Verify(::flatbuffers::VerifierTemplate &verifier) const { @@ -810,12 +860,8 @@ struct CapsSnapshot FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { verifier.VerifyVectorOfStrings(extensions()) && VerifyOffset(verifier, VT_APIVERSION) && verifier.VerifyString(apiVersion()) && - VerifyOffset(verifier, VT_MAXCOMPUTEWORKGROUPCOUNT) && - verifier.VerifyVector(maxComputeWorkGroupCount()) && - VerifyOffset(verifier, VT_MAXCOMPUTEWORKGROUPSIZE) && - verifier.VerifyVector(maxComputeWorkGroupSize()) && - VerifyField(verifier, VT_TABLESLOTMASK, 8) && - VerifyField(verifier, VT_PREFERSCPUXFBPRIMITIVEACCOUNTING, 1) && + VerifyField(verifier, VT_CALLMASK, 8) && + VerifyField(verifier, VT_BACKENDTYPE, 4) && verifier.EndTable(); } }; @@ -839,17 +885,11 @@ struct CapsSnapshotBuilder { void add_apiVersion(::flatbuffers::Offset<::flatbuffers::String> apiVersion) { fbb_.AddOffset(CapsSnapshot::VT_APIVERSION, apiVersion); } - void add_maxComputeWorkGroupCount(::flatbuffers::Offset<::flatbuffers::Vector> maxComputeWorkGroupCount) { - fbb_.AddOffset(CapsSnapshot::VT_MAXCOMPUTEWORKGROUPCOUNT, maxComputeWorkGroupCount); + void add_callMask(uint64_t callMask) { + fbb_.AddElement(CapsSnapshot::VT_CALLMASK, callMask, 0); } - void add_maxComputeWorkGroupSize(::flatbuffers::Offset<::flatbuffers::Vector> maxComputeWorkGroupSize) { - fbb_.AddOffset(CapsSnapshot::VT_MAXCOMPUTEWORKGROUPSIZE, maxComputeWorkGroupSize); - } - void add_tableSlotMask(uint64_t tableSlotMask) { - fbb_.AddElement(CapsSnapshot::VT_TABLESLOTMASK, tableSlotMask, 0); - } - void add_prefersCpuXfbPrimitiveAccounting(bool prefersCpuXfbPrimitiveAccounting) { - fbb_.AddElement(CapsSnapshot::VT_PREFERSCPUXFBPRIMITIVEACCOUNTING, static_cast(prefersCpuXfbPrimitiveAccounting), 0); + void add_backendType(uint32_t backendType) { + fbb_.AddElement(CapsSnapshot::VT_BACKENDTYPE, backendType, 0); } explicit CapsSnapshotBuilder(::flatbuffers::FlatBufferBuilder &_fbb) : fbb_(_fbb) { @@ -869,20 +909,16 @@ inline ::flatbuffers::Offset CreateCapsSnapshot( ::flatbuffers::Offset<::flatbuffers::Vector> formatCaps = 0, ::flatbuffers::Offset<::flatbuffers::Vector<::flatbuffers::Offset<::flatbuffers::String>>> extensions = 0, ::flatbuffers::Offset<::flatbuffers::String> apiVersion = 0, - ::flatbuffers::Offset<::flatbuffers::Vector> maxComputeWorkGroupCount = 0, - ::flatbuffers::Offset<::flatbuffers::Vector> maxComputeWorkGroupSize = 0, - uint64_t tableSlotMask = 0, - bool prefersCpuXfbPrimitiveAccounting = false) { + uint64_t callMask = 0, + uint32_t backendType = 0) { CapsSnapshotBuilder builder_(_fbb); - builder_.add_tableSlotMask(tableSlotMask); - builder_.add_maxComputeWorkGroupSize(maxComputeWorkGroupSize); - builder_.add_maxComputeWorkGroupCount(maxComputeWorkGroupCount); + builder_.add_callMask(callMask); + builder_.add_backendType(backendType); builder_.add_apiVersion(apiVersion); builder_.add_extensions(extensions); builder_.add_formatCaps(formatCaps); builder_.add_rendererInfo(rendererInfo); builder_.add_dynamicParameters(dynamicParameters); - builder_.add_prefersCpuXfbPrimitiveAccounting(prefersCpuXfbPrimitiveAccounting); return builder_.Finish(); } @@ -898,17 +934,13 @@ inline ::flatbuffers::Offset CreateCapsSnapshotDirect( const std::vector *formatCaps = nullptr, const std::vector<::flatbuffers::Offset<::flatbuffers::String>> *extensions = nullptr, const char *apiVersion = nullptr, - const std::vector *maxComputeWorkGroupCount = nullptr, - const std::vector *maxComputeWorkGroupSize = nullptr, - uint64_t tableSlotMask = 0, - bool prefersCpuXfbPrimitiveAccounting = false) { + uint64_t callMask = 0, + uint32_t backendType = 0) { auto dynamicParameters__ = dynamicParameters ? _fbb.CreateVector(*dynamicParameters) : 0; auto rendererInfo__ = rendererInfo ? _fbb.CreateVector(*rendererInfo) : 0; auto formatCaps__ = formatCaps ? _fbb.CreateVector(*formatCaps) : 0; auto extensions__ = extensions ? _fbb.CreateVector<::flatbuffers::Offset<::flatbuffers::String>>(*extensions) : 0; auto apiVersion__ = apiVersion ? _fbb.CreateString(apiVersion) : 0; - auto maxComputeWorkGroupCount__ = maxComputeWorkGroupCount ? _fbb.CreateVector(*maxComputeWorkGroupCount) : 0; - auto maxComputeWorkGroupSize__ = maxComputeWorkGroupSize ? _fbb.CreateVector(*maxComputeWorkGroupSize) : 0; return MobileGL::Wire::CreateCapsSnapshot( _fbb, dynamicParameters__, @@ -916,10 +948,8 @@ inline ::flatbuffers::Offset CreateCapsSnapshotDirect( formatCaps__, extensions__, apiVersion__, - maxComputeWorkGroupCount__, - maxComputeWorkGroupSize__, - tableSlotMask, - prefersCpuXfbPrimitiveAccounting); + callMask, + backendType); } struct DefaultFramebufferInfo FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { diff --git a/MobileGL/MG_Remote/Protocol/protocol.fbs b/MobileGL/MG_Remote/Protocol/protocol.fbs index 968f235b..0e9d52c3 100644 --- a/MobileGL/MG_Remote/Protocol/protocol.fbs +++ b/MobileGL/MG_Remote/Protocol/protocol.fbs @@ -63,6 +63,15 @@ table Hello { backendType: uint; pid: uint; configBlob: [ubyte]; + // CONTRACT-P5 table 0, "ABI agreement": MGPCaps has only a COMPOSITIONAL size + // assertion, because DynamicBackendParameters still carries SizeT and GLenum. + // So the two peers assert they were built from the same struct shapes instead + // of rewriting them fixed-width (that is P7's account). This is + // MG_Remote::CapsAbiFingerprint(): sizeof(DynamicBackendParameters), + // sizeof(MGPCaps), sizeof(GLFunctionsTable), the protocol ABI version and the + // build's git stamp, mixed. A mismatch is Fatal{AbiMismatch} and NEVER a + // downgrade - every alternative silently reads one struct as another. + abiFingerprint: ulong; } table Welcome { @@ -73,6 +82,12 @@ table Welcome { stageRing: SegmentRef; replyPool: SegmentRef; eventRing: SegmentRef; + // The server's half of the assertion above. The string is carried beside the + // mixed value only so a mismatch can name both builds in the Fatal line; the + // COMPARISON is on abiFingerprint, which also covers the three sizeofs the + // string cannot. + buildFingerprint: string; + abiFingerprint: ulong; } // --------------------------------------------------------------------------- @@ -83,16 +98,41 @@ table Welcome { // (plan B appendix A, `get_caps`). The three blobs are byte-for-byte images of // the corresponding POD structs; they are versioned by structSize-first // discipline, not by this schema. +// The four fields this table used to end with are DELETED (CONTRACT-P5 table 0, +// "CapsSnapshot redundancy"), not renamed and not deprecated: +// +// maxComputeWorkGroupCount / maxComputeWorkGroupSize - they ride inside +// `dynamicParameters` already (BackendObject.h:392-393), and two spellings of +// one number is how the two sides come to disagree about it. +// tableSlotMask - GLFunctionsTable has SIXTY-NINE function-pointer slots +// (BackendObject.h:117-292) and a ulong is 64 bits, so the field could never +// address the table its own comment named; and ARCHITECTURE.md:114 already +// retired "is this table slot null" as the capability probe in favour of +// CallMask, so keeping it would re-introduce exactly what replaced it. +// prefersCpuXfbPrimitiveAccounting - answered by kCapCpuXfbPrimitiveAccounting +// in `callMask` below. +// +// They were the LAST four fields, so nothing before them moved a vtable slot. table CapsSnapshot { dynamicParameters: [ubyte]; rendererInfo: [ubyte]; formatCaps: [ubyte]; extensions: [string]; apiVersion: string; - maxComputeWorkGroupCount: [int]; // 3 entries - maxComputeWorkGroupSize: [int]; // 3 entries - tableSlotMask: ulong; // which GLFunctionsTable slots the peer registered - prefersCpuXfbPrimitiveAccounting: bool; + // MGPCaps::CallMask (MGPipeTypes.h:127). Bits 0..8 are MGPCapBit; bits 32..47 + // are the CONSUMER MASK - bit (32+n) means "the server has a consumer for + // MGPipe subsystem bit n" - and MG_Remote/CapsCodec.h holds the four constexprs + // that are the only legal way to fold and test them. It needs a carrier of its + // own because `dynamicParameters` is the image of DynamicBackendParameters, + // which CallMask is not a member of; and without a carrier R-8's rule that the + // client's liveness gates read the caps mirror has nothing to read. + callMask: ulong; + // The SERVER's backend type, for CapsMirror::Backend(). Hello.backendType is + // the CLIENT's request; this is the answer, and the frontend branches that + // switch on it (GL_Framebuffer.cpp:47, GL_Texture.cpp:6536, CompileEnv.cpp:122) + // take a wrong arm rather than fail on a value they do not know, so it may not + // be guessed from the renderer string. + backendType: uint; } table DefaultFramebufferInfo { From 558fb210dcbb27d74d9ab44f3f64a38d5ed189fa Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 11 Sep 2026 14:13:20 -0400 Subject: [PATCH 03/10] [Feat] (MG_Remote, Client, Server): the Hello/Welcome/CapsSnapshot handshake and both sessions' construction and teardown - the ABI assertion runs in Accept before a record is decoded and is Fatal{AbiMismatch} rather than a downgrade, and the two doorbell accessors live on the session rather than on ITransport --- MobileGL/MG_Remote/CapsCodec.cpp | 33 +- MobileGL/MG_Remote/Client/ClientSession.cpp | 380 +++++++++++++++- MobileGL/MG_Remote/Client/ClientSession.h | 72 ++++ MobileGL/MG_Remote/Server/PipeApplier.cpp | 18 +- MobileGL/MG_Remote/Server/ServerSession.cpp | 454 +++++++++++++++++++- MobileGL/MG_Remote/Server/ServerSession.h | 102 ++++- 6 files changed, 1033 insertions(+), 26 deletions(-) diff --git a/MobileGL/MG_Remote/CapsCodec.cpp b/MobileGL/MG_Remote/CapsCodec.cpp index eed88646..dec39a80 100644 --- a/MobileGL/MG_Remote/CapsCodec.cpp +++ b/MobileGL/MG_Remote/CapsCodec.cpp @@ -8,6 +8,9 @@ #include "CapsCodec.h" +#include "Transport/SessionRings.h" + +#include #include #include @@ -49,7 +52,35 @@ namespace MobileGL::MG_Remote { Bool DecodeRendererInfo(const void*, Uint64, RendererInfo&) { MGP5_C0_STUB("DecodeRendererInfo"); } - Uint64 CapsAbiFingerprint() { MGP5_C0_STUB("CapsAbiFingerprint"); } + // s1's half of this file (the two codecs above stay w1's). + // + // MGPCaps has only a COMPOSITIONAL size assertion (MGPipeTypes.h:145-146), + // because DynamicBackendParameters still carries SizeT and GLenum members - + // P0.5's fixed-width rewrite never happened and P5 does not do it either + // (that is P7's account, CONTRACT-P5 table 0). So the two peers assert they + // were built from the SAME struct shapes instead, and a mismatch is + // Fatal{AbiMismatch}, NEVER a downgrade: every alternative to aborting reads + // one struct as another and produces a plausible picture for the wrong reason. + // + // GLFunctionsTable is in the mix even though a split client never receives + // one, because the SERVER's table shape is what the emit table is derived + // from (R-4's 71 slots) and a peer whose table is a different size has a + // different slot numbering. + // + // The git stamp is the weakest of the four inputs and is here for its + // diagnostic value rather than its strength: it is captured at CMAKE + // CONFIGURE time, so an incremental build after a commit still reports the + // configured hash. The three sizeofs are what actually catch a shape change, + // and under P6's spawn - same machine, same binary - all four are trivially + // equal, which is the case this assertion is cheapest in and least needed. + Uint64 CapsAbiFingerprint() { + return Transport::MixAbiFingerprint( + static_cast(sizeof(MG_Backend::DynamicBackendParameters)), + static_cast(sizeof(MG_Pipe::MGPCaps)), + static_cast(sizeof(MG_Backend::GLFunctionsTable)), + MOBILEGL_ABI_VERSION(MOBILEGL_PROTOCOL_ABI_MAJOR, MOBILEGL_PROTOCOL_ABI_MINOR), + GIT_COMMIT_HASH_SHORT); + } #undef MGP5_C0_STUB diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index b800546e..0aa18dfe 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -6,13 +6,22 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P5 c0 stubs for packages s1 (construction, handshake) and c1 (barrier, reply read). +// P5: construction, the handshake and lifetime are package s1's; the verb barrier and the +// reply read that sits inside it are package c1's (EmitAndWait below is still c0's stub). #include "ClientSession.h" +#include "../CapsCodec.h" +#include "../Protocol/generated/protocol_generated.h" +#include "../Server/ServerLoop.h" +#include "../Server/ServerSession.h" +#include "../Transport/InProcessTransport.h" + +#include #include #include +#include namespace MobileGL::MG_Remote::Client { @@ -24,21 +33,350 @@ namespace MobileGL::MG_Remote::Client { std::abort(); \ } while (0) + namespace { + + ClientSession* g_active = nullptr; + + // The same bounded handshake deadline the server uses. Bounded, not kWaitForever: a + // bring-up that never answers has to be a red lane rather than a wedged CI job. + constexpr Uint32 kHandshakeTimeoutMs = 5000; + // Teardown's drain. Also bounded, and for the same reason - table 3's order is + // "publish and wait for the server to drain and acknowledge", and a wait with no + // deadline there turns a lost record into a hung process exit. + constexpr Uint32 kDrainTimeoutMs = 5000; + + MobileGLResult ReceiveEnvelope(Transport::ITransport& transport, std::vector& out, + Uint32 timeoutMs) { + Uint64 size = 0; + MobileGLMutableByteSpan empty{nullptr, 0}; + const MobileGLResult probe = transport.ReceiveFrame(empty, &size, timeoutMs); + if (probe != MOBILEGL_ERR_BUFFER_TOO_SMALL) { + return probe == MOBILEGL_OK ? MOBILEGL_ERR_PROTOCOL_MISMATCH : probe; + } + out.resize(static_cast(size)); + MobileGLMutableByteSpan span{out.data(), out.size()}; + return transport.ReceiveFrame(span, &size, 0); + } + + const ::MobileGL::Wire::CtrlEnvelope* ParseEnvelope(const std::vector& bytes) { + ::flatbuffers::Verifier verifier(bytes.data(), bytes.size()); + if (!::MobileGL::Wire::VerifyCtrlEnvelopeBuffer(verifier)) { + return nullptr; + } + if (!::MobileGL::Wire::CtrlEnvelopeBufferHasIdentifier(bytes.data())) { + return nullptr; + } + return ::MobileGL::Wire::GetCtrlEnvelope(bytes.data()); + } + + [[noreturn]] void FatalAbiMismatch(const char* what, Uint64 ours, Uint64 theirs, + const char* theirStamp) { + MGLOG_F("MGPipe: Fatal{AbiMismatch, \"%s\"} ours=%llu theirs=%llu ourBuild=%s " + "theirBuild=%s - never a downgrade: the caps block's size is ABI-dependent " + "and every field past the first difference would be read at the wrong offset", + what, static_cast(ours), + static_cast(theirs), GIT_COMMIT_HASH_SHORT, + theirStamp == nullptr ? "?" : theirStamp); + std::abort(); + } + + const char* TransportModeName(MG_Config::TransportMode mode) { + switch (mode) { + case MG_Config::TransportMode::Monolith: return "monolith"; + case MG_Config::TransportMode::InProcess: return "inproc"; + case MG_Config::TransportMode::Spawn: return "spawn"; + case MG_Config::TransportMode::UnixSocket: return "unix:"; + case MG_Config::TransportMode::NamedPipe: return "pipe:"; + } + return "?"; + } + + } // namespace + // Null, not a Fatal: MG_Backend::Init() asks whether a session exists before it decides to // install the remote backend object, and that question has a legitimate "no" - it is the // monolith answer. Every call that PRESUMES a session aborts instead. - ClientSession* ClientSession::Active() { return nullptr; } + ClientSession* ClientSession::Active() { return g_active; } - MobileGLResult ClientSession::Start(MG_Config::TransportMode, const String&) { - MGP5_C0_STUB("ClientSession::Start"); + ClientSession& ClientSessionInstance() { + // Leak at exit, deliberately and per ID-8, exactly as ServerSessionInstance does. + static ClientSession* instance = new ClientSession{}; + return *instance; } - void ClientSession::Stop() { MGP5_C0_STUB("ClientSession::Stop"); } + ClientSession::~ClientSession() { + if (m_started) { + Stop(); + } + } + + Bool ClientSession::Started() const { return m_started; } + + MobileGLResult ClientSession::Start(MG_Config::TransportMode mode, const String& endpoint) { + if (m_started) { + return MOBILEGL_ERR_INVALID_ARGUMENT; + } + // A NAMED ERROR, NEVER A FALLBACK TO MONOLITH. A silent fallback here is exactly the + // "the split lane ran monolith and went green" failure the whole phase is built to + // make impossible (ARCHITECTURE.md 10.3), so every mode this build cannot serve is + // refused by name rather than degraded. + if (mode != MG_Config::TransportMode::InProcess) { + MGLOG_E("MG_Remote client: MOBILEGL_TRANSPORT=%s%s is refused by name - P5 implements " + "`inproc` only, and falling back to monolith would make this lane green for " + "the wrong reason. spawn / unix: / pipe: are P6's", + TransportModeName(mode), endpoint.empty() ? "" : endpoint.c_str()); + return MOBILEGL_ERR_UNSUPPORTED; + } + + // ---- 1. the control plane and the two bells. The transport owns the bells; THE + // SESSION owns the rings, and the accessors stay off ITransport (contract §3.9). + Transport::InProcessTransport::CreatePair(m_clientTransport, m_serverTransport); + m_transport = m_clientTransport.get(); + + // ---- 2. Hello. Sent before the server accepts: InProcessTransport queues whole + // messages, so one thread can drive both halves of the handshake in order. + const Uint64 fingerprint = CapsAbiFingerprint(); + { + ::flatbuffers::FlatBufferBuilder builder(512); + auto stamp = builder.CreateString(GIT_COMMIT_HASH_SHORT); + auto hello = ::MobileGL::Wire::CreateHello( + builder, MOBILEGL_PROTOCOL_ABI_MAJOR, MOBILEGL_PROTOCOL_ABI_MINOR, stamp, + /*backendType=*/0u, /*pid=*/0u, /*configBlob=*/0, fingerprint); + auto root = ::MobileGL::Wire::CreateCtrlEnvelope( + builder, ::MobileGL::Wire::CtrlMsg::Hello, hello.Union()); + ::MobileGL::Wire::FinishCtrlEnvelopeBuffer(builder, root); + const MobileGLResult sent = m_transport->SendFrame( + MobileGLByteSpan{builder.GetBufferPointer(), builder.GetSize()}); + if (sent != MOBILEGL_OK) { + Stop(); + return sent; + } + } + + // ---- 3. the server half: ABI assert, four segments, Welcome. + Server::ServerSession& server = Server::ServerSessionInstance(); + const MobileGLResult accepted = server.Accept(*m_serverTransport); + if (accepted != MOBILEGL_OK) { + Stop(); + return accepted; + } + + // ---- 4. Welcome, and this side's half of the ABI assertion. + { + std::vector frame; + const MobileGLResult received = ReceiveEnvelope(*m_transport, frame, kHandshakeTimeoutMs); + if (received != MOBILEGL_OK) { + MGLOG_E("MG_Remote client: no Welcome within %u ms (rc=%d)", kHandshakeTimeoutMs, + static_cast(received)); + Stop(); + return received; + } + const ::MobileGL::Wire::CtrlEnvelope* envelope = ParseEnvelope(frame); + if (envelope == nullptr || envelope->msg_type() != ::MobileGL::Wire::CtrlMsg::Welcome) { + MGLOG_E("MG_Remote client: the server's first control frame is not a verifiable " + "Welcome"); + Stop(); + return MOBILEGL_ERR_PROTOCOL_MISMATCH; + } + const ::MobileGL::Wire::Welcome* welcome = envelope->msg_as_Welcome(); + const char* theirStamp = welcome->buildFingerprint() == nullptr + ? nullptr + : welcome->buildFingerprint()->c_str(); + if (welcome->abiFingerprint() != fingerprint) { + FatalAbiMismatch("struct shapes", fingerprint, welcome->abiFingerprint(), + theirStamp); + } + // The four SegmentRefs are what a spawn client MAPS (P6). Under inproc the mapping + // already exists, so what they are good for here is the cross-check that the two + // sides agree about the geometry at all - which is the assertion that would + // otherwise first run in P6, on the day it is expensive to be wrong. + using Slot = Transport::SessionSegmentSlot; + const auto agrees = [&](const ::MobileGL::Wire::SegmentRef* ref, Slot slot, + const char* name) { + if (ref == nullptr || ref->sizeBytes() != server.Shm().AnnouncedSize(slot)) { + MGLOG_E("MG_Remote client: Welcome's %s SegmentRef announces %llu bytes, the " + "server mapped %llu", + name, + static_cast(ref == nullptr ? 0 : ref->sizeBytes()), + static_cast(server.Shm().AnnouncedSize(slot))); + return false; + } + return true; + }; + if (!agrees(welcome->cmdRing(), Slot::Cmd, "cmd") || + !agrees(welcome->stageRing(), Slot::Stage, "stage") || + !agrees(welcome->replyPool(), Slot::Reply, "reply") || + !agrees(welcome->eventRing(), Slot::Event, "event")) { + Stop(); + return MOBILEGL_ERR_PROTOCOL_MISMATCH; + } + } + + // ---- 5. attach to the four segments. Under `inproc` this is the SAME mapping booked + // under the client role; under `spawn` it becomes ShmSegment::Adopt of the fds the + // server passed by SCM_RIGHTS, which is why nothing below this line knows which it was. + const MobileGLResult attached = + m_shm.AttachInProcess(server.Shm(), Transport::MemoryRole::Client); + if (attached != MOBILEGL_OK) { + Stop(); + return attached; + } + + Transport::RingControl* control = m_shm.CmdControl(); + m_cmd = Transport::RingProducer(control, m_shm.CmdRingBase(), m_shm.CmdRingCapacity(), + Transport::RingCursorSet::Cmd); + m_stage = Transport::RingProducer(control, m_shm.StageBase(), m_shm.StageCapacity(), + Transport::RingCursorSet::Stage); + if (!m_cmd.Valid() || !m_stage.Valid()) { + Stop(); + return MOBILEGL_ERR_INVALID_ARGUMENT; + } + // PeerDoorbell() is the bell the SERVER parks on and this side rings; SelfDoorbell() is + // this side's own. Which is which is the session's knowledge, not the transport's. + m_producer.Attach(control, &m_cmd, &m_stage, &m_clientTransport->PeerDoorbell(), + &m_clientTransport->SelfDoorbell(), MG_Config::Ipc.SpinUs); + + m_replies = Transport::ReplySlotPool(m_shm.ReplyBase(), m_shm.ReplyBytes(), + m_shm.ReplySlotCount()); + m_events = Transport::EventRingConsumer(m_shm.EventControl(), control, + m_shm.EventRingBase(), m_shm.EventRingCapacity(), + m_shm.EventSegmentBase()); + if (!m_replies.Valid() || !m_events.Valid()) { + Stop(); + return MOBILEGL_ERR_INVALID_ARGUMENT; + } + + m_barrierArmed = MG_Config::Ipc.VerbBarrier != 0; + if (!m_barrierArmed) { + MGLOG_W("MG_Remote client: MOBILEGL_IPC_VERB_BARRIER=0 - this is R-1's NEGATIVE " + "CONTROL and is EXPECTED to be red. 31 of the 63 PipeInputs fields are still " + "pulled from a live GLContext by the client's residual fill, so a free-running " + "queue lets the server read a FUTURE value of them"); + } + + // ---- 6. the client's segment table. IT MUST NOT INSTALL THE PROCESS RESOLVER: there is + // exactly one gMGPipeSegmentResolver per process, the SERVER role owns it (table 3), and + // the client never resolves a span at all - it only ever writes Ptr = nullptr (R-2's + // rule B). Two roles racing on that one inline variable is precisely what the server's + // InstallProcessResolver asserts against. + // + // The Install calls themselves are package w1's and are named-Fatal stubs until w1 + // lands; see ServerSession::Accept for why they are called anyway. + m_segments.Install(Wire::kSegCmd, + Wire::SegmentView{m_shm.CmdRingBase(), m_shm.CmdRingCapacity()}); + m_segments.Install(Wire::kSegStage, + Wire::SegmentView{m_shm.StageBase(), m_shm.StageCapacity()}); + m_segments.Install(Wire::kSegReply, + Wire::SegmentView{m_shm.ReplyBase(), m_shm.ReplyBytes()}); + m_segments.Install(Wire::kSegEvent, + Wire::SegmentView{m_shm.EventSegmentBase(), + m_shm.AnnouncedSize(Transport::SessionSegmentSlot::Event)}); + m_encoder = Wire::PipeWireEncoder(control, &m_cmd, &m_stage, &m_segments); + + // ---- 7. the first CapsSnapshot, if the server had a backend to publish one from. + if (m_transport->PeekFrameSize() != 0) { + std::vector frame; + if (ReceiveEnvelope(*m_transport, frame, 0) == MOBILEGL_OK) { + const ::MobileGL::Wire::CtrlEnvelope* envelope = ParseEnvelope(frame); + if (envelope != nullptr && + envelope->msg_type() == ::MobileGL::Wire::CtrlMsg::CapsSnapshot) { + // The mirror's Adopt and the two blob DECODERS are c1's and w1's. s1 stops + // at "the snapshot arrived and is verifiable": adopting it here would put + // the caps mirror's invalidation rule (R-12: a second arrival IS the + // invalidation) in two places. + MGLOG_I("MG_Remote client: first CapsSnapshot received (%llu bytes); adopting " + "it is package c1's CapsMirror::Adopt over package w1's decoders", + static_cast(frame.size())); + } + } + } + + m_started = true; + g_active = this; + LogMemory("handshake"); + + // ---- 8. and only now the apply thread. It is package v1's ServerLoop: it names the + // thread mgl-srv-apply, applies MOBILEGL_IPC_SERVER_AFFINITY and logs the RESOLVED + // mask. Under `inproc` the client is what starts the server role, which is why this + // call is here rather than in some server-side main. + const MobileGLResult running = Server::ServerLoopInstance().Start(server); + if (running != MOBILEGL_OK) { + Stop(); + return running; + } + return MOBILEGL_OK; + } + + void ClientSession::Stop() { + if (!m_started) { + // Start's own failure paths land here with a half-built session; tear down what + // exists and leave nothing mapped. + m_producer.Detach(); + m_shm.Close(); + m_clientTransport.reset(); + m_serverTransport.reset(); + m_transport = nullptr; + return; + } + + // TABLE 3's TEARDOWN ORDER, and every step of it is load-bearing. + // + // 1. publish and let the server drain. Bounded: a lost record must be a red lane, not + // a hung exit. + Transport::RingControl* control = m_shm.CmdControl(); + if (control != nullptr) { + const Uint64 submitted = control->submittedSeq.load(std::memory_order_acquire); + m_producer.PublishAndNotify(submitted); + if (submitted != 0 && + m_producer.WaitForApplied(submitted, kDrainTimeoutMs) != Transport::SessionWait::Reached) { + MGLOG_E("MG_Remote client: the server did not drain to seq %llu within %u ms; " + "tearing down anyway, and anything an emitter still owns is freed below " + "AFTER the join, which is what keeps that from being a use-after-free", + static_cast(submitted), kDrainTimeoutMs); + } + } + + // 2. Doorbell::Kill(). THE ONLY thing that can wake an apply thread parked on + // kWaitForever (Doorbell.h:211-221): a Notify is consumed by one Park, after which + // Doorbell::Wait re-tests a condition nothing published, finds the bell alive and + // parks again, forever. InProcessChannel::Close kills both bells. + if (m_transport != nullptr) { + m_transport->Shutdown(); + } + + // 3. JOIN, bounded - package v1's ServerLoop::Stop, which also destroys the server's + // private BackendObject on that thread before it exits. + Server::ServerLoopInstance().Stop(); + + // 4. and ONLY NOW may anything an emitter owns be released: a var-tail still + // referenced by an unapplied record is a use-after-free the join is what prevents. + LogMemory("teardown"); + m_producer.Detach(); + m_encoder = Wire::PipeWireEncoder(); + m_events = Transport::EventRingConsumer(); + m_replies = Transport::ReplySlotPool(); + m_cmd = Transport::RingProducer(); + m_stage = Transport::RingProducer(); + m_shm.Close(); + Server::ServerSessionInstance().Close(); + m_clientTransport.reset(); + m_serverTransport.reset(); + m_transport = nullptr; + m_started = false; + if (g_active == this) { + g_active = nullptr; + } + } Wire::PipeWireEncoder& ClientSession::Encoder() { return m_encoder; } CapsMirror& ClientSession::Caps() { return CapsMirrorInstance(); } + // PACKAGE c1's. The barrier's wait and the reply's wait are ONE wait (R-3/R-5), which is + // what makes a blocking ReadPixels, MapPersistent's decline and the four Bool acceptances + // cost zero extra round trips - and the client may not re-derive any of those four answers + // locally. s1 supplies the four primitives it composes from: Encoder(), Producer(), + // WaitForApplied() and ReadReply(). Uint64 ClientSession::EmitAndWait(MG_Pipe::MGPWireOp, const void*, Uint64, const void*, Uint64, void*, Uint64, Int32*) { MGP5_C0_STUB("ClientSession::EmitAndWait"); @@ -47,10 +385,40 @@ namespace MobileGL::MG_Remote::Client { Bool ClientSession::BarrierArmed() const { return m_barrierArmed; } // False, not a Fatal, for both: these are the R-1 mutual-exclusion assertion's two probes, - // and an assertion helper that aborts when asked is worse than useless. + // and an assertion helper that aborts when asked is worse than useless. Package c1 gives + // them real answers when it lands the barrier. Bool ClientSession::InBarrierWait() { return false; } Bool ClientSession::ApplyThreadIsInsideApplier() { return false; } + Transport::SessionProducer& ClientSession::Producer() { return m_producer; } + + Transport::SessionWait ClientSession::WaitForApplied(Uint64 seq, Uint32 timeoutMs) { + return m_producer.WaitForApplied(seq, timeoutMs); + } + + Bool ClientSession::ReadReply(Uint64 seq, void* outBytes, Uint64 outCapacity, Int32* outStatus, + Uint64* outSize) { + return m_replies.Read(seq, outBytes, outCapacity, outStatus, outSize); + } + + Uint32 ClientSession::MaxReplyBytes() const { return m_replies.MaxReplyBytes(); } + + Transport::EventRingConsumer& ClientSession::Events() { return m_events; } + + Transport::RingControl* ClientSession::Control() { return m_shm.CmdControl(); } + + Transport::SessionSegments& ClientSession::Shm() { return m_shm; } + + Transport::ITransport* ClientSession::Control_Plane() { return m_transport; } + + Transport::RoleMemorySample ClientSession::SampleMemory() const { + return Transport::SampleRoleMemory(Transport::MemoryRole::Client); + } + + void ClientSession::LogMemory(const char* phase) const { + Transport::LogRoleMemory(phase, SampleMemory()); + } + #undef MGP5_C0_STUB } // namespace MobileGL::MG_Remote::Client diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index 839bed71..61e33c80 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -38,9 +38,22 @@ #include #include +#include "../Transport/Doorbell.h" +#include "../Transport/EventRing.h" +#include "../Transport/ITransport.h" +#include "../Transport/ReplySlot.h" +#include "../Transport/Ring.h" +#include "../Transport/RoleMemory.h" +#include "../Transport/SessionRings.h" #include "../Wire/PipeWireCodec.h" #include "CapsMirror.h" +#include + +namespace MobileGL::MG_Remote::Transport { + class InProcessTransport; +} + namespace MobileGL::MG_Remote::Client { class ClientSession { @@ -48,6 +61,8 @@ namespace MobileGL::MG_Remote::Client { // Null until Start() succeeds; MG_Backend::Init() is the only caller of Start(). static ClientSession* Active(); + ~ClientSession(); + // Builds the four segments, performs Hello/Welcome, takes the first CapsSnapshot, and // - for TransportMode::InProcess - starts the server role's apply thread. Returns a // named error rather than falling back to monolith: a fallback here is the "split lane @@ -85,11 +100,68 @@ namespace MobileGL::MG_Remote::Client { static Bool InBarrierWait(); static Bool ApplyThreadIsInsideApplier(); + // ---- s1's additions: the four primitives c1's EmitAndWait composes --------------- + // + // s1 owns construction and lifetime; c1 owns the barrier POLICY. So the plumbing is + // here and the composition is c1's: encode (w1) -> PublishAndNotify -> WaitForApplied + // -> ReadReply. Splitting it the other way round is how a package ends up + // re-deriving an acceptance answer locally, which is the c0f/c0g defect P4a paid two + // contract corrections for. + + Bool Started() const; + + // Publish the ring head, record submittedSeq, then ring the server IF IT IS PARKED - + // in that order. RingTest.cpp:446 pins the order; SessionProducer is where it lives. + Transport::SessionProducer& Producer(); + + // Wait for RingControl::appliedSeq >= seq. Returns ShutDown when the doorbell died, + // which is the only thing that returns from a kWaitForever park and therefore the + // only way a client blocked in the barrier survives a server that went away. + Transport::SessionWait WaitForApplied(Uint64 seq, Uint32 timeoutMs); + + // The reply slot for `seq`, addressed seq % slots with the seq stamped back into the + // header for self-check (R-3). `outStatus` is 0 OK / 1 DECLINED / 2 ERROR, and + // DECLINED IS A REAL ANSWER - MapPersistent's nullptr and the four Bool acceptances. + Bool ReadReply(Uint64 seq, void* outBytes, Uint64 outCapacity, Int32* outStatus, + Uint64* outSize); + // What one answer may carry. A ReadPixels bigger than this is Fatal rather than + // chunked, so the client checks BEFORE it emits. + Uint32 MaxReplyBytes() const; + + // The reverse channel's reading end: OnBufferWriteback / OnGpuWritten / + // OnSurfaceChanged. Drained by the GL thread between verbs. + Transport::EventRingConsumer& Events(); + + Transport::RingControl* Control(); + Transport::SessionSegments& Shm(); + Transport::ITransport* Control_Plane(); + + // Peak-RSS accounting for t1 (RoleMemory.h). + Transport::RoleMemorySample SampleMemory() const; + void LogMemory(const char* phase) const; + private: Wire::PipeWireEncoder m_encoder; Wire::SegmentTable m_segments; CapsMirror* m_caps = nullptr; Bool m_barrierArmed = true; + + std::unique_ptr m_clientTransport; + std::unique_ptr m_serverTransport; + Transport::SessionSegments m_shm; + Transport::RingProducer m_cmd; + Transport::RingProducer m_stage; + Transport::SessionProducer m_producer; + Transport::EventRingConsumer m_events; + Transport::ReplySlotPool m_replies; + Transport::ITransport* m_transport = nullptr; + Bool m_started = false; }; + // One per process in P5, because P5 serves one context, and LEAKED AT EXIT like every other + // MG_Remote singleton (ID-8): no frontend destructor may reach pipe or backend state from an + // exit handler, and a session destroyed before them would be a use-after-free rather than a + // tidy teardown. MG_Backend::Init() calls Start() on this one. + ClientSession& ClientSessionInstance(); + } // namespace MobileGL::MG_Remote::Client diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index 2d6c3e85..2961477e 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -10,6 +10,8 @@ #include "PipeApplier.h" +#include "../Transport/ReplySlot.h" + #include #include @@ -27,7 +29,21 @@ namespace MobileGL::MG_Remote::Server { ReplyPool::ReplyPool(void* base, Uint64 sizeBytes, Uint32 slotCount, Uint32 slotBytes) : m_base(static_cast(base)), m_size(sizeBytes), m_slots(slotCount), m_slotBytes(slotBytes) {} - void ReplyPool::PostReply(Uint64, Int32, const void*, Uint64) { MGP5_C0_STUB("ReplyPool::PostReply"); } + // PACKAGE s1's, not v1's, even though the class is declared in v1's header: the SEG_REPLY + // slot pool is s1's deliverable (BRIEF §5) and its addressing lives in one place, + // Transport/ReplySlot.h, which the CLIENT reads the same slots back through. Duplicating + // `seq % slots` on this side is how the two halves come to disagree about which slot an + // answer is in - and because seq IS the reply-slot id (R-3), a disagreement reads another + // call's answer instead of failing. + // + // The view is rebuilt per call rather than stored, so that this body does not change + // ReplyPool's four members and therefore does not touch v1's header at all. + void ReplyPool::PostReply(Uint64 seq, Int32 status, const void* bytes, Uint64 size) { + Transport::ReplySlotPool pool(m_base, m_size, m_slots); + // Fatal inside Post when the answer does not fit a slot: P5 does not chunk replies, + // and the client knows an answer's size before it emits the record. + pool.Post(seq, status, bytes, size); + } Uint32 ReplyPool::SlotBytes() const { return m_slotBytes; } diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index d5989d79..500def56 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/MobileGL/MG_Remote/Server/ServerSession.cpp @@ -6,45 +6,465 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// P5 c0 stubs for package s1. +// P5 package s1: the server half of a session. #include "ServerSession.h" +#include "../CapsCodec.h" +#include "../Protocol/generated/protocol_generated.h" +#include "../Transport/InProcessTransport.h" + +#include +#include +#include +#include #include #include +#include namespace MobileGL::MG_Remote::Server { -#define MGP5_C0_STUB(what) \ - do { \ - MGLOG_F("MGPipe: Fatal{UnimplementedServerSession, \"%s\"} - P5 package s1 has not landed " \ - "this yet; c0 shipped the signature only", \ - what); \ - std::abort(); \ - } while (0) + namespace { - ServerSession* ServerSession::Active() { return nullptr; } + // A control-plane frame is small by construction (ITransport.h:56-58: bulk bytes + // belong in shm, never here), so one stack-free vector sized from PeekFrameSize is + // the whole reader. The BUFFER_TOO_SMALL half of ReceiveFrame's contract is what + // makes the two-step safe: a short buffer leaves the message queued. + MobileGLResult ReceiveEnvelope(Transport::ITransport& transport, std::vector& out, + Uint32 timeoutMs) { + Uint64 size = 0; + MobileGLMutableByteSpan empty{nullptr, 0}; + const MobileGLResult probe = transport.ReceiveFrame(empty, &size, timeoutMs); + if (probe != MOBILEGL_ERR_BUFFER_TOO_SMALL) { + // OK with a zero-size message, or a real failure. A zero-length control + // frame is not a legal CtrlEnvelope either way. + return probe == MOBILEGL_OK ? MOBILEGL_ERR_PROTOCOL_MISMATCH : probe; + } + out.resize(static_cast(size)); + MobileGLMutableByteSpan span{out.data(), out.size()}; + return transport.ReceiveFrame(span, &size, 0); + } - MobileGLResult ServerSession::Accept(Transport::ITransport&) { MGP5_C0_STUB("ServerSession::Accept"); } + MobileGLResult SendEnvelope(Transport::ITransport& transport, + ::flatbuffers::FlatBufferBuilder& builder) { + return transport.SendFrame( + MobileGLByteSpan{builder.GetBufferPointer(), builder.GetSize()}); + } + + // Every message from the peer is verified before a single field is read: the control + // plane is parsed from another process's memory (P6) and from another role's (P5). + const ::MobileGL::Wire::CtrlEnvelope* ParseEnvelope(const std::vector& bytes) { + ::flatbuffers::Verifier verifier(bytes.data(), bytes.size()); + if (!::MobileGL::Wire::VerifyCtrlEnvelopeBuffer(verifier)) { + return nullptr; + } + if (!::MobileGL::Wire::CtrlEnvelopeBufferHasIdentifier(bytes.data())) { + return nullptr; + } + return ::MobileGL::Wire::GetCtrlEnvelope(bytes.data()); + } + + [[noreturn]] void FatalAbiMismatch(const char* what, Uint64 ours, Uint64 theirs, + const char* ourStamp, const char* theirStamp) { + // NEVER a downgrade. Every alternative to aborting here reads one struct as + // another - MGPCaps has only a compositional size assertion because + // DynamicBackendParameters still carries SizeT, so a peer built from a different + // tree hands over a caps block whose members are at different offsets and whose + // bytes are all individually plausible. + MGLOG_F("MGPipe: Fatal{AbiMismatch, \"%s\"} ours=%llu theirs=%llu ourBuild=%s " + "theirBuild=%s - the two peers were not built from the same struct shapes, " + "and there is no downgrade path: the caps block's size is ABI-dependent " + "(MGPipeTypes.h:145-146) and every field past the first difference would be " + "read at the wrong offset", + what, static_cast(ours), + static_cast(theirs), + ourStamp == nullptr ? "?" : ourStamp, theirStamp == nullptr ? "?" : theirStamp); + std::abort(); + } + + Transport::SessionSegmentSizes SizesFromConfig() { + Transport::SessionSegmentSizes sizes; +#if MOBILEGL_BUILD_DISAGGREGATED + const Uint64 ringMb = MG_Config::Ipc.RingMb == 0 ? 8u : MG_Config::Ipc.RingMb; + const Uint64 stageMb = MG_Config::Ipc.StageMb == 0 ? 32u : MG_Config::Ipc.StageMb; + sizes.CmdBytes = ringMb * 1024ull * 1024ull; + sizes.StageBytes = stageMb * 1024ull * 1024ull; +#endif + return sizes; + } + + Uint32 SpinUsFromConfig() { +#if MOBILEGL_BUILD_DISAGGREGATED + return MG_Config::Ipc.SpinUs; +#else + return Transport::kDefaultSpinUs; +#endif + } + + // The handshake's own deadline. Bounded rather than kWaitForever on purpose: a + // bring-up that never answers must be a red lane, not a wedged CI job - the same + // reason InProcessTransportTest.cpp:344 bounds its join at five seconds. + constexpr Uint32 kHandshakeTimeoutMs = 5000; + + // WHICH SUBSYSTEMS THIS SERVER CONSUMES, derived from the only registration the tree + // actually has. PipeFill.cpp:920's P4aFamilyHasItsConsumer is the evidence: every P4a + // family hangs off MGPipeGetResourceOps() and there is deliberately "no per-family + // registration to add". Below that, P2's bits 0..6 are consumed by the APPLIER, which + // a server role has by construction. + // + // FLAGGED FOR v1: this is a default, not a ruling. v1 owns the apply thread and knows + // what its backend actually took over, and SetConsumedSubsystems is how it says so. + // Publishing a bit the server does not consume is ID-39's shape from the other side - + // the client emits and nothing applies - so a wrong answer here is not benign. + Uint64 DeriveConsumedSubsystems() { + if (MG_Pipe::MGPipeGetResourceOps() == nullptr) { + return MG_Pipe::kMGPipeSubsystemsMigratedAtP2; + } + return MG_Pipe::kMGPipeSubsystemsMigratedAtP4a; + } + + // The ONE MGPCapBit that is derivable from the server's own registration. Every other + // bit belongs to the package that owns the question it answers; 0 is the honest + // default for those, because a cap bit set on a guess is a capability probe that + // answers "supported" for a path that does not exist (R-15's cross-cutting rule). + Uint64 DeriveCapabilityBits() { + Uint64 bits = 0; + if (MG_Pipe::MGPipeResourceOpsHaveSubDataResident()) { + bits |= static_cast(MG_Pipe::kCapResidentSubData); + } + // kCapNeedsHostIndexBytes and kCapNeedsHostUboBytes are 0 for the whole of P5 by + // ruling, and that is the cheapest way to keep every MGHostSpan out of the first + // IPC frame: they are the only two things that ask for one. + return bits; + } + + ServerSession* g_active = nullptr; + + } // namespace + + ServerSession& ServerSessionInstance() { + // Leak at exit, deliberately and per ID-8: frontend destructors reach pipe and backend + // state from exit handlers, and a session destroyed before them would be a + // use-after-free rather than a tidy teardown. + static ServerSession* instance = new ServerSession{}; + return *instance; + } + + ServerSession* ServerSession::Active() { return g_active; } + + ServerSession::~ServerSession() { Close(); } + + void ServerSession::SetSegmentSizes(const Transport::SessionSegmentSizes& sizes) { + if (m_accepted) { + MGLOG_E("MG_Remote server: SetSegmentSizes after Accept is ignored - the geometry is " + "already on the wire in Welcome and the peer has mapped it"); + return; + } + m_sizes = sizes; + } + + void ServerSession::SetBackend(MG_Backend::BackendObject* backend) { m_backend = backend; } + + void ServerSession::SetCapabilityBits(Uint64 capBits) { + m_capBits = capBits; + m_capBitsSet = true; + } + + void ServerSession::SetConsumedSubsystems(Uint64 subsystemMask) { + m_consumedSubsystems = subsystemMask; + m_consumedSet = true; + } + + Uint64 ServerSession::CallMask() const { + const Uint64 capBits = m_capBitsSet ? m_capBits : DeriveCapabilityBits(); + const Uint64 consumed = m_consumedSet ? m_consumedSubsystems : DeriveConsumedSubsystems(); + return capBits | MGCapsConsumerBits(consumed); + } + + Bool ServerSession::Accepted() const { return m_accepted; } + + MobileGLResult ServerSession::Accept(Transport::ITransport& transport) { + if (m_accepted) { + return MOBILEGL_ERR_INVALID_ARGUMENT; + } + m_transport = &transport; + if (m_sizes.CmdBytes == 0) { + m_sizes = SizesFromConfig(); + } + + // ---- 1/2. the ABI assertion, BEFORE a single record is decoded and before a byte of + // shared memory exists. Its whole purpose is to refuse to interpret the peer's bytes. + std::vector frame; + const MobileGLResult received = ReceiveEnvelope(transport, frame, kHandshakeTimeoutMs); + if (received != MOBILEGL_OK) { + MGLOG_E("MG_Remote server: no Hello within %u ms (rc=%d)", kHandshakeTimeoutMs, + static_cast(received)); + return received; + } + const ::MobileGL::Wire::CtrlEnvelope* envelope = ParseEnvelope(frame); + if (envelope == nullptr || envelope->msg_type() != ::MobileGL::Wire::CtrlMsg::Hello) { + MGLOG_E("MG_Remote server: the first control frame is not a verifiable Hello"); + return MOBILEGL_ERR_PROTOCOL_MISMATCH; + } + const ::MobileGL::Wire::Hello* hello = envelope->msg_as_Hello(); + const char* theirStamp = + hello->buildFingerprint() == nullptr ? nullptr : hello->buildFingerprint()->c_str(); + + if (hello->abiMajor() != static_cast(MOBILEGL_PROTOCOL_ABI_MAJOR) || + hello->abiMinor() != static_cast(MOBILEGL_PROTOCOL_ABI_MINOR)) { + FatalAbiMismatch("protocol version", + MOBILEGL_ABI_VERSION(MOBILEGL_PROTOCOL_ABI_MAJOR, + MOBILEGL_PROTOCOL_ABI_MINOR), + MOBILEGL_ABI_VERSION(hello->abiMajor(), hello->abiMinor()), + GIT_COMMIT_HASH_SHORT, theirStamp); + } + const Uint64 ourFingerprint = CapsAbiFingerprint(); + if (hello->abiFingerprint() != ourFingerprint) { + FatalAbiMismatch("struct shapes", ourFingerprint, hello->abiFingerprint(), + GIT_COMMIT_HASH_SHORT, theirStamp); + } + + // ---- 3. the four segments, both control pages, the rings. + const MobileGLResult created = m_shm.Create(m_sizes, Transport::MemoryRole::Server); + if (created != MOBILEGL_OK) { + return created; + } + + Transport::RingControl* control = m_shm.CmdControl(); + m_commands = Transport::RingConsumer(control, m_shm.CmdRingBase(), m_shm.CmdRingCapacity(), + Transport::RingCursorSet::Cmd); + if (!m_commands.Valid()) { + m_shm.Close(); + return MOBILEGL_ERR_INVALID_ARGUMENT; + } + m_consumer.Attach(control, &m_commands, &ProducerDoorbell(), &ConsumerDoorbell(), + SpinUsFromConfig()); + + { + Transport::ReplySlotPool pool(m_shm.ReplyBase(), m_shm.ReplyBytes(), + m_shm.ReplySlotCount()); + if (!pool.Valid()) { + m_shm.Close(); + return MOBILEGL_ERR_INVALID_ARGUMENT; + } + // A stale stamp from a previous session must never read as this session's answer. + pool.Clear(); + m_replies = ReplyPool(m_shm.ReplyBase(), m_shm.ReplyBytes(), m_shm.ReplySlotCount(), + pool.SlotBytes()); + } + + m_events = Transport::EventRingProducer(m_shm.EventControl(), control, m_shm.EventRingBase(), + m_shm.EventRingCapacity()); + m_applier = PipeApplier(&m_segments, &m_replies); + + // ---- 4. Welcome: the four SegmentRefs, plus this side's half of the ABI statement. + { + ::flatbuffers::FlatBufferBuilder builder(1024); + using Slot = Transport::SessionSegmentSlot; + const auto segmentRef = [&](Uint32 id, ::MobileGL::Wire::SegmentKind kind, Slot slot) { + return ::MobileGL::Wire::CreateSegmentRefDirect(builder, id, kind, + m_shm.AnnouncedSize(slot), + m_shm.AnnouncedName(slot)); + }; + auto cmd = segmentRef(1, ::MobileGL::Wire::SegmentKind::Cmd, Slot::Cmd); + auto stage = segmentRef(2, ::MobileGL::Wire::SegmentKind::Stage, Slot::Stage); + auto reply = segmentRef(3, ::MobileGL::Wire::SegmentKind::Reply, Slot::Reply); + auto event = segmentRef(4, ::MobileGL::Wire::SegmentKind::Event, Slot::Event); + auto stamp = builder.CreateString(GIT_COMMIT_HASH_SHORT); + auto welcome = ::MobileGL::Wire::CreateWelcome( + builder, MOBILEGL_PROTOCOL_ABI_MAJOR, MOBILEGL_PROTOCOL_ABI_MINOR, + static_cast(hello->pid()), cmd, stage, reply, event, stamp, ourFingerprint); + auto root = ::MobileGL::Wire::CreateCtrlEnvelope( + builder, ::MobileGL::Wire::CtrlMsg::Welcome, welcome.Union()); + ::MobileGL::Wire::FinishCtrlEnvelopeBuffer(builder, root); + const MobileGLResult sent = SendEnvelope(transport, builder); + if (sent != MOBILEGL_OK) { + m_shm.Close(); + return sent; + } + } + + // ---- 5. the segment table and the ONE process-wide resolver. + // + // Table 3's ruling: gMGPipeSegmentResolver is a plain non-atomic inline variable and + // there is exactly one per process, so the SERVER role installs it and the client never + // resolves a span at all - it only ever writes Ptr = nullptr. Installed BEFORE the apply + // thread starts, uninstalled after the join and never before. + // + // Both calls are package w1's (Wire/PipeWireCodec.cpp) and are named-Fatal stubs until + // w1 lands. That is deliberate and is where the bring-up currently stops: a session + // that skipped them and carried on would be a decoder with no segments, which is the + // "split lane ran monolith and went green" shape. + m_segments.Install(Wire::kSegCmd, + Wire::SegmentView{m_shm.CmdRingBase(), m_shm.CmdRingCapacity()}); + m_segments.Install(Wire::kSegStage, + Wire::SegmentView{m_shm.StageBase(), m_shm.StageCapacity()}); + m_segments.Install(Wire::kSegReply, + Wire::SegmentView{m_shm.ReplyBase(), m_shm.ReplyBytes()}); + m_segments.Install(Wire::kSegEvent, Wire::SegmentView{m_shm.EventSegmentBase(), + m_shm.AnnouncedSize( + Transport::SessionSegmentSlot::Event)}); + m_segments.InstallProcessResolver(); + + m_accepted = true; + g_active = this; + LogMemory("accept"); + + // ---- 6. the first CapsSnapshot, if there is a backend to take it from. + if (m_backend != nullptr) { + const MobileGLResult published = PublishCapsSnapshot(); + if (published != MOBILEGL_OK) { + return published; + } + } else { + MGLOG_W("MG_Remote server: accepted with NO backend, so the first CapsSnapshot is " + "deferred. Call SetBackend() then PublishCapsSnapshot(). A client that emits " + "before the snapshot arrives reads a placeholder caps mirror"); + } + return MOBILEGL_OK; + } MobileGLResult ServerSession::PublishCapsSnapshot() { - MGP5_C0_STUB("ServerSession::PublishCapsSnapshot"); + if (!m_accepted || m_transport == nullptr) { + return MOBILEGL_ERR_NOT_INITIALIZED; + } + if (m_backend == nullptr) { + MGLOG_E("MG_Remote server: PublishCapsSnapshot with no backend"); + return MOBILEGL_ERR_NOT_INITIALIZED; + } + + // The two blob codecs are package w1's (CapsCodec.h). They are named-Fatal stubs + // today; nothing here may substitute a memcpy for them, because both structures hold + // Vectors and Strings and a memcpy of either crosses a host pointer (R-2's rule B). + Vector formats; + Vector renderer; + if (!EncodeFormatCapabilities(m_backend->GetFormatCapabilities(), formats)) { + return MOBILEGL_ERR_PROTOCOL_MISMATCH; + } + if (!EncodeRendererInfo(m_backend->GetRendererInfo(), renderer)) { + return MOBILEGL_ERR_PROTOCOL_MISMATCH; + } + + const MG_Backend::DynamicBackendParameters& dynamic = m_backend->GetDynamicParameters(); + const auto* dynamicBytes = reinterpret_cast(&dynamic); + + // R-8/C-4: bits 32..47 of CallMask are the CONSUMER MASK, and this is the only place + // they are produced. + const Uint64 callMask = CallMask(); + + ::flatbuffers::FlatBufferBuilder builder(4096); + auto dynamicVector = + builder.CreateVector(dynamicBytes, static_cast<::flatbuffers::uoffset_t>(sizeof(dynamic))); + auto rendererVector = builder.CreateVector(renderer.data(), renderer.size()); + auto formatsVector = builder.CreateVector(formats.data(), formats.size()); + const RendererInfo& info = m_backend->GetRendererInfo(); + auto apiVersion = builder.CreateString(info.RendererGLInfo.TargetGLVersion.toString()); + auto snapshot = ::MobileGL::Wire::CreateCapsSnapshot( + builder, dynamicVector, rendererVector, formatsVector, /*extensions=*/0, apiVersion, + callMask, static_cast(m_backend->GetBackendType())); + auto root = ::MobileGL::Wire::CreateCtrlEnvelope( + builder, ::MobileGL::Wire::CtrlMsg::CapsSnapshot, snapshot.Union()); + ::MobileGL::Wire::FinishCtrlEnvelopeBuffer(builder, root); + return SendEnvelope(*m_transport, builder); + } + + void ServerSession::Close() { + if (g_active == this) { + g_active = nullptr; + } + if (m_accepted) { + // Uninstall AFTER the apply thread has joined, never before: a record still in + // flight can still resolve a segment offset (table 3's fourth column). + Wire::SegmentTable::UninstallProcessResolver(); + } + m_consumer.Detach(); + m_commands = Transport::RingConsumer(); + m_events = Transport::EventRingProducer(); + m_replies = ReplyPool(); + m_shm.Close(); + m_transport = nullptr; + m_accepted = false; } Transport::RingConsumer& ServerSession::CommandRing() { return m_commands; } - Transport::RingControl& ServerSession::Control() { MGP5_C0_STUB("ServerSession::Control"); } + + Transport::RingControl& ServerSession::Control() { + Transport::RingControl* control = m_shm.CmdControl(); + if (control == nullptr) { + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"ServerSession::Control\"} - the control " + "page does not exist until Accept() has mapped SEG_CMD"); + std::abort(); + } + return *control; + } + Wire::SegmentTable& ServerSession::Segments() { return m_segments; } PipeApplier& ServerSession::Applier() { return m_applier; } ReplyPool& ServerSession::Replies() { return m_replies; } + // The bell the apply thread parks on. On the server endpoint of an InProcessTransport that + // is SelfDoorbell(); the client reaches the same bell through its own PeerDoorbell(). Transport::Doorbell& ServerSession::ConsumerDoorbell() { - MGP5_C0_STUB("ServerSession::ConsumerDoorbell"); - } - Transport::Doorbell& ServerSession::ProducerDoorbell() { - MGP5_C0_STUB("ServerSession::ProducerDoorbell"); + if (m_transport == nullptr) { + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"ServerSession::ConsumerDoorbell\"} - no " + "transport; Accept() has not run"); + std::abort(); + } + if (m_transport->Role() == Transport::TransportRole::InProcess) { + return static_cast(m_transport)->SelfDoorbell(); + } + // P6: SocketTransport's pair. The accessors stay off ITransport by ruling (contract + // §3.9) precisely so that this stays one switch in one file rather than two virtuals + // every transport has to invent a home for. + MGLOG_F("MGPipe: Fatal{UnmigratedVerb, \"ServerSession::ConsumerDoorbell\"} - transport " + "role %u has no doorbell pair yet; that is P6's SocketTransport", + static_cast(m_transport->Role())); + std::abort(); } -#undef MGP5_C0_STUB + Transport::Doorbell& ServerSession::ProducerDoorbell() { + if (m_transport == nullptr) { + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"ServerSession::ProducerDoorbell\"} - no " + "transport; Accept() has not run"); + std::abort(); + } + if (m_transport->Role() == Transport::TransportRole::InProcess) { + return static_cast(m_transport)->PeerDoorbell(); + } + MGLOG_F("MGPipe: Fatal{UnmigratedVerb, \"ServerSession::ProducerDoorbell\"} - transport " + "role %u has no doorbell pair yet; that is P6's SocketTransport", + static_cast(m_transport->Role())); + std::abort(); + } + + Transport::SessionSegments& ServerSession::Shm() { return m_shm; } + Transport::SessionConsumer& ServerSession::Consumer() { return m_consumer; } + Transport::EventRingProducer& ServerSession::Events() { return m_events; } + Transport::ITransport* ServerSession::Control_Plane() { return m_transport; } + + void ServerSession::AdvanceCompletedFrame(Uint64 serial) { + if (!m_accepted) { + return; + } + Transport::Watermark::AdvanceCompletedFrame(Control(), serial); + m_consumer.NotifyClient(); + } + + void ServerSession::ReturnPresentCredit(Uint64 serial) { + if (!m_accepted) { + return; + } + Transport::Watermark::AdvancePresentAck(Control(), serial); + m_consumer.NotifyClient(); + } + + Transport::RoleMemorySample ServerSession::SampleMemory() const { + return Transport::SampleRoleMemory(Transport::MemoryRole::Server); + } + + void ServerSession::LogMemory(const char* phase) const { + Transport::LogRoleMemory(phase, SampleMemory()); + } } // namespace MobileGL::MG_Remote::Server diff --git a/MobileGL/MG_Remote/Server/ServerSession.h b/MobileGL/MG_Remote/Server/ServerSession.h index a2b9fda8..38b6073b 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.h +++ b/MobileGL/MG_Remote/Server/ServerSession.h @@ -12,13 +12,32 @@ // 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, so the default 8 MiB caps a record at 4 MiB (R-10). +// 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. #pragma once #include @@ -26,8 +45,12 @@ #include #include "../Transport/Doorbell.h" +#include "../Transport/EventRing.h" #include "../Transport/ITransport.h" +#include "../Transport/ReplySlot.h" #include "../Transport/Ring.h" +#include "../Transport/RoleMemory.h" +#include "../Transport/SessionRings.h" #include "../Wire/PipeWireCodec.h" #include "PipeApplier.h" @@ -37,6 +60,8 @@ namespace MobileGL::MG_Remote::Server { public: static ServerSession* Active(); + ~ServerSession(); + // Maps the four segments, answers Hello with Welcome, and publishes the first // CapsSnapshot. The ABI assertion (CapsCodec.h) happens HERE, before a single record is // decoded: sizeof(DynamicBackendParameters), sizeof(MGPCaps), sizeof(GLFunctionsTable) @@ -62,11 +87,86 @@ namespace MobileGL::MG_Remote::Server { // cache line otherwise burns a big core for a whole frame on a phone). Transport::Doorbell& ProducerDoorbell(); + // ---- s1's additions beyond c0's signature block --------------------------------- + + // The sizes to create the segments with. Must be called before Accept; after it, the + // geometry is on the wire in Welcome and changing it would desynchronise the peer. + void SetSegmentSizes(const Transport::SessionSegmentSizes& sizes); + + // The server role's private BackendObject (ServerLoop::Backend(), v1's). It is NOT + // pActiveBackendObject - that global holds the client's BackendObject_Remote (table 3). + // Set before Accept to have the first CapsSnapshot published there; set later and call + // PublishCapsSnapshot yourself. + void SetBackend(MG_Backend::BackendObject* backend); + + // MGPCaps::CallMask's two halves, kept apart because they have different owners. + // + // BITS 0..8, THE MGPCapBit FEATURE BITS: nothing in the tree produces them today - + // CallMask is declared at MGPipeTypes.h:133 and written by nobody - and what each one + // answers belongs to the package that owns the question (kCapTimerQuery to the query + // family, kCapResidentSubData to b1, and so on). The default below derives only the + // ONE bit that is mechanically derivable from the server's own registration, and + // kCapNeedsHostIndexBytes / kCapNeedsHostUboBytes stay 0 for the whole of P5 by + // ruling (CONTRACT-P5 table 0), which is what keeps every MGHostSpan out of the first + // IPC frame. Everything else is an owner's to set here. + void SetCapabilityBits(Uint64 capBits); + + // BITS 32..47, THE CONSUMER MASK (R-8 / C-4): which MGPipe subsystems this server has + // a consumer for. The client's liveness gates read it back through + // CapsMirror::ServerConsumes and may NEVER read MGPipeGetResourceOps() - that is the + // server's registration, which under inproc a client reads correctly by accident and + // under spawn reads as null, silently disabling five whole record families. + void SetConsumedSubsystems(Uint64 subsystemMask); + + // What PublishCapsSnapshot puts on the wire: capBits | MGCapsConsumerBits(subsystems). + Uint64 CallMask() const; + + Bool Accepted() const; + // Teardown: after the apply thread has been joined, never before - a record still in + // flight can still resolve a segment offset (table 3's fourth column). + void Close(); + + Transport::SessionSegments& Shm(); + // The apply loop's end of the rings: WaitForWork / ApplyOne / RetireThrough. This is + // the only thing that advances appliedSeq, and it advances it by exactly one per + // record (R-9's ban on batching it while the verb barrier exists). + Transport::SessionConsumer& Consumer(); + // The reverse channel. P5 only has to be able to CARRY OnBufferWriteback / + // OnGpuWritten / OnSurfaceChanged; the overflow policy is P9's. + Transport::EventRingProducer& Events(); + Transport::ITransport* Control_Plane(); + + // completedFrameSerial / presentAckSerial: the two watermarks only the server can + // advance, kept together with the other three rather than poked into RingControl from + // whatever code happens to notice a present finished. + void AdvanceCompletedFrame(Uint64 serial); + void ReturnPresentCredit(Uint64 serial); + + // Peak-RSS accounting for t1 (RoleMemory.h). `phase` is a short tag. + Transport::RoleMemorySample SampleMemory() const; + void LogMemory(const char* phase) const; + private: Transport::RingConsumer m_commands; Wire::SegmentTable m_segments; PipeApplier m_applier; ReplyPool m_replies; + + Transport::SessionSegments m_shm; + Transport::SessionConsumer m_consumer; + Transport::EventRingProducer m_events; + Transport::ITransport* m_transport = nullptr; + MG_Backend::BackendObject* m_backend = nullptr; + Transport::SessionSegmentSizes m_sizes; + Uint64 m_capBits = 0; + Uint64 m_consumedSubsystems = 0; + Bool m_capBitsSet = false; + Bool m_consumedSet = false; + Bool m_accepted = false; }; + // Leak-at-exit like every other MG_Remote singleton (ID-8): no frontend destructor may + // reach pipe or backend state from an exit handler. + ServerSession& ServerSessionInstance(); + } // namespace MobileGL::MG_Remote::Server From 1f953c09eabd1413a5d74be8a7d816476c7c198f Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 11 Sep 2026 14:13:20 -0400 Subject: [PATCH 04/10] [Test] (MG_Test, Wire): pin the session pair - twenty thousand records across two real threads, one case per watermark rule, the kRecPad rule against the session's own counter, the reply slot's seq stamp and a shutdown whose join is bounded at five seconds so a lost wakeup fails red --- MobileGL/MG_Test/Wire/CMakeLists.txt | 3 + MobileGL/MG_Test/Wire/SessionTest.cpp | 769 ++++++++++++++++++++++++++ 2 files changed, 772 insertions(+) create mode 100644 MobileGL/MG_Test/Wire/SessionTest.cpp diff --git a/MobileGL/MG_Test/Wire/CMakeLists.txt b/MobileGL/MG_Test/Wire/CMakeLists.txt index 0af34f0b..76d308ea 100644 --- a/MobileGL/MG_Test/Wire/CMakeLists.txt +++ b/MobileGL/MG_Test/Wire/CMakeLists.txt @@ -9,6 +9,9 @@ set(MOBILEGL_WIRE_TESTS RingTest InProcessTransportTest ProtocolSmokeTest + # P5 s1: the ring-owning session pair - four ShmSegment-backed segments, the five + # watermarks with real writers, the reply slot pool and the event ring. + SessionTest ) if (NOT WIN32) diff --git a/MobileGL/MG_Test/Wire/SessionTest.cpp b/MobileGL/MG_Test/Wire/SessionTest.cpp new file mode 100644 index 00000000..f2fff637 --- /dev/null +++ b/MobileGL/MG_Test/Wire/SessionTest.cpp @@ -0,0 +1,769 @@ +// MobileGL - MobileGL/MG_Test/Wire/SessionTest.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// The ring-owning session pair: four ShmSegment-backed segments, two real threads over the +// SEG_CMD ring, the five watermarks with real writers, the SEG_REPLY slot pool, the SEG_EVENT +// reverse channel, and a shutdown that a lost wakeup turns RED rather than hanging. +// +// WHY THIS SUITE EXISTS ALONGSIDE RingTest. RingTest pins the ring's own mechanics against a +// fixture whose control page is on the stack and whose byte area is a std::vector, and it pins +// the five watermark RULES against nobody, because until P5 nothing in the tree wrote one +// (every watermark was declared, zeroed by InitRingControl and written by no code at all). +// This suite pins the WRITERS: SessionProducer, SessionConsumer and the Watermark namespace are +// the only things that advance them, and they do it over memory that came from ShmSegment. +// +// AND THAT LAST PART IS THE POINT. `inproc` allocating its rings with new[] would work, would +// be shorter, and would move every question about mapping, alignment, size rounding and +// lifetime into P6 - onto the day the second process appears. So the session uses ShmSegment in +// both delivery modes and SessionSegmentsAreRealSharedMemory below is the mechanical check that +// it still does. + +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include +#include +#include +#include +#include +#include + +using namespace MobileGL::MG_Remote::Transport; + +namespace { + + // Small enough to be cheap in CI, large enough that 20 000 sixteen-byte records wrap the + // command ring many times over - which is what puts the kRecPad rule under load rather + // than under a contrived single wrap. + SessionSegmentSizes TestSizes() { + SessionSegmentSizes sizes; + sizes.CmdBytes = 64ull * 1024; // -> 32 KiB ring after the control page + sizes.StageBytes = 64ull * 1024; // -> 64 KiB, no control page of its own + sizes.ReplyBytes = 64ull * 1024; // -> 8 slots of 8 KiB + sizes.EventBytes = 32ull * 1024; // -> 16 KiB ring after the control page + sizes.ReplySlotCount = 8; + return sizes; + } + + // One session's worth of everything, wired the way ClientSession and ServerSession wire it: + // the server owns the segments, the client attaches to the same mapping, and the two + // doorbells come from the transport while the RINGS come from here. + struct SessionFixture { + std::unique_ptr clientTransport; + std::unique_ptr serverTransport; + SessionSegments serverSegments; + SessionSegments clientSegments; + RingProducer cmdProducer; + RingProducer stageProducer; + RingConsumer cmdConsumer; + SessionProducer producer; + SessionConsumer consumer; + ReplySlotPool replies; + EventRingProducer eventOut; + EventRingConsumer eventIn; + + bool Build(const SessionSegmentSizes& sizes) { + InProcessTransport::CreatePair(clientTransport, serverTransport); + if (serverSegments.Create(sizes, MemoryRole::Server) != MOBILEGL_OK) { + return false; + } + if (clientSegments.AttachInProcess(serverSegments, MemoryRole::Client) != MOBILEGL_OK) { + return false; + } + RingControl* control = serverSegments.CmdControl(); + cmdProducer = RingProducer(control, clientSegments.CmdRingBase(), + clientSegments.CmdRingCapacity(), RingCursorSet::Cmd); + stageProducer = RingProducer(control, clientSegments.StageBase(), + clientSegments.StageCapacity(), RingCursorSet::Stage); + cmdConsumer = RingConsumer(control, serverSegments.CmdRingBase(), + serverSegments.CmdRingCapacity(), RingCursorSet::Cmd); + if (!cmdProducer.Valid() || !stageProducer.Valid() || !cmdConsumer.Valid()) { + return false; + } + // PeerDoorbell is the bell the OTHER end parks on; SelfDoorbell is this end's own. + // Which is which is the session's knowledge, never ITransport's (contract §3.9). + producer.Attach(control, &cmdProducer, &stageProducer, &clientTransport->PeerDoorbell(), + &clientTransport->SelfDoorbell(), kDefaultSpinUs); + consumer.Attach(control, &cmdConsumer, &serverTransport->PeerDoorbell(), + &serverTransport->SelfDoorbell(), kDefaultSpinUs); + replies = ReplySlotPool(serverSegments.ReplyBase(), serverSegments.ReplyBytes(), + serverSegments.ReplySlotCount()); + replies.Clear(); + eventOut = EventRingProducer(serverSegments.EventControl(), control, + serverSegments.EventRingBase(), + serverSegments.EventRingCapacity()); + eventIn = EventRingConsumer(clientSegments.EventControl(), control, + clientSegments.EventRingBase(), + clientSegments.EventRingCapacity(), + clientSegments.EventSegmentBase()); + return replies.Valid() && eventOut.Valid() && eventIn.Valid(); + } + + RingControl& Control() { return *serverSegments.CmdControl(); } + }; + +} // namespace + +// --------------------------------------------------------------------------- +// The segments themselves +// --------------------------------------------------------------------------- + +// `inproc` must not quietly become new[]. A descriptor (POSIX) or a native handle (Windows) is +// the mechanical difference between a session whose spawn path is the same code and one whose +// spawn path is written for the first time in P6. +TEST(SessionTest, SessionSegmentsAreRealSharedMemoryAndNotAHeapAllocation) { + SessionSegments segments; + ASSERT_EQ(segments.Create(TestSizes(), MemoryRole::Server), MOBILEGL_OK); + EXPECT_TRUE(segments.Valid()); +#if !defined(_WIN32) + EXPECT_GE(segments.DescriptorFor(SessionSegmentSlot::Cmd), 0); + EXPECT_GE(segments.DescriptorFor(SessionSegmentSlot::Stage), 0); + EXPECT_GE(segments.DescriptorFor(SessionSegmentSlot::Reply), 0); + EXPECT_GE(segments.DescriptorFor(SessionSegmentSlot::Event), 0); +#endif + // Both control pages start initialised, with the two generations at 1 and every watermark + // at 0 - the two conventions are opposite on purpose (Ring.h). + ASSERT_NE(segments.CmdControl(), nullptr); + ASSERT_NE(segments.EventControl(), nullptr); + EXPECT_EQ(segments.CmdControl()->ringGeneration.load(), 1u); + EXPECT_EQ(segments.EventControl()->ringGeneration.load(), 1u); + EXPECT_EQ(segments.CmdControl()->appliedSeq.load(), 0u); + segments.Close(); + EXPECT_FALSE(segments.Valid()); +} + +// The arithmetic the whole geometry rests on, and the one place CONTRACT-P5 §5's "8 MiB caps +// one record at 4 MiB" is corrected: the control page sits at the HEAD of SEG_CMD and the ring +// capacity must be a power of two, so an 8 MiB segment yields a 4 MiB ring and a 2 MiB record. +TEST(SessionTest, TheRingIsTheLargestPowerOfTwoLeftAfterTheControlPage) { + EXPECT_EQ(LargestPowerOfTwoAtMost(0u), 0u); + EXPECT_EQ(LargestPowerOfTwoAtMost(1u), 1u); + EXPECT_EQ(LargestPowerOfTwoAtMost(4095u), 2048u); + EXPECT_EQ(LargestPowerOfTwoAtMost(4096u), 4096u); + EXPECT_EQ(LargestPowerOfTwoAtMost(4097u), 4096u); + + // The four contract sizes, which ProtocolSmokeTest.cpp:72 pins on the wire. + constexpr std::uint64_t kCmd = 8ull * 1024 * 1024; + constexpr std::uint64_t kEvent = 256ull * 1024; + EXPECT_EQ(RingCapacityForSegment(kCmd), 4ull * 1024 * 1024); + EXPECT_EQ(RingCapacityForSegment(kEvent), 128ull * 1024); + // ... and therefore the real cap on one record, which R-10 obliges the codec to prove it + // never approaches. Half of the ring, not half of the segment. + RingControl control{}; + InitRingControl(control); + std::vector bytes(static_cast(RingCapacityForSegment(kCmd))); + RingProducer producer(&control, bytes.data(), RingCapacityForSegment(kCmd), RingCursorSet::Cmd); + ASSERT_TRUE(producer.Valid()); + EXPECT_EQ(producer.MaxRecordBytes(), 2ull * 1024 * 1024); + + // A segment that cannot hold the control page plus the smallest ring has NO ring, rather + // than a ring of some rounded-down nonsense. + EXPECT_EQ(RingCapacityForSegment(sizeof(RingControl)), 0u); + EXPECT_EQ(RingCapacityForSegment(sizeof(RingControl) + 8), 0u); +} + +// The default geometry really allocates, and the announced sizes are the MAPPING sizes - what +// a spawn peer must map - not the ring capacity inside them. +TEST(SessionTest, TheDefaultGeometryIsTheFourContractSizes) { + SessionSegments segments; + ASSERT_EQ(segments.Create(SessionSegmentSizes{}, MemoryRole::Server), MOBILEGL_OK); + EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Cmd), 8ull * 1024 * 1024); + EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Stage), 32ull * 1024 * 1024); + EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Reply), 8ull * 1024 * 1024); + EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Event), 256ull * 1024); + EXPECT_EQ(segments.CmdRingCapacity(), 4ull * 1024 * 1024); + // SEG_STAGE carries no control page: RingControl holds both cursor triples, so the whole + // segment is ring and 32 MiB is already a power of two. + EXPECT_EQ(segments.StageCapacity(), 32ull * 1024 * 1024); + segments.Close(); +} + +// The ledger is per role and DOES double-count under inproc, deliberately: the two roles map +// the same pages here and will not under spawn, so the per-role numbers are what t1 subtracts +// with and a silently deduplicated total would hide exactly that difference. +TEST(SessionTest, TheMemoryLedgerIsPerRoleAndIsReleasedOnClose) { + const std::uint64_t clientBefore = LedgerMappedBytes(MemoryRole::Client); + const std::uint64_t serverBefore = LedgerMappedBytes(MemoryRole::Server); + { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + const std::uint64_t mapped = session.serverSegments.MappedBytes(); + EXPECT_GT(mapped, 0u); + EXPECT_EQ(LedgerMappedBytes(MemoryRole::Server), serverBefore + mapped); + EXPECT_EQ(LedgerMappedBytes(MemoryRole::Client), clientBefore + mapped); + + const RoleMemorySample sample = SampleRoleMemory(MemoryRole::Client); + EXPECT_EQ(sample.MappedSegmentBytes, clientBefore + mapped); +#if defined(__linux__) || defined(__ANDROID__) + // VmHWM is the PROCESS's high-water mark, so it is the same number for both roles and + // is only meaningful beside the ledger - which is why RoleMemorySample carries both. + EXPECT_GT(sample.PeakRssBytes, 0u); + EXPECT_GE(sample.PeakRssBytes, sample.CurrentRssBytes); +#endif + } + EXPECT_EQ(LedgerMappedBytes(MemoryRole::Client), clientBefore); + EXPECT_EQ(LedgerMappedBytes(MemoryRole::Server), serverBefore); +} + +// --------------------------------------------------------------------------- +// Two real threads, twenty thousand records +// --------------------------------------------------------------------------- + +// The acceptance run. A 32 KiB command ring and 16-byte records means this wraps about ten +// times, so the wrap filler is exercised under load rather than in one contrived case - and +// the invariant checked at the end is that appliedSeq counted the RECORDS and not the fillers. +TEST(SessionTest, TwoThreadsMoveTwentyThousandRecordsAndAgreeOnEveryOne) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + + constexpr std::uint32_t kRecords = 20000; + std::atomic ok{true}; + std::atomic consumed{0}; + + std::thread apply([&] { + std::uint32_t seen = 0; + while (seen < kRecords && ok.load()) { + const SessionWait woke = session.consumer.WaitForWork(5000); + if (woke == SessionWait::ShutDown) { + return; + } + if (woke == SessionWait::TimedOut) { + ok.store(false); // a lost wakeup, or the producer stalled: RED, never a hang + return; + } + bool corrupt = false; + while (session.consumer.ApplyOne( + [&](const RingRecordView& view) { + std::uint32_t value = 0; + std::memcpy(&value, view.payload, sizeof(value)); + if (value != seen) { + ok.store(false); + } + // appliedSeq is advanced AFTER this returns, once, by the session - which + // is what the barrier's waiter is entitled to assume. + ++seen; + consumed.store(seen); + }, + &corrupt)) { + if (!ok.load()) { + return; + } + } + if (corrupt) { + ok.store(false); + return; + } + session.consumer.RetireThrough(session.consumer.AppliedSeq()); + } + }); + + std::uint64_t emitted = 0; + for (std::uint32_t index = 0; index < kRecords && ok.load(); ++index) { + void* payload = nullptr; + while ((payload = session.cmdProducer.Reserve(1, kRecNone, sizeof(std::uint32_t))) == + nullptr) { + // Never "wait" on a nullptr with enough free bytes - Ring.h:226-233 says that can + // only mean "too big, chunk", and a producer that waited there would stall for ever. + ASSERT_LT(session.cmdProducer.FreeBytes(), 16u); + if (session.producer.WaitForCmdSpace(16, 5000) != SessionWait::Reached) { + ok.store(false); + break; + } + } + if (payload == nullptr) { + break; + } + std::memcpy(payload, &index, sizeof(index)); + ++emitted; + session.producer.PublishAndNotify(emitted); + } + + apply.join(); + EXPECT_TRUE(ok.load()); + EXPECT_EQ(consumed.load(), kRecords); + EXPECT_EQ(emitted, static_cast(kRecords)); + EXPECT_EQ(session.Control().submittedSeq.load(), static_cast(kRecords)); + // The whole point: the two sides' sequence spaces are identical after ten wraps' worth of + // fillers. A side that counted a kRecPad would land here off by the number of wraps. + EXPECT_EQ(session.Control().appliedSeq.load(), static_cast(kRecords)); + EXPECT_EQ(session.consumer.AppliedSeq(), static_cast(kRecords)); + EXPECT_TRUE(RingCursorsValid(session.Control(), RingCursorSet::Cmd, + session.serverSegments.CmdRingCapacity())); +} + +// --------------------------------------------------------------------------- +// One case per watermark rule (R-9), against the writers rather than the rules +// --------------------------------------------------------------------------- + +// submittedSeq: advanced by the PRODUCER after it publishes, and NOBODY WAITS ON IT. The order +// inside PublishAndNotify is publish -> watermark -> ring, and never any other: the doorbell's +// fence only orders what precedes it, so ringing first reopens the lost-wakeup window. +TEST(SessionTest, SubmittedSeqIsThePublishersAndIsPurelyDiagnostic) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + + for (std::uint64_t seq = 1; seq <= 4; ++seq) { + void* payload = session.cmdProducer.Reserve(1, kRecNone, sizeof(std::uint64_t)); + ASSERT_NE(payload, nullptr); + std::memcpy(payload, &seq, sizeof(seq)); + session.producer.PublishAndNotify(seq); + EXPECT_EQ(session.Control().submittedSeq.load(), seq); + // It says nothing about what has been APPLIED, which is the distinction a waiter that + // picked the wrong watermark would lose. + EXPECT_EQ(session.Control().appliedSeq.load(), 0u); + } +} + +// appliedSeq: advanced by the CONSUMER for EVERY SINGLE RECORD. P5 forbids the sixty-four +// record batching this ring was designed for, because the verb barrier and every reply wait +// read it - a batched watermark makes a waiter block on work that already ran or, far worse, +// resume on work that has not. Checked after every record, not at the end. +TEST(SessionTest, AppliedSeqAdvancesExactlyOncePerRecordAndIsNeverBatched) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + + constexpr std::uint64_t kRecords = 12; + for (std::uint64_t seq = 1; seq <= kRecords; ++seq) { + void* payload = session.cmdProducer.Reserve(1, kRecNone, sizeof(std::uint64_t)); + ASSERT_NE(payload, nullptr); + std::memcpy(payload, &seq, sizeof(seq)); + } + session.producer.PublishAndNotify(kRecords); + + std::uint64_t applied = 0; + while (session.consumer.ApplyOne([&](const RingRecordView&) { ++applied; })) { + EXPECT_EQ(session.Control().appliedSeq.load(), applied) + << "appliedSeq did not move with the record; a barrier waiter would be blocked on " + "work that already ran"; + EXPECT_EQ(session.consumer.AppliedSeq(), applied); + } + EXPECT_EQ(applied, kRecords); +} + +// retiredSeq: advanced once the SEG_STAGE bytes a record referenced are finished with, and the +// staging allocator reclaims behind it. Late is merely slow; EARLY hands live bytes back to the +// producer, so the advance clamps to appliedSeq rather than believing its caller. +TEST(SessionTest, RetiredSeqMayTrailTheApplyButCanNeverOvertakeIt) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + + for (std::uint64_t seq = 1; seq <= 3; ++seq) { + ASSERT_NE(session.cmdProducer.Reserve(1, kRecNone, 8), nullptr); + } + session.producer.PublishAndNotify(3); + while (session.consumer.ApplyOne([](const RingRecordView&) {})) { + } + ASSERT_EQ(session.Control().appliedSeq.load(), 3u); + + // Trailing is legal and is what "late" means. + Watermark::AdvanceRetired(session.Control(), 1); + EXPECT_EQ(session.Control().retiredSeq.load(), 1u); + // Running ahead is not: clamped to what has actually been applied. + Watermark::AdvanceRetired(session.Control(), 99); + EXPECT_EQ(session.Control().retiredSeq.load(), 3u); + // And it never goes backwards, because a waiter that already resumed on the higher value + // cannot be un-resumed. + Watermark::AdvanceRetired(session.Control(), 2); + EXPECT_EQ(session.Control().retiredSeq.load(), 3u); +} + +// completedFrameSerial: the SERVER's, advanced when a present completes. It trails appliedSeq +// by the GPU's own depth and must never be conflated with it - recycling and ageing wait on +// this one and would free a resource the GPU is still reading if they waited on the other. +TEST(SessionTest, CompletedFrameSerialIsTheServersAndIsIndependentOfAppliedSeq) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + + for (std::uint64_t seq = 1; seq <= 5; ++seq) { + ASSERT_NE(session.cmdProducer.Reserve(1, kRecNone, 8), nullptr); + } + session.producer.PublishAndNotify(5); + while (session.consumer.ApplyOne([](const RingRecordView&) {})) { + } + EXPECT_EQ(session.Control().appliedSeq.load(), 5u); + // Five records applied, no frame completed: the two are not the same number and nothing + // may derive one from the other. + EXPECT_EQ(session.Control().completedFrameSerial.load(), 0u); + + Watermark::AdvanceCompletedFrame(session.Control(), 2); + EXPECT_EQ(session.Control().completedFrameSerial.load(), 2u); + Watermark::AdvanceCompletedFrame(session.Control(), 1); + EXPECT_EQ(session.Control().completedFrameSerial.load(), 2u) << "a watermark went backwards"; +} + +// presentAckSerial: the only back-pressure that bounds LATENCY rather than bytes. A client +// throttled on it parks, and the server's advance plus the reverse doorbell is what releases +// it - which is the half of the doorbell design that exists so a client wait is not a +// cross-process spin on one shared cache line for a whole frame of a big core. +TEST(SessionTest, PresentAckSerialIsWaitedOnWithGreaterOrEqualAndWakesThroughTheReverseBell) { + auto session = std::make_shared(); + ASSERT_TRUE(session->Build(TestSizes())); + + std::atomic released{false}; + std::atomic result{SessionWait::TimedOut}; + std::thread throttled([session, &released, &result] { + result.store(session->producer.WaitForPresentAck(4, 5000)); + released.store(true, std::memory_order_release); + }); + + // Let it get past the spin and announce itself parked, so the wakeup really travels. + while (session->Control().producerParked.load() == 0 && !released.load()) { + std::this_thread::yield(); + } + // The server jumps STRAIGHT PAST the value the waiter asked for. An equality waiter would + // still be asleep here; the >= waiter this contract mandates is released. + Watermark::AdvancePresentAck(session->Control(), 7); + session->consumer.NotifyClient(); + + throttled.join(); + EXPECT_TRUE(released.load()); + EXPECT_EQ(result.load(), SessionWait::Reached); + EXPECT_EQ(session->Control().presentAckSerial.load(), 7u); + EXPECT_EQ(session->Control().producerParked.load(), 0u); +} + +// --------------------------------------------------------------------------- +// kRecPad (R-9's last sentence, and the one with no other detector) +// --------------------------------------------------------------------------- + +// A wrap filler is FRAMING, not a record: no opcode, no payload meaning, no reply slot. If one +// side counts it and the other does not, the two sequence spaces drift by one per wrap, for +// ever - and because seq IS the reply-slot id (R-3), a drifted seq silently reads ANOTHER +// CALL'S ANSWER rather than failing. Nothing on this ring checksums that. +// +// Here the ring is driven right across the wrap boundary with a record size that cannot divide +// it, so fillers are certain; the session's own appliedSeq must count the records and not them. +TEST(SessionTest, AWrapFillerDoesNotAdvanceTheSessionsAppliedSeq) { + SessionSegmentSizes sizes = TestSizes(); + sizes.CmdBytes = 8192; // -> a 4 KiB ring, so a handful of records wraps it + SessionFixture session; + ASSERT_TRUE(session.Build(sizes)); + ASSERT_EQ(session.serverSegments.CmdRingCapacity(), 4096u); + + // 104 bytes + the 8-byte header = 112, and 4096 / 112 is not an integer, so the boundary + // falls inside a record and the producer must emit a filler on every lap. + constexpr std::uint64_t kPayload = 104; + constexpr std::uint64_t kRecords = 200; // ~5 laps + std::uint64_t emitted = 0; + std::uint64_t applied = 0; + std::uint64_t fillerBytes = 0; + std::uint64_t headBefore = 0; + + while (emitted < kRecords) { + headBefore = session.cmdProducer.LocalHead(); + void* payload = session.cmdProducer.Reserve(1, kRecNone, kPayload); + if (payload == nullptr) { + // Drain and try again; no doorbell needed, this is one thread. + session.producer.PublishAndNotify(emitted); + while (session.consumer.ApplyOne([&](const RingRecordView& view) { + // A filler must NEVER reach the thing that is about to number it. + EXPECT_EQ(view.flags & kRecPad, 0u) << "a wrap filler reached the record counter"; + EXPECT_NE(view.kind, kRingPadRecordKind); + ++applied; + })) { + } + session.consumer.RetireThrough(session.consumer.AppliedSeq()); + continue; + } + std::memset(payload, static_cast(emitted & 0xFF), static_cast(kPayload)); + const std::uint64_t grew = session.cmdProducer.LocalHead() - headBefore; + if (grew > kPayload + sizeof(RingRecordHeader)) { + fillerBytes += grew - (kPayload + sizeof(RingRecordHeader)); + } + ++emitted; + } + session.producer.PublishAndNotify(emitted); + while (session.consumer.ApplyOne([&](const RingRecordView& view) { + EXPECT_EQ(view.flags & kRecPad, 0u) << "a wrap filler reached the record counter"; + ++applied; + })) { + } + + EXPECT_GT(fillerBytes, 0u) << "the ring never wrapped, so this case proved nothing"; + EXPECT_EQ(applied, emitted); + // The session's watermark - the number the barrier's waiter and every reply read use - has + // to be the record count, with the fillers' bytes invisible to it. + EXPECT_EQ(session.Control().appliedSeq.load(), emitted); +} + +// --------------------------------------------------------------------------- +// SEG_REPLY: the slot pool +// --------------------------------------------------------------------------- + +TEST(SessionTest, AReplyIsAddressedBySeqAndCarriesItsSeqBackForSelfCheck) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + ASSERT_EQ(session.replies.SlotCount(), 8u); + EXPECT_EQ(session.replies.SlotBytes(), 64ull * 1024 / 8); + EXPECT_EQ(session.replies.MaxReplyBytes(), + session.replies.SlotBytes() - sizeof(ReplySlotHeader)); + + const std::uint32_t pixels[4] = {1, 2, 3, 4}; + session.replies.Post(5, kReplyStatusOk, pixels, sizeof(pixels)); + + std::uint32_t out[4] = {}; + std::int32_t status = -1; + std::uint64_t size = 0; + ASSERT_TRUE(session.replies.Read(5, out, sizeof(out), &status, &size)); + EXPECT_EQ(status, kReplyStatusOk); + EXPECT_EQ(size, sizeof(pixels)); + EXPECT_EQ(std::memcmp(out, pixels, sizeof(pixels)), 0); + + // The stamp is the self-check. Seq 13 addresses the SAME slot (13 % 8 == 5), and reading it + // as seq 13 must FAIL rather than hand back seq 5's answer - which is exactly what a + // sequence space drifted by a counted kRecPad would do. + EXPECT_FALSE(session.replies.Read(13, out, sizeof(out), &status, &size)); +} + +// DECLINED IS A REAL ANSWER, not a failure: it is how MapPersistent says nullptr (R-6) and how +// the four Bool acceptance entry points say false (R-5). A client that folds it into "error" +// re-creates ID-39's 66 lost uploads from the other side. +TEST(SessionTest, DeclinedIsARealAnswerWithNoPayload) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + + session.replies.Post(1, kReplyStatusDeclined, nullptr, 0); + std::int32_t status = -1; + std::uint64_t size = 99; + EXPECT_TRUE(session.replies.Read(1, nullptr, 0, &status, &size)); + EXPECT_EQ(status, kReplyStatusDeclined); + EXPECT_EQ(size, 0u); + + session.replies.Post(2, kReplyStatusError, nullptr, 0); + EXPECT_TRUE(session.replies.Read(2, nullptr, 0, &status, &size)); + EXPECT_EQ(status, kReplyStatusError); + // Seq 0 is "no record" and can never name a slot: seq is 1-based (R-3). + EXPECT_FALSE(session.replies.Read(0, nullptr, 0, &status, &size)); +} + +#if defined(GTEST_HAS_DEATH_TEST) && GTEST_HAS_DEATH_TEST +// A reply larger than a slot is FATAL, not chunked and not truncated: P5's only large answer is +// a blocking ReadPixels whose size the client knows before it emits, so an overflow means the +// two sides disagree about the frame. A gate that cannot go red is not a gate. +TEST(SessionTestDeath, AReplyLargerThanItsSlotIsFatalRatherThanTruncated) { + SessionSegments segments; + ASSERT_EQ(segments.Create(TestSizes(), MemoryRole::Server), MOBILEGL_OK); + ReplySlotPool pool(segments.ReplyBase(), segments.ReplyBytes(), segments.ReplySlotCount()); + ASSERT_TRUE(pool.Valid()); + std::vector oversize(pool.SlotBytes() + 1, 0xAB); + EXPECT_DEATH(pool.Post(1, kReplyStatusOk, oversize.data(), oversize.size()), ""); +} +#endif + +// --------------------------------------------------------------------------- +// SEG_EVENT: the reverse channel +// --------------------------------------------------------------------------- + +// P5 owes exactly this: the event ring can CARRY the three callbacks the reduced path needs. +// The overflow policy is P9's, so what is pinned here is the mechanism - a full ring latches +// eventRingFull and a dropped lossy event counts in eventDropped - and not a decision between +// them. +TEST(SessionTest, TheEventRingCarriesTheThreeReverseCallbacks) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + + // OnBufferWriteback: the bytes ride INSIDE the record, and the blobref the client hands the + // frontend names SEG_EVENT plus the in-segment offset of those bytes - never a host + // pointer (R-2's rule B). + const std::uint8_t written[8] = {9, 8, 7, 6, 5, 4, 3, 2}; + { + void* slot = session.eventOut.Reserve(kEventBufferWriteback, + sizeof(EventBufferWritebackHead) + sizeof(written)); + ASSERT_NE(slot, nullptr); + EventBufferWritebackHead head{}; + head.Resource = EventHandle{7, 1}; + head.Offset = 64; + head.Size = sizeof(written); + std::memcpy(slot, &head, sizeof(head)); + std::memcpy(static_cast(slot) + sizeof(head), written, sizeof(written)); + } + // OnGpuWritten: a count and a tail of ranges. + { + void* slot = session.eventOut.Reserve(kEventGpuWritten, + sizeof(EventGpuWrittenHead) + 2 * sizeof(EventRange)); + ASSERT_NE(slot, nullptr); + EventGpuWrittenHead head{}; + head.Resource = EventHandle{9, 2}; + head.RangeCount = 2; + std::memcpy(slot, &head, sizeof(head)); + const EventRange ranges[2] = {{0, 16}, {128, 32}}; + std::memcpy(static_cast(slot) + sizeof(head), ranges, sizeof(ranges)); + } + // OnSurfaceChanged: a fixed head, the MGPSurfaceInfo image. + { + void* slot = session.eventOut.Reserve(kEventSurfaceChanged, sizeof(EventSurfaceChangedHead)); + ASSERT_NE(slot, nullptr); + EventSurfaceChangedHead head{}; + head.Width = 1280; + head.Height = 720; + head.IsDefault = 1; + std::memcpy(slot, &head, sizeof(head)); + } + session.eventOut.PublishAndNotify(session.clientTransport->SelfDoorbell(), + session.Control().producerParked); + + RingRecordView view{}; + ASSERT_TRUE(session.eventIn.Pop(view)); + EXPECT_EQ(view.kind, kEventBufferWriteback); + EventBufferWritebackHead writeback{}; + std::memcpy(&writeback, view.payload, sizeof(writeback)); + EXPECT_EQ(writeback.Resource.Slot, 7u); + EXPECT_EQ(writeback.Size, sizeof(written)); + const auto* inlineBytes = static_cast(view.payload) + sizeof(writeback); + EXPECT_EQ(std::memcmp(inlineBytes, written, sizeof(written)), 0); + // The offset a blobref would carry: inside SEG_EVENT and past its control page, never a + // host address. + const std::uint64_t offset = session.eventIn.OffsetInSegment(inlineBytes); + EXPECT_GE(offset, sizeof(RingControl)); + EXPECT_LT(offset, session.clientSegments.AnnouncedSize(SessionSegmentSlot::Event)); + + ASSERT_TRUE(session.eventIn.Pop(view)); + EXPECT_EQ(view.kind, kEventGpuWritten); + EventGpuWrittenHead gpuWritten{}; + std::memcpy(&gpuWritten, view.payload, sizeof(gpuWritten)); + EXPECT_EQ(gpuWritten.RangeCount, 2u); + + ASSERT_TRUE(session.eventIn.Pop(view)); + EXPECT_EQ(view.kind, kEventSurfaceChanged); + EventSurfaceChangedHead surface{}; + std::memcpy(&surface, view.payload, sizeof(surface)); + EXPECT_EQ(surface.Width, 1280u); + EXPECT_EQ(surface.IsDefault, 1u); + + EXPECT_FALSE(session.eventIn.Pop(view)); + session.eventIn.Drained(); + EXPECT_FALSE(session.eventIn.RingIsFull()); + EXPECT_EQ(session.eventIn.DroppedEvents(), 0u); +} + +TEST(SessionTest, AFullEventRingLatchesTheFlagRatherThanDecidingWhatToDoAboutIt) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + + // Fill it. Reserve refuses at half the ring per record, so this terminates. + const std::uint64_t capacity = session.serverSegments.EventRingCapacity(); + std::uint64_t posted = 0; + while (session.eventOut.Reserve(kEventGpuWritten, 256) != nullptr) { + ++posted; + ASSERT_LT(posted, capacity); // a producer that never fills is a broken case + } + EXPECT_TRUE(session.eventIn.RingIsFull()); + + session.eventOut.CountDrop(); + EXPECT_EQ(session.eventIn.DroppedEvents(), 1u); + + session.eventOut.PublishAndNotify(session.clientTransport->SelfDoorbell(), + session.Control().producerParked); + RingRecordView view{}; + std::uint64_t drained = 0; + while (session.eventIn.Pop(view)) { + ++drained; + } + EXPECT_EQ(drained, posted); + session.eventIn.Drained(); + EXPECT_FALSE(session.eventIn.RingIsFull()); +} + +// --------------------------------------------------------------------------- +// Shutdown +// --------------------------------------------------------------------------- + +// The design's own steady state: the apply thread spun, set consumerParked and blocked with NO +// DEADLINE. Only CondVarDoorbell::Kill() can bring it back - a single Notify is consumed by one +// Park, after which Doorbell::Wait re-tests a condition nothing published, finds the bell alive +// and parks again, forever. InProcessChannel::Close kills both bells, and SessionConsumer turns +// `Wait == false && Dead()` into SessionWait::ShutDown. +// +// A REGRESSION HERE IS A HANG, so the join is bounded at five seconds and the waiter is +// detached on timeout: the test goes red instead of wedging the CI job. That is +// InProcessTransportTest.cpp:344's shape, and it is copied on purpose. +TEST(SessionTest, ShutdownUnparksTheApplyThreadAndTheJoinIsBounded) { + struct Shared { + SessionFixture session; + std::atomic returned{false}; + std::atomic verdict{SessionWait::Reached}; + std::atomic started{false}; + }; + auto shared = std::make_shared(); + ASSERT_TRUE(shared->session.Build(TestSizes())); + + std::thread apply([shared] { + shared->started.store(true, std::memory_order_release); + // kWaitForever, exactly as the real apply loop parks. + shared->verdict.store(shared->session.consumer.WaitForWork(kWaitForever)); + shared->returned.store(true, std::memory_order_release); + }); + + while (shared->session.Control().consumerParked.load() == 0 && + !shared->returned.load(std::memory_order_acquire)) { + std::this_thread::yield(); + } + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + ASSERT_FALSE(shared->returned.load(std::memory_order_acquire)) + << "the apply thread returned before anything shut the session down"; + + shared->session.clientTransport->Shutdown(); + + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (!shared->returned.load(std::memory_order_acquire) && + std::chrono::steady_clock::now() < deadline) { + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } + if (!shared->returned.load(std::memory_order_acquire)) { + apply.detach(); + FAIL() << "Shutdown did not unpark the apply thread within 5 s: without a doorbell death " + "state a waiter consumes the ring and parks again, and teardown can never join"; + } + apply.join(); + + EXPECT_EQ(shared->verdict.load(), SessionWait::ShutDown); + EXPECT_TRUE(shared->session.serverTransport->SelfDoorbell().Dead()); + EXPECT_EQ(shared->session.Control().consumerParked.load(), 0u); + + // Sticky: a wait that ARRIVES after the shutdown returns at once rather than parking, so a + // late thread cannot hang either. + const auto start = std::chrono::steady_clock::now(); + EXPECT_EQ(shared->session.consumer.WaitForWork(kWaitForever), SessionWait::ShutDown); + EXPECT_LT(std::chrono::duration_cast( + std::chrono::steady_clock::now() - start) + .count(), + 1000); + // And so does a producer blocked in the verb barrier: without this, a client waiting for + // appliedSeq when the server died would sit in the barrier for ever. + EXPECT_EQ(shared->session.producer.WaitForApplied(1, kWaitForever), SessionWait::ShutDown); +} + +// --------------------------------------------------------------------------- +// The ABI fingerprint's mixer +// --------------------------------------------------------------------------- + +// A fingerprint that cannot be SHOWN to change is indistinguishable from one that is never +// compared, which is why the mixer takes its sizes as arguments instead of reading sizeof +// directly: a test can vary one byte and prove the answer moves. +TEST(SessionTest, TheAbiFingerprintChangesWhenAnyOfItsInputsDoes) { + const std::uint64_t base = MixAbiFingerprint(1024, 1080, 552, 0x00010000, "abc1234"); + EXPECT_NE(base, 0u) << "0 is reserved for \"not stated\""; + EXPECT_EQ(base, MixAbiFingerprint(1024, 1080, 552, 0x00010000, "abc1234")); + + EXPECT_NE(base, MixAbiFingerprint(1025, 1080, 552, 0x00010000, "abc1234")); + EXPECT_NE(base, MixAbiFingerprint(1024, 1081, 552, 0x00010000, "abc1234")); + EXPECT_NE(base, MixAbiFingerprint(1024, 1080, 553, 0x00010000, "abc1234")); + EXPECT_NE(base, MixAbiFingerprint(1024, 1080, 552, 0x00010001, "abc1234")); + EXPECT_NE(base, MixAbiFingerprint(1024, 1080, 552, 0x00010000, "abc1235")); + // A missing stamp is not the same as an empty one, and neither is the same as a real build. + EXPECT_NE(MixAbiFingerprint(1024, 1080, 552, 0x00010000, nullptr), + MixAbiFingerprint(1024, 1080, 552, 0x00010000, "abc1234")); +} From c5c12b489aed6ba325537e341b95c129813297da Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 11 Sep 2026 15:04:54 -0400 Subject: [PATCH 05/10] [Fix] (MG_Remote, Server): delete CallMask's two derived defaults and make an unset mask Fatal{UnsetCallMask} - the consumer default read MGPipeGetResourceOps, a process-wide global, so under inproc the server answered R-8's client-side gate with the client's own registration and under spawn it collapsed to P2's mask, dropping five P4a families with the dirty flags cleared anyway and the lane green --- MobileGL/MG_Remote/Server/ServerSession.cpp | 117 ++++++++++++-------- MobileGL/MG_Remote/Server/ServerSession.h | 51 ++++++--- 2 files changed, 109 insertions(+), 59 deletions(-) diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index 500def56..cfbe95aa 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/MobileGL/MG_Remote/Server/ServerSession.cpp @@ -16,8 +16,6 @@ #include #include -#include -#include #include #include @@ -83,13 +81,23 @@ namespace MobileGL::MG_Remote::Server { std::abort(); } + // MOBILEGL_IPC_RING_MB / MOBILEGL_IPC_STAGE_MB, actually applied. + // + // The first version of this file called this only `if (m_sizes.CmdBytes == 0)`, and + // SessionSegmentSizes has a default member initialiser of 8 MiB, so the condition was + // never true and the whole function was dead: ConfigLoader parsed both knobs, echoed + // them into the config line, and the session mapped 8/32 MiB regardless. Config.h's + // own comment four lines above the declaration is the statement of that bug - "an + // environment variable that nothing parses is indistinguishable from one that is + // parsed and ignored" - and a knob that REPORTS a value it does not use is worse, + // because it makes every measurement taken with it a lie. Transport::SessionSegmentSizes SizesFromConfig() { Transport::SessionSegmentSizes sizes; #if MOBILEGL_BUILD_DISAGGREGATED const Uint64 ringMb = MG_Config::Ipc.RingMb == 0 ? 8u : MG_Config::Ipc.RingMb; const Uint64 stageMb = MG_Config::Ipc.StageMb == 0 ? 32u : MG_Config::Ipc.StageMb; - sizes.CmdBytes = ringMb * 1024ull * 1024ull; - sizes.StageBytes = stageMb * 1024ull * 1024ull; + sizes.CmdRingBytes = ringMb * 1024ull * 1024ull; + sizes.StageRingBytes = stageMb * 1024ull * 1024ull; #endif return sizes; } @@ -107,36 +115,21 @@ namespace MobileGL::MG_Remote::Server { // reason InProcessTransportTest.cpp:344 bounds its join at five seconds. constexpr Uint32 kHandshakeTimeoutMs = 5000; - // WHICH SUBSYSTEMS THIS SERVER CONSUMES, derived from the only registration the tree - // actually has. PipeFill.cpp:920's P4aFamilyHasItsConsumer is the evidence: every P4a - // family hangs off MGPipeGetResourceOps() and there is deliberately "no per-family - // registration to add". Below that, P2's bits 0..6 are consumed by the APPLIER, which - // a server role has by construction. - // - // FLAGGED FOR v1: this is a default, not a ruling. v1 owns the apply thread and knows - // what its backend actually took over, and SetConsumedSubsystems is how it says so. - // Publishing a bit the server does not consume is ID-39's shape from the other side - - // the client emits and nothing applies - so a wrong answer here is not benign. - Uint64 DeriveConsumedSubsystems() { - if (MG_Pipe::MGPipeGetResourceOps() == nullptr) { - return MG_Pipe::kMGPipeSubsystemsMigratedAtP2; - } - return MG_Pipe::kMGPipeSubsystemsMigratedAtP4a; - } - - // The ONE MGPCapBit that is derivable from the server's own registration. Every other - // bit belongs to the package that owns the question it answers; 0 is the honest - // default for those, because a cap bit set on a guess is a capability probe that - // answers "supported" for a path that does not exist (R-15's cross-cutting rule). - Uint64 DeriveCapabilityBits() { - Uint64 bits = 0; - if (MG_Pipe::MGPipeResourceOpsHaveSubDataResident()) { - bits |= static_cast(MG_Pipe::kCapResidentSubData); - } - // kCapNeedsHostIndexBytes and kCapNeedsHostUboBytes are 0 for the whole of P5 by - // ruling, and that is the cheapest way to keep every MGHostSpan out of the first - // IPC frame: they are the only two things that ask for one. - return bits; + // THERE IS NO DERIVATION OF CallMask, BY RULING. See ServerSession.h's block on + // SetCapabilityBits / SetConsumedSubsystems for why the one that used to be here was + // the phase's marquee defect committed from the server's side. + [[noreturn]] void FatalUnsetCallMask(Bool capBitsSet, Bool consumedSet) { + MGLOG_F("MGPipe: Fatal{UnsetCallMask} - %s%s%s was never set on this ServerSession, " + "and there is no default: a guessed consumer mask makes the client's R-8 " + "liveness gates answer from a server-side fact the server never stated. The " + "client would stop emitting whole record families, clear its dirty flags on " + "acceptance anyway, and the lane would go green with the uploads lost " + "(ID-39, reflected). Call SetConsumedSubsystems() and SetCapabilityBits() " + "before Accept(); SetCapabilityBits(0) is a legitimate explicit answer", + capBitsSet ? "" : "SetCapabilityBits", + (!capBitsSet && !consumedSet) ? " and " : "", + consumedSet ? "" : "SetConsumedSubsystems"); + std::abort(); } ServerSession* g_active = nullptr; @@ -162,6 +155,7 @@ namespace MobileGL::MG_Remote::Server { return; } m_sizes = sizes; + m_sizesSet = true; } void ServerSession::SetBackend(MG_Backend::BackendObject* backend) { m_backend = backend; } @@ -176,10 +170,13 @@ namespace MobileGL::MG_Remote::Server { m_consumedSet = true; } + Bool ServerSession::CallMaskIsSet() const { return m_capBitsSet && m_consumedSet; } + Uint64 ServerSession::CallMask() const { - const Uint64 capBits = m_capBitsSet ? m_capBits : DeriveCapabilityBits(); - const Uint64 consumed = m_consumedSet ? m_consumedSubsystems : DeriveConsumedSubsystems(); - return capBits | MGCapsConsumerBits(consumed); + if (!CallMaskIsSet()) { + FatalUnsetCallMask(m_capBitsSet, m_consumedSet); + } + return m_capBits | MGCapsConsumerBits(m_consumedSubsystems); } Bool ServerSession::Accepted() const { return m_accepted; } @@ -189,7 +186,9 @@ namespace MobileGL::MG_Remote::Server { return MOBILEGL_ERR_INVALID_ARGUMENT; } m_transport = &transport; - if (m_sizes.CmdBytes == 0) { + // MOBILEGL_IPC_RING_MB / _STAGE_MB unless SetSegmentSizes overrode them. Unconditional + // on purpose - see SizesFromConfig. + if (!m_sizesSet) { m_sizes = SizesFromConfig(); } @@ -203,7 +202,13 @@ namespace MobileGL::MG_Remote::Server { return received; } const ::MobileGL::Wire::CtrlEnvelope* envelope = ParseEnvelope(frame); - if (envelope == nullptr || envelope->msg_type() != ::MobileGL::Wire::CtrlMsg::Hello) { + // msg_as_Hello() IS PART OF THE GUARD, not a consequence of it. FlatBuffers' + // Verifier::VerifyTable is `return !table || table->Verify(*this)`, so a NULL union + // member passes verification: a 24-byte frame verifies, carries the identifier, + // reports msg_type() == Hello and returns nullptr from msg_as_Hello(). A malformed + // frame has to be refused, never dereferenced. + if (envelope == nullptr || envelope->msg_type() != ::MobileGL::Wire::CtrlMsg::Hello || + envelope->msg_as_Hello() == nullptr) { MGLOG_E("MG_Remote server: the first control frame is not a verifiable Hello"); return MOBILEGL_ERR_PROTOCOL_MISMATCH; } @@ -235,7 +240,7 @@ namespace MobileGL::MG_Remote::Server { m_commands = Transport::RingConsumer(control, m_shm.CmdRingBase(), m_shm.CmdRingCapacity(), Transport::RingCursorSet::Cmd); if (!m_commands.Valid()) { - m_shm.Close(); + Close(); return MOBILEGL_ERR_INVALID_ARGUMENT; } m_consumer.Attach(control, &m_commands, &ProducerDoorbell(), &ConsumerDoorbell(), @@ -245,7 +250,7 @@ namespace MobileGL::MG_Remote::Server { Transport::ReplySlotPool pool(m_shm.ReplyBase(), m_shm.ReplyBytes(), m_shm.ReplySlotCount()); if (!pool.Valid()) { - m_shm.Close(); + Close(); return MOBILEGL_ERR_INVALID_ARGUMENT; } // A stale stamp from a previous session must never read as this session's answer. @@ -280,7 +285,7 @@ namespace MobileGL::MG_Remote::Server { ::MobileGL::Wire::FinishCtrlEnvelopeBuffer(builder, root); const MobileGLResult sent = SendEnvelope(transport, builder); if (sent != MOBILEGL_OK) { - m_shm.Close(); + Close(); return sent; } } @@ -311,6 +316,16 @@ namespace MobileGL::MG_Remote::Server { g_active = this; LogMemory("accept"); + if (!CallMaskIsSet()) { + // Not fatal HERE, because a session with no backend legitimately publishes no + // snapshot at all and the mask is only needed by one. It becomes + // Fatal{UnsetCallMask} the moment PublishCapsSnapshot asks for it, which is the + // first thing that would put a guess on the wire. + MGLOG_W("MG_Remote server: accepted with no CallMask - SetConsumedSubsystems() and/or " + "SetCapabilityBits() were never called. There is no default and there will be " + "no guess: the first CapsSnapshot will abort instead"); + } + // ---- 6. the first CapsSnapshot, if there is a backend to take it from. if (m_backend != nullptr) { const MobileGLResult published = PublishCapsSnapshot(); @@ -443,20 +458,30 @@ namespace MobileGL::MG_Remote::Server { Transport::EventRingProducer& ServerSession::Events() { return m_events; } Transport::ITransport* ServerSession::Control_Plane() { return m_transport; } + void ServerSession::PublishEvents() { + if (!m_accepted) { + return; + } + // Publish, THEN ring - the same order as the forward direction, and the session picks + // the bell so that no caller can pair the right ring with the wrong flag. + m_events.Ring().Publish(); + m_consumer.NotifyClient(); + } + + // Both of these advance AND ring, through SessionConsumer. The free functions in namespace + // Watermark do not ring: a client parked in WaitForPresentAck(kWaitForever) needs the pair. void ServerSession::AdvanceCompletedFrame(Uint64 serial) { if (!m_accepted) { return; } - Transport::Watermark::AdvanceCompletedFrame(Control(), serial); - m_consumer.NotifyClient(); + m_consumer.CompleteFrame(serial); } void ServerSession::ReturnPresentCredit(Uint64 serial) { if (!m_accepted) { return; } - Transport::Watermark::AdvancePresentAck(Control(), serial); - m_consumer.NotifyClient(); + m_consumer.ReturnPresentCredit(serial); } Transport::RoleMemorySample ServerSession::SampleMemory() const { diff --git a/MobileGL/MG_Remote/Server/ServerSession.h b/MobileGL/MG_Remote/Server/ServerSession.h index 38b6073b..9186eced 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.h +++ b/MobileGL/MG_Remote/Server/ServerSession.h @@ -99,26 +99,45 @@ namespace MobileGL::MG_Remote::Server { // PublishCapsSnapshot yourself. void SetBackend(MG_Backend::BackendObject* backend); - // MGPCaps::CallMask's two halves, kept apart because they have different owners. + // MGPCaps::CallMask's two halves. BOTH ARE MANDATORY AND NEITHER HAS A DEFAULT. // - // BITS 0..8, THE MGPCapBit FEATURE BITS: nothing in the tree produces them today - - // CallMask is declared at MGPipeTypes.h:133 and written by nobody - and what each one - // answers belongs to the package that owns the question (kCapTimerQuery to the query - // family, kCapResidentSubData to b1, and so on). The default below derives only the - // ONE bit that is mechanically derivable from the server's own registration, and - // kCapNeedsHostIndexBytes / kCapNeedsHostUboBytes stay 0 for the whole of P5 by - // ruling (CONTRACT-P5 table 0), which is what keeps every MGHostSpan out of the first - // IPC frame. Everything else is an owner's to set here. + // 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 must stay 0 for the whole of + // P5 by ruling (CONTRACT-P5 table 0): they are the only two things that ask for an + // MGHostSpan, and 0 is what keeps every one of them out of the first IPC frame. void SetCapabilityBits(Uint64 capBits); // BITS 32..47, THE CONSUMER MASK (R-8 / C-4): which MGPipe subsystems this server has - // a consumer for. The client's liveness gates read it back through - // CapsMirror::ServerConsumes and may NEVER read MGPipeGetResourceOps() - that is the - // server's registration, which under inproc a client reads correctly by accident and - // under spawn reads as null, silently disabling five whole record families. + // a consumer for. v1 owns the answer - it owns the apply thread and knows what its + // backend took over. Publishing a bit the server does not consume is the failure + // above; withholding one the server does consume merely leaves the legacy pull path + // running, which is the safe direction. void SetConsumedSubsystems(Uint64 subsystemMask); + // False until BOTH setters have been called. A caller that can handle the absence + // asks this; PublishCapsSnapshot and CallMask abort on it. + Bool CallMaskIsSet() const; + // What PublishCapsSnapshot puts on the wire: capBits | MGCapsConsumerBits(subsystems). + // Fatal{UnsetCallMask} if either half was never set. Uint64 CallMask() const; Bool Accepted() const; @@ -134,6 +153,11 @@ namespace MobileGL::MG_Remote::Server { // The reverse channel. P5 only has to be able to CARRY OnBufferWriteback / // OnGpuWritten / OnSurfaceChanged; the overflow policy is P9's. Transport::EventRingProducer& Events(); + // Publish everything reserved on SEG_EVENT and ring the client. USE THIS rather than + // EventRingProducer::PublishAndNotify, which takes a bell and a park flag from its + // caller and therefore compiles for every wrong pairing; the session is the thing + // that knows which bell belongs to the client. + void PublishEvents(); Transport::ITransport* Control_Plane(); // completedFrameSerial / presentAckSerial: the two watermarks only the server can @@ -162,6 +186,7 @@ namespace MobileGL::MG_Remote::Server { Uint64 m_consumedSubsystems = 0; Bool m_capBitsSet = false; Bool m_consumedSet = false; + Bool m_sizesSet = false; Bool m_accepted = false; }; From ed60c06b9841da1c54fd2314f444bf8e3e543bd2 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 11 Sep 2026 15:04:54 -0400 Subject: [PATCH 06/10] [Fix] (MG_Remote, Protocol): mark CapsSnapshot's four retired fields deprecated instead of deleting them - a deletion FREES the vtable slot, so callMask took slot 14 from an [int] vector as an 8-byte ulong with no ABI-major bump and a fingerprint that mixes struct sizes rather than the schema --- .../Protocol/generated/protocol_generated.h | 4 ++-- MobileGL/MG_Remote/Protocol/protocol.fbs | 22 ++++++++++++++++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/MobileGL/MG_Remote/Protocol/generated/protocol_generated.h b/MobileGL/MG_Remote/Protocol/generated/protocol_generated.h index 8324e1be..d11f5c37 100644 --- a/MobileGL/MG_Remote/Protocol/generated/protocol_generated.h +++ b/MobileGL/MG_Remote/Protocol/generated/protocol_generated.h @@ -822,8 +822,8 @@ struct CapsSnapshot FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { VT_FORMATCAPS = 8, VT_EXTENSIONS = 10, VT_APIVERSION = 12, - VT_CALLMASK = 14, - VT_BACKENDTYPE = 16 + VT_CALLMASK = 22, + VT_BACKENDTYPE = 24 }; const ::flatbuffers::Vector *dynamicParameters() const { return GetPointer *>(VT_DYNAMICPARAMETERS); diff --git a/MobileGL/MG_Remote/Protocol/protocol.fbs b/MobileGL/MG_Remote/Protocol/protocol.fbs index 0e9d52c3..04402962 100644 --- a/MobileGL/MG_Remote/Protocol/protocol.fbs +++ b/MobileGL/MG_Remote/Protocol/protocol.fbs @@ -98,8 +98,8 @@ table Welcome { // (plan B appendix A, `get_caps`). The three blobs are byte-for-byte images of // the corresponding POD structs; they are versioned by structSize-first // discipline, not by this schema. -// The four fields this table used to end with are DELETED (CONTRACT-P5 table 0, -// "CapsSnapshot redundancy"), not renamed and not deprecated: +// The four fields this table used to end with are RETIRED (CONTRACT-P5 table 0, +// "CapsSnapshot redundancy"): // // maxComputeWorkGroupCount / maxComputeWorkGroupSize - they ride inside // `dynamicParameters` already (BackendObject.h:392-393), and two spellings of @@ -112,13 +112,29 @@ table Welcome { // prefersCpuXfbPrimitiveAccounting - answered by kCapCpuXfbPrimitiveAccounting // in `callMask` below. // -// They were the LAST four fields, so nothing before them moved a vtable slot. +// THEY ARE `(deprecated)`, NOT DELETED, AND THAT IS NOT A STYLE CHOICE. In +// FlatBuffers a table field's id IS its vtable slot, and REMOVING a field FREES +// that slot for the next field appended to the table - so plainly deleting these +// four would have handed slots 14 and 16, which used to carry `[int]` vectors +// (4-byte uoffsets), to `callMask` (an 8-byte inline ulong) and `backendType` (a +// 4-byte inline uint). Two peers straddling that edit both still announce +// abiMajor 1, and the ABI fingerprint mixes struct sizes and a git stamp, not the +// schema, so neither the handshake nor the fingerprint could see it: the reader +// would parse a uoffset as a ulong. `(deprecated)` keeps 14/18/20 burned, pushes +// the two new fields to 22/24, generates no accessor for the retired names so +// nothing can read or write them, and costs zero bytes on the wire. The +// alternative - bumping MOBILEGL_PROTOCOL_ABI_MAJOR - is a real break for a +// change that does not need to be one. table CapsSnapshot { dynamicParameters: [ubyte]; rendererInfo: [ubyte]; formatCaps: [ubyte]; extensions: [string]; apiVersion: string; + maxComputeWorkGroupCount: [int] (deprecated); + maxComputeWorkGroupSize: [int] (deprecated); + tableSlotMask: ulong (deprecated); + prefersCpuXfbPrimitiveAccounting: bool (deprecated); // MGPCaps::CallMask (MGPipeTypes.h:127). Bits 0..8 are MGPCapBit; bits 32..47 // are the CONSUMER MASK - bit (32+n) means "the server has a consumer for // MGPipe subsystem bit n" - and MG_Remote/CapsCodec.h holds the four constexprs From da8f030459c72ae9a59b2323b2b5bafee165f949 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 11 Sep 2026 15:04:54 -0400 Subject: [PATCH 07/10] [Fix] (MG_Remote, Transport): the knob names the ring and the segment adds a control page, a backwards watermark is Fatal rather than a permanent hang, RetireThrough stops at a borrowed slot, the inproc peer gets a real second mapping through dup and Adopt, and the producer remembers its own published seq --- MobileGL/MG_Remote/Transport/ReplySlot.h | 27 +++- MobileGL/MG_Remote/Transport/Ring.cpp | 88 ++++++++++- MobileGL/MG_Remote/Transport/RoleMemory.h | 6 +- MobileGL/MG_Remote/Transport/SessionRings.h | 162 +++++++++++++++----- MobileGL/MG_Remote/Transport/ShmSegment.cpp | 72 ++++++++- 5 files changed, 303 insertions(+), 52 deletions(-) diff --git a/MobileGL/MG_Remote/Transport/ReplySlot.h b/MobileGL/MG_Remote/Transport/ReplySlot.h index 068dbb72..5595d6a6 100644 --- a/MobileGL/MG_Remote/Transport/ReplySlot.h +++ b/MobileGL/MG_Remote/Transport/ReplySlot.h @@ -50,6 +50,14 @@ #include #include +// P9's account, named here rather than left to be rediscovered: the seq stamp +// catches a sequence space drifted by anything that is NOT a multiple of +// slotCount. A drift of exactly 8, 16, ... lands on the same slot with a matching +// stamp and reads as this call's answer. Under R-1's verb barrier the in-flight +// depth is one and a drift cannot open at all; P9 is what removes the barrier, +// and it is what has to widen the stamp (a generation beside the seq) or bound +// the drift some other way. + namespace MobileGL::MG_Remote::Transport { // CONTRACT-P5 table 0, "reply slot header". @@ -106,8 +114,24 @@ namespace MobileGL::MG_Remote::Transport { static_cast(sizeof(ReplySlotHeader))); return; } + // EVERY SLOT MUST START 8-ALIGNED. The header is written by one thread + // and read by another; the fences below order the payload against the + // stamp, but the stamp's own 8-byte Seq has to be untorn for the + // wrong-slot self-check to mean anything, and that is only true while + // it is naturally aligned. A geometry whose slotBytes is not a + // multiple of 8 puts later slots on odd boundaries, so it is refused + // here rather than left to a future caller to discover. + if ((slotBytes % 8) != 0 || + (reinterpret_cast(base) % alignof(ReplySlotHeader)) != 0) { + WireLogError("MG_Remote reply pool: rejected, a %llu byte slot at base alignment " + "%llu would put a slot header on an unaligned address, and the seq " + "stamp the wrong-slot check reads has to be untorn", + static_cast(slotBytes), + static_cast( + reinterpret_cast(base) % alignof(ReplySlotHeader))); + return; + } m_base = static_cast(base); - m_size = sizeBytes; m_slots = slotCount; m_mask = slotCount - 1; // Truncated to 32 bits deliberately: the header's Size field is @@ -242,7 +266,6 @@ namespace MobileGL::MG_Remote::Transport { } std::uint8_t* m_base = nullptr; - std::uint64_t m_size = 0; std::uint32_t m_slots = 0; std::uint32_t m_mask = 0; std::uint32_t m_slotBytes = 0; diff --git a/MobileGL/MG_Remote/Transport/Ring.cpp b/MobileGL/MG_Remote/Transport/Ring.cpp index b54f4635..b5ff5d6f 100644 --- a/MobileGL/MG_Remote/Transport/Ring.cpp +++ b/MobileGL/MG_Remote/Transport/Ring.cpp @@ -12,6 +12,7 @@ #include +#include #include namespace MobileGL::MG_Remote::Transport { @@ -320,6 +321,14 @@ namespace MobileGL::MG_Remote::Transport { return value; } + std::uint64_t SegmentBytesForRing(std::uint64_t ringBytes) { + const std::uint64_t ring = LargestPowerOfTwoAtMost(ringBytes); + if (ring < kMinRingCapacity || ring > kMaxRingCapacity) { + return 0; + } + return ring + sizeof(RingControl); + } + std::uint64_t RingCapacityForSegment(std::uint64_t segmentBytes) { if (segmentBytes <= sizeof(RingControl)) { return 0; @@ -333,18 +342,26 @@ namespace MobileGL::MG_Remote::Transport { namespace { // A watermark may be published LATE but never EARLY, and it may never // move BACKWARDS. Backwards is the half that is mechanically - // detectable from inside, so it is refused loudly here; "early" can - // only be caught at the call site, which is why every advance below - // has exactly one caller and a named unit case. + // detectable from inside, and it is FATAL rather than logged: the + // consumer keeps its own counter, so a shared watermark left behind + // makes every later advance a no-op and every WaitForApplied on + // kWaitForever - the verb barrier and every reply wait - block for + // ever. A hang with one ERROR line in the log is strictly worse than + // an abort at the instruction that caused it, and Ring.h:181-185 + // already rules the same way for the cursor invariants on this page. + // "Early" can only be caught at the call site, which is why every + // advance below has exactly one caller and a named unit case. void AdvanceMonotonic(std::atomic& watermark, std::uint64_t to, const char* name) { const std::uint64_t current = watermark.load(std::memory_order_relaxed); if (to < current) { - MGLOG_E("MG_Remote watermark: refusing to move %s backwards, %llu -> %llu; a " - "waiter that already resumed on the higher value cannot be un-resumed", + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"watermark\"} %s moved backwards, " + "%llu -> %llu. A waiter that already resumed on the higher value cannot " + "be un-resumed, and every later advance of this watermark would be a " + "no-op, so the verb barrier and every reply wait would block for ever", name, static_cast(current), static_cast(to)); - return; + std::abort(); } if (to == current) { return; @@ -404,6 +421,7 @@ namespace MobileGL::MG_Remote::Transport { m_stage = nullptr; m_peerBell = nullptr; m_selfBell = nullptr; + m_lastPublishedSeq = 0; } void SessionProducer::PublishAndNotify(std::uint64_t submittedSeq) { @@ -415,8 +433,20 @@ namespace MobileGL::MG_Remote::Transport { if (m_stage != nullptr) { m_stage->Publish(); } + // Kept locally as well as on the shared page: the shared watermark is + // allowed to lag (Ring.h:72-77), and teardown's drain must not. + // + // Clamped UP, not passed through. A caller that republishes an older + // bound - teardown does exactly that, and so does any batched publisher + // that lost track - is publishing LATE, which R-9 permits; it is not the + // same thing as a watermark moving backwards, which is Fatal. Doing the + // clamp here keeps that distinction at the one boundary where a stale + // argument is legitimate. + if (submittedSeq > m_lastPublishedSeq) { + m_lastPublishedSeq = submittedSeq; + } // 2. the diagnostic watermark, after the bytes it describes. - Watermark::AdvanceSubmitted(*m_control, submittedSeq); + Watermark::AdvanceSubmitted(*m_control, m_lastPublishedSeq); // 3. and only now the bell. Publish-then-ring, never ring-then-publish. if (m_peerBell != nullptr) { NotifyIfParked(*m_peerBell, m_control->consumerParked); @@ -483,6 +513,8 @@ namespace MobileGL::MG_Remote::Transport { m_selfBell = selfBell; m_spinUs = spinUs; m_appliedSeq = control == nullptr ? 0 : control->appliedSeq.load(std::memory_order_acquire); + m_retirableCursor = cmd == nullptr ? 0 : cmd->LocalTail(); + m_borrowHeld = false; } void SessionConsumer::Detach() { @@ -490,6 +522,8 @@ namespace MobileGL::MG_Remote::Transport { m_cmd = nullptr; m_peerBell = nullptr; m_selfBell = nullptr; + m_retirableCursor = 0; + m_borrowHeld = false; } SessionWait SessionConsumer::WaitForWork(std::uint32_t timeoutMs) { @@ -515,7 +549,45 @@ namespace MobileGL::MG_Remote::Transport { return; } Watermark::AdvanceRetired(*m_control, seq); - m_cmd->PublishRetired(); + // PublishRetiredUpTo, NOT PublishRetired: the latter stores m_localTail + // into BOTH tails, i.e. it hands back every byte the consumer has popped + // whether or not a record among them was borrowed into the GPU timeline. + // That would defeat the whole reason the ring carries two tails + // (Ring.h:24-27) and would let the producer overwrite a slot the GPU is + // still reading. m_retirableCursor stops at the first borrowed record. + m_cmd->PublishApplied(); + m_cmd->PublishRetiredUpTo(m_retirableCursor); + NotifyClient(); + } + + void SessionConsumer::RetireBorrowedUpTo(std::uint64_t cursor) { + if (!Valid()) { + return; + } + if (cursor > m_retirableCursor) { + m_retirableCursor = cursor; + // A release that reaches everything popped so far clears the latch; + // anything still unreleased keeps it set, so a second borrow behind + // the first is not skipped. + m_borrowHeld = cursor < m_cmd->LocalTail(); + } + m_cmd->PublishRetiredUpTo(m_retirableCursor); + NotifyClient(); + } + + void SessionConsumer::CompleteFrame(std::uint64_t serial) { + if (!Valid()) { + return; + } + Watermark::AdvanceCompletedFrame(*m_control, serial); + NotifyClient(); + } + + void SessionConsumer::ReturnPresentCredit(std::uint64_t serial) { + if (!Valid()) { + return; + } + Watermark::AdvancePresentAck(*m_control, serial); NotifyClient(); } diff --git a/MobileGL/MG_Remote/Transport/RoleMemory.h b/MobileGL/MG_Remote/Transport/RoleMemory.h index fcbe30ad..4fc61061 100644 --- a/MobileGL/MG_Remote/Transport/RoleMemory.h +++ b/MobileGL/MG_Remote/Transport/RoleMemory.h @@ -77,8 +77,10 @@ namespace MobileGL::MG_Remote::Transport { RoleMemorySample SampleRoleMemory(MemoryRole role); - // Emits one line at ERROR level (the wire layer's only level - WireLog.h) so - // t1's harness can grep it out of a lane log without a new log sink. + // Emits one line at INFO level, which is what every P5 lane builds at, so + // t1's harness can grep it out of a lane log without a new log sink. Not + // DEBUG, which the INFO build compiles out; not ERROR, which this is not. + // The grep tag is `MG_Remote memory[`. // `phase` is a short tag: "handshake", "first-frame", "teardown". void LogRoleMemory(const char* phase, const RoleMemorySample& sample); diff --git a/MobileGL/MG_Remote/Transport/SessionRings.h b/MobileGL/MG_Remote/Transport/SessionRings.h index a52429e3..381bdc7b 100644 --- a/MobileGL/MG_Remote/Transport/SessionRings.h +++ b/MobileGL/MG_Remote/Transport/SessionRings.h @@ -28,27 +28,37 @@ // phase is trying not to repeat. // // --------------------------------------------------------------------------- -// THE RING CAPACITY IS HALF THE SEGMENT, AND THAT IS ARITHMETIC, NOT A CHOICE. +// THE KNOB NAMES THE RING; THE SEGMENT IS THE RING PLUS ONE CONTROL PAGE. // -// Ring.h:11-13 puts RingControl at the HEAD of SEG_CMD, and RingProducer requires -// a POWER-OF-TWO capacity (Ring.cpp:89-103, the mask is the indexing). A segment -// of 8 MiB therefore has 8 MiB - 4096 bytes left for records, and the largest -// power of two that fits is 4 MiB. A record may be at most half the ring -// (RingProducer::MaxRecordBytes), so the real cap on one record is 2 MiB. +// Ring.h:11-13 puts RingControl at the HEAD of SEG_CMD and RingProducer requires +// a POWER-OF-TWO capacity (Ring.cpp:89-103 - the mask IS the indexing). Those two +// facts together mean a segment and its ring cannot both be 8 MiB, and one of the +// two numbers has to give. // -// CONTRACT-P5 §5 and Config.h's MOBILEGL_IPC_RING_MB comment both say "8 MiB caps -// one record at 4 MiB". That arithmetic assumed the whole segment is ring bytes -// and did not subtract the control page. The number here is HALF of theirs, and -// the deviation is deliberately in the SAFE direction: R-10's obligation is to -// PROVE no record ever approaches the cap, and a lower cap makes that proof fire -// earlier and louder rather than later and silently. The alternatives were both -// worse - announcing SegmentRef.sizeBytes as 4096 + 8 MiB breaks the four sizes -// ProtocolSmokeTest.cpp:72 pins, and moving RingControl out of SEG_CMD needs a -// fifth SegmentRef that Welcome does not have. +// The one that gives is the SEGMENT: SEG_CMD is `MOBILEGL_IPC_RING_MB` MiB PLUS +// 4096, so the ring inside it is exactly MOBILEGL_IPC_RING_MB MiB and +// RingProducer::MaxRecordBytes() is exactly half of that. CONTRACT-P5 §5 and +// Config.h's MOBILEGL_IPC_RING_MB comment - "A RECORD MAY BE AT MOST HALF OF +// THIS, so 8 MiB caps one record at 4 MiB" - are then TRUE AS WRITTEN, which +// matters because that sentence is what every other package sizes against. +// +// The first version of this file did the opposite: an 8 MiB segment with a 4 MiB +// ring and a 2 MiB record cap, on the grounds that ProtocolSmokeTest.cpp:72 pinned +// the four announced sizes. That was wrong on the facts - that test builds four +// SegmentRefs from its own literals and round-trips them through the schema; it +// says nothing about what a session announces, and it never mentions +// SessionSegments at all. So the alternative was available at no cost, and the +// version that made two live documents false and left half of SEG_CMD mapped and +// unreachable was the worse of the two. +// +// SegmentRef.sizeBytes therefore announces the MAPPING size (ring + page), which +// is what a spawn peer must mmap. The four numbers a reader recognises - 8 MiB / +// 32 MiB / 8 MiB / 256 KiB - are the RING sizes, which is what the knobs name. // // SEG_STAGE has no control page of its own: RingControl carries TWO cursor -// triples (Ring.h:101-109) and the stage triple is the second. So SEG_STAGE's -// capacity is its whole segment, and 32 MiB is already a power of two. +// triples (Ring.h:101-109) and the stage triple is the second. So SEG_STAGE is +// exactly its ring, and 32 MiB is already a power of two. SEG_REPLY is not a ring +// at all. // --------------------------------------------------------------------------- #pragma once @@ -65,22 +75,29 @@ namespace MobileGL::MG_Remote::Transport { - // The four sizes are CONTRACT-P5's and are pinned by ProtocolSmokeTest.cpp:72. - // MOBILEGL_IPC_RING_MB / MOBILEGL_IPC_STAGE_MB move the first two. + // RING sizes, not segment sizes - see the header block. The four defaults are + // CONTRACT-P5's; MOBILEGL_IPC_RING_MB / MOBILEGL_IPC_STAGE_MB move the first + // two, and ServerSession applies them unless SetSegmentSizes overrode them. struct SessionSegmentSizes { - std::uint64_t CmdBytes = 8ull * 1024 * 1024; - std::uint64_t StageBytes = 32ull * 1024 * 1024; - std::uint64_t ReplyBytes = 8ull * 1024 * 1024; - std::uint64_t EventBytes = 256ull * 1024; + std::uint64_t CmdRingBytes = 8ull * 1024 * 1024; // + one control page + std::uint64_t StageRingBytes = 32ull * 1024 * 1024; // no control page + std::uint64_t ReplyBytes = 8ull * 1024 * 1024; // not a ring + std::uint64_t EventRingBytes = 256ull * 1024; // + one control page std::uint32_t ReplySlotCount = kDefaultReplySlotCount; }; // Largest power of two <= `bytes`, or 0 when there is none. The ring's - // indexing is a mask, so this is what any segment's usable ring area is. + // indexing is a mask, so this is what a ring capacity has to be rounded to. std::uint64_t LargestPowerOfTwoAtMost(std::uint64_t bytes); - // Usable ring capacity of a segment that carries a RingControl page at its - // head. See the header block above for why this is half the segment. + // How big a segment has to be to hold `ringBytes` of ring behind its control + // page. `ringBytes` is rounded DOWN to a power of two first, so an operator + // who asks for 6 MiB gets a 4 MiB ring in a 4 MiB + 4096 segment rather than + // a segment whose tail can never be addressed. + std::uint64_t SegmentBytesForRing(std::uint64_t ringBytes); + + // The usable ring inside a segment that carries a RingControl page at its + // head. The inverse of SegmentBytesForRing for any size it produced. std::uint64_t RingCapacityForSegment(std::uint64_t segmentBytes); enum class SessionSegmentSlot : std::uint32_t { @@ -111,9 +128,24 @@ namespace MobileGL::MG_Remote::Transport { // and SEG_EVENT's), and books the mapping in `role`'s ledger. MobileGLResult Create(const SessionSegmentSizes& sizes, MemoryRole role); - // The inproc peer's view: the SAME mapping, booked under the OTHER role. - // It does not re-init the control pages - there is one shared page and - // re-initialising it would zero the owner's cursors under it. + // The inproc peer's view of the owner's four segments, booked under the + // OTHER role. It does NOT re-init the control pages - there is one shared + // page per ring and re-initialising it would zero the owner's cursors out + // from under whoever is already using them. + // + // ON POSIX THIS IS A REAL SECOND MAPPING, NOT AN ALIAS: each descriptor is + // dup()ed and adopted through ShmSegment::Adopt + Map, so the peer gets + // its own virtual addresses over the same memfd. That is the same reason + // inproc uses ShmSegment at all (see the header block): the ATTACH half is + // the half P6 replaces with an SCM_RIGHTS Adopt, and aliasing the owner's + // ShmSegment objects would leave it first exercised on the day the second + // process appears - which is exactly the criticism this file levels at + // new[]. It also removes a raw lifetime coupling: an aliased view holds + // pointers into the owner's members with no ownership, so the two Closes + // have to be ordered by hand. + // + // Windows has no Adopt (ShmSegment::Adopt is POSIX-only; the section name + // travels in SegmentRef instead), so there it still aliases and says so. MobileGLResult AttachInProcess(SessionSegments& owner, MemoryRole role); void Close(); @@ -177,10 +209,19 @@ namespace MobileGL::MG_Remote::Transport { // THE ONE RULE THAT MATTERS: a watermark may be published LATE but NEVER // EARLY. Late costs a waiter some latency; early makes every waiter a silent // use of work that has not happened, and there is no checksum anywhere on - // this ring that would catch it. So the advances below REFUSE to move a - // watermark backwards (that is the detectable half) and the callers are - // responsible for never calling them before the work is done (that is the - // half only a call-site review and R-9's unit cases can enforce). + // this ring that would catch it. The callers are responsible for never + // calling an advance before the work is done - that is the half only a + // call-site review and R-9's unit cases can enforce. + // + // THE HALF THAT IS MECHANICALLY DETECTABLE - a watermark moving BACKWARDS - + // IS FATAL, not logged-and-ignored. Logging it and returning was the first + // version of this file and it was worse than useless: SessionConsumer keeps + // its own counter, so once the shared appliedSeq is behind, every later + // advance is a no-op for ever and every WaitForApplied(seq, kWaitForever) - + // the verb barrier and every reply wait - blocks permanently. The user sees a + // hang and the only evidence is one ERROR line. This is the same class as + // RingCursorsValid returning false, and Ring.h:181-185 already calls that "a + // Fatal{ProtocolCorruption}, never a retry". namespace Watermark { // Producer, after Publish. Nobody waits on it - it is the answer to "how @@ -247,6 +288,17 @@ namespace MobileGL::MG_Remote::Transport { // call order; this is the one place production code performs it. void PublishAndNotify(std::uint64_t submittedSeq); + // The last seq THIS producer published, kept locally rather than read back + // out of RingControl::submittedSeq. Teardown's drain needs it: Ring.h:72-77 + // explicitly permits submittedSeq to be published LAZILY and Ring.h:243 + // encourages batching the publish, so the shared watermark may lag the + // emitter - and a drain that waits for `appliedSeq >= submittedSeq` would + // then under-wait and free an emitter's var-tail while a record still + // names it. With the verb barrier armed the two are equal; with + // MOBILEGL_IPC_VERB_BARRIER=0, R-1's negative control which the phase has + // to run once, they are not. + std::uint64_t LastPublishedSeq() const { return m_lastPublishedSeq; } + // The verb barrier's wait, AND the reply's wait: they are the same wait // (R-3/R-5), which is why a blocking ReadPixels, MapPersistent's decline // and the four Bool acceptances cost ZERO extra round trips. @@ -277,6 +329,7 @@ namespace MobileGL::MG_Remote::Transport { Doorbell* m_peerBell = nullptr; Doorbell* m_selfBell = nullptr; std::uint32_t m_spinUs = kDefaultSpinUs; + std::uint64_t m_lastPublishedSeq = 0; }; // ----------------------------------------------------------------------- @@ -322,15 +375,50 @@ namespace MobileGL::MG_Remote::Transport { ++m_appliedSeq; Watermark::AdvanceApplied(*m_control, m_appliedSeq); m_cmd->PublishApplied(); + // THE BYTE CURSOR RetireThrough MAY RECLAIM TO, which is NOT simply + // "everything popped". A record carrying kRecBorrowSlot has been lent + // into the GPU timeline and its slot can only be recycled after + // completedFrameSerial (Ring.h:24-27), so the reclaimable cursor stops + // AT the first borrowed record and does not move again until + // RetireBorrowedUpTo releases it. Nothing sets kRecBorrowSlot yet; + // this is here so that the day something does, the producer does not + // overwrite a slot the GPU is still reading. + if ((view.flags & kRecBorrowSlot) == 0 && !m_borrowHeld) { + m_retirableCursor = view.cursor + sizeof(RingRecordHeader) + view.payloadSize; + } else { + m_borrowHeld = true; + } NotifyClient(); return true; } - // Records without kRecBorrowSlot retire as soon as they are applied; a - // borrowed slot retires on completedFrameSerial, which is why this is a - // separate call and not folded into ApplyOne. + // Publish the retire watermark and hand back every byte up to the first + // still-borrowed record. + // + // IT IS MANDATORY, NOT OPTIONAL. RingProducer::FreeBytes() reclaims against + // retiredTail ONLY (Ring.cpp:110-115) and nothing else in this class + // publishes it, so an apply loop that calls ApplyOne and never this wedges + // the producer on the first full ring. Call it once per drain batch. void RetireThrough(std::uint64_t seq); + // Release borrowed slots up to `cursor` once completedFrameSerial has + // passed them. `cursor` is a RingRecordView::cursor the apply loop kept. + // This is the only thing that moves the reclaim point past a borrowed + // record - see ApplyOne. + void RetireBorrowedUpTo(std::uint64_t cursor); + + // The byte cursor RetireThrough would reclaim to right now. Diagnostic; + // a borrow that is never released shows up as this number standing still. + std::uint64_t RetirableCursor() const { return m_retirableCursor; } + + // completedFrameSerial and presentAckSerial, advanced AND rung. The free + // functions in namespace Watermark advance only: a v1 caller that used one + // directly would leave a client parked in WaitForPresentAck(kWaitForever) + // with nothing to wake it, because the advance and the doorbell are two + // separate stores and only the pair is a wakeup. + void CompleteFrame(std::uint64_t serial); + void ReturnPresentCredit(std::uint64_t serial); + // Ring the client's bell, but only when it said it is parked: a store to // a shared cache line otherwise burns a big core for a whole frame on a // phone (Doorbell.h:13-22). @@ -350,6 +438,8 @@ namespace MobileGL::MG_Remote::Transport { Doorbell* m_selfBell = nullptr; std::uint32_t m_spinUs = kDefaultSpinUs; std::uint64_t m_appliedSeq = 0; + std::uint64_t m_retirableCursor = 0; + bool m_borrowHeld = false; }; // ----------------------------------------------------------------------- diff --git a/MobileGL/MG_Remote/Transport/ShmSegment.cpp b/MobileGL/MG_Remote/Transport/ShmSegment.cpp index 54a7f715..e07be3ea 100644 --- a/MobileGL/MG_Remote/Transport/ShmSegment.cpp +++ b/MobileGL/MG_Remote/Transport/ShmSegment.cpp @@ -25,6 +25,10 @@ #include #include +#if !defined(_WIN32) +#include +#endif + namespace MobileGL::MG_Remote::Transport { ShmSegment::~ShmSegment() { Close(); } @@ -180,17 +184,37 @@ namespace MobileGL::MG_Remote::Transport { MobileGLResult SessionSegments::Create(const SessionSegmentSizes& sizes, MemoryRole role) { Close(); + // Set BEFORE the loop, so that Close() on a failure INSIDE it really + // closes what has already been created: Close only walks m_owned when + // m_owns is true, and setting it afterwards left a failure at segment 3 + // holding segments 0-2's descriptors and mappings open with Valid() + // false, against ShmSegment.h:66's "unmaps and releases the descriptor". + m_owns = true; struct Spec { const char* name; std::uint64_t bytes; }; + // The sizes are RING sizes; a segment that carries a control page at its + // head is that much bigger. SEG_STAGE drives the SECOND cursor triple of + // SEG_CMD's page and SEG_REPLY is not a ring at all, so neither of those + // two grows. const Spec specs[kSlotCount] = { - {"mgl-cmd", sizes.CmdBytes}, - {"mgl-stage", sizes.StageBytes}, + {"mgl-cmd", SegmentBytesForRing(sizes.CmdRingBytes)}, + {"mgl-stage", LargestPowerOfTwoAtMost(sizes.StageRingBytes)}, {"mgl-reply", sizes.ReplyBytes}, - {"mgl-event", sizes.EventBytes}, + {"mgl-event", SegmentBytesForRing(sizes.EventRingBytes)}, }; + for (std::size_t index = 0; index < kSlotCount; ++index) { + if (specs[index].bytes == 0) { + MGLOG_E("MG_Remote session: segment %s was asked for a ring size that cannot be " + "made into one (a ring is a power of two between %llu and %llu bytes)", + specs[index].name, static_cast(kMinRingCapacity), + static_cast(kMaxRingCapacity)); + Close(); + return MOBILEGL_ERR_INVALID_ARGUMENT; + } + } for (std::size_t index = 0; index < kSlotCount; ++index) { const MobileGLResult created = @@ -215,7 +239,6 @@ namespace MobileGL::MG_Remote::Transport { } } - m_owns = true; m_replySlotCount = sizes.ReplySlotCount; DeriveViews(); if (!m_valid) { @@ -240,10 +263,51 @@ namespace MobileGL::MG_Remote::Transport { if (!owner.Valid()) { return MOBILEGL_ERR_NOT_INITIALIZED; } +#if !defined(_WIN32) + // A REAL SECOND MAPPING, not an alias. dup + Adopt + Map is byte for byte + // the call sequence P6's SCM_RIGHTS client runs, so mapping, the fstat + // size check inside Adopt (ShmSegmentPosix.cpp:119-141), alignment and the + // peer's own lifetime are all exercised now rather than on the day the + // second process appears. Aliasing the owner's ShmSegment objects would + // leave the attach half untested for exactly the reason this file refuses + // to allocate the rings with new[]. + m_owns = true; + for (std::size_t index = 0; index < kSlotCount; ++index) { + const ShmSegment* theirs = owner.m_segments[index]; + const int duplicate = theirs == nullptr ? -1 : ::dup(theirs->Fd()); + if (duplicate < 0) { + MGLOG_E("MG_Remote session: could not dup the owner's descriptor for segment %zu", + index); + Close(); + return MOBILEGL_ERR_INVALID_ARGUMENT; + } + // Adopt takes ownership of `duplicate` on success only. + const MobileGLResult adopted = + ShmSegment::Adopt(duplicate, theirs->Size(), m_owned[index]); + if (adopted != MOBILEGL_OK) { + ::close(duplicate); + Close(); + return adopted; + } + // Read/write: under inproc the client writes SEG_CMD and SEG_STAGE and + // reads SEG_REPLY and SEG_EVENT, and one ShmSegment maps the whole + // thing one way. The per-segment read-only peer view is P6's, where + // the roles are separable. + const MobileGLResult mapped = m_owned[index].Map(false); + if (mapped != MOBILEGL_OK) { + Close(); + return mapped; + } + } +#else + // Windows has no Adopt (ShmSegment::Adopt is POSIX-only; a Windows peer + // resolves the section by the name carried in SegmentRef). Alias, and say + // so: this arm does not exercise the attach path P6 replaces. for (std::size_t index = 0; index < kSlotCount; ++index) { m_segments[index] = owner.m_segments[index]; } m_owns = false; +#endif m_replySlotCount = owner.m_replySlotCount; DeriveViews(); if (!m_valid) { From 8024dab9f4d3a5c3864d5a843a30a6e22a838814 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 11 Sep 2026 15:04:54 -0400 Subject: [PATCH 08/10] [Fix] (MG_Remote, Client): fold the null union payload into both handshake guards, close the server session on every Start failure that ran after Accept succeeded, and drain teardown against the producer's own published seq rather than a watermark the ring lets lag --- MobileGL/MG_Remote/Client/ClientSession.cpp | 51 ++++++++++++++++++--- MobileGL/MG_Remote/Client/ClientSession.h | 1 - 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index 0aa18dfe..06d9601e 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -80,6 +80,26 @@ namespace MobileGL::MG_Remote::Client { std::abort(); } + // Guarded the same way ServerSession's helpers are: MG_Config::Ipc only exists + // behind MOBILEGL_BUILD_DISAGGREGATED (Config.h), and this file is only compiled + // there today - but the guard is what keeps that true if the source list ever + // changes, and an unguarded read would be a compile error nobody could read. + Uint32 SpinUsFromConfig() { +#if MOBILEGL_BUILD_DISAGGREGATED + return MG_Config::Ipc.SpinUs; +#else + return Transport::kDefaultSpinUs; +#endif + } + + Bool VerbBarrierFromConfig() { +#if MOBILEGL_BUILD_DISAGGREGATED + return MG_Config::Ipc.VerbBarrier != 0; +#else + return true; +#endif + } + const char* TransportModeName(MG_Config::TransportMode mode) { switch (mode) { case MG_Config::TransportMode::Monolith: return "monolith"; @@ -172,7 +192,11 @@ namespace MobileGL::MG_Remote::Client { return received; } const ::MobileGL::Wire::CtrlEnvelope* envelope = ParseEnvelope(frame); - if (envelope == nullptr || envelope->msg_type() != ::MobileGL::Wire::CtrlMsg::Welcome) { + // msg_as_Welcome() IS PART OF THE GUARD - flatbuffers' Verifier::VerifyTable is + // `return !table || table->Verify(*this)`, so a NULL union member verifies while + // msg_type() still reports Welcome. See ServerSession::Accept for the same guard. + if (envelope == nullptr || envelope->msg_type() != ::MobileGL::Wire::CtrlMsg::Welcome || + envelope->msg_as_Welcome() == nullptr) { MGLOG_E("MG_Remote client: the server's first control frame is not a verifiable " "Welcome"); Stop(); @@ -234,7 +258,7 @@ namespace MobileGL::MG_Remote::Client { // PeerDoorbell() is the bell the SERVER parks on and this side rings; SelfDoorbell() is // this side's own. Which is which is the session's knowledge, not the transport's. m_producer.Attach(control, &m_cmd, &m_stage, &m_clientTransport->PeerDoorbell(), - &m_clientTransport->SelfDoorbell(), MG_Config::Ipc.SpinUs); + &m_clientTransport->SelfDoorbell(), SpinUsFromConfig()); m_replies = Transport::ReplySlotPool(m_shm.ReplyBase(), m_shm.ReplyBytes(), m_shm.ReplySlotCount()); @@ -246,7 +270,7 @@ namespace MobileGL::MG_Remote::Client { return MOBILEGL_ERR_INVALID_ARGUMENT; } - m_barrierArmed = MG_Config::Ipc.VerbBarrier != 0; + m_barrierArmed = VerbBarrierFromConfig(); if (!m_barrierArmed) { MGLOG_W("MG_Remote client: MOBILEGL_IPC_VERB_BARRIER=0 - this is R-1's NEGATIVE " "CONTROL and is EXPECTED to be red. 31 of the 63 PipeInputs fields are still " @@ -309,10 +333,18 @@ namespace MobileGL::MG_Remote::Client { void ClientSession::Stop() { if (!m_started) { - // Start's own failure paths land here with a half-built session; tear down what - // exists and leave nothing mapped. + // Start's own failure paths land here with a half-built session. FIVE of them are + // reached AFTER ServerSession::Accept has already returned OK, so tearing down + // only the client half is not enough and gets three things wrong at once: the + // server keeps its four mappings and stays m_accepted, so Accept's own guard + // refuses every later Start and the process can never open a session again; the + // process-wide segment resolver stays installed; and resetting m_serverTransport + // destroys a transport that ServerSession::m_transport and its two Doorbell* + // still point at. The server closes FIRST, in the same order the started path + // gets right, and only then do the transports go. m_producer.Detach(); m_shm.Close(); + Server::ServerSessionInstance().Close(); m_clientTransport.reset(); m_serverTransport.reset(); m_transport = nullptr; @@ -325,7 +357,14 @@ namespace MobileGL::MG_Remote::Client { // a hung exit. Transport::RingControl* control = m_shm.CmdControl(); if (control != nullptr) { - const Uint64 submitted = control->submittedSeq.load(std::memory_order_acquire); + // The PRODUCER's own last-published seq, not RingControl::submittedSeq. Ring.h:72-77 + // permits submittedSeq to be published lazily and Ring.h:243 encourages batching + // the publish, so the shared watermark is allowed to lag the emitter - and a drain + // that waited for `appliedSeq >= submittedSeq` would then under-wait and free an + // emitter-owned var-tail while a record still names it. With the verb barrier armed + // the two are equal; under MOBILEGL_IPC_VERB_BARRIER=0, R-1's negative control that + // the phase has to run once, they are not. + const Uint64 submitted = m_producer.LastPublishedSeq(); m_producer.PublishAndNotify(submitted); if (submitted != 0 && m_producer.WaitForApplied(submitted, kDrainTimeoutMs) != Transport::SessionWait::Reached) { diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index 61e33c80..dd818421 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -143,7 +143,6 @@ namespace MobileGL::MG_Remote::Client { private: Wire::PipeWireEncoder m_encoder; Wire::SegmentTable m_segments; - CapsMirror* m_caps = nullptr; Bool m_barrierArmed = true; std::unique_ptr m_clientTransport; From 3f1eaee787b7bebae868709d2ac3c899fb279f7b Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 11 Sep 2026 15:04:54 -0400 Subject: [PATCH 09/10] [Test] (MG_Test, Wire): pin the seven findings of fix round one - the frozen CapsSnapshot field ids, the verifiable frame with a null union, the borrowed slot that is not handed back, the peer's own mapping, the backwards-watermark abort, the lagging watermark and the refused reply geometry --- MobileGL/MG_Test/Wire/SessionTest.cpp | 305 ++++++++++++++++++++++---- 1 file changed, 264 insertions(+), 41 deletions(-) diff --git a/MobileGL/MG_Test/Wire/SessionTest.cpp b/MobileGL/MG_Test/Wire/SessionTest.cpp index f2fff637..c0de8e7c 100644 --- a/MobileGL/MG_Test/Wire/SessionTest.cpp +++ b/MobileGL/MG_Test/Wire/SessionTest.cpp @@ -23,6 +23,7 @@ // both delivery modes and SessionSegmentsAreRealSharedMemory below is the mechanical check that // it still does. +#include #include #include #include @@ -49,11 +50,12 @@ namespace { // command ring many times over - which is what puts the kRecPad rule under load rather // than under a contrived single wrap. SessionSegmentSizes TestSizes() { + // RING sizes. The SEG_CMD and SEG_EVENT segments are each one control page bigger. SessionSegmentSizes sizes; - sizes.CmdBytes = 64ull * 1024; // -> 32 KiB ring after the control page - sizes.StageBytes = 64ull * 1024; // -> 64 KiB, no control page of its own + sizes.CmdRingBytes = 32ull * 1024; + sizes.StageRingBytes = 64ull * 1024; sizes.ReplyBytes = 64ull * 1024; // -> 8 slots of 8 KiB - sizes.EventBytes = 32ull * 1024; // -> 16 KiB ring after the control page + sizes.EventRingBytes = 16ull * 1024; sizes.ReplySlotCount = 8; return sizes; } @@ -83,29 +85,35 @@ namespace { if (clientSegments.AttachInProcess(serverSegments, MemoryRole::Client) != MOBILEGL_OK) { return false; } - RingControl* control = serverSegments.CmdControl(); - cmdProducer = RingProducer(control, clientSegments.CmdRingBase(), + // EACH ROLE DRIVES ITS OWN MAPPING. Under inproc AttachInProcess dups the + // owner's descriptors and maps them again, so the client's RingControl is a + // different VIRTUAL address over the same physical page - which is exactly the + // shape spawn has, and the reason the fixture does not share one pointer. + RingControl* clientControl = clientSegments.CmdControl(); + RingControl* serverControl = serverSegments.CmdControl(); + cmdProducer = RingProducer(clientControl, clientSegments.CmdRingBase(), clientSegments.CmdRingCapacity(), RingCursorSet::Cmd); - stageProducer = RingProducer(control, clientSegments.StageBase(), + stageProducer = RingProducer(clientControl, clientSegments.StageBase(), clientSegments.StageCapacity(), RingCursorSet::Stage); - cmdConsumer = RingConsumer(control, serverSegments.CmdRingBase(), + cmdConsumer = RingConsumer(serverControl, serverSegments.CmdRingBase(), serverSegments.CmdRingCapacity(), RingCursorSet::Cmd); if (!cmdProducer.Valid() || !stageProducer.Valid() || !cmdConsumer.Valid()) { return false; } // PeerDoorbell is the bell the OTHER end parks on; SelfDoorbell is this end's own. // Which is which is the session's knowledge, never ITransport's (contract §3.9). - producer.Attach(control, &cmdProducer, &stageProducer, &clientTransport->PeerDoorbell(), - &clientTransport->SelfDoorbell(), kDefaultSpinUs); - consumer.Attach(control, &cmdConsumer, &serverTransport->PeerDoorbell(), + producer.Attach(clientControl, &cmdProducer, &stageProducer, + &clientTransport->PeerDoorbell(), &clientTransport->SelfDoorbell(), + kDefaultSpinUs); + consumer.Attach(serverControl, &cmdConsumer, &serverTransport->PeerDoorbell(), &serverTransport->SelfDoorbell(), kDefaultSpinUs); replies = ReplySlotPool(serverSegments.ReplyBase(), serverSegments.ReplyBytes(), serverSegments.ReplySlotCount()); replies.Clear(); - eventOut = EventRingProducer(serverSegments.EventControl(), control, + eventOut = EventRingProducer(serverSegments.EventControl(), serverControl, serverSegments.EventRingBase(), serverSegments.EventRingCapacity()); - eventIn = EventRingConsumer(clientSegments.EventControl(), control, + eventIn = EventRingConsumer(clientSegments.EventControl(), clientControl, clientSegments.EventRingBase(), clientSegments.EventRingCapacity(), clientSegments.EventSegmentBase()); @@ -145,49 +153,64 @@ TEST(SessionTest, SessionSegmentsAreRealSharedMemoryAndNotAHeapAllocation) { EXPECT_FALSE(segments.Valid()); } -// The arithmetic the whole geometry rests on, and the one place CONTRACT-P5 §5's "8 MiB caps -// one record at 4 MiB" is corrected: the control page sits at the HEAD of SEG_CMD and the ring -// capacity must be a power of two, so an 8 MiB segment yields a 4 MiB ring and a 2 MiB record. -TEST(SessionTest, TheRingIsTheLargestPowerOfTwoLeftAfterTheControlPage) { +// The knob names the RING and the segment carries one control page on top, so CONTRACT-P5 §5 +// and Config.h's "A RECORD MAY BE AT MOST HALF OF THIS, so 8 MiB caps one record at 4 MiB" are +// true as written. Getting this the other way round - an 8 MiB segment with a 4 MiB ring - +// made both of those documents false and left half of SEG_CMD mapped and unreachable. +TEST(SessionTest, TheKnobNamesTheRingAndTheSegmentAddsOneControlPage) { EXPECT_EQ(LargestPowerOfTwoAtMost(0u), 0u); EXPECT_EQ(LargestPowerOfTwoAtMost(1u), 1u); EXPECT_EQ(LargestPowerOfTwoAtMost(4095u), 2048u); EXPECT_EQ(LargestPowerOfTwoAtMost(4096u), 4096u); EXPECT_EQ(LargestPowerOfTwoAtMost(4097u), 4096u); - // The four contract sizes, which ProtocolSmokeTest.cpp:72 pins on the wire. - constexpr std::uint64_t kCmd = 8ull * 1024 * 1024; - constexpr std::uint64_t kEvent = 256ull * 1024; - EXPECT_EQ(RingCapacityForSegment(kCmd), 4ull * 1024 * 1024); - EXPECT_EQ(RingCapacityForSegment(kEvent), 128ull * 1024); + constexpr std::uint64_t kCmdRing = 8ull * 1024 * 1024; + constexpr std::uint64_t kEventRing = 256ull * 1024; + EXPECT_EQ(SegmentBytesForRing(kCmdRing), kCmdRing + sizeof(RingControl)); + EXPECT_EQ(SegmentBytesForRing(kEventRing), kEventRing + sizeof(RingControl)); + // Round trip: the capacity inside a segment SegmentBytesForRing produced is the ring + // that was asked for, exactly. + EXPECT_EQ(RingCapacityForSegment(SegmentBytesForRing(kCmdRing)), kCmdRing); + EXPECT_EQ(RingCapacityForSegment(SegmentBytesForRing(kEventRing)), kEventRing); + // A ring size that is not a power of two is rounded DOWN rather than silently producing a + // segment whose tail can never be addressed. + EXPECT_EQ(SegmentBytesForRing(6ull * 1024 * 1024), 4ull * 1024 * 1024 + sizeof(RingControl)); + // ... and therefore the real cap on one record, which R-10 obliges the codec to prove it - // never approaches. Half of the ring, not half of the segment. + // never approaches: half of MOBILEGL_IPC_RING_MB, which is what §5 says. RingControl control{}; InitRingControl(control); - std::vector bytes(static_cast(RingCapacityForSegment(kCmd))); - RingProducer producer(&control, bytes.data(), RingCapacityForSegment(kCmd), RingCursorSet::Cmd); + std::vector bytes(static_cast(kCmdRing)); + RingProducer producer(&control, bytes.data(), kCmdRing, RingCursorSet::Cmd); ASSERT_TRUE(producer.Valid()); - EXPECT_EQ(producer.MaxRecordBytes(), 2ull * 1024 * 1024); + EXPECT_EQ(producer.MaxRecordBytes(), 4ull * 1024 * 1024); // A segment that cannot hold the control page plus the smallest ring has NO ring, rather // than a ring of some rounded-down nonsense. EXPECT_EQ(RingCapacityForSegment(sizeof(RingControl)), 0u); EXPECT_EQ(RingCapacityForSegment(sizeof(RingControl) + 8), 0u); + EXPECT_EQ(SegmentBytesForRing(8u), 0u); } -// The default geometry really allocates, and the announced sizes are the MAPPING sizes - what -// a spawn peer must map - not the ring capacity inside them. -TEST(SessionTest, TheDefaultGeometryIsTheFourContractSizes) { +// The default geometry really allocates. The four RINGS are the four contract numbers; the two +// segments that carry a control page announce that much more, because sizeBytes is what a spawn +// peer must mmap. +TEST(SessionTest, TheDefaultGeometryIsTheFourContractRingSizes) { SessionSegments segments; ASSERT_EQ(segments.Create(SessionSegmentSizes{}, MemoryRole::Server), MOBILEGL_OK); - EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Cmd), 8ull * 1024 * 1024); + EXPECT_EQ(segments.CmdRingCapacity(), 8ull * 1024 * 1024); + EXPECT_EQ(segments.StageCapacity(), 32ull * 1024 * 1024); + EXPECT_EQ(segments.ReplyBytes(), 8ull * 1024 * 1024); + EXPECT_EQ(segments.EventRingCapacity(), 256ull * 1024); + + EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Cmd), + 8ull * 1024 * 1024 + sizeof(RingControl)); + // SEG_STAGE carries no control page: RingControl holds both cursor triples, so the whole + // segment is ring and 32 MiB is already a power of two. SEG_REPLY is not a ring at all. EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Stage), 32ull * 1024 * 1024); EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Reply), 8ull * 1024 * 1024); - EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Event), 256ull * 1024); - EXPECT_EQ(segments.CmdRingCapacity(), 4ull * 1024 * 1024); - // SEG_STAGE carries no control page: RingControl holds both cursor triples, so the whole - // segment is ring and 32 MiB is already a power of two. - EXPECT_EQ(segments.StageCapacity(), 32ull * 1024 * 1024); + EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Event), + 256ull * 1024 + sizeof(RingControl)); segments.Close(); } @@ -374,10 +397,7 @@ TEST(SessionTest, RetiredSeqMayTrailTheApplyButCanNeverOvertakeIt) { // Running ahead is not: clamped to what has actually been applied. Watermark::AdvanceRetired(session.Control(), 99); EXPECT_EQ(session.Control().retiredSeq.load(), 3u); - // And it never goes backwards, because a waiter that already resumed on the higher value - // cannot be un-resumed. - Watermark::AdvanceRetired(session.Control(), 2); - EXPECT_EQ(session.Control().retiredSeq.load(), 3u); + // Going BACKWARDS is Fatal, not clamped - see AWatermarkThatMovesBackwardsIsFatal below. } // completedFrameSerial: the SERVER's, advanced when a present completes. It trails appliedSeq @@ -400,8 +420,9 @@ TEST(SessionTest, CompletedFrameSerialIsTheServersAndIsIndependentOfAppliedSeq) Watermark::AdvanceCompletedFrame(session.Control(), 2); EXPECT_EQ(session.Control().completedFrameSerial.load(), 2u); - Watermark::AdvanceCompletedFrame(session.Control(), 1); - EXPECT_EQ(session.Control().completedFrameSerial.load(), 2u) << "a watermark went backwards"; + // Republishing the SAME serial is legal and is what a lazy publisher does. + Watermark::AdvanceCompletedFrame(session.Control(), 2); + EXPECT_EQ(session.Control().completedFrameSerial.load(), 2u); } // presentAckSerial: the only back-pressure that bounds LATENCY rather than bytes. A client @@ -448,7 +469,7 @@ TEST(SessionTest, PresentAckSerialIsWaitedOnWithGreaterOrEqualAndWakesThroughThe // it, so fillers are certain; the session's own appliedSeq must count the records and not them. TEST(SessionTest, AWrapFillerDoesNotAdvanceTheSessionsAppliedSeq) { SessionSegmentSizes sizes = TestSizes(); - sizes.CmdBytes = 8192; // -> a 4 KiB ring, so a handful of records wraps it + sizes.CmdRingBytes = 4096; // a handful of records wraps it several times SessionFixture session; ASSERT_TRUE(session.Build(sizes)); ASSERT_EQ(session.serverSegments.CmdRingCapacity(), 4096u); @@ -767,3 +788,205 @@ TEST(SessionTest, TheAbiFingerprintChangesWhenAnyOfItsInputsDoes) { EXPECT_NE(MixAbiFingerprint(1024, 1080, 552, 0x00010000, nullptr), MixAbiFingerprint(1024, 1080, 552, 0x00010000, "abc1234")); } + +// --------------------------------------------------------------------------- +// Fix round 1 - the cases the adversarial review's findings earned +// --------------------------------------------------------------------------- + +// M-8. A watermark moving BACKWARDS is Fatal, not logged-and-ignored. Logging it and returning +// was strictly worse than an abort: SessionConsumer keeps its own counter, so once the shared +// appliedSeq is behind, every later advance is a no-op for ever and every WaitForApplied on +// kWaitForever - the verb barrier and every reply wait - blocks permanently. A hang whose only +// evidence is one ERROR line is not a diagnosis. +#if defined(GTEST_HAS_DEATH_TEST) && GTEST_HAS_DEATH_TEST +TEST(SessionTestDeath, AWatermarkThatMovesBackwardsIsFatalRatherThanIgnored) { + alignas(4096) RingControl control{}; + InitRingControl(control); + Watermark::AdvanceApplied(control, 10); + ASSERT_EQ(control.appliedSeq.load(), 10u); + EXPECT_DEATH(Watermark::AdvanceApplied(control, 9), ""); +} +#endif + +// M-6. `inproc`'s ATTACH half is a real second mapping, not an alias of the owner's ShmSegment +// objects. The attach side is the side P6 replaces with an SCM_RIGHTS Adopt, so leaving it +// aliased would mean dup/Adopt/Map/the fstat size check were first exercised on the day the +// second process appears - the same criticism this package levels at allocating with new[]. +TEST(SessionTest, TheInprocPeerGetsItsOwnMappingOfTheSameSharedMemory) { + SessionSegments owner; + ASSERT_EQ(owner.Create(TestSizes(), MemoryRole::Server), MOBILEGL_OK); + SessionSegments peer; + ASSERT_EQ(peer.AttachInProcess(owner, MemoryRole::Client), MOBILEGL_OK); + ASSERT_TRUE(peer.Valid()); + +#if !defined(_WIN32) + // Different descriptors, different virtual addresses... + EXPECT_NE(peer.DescriptorFor(SessionSegmentSlot::Cmd), + owner.DescriptorFor(SessionSegmentSlot::Cmd)); + EXPECT_GE(peer.DescriptorFor(SessionSegmentSlot::Cmd), 0); + EXPECT_NE(static_cast(peer.CmdControl()), static_cast(owner.CmdControl())); + EXPECT_NE(peer.CmdRingBase(), owner.CmdRingBase()); +#endif + // ... over the SAME physical page, which is the whole point: a store through one mapping is + // visible through the other, exactly as it is across two processes under spawn. + EXPECT_EQ(peer.AnnouncedSize(SessionSegmentSlot::Cmd), + owner.AnnouncedSize(SessionSegmentSlot::Cmd)); + owner.CmdControl()->appliedSeq.store(4242, std::memory_order_release); + EXPECT_EQ(peer.CmdControl()->appliedSeq.load(std::memory_order_acquire), 4242u); + peer.CmdControl()->presentAckSerial.store(77, std::memory_order_release); + EXPECT_EQ(owner.CmdControl()->presentAckSerial.load(std::memory_order_acquire), 77u); + + peer.Close(); + // The owner is untouched by the peer's teardown - there is no shared ownership left whose + // Close order has to be got right by hand. + EXPECT_TRUE(owner.Valid()); + EXPECT_EQ(owner.CmdControl()->appliedSeq.load(), 4242u); + owner.Close(); +} + +// M-5. RetireThrough must NOT hand back a slot borrowed into the GPU timeline. The first +// version called RingConsumer::PublishRetired(), which stores the local tail into BOTH tails +// and therefore frees every popped byte regardless of kRecBorrowSlot - defeating the reason the +// ring carries two tails at all (Ring.h:24-27) and letting the producer overwrite a slot the +// GPU is still reading. Nothing sets kRecBorrowSlot yet; this is the case that has to be true +// on the day something does. +TEST(SessionTest, ABorrowedSlotIsNotHandedBackUntilItIsReleased) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + + constexpr std::uint64_t kPayload = 64; + for (int index = 0; index < 3; ++index) { + const std::uint16_t flags = index == 1 ? kRecBorrowSlot : kRecNone; + ASSERT_NE(session.cmdProducer.Reserve(1, flags, kPayload), nullptr); + } + session.producer.PublishAndNotify(3); + + std::uint64_t firstCursor = 0; + bool sawBorrow = false; + int seen = 0; + while (session.consumer.ApplyOne([&](const RingRecordView& view) { + if (seen == 0) { + firstCursor = view.cursor; + } + if ((view.flags & kRecBorrowSlot) != 0) { + sawBorrow = true; + } + ++seen; + })) { + } + ASSERT_EQ(seen, 3); + ASSERT_TRUE(sawBorrow) << "the producer dropped kRecBorrowSlot, so this case proved nothing"; + + EXPECT_EQ(session.Control().appliedSeq.load(), 3u); + session.consumer.RetireThrough(session.consumer.AppliedSeq()); + + // The reclaim point stops AT the borrowed record: only the first record's bytes came back. + const std::uint64_t firstRecordEnd = firstCursor + sizeof(RingRecordHeader) + kPayload; + EXPECT_EQ(session.consumer.RetirableCursor(), firstRecordEnd); + EXPECT_EQ(session.Control().cmdRetiredTail.load(), firstRecordEnd); + EXPECT_LT(session.Control().cmdRetiredTail.load(), session.Control().cmdAppliedTail.load()) + << "a borrowed slot was handed back to the producer while the GPU may still read it"; + EXPECT_TRUE(RingCursorsValid(session.Control(), RingCursorSet::Cmd, + session.serverSegments.CmdRingCapacity())); + + // completedFrameSerial passed it: now the rest comes back. + session.consumer.RetireBorrowedUpTo(session.cmdConsumer.LocalTail()); + EXPECT_EQ(session.Control().cmdRetiredTail.load(), session.Control().cmdAppliedTail.load()); + EXPECT_TRUE(RingCursorsValid(session.Control(), RingCursorSet::Cmd, + session.serverSegments.CmdRingCapacity())); +} + +// q-1. Teardown's drain keys on the PRODUCER's own last-published seq, not on +// RingControl::submittedSeq: Ring.h:72-77 permits that watermark to be published lazily, so a +// drain that trusted it would under-wait and free an emitter-owned var-tail while a record +// still names it. With the verb barrier armed the two are equal; under +// MOBILEGL_IPC_VERB_BARRIER=0 - R-1's negative control, which the phase must run once - they +// are not. +TEST(SessionTest, TheProducerRemembersWhatItPublishedEvenIfTheWatermarkLags) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + + for (int index = 0; index < 4; ++index) { + ASSERT_NE(session.cmdProducer.Reserve(1, kRecNone, 8), nullptr); + } + // A lazy publisher: four records are visible, the shared watermark says two. + session.producer.PublishAndNotify(2); + EXPECT_EQ(session.Control().submittedSeq.load(), 2u); + EXPECT_EQ(session.producer.LastPublishedSeq(), 2u); + + session.producer.PublishAndNotify(4); + EXPECT_EQ(session.producer.LastPublishedSeq(), 4u); + EXPECT_EQ(session.Control().submittedSeq.load(), 4u); + // The drain's bound may only grow, whatever a caller passes. Republishing an older bound is + // publishing LATE, which R-9 permits - it is clamped up rather than treated as a watermark + // moving backwards, which is Fatal. Teardown does exactly this. + session.producer.PublishAndNotify(3); + EXPECT_EQ(session.producer.LastPublishedSeq(), 4u); + EXPECT_EQ(session.Control().submittedSeq.load(), 4u); +} + +// M-3. FlatBuffers' Verifier::VerifyTable is `return !table || table->Verify(*this)`, so a NULL +// union member PASSES verification: a 24-byte frame verifies, carries the file identifier, +// reports msg_type() == Hello, and returns nullptr from msg_as_Hello(). Both handshakes now +// fold that into their guard instead of dereferencing it. This case is the proof the shape is +// reachable at all, so the guard cannot be "simplified" away later. +TEST(SessionTest, AVerifiableFrameCanCarryANullUnionPayload) { + ::flatbuffers::FlatBufferBuilder builder(256); + auto envelope = ::MobileGL::Wire::CreateCtrlEnvelope(builder, ::MobileGL::Wire::CtrlMsg::Hello, + ::flatbuffers::Offset()); + ::MobileGL::Wire::FinishCtrlEnvelopeBuffer(builder, envelope); + + ::flatbuffers::Verifier verifier(builder.GetBufferPointer(), builder.GetSize()); + ASSERT_TRUE(::MobileGL::Wire::VerifyCtrlEnvelopeBuffer(verifier)) + << "if this ever starts failing, the guard in both handshakes may be relaxed"; + ASSERT_TRUE(::MobileGL::Wire::CtrlEnvelopeBufferHasIdentifier(builder.GetBufferPointer())); + + const ::MobileGL::Wire::CtrlEnvelope* parsed = + ::MobileGL::Wire::GetCtrlEnvelope(builder.GetBufferPointer()); + ASSERT_NE(parsed, nullptr); + EXPECT_EQ(parsed->msg_type(), ::MobileGL::Wire::CtrlMsg::Hello); + // The whole finding, in one line: the tag says Hello and there is no Hello. + EXPECT_EQ(parsed->msg_as_Hello(), nullptr); +} + +// M-1. The four retired CapsSnapshot fields are `(deprecated)`, so their vtable slots stay +// burned and the two new fields sit past them. Plainly deleting them handed slot 14 - which +// used to carry an `[int]` vector, a 4-byte uoffset - to `callMask`, an 8-byte inline ulong, +// with no ABI-major bump and an ABI fingerprint that mixes struct sizes rather than the schema. +// THIS IS THE FIRST TEST IN THE TREE THAT PINS A TABLE FIELD ID: ProtocolSmokeTest's +// UnionTagsAreFrozenWireValues pins union tags and enum values only and says nothing about one. +TEST(SessionTest, CapsSnapshotFieldIdsAreFrozenAndTheRetiredSlotsStayBurned) { + using CapsSnapshot = ::MobileGL::Wire::CapsSnapshot; + EXPECT_EQ(static_cast(CapsSnapshot::VT_DYNAMICPARAMETERS), 4); + EXPECT_EQ(static_cast(CapsSnapshot::VT_RENDERERINFO), 6); + EXPECT_EQ(static_cast(CapsSnapshot::VT_FORMATCAPS), 8); + EXPECT_EQ(static_cast(CapsSnapshot::VT_EXTENSIONS), 10); + EXPECT_EQ(static_cast(CapsSnapshot::VT_APIVERSION), 12); + // 14, 16, 18 and 20 are the four retired fields and must stay unreachable for ever. + EXPECT_EQ(static_cast(CapsSnapshot::VT_CALLMASK), 22); + EXPECT_EQ(static_cast(CapsSnapshot::VT_BACKENDTYPE), 24); + + // The appended Hello / Welcome fields really are tail appends onto slots nothing occupied. + EXPECT_EQ(static_cast(::MobileGL::Wire::Hello::VT_ABIFINGERPRINT), 16); + EXPECT_EQ(static_cast(::MobileGL::Wire::Welcome::VT_EVENTRING), 16); + EXPECT_EQ(static_cast(::MobileGL::Wire::Welcome::VT_BUILDFINGERPRINT), 18); + EXPECT_EQ(static_cast(::MobileGL::Wire::Welcome::VT_ABIFINGERPRINT), 20); +} + +// m-4. The reply pool refuses a geometry whose slots would not be 8-aligned: the fences order +// the payload against the stamp, but the stamp's own 8-byte Seq has to be untorn for the +// wrong-slot self-check to mean anything, and that is only true while it is naturally aligned. +TEST(SessionTest, AReplyGeometryThatWouldMisalignASlotHeaderIsRefused) { + std::vector aligned(1024); + void* base = aligned.data(); + // 4100 / 4 = 1025 -> every slot after the first lands on an odd boundary. + EXPECT_FALSE(ReplySlotPool(base, 4100, 4).Valid()); + // A base that is not itself 8-aligned is refused too, whatever the slot size. + EXPECT_FALSE(ReplySlotPool(static_cast(base) + 1, 4096, 8).Valid()); + // Not a power of two: the addressing is a mask. + EXPECT_FALSE(ReplySlotPool(base, 4096, 6).Valid()); + // No room for a payload past the 16-byte header. + EXPECT_FALSE(ReplySlotPool(base, 64, 8).Valid()); + // And the shape that is legal. + EXPECT_TRUE(ReplySlotPool(base, 4096, 8).Valid()); +} From f33e5d60907466f69a84f4fae51207e330471f44 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 11 Sep 2026 15:41:44 -0400 Subject: [PATCH 10/10] [Fix] (MG_Remote, Transport, Client): SEG_STAGE is w1's encoder-local linear allocator and not a ring - drop the stage RingProducer and its power-of-two rounding, pin the stage cursor triple dead, require a wrap filler to carry kind kRingPadRecordKind as well as the flag it shares with kVarTail, and name every kRecBorrowSlot sighting --- MobileGL/MG_Remote/Client/ClientSession.cpp | 23 ++-- MobileGL/MG_Remote/Client/ClientSession.h | 1 - MobileGL/MG_Remote/Server/ServerSession.cpp | 27 ++++- MobileGL/MG_Remote/Transport/Ring.cpp | 44 +++++--- MobileGL/MG_Remote/Transport/Ring.h | 52 ++++++++- MobileGL/MG_Remote/Transport/SessionRings.h | 52 +++++++-- MobileGL/MG_Remote/Transport/ShmSegment.cpp | 15 +-- MobileGL/MG_Test/Wire/SessionTest.cpp | 116 ++++++++++++++++++-- 8 files changed, 273 insertions(+), 57 deletions(-) diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index 06d9601e..4fb94dd1 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -249,15 +249,20 @@ namespace MobileGL::MG_Remote::Client { Transport::RingControl* control = m_shm.CmdControl(); m_cmd = Transport::RingProducer(control, m_shm.CmdRingBase(), m_shm.CmdRingCapacity(), Transport::RingCursorSet::Cmd); - m_stage = Transport::RingProducer(control, m_shm.StageBase(), m_shm.StageCapacity(), - Transport::RingCursorSet::Stage); - if (!m_cmd.Valid() || !m_stage.Valid()) { + // NO STAGE RING. SEG_STAGE is package w1's encoder-local LINEAR ALLOCATOR: + // a staged byte run carries no RingRecordHeader, nothing consumes SEG_STAGE, + // and the allocator reclaims on retiredSeq. A RingProducer over + // RingCursorSet::Stage would publish stageHead with nothing advancing the + // two tails, so FreeBytes() would fall to zero the first time the head + // lapped the capacity and never recover - a guaranteed hang. See + // RingControl's stage triple in Ring.h. + if (!m_cmd.Valid()) { Stop(); return MOBILEGL_ERR_INVALID_ARGUMENT; } // PeerDoorbell() is the bell the SERVER parks on and this side rings; SelfDoorbell() is // this side's own. Which is which is the session's knowledge, not the transport's. - m_producer.Attach(control, &m_cmd, &m_stage, &m_clientTransport->PeerDoorbell(), + m_producer.Attach(control, &m_cmd, &m_clientTransport->PeerDoorbell(), &m_clientTransport->SelfDoorbell(), SpinUsFromConfig()); m_replies = Transport::ReplySlotPool(m_shm.ReplyBase(), m_shm.ReplyBytes(), @@ -289,13 +294,18 @@ namespace MobileGL::MG_Remote::Client { m_segments.Install(Wire::kSegCmd, Wire::SegmentView{m_shm.CmdRingBase(), m_shm.CmdRingCapacity()}); m_segments.Install(Wire::kSegStage, - Wire::SegmentView{m_shm.StageBase(), m_shm.StageCapacity()}); + Wire::SegmentView{m_shm.StageBase(), m_shm.StageBytes()}); m_segments.Install(Wire::kSegReply, Wire::SegmentView{m_shm.ReplyBase(), m_shm.ReplyBytes()}); m_segments.Install(Wire::kSegEvent, Wire::SegmentView{m_shm.EventSegmentBase(), m_shm.AnnouncedSize(Transport::SessionSegmentSlot::Event)}); - m_encoder = Wire::PipeWireEncoder(control, &m_cmd, &m_stage, &m_segments); + // nullptr for the stage producer, and that is the honest value: c0's + // signature predates w1's ruling that SEG_STAGE is a linear allocator, and + // the encoder reaches its bytes through the SegmentTable above. Handing it + // a live RingProducer over a cursor triple nobody consumes would be the + // half-wired shape this session exists not to have. + m_encoder = Wire::PipeWireEncoder(control, &m_cmd, nullptr, &m_segments); // ---- 7. the first CapsSnapshot, if the server had a backend to publish one from. if (m_transport->PeekFrameSize() != 0) { @@ -395,7 +405,6 @@ namespace MobileGL::MG_Remote::Client { m_events = Transport::EventRingConsumer(); m_replies = Transport::ReplySlotPool(); m_cmd = Transport::RingProducer(); - m_stage = Transport::RingProducer(); m_shm.Close(); Server::ServerSessionInstance().Close(); m_clientTransport.reset(); diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index dd818421..ad1cdae6 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -149,7 +149,6 @@ namespace MobileGL::MG_Remote::Client { std::unique_ptr m_serverTransport; Transport::SessionSegments m_shm; Transport::RingProducer m_cmd; - Transport::RingProducer m_stage; Transport::SessionProducer m_producer; Transport::EventRingConsumer m_events; Transport::ReplySlotPool m_replies; diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index cfbe95aa..6fa3a55b 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/MobileGL/MG_Remote/Server/ServerSession.cpp @@ -23,6 +23,29 @@ namespace MobileGL::MG_Remote::Server { + // THE THREE FLAG-SPACE COLLISIONS, AS TRIPWIRES RATHER THAN AS A COMMENT. + // + // MGPipeCallFlags (MG_Pipe/MGPipe.h:42-54) and RingRecordFlags (Ring.h) are separate spaces + // that overlap, and three bits mean DIFFERENT things in each. Ring.h's enum carries the + // table; these are the assertions that break the build if either enum is renumbered, so the + // collision can never become news again. They live here because this is the nearest .cpp + // that legally sees both headers - nothing under Transport/ may reach MobileGL/Includes.h. + static_assert(static_cast(MG_Pipe::kVarTail) == Transport::kRecPad, + "MGPipeCallFlags::kVarTail and kRecPad share bit 2: an encoder that copies call " + "flags into RingRecordHeader::flags makes every var-tail record read as a wrap " + "filler. RingConsumer::Pop requires kind == kRingPadRecordKind as well, which is " + "what keeps that from eating the record - do not relax it"); + static_assert(static_cast(MG_Pipe::kHostSpan) == Transport::kRecBorrowSlot, + "MGPipeCallFlags::kHostSpan and kRecBorrowSlot share bit 3: a host-span record " + "would read as borrowed into the GPU timeline and stop the consumer reclaiming " + "ring bytes behind it. SessionConsumer counts and names every sighting"); + static_assert(static_cast(MG_Pipe::kReplySlot) == Transport::kRecVarTail, + "MGPipeCallFlags::kReplySlot and kRecVarTail share bit 4"); + static_assert(static_cast(MG_Pipe::kNeedsAck) == Transport::kRecNeedsAck && + static_cast(MG_Pipe::kHasBlob) == Transport::kRecHasBlob, + "the two bits that DO mean the same thing in both spaces have drifted apart, " + "which is a different and worse problem than the three that collide"); + namespace { // A control-plane frame is small by construction (ITransport.h:56-58: bulk bytes @@ -97,7 +120,7 @@ namespace MobileGL::MG_Remote::Server { const Uint64 ringMb = MG_Config::Ipc.RingMb == 0 ? 8u : MG_Config::Ipc.RingMb; const Uint64 stageMb = MG_Config::Ipc.StageMb == 0 ? 32u : MG_Config::Ipc.StageMb; sizes.CmdRingBytes = ringMb * 1024ull * 1024ull; - sizes.StageRingBytes = stageMb * 1024ull * 1024ull; + sizes.StageBytes = stageMb * 1024ull * 1024ull; #endif return sizes; } @@ -304,7 +327,7 @@ namespace MobileGL::MG_Remote::Server { m_segments.Install(Wire::kSegCmd, Wire::SegmentView{m_shm.CmdRingBase(), m_shm.CmdRingCapacity()}); m_segments.Install(Wire::kSegStage, - Wire::SegmentView{m_shm.StageBase(), m_shm.StageCapacity()}); + Wire::SegmentView{m_shm.StageBase(), m_shm.StageBytes()}); m_segments.Install(Wire::kSegReply, Wire::SegmentView{m_shm.ReplyBase(), m_shm.ReplyBytes()}); m_segments.Install(Wire::kSegEvent, Wire::SegmentView{m_shm.EventSegmentBase(), diff --git a/MobileGL/MG_Remote/Transport/Ring.cpp b/MobileGL/MG_Remote/Transport/Ring.cpp index b5ff5d6f..60188ae8 100644 --- a/MobileGL/MG_Remote/Transport/Ring.cpp +++ b/MobileGL/MG_Remote/Transport/Ring.cpp @@ -260,7 +260,16 @@ namespace MobileGL::MG_Remote::Transport { return false; } - if ((header.flags & kRecPad) != 0) { + // BOTH, not just the flag. kRecPad (1<<2) is the same bit as + // MGPipeCallFlags::kVarTail, so an encoder that copied a call's flags + // into this framing field verbatim would have every var-tail record + // skipped HERE, silently, with the record lost and nothing logged on + // either side. A genuine filler is always kind kRingPadRecordKind - + // Reserve writes it two dozen lines above - and a call record always + // carries a real opcode, because the catalogue starts at 1. Requiring + // the pair costs one comparison and turns that collision from a lost + // record into a record the decoder gets and can reject by name. + if ((header.flags & kRecPad) != 0 && header.kind == kRingPadRecordKind) { m_localTail += size; continue; } @@ -405,11 +414,10 @@ namespace MobileGL::MG_Remote::Transport { // SessionProducer // ----------------------------------------------------------------------- - void SessionProducer::Attach(RingControl* control, RingProducer* cmd, RingProducer* stage, - Doorbell* peerBell, Doorbell* selfBell, std::uint32_t spinUs) { + void SessionProducer::Attach(RingControl* control, RingProducer* cmd, Doorbell* peerBell, + Doorbell* selfBell, std::uint32_t spinUs) { m_control = control; m_cmd = cmd; - m_stage = stage; m_peerBell = peerBell; m_selfBell = selfBell; m_spinUs = spinUs; @@ -418,7 +426,6 @@ namespace MobileGL::MG_Remote::Transport { void SessionProducer::Detach() { m_control = nullptr; m_cmd = nullptr; - m_stage = nullptr; m_peerBell = nullptr; m_selfBell = nullptr; m_lastPublishedSeq = 0; @@ -428,11 +435,9 @@ namespace MobileGL::MG_Remote::Transport { if (!Valid()) { return; } - // 1. the records themselves. + // 1. the records themselves. ONLY SEG_CMD: SEG_STAGE is not a ring and + // RingCursorSet::Stage is driven by nobody (Ring.h's stage triple). m_cmd->Publish(); - if (m_stage != nullptr) { - m_stage->Publish(); - } // Kept locally as well as on the shared page: the shared watermark is // allowed to lag (Ring.h:72-77), and teardown's drain must not. // @@ -493,14 +498,6 @@ namespace MobileGL::MG_Remote::Transport { return Park([cmd, bytes] { return cmd->FreeBytes() >= bytes; }, timeoutMs); } - SessionWait SessionProducer::WaitForStageSpace(std::uint64_t bytes, std::uint32_t timeoutMs) { - if (!Valid() || m_stage == nullptr) { - return SessionWait::TimedOut; - } - RingProducer* stage = m_stage; - return Park([stage, bytes] { return stage->FreeBytes() >= bytes; }, timeoutMs); - } - // ----------------------------------------------------------------------- // SessionConsumer // ----------------------------------------------------------------------- @@ -560,6 +557,19 @@ namespace MobileGL::MG_Remote::Transport { NotifyClient(); } + void SessionConsumer::NoteBorrowedRecord(std::uint16_t kind, std::uint16_t flags) { + ++m_borrowedSeen; + if (m_borrowedSeen == 1) { + MGLOG_E("MG_Remote ring: record kind %u carries kRecBorrowSlot (flags=0x%04X). P5 " + "implements NO borrowed slots, and that bit is also MGPipeCallFlags::" + "kHostSpan, which P5's reduced path is ruled to produce none of either. " + "Nothing past this record will be reclaimed until RetireBorrowedUpTo " + "releases it, so a producer that then wedges on a full ring is THIS line's " + "fault and not the ring's", + static_cast(kind), static_cast(flags)); + } + } + void SessionConsumer::RetireBorrowedUpTo(std::uint64_t cursor) { if (!Valid()) { return; diff --git a/MobileGL/MG_Remote/Transport/Ring.h b/MobileGL/MG_Remote/Transport/Ring.h index d6be4af0..967cbbf0 100644 --- a/MobileGL/MG_Remote/Transport/Ring.h +++ b/MobileGL/MG_Remote/Transport/Ring.h @@ -103,7 +103,30 @@ namespace MobileGL::MG_Remote::Transport { alignas(64) std::atomic cmdAppliedTail; // consumer: bytes decoded/copied out std::atomic cmdRetiredTail; // consumer: borrowed slots released - // ---- SEG_STAGE cursors ---------------------------------------------- + // ---- SEG_STAGE cursors ------------------------------------------------ + // + // DEAD IN P5, DELIBERATELY, AND NOBODY MAY WIRE THEM UP HALFWAY. + // + // SEG_STAGE is NOT a ring any more. Package w1's encoder owns staging as + // an ENCODER-LOCAL LINEAR ALLOCATOR: a staged byte run carries no + // RingRecordHeader, there is no consumer walking SEG_STAGE, and the + // allocator reclaims on `retiredSeq` - the sequence watermark below - + // rather than on these three cursors. So all three stay ZERO for the + // whole of P5, `RingCursorSet::Stage` has no producer and no consumer, + // and `SessionTest.TheStageCursorTripleStaysDeadAcrossAWholeSession` + // pins that rather than leaving it to be noticed. + // + // They are kept rather than deleted because RingCursorSet, the three + // cursor accessors in Ring.cpp and RingTest's fixture are all written + // against a two-triple page, and P8/P11's shadow and adopt segments are + // the ring-shaped users this triple was reserved for. What is NOT + // acceptable is the middle state: a producer publishing `stageHead` with + // nothing advancing the two tails makes FreeBytes() fall to zero the + // first time the head laps the capacity and never recover, which is a + // guaranteed hang rather than a slow path. Five watermarks already spent + // a whole phase declared-and-written-by-nobody; this is the sixth, and + // it is declared-and-written-by-nobody ON PURPOSE, which is only + // different if it is written down. alignas(64) std::atomic stageHead; alignas(64) std::atomic stageAppliedTail; std::atomic stageRetiredTail; @@ -140,6 +163,33 @@ namespace MobileGL::MG_Remote::Transport { }; static_assert(sizeof(RingRecordHeader) == 8, "RecHeader is 8 bytes on the wire"); + // THESE ARE RING FLAGS AND THEY ARE NOT MGPipeCallFlags, AND THREE OF THE + // BITS COLLIDE WITH A DIFFERENT MEANING. `MGPipeCallFlags` (MG_Pipe/MGPipe.h: + // 42-54) is a SEPARATE SPACE that happens to overlap this one, and an encoder + // that copies `MGPipeCallFlagsFor(op)` into RingRecordHeader::flags without + // translating puts a call's bits into a framing field: + // + // bit 0 kNeedsAck == kRecNeedsAck same meaning, harmless + // bit 1 kHasBlob == kRecHasBlob same meaning, harmless + // bit 2 kVarTail == kRecPad WORST: a var-tail record would read + // as a WRAP FILLER and be skipped + // silently by Pop, losing the record + // with nothing logged anywhere + // bit 3 kHostSpan == kRecBorrowSlot a host-span record would read as + // borrowed into the GPU timeline, and + // the consumer would stop reclaiming + // ring bytes behind it for ever + // bit 4 kReplySlot == kRecVarTail a blocking call would read as having + // a tail it does not have + // bit 5 kOptional == (unused here) + // + // Translating is the ENCODER's job. Two things on this side make the first + // two of those survivable anyway rather than trusting it: `Pop` requires a + // filler to carry BOTH kRecPad AND kind == kRingPadRecordKind, so a real + // record with bit 2 set is delivered rather than eaten (a call record always + // has a real opcode kind, the catalogue starts at 1); and SessionConsumer + // counts and NAMES every kRecBorrowSlot it sees, because P5 produces no + // borrowed slots at all and the bit arriving means the collision did. enum RingRecordFlags : std::uint16_t { kRecNone = 0, kRecNeedsAck = 1u << 0, diff --git a/MobileGL/MG_Remote/Transport/SessionRings.h b/MobileGL/MG_Remote/Transport/SessionRings.h index 381bdc7b..4a78f2ef 100644 --- a/MobileGL/MG_Remote/Transport/SessionRings.h +++ b/MobileGL/MG_Remote/Transport/SessionRings.h @@ -79,10 +79,15 @@ namespace MobileGL::MG_Remote::Transport { // CONTRACT-P5's; MOBILEGL_IPC_RING_MB / MOBILEGL_IPC_STAGE_MB move the first // two, and ServerSession applies them unless SetSegmentSizes overrode them. struct SessionSegmentSizes { - std::uint64_t CmdRingBytes = 8ull * 1024 * 1024; // + one control page - std::uint64_t StageRingBytes = 32ull * 1024 * 1024; // no control page - std::uint64_t ReplyBytes = 8ull * 1024 * 1024; // not a ring - std::uint64_t EventRingBytes = 256ull * 1024; // + one control page + std::uint64_t CmdRingBytes = 8ull * 1024 * 1024; // + one control page + // SEG_STAGE IS NOT A RING. Package w1's encoder owns it as an + // encoder-local LINEAR ALLOCATOR that reclaims on retiredSeq, so this is + // a plain byte count: no control page, and no rounding down to a power of + // two either. Rounding was a ring requirement and keeping it would have + // silently turned an operator's MOBILEGL_IPC_STAGE_MB=24 into 16. + std::uint64_t StageBytes = 32ull * 1024 * 1024; + std::uint64_t ReplyBytes = 8ull * 1024 * 1024; // slot pool, not a ring + std::uint64_t EventRingBytes = 256ull * 1024; // + one control page std::uint32_t ReplySlotCount = kDefaultReplySlotCount; }; @@ -155,8 +160,11 @@ namespace MobileGL::MG_Remote::Transport { void* CmdRingBase() const { return m_cmdRingBase; } std::uint64_t CmdRingCapacity() const { return m_cmdRingCapacity; } + // The whole SEG_STAGE mapping, for w1's linear allocator and for the + // SegmentTable view the decoder resolves blobrefs against. There is no + // stage RING and no RingProducer/RingConsumer over RingCursorSet::Stage. void* StageBase() const { return m_stageBase; } - std::uint64_t StageCapacity() const { return m_stageCapacity; } + std::uint64_t StageBytes() const { return m_stageBytes; } void* ReplyBase() const { return m_replyBase; } std::uint64_t ReplyBytes() const { return m_replyBytes; } @@ -185,7 +193,7 @@ namespace MobileGL::MG_Remote::Transport { void* m_cmdRingBase = nullptr; std::uint64_t m_cmdRingCapacity = 0; void* m_stageBase = nullptr; - std::uint64_t m_stageCapacity = 0; + std::uint64_t m_stageBytes = 0; void* m_replyBase = nullptr; std::uint64_t m_replyBytes = 0; std::uint32_t m_replySlotCount = kDefaultReplySlotCount; @@ -276,8 +284,14 @@ namespace MobileGL::MG_Remote::Transport { // `peerBell` is the bell the SERVER parks on and this side rings; // `selfBell` is this side's own. InProcessTransport::PeerDoorbell() and // SelfDoorbell() are exactly that pair, from the client endpoint. - void Attach(RingControl* control, RingProducer* cmd, RingProducer* stage, Doorbell* peerBell, - Doorbell* selfBell, std::uint32_t spinUs); + // NO STAGE RING. SEG_STAGE is w1's encoder-local linear allocator and + // RingCursorSet::Stage has no producer and no consumer in P5 - see + // RingControl's stage triple in Ring.h. A producer here would publish + // stageHead with nothing advancing the two tails, so FreeBytes() would + // fall to zero the first time the head lapped the capacity and never + // recover: a guaranteed hang, not a slow path. + void Attach(RingControl* control, RingProducer* cmd, Doorbell* peerBell, Doorbell* selfBell, + std::uint32_t spinUs); void Detach(); bool Valid() const { return m_control != nullptr && m_cmd != nullptr; } @@ -310,11 +324,9 @@ namespace MobileGL::MG_Remote::Transport { // enough free bytes can only mean "too big, chunk", and waiting on it // stalls forever. SessionWait WaitForCmdSpace(std::uint64_t bytes, std::uint32_t timeoutMs); - SessionWait WaitForStageSpace(std::uint64_t bytes, std::uint32_t timeoutMs); RingControl* Control() const { return m_control; } RingProducer* Cmd() const { return m_cmd; } - RingProducer* Stage() const { return m_stage; } Doorbell* PeerDoorbell() const { return m_peerBell; } Doorbell* SelfDoorbell() const { return m_selfBell; } std::uint32_t SpinUs() const { return m_spinUs; } @@ -325,7 +337,6 @@ namespace MobileGL::MG_Remote::Transport { RingControl* m_control = nullptr; RingProducer* m_cmd = nullptr; - RingProducer* m_stage = nullptr; Doorbell* m_peerBell = nullptr; Doorbell* m_selfBell = nullptr; std::uint32_t m_spinUs = kDefaultSpinUs; @@ -386,6 +397,17 @@ namespace MobileGL::MG_Remote::Transport { if ((view.flags & kRecBorrowSlot) == 0 && !m_borrowHeld) { m_retirableCursor = view.cursor + sizeof(RingRecordHeader) + view.payloadSize; } else { + if ((view.flags & kRecBorrowSlot) != 0) { + // P5 PRODUCES NO BORROWED SLOTS AT ALL, so this bit arriving + // is either a borrow nobody implemented or MGPipeCallFlags:: + // kHostSpan wearing kRecBorrowSlot's bit (Ring.h's collision + // table) - and P5's reduced path is ruled to produce zero + // host spans too. Either way the conservative arm is taken + // (nothing past it is reclaimed) and the sighting is NAMED, + // because the alternative is a producer that wedges on the + // first full ring with no line anywhere saying why. + NoteBorrowedRecord(view.kind, view.flags); + } m_borrowHeld = true; } NotifyClient(); @@ -425,6 +447,9 @@ namespace MobileGL::MG_Remote::Transport { void NotifyClient(); std::uint64_t AppliedSeq() const { return m_appliedSeq; } + // How many kRecBorrowSlot records this consumer has seen. Non-zero in P5 + // is a finding, not a statistic. + std::uint64_t BorrowedRecordsSeen() const { return m_borrowedSeen; } RingControl* Control() const { return m_control; } RingConsumer* Cmd() const { return m_cmd; } Doorbell* PeerDoorbell() const { return m_peerBell; } @@ -439,7 +464,12 @@ namespace MobileGL::MG_Remote::Transport { std::uint32_t m_spinUs = kDefaultSpinUs; std::uint64_t m_appliedSeq = 0; std::uint64_t m_retirableCursor = 0; + std::uint64_t m_borrowedSeen = 0; bool m_borrowHeld = false; + + // Out of line so ApplyOne, which is a template in a header this layer + // keeps free of MobileGL/Includes.h, can still log. + void NoteBorrowedRecord(std::uint16_t kind, std::uint16_t flags); }; // ----------------------------------------------------------------------- diff --git a/MobileGL/MG_Remote/Transport/ShmSegment.cpp b/MobileGL/MG_Remote/Transport/ShmSegment.cpp index e07be3ea..3ec969ae 100644 --- a/MobileGL/MG_Remote/Transport/ShmSegment.cpp +++ b/MobileGL/MG_Remote/Transport/ShmSegment.cpp @@ -201,7 +201,7 @@ namespace MobileGL::MG_Remote::Transport { // two grows. const Spec specs[kSlotCount] = { {"mgl-cmd", SegmentBytesForRing(sizes.CmdRingBytes)}, - {"mgl-stage", LargestPowerOfTwoAtMost(sizes.StageRingBytes)}, + {"mgl-stage", sizes.StageBytes}, {"mgl-reply", sizes.ReplyBytes}, {"mgl-event", SegmentBytesForRing(sizes.EventRingBytes)}, }; @@ -341,10 +341,11 @@ namespace MobileGL::MG_Remote::Transport { m_cmdRingBase = cmdBase + sizeof(RingControl); m_cmdRingCapacity = RingCapacityForSegment(m_segments[0]->Size()); - // SEG_STAGE carries no control page of its own: RingControl holds TWO - // cursor triples and the stage triple is the second (Ring.h:106-109). + // SEG_STAGE IS NOT A RING: no control page, no cursor triple, no power-of- + // two rounding. Package w1's encoder owns it as a linear allocator that + // reclaims on retiredSeq, so the whole mapping is usable bytes. m_stageBase = m_segments[1]->Data(); - m_stageCapacity = LargestPowerOfTwoAtMost(m_segments[1]->Size()); + m_stageBytes = m_segments[1]->Size(); m_replyBase = m_segments[2]->Data(); m_replyBytes = m_segments[2]->Size(); @@ -360,14 +361,14 @@ namespace MobileGL::MG_Remote::Transport { m_mappedBytes += m_segments[index]->Size(); } - if (m_cmdRingCapacity == 0 || m_stageCapacity == 0 || m_eventRingCapacity == 0 || + if (m_cmdRingCapacity == 0 || m_stageBytes == 0 || m_eventRingCapacity == 0 || m_replyBytes == 0) { MGLOG_E("MG_Remote session: segment sizes leave no usable ring (cmd cap=%llu stage " "cap=%llu event cap=%llu reply=%llu). A ring is the largest POWER OF TWO that " "fits after the 4096 byte control page, so a segment must be strictly larger " "than one page plus the smallest ring", static_cast(m_cmdRingCapacity), - static_cast(m_stageCapacity), + static_cast(m_stageBytes), static_cast(m_eventRingCapacity), static_cast(m_replyBytes)); return; @@ -392,7 +393,7 @@ namespace MobileGL::MG_Remote::Transport { m_cmdRingBase = nullptr; m_cmdRingCapacity = 0; m_stageBase = nullptr; - m_stageCapacity = 0; + m_stageBytes = 0; m_replyBase = nullptr; m_replyBytes = 0; m_eventControl = nullptr; diff --git a/MobileGL/MG_Test/Wire/SessionTest.cpp b/MobileGL/MG_Test/Wire/SessionTest.cpp index c0de8e7c..e294d892 100644 --- a/MobileGL/MG_Test/Wire/SessionTest.cpp +++ b/MobileGL/MG_Test/Wire/SessionTest.cpp @@ -53,7 +53,7 @@ namespace { // RING sizes. The SEG_CMD and SEG_EVENT segments are each one control page bigger. SessionSegmentSizes sizes; sizes.CmdRingBytes = 32ull * 1024; - sizes.StageRingBytes = 64ull * 1024; + sizes.StageBytes = 48ull * 1024; sizes.ReplyBytes = 64ull * 1024; // -> 8 slots of 8 KiB sizes.EventRingBytes = 16ull * 1024; sizes.ReplySlotCount = 8; @@ -69,7 +69,8 @@ namespace { SessionSegments serverSegments; SessionSegments clientSegments; RingProducer cmdProducer; - RingProducer stageProducer; + // NO stageProducer and NO stage consumer: SEG_STAGE is w1's encoder-local linear + // allocator and RingCursorSet::Stage is driven by nobody (Ring.h's stage triple). RingConsumer cmdConsumer; SessionProducer producer; SessionConsumer consumer; @@ -93,18 +94,15 @@ namespace { RingControl* serverControl = serverSegments.CmdControl(); cmdProducer = RingProducer(clientControl, clientSegments.CmdRingBase(), clientSegments.CmdRingCapacity(), RingCursorSet::Cmd); - stageProducer = RingProducer(clientControl, clientSegments.StageBase(), - clientSegments.StageCapacity(), RingCursorSet::Stage); cmdConsumer = RingConsumer(serverControl, serverSegments.CmdRingBase(), serverSegments.CmdRingCapacity(), RingCursorSet::Cmd); - if (!cmdProducer.Valid() || !stageProducer.Valid() || !cmdConsumer.Valid()) { + if (!cmdProducer.Valid() || !cmdConsumer.Valid()) { return false; } // PeerDoorbell is the bell the OTHER end parks on; SelfDoorbell is this end's own. // Which is which is the session's knowledge, never ITransport's (contract §3.9). - producer.Attach(clientControl, &cmdProducer, &stageProducer, - &clientTransport->PeerDoorbell(), &clientTransport->SelfDoorbell(), - kDefaultSpinUs); + producer.Attach(clientControl, &cmdProducer, &clientTransport->PeerDoorbell(), + &clientTransport->SelfDoorbell(), kDefaultSpinUs); consumer.Attach(serverControl, &cmdConsumer, &serverTransport->PeerDoorbell(), &serverTransport->SelfDoorbell(), kDefaultSpinUs); replies = ReplySlotPool(serverSegments.ReplyBase(), serverSegments.ReplyBytes(), @@ -199,14 +197,14 @@ TEST(SessionTest, TheDefaultGeometryIsTheFourContractRingSizes) { SessionSegments segments; ASSERT_EQ(segments.Create(SessionSegmentSizes{}, MemoryRole::Server), MOBILEGL_OK); EXPECT_EQ(segments.CmdRingCapacity(), 8ull * 1024 * 1024); - EXPECT_EQ(segments.StageCapacity(), 32ull * 1024 * 1024); + EXPECT_EQ(segments.StageBytes(), 32ull * 1024 * 1024); EXPECT_EQ(segments.ReplyBytes(), 8ull * 1024 * 1024); EXPECT_EQ(segments.EventRingCapacity(), 256ull * 1024); EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Cmd), 8ull * 1024 * 1024 + sizeof(RingControl)); - // SEG_STAGE carries no control page: RingControl holds both cursor triples, so the whole - // segment is ring and 32 MiB is already a power of two. SEG_REPLY is not a ring at all. + // SEG_STAGE is not a ring at all - no control page, no cursor triple, no power-of-two + // rounding - and neither is SEG_REPLY, so both announce exactly what was asked for. EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Stage), 32ull * 1024 * 1024); EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Reply), 8ull * 1024 * 1024); EXPECT_EQ(segments.AnnouncedSize(SessionSegmentSlot::Event), @@ -973,6 +971,102 @@ TEST(SessionTest, CapsSnapshotFieldIdsAreFrozenAndTheRetiredSlotsStayBurned) { EXPECT_EQ(static_cast(::MobileGL::Wire::Welcome::VT_ABIFINGERPRINT), 20); } +// --------------------------------------------------------------------------- +// Fix round 2 - SEG_STAGE stopped being a ring, and the flag-space collisions +// --------------------------------------------------------------------------- + +// Package w1's encoder owns SEG_STAGE as a LINEAR ALLOCATOR that reclaims on retiredSeq: a staged +// byte run carries no RingRecordHeader and nothing walks SEG_STAGE. So RingCursorSet::Stage has no +// producer and no consumer, and all three of its cursors stay ZERO for the whole of a session. +// +// This case exists because "declared and written by nobody" is exactly the state the five sequence +// watermarks were in for a whole phase before this one. The stage triple is in that state ON +// PURPOSE now, and the difference between "on purpose" and "forgotten" is that one of them is +// pinned. A half-wiring - a producer publishing stageHead with nothing advancing the two tails - +// would make FreeBytes() fall to zero on the first lap and never recover: a hang, not a slow path. +TEST(SessionTest, TheStageCursorTripleStaysDeadAcrossAWholeSession) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + + // SEG_STAGE is still mapped and still usable bytes - w1's allocator needs them, and the + // decoder resolves blobrefs against the same view - it is just not a ring. + ASSERT_NE(session.serverSegments.StageBase(), nullptr); + EXPECT_EQ(session.serverSegments.StageBytes(), 48ull * 1024) + << "SEG_STAGE is no longer rounded down to a power of two; that was a ring requirement " + "and keeping it would silently turn MOBILEGL_IPC_STAGE_MB=24 into 16"; + + // Drive a full session's worth of traffic through SEG_CMD. + constexpr int kRecords = 500; + int applied = 0; + for (int index = 0; index < kRecords; ++index) { + void* payload = nullptr; + while ((payload = session.cmdProducer.Reserve(1, kRecNone, 32)) == nullptr) { + while (session.consumer.ApplyOne([&](const RingRecordView&) { ++applied; })) { + } + session.consumer.RetireThrough(session.consumer.AppliedSeq()); + } + std::memset(payload, index & 0xFF, 32); + session.producer.PublishAndNotify(static_cast(index + 1)); + } + while (session.consumer.ApplyOne([&](const RingRecordView&) { ++applied; })) { + } + session.consumer.RetireThrough(session.consumer.AppliedSeq()); + ASSERT_EQ(applied, kRecords); + + // The command triple moved. The stage triple did not, and nothing in the session touches it. + EXPECT_GT(session.Control().cmdHead.load(), 0u); + EXPECT_EQ(session.Control().stageHead.load(), 0u); + EXPECT_EQ(session.Control().stageAppliedTail.load(), 0u); + EXPECT_EQ(session.Control().stageRetiredTail.load(), 0u); + // retiredSeq is the watermark w1's allocator reclaims behind, and it is a SEQUENCE - not one + // of the three byte cursors above. + EXPECT_GT(session.Control().retiredSeq.load(), 0u); +} + +// The kVarTail/kRecPad collision, made harmless. Those two are bit 2 of two different flag spaces, +// so an encoder that copied MGPipeCallFlagsFor(op) into RingRecordHeader::flags verbatim would +// have every var-tail record SKIPPED by Pop, silently, with the record lost and nothing logged on +// either side. Pop now requires a filler to carry both the flag AND kind == kRingPadRecordKind, so +// a real record wearing that bit is delivered instead of eaten. +TEST(SessionTest, ARecordWearingThePadBitIsDeliveredRatherThanEatenAsAFiller) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + + // kRecPad is MGPipeCallFlags::kVarTail's bit. Kind 7 is a real opcode, not a filler. + void* payload = session.cmdProducer.Reserve(7, kRecPad, 16); + ASSERT_NE(payload, nullptr); + std::memset(payload, 0xAB, 16); + session.producer.PublishAndNotify(1); + + int seen = 0; + std::uint16_t seenKind = 0; + while (session.consumer.ApplyOne([&](const RingRecordView& view) { + seenKind = view.kind; + ++seen; + })) { + } + EXPECT_EQ(seen, 1) << "the record was skipped as a wrap filler because it wore bit 2"; + EXPECT_EQ(seenKind, 7); + EXPECT_EQ(session.Control().appliedSeq.load(), 1u); +} + +// The kHostSpan/kRecBorrowSlot collision, made loud. P5 implements no borrowed slots and is ruled +// to produce no host spans either, so the bit arriving means the collision did - and the +// conservative arm it takes (stop reclaiming) would otherwise wedge the producer on the first full +// ring with nothing in any log saying why. +TEST(SessionTest, ABorrowSlotSightingIsCountedRatherThanJustActedOn) { + SessionFixture session; + ASSERT_TRUE(session.Build(TestSizes())); + EXPECT_EQ(session.consumer.BorrowedRecordsSeen(), 0u); + + ASSERT_NE(session.cmdProducer.Reserve(3, kRecBorrowSlot, 16), nullptr); + session.producer.PublishAndNotify(1); + while (session.consumer.ApplyOne([](const RingRecordView&) {})) { + } + EXPECT_EQ(session.consumer.BorrowedRecordsSeen(), 1u) + << "a kRecBorrowSlot record went by unnamed; in P5 that bit can only be kHostSpan"; +} + // m-4. The reply pool refuses a geometry whose slots would not be 8-aligned: the fences order // the payload against the stamp, but the stamp's own 8-byte Seq has to be untorn for the // wrong-slot self-check to mean anything, and that is only true while it is naturally aligned.