diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index aa55c371..dc8d5bc4 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -2101,10 +2101,13 @@ if (MOBILEGL_BUILD_DISAGGREGATED) RESOURCE_LOCK mobilegl-tcp) # Ph fuzz arm 3 (PH-6, ID-P7-2): A PEER THAT STOPS DRAINING SEG_EVENT, on both data planes. # MG_Test/Wire/EventForfeitPeerTest.cpp starts its own `--serve` supervisor (an abstract unix - # endpoint for the shm plane, a free loopback port for the stream plane) with - # MOBILEGL_IPC_EVENT_WAIT_MS=2000, and asserts the whole arm: the server forfeits the reverse - # channel by name, counts the drop, ends the session with exit 0 inside the knob rather than - # the old 30 s Fatal, and the SAME supervisor welcomes the next connection. The names are not + # endpoint for the shm plane, a free loopback port for the stream plane) with a NON-default + # MOBILEGL_IPC_EVENT_WAIT_MS, and asserts the whole arm: the server forfeits the reverse + # channel by name against that knob, counts the drop, ends the session with exit 0 inside the + # knob rather than the old 30 s Fatal, and the SAME supervisor welcomes the next connection. + # Each entry also runs the kill-the-peer case (knob 8000, over ServerLoop's 5000 ms join: the + # server must say PeerGone at once and exit 0), and the shm entry the LaunchServer case (the + # knob must cross ServerSpawn.cpp's scrub). The gtest filters are per plane. The names are not # `DirectGLES..` on purpose: spawn_lane_parity.py compares the scenario arms and # this is a supervisor control, one per lane, like the entry above. It needs no fixture and no # resource lock (its supervisor is its own), and the CI lane names both entries to @@ -2115,12 +2118,12 @@ if (MOBILEGL_BUILD_DISAGGREGATED) set(MGL_ITEST_EVENT_FORFEIT_ENVIRONMENT "MOBILEGL_TEST_SERVER_PATH=${MGL_ITEST_SPLIT_SERVER_PATH}" ${MGL_ITEST_COMMON_ENV}) add_test(NAME SpawnLane.EventForfeitPeer COMMAND $ - --gtest_filter=EventForfeitPeer.ShmPeerThatStopsDrainingIsForfeitedAndTheNextConnectionIsWelcomed) + --gtest_filter=EventForfeitPeer.Shm*) set_tests_properties(SpawnLane.EventForfeitPeer PROPERTIES LABELS integration-spawn TIMEOUT 120 ENVIRONMENT "${MGL_ITEST_EVENT_FORFEIT_ENVIRONMENT}") add_test(NAME TcpLane.EventForfeitPeer COMMAND $ - --gtest_filter=EventForfeitPeer.StreamPeerThatStopsDrainingIsForfeitedAndTheNextConnectionIsWelcomed) + --gtest_filter=EventForfeitPeer.Stream*) set_tests_properties(TcpLane.EventForfeitPeer PROPERTIES LABELS integration-tcp TIMEOUT 120 ENVIRONMENT "${MGL_ITEST_EVENT_FORFEIT_ENVIRONMENT}") diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index df0d3fcf..48fd5a87 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -92,6 +92,13 @@ namespace MobileGL::MG_Remote::Client { // then watch this object, not a side effect three layers up. Transport::Doorbell* SelfDoorbellForTest() { return m_producer.SelfDoorbell(); } + // PH-6's fuzz arm 3 ONLY (EventForfeitPeerTest): the control stream, so a peer that has + // stopped draining can still put on it the frames no ClientSession path would send at that + // moment - a malformed one, or a surface op queued behind a wait it did not drain for. + // Every production path that writes control holds + // m_remoteControlMutex and drains first; a caller of this does neither, which is the point. + Transport::ITransport* ControlTransportForTest() { return m_transport; } + // CONTRACT-P6 4.3: THE PID THE SERVER STATED IN ITS Welcome, which is not the same fact // as the pid this session's launcher recorded - that one is local knowledge, this one // CROSSED THE WIRE. Under spawn they must agree and neither may be ours; under inproc diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index b4daeb46..be672f9a 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -210,6 +210,9 @@ namespace MobileGL::MG_Remote::Server { } m_session = &session; m_stopRequested.store(false, std::memory_order_release); + // PH-6 fix round: the session's copy of the request, for ReserveEventOrBlock's wait. Lowered + // with the loop's own, so a loop restarted on the same session does not forfeit at once. + session.ClearApplyStopRequest(); // 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 @@ -816,6 +819,11 @@ namespace MobileGL::MG_Remote::Server { } m_stopRequested.store(true, std::memory_order_release); if (m_session != nullptr) { + // PH-6 fix round: the apply thread may be parked in ReserveEventOrBlock rather than in + // the loop's own park, and that wait's predicate is the session's, not this loop's. + // Raised BEFORE the ring below for the same publish-then-ring reason as the flag above; + // without it a knob longer than kJoinTimeoutMs outlives this join. + m_session->RequestApplyStop(); // The bell may already be dead - ClientSession::Stop calls transport->Shutdown() // first, which is what table 3's step 2 requires - and Notify on a dead bell is // harmless. Ringing anyway covers the paths that Stop without a Kill. diff --git a/MobileGL/MG_Remote/Server/ServerMain.cpp b/MobileGL/MG_Remote/Server/ServerMain.cpp index d0c83d21..e2df8d5b 100644 --- a/MobileGL/MG_Remote/Server/ServerMain.cpp +++ b/MobileGL/MG_Remote/Server/ServerMain.cpp @@ -340,6 +340,17 @@ void SendLogAck(void* pointer) { // clientBell[0] is the CLIENT's park end and no bell here owns it; the // other three belong to selfBell and peerBell and are closed with them. ::close(clientBell[0]); + // PH-6 fix round: THE SERVER'S BELL GETS A DEATH WITNESS TOO (CONTRACT-P6 D5c, mirrored). + // selfBell parks on serverBell[0] and this process rings itself through serverBell[1], so + // it holds both ends of that pair and the pair can never hang up, however dead the client + // is: Dead() stayed false for the life of a shm session, and ReserveEventOrBlock's "dead + // first" check could only ever report a peer killed mid-wait as NotDraining after the + // whole MOBILEGL_IPC_EVENT_WAIT_MS. The control socket's far end is the client's alone, so + // its hangup IS the death; the bell polls it for hangup only and never reads from it, so + // SocketTransport's reassembler below keeps every byte. Set before loop.Start(): the apply + // thread is the one that parks on this bell. (The stream plane's bell is the data socket + // itself and dies with the peer already.) + selfBell->SetDeathWitness(control->StreamFd()); } const char* forwarding = std::getenv("MOBILEGL_IPC_LOG_FORWARD"); @@ -371,7 +382,13 @@ void SendLogAck(void* pointer) { break; } if (result == MOBILEGL_ERR_BUFFER_TOO_SMALL) { buffer.resize(static_cast(size)); continue; } - if (result != MOBILEGL_OK) break; + if (result != MOBILEGL_OK) { + // EOF or a transport error: the peer is gone. Said to the session BEFORE loop.Stop() + // below, so an apply thread that the stop wakes inside ReserveEventOrBlock names the + // hangup (PeerGone) rather than the stop (PH-6 fix round). + session.NoteControlStreamEnded(); + break; + } // THE FILE IDENTIFIER, ASKED FIRST AND SAID OUT LOUD (P7 wave 0). // // The plan asked for CtrlEnvelopeBufferHasIdentifier here because ServerSession:: @@ -435,7 +452,10 @@ void SendLogAck(void* pointer) { if (session.DataLink()) reply.eventHead = session.DataLink()->EventPublishedHead(); flatbuffers::FlatBufferBuilder builder(256); EncodeSurfaceReplyFrame(reply, &builder); - if (control->SendFrame({builder.GetBufferPointer(), builder.GetSize()}) != MOBILEGL_OK) break; + if (control->SendFrame({builder.GetBufferPointer(), builder.GetSize()}) != MOBILEGL_OK) { + session.NoteControlStreamEnded(); + break; + } } loop.Stop(); // Publish the final server window while log forwarding is still attached. diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index 1c1e841b..77c343f1 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/MobileGL/MG_Remote/Server/ServerSession.cpp @@ -295,6 +295,13 @@ namespace MobileGL::MG_Remote::Server { const auto drained = [signals] { return signals.EventRingFull->load(std::memory_order_acquire) == 0; }; + // THE WAIT ALSO ENDS ON A STOP (PH-6 fix round). ServerLoop::Stop() raises the session's + // request and rings this same bell; a predicate that asked only "drained?" swallowed + // that ring and parked again for the rest of the knob, so any knob over Stop()'s 5000 ms + // join turned an ordinary end of the control stream into Fatal{ApplyThreadJoinTimeout}. + const auto drainedOrStopping = [&drained, &session] { + return drained() || session.ApplyStopRequested(); + }; // ONE DEADLINE FOR THE WHOLE RESERVATION, NOT ONE PER PARK. The loop goes round as // often as the client clears the latch - that is how a client that drains in pieces // eventually makes room - but every round spends the same budget, so a peer that @@ -320,6 +327,22 @@ namespace MobileGL::MG_Remote::Server { eventName, payloadBytes, MillisecondsSince(start), budgetMs, shortDrains); }; + // Asked AFTER Dead() everywhere below: under spawn a stop usually FOLLOWS the peer's + // hangup (control EOF), and the hangup is the more specific answer. The same fact can + // reach the stop first from the other side - ServerMain notes an ended control stream + // before it calls Stop() - and then it is still a hangup, not a stop. + const auto stopped = [&]() { + if (session.ControlStreamEnded()) { + return session.ForfeitReverseChannel( + "PeerGone", "the client's control stream ended while the server waited - the peer is gone", + eventName, payloadBytes, MillisecondsSince(start), budgetMs, shortDrains); + } + return session.ForfeitReverseChannel("Stopped", + "the session was stopped while the server waited for the " + "client to drain SEG_EVENT", + eventName, payloadBytes, MillisecondsSince(start), budgetMs, + shortDrains); + }; for (;;) { // PUBLISH AND RING FIRST. The client cannot drain a ring whose head it has not // been shown, and the latch Reserve just set is only useful to a client that @@ -327,17 +350,21 @@ namespace MobileGL::MG_Remote::Server { session.PublishEvents(); // DEAD FIRST (the plan's own clause): a bell whose peer is gone answers every // Wait with an immediate false, and reporting that as "waited N ms" would name - // a timeout for what is a hangup. + // a timeout for what is a hangup. Under spawn on the shm plane this needs the + // bell's death witness (ServerMain.cpp, the control socket): the server holds + // both ends of its own bell pair, so without one it can never go Dead(). if (bell.Dead()) { return session.ForfeitReverseChannel("PeerGone", "the client's doorbell is dead - the peer is gone", eventName, payloadBytes, MillisecondsSince(start), budgetMs, shortDrains); } if (!drained()) { + if (session.ApplyStopRequested()) return stopped(); const auto left = std::chrono::duration_cast( deadline - std::chrono::steady_clock::now()).count(); if (left <= 0) return outOfPatience(); - if (!bell.Wait(*signals.ConsumerParked, drained, /*spinUs=*/0, static_cast(left))) { + if (!bell.Wait(*signals.ConsumerParked, drainedOrStopping, /*spinUs=*/0, + static_cast(left))) { if (bell.Dead()) { return session.ForfeitReverseChannel( "PeerGone", "the client's doorbell died while the server waited - the peer is gone", @@ -345,6 +372,16 @@ namespace MobileGL::MG_Remote::Server { } return outOfPatience(); } + // Woken by Stop() and not by a drain. A drain that raced the stop still gets + // its Reserve below; the next round then sees the request and stops. + if (!drained()) { + if (bell.Dead()) { + return session.ForfeitReverseChannel( + "PeerGone", "the client's doorbell died while the server waited - the peer is gone", + eventName, payloadBytes, MillisecondsSince(start), budgetMs, shortDrains); + } + return stopped(); + } } slot = session.Events().Reserve(kind, payloadBytes); if (slot != nullptr) return slot; @@ -353,6 +390,10 @@ namespace MobileGL::MG_Remote::Server { // gets the rest of the same budget - Reserve has just raised the latch again, so // the next round parks until the client clears it once more. ++shortDrains; + // A peer that clears the latch itself (it is on a page the peer can write) keeps + // drained() true and this loop from parking; the budget still bounds it, and so + // does a stop. + if (session.ApplyStopRequested()) return stopped(); if (std::chrono::steady_clock::now() >= deadline) return outOfPatience(); } } diff --git a/MobileGL/MG_Remote/Server/ServerSession.h b/MobileGL/MG_Remote/Server/ServerSession.h index 3985d1f2..b56b642d 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.h +++ b/MobileGL/MG_Remote/Server/ServerSession.h @@ -244,6 +244,29 @@ namespace MobileGL::MG_Remote::Server { Uint32 shortDrains); void* DropEventAfterForfeit(Uint64 payloadBytes); + // PH-6 fix round. SERVERLOOP::STOP'S REQUEST, SEEN BY THE ONE APPLY-THREAD WAIT THAT IS NOT + // THE LOOP'S OWN PARK. ReserveEventOrBlock parks on the same consumer bell the loop parks + // on, but with its own predicate, so a Stop() that rang that bell used to wake it only to + // have it re-test "has the client drained?", find no, and park again for the rest of + // MOBILEGL_IPC_EVENT_WAIT_MS - while Stop()'s bounded join (5000 ms) ran out and the + // session died of Fatal{ApplyThreadJoinTimeout}. A control stream that ends (EOF, a + // malformed frame, a LogFlush ack) while the apply thread waits for a drain is exactly + // that shape. Stop() raises this BEFORE it rings; the wait's predicate reads it and the + // reservation forfeits as `Stopped`. Start() lowers it for the next run of the loop. + void RequestApplyStop() { m_applyStopRequested.store(true, std::memory_order_release); } + void ClearApplyStopRequest() { + m_controlStreamEnded.store(false, std::memory_order_release); + m_applyStopRequested.store(false, std::memory_order_release); + } + Bool ApplyStopRequested() const { return m_applyStopRequested.load(std::memory_order_acquire); } + // WHY the stop came, when the answer is "the peer is gone": ServerMain's control loop + // notes an EOF or a transport error on the control stream BEFORE it stops the loop, so a + // reservation that the stop wakes names the hangup (PeerGone) and not the stop. Needed + // because the stop and the data bell's own death race - on the stream plane the control + // EOF routinely wins - and a peer killed mid-wait is not a session somebody stopped. + void NoteControlStreamEnded() { m_controlStreamEnded.store(true, std::memory_order_release); } + Bool ControlStreamEnded() const { return m_controlStreamEnded.load(std::memory_order_acquire); } + // completedFrameSerial / presentAckSerial: the two watermarks only the server can // advance, kept together with the other three rather than poked into RingControl from // whatever code happens to notice a present finished. @@ -283,6 +306,10 @@ namespace MobileGL::MG_Remote::Server { std::atomic m_reverseChannelForfeit{false}; std::atomic m_forfeitDrops{0}; std::vector m_forfeitScratch; + // PH-6 fix round. Written by ServerLoop::Start/Stop and ServerMain's control loop, read by + // the apply thread's event wait. + std::atomic m_applyStopRequested{false}; + std::atomic m_controlStreamEnded{false}; }; // Leak-at-exit like every other MG_Remote singleton (ID-8): no frontend destructor may diff --git a/MobileGL/MG_Test/Wire/EventForfeitPeerTest.cpp b/MobileGL/MG_Test/Wire/EventForfeitPeerTest.cpp index 2a1e7d14..3b6ed74a 100644 --- a/MobileGL/MG_Test/Wire/EventForfeitPeerTest.cpp +++ b/MobileGL/MG_Test/Wire/EventForfeitPeerTest.cpp @@ -23,19 +23,38 @@ // Before PH-6 the server parked on that fifth one for 30 000 ms and then aborted with // Fatal{EventRingOverflow}; now it forfeits the reverse channel after MOBILEGL_IPC_EVENT_WAIT_MS. // -// BOTH DATA PLANES. The Shm case is the spawn lane's plane (a unix control socket, SEG_EVENT in -// shared memory, the peer's drain is a cursor it never moves); the Stream case is the tcp lane's +// BOTH DATA PLANES. The Shm cases are the spawn lane's plane (a unix control socket, SEG_EVENT in +// shared memory, the peer's drain is a cursor it never moves); the Stream cases are the tcp lane's // (the event bytes cross the data connection, the peer's drain is a progress frame it never -// sends). Each runs in its own ctest entry under its own lane label. +// sends). Each plane runs in its own ctest entry under its own lane label. +// +// THE SCENARIOS (the last four are the PH-6 fix round's): +// * the peer stops draining and stays - NotDraining after the knob; +// * the peer is KILLED while the server waits, with a knob longer than ServerLoop::Stop()'s +// 5000 ms join - PeerGone well inside the knob, exit 0. The control EOF reaches ServerMain, +// which says so to the session and stops the loop; the reservation's wait ends on that stop; +// * the same kill with a SURFACE OP QUEUED behind the wait - ServerMain is then blocked in the +// apply thread's mailbox and cannot see the EOF, so only the data bell can report the death: +// on the shm plane that is the server bell's death witness on the control socket; +// * the peer stays but its control stream ENDS (a malformed frame) while the server waits - +// `Stopped`, at once, exit 0, instead of Stop()'s join running out under the long knob; +// * the shm plane's server started by Server::LaunchServer, the launcher that SCRUBS the child's +// MOBILEGL_IPC_* - the knob must survive the scrub (ServerSpawn.cpp's allow-list). +// Every knob here is NOT the default (2000), and every forfeit line is read for the value it +// names, so a knob the server silently ignores is red. // // Everything that uses a ClientSession runs in a FORKED CHILD. The session singleton latches // device-lost for the rest of its process once its server goes away, so the "next connection" // must come from a process that never had a first one. #include +#include #include +#include +#include #include #include +#include #include #include #include @@ -46,8 +65,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -68,10 +89,24 @@ namespace MGP = MobileGL::MG_Pipe; namespace { - // The server's patience in this scenario. Short enough that a lane pays little for it, long - // enough that "the session ended after the knob" and "the session ended at once" are - // different readings, and 15x under the 30 s constant it replaced. - constexpr std::uint32_t kEventWaitMs = 2000; + // The server's patience in the stop-draining scenario. NOT the default (2000): the forfeit line + // names the budget it used, and reading this value back is what proves the server parsed the + // variable rather than fell back. Short enough that a lane pays little for it, long enough that + // "the session ended after the knob" and "the session ended at once" are different readings. + constexpr std::uint32_t kEventWaitMs = 1500; + // The kill scenario's knob: longer than ServerLoop's 5000 ms bounded join (kJoinTimeoutMs), so + // a wait that only a drain or the knob can end turns the peer's death into + // Fatal{ApplyThreadJoinTimeout} - and a PeerGone that arrives "well inside" it cannot be the + // knob running out. + constexpr std::uint32_t kLongEventWaitMs = 8000; + // The launched-server scenario's knob, set in the PEER's environment: it reaches the server + // only through Server::LaunchServer's scrubbed environment. + constexpr std::uint32_t kLaunchedEventWaitMs = 1200; + // How long after its readbacks a peer waits before its raw control frame (so the server is + // parked in the reservation by then), and how long the kill scenarios' peer lives: far under + // every knob here. + constexpr std::uint32_t kRawFrameAfterMs = 200; + constexpr std::uint32_t kKillAfterMs = 400; // Five readbacks of one writeback slice each - a quarter of the ring the server announced, // less a little for the event's own head and record header, which is the producer-side // slicing rule (MGPipeBufferWritebackSliceBytes) - so four fill SEG_EVENT to within a few @@ -85,6 +120,10 @@ namespace { return "libMobileGLServer.so"; } + std::string WaitKnobEntry(std::uint32_t waitMs) { + return "MOBILEGL_IPC_EVENT_WAIT_MS=" + std::to_string(waitMs); + } + std::uint16_t FreeLoopbackPort() { const int fd = ::socket(AF_INET, SOCK_STREAM, 0); if (fd < 0) return 0; @@ -107,7 +146,7 @@ namespace { // leak into the server - so every MOBILEGL_IPC_* goes and the few this scenario means are // stated. MOBILEGL_IPC_LOG_FORWARD=0 keeps the server's lines in its own role log, which is // what the assertions below read. - std::vector SupervisorEnvironment(const std::string& logBase) { + std::vector SupervisorEnvironment(const std::string& logBase, std::uint32_t waitMs) { std::vector env; for (char** e = ::environ; e != nullptr && *e != nullptr; ++e) { const std::string entry(*e); @@ -120,7 +159,7 @@ namespace { env.emplace_back("MOBILEGL_IPC_ROLE=server"); env.emplace_back("MOBILEGL_IPC_DIAL=no"); env.emplace_back("MOBILEGL_IPC_LOG_FORWARD=0"); - env.emplace_back("MOBILEGL_IPC_EVENT_WAIT_MS=" + std::to_string(kEventWaitMs)); + env.emplace_back(WaitKnobEntry(waitMs)); env.emplace_back("MOBILEGL_LOG_FILE_PATH=" + logBase); const std::string image = ServerImage(); const auto slash = image.find_last_of('/'); @@ -128,9 +167,9 @@ namespace { return env; } - pid_t LaunchSupervisor(const std::string& endpoint, const std::string& logBase) { + pid_t LaunchSupervisor(const std::string& endpoint, const std::string& logBase, std::uint32_t waitMs) { const std::string image = ServerImage(); - std::vector env = SupervisorEnvironment(logBase); + std::vector env = SupervisorEnvironment(logBase, waitMs); std::vector envp; for (auto& entry : env) envp.push_back(entry.data()); envp.push_back(nullptr); @@ -169,6 +208,16 @@ namespace { return n; } + // The one log line that holds `needle`, for asserting what ELSE that line says. + std::string LineWith(const std::string& text, const std::string& needle) { + const auto at = text.find(needle); + if (at == std::string::npos) return std::string(); + const auto begin = text.rfind('\n', at); + const auto end = text.find('\n', at); + return text.substr(begin == std::string::npos ? 0 : begin + 1, + (end == std::string::npos ? text.size() : end) - (begin == std::string::npos ? 0 : begin + 1)); + } + // Polls the supervisor's log until `needle` has appeared `times` times, or the budget runs // out. Returns the milliseconds it took, or -1. long long WaitForLog(const std::string& logBase, const std::string& needle, std::size_t times, @@ -218,16 +267,15 @@ namespace { kBufferRespecify = 15, kBufferContent = 16, kReadbackPublish = 17, + kLaunch = 18, + kServerNotReaped = 19, + kServerExitedNonZero = 20, + kRawFrame = 21, }; - // THE PEER THAT STOPS DRAINING. Writes one byte to `published` when the fourth readback is on - // the wire, then does nothing at all - no wait, no drain, no teardown - until the parent says - // the supervisor has reaped its session, and leaves by _exit: a clean Stop() would drain. - [[noreturn]] void NonDrainingPeer(const std::string& control, int published, int release) { - ConfigureClient(control); - auto& client = Client::ClientSessionInstance(); - if (client.StartSpawned() != MOBILEGL_OK) ::_exit(kNoWelcome); - + // Everything a peer does between its handshake and the moment SEG_EVENT is full with the + // server waiting on one more writeback. Returns kChildOk or the step that failed. + int FillTheEventRingWithoutDraining(Client::ClientSession& client) { // A pbuffer context on the server's apply thread, through the same forwarders // BackendObject_Remote uses. The handles are the client's names for them; the server keys // its surfaces by them and never dereferences one. @@ -235,9 +283,9 @@ namespace { const auto surface = reinterpret_cast(std::uintptr_t{0x5346}); const auto context = reinterpret_cast(std::uintptr_t{0x4358}); EGLint major = 0, minor = 0; - if (!Server::ServerInitializeEGLDisplay(display, &major, &minor)) ::_exit(kEglDisplay); - if (!Server::ServerCreateEGLPbufferSurface(surface, 16, 16)) ::_exit(kEglSurface); - if (!Server::ServerMakeEGLCurrent(display, surface, surface, context)) ::_exit(kEglCurrent); + if (!Server::ServerInitializeEGLDisplay(display, &major, &minor)) return kEglDisplay; + if (!Server::ServerCreateEGLPbufferSurface(surface, 16, 16)) return kEglSurface; + if (!Server::ServerMakeEGLCurrent(display, surface, surface, context)) return kEglCurrent; // One buffer with defined content, staged whole: create, respecify, one sub-data record. // These three WAIT, and that is fine - the ring holds one small surface event at most so @@ -252,7 +300,7 @@ namespace { if (client.EmitAndWait(MGP::MGPWireOp::ResourceCreate, &desc, sizeof(desc), nullptr, 0, nullptr, 0, &status) == Wire::kInvalidSeq || status != Wire::ReplySink::kStatusOk) { - ::_exit(kBufferCreate); + return kBufferCreate; } desc.Width = static_cast(kSliceBytes); desc.Usage = 0; // BufferUsage::StreamDraw, the enum's first value @@ -261,31 +309,83 @@ namespace { if (client.EmitAndWait(MGP::MGPWireOp::ResourceRespecify, &desc, sizeof(desc), nullptr, 0, nullptr, 0, &status) == Wire::kInvalidSeq || status != Wire::ReplySink::kStatusOk) { - ::_exit(kBufferRespecify); + return kBufferRespecify; } std::vector bytes(kSliceBytes, 0x5A); MGP::MGPSubData content{}; content.Res = kBuffer; content.Target = MGP::kMGPipeResourceTargetBuffer; - if (!MGP::MGPipeSetSubDataBufferRange(content, 0, kSliceBytes)) ::_exit(kBufferContent); + if (!MGP::MGPipeSetSubDataBufferRange(content, 0, kSliceBytes)) return kBufferContent; content.Blob = client.Encoder().StageBytes(bytes.data(), bytes.size()); status = Wire::ReplySink::kStatusError; if (client.EmitAndWait(MGP::MGPWireOp::ResourceSubData, &content, sizeof(content), nullptr, 0, nullptr, 0, &status) == Wire::kInvalidSeq || status != Wire::ReplySink::kStatusOk) { - ::_exit(kBufferContent); + return kBufferContent; } - // THE PART UNDER TEST. Four whole-buffer readbacks, published and never waited for. + // THE PART UNDER TEST. kReadbacks (five) whole-buffer readbacks, published and never + // waited for: the first four writebacks fill SEG_EVENT, the fifth is the one the server + // waits on. for (std::uint32_t i = 0; i < kReadbacks; ++i) { MGP::MGPReadback readback{}; readback.Res = kBuffer; readback.Offset = 0; readback.Size = kSliceBytes; - if (!PublishWithoutWaiting(client, MGP::MGPWireOp::ResourceReadback, readback)) ::_exit(kReadbackPublish); + if (!PublishWithoutWaiting(client, MGP::MGPWireOp::ResourceReadback, readback)) return kReadbackPublish; } + return kChildOk; + } + + enum class Scenario { + StopsDraining, // the peer stays, silent: NotDraining after the knob + KilledWhileWaiting, // SIGKILLed while the server waits: PeerGone, at once + KilledWithAControlOpQueued, // the same, with a surface op stuck behind the wait + ControlEndsWhileWaiting, // the peer stays; its control stream ends: Stopped, at once + }; + + // A surface op the server's control loop has to post into the apply thread's mailbox - and + // cannot, while that thread is parked in the reservation: ServerMain blocks there with it. + // The seq is far from the session's own dense mint; nothing will read the reply. + bool SendSurfaceOpBehindTheWait(Client::ClientSession& client) { + Server::SurfaceControlFrame frame{}; + frame.kind = Server::SurfaceControlOp::InitializeDisplay; + frame.seq = 0x7fffffffull; + frame.display = 0x4d47; + flatbuffers::FlatBufferBuilder builder(256); + if (EncodeSurfaceOpFrame(frame, &builder) != SurfaceWireError::None) return false; + auto* control = client.ControlTransportForTest(); + return control != nullptr && + control->SendFrame(MobileGLByteSpan{builder.GetBufferPointer(), builder.GetSize()}) == MOBILEGL_OK; + } + + // Four bytes: too short to carry a CtrlEnvelope, which ServerMain answers by ending the session + // (P7 wave 0's named refusal) - a control stream that ENDS while the peer is still there. + bool SendMalformedControlFrame(Client::ClientSession& client) { + static const std::uint8_t kJunk[4] = {0xde, 0xad, 0xbe, 0xef}; + auto* control = client.ControlTransportForTest(); + return control != nullptr && control->SendFrame(MobileGLByteSpan{kJunk, sizeof(kJunk)}) == MOBILEGL_OK; + } + + // THE PEER THAT STOPS DRAINING. Writes one byte to `published` when the last readback is on + // the wire, puts the scenario's raw control frame (if any) on the control stream once the server + // is parked, then does nothing at all - no wait, no drain, no teardown - until the parent says + // the supervisor has reaped its session (or kills it), and leaves by _exit: a clean Stop() + // would drain. + [[noreturn]] void NonDrainingPeer(const std::string& control, Scenario scenario, int published, int release) { + ConfigureClient(control); + auto& client = Client::ClientSessionInstance(); + if (client.StartSpawned() != MOBILEGL_OK) ::_exit(kNoWelcome); + const int filled = FillTheEventRingWithoutDraining(client); + if (filled != kChildOk) ::_exit(filled); const char one = 1; (void)::write(published, &one, 1); + if (scenario == Scenario::KilledWithAControlOpQueued || scenario == Scenario::ControlEndsWhileWaiting) { + std::this_thread::sleep_for(std::chrono::milliseconds(kRawFrameAfterMs)); + const bool sent = scenario == Scenario::KilledWithAControlOpQueued ? SendSurfaceOpBehindTheWait(client) + : SendMalformedControlFrame(client); + if (!sent) ::_exit(kRawFrame); + } char go = 0; (void)::read(release, &go, 1); ::_exit(kChildOk); @@ -316,8 +416,45 @@ namespace { } } - void RunArm(bool tcp) { - const std::string tag = std::string(tcp ? "stream" : "shm") + "-" + std::to_string(::getpid()); + void RemoveRoleLogsIfGreen(const std::string& logBase) { + // A green run leaves nothing in /tmp; a red one keeps both role logs for the triage. The + // forfeit line and the reap lines go to this test's own output first, so a green lane log + // still says what the server named and how the session ended. + { + const std::string log = ServerLog(logBase); + for (const char* needle : {"ReverseChannelForfeit{", "mgl-srv-apply stops on", "reaped "}) { + for (auto at = log.find(needle); at != std::string::npos; at = log.find(needle, at + 1)) { + const std::string line = LineWith(log.substr(at), needle); + std::printf("[fz3] %.260s\n", line.c_str()); + } + } + std::fflush(stdout); + } + if (::testing::Test::HasFailure()) return; + for (const char* role : {".client.log", ".server.log"}) { + ::unlink((logBase.substr(0, logBase.size() - 4) + role).c_str()); + } + } + + const char* ScenarioTag(Scenario scenario) { + switch (scenario) { + case Scenario::StopsDraining: return "silent"; + case Scenario::KilledWhileWaiting: return "kill"; + case Scenario::KilledWithAControlOpQueued: return "kill-op"; + case Scenario::ControlEndsWhileWaiting: return "ctl-end"; + } + return "?"; + } + + void RunArm(bool tcp, Scenario scenario) { + // A peer the parent kills leaves the release pipe with no reader; the parent's write to it + // must be an EPIPE, not a SIGPIPE that takes this process down before the supervisor goes. + std::signal(SIGPIPE, SIG_IGN); + const bool killed = + scenario == Scenario::KilledWhileWaiting || scenario == Scenario::KilledWithAControlOpQueued; + const std::uint32_t waitMs = scenario == Scenario::StopsDraining ? kEventWaitMs : kLongEventWaitMs; + const std::string tag = std::string(tcp ? "stream-" : "shm-") + ScenarioTag(scenario) + "-" + + std::to_string(::getpid()); const std::string logBase = "/tmp/mgl-fz3-" + tag + ".log"; MobileGL::MG_Util::Debug::TruncateRoleLogs(logBase.c_str()); std::string endpoint, control; @@ -330,7 +467,7 @@ namespace { endpoint = "@mgl-fz3-" + tag; control = "unix:" + endpoint; } - const pid_t supervisor = LaunchSupervisor(endpoint, logBase); + const pid_t supervisor = LaunchSupervisor(endpoint, logBase, waitMs); ASSERT_GT(supervisor, 0); struct Reap { pid_t pid; @@ -348,7 +485,7 @@ namespace { if (peer == 0) { ::close(published[0]); ::close(release[1]); - NonDrainingPeer(control, published[1], release[0]); + NonDrainingPeer(control, scenario, published[1], release[0]); } ::close(published[1]); ::close(release[0]); @@ -360,54 +497,106 @@ namespace { (void)::write(fd, &go, 1); ::close(fd); int status = 0; - if (::waitpid(pid, &status, WNOHANG) == 0) { + if (pid > 0 && ::waitpid(pid, &status, WNOHANG) == 0) { ::kill(pid, SIGKILL); ::waitpid(pid, &status, 0); } } } releasePeer{release[1], peer}; - // THE CLOCK STARTS WHEN THE FOURTH READBACK IS ON THE WIRE. A peer that failed before it + // THE CLOCK STARTS WHEN THE LAST READBACK IS ON THE WIRE. A peer that failed before it // got there exits instead of writing, and read() returns 0. char byte = 0; const ssize_t got = ::read(published[0], &byte, 1); ::close(published[0]); if (got != 1) { const int code = WaitChild(peer, std::chrono::milliseconds(5000)); + releasePeer.pid = -1; FAIL() << "the non-draining peer did not reach its readbacks (child exit " << code << ")\n" << ServerLog(logBase); } const auto published_at = std::chrono::steady_clock::now(); - // The server waits the knob, forfeits by name, stops the apply thread, ends the session; - // the supervisor reaps it. Budget: the knob plus generous slack - and far under 30 s. - const long long stoppedMs = - WaitForLog(logBase, "mgl-srv-apply stops on ReverseChannelForfeit", 1, - std::chrono::milliseconds(kEventWaitMs + 20000)); - const long long reapedMs = WaitForLog(logBase, "reaped exit=0 sessionsFaulted=0", 1, - std::chrono::milliseconds(kEventWaitMs + 20000)); - const auto sinceReadbacks = std::chrono::duration_cast( - std::chrono::steady_clock::now() - published_at).count(); - const std::string log = ServerLog(logBase); - ASSERT_GE(stoppedMs, 0) << "the server never stopped on a forfeit\n" << log; - ASSERT_GE(reapedMs, 0) << "the supervisor never reaped the forfeited session with exit 0\n" << log; - EXPECT_NE(log.find("ReverseChannelForfeit{NotDraining} - kEventBufferWriteback"), std::string::npos) - << "the forfeit was not named for the event that could not be placed\n" << log; - EXPECT_NE(log.find("mgl-srv-apply stops on ReverseChannelForfeit - 1 reverse-channel event(s) dropped"), - std::string::npos) - << "the drop was not counted (one event: the fourth writeback)\n" << log; - EXPECT_NE(log.find("the apply thread has stopped (ReverseChannelForfeit"), std::string::npos) - << "the session child did not end the session from the server's side\n" << log; - EXPECT_EQ(log.find("Fatal{EventRingOverflow"), std::string::npos) << log; - EXPECT_EQ(log.find("reaped signal="), std::string::npos) << "a session died of a signal\n" << log; - // THE KNOB, NOT 30 s: the forfeit waited most of MOBILEGL_IPC_EVENT_WAIT_MS (it cannot - // have happened before the knob ran out on a peer that never drains), and the session was - // gone well inside the knob plus the supervisor's 250 ms reap poll. - EXPECT_GE(stoppedMs, static_cast(kEventWaitMs) * 8 / 10) << log; - EXPECT_LE(sinceReadbacks, static_cast(kEventWaitMs) + 8000) << log; + if (scenario != Scenario::StopsDraining) { + // THE WAIT ENDS EARLY, BY NAME. The server is parked in the fifth writeback's + // reservation with 8000 ms of patience - over ServerLoop::Stop()'s 5000 ms join - and + // either the peer dies (PeerGone) or its control stream ends under it while it stays + // (Stopped). Both must end that wait well inside the knob, and the session must still + // end the ordinary way, exit 0. Before the fix round the shm plane could not see the + // death at all: the control EOF made ServerMain call loop.Stop(), the reservation + // swallowed Stop()'s ring and waited on, and after 5000 ms the child died of + // Fatal{ApplyThreadJoinTimeout} (`reaped signal=6 sessionsFaulted=1`); at a knob under + // the join it was reported as NotDraining after the whole knob. + const char* expected = killed ? "ReverseChannelForfeit{PeerGone} - kEventBufferWriteback" + : "ReverseChannelForfeit{Stopped} - kEventBufferWriteback"; + auto endedAt = std::chrono::steady_clock::now(); + if (killed) { + std::this_thread::sleep_for(std::chrono::milliseconds(kKillAfterMs)); + ASSERT_EQ(::kill(peer, SIGKILL), 0); + int status = 0; + ::waitpid(peer, &status, 0); + releasePeer.pid = -1; + endedAt = std::chrono::steady_clock::now(); + } + const long long stoppedMs = WaitForLog(logBase, "mgl-srv-apply stops on ReverseChannelForfeit", 1, + std::chrono::milliseconds(waitMs + 20000)); + const auto stoppedAfterEnd = std::chrono::duration_cast( + std::chrono::steady_clock::now() - endedAt).count(); + const long long reapedMs = WaitForLog(logBase, "reaped exit=0 sessionsFaulted=0", 1, + std::chrono::milliseconds(waitMs + 20000)); + const std::string log = ServerLog(logBase); + ASSERT_GE(stoppedMs, 0) << "the server never stopped on a forfeit\n" << log; + ASSERT_GE(reapedMs, 0) << "the supervisor never reaped the session with exit 0\n" << log; + const std::string forfeit = LineWith(log, "ReverseChannelForfeit{"); + EXPECT_NE(forfeit.find(expected), std::string::npos) + << "the early end of the wait was not named " << expected << "\n" << log; + EXPECT_NE(forfeit.find(WaitKnobEntry(waitMs)), std::string::npos) + << "the forfeit did not run against the knob this supervisor was given\n" << log; + if (scenario == Scenario::ControlEndsWhileWaiting) { + EXPECT_NE(log.find("control frame is 4 bytes"), std::string::npos) + << "the malformed control frame never reached the server's control loop\n" << log; + } + // WELL INSIDE THE KNOB: from a descriptor or the stop request, not from the 8000 ms + // running out, and not from Stop()'s 5000 ms join either. + EXPECT_LE(stoppedAfterEnd, 3000) << log; + EXPECT_EQ(log.find("Fatal{"), std::string::npos) << log; + EXPECT_EQ(log.find("reaped signal="), std::string::npos) << "a session died of a signal\n" << log; + } else { + // The server waits the knob, forfeits by name, stops the apply thread, ends the session; + // the supervisor reaps it. Budget: the knob plus generous slack - and far under 30 s. + const long long stoppedMs = + WaitForLog(logBase, "mgl-srv-apply stops on ReverseChannelForfeit", 1, + std::chrono::milliseconds(waitMs + 20000)); + const long long reapedMs = WaitForLog(logBase, "reaped exit=0 sessionsFaulted=0", 1, + std::chrono::milliseconds(waitMs + 20000)); + const auto sinceReadbacks = std::chrono::duration_cast( + std::chrono::steady_clock::now() - published_at).count(); + const std::string log = ServerLog(logBase); + ASSERT_GE(stoppedMs, 0) << "the server never stopped on a forfeit\n" << log; + ASSERT_GE(reapedMs, 0) << "the supervisor never reaped the forfeited session with exit 0\n" << log; + const std::string forfeit = LineWith(log, "ReverseChannelForfeit{"); + EXPECT_NE(forfeit.find("ReverseChannelForfeit{NotDraining} - kEventBufferWriteback"), std::string::npos) + << "the forfeit was not named for the event that could not be placed\n" << log; + // THE VARIABLE WAS READ: the budget the server names is the one this supervisor was + // given, not the 2000 ms default it would fall back to. + EXPECT_NE(forfeit.find(WaitKnobEntry(waitMs)), std::string::npos) + << "the server did not wait against the MOBILEGL_IPC_EVENT_WAIT_MS it was given\n" << log; + EXPECT_NE(log.find("mgl-srv-apply stops on ReverseChannelForfeit - 1 reverse-channel event(s) dropped"), + std::string::npos) + << "the drop was not counted (one event: the fifth writeback)\n" << log; + EXPECT_NE(log.find("the apply thread has stopped (ReverseChannelForfeit"), std::string::npos) + << "the session child did not end the session from the server's side\n" << log; + EXPECT_EQ(log.find("Fatal{EventRingOverflow"), std::string::npos) << log; + EXPECT_EQ(log.find("reaped signal="), std::string::npos) << "a session died of a signal\n" << log; + // THE KNOB, NOT 30 s: the forfeit waited most of MOBILEGL_IPC_EVENT_WAIT_MS (it cannot + // have happened before the knob ran out on a peer that never drains), and the session was + // gone well inside the knob plus the supervisor's 250 ms reap poll. + EXPECT_GE(stoppedMs, static_cast(waitMs) * 8 / 10) << log; + EXPECT_LE(sinceReadbacks, static_cast(waitMs) + 8000) << log; + } - // THE NEXT CONNECTION IS WELCOMED, by the same supervisor, while the silent peer still - // holds its end of the old connection open. + // THE NEXT CONNECTION IS WELCOMED, by the same supervisor - while the silent peer still + // holds its end of the old connection open, or after it died mid-wait. std::fflush(nullptr); const pid_t next = ::fork(); ASSERT_GE(next, 0); @@ -418,18 +607,113 @@ namespace { EXPECT_GE(WaitForLog(logBase, "reaped exit=0 sessionsFaulted=0", 2, std::chrono::milliseconds(10000)), 0) << "the welcomed session did not end cleanly\n" << ServerLog(logBase); EXPECT_EQ(Count(ServerLog(logBase), "Refuse{Busy}"), 0u) << ServerLog(logBase); - // A green run leaves nothing in /tmp; a red one keeps both role logs for the triage. - if (!::testing::Test::HasFailure()) { - for (const char* role : {".client.log", ".server.log"}) { - ::unlink((logBase.substr(0, logBase.size() - 4) + role).c_str()); + RemoveRoleLogsIfGreen(logBase); + } + + // THE LAUNCHER'S SCRUB, crossed (PH-6 fix round). Server::LaunchServer builds the child's + // environment by removing every MOBILEGL_IPC_* except the few the server owns + // (ServerSpawn.cpp ShouldScrub's allow-list). The supervisor arms above are started by this test + // with an environment it writes itself, so they cannot see that list; this one is started the + // way a fork-dial client starts its server, with the knob set in the PEER's environment, and + // the server's own forfeit line has to name it. + void RunLaunchedArm() { + const std::string tag = "launched-" + std::to_string(::getpid()); + const std::string logBase = "/tmp/mgl-fz3-" + tag + ".log"; + const std::string endpoint = "@mgl-fz3-" + tag; + MobileGL::MG_Util::Debug::TruncateRoleLogs(logBase.c_str()); + + int published[2] = {-1, -1}; + ASSERT_EQ(::pipe(published), 0); + std::fflush(nullptr); + const pid_t peer = ::fork(); + ASSERT_GE(peer, 0); + if (peer == 0) { + ::close(published[0]); + // What a lane would have exported: the client's own environment, which the launcher + // scrubs on the way to the server. + ::setenv("MOBILEGL_IPC_EVENT_WAIT_MS", std::to_string(kLaunchedEventWaitMs).c_str(), 1); + ::setenv("MOBILEGL_LOG_FILE_PATH", logBase.c_str(), 1); + MobileGL::MG_Config::Transport = MobileGL::MG_Config::TransportMode::Spawn; + MobileGL::MG_Config::ActiveBackendType = MobileGL::BackendType::DirectGLES; + ::unsetenv("MOBILEGL_IPC_TOKEN"); + Server::LaunchedServer server; + if (Server::LaunchServer(ServerImage(), endpoint, &server) != MOBILEGL_OK) ::_exit(kLaunch); + std::unique_ptr transport; + if (Transport::SocketTransport::ConnectTo(endpoint, 10000, transport) != MOBILEGL_OK) ::_exit(kNoWelcome); + auto& client = Client::ClientSessionInstance(); + if (client.StartOverSocket(std::move(transport)) != MOBILEGL_OK) ::_exit(kNoWelcome); + const int filled = FillTheEventRingWithoutDraining(client); + if (filled != kChildOk) ::_exit(filled); + const char one = 1; + (void)::write(published[1], &one, 1); + // The single-session server ends its session on the forfeit and exits by itself; this + // peer, its parent, is the one that can say how. + int exitCode = -1; + if (Server::ReapServer(server, kLaunchedEventWaitMs + 15000, &exitCode) != MOBILEGL_OK) { + ::kill(server.pid, SIGKILL); + ::_exit(kServerNotReaped); } + ::_exit(exitCode == 0 ? kChildOk : kServerExitedNonZero); } + ::close(published[1]); + char byte = 0; + const ssize_t got = ::read(published[0], &byte, 1); + ::close(published[0]); + const auto published_at = std::chrono::steady_clock::now(); + const int code = WaitChild(peer, std::chrono::milliseconds(kLaunchedEventWaitMs + 20000)); + const auto sinceReadbacks = std::chrono::duration_cast( + std::chrono::steady_clock::now() - published_at).count(); + const std::string log = ServerLog(logBase); + ASSERT_EQ(got, 1) << "the peer did not reach its readbacks (child exit " << code << ")\n" << log; + EXPECT_EQ(code, static_cast(kChildOk)) + << "the launched server was not reaped with exit 0 (child exit " << code << ")\n" << log; + const std::string forfeit = LineWith(log, "ReverseChannelForfeit{"); + EXPECT_NE(forfeit.find("ReverseChannelForfeit{NotDraining} - kEventBufferWriteback"), std::string::npos) + << log; + EXPECT_NE(forfeit.find(WaitKnobEntry(kLaunchedEventWaitMs)), std::string::npos) + << "the launched server did not wait against the knob its client exported - the launcher " + "scrubbed it\n" << log; + EXPECT_NE(log.find("the apply thread has stopped (ReverseChannelForfeit"), std::string::npos) << log; + EXPECT_EQ(log.find("Fatal{"), std::string::npos) << log; + EXPECT_GE(sinceReadbacks, static_cast(kLaunchedEventWaitMs) * 8 / 10) << log; + EXPECT_LE(sinceReadbacks, static_cast(kLaunchedEventWaitMs) + 8000) << log; + RemoveRoleLogsIfGreen(logBase); } } // namespace -TEST(EventForfeitPeer, ShmPeerThatStopsDrainingIsForfeitedAndTheNextConnectionIsWelcomed) { RunArm(false); } +TEST(EventForfeitPeer, ShmPeerThatStopsDrainingIsForfeitedAndTheNextConnectionIsWelcomed) { + RunArm(false, Scenario::StopsDraining); +} -TEST(EventForfeitPeer, StreamPeerThatStopsDrainingIsForfeitedAndTheNextConnectionIsWelcomed) { RunArm(true); } +TEST(EventForfeitPeer, ShmPeerKilledWhileTheServerWaitsIsPeerGoneWellInsideTheKnob) { + RunArm(false, Scenario::KilledWhileWaiting); +} + +TEST(EventForfeitPeer, ShmPeerKilledWithASurfaceOpQueuedBehindTheWaitIsPeerGoneFromTheBell) { + RunArm(false, Scenario::KilledWithAControlOpQueued); +} + +TEST(EventForfeitPeer, ShmControlStreamThatEndsWhileTheServerWaitsStopsTheWaitByName) { + RunArm(false, Scenario::ControlEndsWhileWaiting); +} + +TEST(EventForfeitPeer, ShmLaunchedServerTakesTheWaitKnobThroughTheScrubbedEnvironment) { RunLaunchedArm(); } + +TEST(EventForfeitPeer, StreamPeerThatStopsDrainingIsForfeitedAndTheNextConnectionIsWelcomed) { + RunArm(true, Scenario::StopsDraining); +} + +TEST(EventForfeitPeer, StreamPeerKilledWhileTheServerWaitsIsPeerGoneWellInsideTheKnob) { + RunArm(true, Scenario::KilledWhileWaiting); +} + +TEST(EventForfeitPeer, StreamPeerKilledWithASurfaceOpQueuedBehindTheWaitIsPeerGoneFromTheBell) { + RunArm(true, Scenario::KilledWithAControlOpQueued); +} + +TEST(EventForfeitPeer, StreamControlStreamThatEndsWhileTheServerWaitsStopsTheWaitByName) { + RunArm(true, Scenario::ControlEndsWhileWaiting); +} #endif diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index 57922244..1ba65d72 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -2963,6 +2963,59 @@ TEST(RemoteRunAhead, ATricklingClientGetsTheWholeWaitThenForfeitsByNameInsteadOf EXPECT_EQ(child.Log.find("Fatal{EventRingOverflow"), std::string::npos) << child.Log; } +// STOP WHILE THE SERVER WAITS (PH-6 fix round). ServerLoop::Stop() arrives while the apply thread is +// parked inside the reservation, on a bell that is still ALIVE - the spawn session's shape when its +// control stream ends (EOF on a half-close, a malformed frame, a LogFlush ack) while the client is +// not draining. The knob is 20 s, four times Stop()'s 5000 ms bounded join, so this child can only +// exit cleanly if the wait ends on the stop request itself and forfeits as `Stopped`. +// +// THE RED: take ApplyStopRequested() out of ReserveEventOrBlock's predicate and the ring Stop() +// sends is swallowed by a wait that re-tests only "has the client drained?"; the reservation sits +// out its 20 s and Stop() aborts the child after 5 s with Fatal{ApplyThreadJoinTimeout}. +TEST(RemoteRunAhead, AStopWhileTheServerWaitsForADrainEndsTheWaitByNameInsteadOfTheJoinTimeout) { + const auto child = RunInChild([] { + MG_Config::Ipc.EventWaitMs = 20000; + StartRunAheadSession(); + // The producer's probe blocks inside the reservation, so it is posted from a thread of its + // own: RunProbeOnApplyThreadForTesting waits for the probe to return. + std::thread producer([] { + (void)Srv::ServerLoopInstance().RunProbeOnApplyThreadForTesting( + +[](void*) -> MobileGLResult { + MG_Pipe::MGPRange ranges[64]; + for (Uint32 r = 0; r < 64; ++r) ranges[r] = MG_Pipe::MGPRange{r * 64ull, 64ull}; + for (Uint32 i = 0; i < 3000; ++i) { + MG_Pipe::gMGPipeCallbacks.OnGpuWritten(MG_Pipe::MGPipeHandle{7, 1}, 64, ranges); + } + return MOBILEGL_OK; + }, + nullptr); + }); + // The reservation is waiting once SEG_EVENT's latch is up: nothing in this child drains. + const auto signals = Srv::ServerSessionInstance().DataLink()->Signals(); + const auto until = std::chrono::steady_clock::now() + std::chrono::milliseconds(5000); + while (signals.EventRingFull->load(std::memory_order_acquire) == 0) { + if (std::chrono::steady_clock::now() > until) ::_exit(106); + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } + std::this_thread::sleep_for(std::chrono::milliseconds(200)); // well into the park + const auto start = std::chrono::steady_clock::now(); + Srv::ServerLoopInstance().Stop(); + const Uint64 stopMs = MillisecondsSince(start); + producer.join(); + auto& server = Srv::ServerSessionInstance(); + if (!server.ReverseChannelForfeited()) ::_exit(101); + if (server.ForfeitDrops() == 0) ::_exit(102); + // Stop() came back on the request, not on the join's 5000 ms or the knob's 20 s. + if (stopMs > 2000u) ::_exit(104); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); + EXPECT_NE(child.Log.find("ReverseChannelForfeit{Stopped} - kEventGpuWritten"), std::string::npos) + << child.Log; + EXPECT_EQ(child.Log.find("Fatal{ApplyThreadJoinTimeout"), std::string::npos) << child.Log; + EXPECT_EQ(child.Log.find("Fatal{EventRingOverflow"), std::string::npos) << child.Log; +} + // ยง2.7 / ruling 13: the deferred-destroy queue stays, and an enqueue from an unbarriered // apply is the finding. Under strict it is the abort; this is the arm the lane owns. TEST(RemoteGuards, ADeferredDestroyFromAnUnbarrieredApplyIsFatalUnderStrict) {