[Merge] (MGPipe, P5): land v1 round 3 - the review residue closed, bind-once per tuple, the R-11 and coverage gates (ID-62, ID-67)

MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp

@@ -32,6 +32,54 @@ namespace MobileGL::MG_Backend::DirectGLES {
             return draw == EGL_NO_SURFACE && read == EGL_NO_SURFACE && ctx == EGL_NO_CONTEXT;
         }
 
+#if MOBILEGL_BUILD_DISAGGREGATED
+        // ID-54 / ID-67 (v1, under ID-52/ID-59's grant for this file). Which VIRTUAL (dpy, draw,
+        // read, ctx) the process's one native ES context is currently bound FOR, on the apply
+        // thread. DirectGLES has one native context and one native surface (g_Context, g_Surface)
+        // whatever virtual handles the client uses, so "is a native eglMakeCurrent needed" is never
+        // "is the native triple different" - it is "did the VIRTUAL context change":
+        // DirectGLES::MakeCurrent is also where the seven caches that describe the frontend context
+        // are invalidated, and a different virtual context needs them invalidated even though the
+        // driver binds the same triple. ID-67: a make-current with a DIFFERENT tuple is a real
+        // native bind (and a caps republish, ServerLoop's half); an IDENTICAL one is neither.
+        //
+        // Three states. NotBound: the next bind is native. FreshFromSurfaceCreation: the surface's
+        // own creation (InitPbufferSurface / InitWindowSurface) bound natively and invalidated, and
+        // no virtual tuple has claimed that bind yet - the first tuple adopts it, which is the
+        // "2 -> 1 native binds per process" of round 3. BoundForTuple: bound and invalidated for
+        // the recorded tuple; only that exact tuple may skip. Process-wide like the native state it
+        // mirrors; under split exactly one DirectGLES object exists (the server's), and only the
+        // apply thread reaches these.
+        enum class NativeBindState : Uint8 { NotBound, FreshFromSurfaceCreation, BoundForTuple };
+        NativeBindState g_nativeBindState = NativeBindState::NotBound;
+        EGLDisplay g_nativeBoundDpy = EGL_NO_DISPLAY;
+        EGLSurface g_nativeBoundDraw = EGL_NO_SURFACE;
+        EGLSurface g_nativeBoundRead = EGL_NO_SURFACE;
+        EGLContext g_nativeBoundCtx = EGL_NO_CONTEXT;
+
+        void NoteNativeContextGone() { g_nativeBindState = NativeBindState::NotBound; }
+        void NoteNativeContextFreshFromSurfaceCreation() {
+            g_nativeBindState = NativeBindState::FreshFromSurfaceCreation;
+        }
+        void NoteNativeBoundFor(EGLDisplay dpy, EGLSurface draw, EGLSurface read, EGLContext ctx) {
+            g_nativeBindState = NativeBindState::BoundForTuple;
+            g_nativeBoundDpy = dpy;
+            g_nativeBoundDraw = draw;
+            g_nativeBoundRead = read;
+            g_nativeBoundCtx = ctx;
+        }
+        Bool NativeBindCanBeSkippedFor(EGLDisplay dpy, EGLSurface draw, EGLSurface read, EGLContext ctx) {
+            switch (g_nativeBindState) {
+            case NativeBindState::NotBound: return false;
+            case NativeBindState::FreshFromSurfaceCreation: return true;
+            case NativeBindState::BoundForTuple:
+                return g_nativeBoundDpy == dpy && g_nativeBoundDraw == draw && g_nativeBoundRead == read &&
+                       g_nativeBoundCtx == ctx;
+            }
+            return false;
+        }
+#endif
+
         void ClearGLErrors(const MG_External::GLESFunctionsTable& gl) {
             if (!gl.glGetError) return;
             while (gl.glGetError() != GL_NO_ERROR) {}
@@ -843,6 +891,9 @@ namespace MobileGL::MG_Backend::DirectGLES {
 
     BackendObject_DirectGLES::~BackendObject_DirectGLES() {
         DestroyEGLContext();
+#if MOBILEGL_BUILD_DISAGGREGATED
+        NoteNativeContextGone();
+#endif
     }
 
     Bool BackendObject_DirectGLES::InitWindowSurface() {
@@ -920,7 +971,20 @@ namespace MobileGL::MG_Backend::DirectGLES {
             ResetEGLRuntimeState();
         }
 
+#if MOBILEGL_BUILD_DISAGGREGATED
+        // ID-54 / ID-67: the surface's creation bound natively (InitWindowSurface -> MakeCurrent);
+        // the first virtual tuple adopts that bind. On failure nothing is known to be bound. The
+        // pull arm below is the original statement, byte for byte (G1).
+        const Bool created = BackendObject::CreateEGLWindowSurface(surface, handle);
+        if (created) {
+            NoteNativeContextFreshFromSurfaceCreation();
+        } else {
+            NoteNativeContextGone();
+        }
+        return created;
+#else
         return BackendObject::CreateEGLWindowSurface(surface, handle);
+#endif
     }
 
     Bool BackendObject_DirectGLES::CreateEGLPbufferSurface(EGLSurface surface, EGLint width, EGLint height) {
@@ -939,7 +1003,19 @@ namespace MobileGL::MG_Backend::DirectGLES {
             ResetEGLRuntimeState();
         }
 
+#if MOBILEGL_BUILD_DISAGGREGATED
+        // ID-54 / ID-67: as for the window surface - InitPbufferSurface bound natively. The pull
+        // arm below is the original statement, byte for byte (G1).
+        const Bool created = BackendObject::CreateEGLPbufferSurface(surface, width, height);
+        if (created) {
+            NoteNativeContextFreshFromSurfaceCreation();
+        } else {
+            NoteNativeContextGone();
+        }
+        return created;
+#else
         return BackendObject::CreateEGLPbufferSurface(surface, width, height);
+#endif
     }
 
     Bool BackendObject_DirectGLES::InitPbufferSurface(EGLint width, EGLint height) {
@@ -952,6 +1028,9 @@ namespace MobileGL::MG_Backend::DirectGLES {
             if (!DirectGLES::ReleaseCurrent()) {
                 return false;
             }
+#if MOBILEGL_BUILD_DISAGGREGATED
+            NoteNativeContextGone();
+#endif
             return BackendObject::MakeEGLCurrent(dpy, draw, read, ctx);
         }
 
@@ -972,12 +1051,50 @@ namespace MobileGL::MG_Backend::DirectGLES {
             return false;
         }
 
+#if MOBILEGL_BUILD_DISAGGREGATED
+        // ID-54 / C7, the native half of "bind once per tuple" (v1, under ID-52/ID-59's grant for
+        // this file; #if-guarded so the pull build is byte-identical). Under an active transport
+        // this runs on the apply thread, which is the ONLY thread that ever binds the server's
+        // context, and the surface it is asked for was made natively current on this very thread
+        // by its own creation (InitPbufferSurface / InitWindowSurface -> DirectGLES::MakeCurrent).
+        // A second native eglMakeCurrent for the same surface is then a repeat: it rewrites the
+        // owner with the same thread, re-registers the same op table and invalidates seven caches
+        // that describe a context that did not change - the per-client-make-current storm the
+        // v2 review measured as "2 native binds per process, unchanged". So when the requested
+        // draw surface IS the active one and EGL itself says this thread holds the context
+        // (IsBackendContextCurrentOnThisThread re-verifies against eglGetCurrentContext), the
+        // native call is skipped and only the base class's bookkeeping below runs - which is
+        // still required: it is what InitCapabilities and SwapEGLBuffers' current-thread record
+        // hang off. Monolith transport in this build, and the pull build, bind exactly as before.
+        //
+        // AND ONLY FOR THE SAME VIRTUAL TUPLE (ID-67). The skip is keyed on NativeBindCanBeSkippedFor:
+        // the surface's own creation bind is adopted by the FIRST tuple, an identical tuple skips,
+        // and a DIFFERENT tuple - a second MobileGL context onto the same surface - runs the native
+        // call again even though the driver's triple is the same, because MakeCurrent's seven
+        // invalidations describe the frontend context that is changing. Red once, two ways:
+        // make this arm unconditional (ServerLoopTest's C7 control reads 2 native binds at the EGL
+        // function table instead of 1), or make NativeBindCanBeSkippedFor answer true for any tuple
+        // (the ID-67 control's different tuple stays at 1 native bind where 2 are required).
+        const Bool nativelyCurrentAlready = MG_Config::Transport != MG_Config::TransportMode::Monolith &&
+                                            m_eglSurfaceInitialized && m_eglSurface == draw &&
+                                            DirectGLES::IsBackendContextCurrentOnThisThread() &&
+                                            NativeBindCanBeSkippedFor(dpy, draw, read, ctx);
+        if (!nativelyCurrentAlready && !DirectGLES::MakeCurrent()) {
+            NoteNativeContextGone();
+            return false;
+        }
+        NoteNativeBoundFor(dpy, draw, read, ctx);
+#else
         if (!DirectGLES::MakeCurrent()) {
             return false;
         }
+#endif
 
         if (!BackendObject::MakeEGLCurrent(dpy, draw, read, ctx)) {
             (void)DirectGLES::ReleaseCurrent();
+#if MOBILEGL_BUILD_DISAGGREGATED
+            NoteNativeContextGone();
+#endif
             return false;
         }
         return true;
@@ -995,12 +1112,18 @@ namespace MobileGL::MG_Backend::DirectGLES {
     void BackendObject_DirectGLES::ReleaseEGLResources() {
         const std::lock_guard<std::recursive_mutex> lock(m_eglStateMutex);
         DestroyEGLContext();
+#if MOBILEGL_BUILD_DISAGGREGATED
+        NoteNativeContextGone();
+#endif
         BackendObject::ReleaseEGLResources();
     }
 
     void BackendObject_DirectGLES::OnEGLSurfaceReleased(EGLSurface surface) {
         (void)surface;
         DestroyEGLContext();
+#if MOBILEGL_BUILD_DISAGGREGATED
+        NoteNativeContextGone();
+#endif
     }
 
     const RendererInfo& BackendObject_DirectGLES::GetRendererInfo() const {

MobileGL/MG_Backend/DirectGLES/Managers.cpp

@@ -2003,6 +2003,23 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 const auto* record = ResourceRecordOf(res);
                 return record != nullptr ? static_cast<SizeT>(record->Desc.Width) : 0;
             }
+#if MOBILEGL_BUILD_DISAGGREGATED
+            // M-3's rule for the two WHOLE-STORE readers (v1 round 3). The descriptor is the
+            // application's own statement about the store: HasDefinedContent set means it SUPPLIED
+            // the content (glBufferData(size, data)), which under split arrives as resource_subdata
+            // records behind the respecify (table 1 row 19) - so a coverage gap at the draw is a
+            // MISSING RECORD and the zero-fill past the coverage is not the application's bytes.
+            // Clear means it ORPHANED the store (glBufferData(size, NULL), glBufferStorage(NULL)):
+            // every byte it has not staged since is UNDEFINED by its own declaration, the streaming
+            // idiom (orphan, partial glBufferSubData, draw) is the ordinary case, and uploading the
+            // shadow's zero-fill for the rest is exactly what the monolith arm uploads from
+            // MappedData(). Round 2 refused both shapes and aborted six LargeArenaAdoption /
+            // ResourceSubsystemControl entries on the joint by name (v1-v3.md 6).
+            Bool ResourceContentIsDeclared(MG_Pipe::MGPipeHandle res) {
+                const auto* record = ResourceRecordOf(res);
+                return record != nullptr && record->Desc.HasDefinedContent != 0;
+            }
+#endif
 
             void Ops_H_Create(MG_Pipe::MGPipeHandle res, const MG_Pipe::MGPResourceDesc& desc) {
                 (void)res;
@@ -2980,7 +2997,15 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 // is still live; nulling it then would drop the reduced path's own bytes (it did,
                 // and TriangleScenario read the wrong VBO). HasShadow is the discriminator: false
                 // after DropAll, true after an ordinary Adopt.
-                if (!ServerStaged().HasShadow(resource)) {
+                //
+                // TRANSPORT-GUARDED like the other two split hunks in this file (review v2 N-7):
+                // under MONOLITH transport in a disaggregated build the store never copies, Adopt
+                // never sets m_any, HasShadow answers false for everything, and this null would
+                // run on every twin's first ensure - harmless there only because liveHostBase()
+                // prefers MappedData() under monolith, and "under monolith nothing changes" should
+                // be true by construction rather than by luck.
+                if (MG_Config::Transport != MG_Config::TransportMode::Monolith &&
+                    !ServerStaged().HasShadow(resource)) {
                     resource->hostBytes = nullptr;
                 }
 #endif
@@ -3020,13 +3045,19 @@ namespace MobileGL::MG_Backend::DirectGLES {
                     resource->pendingRespecify = false;
                     BindBufferId(TempBufferTarget, reused);
                     // M-3 / codex 4: this is a WHOLE-STORE upload from the base, and under split
-                    // the base is the server shadow (M-2), whose zero-filled bytes past the staged
+                    // the base is the server shadow (M-2). For a store whose content the
-                    // coverage are not the application's - uploading them is the silent data loss
+                    // application SUPPLIED, zero-filled bytes past the staged coverage are not the
-                    // the M-6 ruling forbids. RequireCoverage is a no-op for the legacy arm's
+                    // application's - uploading them is the silent data loss the M-6 ruling
-                    // MappedData() and for a non-copying store; under split it Fatals by name on a
+                    // forbids, and a gap is a missing record: Fatal by name. For a store the
-                    // sparse shadow rather than seeding the driver with zeroes.
+                    // application ORPHANED the gap is its own undefined content and the upload is
-                    MGL_SERVER_STAGED_REQUIRE(*resource, liveHostBase(), 0, poolSize,
+                    // legal (ResourceContentIsDeclared, above). RequireCoverage is a no-op for the
-                                              "pool_reuse_whole_store");
+                    // legacy arm's MappedData() and for a non-copying store.
+#if MOBILEGL_BUILD_DISAGGREGATED
+                    if (ResourceContentIsDeclared(res)) {
+                        MGL_SERVER_STAGED_REQUIRE(*resource, liveHostBase(), 0, poolSize,
+                                                  "pool_reuse_whole_store");
+                    }
+#endif
                     g_GLESFuncs.glBufferSubData(TempBufferTarget, 0, (GLsizeiptr)poolSize, liveHostBase());
                     if (MG_Util::PipeStats::Enabled()) {
                         MG_Util::PipeStats::AddBytes(MG_Util::PipeStats::ByteClass::StageBuffer,
@@ -3104,13 +3135,22 @@ namespace MobileGL::MG_Backend::DirectGLES {
             // [0, size) upload from the base, so it owes the same coverage the pending-range drain
             // below owes - the two respecify arms were the readers M-6's "never widened" rule did
             // not reach. No-op for the legacy arm's MappedData() and for a non-copying store; a
-            // Fatal{StageSnapshotTooNarrow, "respecify_whole_store"} under split when the shadow's
+            // Fatal{StageSnapshotTooNarrow, "respecify_whole_store"} under split when the
-            // coverage does not span the store, instead of uploading its zero-fill as content.
+            // DESCRIPTOR says the application supplied the content and the shadow's coverage does
+            // not span the store (a missing record), instead of uploading its zero-fill as content.
+            // NOT for a store the application orphaned: there `shadowHasContent` is the frontend
+            // object's flag, which the streaming idiom's partial glBufferSubData flips to true, and
+            // the bytes it did not write are undefined by its own glBufferData(NULL) - the rule at
+            // ResourceContentIsDeclared. Round 2 refused that idiom and aborted six joint entries.
             const auto requireWholeStoreCoverage = [&]() {
-                if (initialData != nullptr) {
+#if MOBILEGL_BUILD_DISAGGREGATED
+                if (initialData != nullptr && record->Desc.HasDefinedContent != 0) {
                     MGL_SERVER_STAGED_REQUIRE(*resource, static_cast<const Uint8*>(initialData), 0,
                                               size, "respecify_whole_store");
                 }
+#else
+                (void)initialData;
+#endif
             };
 
             if (resource->pendingRespecify || !resource->storageInitialized || resource->storageSize != size) {

MobileGL/MG_Backend/Init.cpp

@@ -117,6 +117,13 @@ namespace MobileGL::MG_Backend {
             }
         }
 
+        // The resource op table the SERVER's backend registered at step 1 (BackendObject_DirectGLES::
+        // Initialize -> RegisterBufferBackendOps), as step 2 saw it. Step 5 compares against it
+        // (review v2 N-8): a client object that registered a table of its own would have made
+        // AssertConsumerMaskIsHonest's "is a table registered" answer TRUE, so re-asking that
+        // question could never notice the swap - only the pointer can.
+        const MG_Pipe::MGPipeResourceOps* g_resourceOpsAtStep2 = nullptr;
+
         // The single hook (ARCHITECTURE.md:29). Returns false when the split could not be
         // brought up, and the caller then REFUSES TO CONTINUE rather than falling back to the
         // switch below - a fallback here is "the split lane ran monolith and went green".
@@ -135,6 +142,7 @@ namespace MobileGL::MG_Backend {
             Server::ServerSession& session = Server::ServerSessionInstance();
             const Uint64 consumed = ConsumedSubsystemsFor(MG_Config::ActiveBackendType);
             AssertConsumerMaskIsHonest(consumed);
+            g_resourceOpsAtStep2 = MG_Pipe::MGPipeGetResourceOps();
             session.SetConsumedSubsystems(consumed);
             // ZERO IS THE EXPLICIT ANSWER FOR P5, not an omission (ServerSession.h's block):
             // every optional capability bit belongs to the package that owns its question, and
@@ -210,13 +218,27 @@ namespace MobileGL::MG_Backend {
                 MGLOG_W("Failed to initialize MobileGL backend libraries for the remote object");
                 return;
             }
-            // m-6: the honesty cross-check runs a SECOND time, now that step 4's
+            // m-6, re-worded per review v2 N-8. The honesty cross-check runs a SECOND time, now
-            // pActiveBackendObject (the client's BackendObject_Remote) exists and its
+            // that step 4's pActiveBackendObject (the client's BackendObject_Remote) exists and
-            // Initialize() has run inside InitSpecificBackendLibs. Anything the client object
+            // its Initialize() has run inside InitSpecificBackendLibs. What the re-run CAN catch
-            // registered into MGPipeSetResourceOps after the step-2 check is invisible to that
+            // is a table that was REMOVED between step 2 and here (the claim would then be a lie
-            // first call; re-asserting here costs one call and closes the window in which a
+            // again). What it cannot catch - and its first comment claimed it could - is a client
-            // client-registered g_resourceOps would flip the answer under the applier's feet.
+            // object that REGISTERED a table of its own: that leaves "is a table registered"
+            // true. Only the pointer tells those apart, so the table is compared against the one
+            // step 2 saw and a swap is refused by name: the applier would otherwise dispatch the
+            // server's resource records into the CLIENT object's table under its feet.
             AssertConsumerMaskIsHonest(ConsumedSubsystemsFor(MG_Config::ActiveBackendType));
+            if (MG_Pipe::MGPipeGetResourceOps() != g_resourceOpsAtStep2) {
+                MGLOG_F("MGPipe: Fatal{ConsumerMaskLie, \"resource ops table replaced\"} - the "
+                        "resource op table MGPipeGetResourceOps() answers with is not the one the "
+                        "server's backend registered at step 1 (%p now, %p then). Something between "
+                        "ServerSession::Accept and the client object's Initialize() registered its "
+                        "own table, and the applier would dispatch every resource record into it. A "
+                        "mask is a statement about the server's backend, and so is the table",
+                        static_cast<const void*>(MG_Pipe::MGPipeGetResourceOps()),
+                        static_cast<const void*>(g_resourceOpsAtStep2));
+                std::abort();
+            }
             LogBackendInfo();
             return;
         }

MobileGL/MG_Remote/Server/PipeApplier.h

@@ -128,6 +128,10 @@ namespace MobileGL::MG_Remote::Server {
         Uint64 Presents() const { return m_presents; }
         Uint64 LastPresentSerial() const { return m_lastPresentSerial; }
         Uint64 ReadbackBytes() const { return m_readbackBytes; }
+        // ID-49's tight-size control reads this: the scratch a read_pixels grew to. It must equal
+        // the tight w*h*bpp extent of the read, never the client's DstSize - a scratch sized from
+        // DstSize is exactly the heap overflow codex 1 found, one field over.
+        Uint64 ReadbackScratchBytes() const { return static_cast<Uint64>(m_readbackScratch.size()); }
 
     private:
         const MG_Backend::GlobalBackendFunctionsTable* Table(const char* verb) const;

MobileGL/MG_Remote/Server/ServerLoop.cpp

@@ -213,6 +213,7 @@ namespace MobileGL::MG_Remote::Server {
         m_parks.store(0, std::memory_order_release);
         m_nativeBinds.store(0, std::memory_order_release);
         m_clientReleases.store(0, std::memory_order_release);
+        m_makeCurrentRepublishes.store(0, std::memory_order_release);
         m_haveCurrentTuple = false;
         {
             const std::lock_guard<std::mutex> lock(m_exitMutex);
@@ -238,6 +239,12 @@ namespace MobileGL::MG_Remote::Server {
     Uint64 ServerLoop::ClientReleaseCount() const {
         return m_clientReleases.load(std::memory_order_acquire);
     }
+    Uint64 ServerLoop::MakeCurrentRepublishCount() const {
+        return m_makeCurrentRepublishes.load(std::memory_order_acquire);
+    }
+    void ServerLoop::NoteMakeCurrentRepublished() {
+        m_makeCurrentRepublishes.fetch_add(1, std::memory_order_acq_rel);
+    }
 
     // C7 / ID-54. A release-current request (the three NO_* markers, exactly IsReleaseCurrentRequest's
     // test in BackendObject_DirectGLES.cpp) is a ClientRelease the server records but does not
@@ -276,12 +283,22 @@ namespace MobileGL::MG_Remote::Server {
             outcome.boundNatively = false;
             return outcome;
         case EglBindAction::RepeatNoOp:
+            // ID-54's "a no-op apart from the R-12 republish decision": the decision for an
+            // identical repeat is NO republish, because nothing ran that could have moved the
+            // caps - InitCapabilities runs inside the backend's MakeEGLCurrent, which this arm
+            // does not reach, so a republish here would re-send the snapshot the last real bind
+            // already sent. (c1's BackendObject_Remote::InitCapabilities does not lean on this
+            // either way: it asks the server through ServerInitCapabilities, which publishes.)
             outcome.ok = true;
             outcome.boundNatively = false;
             return outcome;
         case EglBindAction::NativeBind:
             break;
         }
+        // "Forwarded" is the honest word: the backend object decides for itself whether the
+        // driver needs a native eglMakeCurrent (BackendObject_DirectGLES skips it for a surface
+        // that is already current on this thread - its own creation bound it), and the C7
+        // control counts THAT at the EGL function table. This counter counts forwards.
         outcome.ok = backend->MakeEGLCurrent(dpy, draw, read, ctx);
         if (!outcome.ok) return outcome;
         m_haveCurrentTuple = true;
@@ -294,6 +311,19 @@ namespace MobileGL::MG_Remote::Server {
         return outcome;
     }
 
+    void ServerLoop::ForgetCurrentTuple() {
+        m_haveCurrentTuple = false;
+        m_curDpy = EGL_NO_DISPLAY;
+        m_curDraw = EGL_NO_SURFACE;
+        m_curRead = EGL_NO_SURFACE;
+        m_curCtx = EGL_NO_CONTEXT;
+    }
+
+    void ServerLoop::ForgetCurrentTupleIfItNames(EGLSurface surface) {
+        if (!m_haveCurrentTuple) return;
+        if (m_curDraw == surface || m_curRead == surface) ForgetCurrentTuple();
+    }
+
     void ServerLoop::ApplyThreadMain() {
         m_applyThreadId.store(std::this_thread::get_id(), std::memory_order_release);
         NameThisThread("mgl-srv-apply");
@@ -389,6 +419,9 @@ namespace MobileGL::MG_Remote::Server {
                     "which is the context owner");
             m_backend.reset();
         }
+        // N-3: the context died with the backend; a tuple that outlives it would make the next
+        // session's first make-current onto the same (recycled) handle values a RepeatNoOp.
+        ForgetCurrentTuple();
 
         // C2: THE m_running CLEAR IS INSIDE THIS SAME CRITICAL SECTION AS THE FINAL DRAIN OF THE
         // MAILBOX. It used to be a separate store after the block, and that gap was a lost-forever
@@ -578,6 +611,9 @@ namespace MobileGL::MG_Remote::Server {
             // thread called Stop. No context was ever made current from another thread in that
             // case, which is exactly the condition that makes this safe.
             if (m_backend != nullptr) m_backend.reset();
+            // N-3, same reason as ApplyThreadMain's exit: no thread runs, so the apply-thread-only
+            // rule on the tuple has no other writer to race.
+            ForgetCurrentTuple();
             m_running.store(false, std::memory_order_release);
             return;
         }
@@ -674,6 +710,9 @@ namespace MobileGL::MG_Remote::Server {
                 MG_Backend::BackendObject* backend = ServerBackendOrNull();
                 if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED;
                 ok = backend->CreateEGLWindowSurface(surface, *handle);
+                // N-3: BackendObject_DirectGLES destroys and recreates the native context to
+                // create a DIFFERENT surface, so whatever tuple was bound names a dead context.
+                if (ok) ServerLoopInstance().ForgetCurrentTuple();
                 return MOBILEGL_OK;
             }
         } args{surface, &handle};
@@ -706,6 +745,10 @@ namespace MobileGL::MG_Remote::Server {
                 MG_Backend::BackendObject* backend = ServerBackendOrNull();
                 if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED;
                 ok = backend->CreateEGLPbufferSurface(surface, width, height);
+                // N-3: as for the window surface - a (re)creation may have destroyed the context
+                // the held tuple named. The surface's own creation binds natively, so the client's
+                // make-current that follows is forwarded and deduped one layer down (ID-54).
+                if (ok) ServerLoopInstance().ForgetCurrentTuple();
                 return MOBILEGL_OK;
             }
         } args{surface, width, height};
@@ -722,25 +765,31 @@ namespace MobileGL::MG_Remote::Server {
             MobileGLResult Run() {
                 MG_Backend::BackendObject* backend = ServerBackendOrNull();
                 if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED;
-                // C7 / ID-54: the apply thread binds the native context ONCE per tuple and holds
+                // C7 / ID-54: the apply thread binds the native context ONCE per context
-                // it for life. ApplyMakeCurrent forwards a real bind only for a new tuple, treats
+                // lifetime and holds it for life. ApplyMakeCurrent forwards a bind only for a
-                // an identical repeat as a no-op, and records a client release-current WITHOUT
+                // tuple it does not hold, treats an identical repeat as a no-op, and records a
-                // unbinding - so "the owner slot is written once" is true here even though
+                // client release-current WITHOUT unbinding; the native call for a surface already
-                // DirectGLES::MakeCurrent itself has no shortcut.
+                // current on this thread is skipped one layer down (BackendObject_DirectGLES's
+                // ID-54 arm), which is what makes "the owner slot is written once" TRUE and
+                // measured (ServerLoopTest's C7 control) rather than claimed.
                 const ServerLoop::MakeCurrentOutcome outcome =
                     ServerLoopInstance().ApplyMakeCurrent(backend, dpy, draw, read, ctx);
                 ok = outcome.ok;
                 if (!ok || !outcome.boundNatively) return MOBILEGL_OK;
                 // R-12, arm (a): the caps snapshot is REPUBLISHED because InitCapabilities has
-                // now run for real - and ONLY on a real native bind, not on an identical repeat
+                // now run for real - on every forwarded bind of a tuple this loop did not hold
-                // (a repeat re-published nothing). DirectGLES has no OnCapsInvalidated producer at
+                // (the first, and every DIFFERENT tuple after it), and NEVER on an identical
-                // all, and c0's answer is that a SECOND arrival IS the invalidation - so the
+                // repeat (ID-67: a repeat is a native no-op AND publishes nothing, so the client's
-                // client's mirror is refreshed with no dev-shaped backend edit and with no
+                // mirror generation does not move and nothing accumulates). DirectGLES has no
-                // eleventh MGPipeCallbacks slot (MGPipeCallbacks.h:56-58's static_assert exists to
+                // OnCapsInvalidated producer at all, and c0's answer is that a SECOND arrival IS the
-                // make that cost visible).
+                // invalidation - so the client's mirror is refreshed with no dev-shaped backend edit
+                // and with no eleventh MGPipeCallbacks slot (MGPipeCallbacks.h:56-58's static_assert
+                // exists to make that cost visible). Red once by republishing only on the first
+                // bind: the ID-67 control's different tuple reads 1 republish where 2 are required.
                 ServerSession* session = ServerSession::Active();
                 if (session != nullptr && session->Accepted()) {
                     const MobileGLResult published = session->PublishCapsSnapshot();
+                    if (published == MOBILEGL_OK) ServerLoopInstance().NoteMakeCurrentRepublished();
                     if (published != MOBILEGL_OK) {
                         MGLOG_E("MG_Remote server: the post-make-current CapsSnapshot could not "
                                 "be published (rc=%d); the client's mirror still holds the empty "
@@ -789,6 +838,12 @@ namespace MobileGL::MG_Remote::Server {
                 MG_Backend::BackendObject* backend = ServerBackendOrNull();
                 if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED;
                 backend->ReleaseEGLSurface(surface);
+                // N-3: a released surface the held tuple names may have taken the context with
+                // it (BackendObject::ReleaseEGLSurface -> OnEGLSurfaceReleased -> DestroyEGLContext
+                // once nothing holds it current). Forgetting when the base class only DEFERRED
+                // the destroy costs one forwarded bind; remembering when it did not would cost a
+                // silent no-context-current.
+                ServerLoopInstance().ForgetCurrentTupleIfItNames(surface);
                 return MOBILEGL_OK;
             }
         } args{surface};
@@ -807,6 +862,12 @@ namespace MobileGL::MG_Remote::Server {
                 MG_Backend::BackendObject* backend = ServerBackendOrNull();
                 if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED;
                 backend->ReleaseEGLResources();
+                // N-3: DestroyEGLContext just ran; the tuple names nothing. Without this a
+                // destroy-recreate with the same handle values (every EGL handle on this host
+                // is 0x1) classified as a RepeatNoOp, bound nothing, and republished no caps.
+                // Red once by deleting it: ServerLoopTest's recreate control reads
+                // NativeBindCount() == 1 where 2 is required.
+                ServerLoopInstance().ForgetCurrentTuple();
                 return MOBILEGL_OK;
             }
         } args{};

MobileGL/MG_Remote/Server/ServerLoop.h

@@ -32,17 +32,34 @@
 // before the client frees any emitter-owned Vector; and the join must be bounded (that test uses
 // 5 s) so a regression is a red test and not a hung CI job.
 //
-// THE EGL OWNERSHIP MOVE. eglMakeCurrent runs ONCE per (dpy, draw, read, ctx) tuple on this
+// THE EGL OWNERSHIP MOVE, AS MEASURED (ID-54; review v2 item 10 and N-3). The native
-// thread and the context is then held for life. The "once" is NOT free at the DirectGLES layer -
+// eglMakeCurrent for a surface runs ONCE PER CONTEXT LIFETIME on this thread and the context is
-// DirectGLES::MakeCurrent always calls native eglMakeCurrent and rewrites the owner (codex C7) -
+// then held for life. That "once" lives in TWO layers, because one is not enough:
-// so ServerMakeEGLCurrent is where the dedup lives (ID-54): an identical repeat is a no-op apart
+// DirectGLES::MakeCurrent always calls native eglMakeCurrent and rewrites the owner (codex C7),
-// from the R-12 republish decision, a different tuple is a real rebind, and a client
+// and it is reached twice per bring-up - once from InitPbufferSurface/InitWindowSurface when the
-// release-current is RECORDED (NativeBindCount / ClientReleaseCount) but NOT forwarded - the apply
+// surface is CREATED, and once more from the client's first eglMakeCurrent. So (1)
-// thread keeps the context current until ~BackendObject_DirectGLES or context loss, which is what
+// ServerMakeEGLCurrent classifies the request against the tuple it last bound
-// makes DirectGLES.cpp's six cache invalidations a one-off startup cost and the 16
+// (ClassifyEglMakeCurrent): an identical repeat is a no-op, and the R-12 republish decision for
-// IsBackendContextCurrentOnThisThread() / 16 CanTouchGLNow() sites answer TRUE on the server. The
+// it is "nothing to republish" (ID-67: the client's mirror generation must not move); a different
-// client's nine EGL virtuals become BLOCKING control requests executed here. ReleaseEGLResources
+// tuple is a real forwarded bind AND a caps republish; a client release-current is RECORDED
-// and ~BackendObject_DirectGLES MUST be blocking: MobileGL::Destroy() (MobileGL/Init.cpp:68)
+// (ClientReleaseCount) and NOT forwarded. And (2) BackendObject_DirectGLES::MakeEGLCurrent, under
+// an active transport only, skips the native call when the requested draw surface is the one
+// already natively current on this thread (IsBackendContextCurrentOnThisThread, which is EGL
+// ground truth) AND the virtual tuple is the one that bind was for - or the bind is the surface's
+// own creation, which the first tuple adopts; a different virtual context onto the same surface
+// binds natively again, because MakeCurrent's invalidations describe the frontend context that
+// changed (ID-67). Measured at the EGL function
+// table by ServerLoopTest's C7 control on a real llvmpipe context: surface creation + two
+// identical make-currents + a client release + a bind after the release = ONE native
+// eglMakeCurrent, ZERO native releases, and the apply thread still the owner afterwards. So
+// DirectGLES.cpp's six cache invalidations run once per context lifetime rather than once per
+// client make-current, and the 16 IsBackendContextCurrentOnThisThread() / 16 CanTouchGLNow()
+// sites answer TRUE on the server. The tuple is FORGOTTEN (N-3) on every event after which the
+// native context it names may be gone - ReleaseEGLResources, ReleaseEGLSurface of the surface it
+// names, a surface (re)creation, backend destruction - so a recycled handle value after a destroy
+// is a real bind again and never a silent no-context-current. The client's nine EGL virtuals
+// become BLOCKING control requests executed here. ReleaseEGLResources and
+// ~BackendObject_DirectGLES MUST be blocking: MobileGL::Destroy() (MobileGL/Init.cpp:68)
 // otherwise walks on while the server still holds the context.
 //
 // THE FALLBACK IS PRE-DECLARED, NOT INVENTED UNDER PRESSURE (R-1). If the context migration is
@@ -130,12 +147,20 @@ namespace MobileGL::MG_Remote::Server {
         Uint64 DrainedRecords() const;
         Uint64 ParkCount() const;
 
-        // C7 / ID-54 diagnostics, read by the C7 control. NativeBindCount is how many times
+        // C7 / ID-54 diagnostics, read by ServerLoopTest's C7 and N-3 controls. NativeBindCount is
-        // ServerMakeEGLCurrent forwarded a REAL native bind (a new tuple); ClientReleaseCount how
+        // how many times ApplyMakeCurrent FORWARDED a bind to the backend (a tuple it did not
-        // many client release-current requests were recorded-and-not-forwarded. Two identical
+        // hold); ClientReleaseCount how many client release-current requests were recorded and
-        // binds must move the first by one and the second not at all.
+        // not forwarded. Two identical binds must move the first by one and the second not at
+        // all. The number of native eglMakeCurrent calls the DRIVER saw is a different number -
+        // the backend object skips the native call for a surface already current (header block)
+        // - and the control reads that one at the EGL function table, not here.
         Uint64 NativeBindCount() const;
         Uint64 ClientReleaseCount() const;
+        // ID-67: how many caps snapshots ServerMakeEGLCurrent has re-published (R-12 arm (a)) - one
+        // per forwarded bind of a tuple it did not hold, never for an identical repeat, so the
+        // client's mirror generation moves exactly when the server's answers could have.
+        Uint64 MakeCurrentRepublishCount() const;
+        void NoteMakeCurrentRepublished();
 
         // The deduped make-current, on the apply thread. Classifies the request (see
         // ClassifyEglMakeCurrent), forwards a native bind only for a genuinely new tuple, records
@@ -148,6 +173,20 @@ namespace MobileGL::MG_Remote::Server {
         MakeCurrentOutcome ApplyMakeCurrent(MG_Backend::BackendObject* backend, EGLDisplay dpy,
                                             EGLSurface draw, EGLSurface read, EGLContext ctx);
 
+        // N-3: forget the tuple ApplyMakeCurrent last bound. Apply thread only, like the tuple
+        // itself (the forwarders that call these run their Args::Run there). Called on every
+        // event after which the native context that tuple named may no longer exist -
+        // ReleaseEGLResources, ReleaseEGLSurface of a surface the tuple names, a surface
+        // (re)creation (BackendObject_DirectGLES destroys the context to create a different
+        // surface), backend destruction - so the next make-current with the SAME handle values
+        // (EGL handles are recycled; on this host every one of them is literally 0x1) is
+        // classified as a real bind, not as a RepeatNoOp that binds nothing, runs no base-class
+        // bookkeeping and republishes no caps. Forgetting is always safe: the cost of a
+        // forgotten-but-still-current tuple is one forwarded bind the backend object dedups
+        // natively; the cost of a remembered-but-dead one is a silent no-context-current.
+        void ForgetCurrentTuple();
+        void ForgetCurrentTupleIfItNames(EGLSurface surface);
+
     private:
         void ApplyThreadMain();
         // Part of the apply thread's park predicate: a posted control request must be able to
@@ -202,6 +241,7 @@ namespace MobileGL::MG_Remote::Server {
         EGLContext m_curCtx = EGL_NO_CONTEXT;
         std::atomic<Uint64> m_nativeBinds{0};
         std::atomic<Uint64> m_clientReleases{0};
+        std::atomic<Uint64> m_makeCurrentRepublishes{0};
     };
 
     ServerLoop& ServerLoopInstance();
@@ -224,12 +264,15 @@ namespace MobileGL::MG_Remote::Server {
     // owner slot g_backendContextOwnerThread (DirectGLES.cpp:11865) is stamped with whatever
     // thread got there. So BackendObject_Remote's nine EGL virtuals - package c1's - call these
     // twelve, each of which is a BLOCKING control request that runs the SERVER's backend object
-    // on mgl-srv-apply. eglMakeCurrent then runs ONCE, on that thread, and is never released:
+    // on mgl-srv-apply. The native eglMakeCurrent then runs once per context lifetime on that
-    // g_backendContextOwnerThread is written once, DirectGLES.cpp:11933-11953's six cache
+    // thread (the surface's own creation binds; the client's make-currents onto that surface
-    // invalidations become a one-off startup cost instead of a per-migration storm, the
+    // are deduped at both layers, header block above) and a client release is never forwarded,
-    // per-frame EGL re-verification stamp is permanently true, and the 16
+    // so g_backendContextOwnerThread is written once per context lifetime, DirectGLES.cpp's six
+    // cache invalidations run once per context lifetime rather than once per client
+    // make-current, the per-frame EGL re-verification stamp holds, and the 16
     // IsBackendContextCurrentOnThisThread() sites plus the 16 CanTouchGLNow() sites answer TRUE
-    // on the server instead of silently degrading.
+    // on the server instead of silently degrading. Measured, not assumed: ServerLoopTest's C7
+    // control counts the driver's eglMakeCurrent calls at the EGL function table.
     //
     // TWO OF THEM MUST BLOCK OR THE PROCESS TEARS ITS OWN CONTEXT DOWN UNDER ITSELF:
     // ReleaseEGLResources (reached from EGLImpl.cpp:326, which for DirectGLES runs