[Merge] (P5b): land r1 core fixes with multi-tail command-ring retry

MobileGL/MG_Backend/DirectGLES/Managers.cpp

@@ -20,6 +20,7 @@
 // R-11's server-owned staging copy. Header-only and package v1's; see its own header block for
 // why GLESBufferResource does not simply gain a member.
 #include <MG_Remote/Server/StagedShadow.h>
+#include <MG_Remote/Server/ServerLoop.h>
 #endif
 
 #include "Utils.h"
@@ -204,6 +205,21 @@ namespace MobileGL::MG_Backend::DirectGLES {
         // garbage for a later collection - there is none on this arm.
         void OnFrontendStateObjectDestroyed(MG_Pipe::MGPipeKind kind, Uint64 lifetimeId) {
             if (InProcessTeardown()) return;
+#if MOBILEGL_BUILD_DISAGGREGATED
+            // Death notices have no framebuffer wire opcode. Keep the lifetime/slot valid
+            // until the context owner has destroyed its twin and updated its binding cache.
+            if (MG_Config::Transport != MG_Config::TransportMode::Monolith &&
+                !MG_Remote::Server::ServerLoop::OnApplyThread()) {
+                struct Death { MG_Pipe::MGPipeKind kind; Uint64 lifetimeId; } death{kind, lifetimeId};
+                MG_Remote::Server::ServerLoopInstance().RunOnApplyThread(
+                    +[](void* user) -> MobileGLResult {
+                        const auto& death = *static_cast<Death*>(user);
+                        OnFrontendStateObjectDestroyed(death.kind, death.lifetimeId);
+                        return MOBILEGL_OK;
+                    }, &death);
+                return;
+            }
+#endif
             switch (kind) {
             case MG_Pipe::MGPipeKind::Texture:
                 TextureImpl::g_backendTextureObjects.DestroyByLifetimeId(lifetimeId);

MobileGL/MG_IntegrationTest/Scenarios/PixelStoreSweepScenario.cpp

@@ -219,6 +219,44 @@ namespace MGITest {
         EXPECT_EQ(FirstGLError(), 0u);
     }
 
+    TEST_F(PixelStoreSweepScenario, ReadPixelsSwapsUnsignedShortComponentsAndPreservesPackGaps) {
+        if (!Ready()) return;
+        ResetAllPixelStoreModes();
+        GLuint texture = 0, framebuffer = 0;
+        glGenTextures(1, &texture);
+        glBindTexture(GL_TEXTURE_2D, texture);
+        // RGBA8 expands to repeated bytes in a ushort and cannot expose a missing swap.
+        const GLfloat color[] = {0.1f, 0.2f, 0.3f, 0.4f};
+        glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA16F, 1, 1, 0, GL_RGBA, GL_FLOAT, color);
+        glGenFramebuffers(1, &framebuffer);
+        glBindFramebuffer(GL_FRAMEBUFFER, framebuffer);
+        glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0);
+        ASSERT_EQ(glCheckFramebufferStatus(GL_FRAMEBUFFER), GLenum(GL_FRAMEBUFFER_COMPLETE));
+        std::uint8_t native[8]{};
+        glReadPixels(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_SHORT, native);
+        ASSERT_NE(native[0], native[1]) << "the source must contain a non-symmetric ushort";
+        glPixelStorei(GL_PACK_SWAP_BYTES, GL_TRUE);
+        std::uint8_t tight[8]{};
+        glReadPixels(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_SHORT, tight);
+        glPixelStorei(GL_PACK_ROW_LENGTH, 3);
+        glPixelStorei(GL_PACK_SKIP_ROWS, 1);
+        glPixelStorei(GL_PACK_SKIP_PIXELS, 1);
+        std::vector<std::uint8_t> scattered(64, 0xCD);
+        glReadPixels(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_SHORT, scattered.data());
+        for (std::size_t i = 0; i < sizeof(native); ++i) {
+            EXPECT_EQ(tight[i], native[i ^ 1]);
+            EXPECT_EQ(scattered[32 + i], native[i ^ 1]);
+        }
+        for (std::size_t i = 0; i < scattered.size(); ++i) {
+            if (i < 32 || i >= 40) EXPECT_EQ(scattered[i], 0xCD);
+        }
+        EXPECT_EQ(FirstGLError(), 0u);
+        ResetAllPixelStoreModes();
+        glBindFramebuffer(GL_FRAMEBUFFER, 0);
+        glDeleteFramebuffers(1, &framebuffer);
+        glDeleteTextures(1, &texture);
+    }
+
     // The leak regression. Each iteration is one complete CTS inner step, and every readback has
     // to be exactly the gradient THIS iteration uploaded - never the previous one's. Before the
     // missing destructors were added, the driver-side framebuffer count grew without bound here.

MobileGL/MG_Remote/Client/ClientSession.cpp

@@ -729,17 +729,31 @@ namespace MobileGL::MG_Remote::Client {
             std::abort();
         }
 
-        const Uint64 seq = m_encoder.EncodeRecord(op, payload, payloadBytes, tails, tailCount);
+        Uint64 seq = m_encoder.EncodeRecord(op, payload, payloadBytes, tails, tailCount);
         if (seq == Wire::kInvalidSeq) {
-            // The ring refused it. NOT a silent drop and not a retry loop: R-10 says P5 does no
+            // EncodeRecord already refused individually oversized records. This one fits,
-            // chunking and must prove it needs none, so a refusal is the proof failing.
+            // but appliedSeq may be ahead of retiredTail: wait for actual reclaimable space,
-            MGLOG_F("MGPipe: Fatal{RingOverrun, \"%s\"} - the command ring refused a %llu-byte "
+            // including the wrap pad. Prior EmitAndWait calls have published every record.
-                    "record. P5 does not chunk (R-10); this is the proof obligation failing, not "
+            Wire::WireRecordLayout layout{};
-                    "a back-pressure case",
+            Wire::MGPipeWireRecordLayout(op, payload, layout);
-                    Wire::WireOpName(op),
+            const Uint64 toEnd = m_cmd.Capacity() - m_cmd.LocalHead() % m_cmd.Capacity();
-                    static_cast<unsigned long long>(payloadBytes + varTailBytes));
+            const Uint64 needed = layout.TotalBytes + (toEnd < layout.TotalBytes ? toEnd : 0);
+            const BarrierWaitScope waiting;
+            const auto wait = m_producer.WaitForCmdSpace(needed, kBarrierTimeoutMs);
+            if (wait != Transport::SessionWait::Reached) {
+                MGLOG_F("MGPipe: Fatal{RetirementWaitFailed, \"SEG_CMD\"} - %s needs %llu "
+                        "reclaimable bytes; the retirement wait ended on %s",
+                        Wire::WireOpName(op), static_cast<unsigned long long>(needed),
+                        wait == Transport::SessionWait::ShutDown ? "shutdown" : "timeout");
                 std::abort();
             }
+            seq = m_encoder.EncodeRecord(op, payload, payloadBytes, tails, tailCount);
+            if (seq == Wire::kInvalidSeq) {
+                MGLOG_F("MGPipe: Fatal{RingOverrun, \"SEG_CMD\"} - %s refused after sufficient "
+                        "space retired", Wire::WireOpName(op));
+                std::abort();
+            }
+        }
 
         // Publish the head, record submittedSeq, THEN ring - in that order, which is
         // SessionProducer's one job and RingTest.cpp:446's pin. Notify-then-publish loses the

MobileGL/MG_Remote/Client/EmitTables.cpp

@@ -779,6 +779,23 @@ namespace MobileGL::MG_Remote::Client {
                    ReadbackBytesPerPixel(format, type);
         }
 
+        void ApplyReadbackByteSwap(void* pixels, Uint64 bytes, GLenum type,
+                                   const PixelStoreParameters& pack) {
+            if (!pack.SwapBytes || pixels == nullptr) return;
+            const auto dataType = MG_Util::ConvertGLEnumToTexturePixelDataType(type);
+            SizeT group = MG_Util::GetSizedTexturePixelDataTypeSize(dataType);
+            if (group == 0) group = MG_Util::GetBaseTexturePixelDataTypeSize(dataType);
+            // This packed depth/stencil type contains two independent 32-bit words.
+            if (type == GL_FLOAT_32_UNSIGNED_INT_24_8_REV) group = 4;
+            if (group <= 1) return;
+            auto* data = static_cast<Uint8*>(pixels);
+            for (Uint64 at = 0; at + group <= bytes; at += group) {
+                for (SizeT i = 0; i < group / 2; ++i) {
+                    std::swap(data[at + i], data[at + group - 1 - i]);
+                }
+            }
+        }
+
         // M2 / codex 11: the reply the server posted is COMPLETE and OK. A short OK reply, and a
         // DECLINED or ERROR reply with a zero payload, both leave the destination full of stale
         // bytes; scattering or returning it is the silently truncated picture ID-47's own comment
@@ -898,6 +915,7 @@ namespace MobileGL::MG_Remote::Client {
                 // handed back as pixels. The Fatal aborts before the application reads the buffer,
                 // so the bytes EmitAndWait already copied into `pixels` are never observed.
                 RequireReadbackReplyComplete(status, replySize, tight);
+                ApplyReadbackByteSwap(pixels, tight, type, pack);
                 return;
             }
 
@@ -910,6 +928,7 @@ namespace MobileGL::MG_Remote::Client {
             // BEFORE THE SCATTER, so a short or non-OK reply never reaches the application's
             // pointer at all (the bounce is the only thing that held the partial bytes).
             RequireReadbackReplyComplete(status, replySize, tight);
+            ApplyReadbackByteSwap(bounce.data(), tight, type, pack);
             ScatterTightReadbackIntoPackState(bounce.data(), pixels, width, height, bytesPerPixel,
                                               pack);
         }

MobileGL/MG_Remote/Client/PersistentMapTracker.cpp

@@ -10,6 +10,7 @@
 
 #include <MG_State/GLState/BufferState/BufferObject.h>
 #include <MG_Util/Debug/Log.h>
+#include <MG_Remote/Server/ServerLoop.h>
 
 #include <cstdlib>
 
@@ -34,6 +35,8 @@ namespace MobileGL::MG_Remote::Client {
         return MG_Config::Transport != MG_Config::TransportMode::Monolith;
     }
 
+    Bool PersistentMapTracker::OnServerRole() { return Server::ServerLoop::OnApplyThread(); }
+
     // SyncPersistentMappedRange's early-out chain (BufferObject.cpp:341-353), in its order,
     // read as a membership test. Every line here has a line there; if one of them moves, the
     // unit case that drives both against each other is what says so.
@@ -72,6 +75,11 @@ namespace MobileGL::MG_Remote::Client {
 
     void PersistentMapTracker::PushBlocksFor(BufferObject& buffer) {
         if (!PushIsArmed()) return;
+        if (OnServerRole()) {
+            MGLOG_F("MGPipe: Fatal{RoleViolation, \"PushBlocksFor\"} - the persistent-map "
+                    "producer belongs to the client; the server consumes transported bytes");
+            std::abort();
+        }
         // Re-checked rather than trusted. The set is maintained at five events and a sixth
         // one arriving without a NoteMapStateChanged would otherwise push a buffer whose
         // shadow has been released - an adopted store's Bytes() is the GPU map, and reading
@@ -120,6 +128,11 @@ namespace MobileGL::MG_Remote::Client {
 
     void PersistentMapTracker::PushAllMembers() {
         if (!PushIsArmed()) return;
+        if (OnServerRole()) {
+            MGLOG_F("MGPipe: Fatal{RoleViolation, \"PushAllMembers\"} - the persistent-map "
+                    "producer belongs to the client; the server consumes transported bytes");
+            std::abort();
+        }
         if (m_livePersistentMaps.empty()) return;
         // Copied out first: PushBlocksFor can erase its own entry (a member that stopped
         // being one), and ska::flat_hash_map invalidates on erase.

MobileGL/MG_Remote/Client/PersistentMapTracker.h

@@ -72,6 +72,17 @@ namespace MobileGL::MG_Remote::Client {
         // resource_subdata record there would be new behaviour, which D-J forbids.
         static Bool PushIsArmed();
 
+        // r1 (P5-close codex finding 1, ID-52): true on the SERVER role's thread - the apply thread
+        // of a running ServerLoop, which under inproc lives in this very process. This module is
+        // the CLIENT's producer: it reads the frontend object's MappedData() and pushes it as
+        // resource_subdata. On the apply thread that record is routed through the monolith adapter
+        // straight into the server's shadow (Ops_H_SubData), which REPLACES the transported bytes
+        // with client memory - and under an active transport the staged copy is the draw's ONLY
+        // base (ID-52 item 3). So the producer asks this before it runs: on the server role the
+        // answer is "there is nothing to sync" (SyncPersistentMappedRange) or a refusal by name
+        // (PushMappedSpanBlock). False in every monolith process, where no ServerLoop runs.
+        static Bool OnServerRole();
+
         // SyncPersistentMappedRange's early-out chain as a predicate. THE only spelling.
         static Bool IsLivePersistentMap(const MG_State::GLState::BufferObject& buffer);
 

MobileGL/MG_State/GLState/BufferState/BufferObject.cpp

@@ -18,6 +18,9 @@
 // edge exists only in a build that has the transport at all.
 #include <MG_Remote/Client/GpuWritePending.h>
 #include <MG_Remote/Client/PersistentMapTracker.h>
+#include <MG_Util/Debug/Log.h>
+
+#include <cstdlib>
 #include <MG_Util/Metrics/PipeStats.h>
 #endif
 
@@ -375,17 +378,11 @@ namespace MobileGL::MG_State::GLState {
 
     void BufferObject::SyncPersistentMappedRange() {
 #if MOBILEGL_BUILD_DISAGGREGATED
-        // SPLIT: the same span, cut into MOBILEGL_IPC_PERSISTENT_BLOCK_KB blocks, and the
+        // Split's client pre-verb hook publishes these bytes. The retained backend sync
-        // membership test is this function's own early-out chain read by the tracker rather
+        // sites must do nothing on the apply thread: re-entering this producer there would
-        // than re-derived there. The 21 sites that call this (CONTRACT-P5.md section 3: 9
+        // overwrite the server shadow through the monolith adapter without crossing the wire.
-        // Espryt + 12 Magma) therefore keep pushing at exactly the points monolith pushes
-        // at, which is what makes the split arm comparable to the monolith one at all; they
-        // retire into MG_Remote::Client::PushPersistentMapsBeforeVerb at P8.
-        //
-        // A block size of 0 disables the push (E3(a)'s negative control) and this is where
-        // that is felt: the bytes the application wrote through the pointer never leave, the
-        // frame draws the last uploaded ones, and PersistentCoherentMapScenario goes red.
         if (MG_Remote::Client::PersistentMapTracker::PushIsArmed()) {
+            if (MG_Remote::Client::PersistentMapTracker::OnServerRole()) return;
             MG_Remote::Client::PersistentMapTracker::Instance().PushBlocksFor(*this);
             return;
         }
@@ -409,6 +406,16 @@ namespace MobileGL::MG_State::GLState {
         // defined-content promotion and the legacy-ops fallback are what the monolith span
         // push does, and a second route to the same record is a second thing to keep in step.
         if (size == 0) return;
+        // Assert ownership at the final producer entry too, before serial/aggregate updates.
+        if (MG_Remote::Client::PersistentMapTracker::OnServerRole()) {
+            MGLOG_F("MGPipe: Fatal{RoleViolation, \"PushMappedSpanBlock\"} - the server role (the apply "
+                    "thread) reached the CLIENT persistent-map producer for buffer lifetime %llu "
+                    "[%zu, +%zu). Under an active transport the server's staged copy is the draw's "
+                    "only base (ID-52); a push from here would replace the transported bytes with "
+                    "client memory through the monolith adapter",
+                    static_cast<unsigned long long>(m_lifetimeId), offset, size);
+            std::abort();
+        }
         NotifySubData(offset, size);
         if (MG_Util::PipeStats::Enabled()) {
             // THE persistent-map-push SITE: the bytes an application wrote through a map with
@@ -429,6 +436,8 @@ namespace MobileGL::MG_State::GLState {
     void BufferObject::NotePersistentMapStateChanged() {
         if (!MG_Remote::Client::PersistentMapTracker::PushIsArmed()) return;
         MG_Remote::Client::PersistentMapTracker::Instance().NoteMapStateChanged(*this);
+        // Teardown must retire membership, but only the client publishes live-host-write state.
+        if (MG_Remote::Client::PersistentMapTracker::OnServerRole()) return;
 
         // THE LIVE-HOST-WRITES BIT (ARCHITECTURE.md 12, CONTRACT-P5.md section 3). A live
         // WRITE map - persistent or not - mutates the shadow with no call, no serial and no

MobileGL/MG_Test/Wire/RemoteClientControls.inc

@@ -393,4 +393,136 @@ int RunPackGpuControl(bool split) {
                 ok ? "PASS" : "FAIL", DescribeStatus(child).c_str(), child.Log.c_str());
     return ok ? 0 : 1;
 }
+namespace {
+struct PatternReadPeer : ReadPeer {
+    Bool OnReadPixels(const MGPReadbackInfo& info, Uint64 seq, Codec::ReplySink* replies) override {
+        Uint8 bytes[32];
+        for (Uint8 i = 0; i < sizeof(bytes); ++i) bytes[i] = i + 1;
+        replies->PostReply(seq, 0, bytes, info.DstSize);
+        return true;
+    }
+};
+
+std::atomic<Bool> retirementRetryStarted{false};
+std::atomic<Bool> retirementWaitObserved{false};
+void HoldRetirementUntilProducerWaits() {
+    const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(2);
+    while (std::chrono::steady_clock::now() < deadline) {
+        if (retirementRetryStarted.load() && ClientSessionInstance().Control()->producerParked.load()) {
+            retirementWaitObserved.store(true);
+            break;
+        }
+        std::this_thread::yield();
+    }
+    Srv::ServerLoopInstance().SetBeforeRetireHookForTesting(nullptr);
+}
+
+struct DrawPeer : Codec::WireVerbSink {
+    Bool OnDrawVbo(const MGPDrawInfo&, const MGPDrawRange*, const MGHostSpan*,
+                   const MGPDrawIndirect*) override { return true; }
+    void Install() {
+        Srv::ServerLoopInstance().RunOnApplyThread([](void* self) {
+            auto& decoder = Srv::ServerSessionInstance().Applier().*PeerMember(DecoderTag{});
+            decoder.SetVerbSink(static_cast<DrawPeer*>(self));
+            return MOBILEGL_OK;
+        }, this);
+    }
+};
+}
+
+TEST(RemoteClientControls, ReadPixelsSwapsComponentsInTightAndScatteredReplies) {
+    const auto child = RunInChild([] {
+        StartControlSession();
+        MG_State::pGLContext = MakeUnique<MG_State::GLState::GLContext>();
+        PatternReadPeer peer;
+        peer.Install();
+        struct Pair { GLenum format; GLenum type; Uint32 bpp; Uint32 group; };
+        for (const Pair pair : {Pair{GL_RGBA, GL_UNSIGNED_SHORT, 8, 2},
+                                Pair{GL_RGBA, GL_FLOAT, 16, 4},
+                                Pair{GL_RGBA, GL_UNSIGNED_INT_8_8_8_8, 4, 4},
+                                Pair{GL_DEPTH_STENCIL, GL_FLOAT_32_UNSIGNED_INT_24_8_REV, 8, 4},
+                                Pair{GL_RGBA, GL_UNSIGNED_BYTE, 4, 1}}) {
+            for (int scatter : {0, 1}) for (int swap : {0, 1}) {
+                auto& context = *MG_State::pGLContext;
+                context.SetPixelStoreParam(PixelStoreParam::PackSwapBytes, swap);
+                context.SetPixelStoreParam(PixelStoreParam::PackRowLength, scatter ? 3 : 0);
+                context.SetPixelStoreParam(PixelStoreParam::PackSkipPixels, scatter);
+                Uint8 result[160];
+                std::memset(result, 0xCD, sizeof(result));
+                RemoteEmitTable().GL.ReadPixels(0, 0, 1, 2, pair.format, pair.type, result);
+                for (Uint32 at = 0; at < sizeof(result); ++at) {
+                    Uint8 expected = 0xCD;
+                    for (Uint32 row = 0; row < 2; ++row) {
+                        const Uint32 begin = (scatter ? 1 + 3 * row : row) * pair.bpp;
+                        if (at >= begin && at < begin + pair.bpp) {
+                            Uint32 i = at - begin;
+                            if (swap) i = (i / pair.group) * pair.group + pair.group - 1 - i % pair.group;
+                            expected = row * pair.bpp + i + 1;
+                        }
+                    }
+                    if (result[at] != expected) ::_exit(110);
+                }
+            }
+        }
+        ClientSessionInstance().Stop();
+    });
+    ExpectChildSuccess(child);
+}
+
+TEST(RemoteClientControls, PersistentMapProducerRefusesTheApplyThread) {
+    const auto child = RunInChild([] {
+        StartControlSession();
+        Srv::ServerLoopInstance().RunOnApplyThread([](void*) {
+            PersistentMapTracker::Instance().PushAllMembers();
+            return MOBILEGL_OK;
+        }, nullptr);
+    });
+    ExpectNamedAbort(child, "Fatal{RoleViolation, \"PushAllMembers\"}");
+}
+
+TEST(RemoteClientControls, CommandWrapWaitsForRetirementAfterThePriorRecordApplied) {
+    const auto child = RunInChild([] {
+        MG_Config::Ipc.RingMb = 1;
+        StartControlSession();
+        DrawPeer peer;
+        peer.Install();
+        Vector<MGPDrawRange> ranges(32768);
+        MGPDrawInfo info{};
+        auto emit = [&](Uint32 count) {
+            info.NumDraws = count;
+            return ClientSessionInstance().EmitAndWait(MGPWireOp::DrawVbo, &info, sizeof(info),
+                ranges.data(), count * sizeof(MGPDrawRange), nullptr, 0, nullptr);
+        };
+        emit(28000); // Position the next record so that its successor needs a wrap pad.
+        peer.Install(); // A mailbox round trip also lets the previous drain retire.
+        Srv::ServerLoopInstance().SetBeforeRetireHookForTesting(&HoldRetirementUntilProducerWaits);
+        const auto prior = emit(32768);
+        if (ClientSessionInstance().Control()->retiredSeq.load() >= prior) ::_exit(111);
+        retirementRetryStarted.store(true);
+        const auto next = emit(32768);
+        if (next != prior + 1 || !retirementWaitObserved.load()) ::_exit(112);
+        ClientSessionInstance().Stop();
+    });
+    ExpectChildSuccess(child);
+}
+
+TEST(RemoteClientControls, StagingWaitsForRetirementAfterThePriorRecordApplied) {
+    const auto child = RunInChild([] {
+        MG_Config::Ipc.StageMb = 1;
+        StartControlSession();
+        Vector<Uint8> bytes(768 * 1024, 0x57);
+        auto& session = ClientSessionInstance();
+        session.Encoder().StageBytes(bytes.data(), bytes.size());
+        Srv::ServerLoopInstance().SetBeforeRetireHookForTesting(&HoldRetirementUntilProducerWaits);
+        MGPBindRenderState bind{};
+        const auto prior = session.EmitAndWait(MGPWireOp::BindRenderState, &bind, sizeof(bind),
+                                               nullptr, 0, nullptr, 0, nullptr);
+        if (session.Control()->retiredSeq.load() >= prior) ::_exit(113);
+        retirementRetryStarted.store(true);
+        session.Encoder().StageBytes(bytes.data(), bytes.size());
+        if (!retirementWaitObserved.load() || session.Encoder().StageReclaimWaits() != 1) ::_exit(114);
+        session.Stop();
+    });
+    ExpectChildSuccess(child);
+}
 #endif

MobileGL/MG_Test/Wire/RemoteClientTest.cpp

@@ -39,6 +39,7 @@
 #include <MG_Remote/Client/EmitTables.h>
 #include <MG_Remote/Client/WireTables.h>
 #include <MG_Remote/Client/ClientSession.h>
+#include <MG_Remote/Client/PersistentMapTracker.h>
 #include <MG_Remote/Client/BackendObject_Remote.h>
 #include <MG_Remote/Server/ServerLoop.h>
 #include <MG_State/GLState/Core.h>
@@ -51,6 +52,9 @@
 #include <MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.h>
 #include <MG_Impl/GLImpl/Drawing/GL_Drawing.h>
 #include <MG_Impl/GLImpl/RenderState/GL_RenderState.h>
+#include <atomic>
+#include <chrono>
+#include <thread>
 
 #if !defined(_WIN32)
 #include <csignal>

MobileGL/MG_Test/Wire/ServerLoopTest.cpp

@@ -1629,6 +1629,44 @@ TEST(StagedShadowProductionTest, ATwinSurvivingContextLossDropsItsFreedShadowBas
 // shows it": B is the corruption, the upload is the draw, the mapped read-back is the picture. Red
 // once by restoring the MappedData() fallback in liveHostBase(): the store then holds A, the
 // client's bytes, and the R-2.5 audit could never reach a draw again.
+TEST(ServerLoopEglTest, FrontendFramebufferDeathDeletesOnTheContextOwner) {
+    MG_Config::Features.PipePush |= MG_Pipe::kMGPipeSubsystemEsprytSlots;
+    EglServerFixture fixture;
+    MGL_EGL_BRING_UP_OR_BAIL(fixture);
+    ASSERT_TRUE(fixture.MakeCurrent());
+    namespace GLES = MG_Backend::DirectGLES;
+    static auto nativeDelete = GLES::g_GLESFuncs.glDeleteFramebuffers;
+    static std::atomic<Uint32> deleted{0};
+    static std::atomic<Bool> wrongThread{false};
+    nativeDelete = GLES::g_GLESFuncs.glDeleteFramebuffers;
+    deleted.store(0);
+    wrongThread.store(false);
+    auto framebuffer = MakeShared<MG_State::GLState::FramebufferObject>(901u);
+    GLuint driverId = 0;
+    ASSERT_EQ(OnApply([&] {
+        auto& twin = GLES::FramebufferImpl::g_backendFramebufferObjects.GetOrCreate(framebuffer);
+        twin = MakeShared<GLES::FramebufferImpl::BackendFramebufferObject>();
+        driverId = twin->GetBackendFramebufferId();
+        twin->Bind(FramebufferTarget::Draw);
+        GLES::g_GLESFuncs.glDeleteFramebuffers = +[](GLsizei count, const GLuint* names) {
+            if (!Server::ServerLoop::OnApplyThread()) wrongThread.store(true);
+            deleted.fetch_add(count);
+            nativeDelete(count, names);
+        };
+    }), MOBILEGL_OK);
+    ASSERT_NE(driverId, 0u);
+    framebuffer.reset(); // Frontend destructor runs on this client thread.
+    EXPECT_EQ(deleted.load(), 1u);
+    EXPECT_FALSE(wrongThread.load());
+    ASSERT_EQ(OnApply([&] {
+        GLES::g_GLESFuncs.glDeleteFramebuffers = nativeDelete;
+        GLint bound = -1;
+        GLES::g_GLESFuncs.glGetIntegerv(GL_DRAW_FRAMEBUFFER_BINDING, &bound);
+        EXPECT_EQ(bound, 0) << "native deletion must unbind the framebuffer in the owner context";
+    }), MOBILEGL_OK);
+    fixture.TearDown();
+}
+
 TEST(StagedShadowProductionTest, TheEnsurePathUploadsTheServerShadowNotTheClientObjectsBytes) {
     EglServerFixture fixture;
     MGL_EGL_BRING_UP_OR_BAIL(fixture);
@@ -1642,6 +1680,12 @@ TEST(StagedShadowProductionTest, TheEnsurePathUploadsTheServerShadowNotTheClient
     ASSERT_EQ(buffer->MappedData()[0], 0xA5);
     ASSERT_TRUE(buffer->HasDefinedContent());
 
+    // Keep this object coherently mapped: Ensure's retained backend sync site must not
+    // re-enter the client producer and replace the staged 0x5B bytes with this 0xA5 map.
+    ASSERT_NE(buffer->AcquireMemoryRange({0, 64}, BufferMappingAccessBit::Write |
+                         BufferMappingAccessBit::Persistent | BufferMappingAccessBit::Coherent), nullptr);
+    ASSERT_TRUE(buffer->IsMapped());
+
     // The server shadow for the twin the draw will use: B, staged the way the wire delivers it.
     const MG_Pipe::MGPipeHandle res = DeclareBuffer(BufferDesc(26, 64, true));
     StageBytes(res, 0, 64, 0x5B);