From fa823e4b1cfaf8621cd35fc4e350e3a840b80ebf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= <Bruebach.Lukas@bdosecurity.de>
Date: Sun, 23 Aug 2026 00:19:18 +0200
Subject: [PATCH] [Client] Discard suspend-spanning gaps in LagMonitor

Windows counts sleep time in its monotonic clock, so a suspend would
fabricate one bogus stall per resume. Reset the clock on application
state changes and drop implausibly huge gaps; extract recordGap() for
testability.

Took 3 minutes
---
 cockatrice/src/client/lag_monitor.cpp | 32 ++++++++++++++++++++++-----
 cockatrice/src/client/lag_monitor.h   | 24 ++++++++++++++++++--
 2 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/cockatrice/src/client/lag_monitor.cpp b/cockatrice/src/client/lag_monitor.cpp
index 5a7efe6df..383d64644 100644
--- a/cockatrice/src/client/lag_monitor.cpp
+++ b/cockatrice/src/client/lag_monitor.cpp
@@ -1,9 +1,13 @@
 #include "lag_monitor.h"
 
+#include <QCoreApplication>
+#include <QEvent>
 #include <QTimer>
 
 LagMonitor::LagMonitor(QObject *parent) : QObject(parent)
 {
+    qApp->installEventFilter(this);
+
     timer = new QTimer(this);
     timer->setInterval(TICK_INTERVAL_MS);
     connect(timer, &QTimer::timeout, this, &LagMonitor::checkTick);
@@ -21,23 +25,39 @@ void LagMonitor::clearStalls()
     stalls.clear();
 }
 
+bool LagMonitor::eventFilter(QObject *obj, QEvent *event)
+{
+    if (event->type() == QEvent::ApplicationStateChange) {
+        // The transition may span a suspend or an arbitrary unfocused period;
+        // discard the gap so it cannot be mistaken for a stall.
+        tickClock.restart();
+    }
+    return QObject::eventFilter(obj, event);
+}
+
 void LagMonitor::checkTick()
 {
-    const qint64 gap = tickClock.restart();
+    recordGap(tickClock.restart());
+}
 
-    if (gap <= STALL_THRESHOLD_MS) {
+void LagMonitor::recordGap(qint64 gapMs)
+{
+    if (gapMs <= STALL_THRESHOLD_MS) {
         return;
     }
 
-    StallRecord record;
-    record.timestampMsSinceEpoch = QDateTime::currentMSecsSinceEpoch(); // wall time, for log correlation
-    record.durationMs = gap;
+    if (gapMs > MAX_PLAUSIBLE_STALL_MS) {
+        qCDebug(LagMonitorLog, "Ignoring implausible %lld ms gap (likely suspend)", static_cast<long long>(gapMs));
+        return;
+    }
+
+    const StallRecord record{.timestampMsSinceEpoch = QDateTime::currentMSecsSinceEpoch(), .durationMs = gapMs};
 
     stalls.append(record);
     while (stalls.size() > MAX_RECORDED_STALLS) {
         stalls.removeFirst();
     }
 
-    qCWarning(LagMonitorLog, "Event loop stalled for %lld ms (threshold: %d ms)", static_cast<long long>(gap),
+    qCWarning(LagMonitorLog, "Event loop stalled for %lld ms (threshold: %d ms)", static_cast<long long>(gapMs),
               STALL_THRESHOLD_MS);
 }
diff --git a/cockatrice/src/client/lag_monitor.h b/cockatrice/src/client/lag_monitor.h
index 2dfdc2766..8dfb5e1fa 100644
--- a/cockatrice/src/client/lag_monitor.h
+++ b/cockatrice/src/client/lag_monitor.h
@@ -14,6 +14,7 @@
 
 inline Q_LOGGING_CATEGORY(LagMonitorLog, "lag_monitor");
 
+class QEvent;
 class QTimer;
 
 /**
@@ -24,6 +25,11 @@ class QTimer;
  * event loop for roughly the overshooting duration. This is what separates
  * "my client froze" from "the network is lagging" in user reports.
  *
+ * Gaps that span an application state change (suspend, minimize, focus
+ * loss) are discarded, and implausibly huge gaps are dropped, so operating
+ * system power events do not fabricate stalls. This handling is load-bearing
+ * on Windows, where the monotonic clock used by Qt counts sleep time.
+ *
  * Healthy operation costs one timer wakeup per tick and two integer
  * comparisons. Allocations happen only when a stall is actually recorded.
  */
@@ -35,13 +41,16 @@ public:
     struct StallRecord
     {
         qint64 timestampMsSinceEpoch = 0; ///< when the stalled period ended
-        qint64 durationMs = 0;            ///< approximate length of the freeze
+        qint64 durationMs = 0;            ///< approximate length of the freeze; measured tick to tick, so it can exceed the true stall by up to TICK_INTERVAL_MS
     };
 
     static constexpr int TICK_INTERVAL_MS = 500;
     static constexpr int STALL_THRESHOLD_MS = 2000;
     static constexpr int MAX_RECORDED_STALLS = 32;
 
+    /// Gaps beyond this are treated as suspend artifacts rather than stalls.
+    static constexpr qint64 MAX_PLAUSIBLE_STALL_MS = 600000;
+
     explicit LagMonitor(QObject *parent = nullptr);
 
     /**
@@ -54,12 +63,23 @@ public:
 
     void clearStalls();
 
+    /**
+     * @brief Feeds a measured tick-to-tick gap through the detection logic.
+     *
+     * Split out of checkTick so threshold, plausibility, and trim behavior
+     * stay unit-testable without real timing.
+     */
+    void recordGap(qint64 gapMs);
+
+protected:
+    bool eventFilter(QObject *obj, QEvent *event) override;
+
 private slots:
     void checkTick();
 
 private:
     QTimer *timer;
-    QElapsedTimer tickClock; ///< monotonic clock, so wall clock steps and suspend do not fabricate stalls
+    QElapsedTimer tickClock; ///< monotonic clock, so wall clock steps do not fabricate stalls
     QList<StallRecord> stalls;
 };