From 4f3d2d6b95c17062190260ef9c5e40d7da9fac63 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sat, 19 Sep 2026 23:38:55 -0400 Subject: [PATCH 1/4] [MG_Pipe, MG_Remote, MG_Test] (Disaggregated): P5f fv - own reverse errors and reject legacy object announcements --- MobileGL/MG_Impl/Pipe/PipeFill.cpp | 16 +- MobileGL/MG_Impl/Pipe/ResourceTracker.h | 42 ++--- MobileGL/MG_Pipe/MGPipeCallbacks.h | 5 +- MobileGL/MG_Remote/Server/ServerSession.cpp | 21 ++- MobileGL/MG_Test/Wire/ServerLoopTest.cpp | 192 ++++++++++++++++++++ 5 files changed, 236 insertions(+), 40 deletions(-) diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index 07f1cea7..e3ec15db 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -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 -// 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 +// 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 #endif #include @@ -2182,7 +2182,6 @@ namespace MobileGL::MG_Pipe { } void PipeInputs::RecordError(ErrorCode code, UniquePtr 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(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(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(code)); diff --git a/MobileGL/MG_Impl/Pipe/ResourceTracker.h b/MobileGL/MG_Impl/Pipe/ResourceTracker.h index d6870ea1..28ca4305 100644 --- a/MobileGL/MG_Impl/Pipe/ResourceTracker.h +++ b/MobileGL/MG_Impl/Pipe/ResourceTracker.h @@ -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& 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 " diff --git a/MobileGL/MG_Pipe/MGPipeCallbacks.h b/MobileGL/MG_Pipe/MGPipeCallbacks.h index f379acea..0ebe2db4 100644 --- a/MobileGL/MG_Pipe/MGPipeCallbacks.h +++ b/MobileGL/MG_Pipe/MGPipeCallbacks.h @@ -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); diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index f8734b58..e1a66c9d 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/MobileGL/MG_Remote/Server/ServerSession.cpp @@ -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; } diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index 4d062139..bdcd0e24 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -37,7 +37,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -54,6 +56,7 @@ #include #include #include +#include #include #include @@ -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("fv", "draw", "driver message")); + EXPECT_EQ(g_fvErrorCalls, 1u); + EXPECT_EQ(g_fvErrorCode, static_cast(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 foreign( + reinterpret_cast(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("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(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(ErrorCode::InvalidValue), + longMessage.c_str()); + std::fill(longMessage.begin(), longMessage.end(), '?'); + MG_Pipe::gMGPipeCallbacks.OnGlError(static_cast(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(event.payload); + ASSERT_GE(event.payloadSize, sizeof(*head) + head->MessageBytes); + ASSERT_GT(head->MessageBytes, 0u); + const auto* text = reinterpret_cast(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(ErrorCode::InvalidOperation)); + EXPECT_EQ(message, "[fv] driver error"); + } else if (i == 3) { + EXPECT_EQ(head->Code, static_cast(ErrorCode::InvalidValue)); + EXPECT_EQ(head->MessageBytes, Transport::kEventGlErrorMaxMessageBytes); + EXPECT_EQ(message, String(Transport::kEventGlErrorMaxMessageBytes - 1, 'Q')); + } else { + EXPECT_EQ(head->Code, static_cast(ErrorCode::InvalidEnum)); + EXPECT_EQ(head->MessageBytes, 1u); + EXPECT_TRUE(message.empty()); + } + } else if (event.kind == Transport::kEventGpuWritten) { + const auto* head = static_cast(event.payload); + EXPECT_EQ(head->Resource.Slot, 17u); + EXPECT_EQ(head->Resource.Gen, 3u); + EXPECT_EQ(head->RangeCount, 1u); + const auto* range = reinterpret_cast(head + 1); + EXPECT_EQ(range->Size, MG_Pipe::kMGPipeWholeBuffer); + } else { + const auto* head = static_cast(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 From 626d773ba3b802211dfce6b4bc92d3a74f49e487 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sat, 19 Sep 2026 23:43:38 -0400 Subject: [PATCH 2/4] [MG_Backend, MG_Test] (Disaggregated): P5f fv - keep hidden frontend programs off server initialization --- .../DirectVulkan/Renderer/VulkanRenderer.cpp | 12 ++++++++ MobileGL/MG_Test/Wire/ServerLoopTest.cpp | 28 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 2890a878..6e45da15 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -4482,6 +4482,12 @@ void main() { } Bool VulkanRenderer::InitializeBlitResources() { +#if MOBILEGL_BUILD_DISAGGREGATED + // P5f fv: both window and headless transports use WireFramebuffer's native + // operations. These hidden programs are frontend objects, so even creating + // them on the server would retain a client compiler/allocator dependency. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) return true; +#endif ShutdownBlitResources(); auto vertexShader = MakeShared(ShaderStage::Vertex, kHiddenBlitVertexShaderId); @@ -4574,6 +4580,12 @@ void main() { } Bool VulkanRenderer::InitializeDepthMipmapResources() { +#if MOBILEGL_BUILD_DISAGGREGATED + // P5f fv: both window and headless transports use WireFramebuffer's native + // operations. These hidden programs are frontend objects, so even creating + // them on the server would retain a client compiler/allocator dependency. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) return true; +#endif ShutdownDepthMipmapResources(); auto vertexShader = MakeShared(ShaderStage::Vertex, diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index bdcd0e24..ef0b73ee 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -2293,6 +2294,33 @@ namespace { }; } +namespace { + // Exercise the real hidden-resource constructors without adding a production test API. + // Explicit template instantiation permits naming a private member ([temp.explicit]). + using FvRenderer = MG_Backend::DirectVulkan::VulkanRenderer; + struct FvBlitInitTag { + using type = Bool (FvRenderer::*)(); + friend type FvPrivateMember(FvBlitInitTag); + }; + struct FvMipmapInitTag { + using type = Bool (FvRenderer::*)(); + friend type FvPrivateMember(FvMipmapInitTag); + }; + template struct FvMemberAccess { + friend typename Tag::type FvPrivateMember(Tag) { return Member; } + }; + template struct FvMemberAccess; + template struct FvMemberAccess; +} + +TEST(P5fReverseChannel, TransportDoesNotConstructHiddenFrontendPrograms) { + // No Vulkan device, program factory or client GLContext is supplied. A transport + // initialization must not need any of them merely to skip these monolith resources. + FvRenderer renderer({}); + EXPECT_TRUE((renderer.*FvPrivateMember(FvBlitInitTag{}))()); + EXPECT_TRUE((renderer.*FvPrivateMember(FvMipmapInitTag{}))()); +} + TEST(P5fReverseChannel, GlErrorsUseTheOwnedCallbackWithoutAResidualPull) { FvGlobals restore; MG_Config::Ipc.StrictErrors = true; From d0e54d1b9022e233636e69f3c0532fe7564bc5fa Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sat, 19 Sep 2026 23:52:12 -0400 Subject: [PATCH 3/4] [MG_Remote, MG_Test] (Disaggregated): P5f fv - reserve the reverse channel for one accepting session --- MobileGL/MG_Remote/Server/ServerSession.cpp | 39 ++++++++++++++++----- MobileGL/MG_Test/Wire/ServerLoopTest.cpp | 14 ++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index e1a66c9d..27412201 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/MobileGL/MG_Remote/Server/ServerSession.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -357,7 +358,11 @@ namespace MobileGL::MG_Remote::Server { entry = producer; } - ServerSession* g_active = nullptr; + std::atomic g_active{nullptr}; + // One callback table and one process segment resolver: an in-progress Accept + // reserves their owner too. CAS closes the race between distinct session objects + // without making concurrent sessions a supported mode. + std::atomic g_sessionOwner{nullptr}; } // namespace @@ -369,7 +374,7 @@ namespace MobileGL::MG_Remote::Server { return *instance; } - ServerSession* ServerSession::Active() { return g_active; } + ServerSession* ServerSession::Active() { return g_active.load(std::memory_order_acquire); } ServerSession::~ServerSession() { Close(); } @@ -407,9 +412,24 @@ namespace MobileGL::MG_Remote::Server { Bool ServerSession::Accepted() const { return m_accepted; } MobileGLResult ServerSession::Accept(Transport::ITransport& transport) { - if (m_accepted) { - return MOBILEGL_ERR_INVALID_ARGUMENT; + ServerSession* expectedOwner = nullptr; + if (!g_sessionOwner.compare_exchange_strong(expectedOwner, this, + std::memory_order_acq_rel, std::memory_order_acquire)) { + if (expectedOwner == this) return MOBILEGL_ERR_INVALID_ARGUMENT; + MGLOG_F("MGPipe: Fatal{RoleViolation, \"callback-double-install\"} - another " + "ServerSession already owns or is accepting the process reverse channel"); + std::abort(); } + struct ReleaseFailedClaim { + ServerSession* self; + ~ReleaseFailedClaim() { + if (!self->Accepted()) { + ServerSession* expected = self; + g_sessionOwner.compare_exchange_strong(expected, nullptr, + std::memory_order_release, std::memory_order_relaxed); + } + } + } releaseOnFailure{this}; m_transport = &transport; // MOBILEGL_IPC_RING_MB / _STAGE_MB unless SetSegmentSizes overrode them. Unconditional // on purpose - see SizesFromConfig. @@ -538,7 +558,6 @@ namespace MobileGL::MG_Remote::Server { m_segments.InstallProcessResolver(); m_accepted = true; - g_active = this; // ---- 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 @@ -552,6 +571,7 @@ namespace MobileGL::MG_Remote::Server { InstallReverseCallback(MG_Pipe::gMGPipeCallbacks.OnGpuWritten, &ServerOnGpuWritten); InstallReverseCallback(MG_Pipe::gMGPipeCallbacks.OnSurfaceChanged, &ServerOnSurfaceChanged); + g_active.store(this, std::memory_order_release); LogMemory("accept"); @@ -624,9 +644,9 @@ namespace MobileGL::MG_Remote::Server { } void ServerSession::Close() { - if (g_active == this) { - g_active = nullptr; - } + ServerSession* expectedActive = this; + g_active.compare_exchange_strong(expectedActive, nullptr, + std::memory_order_acq_rel, std::memory_order_acquire); 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). @@ -653,6 +673,9 @@ namespace MobileGL::MG_Remote::Server { m_shm.Close(); m_transport = nullptr; m_accepted = false; + ServerSession* expectedOwner = this; + g_sessionOwner.compare_exchange_strong(expectedOwner, nullptr, + std::memory_order_release, std::memory_order_relaxed); } Transport::RingConsumer& ServerSession::CommandRing() { return m_commands; } diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index ef0b73ee..7c137733 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -2375,6 +2375,20 @@ TEST(P5fReverseChannel, GlErrorCallbackRejectsDoubleInstallation) { EXPECT_NE(ReadLog().find("callback-double-install"), std::string::npos); } +TEST(P5fReverseChannel, ASecondSessionCannotClaimTheSameReverseChannel) { + EXPECT_EXIT({ + FvSession owner; + if (!owner.Handshake()) std::_Exit(7); + std::unique_ptr client, server; + Transport::InProcessTransport::CreatePair(client, server); + Server::ServerSession other; + // The owner refusal precedes receiving a Hello or replacing the segment resolver. + (void)other.Accept(*server); + std::_Exit(9); + }, ::testing::KilledBySignal(SIGABRT), ""); + EXPECT_NE(ReadLog().find("another ServerSession already owns"), std::string::npos); +} + TEST(P5fReverseChannel, RecordErrorWithoutCallbackIsNamedFatal) { EXPECT_EXIT({ MG_Config::Ipc.StrictErrors = true; From 1153c561876c1ccb3bc49104e2838c3d78ad9475 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sat, 19 Sep 2026 23:55:17 -0400 Subject: [PATCH 4/4] [MG_Test] (Disaggregated): P5f fv - keep death-test declarations within macro argument rules --- MobileGL/MG_Test/Wire/ServerLoopTest.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index 7c137733..f3bfff50 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -2379,7 +2379,8 @@ TEST(P5fReverseChannel, ASecondSessionCannotClaimTheSameReverseChannel) { EXPECT_EXIT({ FvSession owner; if (!owner.Handshake()) std::_Exit(7); - std::unique_ptr client, server; + std::unique_ptr client; + std::unique_ptr server; Transport::InProcessTransport::CreatePair(client, server); Server::ServerSession other; // The owner refusal precedes receiving a Hello or replacing the segment resolver.