Fix leaking of callback timers

Use two timers to create timeout trigger for framecallback.
Removes multiple timers approach that is subject to race conditions.

Fixes: QTBUG-82914
Change-Id: Ia10b1b4e055eee80f77076137066ecada683ba32
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>

2 files changed, 22 insertions, 28 deletions

diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp
index 7a99363c8..cf8b60f10 100644
--- a/src/client/qwaylandwindow.cpp
+++ b/src/client/qwaylandwindow.cpp
@@ -263,10 +263,7 @@ void QWaylandWindow::reset(bool sendDestroyEvent)
         mFrameCallback = nullptr;
     }
 
-    int timerId  = mFrameCallbackTimerId.fetchAndStoreOrdered(-1);
-    if (timerId != -1) {
-        killTimer(timerId);
-    }
+    mFrameCallbackElapsedTimer.invalidate();
     mWaitingForFrameCallback = false;
     mFrameCallbackTimedOut = false;
 
@@ -635,15 +632,11 @@ const wl_callback_listener QWaylandWindow::callbackListener = {
 
 void QWaylandWindow::handleFrameCallback()
 {
-    // Stop the timer and stop waiting immediately
-    int timerId = mFrameCallbackTimerId.fetchAndStoreOrdered(-1);
     mWaitingForFrameCallback = false;
+    mFrameCallbackElapsedTimer.invalidate();
 
     // The rest can wait until we can run it on the correct thread
-    auto doHandleExpose = [this, timerId]() {
-        if (timerId != -1)
-            killTimer(timerId);
-
+    auto doHandleExpose = [this]() {
         bool wasExposed = isExposed();
         mFrameCallbackTimedOut = false;
         if (!wasExposed && isExposed()) // Did setting mFrameCallbackTimedOut make the window exposed?
@@ -678,13 +671,6 @@ bool QWaylandWindow::waitForFrameSync(int timeout)
         sendExposeEvent(QRect());
     }
 
-    // Stop current frame timer if any, can't use killTimer directly, because we might be on a diffent thread
-    // Ordered semantics is needed to avoid stopping the timer twice and not miss it when it's
-    // started by other writes
-    int fcbId = mFrameCallbackTimerId.fetchAndStoreOrdered(-1);
-    if (fcbId != -1)
-        QMetaObject::invokeMethod(this, [this, fcbId] { killTimer(fcbId); }, Qt::QueuedConnection);
-
     return !mWaitingForFrameCallback;
 }
 
@@ -1097,8 +1083,16 @@ QVariant QWaylandWindow::property(const QString &name, const QVariant &defaultVa
 
 void QWaylandWindow::timerEvent(QTimerEvent *event)
 {
-    if (mFrameCallbackTimerId.testAndSetOrdered(event->timerId(), -1)) {
-        killTimer(event->timerId());
+    if (event->timerId() != mFrameCallbackCheckIntervalTimerId)
+        return;
+
+    bool callbackTimerExpired = mFrameCallbackElapsedTimer.hasExpired(100);
+    if (!mFrameCallbackElapsedTimer.isValid() || callbackTimerExpired ) {
+        killTimer(mFrameCallbackCheckIntervalTimerId);
+        mFrameCallbackCheckIntervalTimerId = -1;
+    }
+    if (mFrameCallbackElapsedTimer.isValid() && callbackTimerExpired) {
+        mFrameCallbackElapsedTimer.invalidate();
         qCDebug(lcWaylandBackingstore) << "Didn't receive frame callback in time, window should now be inexposed";
         mFrameCallbackTimedOut = true;
         mWaitingForUpdate = false;
@@ -1152,15 +1146,13 @@ void QWaylandWindow::handleUpdate()
     mWaitingForFrameCallback = true;
     mWaitingForUpdate = false;
 
-    // Stop current frame timer if any, can't use killTimer directly, see comment above.
-    int fcbId = mFrameCallbackTimerId.fetchAndStoreOrdered(-1);
-    if (fcbId != -1)
-        QMetaObject::invokeMethod(this, [this, fcbId] { killTimer(fcbId); }, Qt::QueuedConnection);
-
     // Start a timer for handling the case when the compositor stops sending frame callbacks.
-    QMetaObject::invokeMethod(this, [this] { // Again; can't do it directly
-        if (mWaitingForFrameCallback)
-            mFrameCallbackTimerId = startTimer(100);
+    QMetaObject::invokeMethod(this, [this] {
+        if (mWaitingForFrameCallback) {
+            if (mFrameCallbackCheckIntervalTimerId < 0)
+                mFrameCallbackCheckIntervalTimerId = startTimer(100);
+            mFrameCallbackElapsedTimer.start();
+        }
     }, Qt::QueuedConnection);
 }
 
diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h
index 0369bd0c2..0b664bda0 100644
--- a/src/client/qwaylandwindow_p.h
+++ b/src/client/qwaylandwindow_p.h
@@ -58,6 +58,7 @@
 #include <QtGui/QIcon>
 #include <QtCore/QVariant>
 #include <QtCore/QLoggingCategory>
+#include <QtCore/QElapsedTimer>
 
 #include <qpa/qplatformwindow.h>
 
@@ -222,7 +223,8 @@ protected:
     WId mWindowId;
     bool mWaitingForFrameCallback = false;
     bool mFrameCallbackTimedOut = false; // Whether the frame callback has timed out
-    QAtomicInt mFrameCallbackTimerId = -1; // Started on commit, reset on frame callback
+    int mFrameCallbackCheckIntervalTimerId = -1;
+    QElapsedTimer mFrameCallbackElapsedTimer;
     struct ::wl_callback *mFrameCallback = nullptr;
     struct ::wl_event_queue *mFrameQueue = nullptr;
     QWaitCondition mFrameSyncWait;