diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index d54b8bd6..0b8d952d 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -15,6 +15,7 @@ #include "../Protocol/generated/protocol_generated.h" #include "../Server/ServerLoop.h" #include "../Server/ServerSession.h" +#include "../Protocol/SurfaceOpCodec.h" #include "../Transport/FdPassing.h" #include "../Transport/InProcessTransport.h" #include "WireTables.h" @@ -52,6 +53,10 @@ namespace MobileGL::MG_Remote::Client { // The same bounded handshake deadline the server uses. Bounded, not kWaitForever: a // bring-up that never answers has to be a red lane rather than a wedged CI job. constexpr Uint32 kHandshakeTimeoutMs = 5000; + // cp (CONTRACT-P6 D5b): bounded, and its expiry is NOT fatal. A peer that + // is gone is a different fact from a peer that is slow, and only the + // doorbell death latch may say which. + constexpr Uint32 kRemoteControlTimeoutMs = 5000; // Teardown's drain. Also bounded, and for the same reason - table 3's order is // "publish and wait for the server to drain and acknowledge", and a wait with no // deadline there turns a lost record into a hung process exit. @@ -769,12 +774,102 @@ namespace MobileGL::MG_Remote::Client { m_socketSelfBell = MakeUnique(fds[5], 1, true); } + // `cp`: from here the twelve EGL forwarders cross the socket instead of + // being posted into a mailbox this process does not own. Installed BEFORE + // FinishStartup, because FinishStartup's caps pump can already trigger + // one. + Server::ServerLoopInstance().SetRemoteControlSink(&RemoteControlSinkThunk, this); + // The apply thread IS ALREADY RUNNING, in the other process: ServerMain // started it before it sent the first descriptor. Starting one here would // put a second applier on a ring that already has one. return FinishStartup(m_socketPeerBell.get(), m_socketSelfBell.get(), false); } + MobileGLResult ClientSession::RemoteControlSinkThunk(void* user, + Server::SurfaceControlFrame& frame) { + return static_cast(user)->RunRemoteSurfaceControlFrame(frame); + } + + MobileGLResult ClientSession::RunRemoteSurfaceControlFrame(Server::SurfaceControlFrame& frame) { + if (m_transport == nullptr) { + return MOBILEGL_ERR_NOT_INITIALIZED; + } + // THE CLIENT MINTS, always, dense from 1 (CONTRACT-P6 §6.3). The server's + // local mint exists for unnumbered LOCAL posts; a wire request that let + // the server renumber it would get back a reply it cannot correlate. + if (frame.seq == 0) { + frame.seq = ++m_remoteControlSeq; + } + + ::flatbuffers::FlatBufferBuilder builder(512); + const MG_Remote::SurfaceWireError encoded = MG_Remote::EncodeSurfaceOpFrame(frame, &builder); + if (encoded != MG_Remote::SurfaceWireError::None) { + // A NAMED refusal, not a dropped op. InprocOnlyOpOnTheWire is the + // live one: three of the twelve forwarders ride the frame channel + // without a wire kind (CONTRACT-P6 §2), and a spawn client that + // reaches one has found a gap rather than a transport error. + MGLOG_E("MG_Remote client: surface op kind %d cannot cross the wire (%s)", + static_cast(frame.kind), MG_Remote::SurfaceWireErrorName(encoded)); + return MOBILEGL_ERR_UNSUPPORTED; + } + + // ONE CONTROL OP AT A TIME. The twelve forwarders are already + // caller-serialised on the server's m_callerMutex under inproc; the same + // discipline has to hold here or two replies race for one correlation. + const std::lock_guard lock(m_remoteControlMutex); + const MobileGLResult sent = m_transport->SendFrame( + MobileGLByteSpan{builder.GetBufferPointer(), builder.GetSize()}); + if (sent != MOBILEGL_OK) { + return sent; + } + + // The reply. A BOUNDED wait whose expiry is NOT fatal (CONTRACT-P6 §5.4 + // D5b): a peer that is gone is a different fact from a peer that is slow, + // and only the doorbell's death latch may say which. + for (;;) { + std::vector reply; + const MobileGLResult received = + ReceiveEnvelope(*m_transport, reply, kRemoteControlTimeoutMs); + if (received != MOBILEGL_OK) { + MGLOG_E("MG_Remote client: no SurfaceReply for seq %llu within %u ms (rc=%d)", + static_cast(frame.seq), kRemoteControlTimeoutMs, + static_cast(received)); + return received; + } + const ::MobileGL::Wire::CtrlEnvelope* envelope = ParseEnvelope(reply); + if (envelope == nullptr) { + return MOBILEGL_ERR_PROTOCOL_MISMATCH; + } + if (envelope->msg_type() == ::MobileGL::Wire::CtrlMsg::CapsSnapshot) { + // The server republishes caps from inside its MakeCurrent + // dispatch (a6 row A4-30), so a snapshot can arrive BEFORE the + // reply to the very op that produced it. Adopting it here rather + // than dropping it is what keeps R-12's "a second arrival IS the + // invalidation" true on this path too. + if (const auto* snapshot = envelope->msg_as_CapsSnapshot()) { + AdoptCapsSnapshot(snapshot); + } + continue; + } + if (envelope->msg_type() != ::MobileGL::Wire::CtrlMsg::SurfaceReply || + envelope->msg_as_SurfaceReply() == nullptr) { + MGLOG_E("MG_Remote client: expected a SurfaceReply, got message type %d", + static_cast(envelope->msg_type())); + return MOBILEGL_ERR_PROTOCOL_MISMATCH; + } + const ::MobileGL::Wire::SurfaceReply* wire = envelope->msg_as_SurfaceReply(); + if (wire->seq() != frame.seq) { + MGLOG_E("MG_Remote client: SurfaceReply names seq %llu, we asked for %llu", + static_cast(wire->seq()), + static_cast(frame.seq)); + return MOBILEGL_ERR_PROTOCOL_MISMATCH; + } + MG_Remote::DecodeWireSurfaceReply(*wire, &frame); + return MOBILEGL_OK; + } + } + // The half of startup that is IDENTICAL for inproc and spawn: the ring // producer, the reply pool, the event ring, the segment table, the encoder, // the first CapsSnapshot and the emitter tables. Extracted by `sm` so the diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index a3ad6e4b..f823fb96 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -38,6 +38,7 @@ #include #include +#include "../Server/SurfaceControlFrame.h" #include "../Transport/Doorbell.h" #include "../Transport/EventRing.h" #include "../Transport/ITransport.h" @@ -93,6 +94,12 @@ namespace MobileGL::MG_Remote::Client { // encoder - is byte for byte the inproc path, which is the property // that makes "P6 is a transport swap" true of THIS function even though // it is not true of the phase. + // `cp`: the remote control sink ServerLoop calls when the server is + // another process. Encode, send, wait for the SurfaceReply that carries + // OUR seq back. + static MobileGLResult RemoteControlSinkThunk(void* user, Server::SurfaceControlFrame& frame); + MobileGLResult RunRemoteSurfaceControlFrame(Server::SurfaceControlFrame& frame); + MobileGLResult FinishStartup(Transport::Doorbell* peerBell, Transport::Doorbell* selfBell, bool startApplyThreadHere); @@ -380,6 +387,10 @@ namespace MobileGL::MG_Remote::Client { // than in the transport because which bell is "mine" is the session's // knowledge, not the transport's - the same ruling as inproc's // PeerDoorbell/SelfDoorbell split. + // cp: one control op at a time, and the client's own seq space. + std::mutex m_remoteControlMutex; + Uint64 m_remoteControlSeq = 0; + std::unique_ptr m_socketTransport; std::unique_ptr m_socketSelfBell; std::unique_ptr m_socketPeerBell; diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index c318b504..615e788b 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -655,8 +655,22 @@ namespace MobileGL::MG_Remote::Server { return applied; } + void ServerLoop::SetRemoteControlSink(RemoteControlSink sink, void* user) { + m_remoteSink = sink; + m_remoteSinkUser = user; + } + MobileGLResult ServerLoop::RunSurfaceControlFrame(SurfaceControlFrame& frame) { if (frame.kind == SurfaceControlOp::None) return MOBILEGL_ERR_INVALID_ARGUMENT; + + // `cp`: the server is another process, so the frame CROSSES instead of + // being posted. Checked before the seq mint below, because a wire request + // must carry the CLIENT's number all the way through dispatch and the + // reply - the local mint at :662 is for unnumbered LOCAL requests and + // renumbering a wire one would make its reply uncorrelatable. + if (m_remoteSink != nullptr) { + return m_remoteSink(m_remoteSinkUser, frame); + } // 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; diff --git a/MobileGL/MG_Remote/Server/ServerLoop.h b/MobileGL/MG_Remote/Server/ServerLoop.h index edfa097b..c53399b7 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.h +++ b/MobileGL/MG_Remote/Server/ServerLoop.h @@ -223,6 +223,21 @@ namespace MobileGL::MG_Remote::Server { // What did NOT move: the blocking handshake, the one slot, and the apply-thread // ownership. Under spawn the same frame is what SurfaceOpCodec encodes into a // Wire::SurfaceOp - this signature is the seam both transports share. + // P6 `cp`: where a control frame goes when the server is ANOTHER PROCESS. + // + // All twelve Server* forwarders funnel through RunSurfaceControlFrame, so + // this is ONE seam rather than twelve (a6 row A4-13 counted them). The + // client installs a sink at handshake; with none installed the behaviour + // is exactly what it was - post into the one-slot mailbox, or run inline + // when we are already on the apply thread. + // + // A HOOK RATHER THAN A CALL INTO ClientSession, deliberately: the server + // half calling the client half is four of the six symbols a6 found + // MG_Remote owes itself (a6-link-experiment §5), and this is the one + // place that would have added a fifth. + using RemoteControlSink = MobileGLResult (*)(void* user, SurfaceControlFrame& frame); + void SetRemoteControlSink(RemoteControlSink sink, void* user); + MobileGLResult RunSurfaceControlFrame(SurfaceControlFrame& frame); // The TEST seam through the same channel: posts a ProbeForTesting frame whose dispatch @@ -364,6 +379,9 @@ namespace MobileGL::MG_Remote::Server { Bool ControlIsPending() const; private: + RemoteControlSink m_remoteSink = nullptr; + void* m_remoteSinkUser = nullptr; + void ApplyThreadMain(); // Runs a posted control frame, if there is one. Returns true if it ran one. Bool PumpControlRequest(); diff --git a/MobileGL/MG_Remote/Server/ServerMain.cpp b/MobileGL/MG_Remote/Server/ServerMain.cpp index 9300e90e..c79350f5 100644 --- a/MobileGL/MG_Remote/Server/ServerMain.cpp +++ b/MobileGL/MG_Remote/Server/ServerMain.cpp @@ -38,8 +38,10 @@ #include "../Transport/FdPassing.h" #include "../Transport/SocketTransport.h" #include "../Transport/WireLog.h" +#include "../Protocol/SurfaceOpCodec.h" #include "ServerLoop.h" #include "ServerSession.h" +#include "SurfaceControlFrame.h" #include @@ -287,13 +289,36 @@ extern "C" __attribute__((visibility("default"))) int mobilegl_server_main(int a static_cast(result)); break; } - // `cp` owns what a control frame MEANS. Until then the pump exists so the - // stream is drained and EOF is noticed; the echo carries this process's - // own pid so a test that talked to itself cannot pass. - std::string reply(reinterpret_cast(buffer.data()), - static_cast(size)); - reply += "|server-pid=" + std::to_string(selfPid); - if (control->SendFrame(MobileGLByteSpan{reply.data(), reply.size()}) != MOBILEGL_OK) { + // `cp`: decode, apply, reply. THIS THREAD IS NOT THE APPLY THREAD, which + // is what CONTRACT-P6 §6.2 requires - a control arrival must not wake the + // applier, and frame decode must not happen on the thread holding the + // native context. ServerApplyWireSurfaceOp posts through ServerLoop's + // existing one-slot mailbox, unchanged. + const ::MobileGL::Wire::CtrlEnvelope* envelope = nullptr; + { + ::flatbuffers::Verifier verifier(buffer.data(), static_cast(size)); + if (::MobileGL::Wire::VerifyCtrlEnvelopeBuffer(verifier)) { + envelope = ::MobileGL::Wire::GetCtrlEnvelope(buffer.data()); + } + } + if (envelope == nullptr || envelope->msg_type() != ::MobileGL::Wire::CtrlMsg::SurfaceOp || + envelope->msg_as_SurfaceOp() == nullptr) { + WireLogError("MG_Remote server: pid=%d a control frame that is not a verifiable " + "SurfaceOp arrived; ignoring it would be the silent drop this project " + "refuses, so the session ends here", + selfPid); + break; + } + + Server::SurfaceControlFrame reply{}; + const MobileGLResult applied = + ServerApplyWireSurfaceOp(*envelope->msg_as_SurfaceOp(), &reply); + (void)applied; // carried in the reply's ok/result, not thrown away here + + ::flatbuffers::FlatBufferBuilder builder(256); + EncodeSurfaceReplyFrame(reply, &builder); + if (control->SendFrame( + MobileGLByteSpan{builder.GetBufferPointer(), builder.GetSize()}) != MOBILEGL_OK) { break; } } diff --git a/MobileGL/MG_Test/Wire/ServerSpawnTest.cpp b/MobileGL/MG_Test/Wire/ServerSpawnTest.cpp index 6575b362..625dca6f 100644 --- a/MobileGL/MG_Test/Wire/ServerSpawnTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerSpawnTest.cpp @@ -34,6 +34,7 @@ // the boundary before the data plane needs it. #include +#include #include #include #include @@ -151,6 +152,45 @@ TEST(ServerSpawnTest, StartsAServerProcessAndHandshakesAcrossIt) { EXPECT_EQ(Server::CountOwnChildren(), before) << "a zombie would still be counted here"; } +TEST(ServerSpawnTest, AnEglControlOpCrossesToTheOtherProcessAndAnswers) { + // `cp`. All twelve Server* EGL forwarders funnel through + // ServerLoop::RunSurfaceControlFrame, so this exercises the ONE seam that + // sends them: encode -> control socket -> the server's pump -> + // ServerApplyWireSurfaceOp -> its one-slot mailbox -> the apply thread -> + // SurfaceReply -> back here. + // + // Under inproc this same call posts into a mailbox in THIS process. The + // assertion that it is not doing that here is the server's pid in the + // handshake plus the fact that ServerLoopInstance() in this process was + // never started - a local post would find no apply thread and time out. + Session session; + ASSERT_TRUE(Bring("eglop", &session)); + ASSERT_EQ(Handshake(session), MOBILEGL_OK); + + // eglInitialize's forwarder: the simplest op that has a real answer. + EGLint major = -1; + EGLint minor = -1; + const bool ok = Server::ServerInitializeEGLDisplay(EGL_NO_DISPLAY, &major, &minor); + + // WHAT IS ASSERTED IS THE ROUND TRIP, not the EGL result. A headless CI + // machine has no display to initialise, so `ok` is expected to be false - + // and that is fine, because `cp` is done when the op reaches the other + // process and an answer comes back. + // + // THE OUT-PARAMETERS ARE THE PROOF. ServerInitializeEGLDisplay writes them + // "whenever the dispatch ran, success or not" (ServerLoop.cpp's own comment + // at the forwarder), so they move off -1 if and only if a SurfaceReply came + // back from the other process. A transport failure returns before the + // write-back and leaves both at -1; that is the difference this asserts. + EXPECT_NE(major, -1) << "no SurfaceReply came back: the op never reached the other process"; + EXPECT_NE(minor, -1) << "no SurfaceReply came back: the op never reached the other process"; + (void)ok; + + Client::ClientSessionInstance().Stop(); + int exitCode = -1; + ASSERT_EQ(Server::ReapServer(session.server, 5000, &exitCode), MOBILEGL_OK); +} + TEST(ServerSpawnTest, AnUnresolvableImageIsANamedRefusalAndStartsNothing) { const int before = Server::CountOwnChildren();