[MG_Pipe, MG_Remote, MG_Test] (Disaggregated): P5f fv - own reverse errors and reject legacy object announcements

This commit is contained in:
2026-09-19 23:38:55 -04:00
parent 4667e13be6
commit 4f3d2d6b95
5 changed files with 236 additions and 40 deletions
+10 -6
View File
@@ -52,9 +52,9 @@
// P5c gt (CONTRACT-P5C §6 layer 2, audit A1): the client-side gPipeInputs check consults
// InBarrierWait() and ApplyThreadIsInsideApplier() - both live here.
#include <MG_Remote/Client/ClientSession.h>
// P5c ev (CONTRACT-P5C §4.2): RecordError's transport arm posts kEventGlError through the
// server session, and InvalidateCompileEnv's forward is deleted with an active transport.
#include <MG_Remote/Server/ServerSession.h>
// P5f fv: backend errors use the same owned reverse callback table as other events.
// The producer, rather than this client-side translation unit, knows the server session.
#include <MG_Pipe/MGPipeCallbacks.h>
#endif
#include <atomic>
@@ -2182,7 +2182,6 @@ namespace MobileGL::MG_Pipe {
}
void PipeInputs::RecordError(ErrorCode code, UniquePtr<ErrorInfo> info) {
MGP_STICKY_FORWARD_PULL(RecordError);
#if MOBILEGL_BUILD_DISAGGREGATED
if (MG_Config::Transport != MG_Config::TransportMode::Monolith) {
// The error queue is CLIENT state and the apply thread may not write it (R4).
@@ -2192,11 +2191,16 @@ namespace MobileGL::MG_Pipe {
// program order of error-then-read but not cross-verb interleaving - the
// accepted P5c shape, stated in the contract rather than discovered in P6.
const String message = info != nullptr ? info->toString() : String{};
MG_Remote::Server::ServerSessionInstance().PostGlError(static_cast<Uint32>(code),
message.c_str());
if (gMGPipeCallbacks.OnGlError == nullptr) {
MGLOG_F("MGPipe: Fatal{RoleViolation, \"OnGlError.callback-missing\"} - "
"a transport backend error has no reverse-channel owner");
std::abort();
}
gMGPipeCallbacks.OnGlError(static_cast<Uint32>(code), message.c_str());
return;
}
#endif
MGP_STICKY_FORWARD_PULL(RecordError);
auto* ctx = LiveContext();
if (ctx == nullptr) {
MGLOG_E_ONCE("PipeInputs::RecordError: no live context, dropping error %d", static_cast<int>(code));
+13 -29
View File
@@ -693,42 +693,26 @@ namespace MobileGL::MG_Pipe {
install(gMGPipeCallbacks.OnGpuWritten, &MGPipeClientOnGpuWritten, "OnGpuWritten");
}
// The GPU-write announcement for a buffer the backend holds only a FRONTEND POINTER to -
// Magma's barrier-pulled binding-point reads (UniformManager / VulkanRenderer), whose
// direct bufferObject->MarkGpuWritten() was the apply thread poking client memory (R2).
// Routed through the reverse channel exactly as DirectGLES' MarkBufferGpuWritten routes
// its own sites: ONE whole-buffer range, stated rather than implied (zero ranges is the
// shape a fully narrowed announcement will legitimately have at P8/P9, and the two must
// not be the same record). The handle comes from the client allocator's lifetime-id
// probe - the mint is unconditional at the BufferObject constructor
// (MGPipeMintResourceHandle), so a live buffer always has one.
//
// The fallback is the monolith shape, preserved byte for byte: no reverse channel
// installed (a pull-arm build, or a process with no session) pokes the object directly,
// which is exactly what these sites did before. A MISSING HANDLE with a live channel is
// loud rather than a silent drop, the same choice MarkBufferGpuWritten makes.
// Legacy-object entry for the monolith backend. Transport consumers carry handles
// and call OnGpuWritten directly; P5f fm rejects Magma's P7 buffer consumers before
// reaching here. Keep a named guard at this boundary so a future caller cannot turn
// a missing callback/handle into a write through a client object.
inline void MGPipeAnnounceBufferGpuWritten(
const SharedPtr<MG_State::GLState::BufferObject>& bufferObject) {
if (bufferObject == nullptr) return;
#if MOBILEGL_BUILD_DISAGGREGATED
if (MG_Config::Transport != MG_Config::TransportMode::Monolith) {
MGLOG_F("MGPipe: Fatal{RoleViolation, \"gpu-written-legacy-object\"} - "
"a transport producer must announce a record handle, never a client BufferObject");
std::abort();
}
#endif
if (gMGPipeCallbacks.OnGpuWritten == nullptr) {
bufferObject->MarkGpuWritten();
return;
}
const MGPipeHandle res = [&]() {
#if MOBILEGL_BUILD_DISAGGREGATED
// CONTRACT-P5C §3.1's named exemption, Magma's half: the binding records that
// would carry this buffer's handle are sb's to emit and Magma's server-side
// binding table is P7's, so until then the probe runs inside the scope - the
// debt's named, greppable form rather than a silent guard removal.
//
// P5e (id), ruling 12: the FOURTH of Magma's apply-thread allocator debts and the
// last user of the scope P5e renamed. Espryt never reaches this arm under a
// transport - MarkBufferGpuWrittenByHandle carries the handle from the record side
// (sb/fb) - so the DirectVulkan key on the exemption costs it nothing.
const MagmaP7AllocatorDebtScope magmaP7AllocatorDebt;
#endif
return MGPipeSlots().FindByLifetimeId(MGPipeKind::Buffer, bufferObject->GetLifetimeId());
}();
const MGPipeHandle res =
MGPipeSlots().FindByLifetimeId(MGPipeKind::Buffer, bufferObject->GetLifetimeId());
if (MGPipeHandleIsNull(res)) {
MGLOG_E_ONCE("MGPipe: no handle for the GPU-write announcement of buffer %u - the "
"reverse channel is installed but the mint is missing, so the mark "
+3 -2
View File
@@ -25,8 +25,9 @@
// an optimization (section 7.4).
namespace MobileGL::MG_Pipe {
struct MGPipeCallbacks {
// A driver-detected GL error that only the server could have seen.
void (*OnGlError)(Uint32 code);
// A driver-detected GL error that only the server could have seen. The text is
// borrowed for this synchronous call; a transport producer copies it before return.
void (*OnGlError)(Uint32 code, const char* message);
// Ranges of a resource the GPU wrote; retires MarkGpuWritten.
void (*OnGpuWritten)(MGPipeHandle res, Uint rangeCount, const MGPRange* ranges);
void (*OnBufferWriteback)(MGPipeHandle res, Uint64 offset, MGPBlobRef bytes);
+18 -3
View File
@@ -159,7 +159,7 @@ namespace MobileGL::MG_Remote::Server {
// ---- P5c ev: the reverse channel's PRODUCER callbacks (CONTRACT-P5C §4.1) --------
//
// With an active transport the three reverse entries of gMGPipeCallbacks belong to
// With an active transport the four reverse entries of gMGPipeCallbacks belong to
// the SERVER session, never to the client: the backend calls them with exactly the
// arguments it always passed (a writeback blobref whose Offset IS the backend's own
// mapped pointer, a range array, a surface info), and what the callback does with
@@ -329,6 +329,17 @@ namespace MobileGL::MG_Remote::Server {
session.PublishEvents();
}
// P5f fv: OnGlError now has the same owner and lifetime as the other producers.
// PostGlError copies message bytes synchronously into the existing FIFO event ring.
void ServerOnGlError(Uint32 code, const char* message) {
auto* session = ServerSession::Active();
if (session == nullptr) {
MGLOG_F("MGPipe: Fatal{RoleViolation, \"OnGlError.session-missing\"}");
std::abort();
}
session->PostGlError(code, message);
}
// One installer for both roles' tables, so the check exists in exactly one spelling:
// writing over an entry somebody else claimed is Fatal{RoleViolation,
// "callback-double-install"} - MGPipeCallbacks' "never over an entry a backend
@@ -339,7 +350,7 @@ namespace MobileGL::MG_Remote::Server {
if (entry != nullptr && entry != producer) {
MGLOG_F("MGPipe: Fatal{RoleViolation, \"callback-double-install\"} - a reverse "
"MGPipeCallbacks entry is already claimed by a different function. With "
"an active transport the three reverse entries are the server "
"an active transport the four reverse entries are the server "
"session's producers; the client installs them under monolith only");
std::abort();
}
@@ -529,12 +540,13 @@ namespace MobileGL::MG_Remote::Server {
m_accepted = true;
g_active = this;
// ---- P5c ev: the three reverse-channel producers are THIS session's (CONTRACT-P5C
// ---- P5c ev: the four reverse-channel producers are THIS session's (CONTRACT-P5C
// §4.1), installed before the apply thread can apply a record that produces one and
// uninstalled at Close after the join. Under monolith these entries are the
// client's (MGPipeInstallClientResourceCallbacks, monolith-only now); a session is
// by definition not monolith, so finding one of them claimed here is the double
// installation the check exists to name.
InstallReverseCallback(MG_Pipe::gMGPipeCallbacks.OnGlError, &ServerOnGlError);
InstallReverseCallback(MG_Pipe::gMGPipeCallbacks.OnBufferWriteback,
&ServerOnBufferWriteback);
InstallReverseCallback(MG_Pipe::gMGPipeCallbacks.OnGpuWritten, &ServerOnGpuWritten);
@@ -621,6 +633,9 @@ namespace MobileGL::MG_Remote::Server {
Wire::SegmentTable::UninstallProcessResolver();
// The reverse-channel producers go with the session that owns them - release
// only the entries that are still OURS, never one a later owner installed.
if (MG_Pipe::gMGPipeCallbacks.OnGlError == &ServerOnGlError) {
MG_Pipe::gMGPipeCallbacks.OnGlError = nullptr;
}
if (MG_Pipe::gMGPipeCallbacks.OnBufferWriteback == &ServerOnBufferWriteback) {
MG_Pipe::gMGPipeCallbacks.OnBufferWriteback = nullptr;
}
+192
View File
@@ -37,7 +37,9 @@
#include <MG_Backend/DirectGLES/Utils.h>
#include <MG_Backend/MGPipe/PipeInputs.h>
#include <MG_Impl/Pipe/PipeFill.h>
#include <MG_Impl/Pipe/ResourceTracker.h>
#include <MG_Pipe/MGPipe.h>
#include <MG_Pipe/MGPipeCallbacks.h>
#include <MG_Pipe/PipeApply.h>
#include <MG_Remote/CapsCodec.h>
#include <MG_Remote/Client/ClientSession.h>
@@ -54,6 +56,7 @@
#include <MG_Remote/Wire/PipeWireCodec.h>
#include <MG_State/GLState/BufferState/BufferObject.h>
#include <MG_State/GLState/Core.h>
#include <MG_State/GLState/ErrorState/ErrorInfo.h>
#include <MG_State/GLState/TextureState/TextureEnum.h>
#include <MGGitHash.h>
@@ -2262,6 +2265,195 @@ TEST(ServerLoopTest, AnIndirectDrawRecordReachesTheSinkWithItsBlockIntact) {
fixture.Stop();
}
namespace {
Uint32 g_fvErrorCode = 0;
String g_fvErrorMessage;
Uint32 g_fvErrorCalls = 0;
void CaptureFvError(Uint32 code, const char* message) {
g_fvErrorCode = code;
g_fvErrorMessage = message ? message : "";
++g_fvErrorCalls;
}
struct FvGlobals {
MG_Pipe::MGPipeCallbacks callbacks = MG_Pipe::gMGPipeCallbacks;
Bool strict = MG_Config::Ipc.StrictErrors;
Bool split = MG_Config::Ipc.RoleSplitState;
~FvGlobals() {
MG_Pipe::MGPipeServerClearVerbBoundary();
MG_Pipe::gMGPipeCallbacks = callbacks;
MG_Config::Ipc.StrictErrors = strict;
MG_Config::Ipc.RoleSplitState = split;
}
};
struct FvSession : ServerFixture {
~FvSession() { Stop(); }
};
}
TEST(P5fReverseChannel, GlErrorsUseTheOwnedCallbackWithoutAResidualPull) {
FvGlobals restore;
MG_Config::Ipc.StrictErrors = true;
MG_Config::Ipc.RoleSplitState = true;
MG_Pipe::gMGPipeCallbacks.OnGlError = &CaptureFvError;
g_fvErrorCalls = 0;
MG_Pipe::MGPipeResetResidualPullCountForTesting();
MG_Pipe::MGPipeServerStampVerbBoundary(MG_Pipe::MGPipeVerb::Clear);
MG_Pipe::gPipeInputs.RecordError(ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("fv", "draw", "driver message"));
EXPECT_EQ(g_fvErrorCalls, 1u);
EXPECT_EQ(g_fvErrorCode, static_cast<Uint32>(ErrorCode::InvalidOperation));
EXPECT_EQ(g_fvErrorMessage, "[fv] [draw] driver message");
EXPECT_EQ(MG_Pipe::MGPipeResidualPullCount(), 0u);
}
TEST(P5fReverseChannel, AcceptCloseAndReacceptOwnTheGlErrorCallback) {
FvGlobals restore;
MG_Pipe::gMGPipeCallbacks.OnGlError = nullptr;
FvSession fixture;
ASSERT_TRUE(fixture.Handshake());
const auto producer = MG_Pipe::gMGPipeCallbacks.OnGlError;
ASSERT_NE(producer, nullptr);
EXPECT_EQ(fixture.session->Accept(*fixture.serverTransport), MOBILEGL_ERR_INVALID_ARGUMENT);
EXPECT_EQ(MG_Pipe::gMGPipeCallbacks.OnGlError, producer);
fixture.Stop();
EXPECT_EQ(MG_Pipe::gMGPipeCallbacks.OnGlError, nullptr);
ASSERT_TRUE(fixture.Handshake());
EXPECT_EQ(MG_Pipe::gMGPipeCallbacks.OnGlError, producer);
fixture.Stop();
EXPECT_EQ(MG_Pipe::gMGPipeCallbacks.OnGlError, nullptr);
}
TEST(P5fReverseChannel, CloseDoesNotEraseAnotherGlErrorOwner) {
FvGlobals restore;
MG_Pipe::gMGPipeCallbacks.OnGlError = nullptr;
FvSession fixture;
ASSERT_TRUE(fixture.Handshake());
MG_Pipe::gMGPipeCallbacks.OnGlError = &CaptureFvError;
fixture.Stop();
EXPECT_EQ(MG_Pipe::gMGPipeCallbacks.OnGlError, &CaptureFvError);
}
#if !defined(_WIN32)
TEST(P5fReverseChannel, GlErrorCallbackRejectsDoubleInstallation) {
EXPECT_EXIT({
MG_Pipe::gMGPipeCallbacks.OnGlError = &CaptureFvError;
FvSession fixture;
(void)fixture.Handshake();
std::_Exit(9);
}, ::testing::KilledBySignal(SIGABRT), "");
EXPECT_NE(ReadLog().find("callback-double-install"), std::string::npos);
}
TEST(P5fReverseChannel, RecordErrorWithoutCallbackIsNamedFatal) {
EXPECT_EXIT({
MG_Config::Ipc.StrictErrors = true;
MG_Config::Ipc.RoleSplitState = true;
MG_Pipe::gMGPipeCallbacks.OnGlError = nullptr;
MG_Pipe::gPipeInputs.RecordError(ErrorCode::InvalidOperation, nullptr);
std::_Exit(9);
}, ::testing::KilledBySignal(SIGABRT), "");
EXPECT_NE(ReadLog().find("OnGlError.callback-missing"), std::string::npos);
}
TEST(P5fReverseChannel, LegacyGpuWrittenNeverReadsAFrontendObjectInTransport) {
for (Bool installed : {false, true}) {
EXPECT_EXIT({
MG_Pipe::gMGPipeCallbacks.OnGpuWritten = installed ?
+[](MG_Pipe::MGPipeHandle, Uint, const MG_Pipe::MGPRange*) {} : nullptr;
// Deliberately unreadable object: a guard after a lifetime-id probe or
// either MarkGpuWritten fallback would SIGSEGV instead of the named abort.
SharedPtr<MG_State::GLState::BufferObject> foreign(
reinterpret_cast<MG_State::GLState::BufferObject*>(std::uintptr_t{1}),
[](MG_State::GLState::BufferObject*) {});
MG_Pipe::MGPipeAnnounceBufferGpuWritten(foreign);
std::_Exit(9);
}, ::testing::KilledBySignal(SIGABRT), "");
}
EXPECT_NE(ReadLog().find("gpu-written-legacy-object"), std::string::npos);
}
#endif
TEST(P5fReverseChannel, GlErrorMessagesShareFifoWithOtherReverseEvents) {
FvGlobals restore;
MG_Config::Ipc.StrictErrors = true;
MG_Config::Ipc.RoleSplitState = true;
MG_Pipe::gMGPipeCallbacks.OnGlError = nullptr;
FvSession fixture;
ASSERT_TRUE(fixture.Handshake());
ASSERT_TRUE(fixture.StartLoop());
ASSERT_EQ(Server::ServerLoopInstance().RunProbeOnApplyThreadForTesting(
+[](void*) -> MobileGLResult {
MG_Pipe::MGPipeServerStampVerbBoundary(MG_Pipe::MGPipeVerb::Clear);
MG_Pipe::gPipeInputs.RecordError(ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("fv", "driver error"));
const MG_Pipe::MGPRange whole{0, MG_Pipe::kMGPipeWholeBuffer};
MG_Pipe::gMGPipeCallbacks.OnGpuWritten({17, 3}, 1, &whole);
Uint8 bytes[] = {11, 22, 33, 44};
MG_Pipe::gMGPipeCallbacks.OnBufferWriteback({17, 3}, 24,
MG_Pipe::MGPBlobRef{reinterpret_cast<Uint64>(bytes), sizeof(bytes),
MG_Pipe::kMGHostSpanSegNone, 0});
std::memset(bytes, 0xDD, sizeof(bytes));
String longMessage(Transport::kEventGlErrorMaxMessageBytes * 2, 'Q');
MG_Pipe::gMGPipeCallbacks.OnGlError(static_cast<Uint32>(ErrorCode::InvalidValue),
longMessage.c_str());
std::fill(longMessage.begin(), longMessage.end(), '?');
MG_Pipe::gMGPipeCallbacks.OnGlError(static_cast<Uint32>(ErrorCode::InvalidEnum), nullptr);
MG_Pipe::MGPipeServerClearVerbBoundary();
return MOBILEGL_OK;
}, nullptr), MOBILEGL_OK);
Transport::EventRingConsumer events(fixture.clientSegments.EventControl(),
fixture.clientSegments.CmdControl(), fixture.clientSegments.EventRingBase(),
fixture.clientSegments.EventRingCapacity(), fixture.clientSegments.EventSegmentBase());
ASSERT_TRUE(events.Valid());
const Uint16 kinds[] = {Transport::kEventGlError, Transport::kEventGpuWritten,
Transport::kEventBufferWriteback, Transport::kEventGlError, Transport::kEventGlError};
for (SizeT i = 0; i < 5; ++i) {
Transport::RingRecordView event;
ASSERT_TRUE(events.Pop(event)) << i;
EXPECT_EQ(event.kind, kinds[i]) << i;
if (event.kind == Transport::kEventGlError) {
const auto* head = static_cast<const Transport::EventGlErrorHead*>(event.payload);
ASSERT_GE(event.payloadSize, sizeof(*head) + head->MessageBytes);
ASSERT_GT(head->MessageBytes, 0u);
const auto* text = reinterpret_cast<const char*>(head + 1);
EXPECT_EQ(text[head->MessageBytes - 1], '\0');
const String message(text, head->MessageBytes - 1);
if (i == 0) {
EXPECT_EQ(head->Code, static_cast<Uint32>(ErrorCode::InvalidOperation));
EXPECT_EQ(message, "[fv] driver error");
} else if (i == 3) {
EXPECT_EQ(head->Code, static_cast<Uint32>(ErrorCode::InvalidValue));
EXPECT_EQ(head->MessageBytes, Transport::kEventGlErrorMaxMessageBytes);
EXPECT_EQ(message, String(Transport::kEventGlErrorMaxMessageBytes - 1, 'Q'));
} else {
EXPECT_EQ(head->Code, static_cast<Uint32>(ErrorCode::InvalidEnum));
EXPECT_EQ(head->MessageBytes, 1u);
EXPECT_TRUE(message.empty());
}
} else if (event.kind == Transport::kEventGpuWritten) {
const auto* head = static_cast<const Transport::EventGpuWrittenHead*>(event.payload);
EXPECT_EQ(head->Resource.Slot, 17u);
EXPECT_EQ(head->Resource.Gen, 3u);
EXPECT_EQ(head->RangeCount, 1u);
const auto* range = reinterpret_cast<const Transport::EventRange*>(head + 1);
EXPECT_EQ(range->Size, MG_Pipe::kMGPipeWholeBuffer);
} else {
const auto* head = static_cast<const Transport::EventBufferWritebackHead*>(event.payload);
EXPECT_EQ(head->Offset, 24u);
EXPECT_EQ(head->Size, 4u);
const Uint8 expected[] = {11, 22, 33, 44};
EXPECT_EQ(std::memcmp(head + 1, expected, sizeof(expected)), 0);
}
}
Transport::RingRecordView extra;
EXPECT_FALSE(events.Pop(extra));
events.Drained();
}
int main(int argc, char** argv) {
// Before anything logs: MG_Util::Debug::InitFile() reads the variable once, on the first
// write, and caches the FILE*. The name carries this process's pid, because