From e7fe3b977033022a61253b5c23128f1567c51eb4 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 07:45:29 -0400 Subject: [PATCH 1/9] [Fix] (MG_Remote/Server): ServerLoop - clear m_running under m_controlMutex and re-check before publishing (C2); a backend with no thread is Fatal{ApplyThreadNotRunning} not an inline app-thread fallback (M-7); ApplyAffinity logs the effective sched_getaffinity mask (M-4/codex-11); ServerMakeEGLCurrent binds once per tuple and records a client release without unbinding (C7/ID-54); Start resets its own diagnostics (m-3); doorbell Fatal text corrected (m-4) --- MobileGL/MG_Remote/Server/ServerLoop.cpp | 190 +++++++++++++++++++---- MobileGL/MG_Remote/Server/ServerLoop.h | 68 ++++++-- 2 files changed, 214 insertions(+), 44 deletions(-) diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index ed1d488d..f0a4bc2f 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -101,7 +101,12 @@ namespace MobileGL::MG_Remote::Server { #endif } - // Returns the mask that was actually APPLIED, which is 0 when nothing was. + // Returns the mask that the kernel ACTUALLY applied - the EFFECTIVE mask read back with + // sched_getaffinity, not the requested one (codex 11). A cpuset that permits only a subset + // of the requested cpus is accepted by sched_setaffinity with the intersection, and + // logging the request would then claim cpus the thread never ran on - which is precisely + // the "an affinity that silently did nothing looks like one that worked" failure the + // resolved mask exists to make visible. 0 when nothing was applied. Uint64 ApplyAffinity(Uint64 requested) { if (requested == 0) return 0; #if defined(__linux__) || defined(__ANDROID__) @@ -116,7 +121,19 @@ namespace MobileGL::MG_Remote::Server { } if (applied == 0) return 0; if (sched_setaffinity(0, sizeof(set), &set) != 0) return 0; - return requested; + // Read back the mask the kernel really honoured. This is the codex-11 fix: the log + // and ResolvedAffinityMask() report the EFFECTIVE set, so a request the cpuset trimmed + // is visible as a smaller resolved mask rather than as a lie that matched the request. + cpu_set_t effective; + CPU_ZERO(&effective); + if (sched_getaffinity(0, sizeof(effective), &effective) != 0) { + return requested; // best effort: the set succeeded, the read did not + } + Uint64 mask = 0; + for (Uint cpu = 0; cpu < 64; ++cpu) { + if (CPU_ISSET(cpu, &effective)) mask |= (1ull << cpu); + } + return mask; #else return 0; #endif @@ -188,6 +205,15 @@ namespace MobileGL::MG_Remote::Server { } m_session = &session; m_stopRequested.store(false, std::memory_order_release); + // m-3: reset THIS session's own diagnostics. DrainedRecords()/ParkCount() are absolute and + // a case asserts == N, so a process that opens a SECOND session (context loss, or + // Initialize after Destroy) must start them at zero rather than carry the first session's + // tally - and a developer running the binary directly gets the count CI sees. + m_drained.store(0, std::memory_order_release); + 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_haveCurrentTuple = false; { const std::lock_guard lock(m_exitMutex); m_exited = false; @@ -208,6 +234,65 @@ namespace MobileGL::MG_Remote::Server { Uint64 ServerLoop::ResolvedAffinityMask() const { return m_affinityMask; } Uint64 ServerLoop::DrainedRecords() const { return m_drained.load(std::memory_order_acquire); } Uint64 ServerLoop::ParkCount() const { return m_parks.load(std::memory_order_acquire); } + Uint64 ServerLoop::NativeBindCount() const { return m_nativeBinds.load(std::memory_order_acquire); } + Uint64 ServerLoop::ClientReleaseCount() const { + return m_clientReleases.load(std::memory_order_acquire); + } + + // 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 + // forward. Otherwise an identical (dpy, draw, read, ctx) already held is a RepeatNoOp, and + // anything else is a real NativeBind. Pure: a unit case drives it with no EGL context. + EglBindAction ClassifyEglMakeCurrent(Bool haveCurrent, EGLDisplay curDpy, EGLSurface curDraw, + EGLSurface curRead, EGLContext curCtx, EGLDisplay dpy, + EGLSurface draw, EGLSurface read, EGLContext ctx) { + if (draw == EGL_NO_SURFACE && read == EGL_NO_SURFACE && ctx == EGL_NO_CONTEXT) { + return EglBindAction::ClientRelease; + } + if (haveCurrent && curDpy == dpy && curDraw == draw && curRead == read && curCtx == ctx) { + return EglBindAction::RepeatNoOp; + } + return EglBindAction::NativeBind; + } + + ServerLoop::MakeCurrentOutcome ServerLoop::ApplyMakeCurrent(MG_Backend::BackendObject* backend, + EGLDisplay dpy, EGLSurface draw, + EGLSurface read, EGLContext ctx) { + // Apply thread only (RunOnApplyThread put us here), so m_haveCurrentTuple/m_cur* need no + // lock. The counters are atomic for the reader on the test thread. + MakeCurrentOutcome outcome; + const EglBindAction action = ClassifyEglMakeCurrent(m_haveCurrentTuple, m_curDpy, m_curDraw, + m_curRead, m_curCtx, dpy, draw, read, ctx); + switch (action) { + case EglBindAction::ClientRelease: + // Recorded, NOT forwarded (ID-54): the context stays current on this thread until + // ~BackendObject_DirectGLES or context loss. Forwarding backend->MakeEGLCurrent here + // would route to DirectGLES::ReleaseCurrent, unbind, and clear + // g_backendContextOwnerThread - reinstating the off-thread degradation the phase + // removes. m_haveCurrentTuple is left intact so a later identical bind is still a + // RepeatNoOp (the driver never lost the context). + m_clientReleases.fetch_add(1, std::memory_order_acq_rel); + outcome.ok = true; + outcome.boundNatively = false; + return outcome; + case EglBindAction::RepeatNoOp: + outcome.ok = true; + outcome.boundNatively = false; + return outcome; + case EglBindAction::NativeBind: + break; + } + outcome.ok = backend->MakeEGLCurrent(dpy, draw, read, ctx); + if (!outcome.ok) return outcome; + m_haveCurrentTuple = true; + m_curDpy = dpy; + m_curDraw = draw; + m_curRead = read; + m_curCtx = ctx; + m_nativeBinds.fetch_add(1, std::memory_order_acq_rel); + outcome.boundNatively = true; + return outcome; + } void ServerLoop::ApplyThreadMain() { m_applyThreadId.store(std::this_thread::get_id(), std::memory_order_release); @@ -305,8 +390,16 @@ namespace MobileGL::MG_Remote::Server { m_backend.reset(); } - // Anything still parked in RunOnApplyThread has to be answered rather than left - // waiting: this thread is the only thing that could have run it. + // 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 + // hang: a caller that read m_running == true just before this thread exited, then had this + // thread run the block (finding nothing pending) and clear m_running OUTSIDE the lock, + // would go on to publish a request into a mailbox no thread will ever pump and block on + // m_controlDone with no answer possible (the bounded join has already succeeded, so there + // is not even a Fatal{ApplyThreadJoinTimeout}). With the clear under the lock, a caller + // either takes the lock FIRST (its request is here and answered NOT_INITIALIZED) or takes + // it AFTER (it sees !m_running under the lock in RunOnApplyThread and returns + // NOT_INITIALIZED without publishing). There is no third order. { const std::lock_guard lock(m_controlMutex); if (m_controlPending) { @@ -316,10 +409,10 @@ namespace MobileGL::MG_Remote::Server { MGLOG_E("MG_Remote server: a control request was still posted when mgl-srv-apply " "exited; it is answered NOT_INITIALIZED rather than left blocking"); } + m_running.store(false, std::memory_order_release); } m_controlDone.notify_all(); - m_running.store(false, std::memory_order_release); SignalExited(); } @@ -420,29 +513,52 @@ namespace MobileGL::MG_Remote::Server { // applier calls that wants "run this where the context is". Running inline is the // correct answer there and the only non-hanging one. if (OnApplyThread()) return work(user); - // NO THREAD YET, OR ALREADY GONE: run inline on the caller. That is right for the - // window between MG_Backend::Init()'s hook and ClientSession::Start (the backend object - // exists, the thread does not, and nothing has made a context current), and for the - // window after Stop(). It is NOT a silent fallback to monolith: the thread's absence in - // those two windows is a fact about the lifecycle, not about the transport. - if (!m_running.load(std::memory_order_acquire)) return work(user); const std::lock_guard callerLock(m_callerMutex); - { - std::unique_lock lock(m_controlMutex); - m_controlWork = work; - m_controlUser = user; - m_controlPending = true; - m_controlFinished = false; + std::unique_lock lock(m_controlMutex); + // C2 + M-7: the m_running check and the publish are ONE critical section, and m_running is + // cleared under this same lock on the way out (ApplyThreadMain's exit block), so this read + // cannot see `true` for a thread that then vanishes before the publish. When there is no + // thread there are two sub-cases and neither is a silent EGL-on-the-app-thread fallback: + if (!m_running.load(std::memory_order_acquire)) { + const Bool backendAlive = m_backend != nullptr; + lock.unlock(); + if (backendAlive) { + // M-7: the backend exists but no thread owns it - ClientSession::Start refused + // (ServerLoop.cpp Start's !Accepted arm) or std::thread's constructor threw. + // Running `work` inline here would issue eglMakeCurrent on the APP thread and + // stamp g_backendContextOwnerThread with it, so a lane called split would render + // correctly with the context on the wrong thread - the one outcome R-1 exists to + // make impossible. Fatal by name, never a fallback; compare CreateBackend's + // default: arm, which also refuses rather than substitutes. + MGLOG_F("MGPipe: Fatal{ApplyThreadNotRunning, \"EGL on the app thread\"} - a " + "control request reached RunOnApplyThread with the server's backend built " + "but no mgl-srv-apply thread running (ClientSession::Start failed after " + "ServerLoop::CreateBackend). Running it inline would make the context " + "current on the APP thread - the split lane's whole premise. Refusing by " + "name rather than falling back to monolith"); + std::abort(); + } + // Backend already gone (post-Stop teardown, or the pre-Init window): the forwarders' + // ServerBackendOrNull() would answer null anyway, so NOT_INITIALIZED is the honest + // result and running inline is pointless. This is the deterministic half of C2 - a + // forwarder call after the loop stopped returns NOT_INITIALIZED, it does not hang and + // it does not run on the caller. + return MOBILEGL_ERR_NOT_INITIALIZED; } + m_controlWork = work; + m_controlUser = user; + m_controlPending = true; + m_controlFinished = false; // Publish THEN ring, in that order and never the other (Doorbell.h:186-193). The bell // is rung unconditionally rather than through NotifyIfParked because this side does not // know whether the apply thread is parked or spinning, and CondVarDoorbell remembers a - // wakeup that arrives while nobody is waiting. + // wakeup that arrives while nobody is waiting. Held under m_controlMutex: wait() releases + // it atomically, so the apply thread cannot observe the request until this side is + // waiting, and the doorbell remembers the notify regardless. if (m_session != nullptr) { m_session->ConsumerDoorbell().Notify(); } - std::unique_lock lock(m_controlMutex); m_controlDone.wait(lock, [this] { return m_controlFinished; }); return m_controlResult; } @@ -488,11 +604,12 @@ namespace MobileGL::MG_Remote::Server { // a use-after-free whose only symptom is an intermittent crash somewhere else. // Aborting here is red, immediate, and names the cause. MGLOG_F("MGPipe: Fatal{ApplyThreadJoinTimeout} - mgl-srv-apply did not exit within " - "%u ms of Stop(). It parks on Doorbell::Wait(kWaitForever), which only " - "CondVarDoorbell::Kill() can break (Doorbell.h:211-221, table 3 step 2), so " - "this is a lost wakeup and not a slow thread. Aborting rather than " - "detaching: a detached apply thread still owns the context and would read " - "rings the client is about to unmap", + "%u ms of Stop(). Stop() published m_stopRequested (in the park predicate) " + "BEFORE it rang, so a plain Notify should already have un-parked the thread; " + "Doorbell::Kill() (Doorbell.h:211-221, table 3 step 2) is the belt to that " + "Notify's braces. If the thread is still parked after both, the wakeup was " + "lost, not slow. Aborting rather than detaching: a detached apply thread still " + "owns the context and would read rings the client is about to unmap", kJoinTimeoutMs); std::abort(); } @@ -605,17 +722,22 @@ namespace MobileGL::MG_Remote::Server { MobileGLResult Run() { MG_Backend::BackendObject* backend = ServerBackendOrNull(); if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED; - // THE ONE eglMakeCurrent OF THE PROCESS'S LIFE, ON THIS THREAD. Every later - // client-side eglMakeCurrent onto the same surface finds the context already - // current here and costs nothing; the owner slot is written once. - ok = backend->MakeEGLCurrent(dpy, draw, read, ctx); - if (!ok) return MOBILEGL_OK; + // C7 / ID-54: the apply thread binds the native context ONCE per tuple and holds + // it for life. ApplyMakeCurrent forwards a real bind only for a new tuple, treats + // an identical repeat as a no-op, and records a client release-current WITHOUT + // unbinding - so "the owner slot is written once" is true here even though + // DirectGLES::MakeCurrent itself has no shortcut. + 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. DirectGLES has no OnCapsInvalidated producer at all, and - // c0's answer is that a SECOND arrival IS the 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). + // now run for real - and ONLY on a real native bind, not on an identical repeat + // (a repeat re-published nothing). DirectGLES has no OnCapsInvalidated producer at + // all, and c0's answer is that a SECOND arrival IS the 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). ServerSession* session = ServerSession::Active(); if (session != nullptr && session->Accepted()) { const MobileGLResult published = session->PublishCapsSnapshot(); diff --git a/MobileGL/MG_Remote/Server/ServerLoop.h b/MobileGL/MG_Remote/Server/ServerLoop.h index 05a85fec..d1eb0cf5 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.h +++ b/MobileGL/MG_Remote/Server/ServerLoop.h @@ -22,17 +22,27 @@ // P6 splits it. // // PARKING AND SHUTDOWN. The thread parks on Doorbell::Wait(consumerParked, ready, spinUs, -// kWaitForever) and shuts down when Wait returns false with Dead() set. Doorbell::Kill() -// (Doorbell.h:211-221) IS THE ONLY THING that wakes a thread parked on kWaitForever - a fact -// ARCHITECTURE.md's teardown order (:537) omits and InProcessTransportTest.cpp:344 already -// pins. Kill BEFORE join; join 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. +// kWaitForever) and shuts down when Wait returns false with Dead() set. TWO things can un-park a +// kWaitForever waiter, not one, and the difference is the whole of review m-4: for the STOP case a +// plain Doorbell::Notify() is sufficient, because m_stopRequested is in the park predicate and +// Stop() publishes it BEFORE it rings (Doorbell.h re-tests ready() after every Park return); +// Doorbell::Kill() (Doorbell.h:211-221) is load-bearing only for a predicate that has NOTHING to +// see, which is why the redcheck's park-predicate-loses-control entry - the one that drops the +// control flag FROM the predicate - is the case that actually times out. Kill BEFORE join; join +// 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 on this thread and is never released -// (DirectGLES.cpp:11925 plus the six cache invalidations at :11933-11953, which become a -// one-time startup cost instead of a per-make-current storm). 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) +// THE EGL OWNERSHIP MOVE. eglMakeCurrent runs ONCE per (dpy, draw, read, ctx) tuple on this +// thread and the context is then held for life. The "once" is NOT free at the DirectGLES layer - +// DirectGLES::MakeCurrent always calls native eglMakeCurrent and rewrites the owner (codex C7) - +// so ServerMakeEGLCurrent is where the dedup lives (ID-54): an identical repeat is a no-op apart +// from the R-12 republish decision, a different tuple is a real rebind, and a client +// release-current is RECORDED (NativeBindCount / ClientReleaseCount) but NOT forwarded - the apply +// thread keeps the context current until ~BackendObject_DirectGLES or context loss, which is what +// makes DirectGLES.cpp's six cache invalidations a one-off startup cost and the 16 +// IsBackendContextCurrentOnThisThread() / 16 CanTouchGLNow() sites answer TRUE on the server. 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 @@ -120,6 +130,24 @@ 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 + // ServerMakeEGLCurrent forwarded a REAL native bind (a new tuple); ClientReleaseCount how + // many client release-current requests were recorded-and-not-forwarded. Two identical + // binds must move the first by one and the second not at all. + Uint64 NativeBindCount() const; + Uint64 ClientReleaseCount() const; + + // The deduped make-current, on the apply thread. Classifies the request (see + // ClassifyEglMakeCurrent), forwards a native bind only for a genuinely new tuple, records + // a release without forwarding it, and returns whether a native bind happened so the + // caller (ServerMakeEGLCurrent) knows whether to re-publish the caps snapshot (R-12). + struct MakeCurrentOutcome { + Bool ok = false; // the request was honoured + Bool boundNatively = false; // a native eglMakeCurrent ran (=> republish caps) + }; + MakeCurrentOutcome ApplyMakeCurrent(MG_Backend::BackendObject* backend, EGLDisplay dpy, + EGLSurface draw, EGLSurface read, EGLContext ctx); + private: void ApplyThreadMain(); // Part of the apply thread's park predicate: a posted control request must be able to @@ -163,10 +191,30 @@ namespace MobileGL::MG_Remote::Server { Uint64 m_affinityMask = 0; std::atomic m_drained{0}; std::atomic m_parks{0}; + + // C7 / ID-54: the (dpy, draw, read, ctx) currently bound on the apply thread. Written and + // read ONLY on the apply thread inside ApplyMakeCurrent, so it needs no lock; the two + // counters beside it are atomic because a test reads them from another thread. + Bool m_haveCurrentTuple = false; + EGLDisplay m_curDpy = EGL_NO_DISPLAY; + EGLSurface m_curDraw = EGL_NO_SURFACE; + EGLSurface m_curRead = EGL_NO_SURFACE; + EGLContext m_curCtx = EGL_NO_CONTEXT; + std::atomic m_nativeBinds{0}; + std::atomic m_clientReleases{0}; }; ServerLoop& ServerLoopInstance(); + // C7 / ID-54, factored out so a unit case can drive the DECISION without a live EGL context + // (the native bind itself needs the joint lane). Given the tuple currently held on the apply + // thread and the request, is this a native bind, an identical no-op repeat, or a client + // release-current the server must record without forwarding? + enum class EglBindAction { NativeBind, RepeatNoOp, ClientRelease }; + EglBindAction ClassifyEglMakeCurrent(Bool haveCurrent, EGLDisplay curDpy, EGLSurface curDraw, + EGLSurface curRead, EGLContext curCtx, EGLDisplay dpy, + EGLSurface draw, EGLSurface read, EGLContext ctx); + // --------------------------------------------------------------------------------- // THE EGL OWNERSHIP MOVE - the part that can sink the phase, expressed as twelve calls // --------------------------------------------------------------------------------- From 0244fc4a50d096f8ac9841ad2d2dade1f8239fbf Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 07:45:29 -0400 Subject: [PATCH 2/9] [Fix] (MG_Backend/Init): ShutdownSplitRoles stops the apply thread and drops the server backend on the early ClientSession::Start-failure path (M-6), and the consumer-mask cross-check re-runs after the client object exists (m-6) --- MobileGL/MG_Backend/Init.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/MobileGL/MG_Backend/Init.cpp b/MobileGL/MG_Backend/Init.cpp index e3d7f9ab..451b9423 100644 --- a/MobileGL/MG_Backend/Init.cpp +++ b/MobileGL/MG_Backend/Init.cpp @@ -181,6 +181,17 @@ namespace MobileGL::MG_Backend { // owns. A var-tail still named by an unapplied record is a use-after-free the join is // what prevents, which is why the order is not a preference. MG_Remote::Client::ClientSessionInstance().Stop(); + // M-6: ClientSession::Stop's !m_started arm (a Start that FAILED after + // ServerSession::Accept - a refused Accept, an invalid cmd/reply ring) tears down only + // the client half and never stops the apply thread or drops the server's private backend, + // which ServerLoop::CreateBackend already built and which holds the process-wide + // g_resourceOps. So call ServerLoop::Stop() here unconditionally. It is idempotent: on the + // started path ClientSession::Stop already joined the thread, so this hits Stop's + // !joinable arm, which resets a backend that never ran a thread and is otherwise a no-op. + // Without this an early Start failure leaves BackendObject_DirectGLES permanently alive + // and every later split bring-up in the process fails at CreateBackend's m_backend!=null + // guard. + MG_Remote::Server::ServerLoopInstance().Stop(); } #endif @@ -204,6 +215,13 @@ 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 + // pActiveBackendObject (the client's BackendObject_Remote) exists and its + // Initialize() has run inside InitSpecificBackendLibs. Anything the client object + // registered into MGPipeSetResourceOps after the step-2 check is invisible to that + // first call; re-asserting here costs one call and closes the window in which a + // client-registered g_resourceOps would flip the answer under the applier's feet. + AssertConsumerMaskIsHonest(ConsumedSubsystemsFor(MG_Config::ActiveBackendType)); LogBackendInfo(); return; } From 0fde95357cded7b6f090835de1d409196e2701b0 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 07:45:29 -0400 Subject: [PATCH 3/9] [Fix] (DirectGLES/Managers): under an active transport the server staged copy is the only base - liveHostBase no longer falls back to the client object (M-2); the pool-reuse and whole-store respecify readers go through RequireCoverage (M-3); a twin surviving DropAll has its freed hostBytes nulled (m-5) --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 51 +++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index a9adb1d4..39af5460 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -2926,6 +2926,19 @@ namespace MobileGL::MG_Backend::DirectGLES { // fallback for the drains that have NO object (the readback flush), and it is // nulled at both of those events. const auto liveHostBase = [&resource, &bufferObject]() -> const Uint8* { +#if MOBILEGL_BUILD_DISAGGREGATED + // M-2 / codex 3 / ID-52 item 3: under an ACTIVE TRANSPORT the server's staged + // copy (resource->hostBytes, filled by MGL_SERVER_STAGED_ADOPT in Ops_H_SubData / + // Ops_H_FlushRange) is the ONLY authoritative base. Preferring the frontend + // object's MappedData() here - which under inproc is always non-null, same process + // - meant every reduced-path drain read the CLIENT's shadow and NEVER the server's, + // so a corrupt staged upload rendered the frontend's correct bytes and the R-2.5 + // 0xDD audit could not reach a draw. R-2 / table 3: honest inproc is inproc that + // does not read the client object's memory. Under monolith nothing changes. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + return resource->hostBytes; + } +#endif if (bufferObject) return bufferObject->MappedData(); return resource->hostBytes; }; @@ -2942,6 +2955,17 @@ namespace MobileGL::MG_Backend::DirectGLES { resource->persistentMapped = false; resource->persistentPtr = nullptr; resource->immutableStorage = false; +#if MOBILEGL_BUILD_DISAGGREGATED + // m-5 / codex 5: OnBackendContextDestroyed ran MGL_SERVER_STAGED_DROP_ALL(), which + // frees every server shadow but does NOT null the hostBytes that name them - so a + // twin that SURVIVES a context loss (this is the block that repairs it) still + // carries a base into the freed allocation. The two other drop sites pair the drop + // with something that makes the base unreachable (Ops_H_Destroy retires the twin; + // the map-persistent site nulls hostBytes on the next line); DropAll did neither. + // Null it here, at the one place a surviving twin is re-armed, so no freed base + // reaches glBufferData/glBufferSubData before the next content record refills it. + resource->hostBytes = nullptr; +#endif } // An immutable store nothing maps any more, retired here on the thread that can. @@ -2977,6 +3001,14 @@ namespace MobileGL::MG_Backend::DirectGLES { resource->storageInitialized = true; 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 + // coverage are not the application's - uploading them is the silent data loss + // the M-6 ruling forbids. RequireCoverage is a no-op for the legacy arm's + // MappedData() and for a non-copying store; under split it Fatals by name on a + // sparse shadow rather than seeding the driver with zeroes. + MGL_SERVER_STAGED_REQUIRE(*resource, liveHostBase(), 0, poolSize, + "pool_reuse_whole_store"); g_GLESFuncs.glBufferSubData(TempBufferTarget, 0, (GLsizeiptr)poolSize, liveHostBase()); if (MG_Util::PipeStats::Enabled()) { MG_Util::PipeStats::AddBytes(MG_Util::PipeStats::ByteClass::StageBuffer, @@ -3050,8 +3082,21 @@ namespace MobileGL::MG_Backend::DirectGLES { const Bool shadowHasContent = bufferObject ? bufferObject->HasDefinedContent() : (record->Desc.HasDefinedContent != 0); const void* initialData = shadowHasContent ? hostBase : nullptr; + // M-3 / codex 4: a RespecifyStorageWith(..., initialData != nullptr) is a WHOLE-STORE + // [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 + // coverage does not span the store, instead of uploading its zero-fill as content. + const auto requireWholeStoreCoverage = [&]() { + if (initialData != nullptr) { + MGL_SERVER_STAGED_REQUIRE(*resource, static_cast(initialData), 0, + size, "respecify_whole_store"); + } + }; if (resource->pendingRespecify || !resource->storageInitialized || resource->storageSize != size) { + requireWholeStoreCoverage(); RespecifyStorageWith(*resource, size, usage, initialData, serial); } else if (!resource->pendingRanges.empty()) { // Same rule as Ops_H_Readback's, at the draw-time drain: with no frontend object @@ -3062,6 +3107,7 @@ namespace MobileGL::MG_Backend::DirectGLES { } else if (resource->syncedChangeSerial != serial) { // Mutations this backend could not track (the table was unregistered between // contexts); re-upload everything. + requireWholeStoreCoverage(); RespecifyStorageWith(*resource, size, usage, initialData, serial); } return resource; @@ -3113,6 +3159,11 @@ namespace MobileGL::MG_Backend::DirectGLES { resource->persistentMapped = false; resource->persistentPtr = nullptr; resource->immutableStorage = false; +#if MOBILEGL_PIPE_PUSH + // m-5 / codex 5: mirror the handle arm - DropAll freed the shadow this base named + // on context loss, so a surviving twin repaired here must not carry it forward. + resource->hostBytes = nullptr; +#endif } // An immutable store nothing maps any more: a respecification of a buffer that From 7a925b32594f2f01e1f84b1da47a8ed8a8acde7e Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 07:45:29 -0400 Subject: [PATCH 4/9] [Fix] (DirectGLES): the seven backend format-capability reads use the server-private cache via ActiveBackendFormatCaps under split, not pActiveBackendObject (C6); guarded so the pull build is byte-identical --- .../DirectGLES/BackendObject_DirectGLES.cpp | 14 ++++++ MobileGL/MG_Backend/DirectGLES/Utils.cpp | 45 +++++++++++++++++++ MobileGL/MG_Backend/DirectGLES/Utils.h | 13 ++++++ 3 files changed, 72 insertions(+) diff --git a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp index b36af187..2598df93 100644 --- a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp @@ -812,6 +812,19 @@ namespace MobileGL::MG_Backend::DirectGLES { Int maxSamples = 0; const SizeT formatIndex = static_cast(logicalFormat); +#if MOBILEGL_BUILD_DISAGGREGATED + // C6 / ID-52: read the ROLE's own cache. Under split that is the server's private backend + // (ActiveBackendFormatCaps), not pActiveBackendObject, which holds the client's mirror. + const FormatCapabilityCache* activeCaps = ActiveBackendFormatCaps(); + if (activeCaps != nullptr && targetIndex < kFormatCapabilityTargetCount && + formatIndex < kFormatCapabilityFormatCount) { + // Descending, so the head is the largest count this device actually allocated. + const Vector& probedCounts = activeCaps->SampleCounts[targetIndex][formatIndex]; + if (!probedCounts.empty()) { + maxSamples = probedCounts.front(); + } + } +#else if (pActiveBackendObject && targetIndex < kFormatCapabilityTargetCount && formatIndex < kFormatCapabilityFormatCount) { // Descending, so the head is the largest count this device actually allocated. @@ -821,6 +834,7 @@ namespace MobileGL::MG_Backend::DirectGLES { maxSamples = probedCounts.front(); } } +#endif if (maxSamples <= 0) { maxSamples = GetGLESFormatMaxSamples(g_GLESCapabilities, logicalFormat, imageFormat); } diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.cpp b/MobileGL/MG_Backend/DirectGLES/Utils.cpp index fa645890..7cfe864f 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Utils.cpp @@ -10,6 +10,9 @@ #include "Utils.h" #include "Managers.h" #include "MG_Backend/BackendObjects.h" +#if MOBILEGL_BUILD_DISAGGREGATED +#include +#endif #include "MG_Util/Converters/GLToMG/FramebufferEnumConverter.h" #include "MG_Util/SelfTest/DriverBugProbes.h" #include "MG_Util/Texture/TextureFormatProcessor.h" @@ -33,6 +36,21 @@ #include namespace MobileGL::MG_Backend::DirectGLES { +#if MOBILEGL_BUILD_DISAGGREGATED + const FormatCapabilityCache* ActiveBackendFormatCaps() { + // Under a live split session the SERVER's private backend is what owns the context on the + // apply thread (and these reads all run there), so its cache is the authoritative one. + // Before the session lands, or under monolith transport in a disaggregated build, fall + // back to the process global exactly as the monolith path always has. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + if (MG_Backend::BackendObject* server = MG_Remote::Server::ServerLoopInstance().Backend()) { + return &server->GetFormatCapabilities(); + } + } + return pActiveBackendObject ? &pActiveBackendObject->GetFormatCapabilities() : nullptr; + } +#endif + namespace { Flags GetForcedPixelFormatNormalizeOptions() { Flags options; @@ -71,15 +89,26 @@ namespace MobileGL::MG_Backend::DirectGLES { SizeT targetIndex, Bool caveat, FormatCapability capability) { +#if MOBILEGL_BUILD_DISAGGREGATED + const FormatCapabilityCache* activeCaps = ActiveBackendFormatCaps(); + if (activeCaps == nullptr || targetIndex >= kFormatCapabilityTargetCount) { + return false; + } +#else if (!pActiveBackendObject || targetIndex >= kFormatCapabilityTargetCount) { return false; } +#endif const SizeT formatIndex = static_cast(internalFormat); if (formatIndex >= kFormatCapabilityFormatCount) { return false; } +#if MOBILEGL_BUILD_DISAGGREGATED + const FormatCapabilityCache& cache = *activeCaps; +#else const FormatCapabilityCache& cache = pActiveBackendObject->GetFormatCapabilities(); +#endif const FormatCapabilityFlags caps = caveat ? cache.CaveatCaps[targetIndex][formatIndex] : cache.FullCaps[targetIndex][formatIndex]; return HasFormatCapability(caps, capability); @@ -123,7 +152,11 @@ namespace MobileGL::MG_Backend::DirectGLES { using namespace MobileGL::MG_Util::TextureFormatProcessor; const GLenum requestedInternalFormat = MG_Util::ConvertTextureInternalFormatToGLEnum(internalFormat); Flags options; +#if MOBILEGL_BUILD_DISAGGREGATED + if (ActiveBackendFormatCaps() == nullptr || ShouldUseCaveatFormat(internalFormat, targetIndex)) { +#else if (!pActiveBackendObject || ShouldUseCaveatFormat(internalFormat, targetIndex)) { +#endif options = GetRuntimeFallbackNormalizeOptions( requestedInternalFormat, TextureImpl::GetRenderTargetNormalizeOptions(g_GLESCapabilities, targetIndex)); @@ -217,9 +250,15 @@ namespace MobileGL::MG_Backend::DirectGLES { // not be resolved yet - a probe run then would latch "cannot tell" as "clean" // forever. Once the backend exists, the first narrow-format image this process // creates runs the probe on a live context. +#if MOBILEGL_BUILD_DISAGGREGATED + if (ActiveBackendFormatCaps() == nullptr) { + return false; + } +#else if (pActiveBackendObject == nullptr) { return false; } +#endif return MG_Util::SelfTest::CopyImageMirrorsPacked16FieldOrder(g_GLESFuncs); } @@ -257,9 +296,15 @@ namespace MobileGL::MG_Backend::DirectGLES { if (!TargetRequiresRenderableFormat(targetIndex)) { return false; } +#if MOBILEGL_BUILD_DISAGGREGATED + if (ActiveBackendFormatCaps() != nullptr && !ShouldUseCaveatFormat(internalFormat, targetIndex)) { + return false; + } +#else if (pActiveBackendObject && !ShouldUseCaveatFormat(internalFormat, targetIndex)) { return false; } +#endif const GLenum requestedInternalFormat = MG_Util::ConvertTextureInternalFormatToGLEnum(internalFormat); const Flags options = GetRuntimeFallbackNormalizeOptions( requestedInternalFormat, GetRenderTargetNormalizeOptions(g_GLESCapabilities, targetIndex)); diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.h b/MobileGL/MG_Backend/DirectGLES/Utils.h index ae7f4af0..ffedd745 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.h +++ b/MobileGL/MG_Backend/DirectGLES/Utils.h @@ -13,6 +13,19 @@ #include namespace MobileGL::MG_Backend::DirectGLES { +#if MOBILEGL_BUILD_DISAGGREGATED + // C6 / ID-52 / CONTRACT-P5 table 3's pActiveBackendObject row. The format-capability cache of + // THIS ROLE's backend. Under an active transport the server's own private + // BackendObject_DirectGLES owns the context on the apply thread, so a backend-internal format + // lookup must read ITS probed cache - not pActiveBackendObject's, which under split is the + // CLIENT's BackendObject_Remote mirror (a caps snapshot, generation-lagged, and on an + // independent server not usable at all). Monolith build/transport: pActiveBackendObject, so a + // pull build never sees this symbol. Null when no backend is up. The seven table-3 reads + // (five in Utils.cpp, ClampSamplesToBackendSupport in BackendObject_DirectGLES.cpp) go through + // this instead of dereferencing pActiveBackendObject directly. + const FormatCapabilityCache* ActiveBackendFormatCaps(); +#endif + namespace DebugImpl { class ErrorLopper { public: From 61d6cd8e30e9e08f14e38d5662099186f598187d Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 07:45:29 -0400 Subject: [PATCH 5/9] [Fix] (MG_Remote/Server): OnReadPixels reads with neutral pack state into a tight w*h*bpp reply so a server-visible pack state cannot overflow the slot (ID-49); the present<->swap 1:1 is a convention not structural (m-1); the reply-ordering invariant is documented against ReplySlot.h (m-7) --- MobileGL/MG_Remote/Server/PipeApplier.cpp | 93 +++++++++++++++++++---- 1 file changed, 80 insertions(+), 13 deletions(-) diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index 2afe9416..a50451b2 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -14,7 +14,10 @@ #include #include +#include +#include #include +#include #include #include @@ -148,10 +151,18 @@ namespace MobileGL::MG_Remote::Server { if (table->Present == nullptr) return false; // Present is the ONLY frame-boundary drain the backend has (DirectGLES.cpp:12424-12470: // the fence poll, the four ring OnPresent hooks, TrimBufferPool, PipeStats::OnPresent), - // which is why ARCHITECTURE.md:531 insists present <-> eglSwapBuffers stays strictly - // 1:1. It is NOT eglSwapBuffers itself: the swap is the client's EGL call and crosses - // as the SwapEGLBuffers control request, which runs Present on this thread through this - // same table. Both paths therefore end here and the 1:1 is structural. + // which is why ARCHITECTURE.md:531 wants present <-> eglSwapBuffers to stay 1:1. + // + // m-1: THAT 1:1 IS A CONVENTION c1 UPHOLDS, NOT A STRUCTURAL GUARANTEE, and the earlier + // claim that it was structural is wrong. This Present() is reached ONLY from a present + // RECORD (ServerVerbSink::OnPresent). ServerSwapEGLBuffers does NOT reach it - it calls + // backend->SwapEGLBuffers -> BackendObject::SwapEGLBuffers -> eglSwapBuffers, and never + // Present() - so the two paths do NOT both end here. The frame count staying in step with + // the swap count rests entirely on c1 emitting exactly one present record per swap; + // nothing here compares Presents() to a swap count. If that drifts, the frame fence and + // TrimBufferPool's recycle watermark stop tracking frames - which is the reason the 1:1 + // was wanted, recorded here so a future swap-without-present is looked for rather than + // assumed impossible. Presents() is exposed for a lane that wants to make the comparison. table->Present(); ++m_presents; // FrameSerial 0 means "the server stamps its own" (c1-v1 8.3): P5 has no client-side @@ -184,19 +195,75 @@ namespace MobileGL::MG_Remote::Server { replies->PostReply(seq, Wire::ReplySink::kStatusError, nullptr, 0); return false; } - // THE CLIENT DECLARES THE BYTE COUNT AND THE SERVER DOES NOT RECOMPUTE IT. DstSize is - // sized on the client from the same GL_PACK_* state the frontend owns, and it is what - // the client will read back out of the slot; a server that recomputed from Box x - // Format x Type would be a SECOND opinion about a pack alignment the client half owns, - // and the two disagreeing is a short read with plausible pixels in it. - if (info.DstSize > m_readbackScratch.size()) { - m_readbackScratch.resize(static_cast(info.DstSize)); + // ID-49: THE REPLY CROSSES TIGHT AND PACK STATE NEVER CROSSES FOR A READ. The server reads + // with NEUTRAL pack state - ROW_LENGTH 0, SKIP_ROWS/PIXELS/IMAGES 0, ALIGNMENT 1 - into a + // w*h*bytesPerPixel extent that IS the reply payload, and restores the pack state + // afterwards; the CLIENT scatters those tight rows into the application's pointer per its + // own GL_PACK_* state (c1's half). Reading with the client's pack state HERE was the + // codex-1 blocker: the backend's ReadPixels honours ROW_LENGTH/SKIP_* and writes PAST a + // DstSize the client sized without the initial skip (a 4x3 RGBA8 read with ROW_LENGTH=8, + // SKIP_ROWS=1, SKIP_PIXELS=2 allocates 80 and lands its last write at 120), which is the + // two DepthReadbackHonoursThePackPixelStoreParameters SEGFAULTs the census omitted. THE + // DstSize FORMULA BOTH SIDES AGREE ON: w * h * bytesPerPixel. A non-default server-visible + // pack state can no longer change either the reply's size or its bytes. + const SizeT bytesPerPixel = MG_Util::GetInputBytesPerPixel( + MG_Util::ConvertGLEnumToTextureInputFormat(static_cast(info.Format)), + MG_Util::ConvertGLEnumToTexturePixelDataType(static_cast(info.Type))); + // Tight size = w*h*bpp, the whole of ID-49's formula. If this build cannot size the + // (format, type) pair (bpp == 0) it trusts the client's DstSize - a neutral read still + // cannot overflow it via row length or skips, and an unsizeable pair is c1's + // Fatal{UnsizedReadback} at emission, not this side's. + const Uint64 tight = bytesPerPixel != 0 + ? static_cast(info.Box.W) * static_cast(info.Box.H) * + static_cast(bytesPerPixel) + : info.DstSize; + if (bytesPerPixel != 0 && tight != info.DstSize) { + // Both halves compute w*h*bpp under ID-49, so a disagreement is the two sides + // disagreeing about the frame. Read (and post) the tight extent this side owns rather + // than the client's number, so a wrong DstSize can never make this a short read into + // uninitialised scratch. + MGLOG_E_ONCE("MG_Remote server: read_pixels DstSize %llu != tight w*h*bpp %llu " + "(%ux%u, bpp %zu); reading the tight extent (ID-49)", + static_cast(info.DstSize), + static_cast(tight), info.Box.W, info.Box.H, + bytesPerPixel); } + if (tight > m_readbackScratch.size()) { + m_readbackScratch.resize(static_cast(tight)); + } + + // Save the server-visible pack state, force neutral for the read, restore. Both go through + // the applier's own set_pixel_pack_state entry point (MGPipeApplySetPixelPackState writes + // gPipeInputs.m_pixelStore[0], which the backend's ReadPixels reads via + // MGB_CTX->GetPixelStoreParameters); the read is synchronous on this thread, so the window + // in which the pack state is neutral does not outlive the call. + const MG_Pipe::PixelStoreParameters savedPack = + MG_Pipe::gPipeInputs.GetPixelStoreParameters(/*isUnpack=*/false); + MG_Pipe::MGPPixelPackState neutralPack{}; + neutralPack.Pack.RowLength = 0; + neutralPack.Pack.SkipRows = 0; + neutralPack.Pack.SkipPixels = 0; + neutralPack.Pack.SkipImages = 0; + neutralPack.Pack.Alignment = 1; + MG_Pipe::MGPipeApplySetPixelPackState(neutralPack); + table->GL.ReadPixels(info.Box.X, info.Box.Y, static_cast(info.Box.W), static_cast(info.Box.H), static_cast(info.Format), static_cast(info.Type), m_readbackScratch.data()); - replies->PostReply(seq, Wire::ReplySink::kStatusOk, m_readbackScratch.data(), info.DstSize); - m_readbackBytes += info.DstSize; + + MG_Pipe::MGPPixelPackState restorePack{}; + restorePack.Pack = savedPack; + MG_Pipe::MGPipeApplySetPixelPackState(restorePack); + + // m-7: the answer is written into the slot HERE, mid-apply, while the verb stamp is still + // up - and that is safe for exactly one reason, which is the contract's and is stated so it + // is not mistaken for luck: the client reaches a reply slot ONLY through appliedSeq + // (ReplySlot.h's ORDERING clause), never by polling the slot's own stamp, and s1's + // SessionConsumer::ApplyOne publishes appliedSeq only AFTER PipeApplier::ApplyOne has run + // LeaveApplier() and (on the joint tree) dropped the ScopedApplierEntry. So by the time the + // client is allowed to look at this slot, the apply-side gPipeInputs flag is already down. + replies->PostReply(seq, Wire::ReplySink::kStatusOk, m_readbackScratch.data(), tight); + m_readbackBytes += tight; ++m_readbacks; return true; } From 05bcb734776f22c42f2917be296b1b7f1671c603 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 07:45:29 -0400 Subject: [PATCH 6/9] [Fix] (MGPipe): resource_flush_range carries no companion bytes under an active transport - the consumer reads the server-owned StagedShadowStore instead of faulting the null carrier as corruption (item-12, R-13.2/ID-37) --- MobileGL/MG_Pipe/PipeApply.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/MobileGL/MG_Pipe/PipeApply.cpp b/MobileGL/MG_Pipe/PipeApply.cpp index 15b3e96a..1cf852d9 100644 --- a/MobileGL/MG_Pipe/PipeApply.cpp +++ b/MobileGL/MG_Pipe/PipeApply.cpp @@ -1870,7 +1870,20 @@ namespace MobileGL::MG_Pipe { if (stored == nullptr) return; const char* fault = BufferRangeFault(record.Offset, record.Size, stored->Desc.Width); - if (fault == nullptr && record.Size != 0 && bytes == nullptr) { + // item-12 / R-13.2 / ID-37: the "a non-empty flush carries no bytes" fault is a MONOLITH + // rule. Under an ACTIVE TRANSPORT resource_flush_range carries no companion pointer AT ALL + // (contract table 1 row 20 / R-13.2): the covering resource_subdata already staged those + // bytes into the server-owned StagedShadowStore (R-11), and Ops_H_FlushRange reads that + // shadow, not the record. So a null `bytes` on a non-empty flush is the NORMAL split case, + // not corruption - and a flush whose bytes were genuinely never staged is caught more + // precisely downstream by R-11's Fatal{StageSnapshotTooNarrow}, which names the missing + // subdata rather than the flush. The check stays exact under monolith + // (MG_Config::Transport is a constexpr Monolith in a non-disaggregated build, so this is + // `&& true` there and the codegen is unchanged). This was the only seam between the joint + // inproc lane's 14/21 and 21/21: all seven PersistentCoherentMapScenario entries aborted + // here (c1-v2.md 6). + if (fault == nullptr && record.Size != 0 && bytes == nullptr && + MG_Config::Transport == MG_Config::TransportMode::Monolith) { fault = "a non-empty flush carries no bytes"; } if (fault != nullptr) { From a85aa954017a140c0588fbfd453c7c35ceb177fe Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 07:45:29 -0400 Subject: [PATCH 7/9] [Test] (MG_Test/Wire): ServerLoopTest - the R-11 production gate through the real ops table (M-1), the C2/M-6/M-7/C6/C7 controls, actual-blocked-park arming (C10), an explicit-mask affinity case (M-4), and the coverage death control naming SIGABRT (m-2) --- MobileGL/MG_Test/Wire/ServerLoopTest.cpp | 268 +++++++++++++++++++++-- 1 file changed, 251 insertions(+), 17 deletions(-) diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index db84ea37..dd41a82f 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -29,6 +29,11 @@ // exists to test: bytes that were copied survive the source being overwritten with 0xDD. #include +#include +#include +#include +#include +#include #include #include #include @@ -41,8 +46,11 @@ #include #include #include +#include #include +#include + #include #include @@ -184,6 +192,23 @@ namespace { return producer.WaitForApplied(seq, timeoutMs) == Transport::SessionWait::Reached; } + // C10: poll the flag the Doorbell sets ONLY while it is actually blocked in Park + // (RingControl::consumerParked, stored inside Doorbell::Wait's blocking section and + // cleared on wake), NOT ServerLoop::ParkCount(), which increments on the way TOWARD a park + // and stays set even if Wait returns without ever blocking. A loop whose wait was replaced + // by `true` never sets this, so a poll for it TIMES OUT - which is what makes "the apply + // thread really parked" a claim that can go red for its own reason. + bool WaitUntilTrulyParked(Uint32 timeoutMs = 5000) { + Transport::RingControl* control = clientSegments.CmdControl(); + if (control == nullptr) return false; + const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(timeoutMs); + while (std::chrono::steady_clock::now() < deadline) { + if (control->consumerParked.load(std::memory_order_acquire) == 1u) return true; + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } + return false; + } + void Stop() { // Table 3's order: the client publishes and lets the server drain (EmitAndWait // already did), then Doorbell::Kill through the transport's Shutdown, THEN the @@ -226,12 +251,12 @@ TEST(ServerLoopTest, AControlRequestRunsOnTheApplyThreadAndUnparksIt) { ASSERT_TRUE(fixture.StartLoop()); Server::ServerLoop& loop = Server::ServerLoopInstance(); - const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); - while (loop.ParkCount() == 0 && std::chrono::steady_clock::now() < deadline) { - std::this_thread::sleep_for(std::chrono::milliseconds(1)); - } - ASSERT_GT(loop.ParkCount(), 0u) - << "the apply thread never parked, so this case would prove nothing about waking it"; + // C10: arm on the ACTUAL blocked park (consumerParked), not ParkCount() - a loop that spun + // instead of blocking would pass a ParkCount() check without the wait ever mattering, which + // was the whole finding. + ASSERT_TRUE(fixture.WaitUntilTrulyParked()) + << "the apply thread never entered the blocking park, so this case would prove nothing " + "about waking it (ParkCount() counts an intention, not a park)"; struct Probe { std::thread::id ranOn{}; @@ -296,11 +321,10 @@ TEST(ServerLoopTest, StopKillsTheDoorbellJoinsAndTheThreadReallyExits) { ASSERT_TRUE(fixture.StartLoop()); Server::ServerLoop& loop = Server::ServerLoopInstance(); - const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); - while (loop.ParkCount() == 0 && std::chrono::steady_clock::now() < deadline) { - std::this_thread::sleep_for(std::chrono::milliseconds(1)); - } - ASSERT_GT(loop.ParkCount(), 0u) << "the thread must be parked for this to be a shutdown test"; + // C10: the same actual-park arming - a shutdown test whose thread never blocked would not be + // exercising the lost-wakeup path table 3 step 2 is about. + ASSERT_TRUE(fixture.WaitUntilTrulyParked()) + << "the thread must be BLOCKED in the park for this to be a shutdown test"; ASSERT_TRUE(loop.Running()); const auto began = std::chrono::steady_clock::now(); @@ -323,11 +347,7 @@ TEST(ServerLoopTest, TheAffinityStringResolvesToAMaskAndTheMaskIsLogged) { ServerFixture fixture; ASSERT_TRUE(fixture.Handshake()); ASSERT_TRUE(fixture.StartLoop()); - const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); - while (Server::ServerLoopInstance().ParkCount() == 0 && - std::chrono::steady_clock::now() < deadline) { - std::this_thread::sleep_for(std::chrono::milliseconds(1)); - } + ASSERT_TRUE(fixture.WaitUntilTrulyParked()); EXPECT_EQ(Server::ServerLoopInstance().ResolvedAffinityMask(), 0u) << "`off` did not resolve to no-affinity"; @@ -336,11 +356,50 @@ TEST(ServerLoopTest, TheAffinityStringResolvesToAMaskAndTheMaskIsLogged) { << "the apply thread did not log its resolved affinity mask; an affinity that silently " "did nothing is indistinguishable from one that worked"; EXPECT_NE(log.find("mgl-srv-apply started"), std::string::npos); + // M-4: `off` must be a RECOGNISED string, not fall through to the numeric-parse "not + // recognised" arm. Deleting the `off` branch in RequestedAffinityMask makes off unrecognised; + // this asserts it did not, so that branch's deletion goes red here (its own reason). + EXPECT_EQ(log.find("is not `auto`, `off` or a number"), std::string::npos) + << "`off` was treated as an unrecognised affinity string; its own branch is gone"; fixture.Stop(); MG_Config::Ipc.ServerAffinity = saved; } +// M-4 / codex 11, the other half: an EXPLICIT mask must resolve to itself and be LOGGED as the +// EFFECTIVE mask the kernel took, not the requested one. `off` -> 0 and `0x3` -> 0x3 are two +// answers that must DIFFER, so the resolver cannot be a constant; and reading the mask back with +// sched_getaffinity is what makes a cpuset that trimmed the request visible (codex 11), which the +// requested-mask log hid. Red once by making ApplyAffinity return `requested`: the effective read +// is what a trimmed request diverges from, and the explicit-mask assert is what catches a broken +// resolver. +TEST(ServerLoopTest, TheExplicitAffinityMaskResolvesToItselfAndIsLogged) { +#if defined(__linux__) || defined(__ANDROID__) + if (std::thread::hardware_concurrency() < 2) { + GTEST_SKIP() << "needs at least 2 online cpus to honour 0x3"; + } + const String saved = MG_Config::Ipc.ServerAffinity; + MG_Config::Ipc.ServerAffinity = "0x3"; // cpu 0 and cpu 1, both online on any 2+-cpu box + + ServerFixture fixture; + ASSERT_TRUE(fixture.Handshake()); + ASSERT_TRUE(fixture.StartLoop()); + ASSERT_TRUE(fixture.WaitUntilTrulyParked()); + + EXPECT_EQ(Server::ServerLoopInstance().ResolvedAffinityMask(), 0x3u) + << "an explicit mask the box can honour did not resolve to itself (0x3); the effective " + "mask read back from the kernel differs from the request, or the resolver is broken"; + const std::string log = ReadLog(); + EXPECT_NE(log.find("RESOLVED mask 0x3"), std::string::npos) + << "the logged RESOLVED mask is not the effective 0x3"; + + fixture.Stop(); + MG_Config::Ipc.ServerAffinity = saved; +#else + GTEST_SKIP() << "affinity is a Linux/Android facility"; +#endif +} + // ===================================================================================== // The record path: stamp, apply, watermark // ===================================================================================== @@ -591,13 +650,188 @@ TEST(StagedShadowTest, ADrainOutsideTheStagedCoverageIsFatalByName) { // legacy arm, whose MappedData() is valid for the whole store. store.RequireCoverage(&key, bytes.data(), 0, 64, "unit"); - EXPECT_DEATH(store.RequireCoverage(&key, base, 0, 64, "unit_out_of_range"), ""); + // m-2: name the death MODE and the diagnostic, not just "the process died". The empty regex + // accepted any death, including a SIGSEGV inside RequireCoverage; KilledBySignal(SIGABRT) pins + // it to the Fatal's abort() (a segfault is SIGSEGV and fails this), and the log grep names the + // exact wording. The log flush is pinned: Log.cpp's WriteToFile fflushes after every write and + // MGLOG_F logs before abort(), so the line is on disk in the forked child before it dies. + EXPECT_EXIT(store.RequireCoverage(&key, base, 0, 64, "unit_out_of_range"), + ::testing::KilledBySignal(SIGABRT), ".*"); const std::string log = ReadLog(); EXPECT_NE(log.find("Fatal{StageSnapshotTooNarrow, \"unit_out_of_range\"}"), std::string::npos) << "the abort happened but not for this rule's reason; the log says: " << log; } #endif +// ===================================================================================== +// R-11's PRODUCTION wiring (M-1 / codex 8), the EGL lifecycle seams (C2/M-6/M-7/C6/C7) +// ===================================================================================== + +// M-1 / codex 8: the R-11 gate that drives the PRODUCTION path - the real resource op table +// installed by RegisterBufferBackendOps, dispatched through MGPipeGetResourceOps()->SubData, i.e. +// the exact Managers.cpp:2118 call site R-11 changed - and NOT StagedShadowStore in isolation. +// StagedShadowTest stays as the container test; this is the gate that goes red under the two +// perturbations the reviewer used (restore `hostBytes = raw - offset`, neuter the coverage clamp). +#if !defined(_WIN32) +TEST(StagedShadowProductionTest, SubDataThroughTheRealOpsTableCopiesAndSurvivesTheSourcePoison) { + // MG_Config::Transport is InProcess (main), so ServerStaged() latches its copying arm on. + MG_Backend::DirectGLES::BufferImpl::RegisterBufferBackendOps(); + const MG_Pipe::MGPipeResourceOps* ops = MG_Pipe::MGPipeGetResourceOps(); + ASSERT_NE(ops, nullptr) << "RegisterBufferBackendOps did not install the resource op table"; + ASSERT_NE(ops->SubData, nullptr); + + MG_Pipe::MGPipeHandle res{}; + res.Slot = 7; + res.Gen = 1; + auto* twin = MG_Backend::DirectGLES::BufferImpl::GetOrCreateBufferResourceForHandle(res); + ASSERT_NE(twin, nullptr); + + Vector src(32, Uint8{0xAB}); + MG_Pipe::MGPSubData rec{}; + rec.Res = res; + rec.Target = MG_Pipe::MGPipePackSubDataTarget(MG_Pipe::kMGPipeResourceTargetBuffer, 0u); + ASSERT_TRUE(MG_Pipe::MGPipeSetSubDataBufferRange(rec, 0, src.size())); + // THE PRODUCTION CALL. Not StagedShadowStore::Adopt directly - the whole point of M-1. + ops->SubData(res, rec, src.data()); + + auto* found = MG_Backend::DirectGLES::BufferImpl::FindBufferResourceForHandle(res); + ASSERT_NE(found, nullptr); + ASSERT_NE(found->hostBytes, nullptr) << "Ops_H_SubData recorded no base at all"; + EXPECT_NE(found->hostBytes, static_cast(src.data())) + << "hostBytes points into the CLIENT's staging bytes (raw - offset), the exact rule-C " + "violation R-11 exists to fix - restore that line and this goes red"; + + // w1's retired-stage poison, by hand and at the right moment: the record has 'retired', so the + // client's staging run is dead. A server that copied still reads the original bytes. + std::fill(src.begin(), src.end(), Uint8{0xDD}); + for (SizeT i = 0; i < src.size(); ++i) { + ASSERT_EQ(found->hostBytes[i], 0xAB) + << "byte " << i << " of the server base is the poison: Ops_H_SubData kept a pointer " + "into SEG_STAGE instead of copying"; + } + MG_Backend::DirectGLES::BufferImpl::UnregisterBufferBackendOps(); +} +#endif + +// C2: after the loop has stopped, a forwarder call must return NOT_INITIALIZED and must NOT run +// inline on the caller. The pre-fix code read m_running outside the control mutex and ran work +// inline for !m_running - which in the race the verifier's latch harness reproduced +// (v1-codex-verify.md C2) hung forever posting into a mailbox no thread pumps. The fix clears +// m_running UNDER the mutex and re-checks it there; this is the deterministic half of it (no +// latch needed: after Stop() m_running is false for certain). +TEST(ServerLoopTest, AForwarderCallAfterTheLoopStoppedReturnsNotInitializedAndDoesNotRunInline) { + ServerFixture fixture; + ASSERT_TRUE(fixture.Handshake()); + ASSERT_TRUE(fixture.StartLoop()); + ASSERT_TRUE(fixture.WaitUntilTrulyParked()); + fixture.Stop(); + ASSERT_FALSE(Server::ServerLoopInstance().Running()); + + struct Probe { + Bool ran = false; + } probe; + const MobileGLResult rc = Server::ServerLoopInstance().RunOnApplyThread( + +[](void* user) -> MobileGLResult { + static_cast(user)->ran = true; + return MOBILEGL_OK; + }, + &probe); + + EXPECT_EQ(rc, MOBILEGL_ERR_NOT_INITIALIZED) + << "a forwarder call after Stop() did not return NOT_INITIALIZED (old code ran it inline)"; + EXPECT_FALSE(probe.ran) << "the work ran on the caller after the loop stopped"; +} + +// M-7: the OTHER !m_running arm - a backend built but no thread (Start refused / std::thread +// threw) - must be a NAMED Fatal, never a silent inline EGL-on-the-app-thread fallback. A split +// lane that ran eglMakeCurrent on the app thread would render correctly with the context on the +// wrong thread, which is the one outcome R-1 exists to make impossible. +#if !defined(_WIN32) +TEST(ServerLoopTest, AForwarderWithABackendButNoThreadIsFatalNotAnAppThreadFallback) { + Server::ServerLoop& loop = Server::ServerLoopInstance(); + ASSERT_EQ(loop.CreateBackend(BackendType::DirectGLES), MOBILEGL_OK); + ASSERT_FALSE(loop.Running()); + // A backend exists, no apply thread runs. A forwarder must abort by name. + EXPECT_EXIT(Server::ServerReleaseEGLResources(), ::testing::KilledBySignal(SIGABRT), ".*"); + const std::string log = ReadLog(); + EXPECT_NE(log.find("Fatal{ApplyThreadNotRunning"), std::string::npos) + << "the abort was not the M-7 named Fatal; the log says: " << log; + loop.Stop(); // drop the backend in the parent +} +#endif + +// M-6: ServerLoop::Stop() with no thread ever started must still DROP the private backend, which +// ShutdownSplitRoles relies on for the early-Start-failure path. If it does not, the server's +// BackendObject stays alive with g_resourceOps pointing into it and every later CreateBackend in +// the process is refused - which is exactly the leak M-6 names. +TEST(ServerLoopTest, StopWithoutAStartedThreadStillDropsThePrivateBackend) { + Server::ServerLoop& loop = Server::ServerLoopInstance(); + ASSERT_EQ(loop.CreateBackend(BackendType::DirectGLES), MOBILEGL_OK); + ASSERT_NE(loop.Backend(), nullptr); + loop.Stop(); // the !joinable arm + EXPECT_EQ(loop.Backend(), nullptr) << "Stop() left the private backend alive with no thread"; + EXPECT_FALSE(loop.Running()); + // A second bring-up must now succeed; CreateBackend refuses if m_backend != nullptr. + EXPECT_EQ(loop.CreateBackend(BackendType::DirectGLES), MOBILEGL_OK) + << "the leaked backend blocks the next split bring-up (CreateBackend's m_backend guard)"; + loop.Stop(); +} + +// C6: the server's format-capability accessor reads the SERVER's own backend cache, not the +// process global (which under split is the CLIENT's BackendObject_Remote). Pointer-identity: no +// mask injection needed - ActiveBackendFormatCaps() must return the server's cache address, not +// the global's. Red once by making ActiveBackendFormatCaps return pActiveBackendObject's. +TEST(ServerLoopTest, TheServerFormatCapsAccessorReadsTheServersOwnBackendNotTheGlobal) { + Server::ServerLoop& loop = Server::ServerLoopInstance(); + ASSERT_EQ(loop.CreateBackend(BackendType::DirectGLES), MOBILEGL_OK); + MG_Backend::BackendObject* server = loop.Backend(); + ASSERT_NE(server, nullptr); + + // A DIFFERENT object in the global the seven C6 reads used to follow. + auto client = MakeUnique(); + MG_Backend::BackendObject* clientRaw = client.get(); + MG_Backend::pActiveBackendObject = std::move(client); + + const MG_Backend::FormatCapabilityCache* caps = MG_Backend::DirectGLES::ActiveBackendFormatCaps(); + EXPECT_EQ(caps, &server->GetFormatCapabilities()) + << "ActiveBackendFormatCaps returned the process global's cache; under split that is the " + "CLIENT's BackendObject_Remote, not the server's private backend"; + EXPECT_NE(caps, &clientRaw->GetFormatCapabilities()); + + loop.Stop(); + MG_Backend::pActiveBackendObject.reset(); +} + +// C7 / ID-54: the make-current decision. A new tuple binds; an identical repeat is a no-op; a +// release request (the three NO_* markers) is recorded, not forwarded. The native-bind COUNT is a +// joint-lane control (it needs a real EGL context); this is the pure decision, which is what +// keeps "the context is bound once and held for life" from being a per-call storm. Red once by +// deleting the RepeatNoOp arm - an identical repeat then classifies as NativeBind. +TEST(ServerLoopTest, MakeCurrentClassifiesRepeatAndReleaseWithoutRebinding) { + const EGLDisplay dpy = reinterpret_cast(0x1); + const EGLSurface draw = reinterpret_cast(0x2); + const EGLSurface read = reinterpret_cast(0x2); + const EGLContext ctx = reinterpret_cast(0x3); + + // Nothing current yet: a genuine bind. + EXPECT_EQ(Server::ClassifyEglMakeCurrent(false, EGL_NO_DISPLAY, EGL_NO_SURFACE, EGL_NO_SURFACE, + EGL_NO_CONTEXT, dpy, draw, read, ctx), + Server::EglBindAction::NativeBind); + // The SAME tuple already held: a no-op, no native bind, no owner write. + EXPECT_EQ(Server::ClassifyEglMakeCurrent(true, dpy, draw, read, ctx, dpy, draw, read, ctx), + Server::EglBindAction::RepeatNoOp) + << "an identical make-current was classified as a native rebind; the owner would be " + "written twice and the caches invalidated for nothing"; + // A DIFFERENT tuple: a real rebind. + const EGLContext otherCtx = reinterpret_cast(0x9); + EXPECT_EQ(Server::ClassifyEglMakeCurrent(true, dpy, draw, read, ctx, dpy, draw, read, otherCtx), + Server::EglBindAction::NativeBind); + // A release request, whatever is current: recorded, not forwarded (the context is held). + EXPECT_EQ(Server::ClassifyEglMakeCurrent(true, dpy, draw, read, ctx, dpy, EGL_NO_SURFACE, + EGL_NO_SURFACE, EGL_NO_CONTEXT), + Server::EglBindAction::ClientRelease); +} + 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 455b475c5df374084d43588f7759b382ccb6a2aa Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 08:03:52 -0400 Subject: [PATCH 8/9] [Fix] (DirectGLES/Managers, MG_Remote/Server): m-5 nulls a stale hostBytes only when its server shadow is actually gone (StagedShadowStore::HasShadow), not on a twins first-ensure generation sync - the unguarded null dropped a subdatas freshly staged base and made the reduced-path VBO read shifted; HasShadow unit control added --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 26 +++++++++++---------- MobileGL/MG_Remote/Server/StagedShadow.h | 13 +++++++++++ MobileGL/MG_Test/Wire/ServerLoopTest.cpp | 19 +++++++++++++++ 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 39af5460..48cca6f1 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -2958,13 +2958,16 @@ namespace MobileGL::MG_Backend::DirectGLES { #if MOBILEGL_BUILD_DISAGGREGATED // m-5 / codex 5: OnBackendContextDestroyed ran MGL_SERVER_STAGED_DROP_ALL(), which // frees every server shadow but does NOT null the hostBytes that name them - so a - // twin that SURVIVES a context loss (this is the block that repairs it) still - // carries a base into the freed allocation. The two other drop sites pair the drop - // with something that makes the base unreachable (Ops_H_Destroy retires the twin; - // the map-persistent site nulls hostBytes on the next line); DropAll did neither. - // Null it here, at the one place a surviving twin is re-armed, so no freed base - // reaches glBufferData/glBufferSubData before the next content record refills it. - resource->hostBytes = nullptr; + // twin that SURVIVES a context loss still carries a base into the freed allocation. + // Null it here, where a surviving twin is re-armed - but ONLY when the shadow is + // really gone. This block also runs on a twin's FIRST ensure (contextGeneration + // starts mismatched), and there the shadow a preceding resource_subdata just staged + // 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)) { + resource->hostBytes = nullptr; + } #endif } @@ -3159,12 +3162,11 @@ namespace MobileGL::MG_Backend::DirectGLES { resource->persistentMapped = false; resource->persistentPtr = nullptr; resource->immutableStorage = false; -#if MOBILEGL_PIPE_PUSH - // m-5 / codex 5: mirror the handle arm - DropAll freed the shadow this base named - // on context loss, so a surviving twin repaired here must not carry it forward. - resource->hostBytes = nullptr; -#endif } + // m-5: the LEGACY arm (EnsureBufferResource) is reached only under monolith/push, where + // liveHostBase reads the frontend object's MappedData rather than a server shadow, so + // there is no freed server base to null here - the split freed-base hazard lives in + // EnsureBufferResourceForHandle above, guarded by HasShadow. // An immutable store nothing maps any more: a respecification of a buffer that // had been persistently mapped, which Ops_Respecify could not retire because it diff --git a/MobileGL/MG_Remote/Server/StagedShadow.h b/MobileGL/MG_Remote/Server/StagedShadow.h index 77e63b75..0a37f6f3 100644 --- a/MobileGL/MG_Remote/Server/StagedShadow.h +++ b/MobileGL/MG_Remote/Server/StagedShadow.h @@ -133,6 +133,19 @@ namespace MobileGL::MG_Remote::Server { return m_shadows.size(); } + // Is there STILL a live shadow for this key? The m-5 discriminator: after DropAll (context + // loss) the key is erased, so a twin's cached hostBytes names freed memory and must be + // nulled; after an ordinary Adopt the key is present and its base is live. The + // generation-reset block runs on a twin's FIRST ensure too (the generation starts + // mismatched), and there the shadow a preceding subdata just staged is present - so nulling + // must key on THIS answer, not on the generation change alone, or the reduced path drops + // its own bytes. + Bool HasShadow(const void* key) const { + if (!m_any.load(std::memory_order_acquire)) return false; + const std::lock_guard lock(m_mutex); + return m_shadows.find(key) != m_shadows.end(); + } + // Adjacent ranges merge - there is no gap between them, so the union really is one run. // Ranges with a gap do NOT merge, and that is the whole mechanism: it is what makes a // missing record detectable instead of papered over. diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index dd41a82f..455afa7c 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -617,6 +617,25 @@ TEST(StagedShadowTest, CoverageIsExactAndAGapIsNotCovered) { EXPECT_TRUE(store.IsCovered(&key, 0, 48)); } +// m-5's discriminator, at unit scope: HasShadow is TRUE for a key that was Adopted and FALSE once +// DropAll has run - which is exactly what tells the generation-reset block whether a twin's +// hostBytes names a live server shadow (keep it) or a freed one (null it). A version that answered +// "always live" would let a freed base reach the driver; "always gone" would drop a base a subdata +// just staged in the same generation (that regression really happened - TriangleScenario read a +// shifted VBO). Both directions are asserted here. +TEST(StagedShadowTest, HasShadowIsTrueAfterAdoptAndFalseAfterTheShadowIsDropped) { + Server::StagedShadowStore store(/*copies=*/true); + const int key = 0; + Vector bytes(16, Uint8{0x44}); + EXPECT_FALSE(store.HasShadow(&key)) << "nothing staged yet"; + store.Adopt(&key, 16, bytes.data(), 0, 16); + EXPECT_TRUE(store.HasShadow(&key)) << "a staged key must read live, or the reset block nulls a " + "base a subdata just filled"; + store.DropAll(); + EXPECT_FALSE(store.HasShadow(&key)) << "after DropAll the base is freed and must read gone, or " + "a surviving twin hands the driver a dangling pointer"; +} + TEST(StagedShadowTest, DropForgetsOneResourceAndDropAllForgetsEveryOne) { Server::StagedShadowStore store(/*copies=*/true); const int a = 0; From c9d84e33c5e51075e9c6ec7374740fb314cc755c Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 08:11:05 -0400 Subject: [PATCH 9/9] [Fix] (DirectGLES/Managers): under an active transport a resource_subdata that finds no twin MINTS one (GetOrCreateBufferResourceForHandle) so its bytes reach the server StagedShadowStore - the twin is otherwise lazy until the first draw (D-A2) and the reduced-path VBO bytes were lost, so the milestone frame rendered a shifted store; monolith keeps the lazy twin (R-11/ID-52 item 3) --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 48cca6f1..37f937a1 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -2093,6 +2093,21 @@ namespace MobileGL::MG_Backend::DirectGLES { const SizeT offset = static_cast(MG_Pipe::MGPipeSubDataBufferOffset(record)); const SizeT size = static_cast(MG_Pipe::MGPipeSubDataBufferSize(record)); auto* resource = FindBufferResourceForHandle(res); +#if MOBILEGL_BUILD_DISAGGREGATED + // R-11 / ID-52 item 3: under an ACTIVE TRANSPORT this record's bytes are the ONLY + // delivery of the buffer's content - the client sends no companion pointer and the + // server has no MappedData to fall back on. But the twin is created LAZILY at the + // first draw (Ops_H_Create is a no-op by D-A2), which is AFTER this record, so a + // subdata that finds no twin would return here and the bytes would be lost - the + // draw then uploads an empty/shifted store (TriangleScenario read a blue triangle + // before this). So under split the subdata MINTS the twin it needs to stage into. + // Monolith is untouched: the twin stays lazy there because the frontend object's + // MappedData is the source and nothing is lost by deferring the allocation (D-A2). + if (resource == nullptr && bytes != nullptr && + MG_Config::Transport != MG_Config::TransportMode::Monolith) { + resource = GetOrCreateBufferResourceForHandle(res); + } +#endif if (!resource) return; // M-2: UNDER pendingMutex, because this line runs BEFORE the CanTouchGLNow() // test below - i.e. on the arm D-A2 deliberately keeps reachable off the render