diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index 02b71fe0..1f194491 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -105,6 +105,14 @@ namespace MobileGL::MG_Remote::Server { // The bounded join. InProcessTransportTest.cpp:344 uses five seconds for the same // reason: a lost wakeup must be a RED TEST and not a hung CI job. constexpr Uint32 kJoinTimeoutMs = 5000; + // THE BUSY BOUND. A thread that is not parked when kJoinTimeoutMs runs out is inside the + // backend - a draw or a fence wait the GPU has not finished - and on a GPU the driver has + // to reset (a shader that never ends: Adreno's preemption fault fires after ~10 s, once per + // stuck submission) that takes several times the lost-wakeup bound. Dying at 5 s there + // took the whole display server - every session, the compositor's included - down with + // the one client whose GPU work hung. The wait goes on, to this bound, which still turns + // a thread that never comes back into a named death. + constexpr Uint32 kBusyJoinTimeoutMs = 60000; // A core counts as "big" if its cpufreq ceiling is within 15% of the fastest core's - // the identical rule and the identical constant as ShaderCompilePool.cpp:28 and :73-95. @@ -673,9 +681,11 @@ namespace MobileGL::MG_Remote::Server { // Flush-on-idle publishes the last reply record even below the batch threshold. session.FlushDataProgress(); const Bool fencePending = verbs.HasUnreportedFences(); + m_applyParked.store(true, std::memory_order_release); const bool woke = bell.Wait(*signals.ConsumerParked, ready, spinUs, fencePending ? kFenceIdlePollMs : Transport::kWaitForever, &m_parkBlocks); + m_applyParked.store(false, std::memory_order_release); if (!woke) { // Wait returns false only on a dead bell or an expired deadline. A dead bell IS the // shutdown signal (table 3's teardown step 2); treating it as anything else would @@ -1292,6 +1302,19 @@ namespace MobileGL::MG_Remote::Server { exited = m_exitCv.wait_for(lock, std::chrono::milliseconds(kJoinTimeoutMs), [this] { return m_exited; }); } + if (!exited && !m_applyParked.load(std::memory_order_acquire)) { + // BUSY, NOT LOST (kBusyJoinTimeoutMs): the thread is applying, most likely blocked on + // GPU work that has not finished. What is still queued behind it belongs to a peer + // that is leaving, so it is not applied; the join waits for the work in hand. + MGLOG_W("MG_Remote server: mgl-srv-apply is still busy in the backend %u ms after Stop() (a " + "GPU stall, not a lost wakeup); the queued records are abandoned and the join waits " + "up to %u ms", + kJoinTimeoutMs, kBusyJoinTimeoutMs); + AbandonQueuedRecords(); + std::unique_lock lock(m_exitMutex); + exited = m_exitCv.wait_for(lock, std::chrono::milliseconds(kBusyJoinTimeoutMs - kJoinTimeoutMs), + [this] { return m_exited; }); + } if (!exited) { // ABORT, NOT DETACH. A detached apply thread still owns the EGL context and would // run on into the client's teardown, reading rings the client is about to unmap - diff --git a/MobileGL/MG_Remote/Server/ServerLoop.h b/MobileGL/MG_Remote/Server/ServerLoop.h index e3284e19..c22ad5b6 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.h +++ b/MobileGL/MG_Remote/Server/ServerLoop.h @@ -602,6 +602,10 @@ namespace MobileGL::MG_Remote::Server { std::mutex m_exitMutex; std::condition_variable m_exitCv; Bool m_exited = false; + // True only while mgl-srv-apply is parked on its consumer bell - the one place a lost + // wakeup can strand it. Stop()'s join reads it to tell a lost wakeup (parked: Fatal at + // the bound) from a thread that is BUSY in the backend (a GPU stall: waited for longer). + std::atomic m_applyParked{false}; Uint64 m_affinityMask = 0; std::atomic m_drained{0}; diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index a7e846d5..a03ed396 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -3908,6 +3908,54 @@ namespace { } } // namespace +namespace { + // A STOP WHILE THE APPLY THREAD IS BUSY IN THE BACKEND. One record whose apply takes 8 s - a + // GPU the driver needs ~10 s to reset blocks a fence wait just like this - is in hand when + // Stop() arrives, longer than the 5 s lost-wakeup bound. The thread is not parked, so the join + // waits for it instead of aborting the process (which, in the display server, is every other + // client's session too). Red with the busy arm removed: Fatal{ApplyThreadJoinTimeout} at 5 s. + std::atomic g_busyRecords{0}; + void StallOnce() { + if (g_busyRecords.fetch_add(1, std::memory_order_acq_rel) == 0) + std::this_thread::sleep_for(std::chrono::milliseconds(8000)); + } + + [[noreturn]] void StopWhileTheApplyThreadIsBusyAndExit() { + Remote::ArmSessionLatch(); + ServerFixture fixture; + if (!fixture.Handshake() || !fixture.StartLoop()) ::_exit(64); + if (!fixture.WaitUntilTrulyParked()) ::_exit(65); + Server::ServerLoop& loop = Server::ServerLoopInstance(); + loop.SetBetweenRecordsHookForTesting(&StallOnce); + MG_Pipe::MGPDrawInfo info{}; + info.Mode = 0x0004; // GL_TRIANGLES + info.InstanceCount = 1; + info.MinIndex = ~0u; + info.MaxIndex = ~0u; + info.NumDraws = 1; + const MG_Pipe::MGPDrawRange range{0, 3, 0}; + const Uint64 last = fixture.encoder.EncodeRecord(MG_Pipe::MGPWireOp::DrawVbo, &info, sizeof(info), &range, + sizeof(range)); + if (last == Codec::kInvalidSeq) ::_exit(66); + fixture.encoder.Publish(); + fixture.producer.PublishAndNotify(last); + if (!PollUntil([] { return g_busyRecords.load(std::memory_order_acquire) >= 1; }, 3000)) ::_exit(67); + const auto started = std::chrono::steady_clock::now(); + loop.Stop(); + const auto stopMs = + std::chrono::duration_cast(std::chrono::steady_clock::now() - started).count(); + std::fprintf(stderr, "[busy-stop] Stop() returned in %lld ms\n", static_cast(stopMs)); + loop.SetBetweenRecordsHookForTesting(nullptr); + fixture.Stop(); + ::_exit(stopMs < 6000 ? 1 : 0); // it must have waited for the stall, not cut it short + } +} // namespace + +TEST(ServerLoopTest, AStopWhileTheApplyThreadIsBusyInTheBackendWaitsForItInsteadOfAborting) { + EXPECT_EXIT(StopWhileTheApplyThreadIsBusyAndExit(), ::testing::ExitedWithCode(0), ".*") + << "1 = Stop() returned before the stall ended; 64+ = setup; SIGABRT = Fatal{ApplyThreadJoinTimeout}"; +} + // P12 review fix. Red with AbandonQueuedRecords a no-op: the batch and the exit-path drain apply the // whole stream, Stop()'s join gives up at 5 s and Fatal{ApplyThreadJoinTimeout} aborts the process. TEST(ServerLoopTest, StoppingTheServerUnderAStreamingClientLeavesTheQueueAndJoinsPromptly) {