[Fix] (Server): wait out an apply thread that is busy in a GPU stall at Stop() instead of aborting every session

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<std::mutex> 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 -

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<Bool> m_applyParked{false};
 
         Uint64 m_affinityMask = 0;
         std::atomic<Uint64> m_drained{0};

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<int> 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::milliseconds>(std::chrono::steady_clock::now() - started).count();
+        std::fprintf(stderr, "[busy-stop] Stop() returned in %lld ms\n", static_cast<long long>(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) {