swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-17 00:28:31 +09:00
Code Issues Packages Projects Releases Wiki Activity

[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

Browse Source
This commit is contained in:
swung0x48
2026-09-11 15:04:54 -04:00
parent 1f953c09ea
commit c5c12b489a
2 changed files with 109 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Remote/Server/ServerSession.cpp
+71 -46
View File
@@ -16,8 +16,6 @@
#include <Config.h>
#include <MGGitHash.h>
#include <MG_Pipe/PipeApply.h>
#include <MG_Pipe/PipeMutation.h>
#include <MG_Util/Debug/Log.h>
#include <cstdlib>
@@ -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<Uint64>(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 {
MobileGL/MG_Remote/Server/ServerSession.h
+38 -13
View File
@@ -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;
};