Merge commit 'b5c60d5a015c68348725ae93297b13a8fff27723' into codex/p5f-wave2

MobileGL/MG_Remote/Protocol/SurfaceOpCodec.cpp

@@ -65,6 +65,7 @@ namespace MobileGL::MG_Remote {
         case SurfaceWireError::InprocOnlyOpOnTheWire: return "InprocOnlyOpOnTheWire";
         case SurfaceWireError::WindowKindNamesNoBackend: return "WindowKindNamesNoBackend";
         case SurfaceWireError::UnknownWindowKind: return "UnknownWindowKind";
+        case SurfaceWireError::MetalLayerArrived: return "MetalLayerArrived";
         case SurfaceWireError::AndroidNativeWindowArrived: return "AndroidNativeWindowArrived";
         }
         return "<unknown SurfaceWireError>";
@@ -159,6 +160,10 @@ namespace MobileGL::MG_Remote {
                 // arrival is P12; until then this is refused by name at ServerApplyWireSurfaceOp.
                 return SurfaceWireError::AndroidNativeWindowArrived;
             }
+            if (op.windowKind() == ::MobileGL::Wire::WindowKind::MetalLayer) {
+                // CAMetalLayer* is a client-process object address, just like ANativeWindow*.
+                return SurfaceWireError::MetalLayerArrived;
+            }
             MG_Backend::WindowBackend backend;
             if (!WindowBackendForWireWindowKind(op.windowKind(), &backend)) {
                 return ::flatbuffers::IsOutRange(op.windowKind(), ::MobileGL::Wire::WindowKind::None,
@@ -199,6 +204,10 @@ namespace MobileGL::MG_Remote {
                         "CLIENT's process and means nothing here. Real window arrival is P12; "
                         "until then the spawn surface path is pbuffer/surfaceless only",
                         ::MobileGL::Wire::EnumNameSurfaceOpKind(op.kind()));
+            } else if (error == SurfaceWireError::MetalLayerArrived) {
+                MGLOG_F("MGPipe: Fatal{UnmigratedSurface, \"MetalLayer@P12\"} - a wire "
+                        "SurfaceOp named a CAMetalLayer* in the CLIENT's process. "
+                        "Real window arrival is P12");
             } else {
                 MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"SurfaceOp\"} - a wire surface op "
                         "failed validation: %s (wire kind %u, window kind %u)",

MobileGL/MG_Remote/Protocol/SurfaceOpCodec.h

@@ -7,17 +7,17 @@
 // End of Source File Header
 
 // P5f, package fc: the codec between the inproc SurfaceControlFrame and the wire's
-// Wire::SurfaceOp / Wire::SurfaceReply (protocol.fbs).
+// ::MobileGL::Wire::SurfaceOp / ::MobileGL::Wire::SurfaceReply (protocol.fbs).
 //
 // The frame is the inproc payload (Server/SurfaceControlFrame.h); this is the ONLY place the
 // frame meets the schema. Nothing in the inproc hot path encodes - the frame crosses the one-slot
 // channel by value - but the codec is what makes "the same struct encodes under spawn" a tested
 // claim rather than a plan, and it owns the two schema seams that are NOT integer translations:
 //
-//   * SurfaceControlOp <-> Wire::SurfaceOpKind: numerically aligned for the ten framed ops and
+//   * SurfaceControlOp <-> ::MobileGL::Wire::SurfaceOpKind: numerically aligned for the ten framed ops and
 //     PINNED by static_asserts in the .cpp, but mapped through an explicit table. The four
 //     inproc-only kinds have no wire kind and refuse to encode by name.
-//   * MG_Backend::WindowBackend <-> Wire::WindowKind: the two enums do not agree (WindowKind's
+//   * MG_Backend::WindowBackend <-> ::MobileGL::Wire::WindowKind: the two enums do not agree (WindowKind's
 //     Surfaceless/Pbuffer are surface shapes, not window backends; WindowBackend's MetalLayer had
 //     no wire enumerator until fc added it), so the mapping is a table with named failures, never
 //     a cast (f0-egl census, schema gap 3).
@@ -28,7 +28,8 @@
 // Fatal{UnmigratedSurface, "AndroidNativeWindow@P12"} (the ANativeWindow* means nothing in the
 // server's process; real window arrival is P12) and everything else dies
 // Fatal{ProtocolCorruption, "SurfaceOp"}. P6's control pump is the intended caller; P5f's unit
-// tests drive it directly.
+// tests drive it directly. MetalLayerArrived likewise dies
+// Fatal{UnmigratedSurface, "MetalLayer@P12"}: CAMetalLayer* is also process-local.
 
 #pragma once
 #include <Includes.h>
@@ -53,6 +54,7 @@ namespace MobileGL::MG_Remote {
         InprocOnlyOpOnTheWire,   // a kind that may ride the inproc channel but never the wire
         WindowKindNamesNoBackend,// Surfaceless/Pbuffer where a window backend is required
         UnknownWindowKind,       // a WindowKind the mapping table does not know
+        MetalLayerArrived,       // -> Fatal{UnmigratedSurface, "MetalLayer@P12"}
         AndroidNativeWindowArrived, // -> Fatal{UnmigratedSurface, "AndroidNativeWindow@P12"}
     };
 
@@ -60,15 +62,15 @@ namespace MobileGL::MG_Remote {
 
     // frame.kind <-> wire kind. Both answer false (and leave *out untouched) for None, the four
     // inproc-only kinds, and any wire value the schema does not define.
-    bool WireKindForSurfaceControlOp(Server::SurfaceControlOp op, Wire::SurfaceOpKind* out);
-    bool SurfaceControlOpForWireKind(Wire::SurfaceOpKind kind, Server::SurfaceControlOp* out);
+    bool WireKindForSurfaceControlOp(Server::SurfaceControlOp op, ::MobileGL::Wire::SurfaceOpKind* out);
+    bool SurfaceControlOpForWireKind(::MobileGL::Wire::SurfaceOpKind kind, Server::SurfaceControlOp* out);
 
     // The explicit WindowBackend <-> WindowKind table. WireWindowKindForWindowBackend also maps
     // WindowBackend::Unknown to WindowKind::None; WindowBackendForWireWindowKind maps
     // WindowKind::None back to Unknown and answers FALSE for Surfaceless/Pbuffer (surface shapes,
     // not window backends).
-    bool WireWindowKindForWindowBackend(MG_Backend::WindowBackend backend, Wire::WindowKind* out);
-    bool WindowBackendForWireWindowKind(Wire::WindowKind kind, MG_Backend::WindowBackend* out);
+    bool WireWindowKindForWindowBackend(MG_Backend::WindowBackend backend, ::MobileGL::Wire::WindowKind* out);
+    bool WindowBackendForWireWindowKind(::MobileGL::Wire::WindowKind kind, MG_Backend::WindowBackend* out);
 
     // Encode one frame as a complete CtrlEnvelope{SurfaceOp} buffer (the builder is left
     // Finished). Fails BEFORE touching the builder for a kind that may not cross.
@@ -77,17 +79,17 @@ namespace MobileGL::MG_Remote {
 
     // Validate + decode a wire SurfaceOp into a frame. Pure - it never aborts; the Fatal
     // decisions belong to ServerApplyWireSurfaceOp. The reply half of the frame is reset.
-    SurfaceWireError DecodeWireSurfaceOp(const Wire::SurfaceOp& op, Server::SurfaceControlFrame* frame);
+    SurfaceWireError DecodeWireSurfaceOp(const ::MobileGL::Wire::SurfaceOp& op, Server::SurfaceControlFrame* frame);
 
     // The reply direction, same shape (CtrlEnvelope{SurfaceReply}).
     void EncodeSurfaceReplyFrame(const Server::SurfaceControlFrame& frame,
                                  flatbuffers::FlatBufferBuilder* builder);
-    void DecodeWireSurfaceReply(const Wire::SurfaceReply& reply, Server::SurfaceControlFrame* frame);
+    void DecodeWireSurfaceReply(const ::MobileGL::Wire::SurfaceReply& reply, Server::SurfaceControlFrame* frame);
 
     // The server-side wire entry: decode, then post the frame through ServerLoop's channel and
     // hand the reply half back. A decode error is a NAMED Fatal (see the header block), never a
     // dropped message. No transport calls this yet - P6's control pump will.
-    MobileGLResult ServerApplyWireSurfaceOp(const Wire::SurfaceOp& op,
+    MobileGLResult ServerApplyWireSurfaceOp(const ::MobileGL::Wire::SurfaceOp& op,
                                             Server::SurfaceControlFrame* replyOut);
 
 } // namespace MobileGL::MG_Remote

MobileGL/MG_Remote/Server/ServerLoop.cpp

@@ -562,7 +562,9 @@ namespace MobileGL::MG_Remote::Server {
         // The dispatch runs OUTSIDE the lock, exactly as the old work(user) did, and fills the
         // frame's reply half; the reply is then published back into the slot under the lock so
         // the poster's copy-out after m_controlDone sees it.
-        const MobileGLResult result = ApplySurfaceControlFrame(frame);
+        const MobileGLResult result = ApplySurfaceControlFrame(
+            frame, m_controlProbeHook.load(std::memory_order_acquire),
+            m_controlProbeUser.load(std::memory_order_acquire));
         {
             const std::lock_guard<std::mutex> lock(m_controlMutex);
             m_controlFrame = frame;
@@ -650,10 +652,9 @@ namespace MobileGL::MG_Remote::Server {
 
     MobileGLResult ServerLoop::RunSurfaceControlFrame(SurfaceControlFrame& frame) {
         if (frame.kind == SurfaceControlOp::None) return MOBILEGL_ERR_INVALID_ARGUMENT;
-        // Minted here and echoed back with the reply half: inproc it is purely diagnostic; under
-        // spawn it is how a SurfaceReply names its op. Atomic rather than under m_callerMutex
-        // because the re-entrant arm below mints without taking that lock.
-        frame.seq = m_controlSeq.fetch_add(1, std::memory_order_acq_rel) + 1;
+        // Zero marks an unnumbered local request. A wire request is already numbered by its
+        // client and MUST retain that number all the way through dispatch and the reply.
+        if (frame.seq == 0) frame.seq = m_controlSeq.fetch_add(1, std::memory_order_acq_rel) + 1;
         // RE-ENTRANCY IS NOT A DEADLOCK. The teardown path posts from the apply thread itself -
         // ~BackendObject_DirectGLES reaches ReleaseEGLResources - and so does anything the
         // applier calls that wants "run this where the context is". Running inline is the
@@ -661,6 +662,10 @@ namespace MobileGL::MG_Remote::Server {
         if (OnApplyThread()) return ApplySurfaceControlFrame(frame);
 
         const std::lock_guard<std::mutex> callerLock(m_callerMutex);
+        return PostSurfaceControlFrameWithCallerLock(frame);
+    }
+
+    MobileGLResult ServerLoop::PostSurfaceControlFrameWithCallerLock(SurfaceControlFrame& frame) {
         std::unique_lock<std::mutex> 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
@@ -717,14 +722,16 @@ namespace MobileGL::MG_Remote::Server {
     }
 
     MobileGLResult ServerLoop::RunProbeOnApplyThreadForTesting(ControlProbeHook hook, void* user) {
-        // The hook pair is published BEFORE the frame: the mailbox handshake (publish under
-        // m_controlMutex, shadow last, then the bell) orders these two relaxed-typed stores for
-        // the apply thread, which reads them inside the dispatch.
-        m_controlProbeHook.store(hook, std::memory_order_release);
-        m_controlProbeUser.store(user, std::memory_order_release);
         SurfaceControlFrame frame;
         frame.kind = SurfaceControlOp::ProbeForTesting;
-        return RunSurfaceControlFrame(frame);
+        frame.seq = m_controlSeq.fetch_add(1, std::memory_order_acq_rel) + 1;
+        // Nested probes never touch the shared pair: another poster may already be waiting,
+        // and its arguments must survive a probe run inline on the apply thread.
+        if (OnApplyThread()) return ApplySurfaceControlFrame(frame, hook, user);
+        const std::lock_guard<std::mutex> callerLock(m_callerMutex);
+        m_controlProbeHook.store(hook, std::memory_order_release);
+        m_controlProbeUser.store(user, std::memory_order_release);
+        return PostSurfaceControlFrameWithCallerLock(frame);
     }
 
     Uint64 ServerLoop::ControlFramesDispatched() const {
@@ -870,15 +877,15 @@ namespace MobileGL::MG_Remote::Server {
 
     } // namespace
 
-    MobileGLResult ServerLoop::ApplySurfaceControlFrame(SurfaceControlFrame& frame) {
+    MobileGLResult ServerLoop::ApplySurfaceControlFrame(SurfaceControlFrame& frame,
+                                                        ControlProbeHook probeHook, void* probeUser) {
         // The frame channel's own tally, bumped for EVERY dispatch including probes: a forwarder
         // that stopped posting frames (the fc red-once revert shape) leaves this unmoved, which
         // is what ServerLoopTest's AVoidForwarderCrossesAsOneDispatchedFrame reads.
         m_controlFramesDispatched.fetch_add(1, std::memory_order_acq_rel);
         if (frame.kind == SurfaceControlOp::ProbeForTesting) {
-            const ControlProbeHook hook = m_controlProbeHook.load(std::memory_order_acquire);
-            if (hook == nullptr) return MOBILEGL_ERR_INVALID_ARGUMENT;
-            return hook(m_controlProbeUser.load(std::memory_order_acquire));
+            if (probeHook == nullptr) return MOBILEGL_ERR_INVALID_ARGUMENT;
+            return probeHook(probeUser);
         }
         // Null means the hook never ran or the session is already down. Every op answers
         // NOT_INITIALIZED rather than dereferencing: a client that calls eglMakeCurrent on a
@@ -970,6 +977,7 @@ namespace MobileGL::MG_Remote::Server {
         }
         case SurfaceControlOp::SetSwapInterval:
             backend->SetEGLSwapInterval(frame.swapInterval);
+            frame.ok = true;
             return MOBILEGL_OK;
         case SurfaceControlOp::ReleaseSurface: {
             const EGLSurface surface = HandleFromToken<EGLSurface>(frame.surface);
@@ -980,6 +988,7 @@ namespace MobileGL::MG_Remote::Server {
             // costs one forwarded bind; remembering when it did not would cost a silent
             // no-context-current.
             ForgetCurrentTupleIfItNames(surface);
+            frame.ok = true;
             return MOBILEGL_OK;
         }
         case SurfaceControlOp::ReleaseResources:
@@ -994,9 +1003,11 @@ namespace MobileGL::MG_Remote::Server {
             // by deleting it: ServerLoopTest's recreate control reads NativeBindCount() == 1
             // where 2 is required.
             ForgetCurrentTuple();
+            frame.ok = true;
             return MOBILEGL_OK;
         case SurfaceControlOp::SetWindowHandle:
             backend->SetWindowHandle(UnpackWindowHandle(frame));
+            frame.ok = true;
             return MOBILEGL_OK;
         case SurfaceControlOp::InitCapabilitiesInprocOnly:
             // Inproc-only (f0-egl §4.2): on the wire the ANSWER to this op is the CapsSnapshot

MobileGL/MG_Remote/Server/ServerLoop.h

@@ -370,7 +370,11 @@ namespace MobileGL::MG_Remote::Server {
         // The dispatch, on the apply thread (or inline for a re-entrant post): executes the
         // frame's op against the private backend and fills the frame's reply half. The eleven
         // wire+inproc op bodies from the old forwarder Args structs live here now.
-        MobileGLResult ApplySurfaceControlFrame(SurfaceControlFrame& frame);
+        MobileGLResult ApplySurfaceControlFrame(SurfaceControlFrame& frame,
+                                                 ControlProbeHook probeHook = nullptr, void* probeUser = nullptr);
+        // The caller already holds m_callerMutex; shared by normal and test-probe posting so
+        // the probe's metadata is protected for the same lifetime as its frame.
+        MobileGLResult PostSurfaceControlFrameWithCallerLock(SurfaceControlFrame& frame);
         // Pops and applies every record currently in the ring; returns how many it applied.
         Uint64 DrainRing();
         void SignalExited();
@@ -418,8 +422,8 @@ namespace MobileGL::MG_Remote::Server {
         std::atomic<Uint64> m_controlSeq{0};
         std::atomic<Uint64> m_controlFramesDispatched{0};
         // The test probe (RunProbeOnApplyThreadForTesting). Members, NOT slot content - the slot
-        // carries only the frame. Written before the probe frame is published, so the mailbox's
-        // own release/acquire handshake orders them for the apply thread.
+        // carries only the frame. Written under m_callerMutex before the probe frame is
+        // published; held until its reply returns. Re-entrant probes use call-local arguments.
         std::atomic<ControlProbeHook> m_controlProbeHook{nullptr};
         std::atomic<void*> m_controlProbeUser{nullptr};
 

MobileGL/MG_Remote/Server/SurfaceControlFrame.h

@@ -16,9 +16,9 @@
 // blocking, one-slot) channel by value; under spawn the SAME struct is what SurfaceOpCodec
 // encodes into a Wire::SurfaceOp. The thread model does not move - only the payload's shape.
 //
-// THE FRAME CARRIES NO POINTERS, and that is enforced by construction (the static_assert below)
-// rather than by review: a field that is a pointer would also be a compile error here. The three
-// places the old Args structs held client addresses map onto values:
+// THE FRAME CARRIES NO POINTERS, and that is enforced by the exhaustive member check below:
+// adding a member requires updating its structured binding; pointer types fail the assertion.
+// The three places the old Args structs held client addresses map onto values:
 //
 //   ServerInitializeEGLDisplay's major/minor out-pointers -> the reply fields eglMajor/eglMinor;
 //   ServerCreateEGLWindowSurface / ServerSetWindowHandle's const WindowHandle* -> windowBackend +
@@ -28,10 +28,9 @@
 //     under spawn the client mints dense tokens instead (P6-CONTRACT-DRAFT table 0) - the field
 //     width is already the token's, so nothing here changes shape on that day.
 //
-// AN ANativeWindow* ON THE WIRE IS REFUSED BY NAME, not carried: nativeToken is only meaningful
-// inside the client's process for AndroidNativeWindow, and a spawn-side decode of that kind dies
-// with Fatal{UnmigratedSurface, "AndroidNativeWindow@P12"} (SurfaceOpCodec). Real window arrival
-// is P12.
+// PROCESS-LOCAL WINDOW OBJECTS ARE REFUSED BY NAME on the wire: ANativeWindow* and
+// CAMetalLayer* mean nothing in the server process. SurfaceOpCodec names their refusals
+// AndroidNativeWindow@P12 and MetalLayer@P12; real window arrival is P12.
 
 #pragma once
 #include <Includes.h>
@@ -93,13 +92,34 @@ namespace MobileGL::MG_Remote::Server {
         Int eglMinor = 0;
     };
 
-    // THE INVARIANT THE PACKAGE EXISTS FOR. The frame crosses by VALUE - copied into the one-slot
-    // channel inproc, encoded into a FlatBuffers table under spawn - so it can never carry an
-    // address: no pointer members, no ownership, trivially copyable, standard layout.
+    namespace Detail {
+        // Pointers are trivially copyable and standard-layout too. Only these value types are
+        // allowed; the constexpr negatives pin the distinction from the old, insufficient check.
+        template <typename T>
+        inline constexpr bool IsSurfaceControlValue = std::is_integral_v<T> || std::is_enum_v<T>;
+        static_assert(!IsSurfaceControlValue<void*> && !IsSurfaceControlValue<void (*)()>);
+
+        constexpr bool SurfaceControlFrameHasOnlyValues() {
+            SurfaceControlFrame frame{};
+            // Exhaustive by the language's aggregate decomposition rule: adding ANY member
+            // without extending this binding is a compile error, so new fields cannot evade it.
+            const auto& [kind, seq, display, surface, readSurface, context, windowBackend,
+                         nativeToken, width, height, swapInterval, ok, eglMajor, eglMinor] = frame;
+            return IsSurfaceControlValue<decltype(kind)> && IsSurfaceControlValue<decltype(seq)> &&
+                   IsSurfaceControlValue<decltype(display)> && IsSurfaceControlValue<decltype(surface)> &&
+                   IsSurfaceControlValue<decltype(readSurface)> && IsSurfaceControlValue<decltype(context)> &&
+                   IsSurfaceControlValue<decltype(windowBackend)> && IsSurfaceControlValue<decltype(nativeToken)> &&
+                   IsSurfaceControlValue<decltype(width)> && IsSurfaceControlValue<decltype(height)> &&
+                   IsSurfaceControlValue<decltype(swapInterval)> && IsSurfaceControlValue<decltype(ok)> &&
+                   IsSurfaceControlValue<decltype(eglMajor)> && IsSurfaceControlValue<decltype(eglMinor)>;
+        }
+    } // namespace Detail
+
+    static_assert(Detail::SurfaceControlFrameHasOnlyValues(),
+                  "every surface control frame member must be an integer or enum, never a pointer");
     static_assert(std::is_trivially_copyable_v<SurfaceControlFrame> &&
                       std::is_standard_layout_v<SurfaceControlFrame>,
-                  "the surface control frame must stay a plain bag of scalars - a pointer member "
-                  "would be a client address crossing to the server, which is what P5f removes");
+                  "the surface control frame must remain a trivially copyable value aggregate");
 
     const char* SurfaceControlOpName(SurfaceControlOp op);
 

MobileGL/MG_Test/Wire/ServerLoopTest.cpp

@@ -42,6 +42,7 @@
 #include <MG_Remote/CapsCodec.h>
 #include <MG_Remote/Client/ClientSession.h>
 #include <MG_Remote/Protocol/generated/protocol_generated.h>
+#include <MG_Remote/Protocol/SurfaceOpCodec.h>
 #include <MG_Remote/Server/PipeApplier.h>
 #include <MG_Remote/Server/ServerLoop.h>
 #include <MG_Remote/Server/ServerSession.h>
@@ -466,6 +467,72 @@ TEST(ServerLoopTest, APostFromTheApplyThreadItselfRunsInlineRatherThanDeadlockin
     fixture.Stop();
 }
 
+TEST(ServerLoopTest, WireSequenceSurvivesRealDispatchAndFailureReply) {
+    ServerFixture fixture;
+    ASSERT_TRUE(fixture.Handshake());
+    ASSERT_TRUE(fixture.StartLoop());
+    flatbuffers::FlatBufferBuilder builder(256);
+    const auto op = ::MobileGL::Wire::CreateSurfaceOp(
+        builder, 4242, ::MobileGL::Wire::SurfaceOpKind::SetSwapInterval);
+    const auto envelope = ::MobileGL::Wire::CreateCtrlEnvelope(
+        builder, ::MobileGL::Wire::CtrlMsg::SurfaceOp, op.Union());
+    ::MobileGL::Wire::FinishCtrlEnvelopeBuffer(builder, envelope);
+    Server::SurfaceControlFrame reply;
+    EXPECT_EQ(MG_Remote::ServerApplyWireSurfaceOp(
+                  *::MobileGL::Wire::GetCtrlEnvelope(builder.GetBufferPointer())->msg_as_SurfaceOp(), &reply),
+              MOBILEGL_ERR_NOT_INITIALIZED);
+    EXPECT_EQ(Server::ServerLoopInstance().ControlFramesDispatched(), 1u);
+    flatbuffers::FlatBufferBuilder replies(256);
+    MG_Remote::EncodeSurfaceReplyFrame(reply, &replies);
+    const auto* wireReply = ::MobileGL::Wire::GetCtrlEnvelope(replies.GetBufferPointer())->msg_as_SurfaceReply();
+    EXPECT_EQ(wireReply->seq(), 4242u);
+    EXPECT_FALSE(wireReply->ok());
+    fixture.Stop();
+}
+
+TEST(ServerLoopTest, ConcurrentProbesKeepTheirArgumentsAcrossNestedProbes) {
+    ServerFixture fixture;
+    ASSERT_TRUE(fixture.Handshake());
+    ASSERT_TRUE(fixture.StartLoop());
+    constexpr int kPosters = 8;
+    constexpr int kCalls = 128;
+    struct Probe {
+        std::atomic<int> calls{0};
+        std::atomic<int> nested{0};
+        std::atomic<int> errors{0};
+    } probes[kPosters];
+    std::atomic<bool> go{false};
+    std::vector<std::thread> posters;
+    for (auto& probe : probes) {
+        posters.emplace_back([&go, &probe] {
+            while (!go.load(std::memory_order_acquire)) std::this_thread::yield();
+            for (int i = 0; i < kCalls; ++i) {
+                const auto rc = Server::ServerLoopInstance().RunProbeOnApplyThreadForTesting(
+                    +[](void* user) -> MobileGLResult {
+                        auto* p = static_cast<Probe*>(user);
+                        p->calls.fetch_add(1);
+                        // Same argument type keeps a regressed mixed pair observable without UB.
+                        return Server::ServerLoopInstance().RunProbeOnApplyThreadForTesting(
+                            +[](void* nestedUser) -> MobileGLResult {
+                                static_cast<Probe*>(nestedUser)->nested.fetch_add(1);
+                                return MOBILEGL_OK;
+                            }, p);
+                    }, &probe);
+                if (rc != MOBILEGL_OK) probe.errors.fetch_add(1);
+            }
+        });
+    }
+    go.store(true, std::memory_order_release);
+    for (auto& poster : posters) poster.join();
+    for (const auto& probe : probes) {
+        EXPECT_EQ(probe.calls.load(), kCalls) << "another poster or nested probe replaced this hook/user pair";
+        EXPECT_EQ(probe.nested.load(), kCalls);
+        EXPECT_EQ(probe.errors.load(), 0);
+    }
+    EXPECT_EQ(Server::ServerLoopInstance().ControlFramesDispatched(), 2u * kPosters * kCalls);
+    fixture.Stop();
+}
+
 // The bounded join. The thread is parked on kWaitForever when Stop() is called, so this is the
 // exact lost-wakeup shape table 3 step 2 is about - and the bound is what turns a regression
 // into a red test rather than a wedged CI job.
@@ -1454,6 +1521,35 @@ namespace {
 
 } // namespace
 
+TEST(ServerLoopEglTest, SuccessfulVoidWireOperationsReplyWithSuccessAndOriginalSequence) {
+    EglServerFixture fixture;
+    MGL_EGL_BRING_UP_OR_BAIL(fixture);
+    // Release last: it deliberately destroys the native context used by the preceding ops.
+    const auto kinds = {::MobileGL::Wire::SurfaceOpKind::SetSwapInterval,
+                        ::MobileGL::Wire::SurfaceOpKind::SetWindowHandle,
+                        ::MobileGL::Wire::SurfaceOpKind::ReleaseSurface,
+                        ::MobileGL::Wire::SurfaceOpKind::ReleaseResources};
+    Uint64 seq = 9000;
+    for (const auto kind : kinds) {
+        flatbuffers::FlatBufferBuilder builder(256);
+        const auto op = ::MobileGL::Wire::CreateSurfaceOp(
+            builder, ++seq, kind, 0, 0, ::MobileGL::Wire::WindowKind::None, 0, 0, 0, 1);
+        const auto envelope = ::MobileGL::Wire::CreateCtrlEnvelope(
+            builder, ::MobileGL::Wire::CtrlMsg::SurfaceOp, op.Union());
+        ::MobileGL::Wire::FinishCtrlEnvelopeBuffer(builder, envelope);
+        Server::SurfaceControlFrame reply;
+        EXPECT_EQ(MG_Remote::ServerApplyWireSurfaceOp(
+                      *::MobileGL::Wire::GetCtrlEnvelope(builder.GetBufferPointer())->msg_as_SurfaceOp(), &reply),
+                  MOBILEGL_OK) << static_cast<int>(kind);
+        flatbuffers::FlatBufferBuilder replies(256);
+        MG_Remote::EncodeSurfaceReplyFrame(reply, &replies);
+        const auto* wireReply = ::MobileGL::Wire::GetCtrlEnvelope(replies.GetBufferPointer())->msg_as_SurfaceReply();
+        EXPECT_EQ(wireReply->seq(), seq) << static_cast<int>(kind);
+        EXPECT_TRUE(wireReply->ok()) << "successful void operation encoded failure: " << static_cast<int>(kind);
+    }
+    fixture.TearDown();
+}
+
 // C7 / ID-54, THE NATIVE HALF, measured where the review said it was not: at the driver. Surface
 // creation binds the context natively on the apply thread (that is bind #1 of the "2 per process"
 // the review counted); the client's first eglMakeCurrent onto that surface used to be bind #2 and is

MobileGL/MG_Test/Wire/SurfaceControlFrameTest.cpp

@@ -99,6 +99,17 @@ TEST(SurfaceControlFrameTest, TheFrameIsAPlainBagOfScalars) {
            "shapes must stay obvious to a reader";
 }
 
+TEST(SurfaceControlFrameTest, EveryMemberIsMechanicallyCheckedAsAnIntegerOrEnum) {
+    static_assert(Server::Detail::SurfaceControlFrameHasOnlyValues());
+    static_assert(!Server::Detail::IsSurfaceControlValue<void*>);
+    static_assert(!Server::Detail::IsSurfaceControlValue<void (*)()>);
+    EXPECT_TRUE(Server::Detail::SurfaceControlFrameHasOnlyValues());
+    // Why trivially-copyable alone did not enforce this invariant.
+    struct PointerBag { void* pointer; };
+    static_assert(std::is_trivially_copyable_v<PointerBag> && std::is_standard_layout_v<PointerBag>);
+    EXPECT_FALSE(Server::Detail::IsSurfaceControlValue<decltype(PointerBag::pointer)>);
+}
+
 TEST(SurfaceControlFrameTest, ExactlyTheTenFramedOpsHaveWireKindsAndTheValuesArePinned) {
     // The ten framed ops, in schema order. The numeric agreement with ::MobileGL::Wire::SurfaceOpKind is
     // asserted per op rather than trusted from the static_asserts, because THIS is the table a
@@ -322,6 +333,25 @@ TEST(SurfaceControlFrameTest, AnAndroidNativeWindowOnTheWireIsRefusedByName) {
 // A kind the schema does not define (a NEWER client's op reaching an OLDER server, or
 // corruption) is Fatal{ProtocolCorruption}, never ignored: an ignored control op is a client
 // waiting on a reply that never comes, which is a hang wearing a green lane.
+TEST(SurfaceControlFrameTest, AMetalLayerOnTheWireIsRefusedByName) {
+    // Both operations that consume a native window must refuse the process-local object.
+    for (const auto kind : {::MobileGL::Wire::SurfaceOpKind::CreateWindowSurface,
+                            ::MobileGL::Wire::SurfaceOpKind::SetWindowHandle}) {
+        flatbuffers::FlatBufferBuilder builder(256);
+        const auto op = ::MobileGL::Wire::CreateSurfaceOp(
+            builder, 19, kind, 1, 2, ::MobileGL::Wire::WindowKind::MetalLayer, 0x1234, 800, 600);
+        const auto envelope = ::MobileGL::Wire::CreateCtrlEnvelope(
+            builder, ::MobileGL::Wire::CtrlMsg::SurfaceOp, op.Union());
+        ::MobileGL::Wire::FinishCtrlEnvelopeBuffer(builder, envelope);
+        const auto* wireOp = ::MobileGL::Wire::GetCtrlEnvelope(builder.GetBufferPointer())->msg_as_SurfaceOp();
+        ASSERT_NE(wireOp, nullptr);
+        SurfaceControlFrame frame;
+        EXPECT_EQ(DecodeWireSurfaceOp(*wireOp, &frame), SurfaceWireError::MetalLayerArrived);
+        EXPECT_EXIT(ServerApplyWireSurfaceOp(*wireOp, nullptr), ::testing::KilledBySignal(SIGABRT), ".*");
+        EXPECT_NE(ReadLog().find("Fatal{UnmigratedSurface, \"MetalLayer@P12\"}"), std::string::npos);
+    }
+}
+
 TEST(SurfaceControlFrameTest, AnUnknownWireOpKindIsProtocolCorruptionByName) {
     flatbuffers::FlatBufferBuilder builder(256);
     const auto op = ::MobileGL::Wire::CreateSurfaceOp(builder, 9, static_cast<::MobileGL::Wire::SurfaceOpKind>(200), 1, 2,

docs/Disaggregated/notes/p5f/fc-contract-draft.md

@@ -17,11 +17,13 @@
    slot's content passes to the channel at publish; the dispatch on the apply thread fills the
    reply half (`ok`, `eglMajor`, `eglMinor`) into the same value; the poster's copy-out happens
    after the blocking handshake returns. No field is a pointer and none may become one — the
-   `static_assert(std::is_trivially_copyable_v<...>)` in `SurfaceControlFrame.h` is the
-   enforcement, not a courtesy.
-2. `seq` is minted by the channel's poster side (`RunSurfaceControlFrame`), monotonically from 1
-   per session, and echoed in the reply. Under spawn the client mints (P6-CONTRACT-DRAFT table 0);
-   the field does not change shape.
+   exhaustive constexpr structured binding in `SurfaceControlFrame.h` checks every member
+   as an integer or enum. Adding a member without extending the binding fails compilation;
+   changing a checked member to a pointer fails its static assertion. Trivially-copyable /
+   standard-layout remain separate copyability checks: neither trait rejects pointers.
+2. An unnumbered local request (`seq == 0`) is numbered by `RunSurfaceControlFrame`, from 1
+   per session. An already numbered wire request retains its client-minted sequence through
+   dispatch and reply (P6-CONTRACT-DRAFT table 0); the server never renumbers it.
 3. `display` / `surface` / `readSurface` / `context` are Uint64 **tokens**. Inproc they are the
    client's EGL handle values bit-cast — legal only because both roles share a process and a
    driver, and the N-3 tuple bookkeeping compares them by value. Under spawn they are
@@ -31,7 +33,8 @@
    names the **backend** enum, never the wire's `WindowKind`; the two enums meet only inside
    `SurfaceOpCodec.cpp`'s explicit tables (`Surfaceless`/`Pbuffer` are surface shapes and answer
    "no backend" there). A `nativeToken` whose backend is Android is an `ANativeWindow*` — an
-   address in the client's process — and may never cross (§3.4).
+   address in the client's process — and may never cross (§3.5). MetalLayer carries a
+   `CAMetalLayer*` and has the same restriction.
 
 ## 2. Ordering
 
@@ -58,16 +61,22 @@
    unchanged).
 3. Backend null at dispatch → `MOBILEGL_ERR_NOT_INITIALIZED` with `ok=false`; the forwarder's
    answer is the old `false`/void-drop, unchanged.
-4. Wire entry (`ServerApplyWireSurfaceOp`, the future P6 control pump's entry point; P5f drives
+4. Successful void operations (`SetSwapInterval`, `ReleaseSurface`, `ReleaseResources`,
+   `SetWindowHandle`) reply `ok=true` after completing on the apply thread; a null backend
+   replies `ok=false`. The wire therefore preserves the success/failure distinction even
+   though the inproc forwarder returns void.
+5. Wire entry (`ServerApplyWireSurfaceOp`, the future P6 control pump's entry point; P5f drives
    it from tests only): a decode failure is **`Fatal{ProtocolCorruption, "SurfaceOp"}`**, never a
    dropped frame (a dropped control frame is a client waiting on a reply that never comes); a
    decoded `WindowKind::AndroidNativeWindow` is
    **`Fatal{UnmigratedSurface, "AndroidNativeWindow@P12"}`** — the window kind is legal schema,
    the token is the part that means nothing in the server's process, and real window arrival is
-   P12. Inproc-only kinds (`InitCapabilitiesInprocOnly`, `SwapBuffersInprocOnly`,
+   P12. A decoded `MetalLayer` is similarly **`Fatal{UnmigratedSurface, "MetalLayer@P12"}`**,
+   because its `CAMetalLayer*` also belongs to the client process. Inproc-only kinds
+   (`InitCapabilitiesInprocOnly`, `SwapBuffersInprocOnly`,
    `InitWindowSurfaceInprocOnly`, `ProbeForTesting`) cannot arrive: the encoder refuses them
    (`SurfaceWireError::InprocOnlyOpOnTheWire`).
-5. A frame that reaches the inproc dispatch with `kind == None` or a kind the switch does not know
+6. A frame that reaches the inproc dispatch with `kind == None` or a kind the switch does not know
    is `Fatal{ProtocolCorruption, "SurfaceOp.kind"}`; a `windowBackend` tag outside the enum is
    `Fatal{ProtocolCorruption, "SurfaceOp.windowBackend"}`. Inproc these are corruption shapes, not
    bad input — same discipline as the ring's header check.

docs/Disaggregated/notes/p5f/fc-report.md

@@ -12,7 +12,7 @@
 |---|---|
 | `MobileGL/MG_Remote/Protocol/protocol.fbs` | **append-only**：`SurfaceOpKind` 补 `SetSwapInterval=8` / `ReleaseResources=9` / `SetWindowHandle=10`（与 P6-SPAWN-PLAN 的预测逐字一致）；`WindowKind` 补 `MetalLayer=6`（f0-egl schema 缺口 3：两枚举不互洽的另一半）；`SurfaceOp` 补 `readSurface`、`context` 两字段（MakeCurrent 的语义单元是四元组） |
 | `.../Protocol/generated/protocol_generated.h` | 用 pinned flatc（`~/w7/p5-s1-flatc-build/flatc`，25.12.19）重新生成；diff 仅 +58/−6 行，全部落在追加的枚举值与字段上 |
-| `.../Server/SurfaceControlFrame.{h,cpp}` | 纯值帧：`SurfaceControlOp`（10 个 wire op + 4 个 inproc-only kind）+ `SurfaceControlFrame`（`static_assert` 钉死 trivially-copyable / standard-layout——帧里**不可能**有指针） |
+| `.../Server/SurfaceControlFrame.{h,cpp}` | 纯值帧：`SurfaceControlOp`（10 个 wire op + 4 个 inproc-only kind）+ `SurfaceControlFrame`（constexpr 穷举结构化绑定 + 每成员 integer/enum 断言禁止指针；trivially-copyable / standard-layout 单独约束复制形态（§8 修正初版断言）） |
 | `.../Protocol/SurfaceOpCodec.{h,cpp}` | 帧 ↔ `Wire::SurfaceOp`/`SurfaceReply` 编解码；`WindowBackend` ↔ `WindowKind` 显式映射表（Surfaceless/Pbuffer 具名无后端）；wire 入口 `ServerApplyWireSurfaceOp` 的两个具名 Fatal（见 §3）；枚举对齐用 static_assert 钉死而非强转 |
 | `.../Server/ServerLoop.{h,cpp}` | **单槽邮箱的载荷从"函数指针 + 栈上 void\*"换成按值携带的一个帧**。`RunOnApplyThread(ControlWork, void*)` → `RunSurfaceControlFrame(SurfaceControlFrame&)`；十二个 forwarder 的 Args::Run 本体逐字搬进成员分发 `ApplySurfaceControlFrame`（C7/ID-54 分类、N-3 遗忘、R-12 重发布全部原位保留）；阻塞握手、影子位、publish-then-ring、C2 出口块全部不动。测试缝：`RunProbeOnApplyThreadForTesting`（hook 是成员变量，**不进槽位**） |
 | `MG_Test/Wire/SurfaceControlFrameTest.cpp` | 新套件 9 用例（见 §3） |
@@ -23,8 +23,8 @@
 
 **grep 证明**（验收项）：`grep -rn "ControlWork\|m_controlWork\|m_controlUser\|RunOnApplyThread"
 MobileGL/ --include="*.h" --include="*.cpp" --include="*.inc"` → **0 命中**；邮箱槽位类型是
-`SurfaceControlFrame m_controlFrame;`（按值，`ServerLoop.h:412`），其 trivially-copyable 由
-`SurfaceControlFrame.h` 的 static_assert 钉死。披露：测试缝的 `m_controlProbeHook` /
+`SurfaceControlFrame m_controlFrame;`（按值，`ServerLoop.h:412`），其无指针约束由
+`SurfaceControlFrame.h` 的穷举成员检查钉死；仅 trivially-copyable 并不能拒绝指针（§8）。披露：测试缝的 `m_controlProbeHook` /
 `m_controlProbeUser` 是 ServerLoop 的**成员**而非槽位内容（同 `m_beforeRetireHook` 的既有先例），
 只在 `ProbeForTesting` kind 的分发臂被读。
 
@@ -115,3 +115,47 @@ ID-67 的重发布计数、M-7/C2 的两个 !m_running 臂、影子位双清用
   append-only 后重生成。
 - `docs/Disaggregated/ROADMAP.md` G4 行、`P5F-WIRE-COMPLETENESS.md` 进度行、`P6-CONTRACT-DRAFT.md`
   §4：文档同点编辑。
+
+
+## 8 集成前局部复审修复（2026-09-19）
+
+本节追加独立修复，§5 的全门数字保留为初版历史；本次只重跑受影响测试，集成全门由集成者负责。
+这次局部复审不代替 P5f 不同模型族终审。
+
+| 缺陷 | 修复与回归用例 |
+|---|---|
+| wire `seq` 在真实 dispatch 入口被重新 mint，codec-only 往返漏检 | 仅 `seq==0` 的本地帧 mint；wire 序号保留。`WireSequenceSurvivesRealDispatchAndFailureReply` 驱动 wire → apply → reply 编码，要求无 backend 的失败回复仍回声 4242；成功臂亦断言原序号。 |
+| 4 个 void 成功臂未填 `ok`，编码成 `false` | `SetSwapInterval` / `ReleaseSurface` / `ReleaseResources` / `SetWindowHandle` 完成后写 `ok=true`。`SuccessfulVoidWireOperationsReplyWithSuccessAndOriginalSequence` 用真实 EGL backend 驱动四种 op 并检查实际编码回复。 |
+| MetalLayer 与 Android 一样携带客户端对象地址，却未拒绝 | `DecodeWireSurfaceOp` 对两种 window op 都返回 `MetalLayerArrived`，入口具名 Fatal `MetalLayer@P12`；新增纯解码检查与死亡用例。 |
+| trivially-copyable + standard-layout 不能排除指针；初版的机械证明错误 | constexpr 结构化绑定穷举 14 个字段，每项只准 integral/enum。添加任何成员而不更新绑定会编译失败；将既有字段改为指针会触发 static_assert。保留旧两个 trait，并加入 object/function pointer 的 constexpr 反例和新用例。 |
+| probe hook/user 在 `m_callerMutex` 外写，可被并发或重入 probe 覆盖 | 发布移入 caller 临界区，锁覆盖整个握手；apply 线程重入用调用栈上的 hook/user 参数，不写共享 pair。8 个并发投递者各 128 次，每次再嵌套一个 probe，逐投递者计数并校验总分发数。 |
+
+真实 dispatch 测试还暴露 `SurfaceOpCodec.h` 的 include 顺序问题：`Wire::` 会被既有
+`MG_Remote::Wire` 命名空间遮蔽；已将公开声明限定为 `::MobileGL::Wire::`。
+
+**G14**：原测试全部保留，新增 5 条（SurfaceControlFrameTest +2、ServerLoopTest +2、
+ServerLoopEglTest +1）。两目标 `cmake --build build-split --target SurfaceControlFrameTest ServerLoopTest -j8`
+通过；`MOBILEGL_ITEST_REQUIRE_GPU=1 ctest --test-dir build-split -R
+'^(SurfaceControlFrameTest|ServerLoopTest|ServerLoopEglTest)[.]' --output-on-failure -j8`
+**42/42 通过，零 skip**。probe 的另外两组调用者也已覆盖：重建 `RemoteClientTest`，
+`RemoteClientControls` **26/26** 与该可执行文件的其余 8 套件 **116/116** 全绿，
+合计本次相关 unit **184/184**。
+
+**真实 red-once，全部在 WSL 验证副本修改并还原，没有提交故障代码：**
+
+| 单独回退 | 红证据 |
+|---|---|
+| seq 恢复无条件 mint | `WireSequenceSurvivesRealDispatchAndFailureReply` 失败：回复 1，预期 4242；ctest rc=8。 |
+| 删除四处 `frame.ok=true` | 四种操作均报 `successful void operation encoded failure`；ctest rc=8。 |
+| 删除 MetalLayer 解码拒绝 | 解码错误码不符，死亡测试 `failed to die`；ctest rc=8。 |
+| probe 恢复锁外共享 pair 发布，并让重入读共享 pair | 并发+nested 用例逐投递者计数不符，报 `another poster or nested probe replaced this hook/user pair`；ctest rc=8。 |
+| 额外新增第 15 个指针成员但不更新检查 | 用真实 SurfaceControlFrame.cpp TU 的 compile_commands 编译参数执行 `-fsyntax-only`，编译失败：`binds to 15 elements, but only 14 names were provided`，rc=1。 |
+| 将既有 nativeToken 改为 void* | 同样的真实 TU 编译失败，正是 `every surface control frame member must be an integer or enum` 断言，rc=1。 |
+
+还原后两目标重建通过，相关测试再次 **42/42 绿**。日志在 WSL
+`/tmp/p5f-fc-fix-{red-seq,red-ok,red-metal,red-probe,red-added-member,red-pointer-type,restored-green}.log`。
+第一次编译负例的检查脚本预期了另一种编译器措辞，实际已按正确原因编译失败；修正日志匹配词后
+单独重跑两个编译负例并恢复验证，没有重跑无关门。
+
+WSL 旧门树 HEAD `7257a372` 与 Windows `d5ed139c` 为兄弟提交，仅报告不同，不能 ff-only。
+本次仅覆盖已确认无本地修改的 7 个相关源/测试文件验证，未 reset WSL 树，未改动其既有子模块状态。