[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

This commit is contained in:
2026-09-11 15:41:56 -04:00
parent 3f1eaee787
commit f33e5d6090
8 changed files with 273 additions and 57 deletions
+16 -7
View File
@@ -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();
@@ -149,7 +149,6 @@ namespace MobileGL::MG_Remote::Client {
std::unique_ptr<Transport::InProcessTransport> 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;
+25 -2
View File
@@ -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<Uint16>(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<Uint16>(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<Uint16>(MG_Pipe::kReplySlot) == Transport::kRecVarTail,
"MGPipeCallFlags::kReplySlot and kRecVarTail share bit 4");
static_assert(static_cast<Uint16>(MG_Pipe::kNeedsAck) == Transport::kRecNeedsAck &&
static_cast<Uint16>(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(),
+27 -17
View File
@@ -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<unsigned>(kind), static_cast<unsigned>(flags));
}
}
void SessionConsumer::RetireBorrowedUpTo(std::uint64_t cursor) {
if (!Valid()) {
return;
+51 -1
View File
@@ -103,7 +103,30 @@ namespace MobileGL::MG_Remote::Transport {
alignas(64) std::atomic<std::uint64_t> cmdAppliedTail; // consumer: bytes decoded/copied out
std::atomic<std::uint64_t> 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<std::uint64_t> stageHead;
alignas(64) std::atomic<std::uint64_t> stageAppliedTail;
std::atomic<std::uint64_t> 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,
+39 -9
View File
@@ -80,8 +80,13 @@ namespace MobileGL::MG_Remote::Transport {
// 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
// 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);
};
// -----------------------------------------------------------------------
+8 -7
View File
@@ -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<unsigned long long>(m_cmdRingCapacity),
static_cast<unsigned long long>(m_stageCapacity),
static_cast<unsigned long long>(m_stageBytes),
static_cast<unsigned long long>(m_eventRingCapacity),
static_cast<unsigned long long>(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;
+105 -11
View File
@@ -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<int>(::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<std::uint64_t>(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.