1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix leaking of callback timers

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

This was mistakenly pushed to 5.12 branch which will not be
automatically merged upstream, so it has to be cherry-picked.

Fixes: QTBUG-82914
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>
(cherry picked from commit d18c29931b0bc889fff66bdbde89133544ba0529)
Change-Id: I258c0c08f0ac5803192fc0024e40ba72e72c37a8
Reviewed-by: Janne Koskinen <janne.p.koskinen@qt.io>
This commit is contained in:
Janne Koskinen 2020-03-24 14:22:35 +02:00 committed by Eskil Abrahamsen Blomfeldt
parent 85f4f2827b
commit be03178c4f
2 changed files with 22 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
src/plugins/platforms/wayland/qwaylandwindow.cpp
View File

@ -257,10 +257,7 @@ void QWaylandWindow::reset(bool sendDestroyEvent)
mFrameCallback = nullptr; mFrameCallback = nullptr;
} }
int timerId = mFrameCallbackTimerId.fetchAndStoreOrdered(-1); mFrameCallbackElapsedTimer.invalidate();
if (timerId != -1) {
killTimer(timerId);
}
mWaitingForFrameCallback = false; mWaitingForFrameCallback = false;
mFrameCallbackTimedOut = false; mFrameCallbackTimedOut = false;
@ -602,15 +599,11 @@ const wl_callback_listener QWaylandWindow::callbackListener = {
void QWaylandWindow::handleFrameCallback() void QWaylandWindow::handleFrameCallback()
{ {
// Stop the timer and stop waiting immediately
int timerId = mFrameCallbackTimerId.fetchAndStoreOrdered(-1);
mWaitingForFrameCallback = false; mWaitingForFrameCallback = false;
mFrameCallbackElapsedTimer.invalidate();
// The rest can wait until we can run it on the correct thread // The rest can wait until we can run it on the correct thread
auto doHandleExpose = [this, timerId]() { auto doHandleExpose = [this]() {
if (timerId != -1)
killTimer(timerId);
bool wasExposed = isExposed(); bool wasExposed = isExposed();
mFrameCallbackTimedOut = false; mFrameCallbackTimedOut = false;
if (!wasExposed && isExposed()) // Did setting mFrameCallbackTimedOut make the window exposed? if (!wasExposed && isExposed()) // Did setting mFrameCallbackTimedOut make the window exposed?
@ -645,13 +638,6 @@ bool QWaylandWindow::waitForFrameSync(int timeout)
sendExposeEvent(QRect()); 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; return !mWaitingForFrameCallback;
} }
@ -1107,8 +1093,16 @@ QVariant QWaylandWindow::property(const QString &name, const QVariant &defaultVa
void QWaylandWindow::timerEvent(QTimerEvent *event) void QWaylandWindow::timerEvent(QTimerEvent *event)
{ {
if (mFrameCallbackTimerId.testAndSetOrdered(event->timerId(), -1)) { if (event->timerId() != mFrameCallbackCheckIntervalTimerId)
killTimer(event->timerId()); 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"; qCDebug(lcWaylandBackingstore) << "Didn't receive frame callback in time, window should now be inexposed";
mFrameCallbackTimedOut = true; mFrameCallbackTimedOut = true;
mWaitingForUpdate = false; mWaitingForUpdate = false;
@ -1162,15 +1156,13 @@ void QWaylandWindow::handleUpdate()
mWaitingForFrameCallback = true; mWaitingForFrameCallback = true;
mWaitingForUpdate = false; 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. // Start a timer for handling the case when the compositor stops sending frame callbacks.
QMetaObject::invokeMethod(this, [this] { // Again; can't do it directly QMetaObject::invokeMethod(this, [this] {
if (mWaitingForFrameCallback) if (mWaitingForFrameCallback) {
mFrameCallbackTimerId = startTimer(100); if (mFrameCallbackCheckIntervalTimerId < 0)
mFrameCallbackCheckIntervalTimerId = startTimer(100);
mFrameCallbackElapsedTimer.start();
}
}, Qt::QueuedConnection); }, Qt::QueuedConnection);
} }

4
src/plugins/platforms/wayland/qwaylandwindow_p.h
View File

@ -58,6 +58,7 @@
#include <QtGui/QIcon> #include <QtGui/QIcon>
#include <QtCore/QVariant> #include <QtCore/QVariant>
#include <QtCore/QLoggingCategory> #include <QtCore/QLoggingCategory>
#include <QtCore/QElapsedTimer>
#include <qpa/qplatformwindow.h> #include <qpa/qplatformwindow.h>
@ -225,7 +226,8 @@ protected:
WId mWindowId; WId mWindowId;
bool mWaitingForFrameCallback = false; bool mWaitingForFrameCallback = false;
bool mFrameCallbackTimedOut = false; // Whether the frame callback has timed out 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_callback *mFrameCallback = nullptr;
struct ::wl_event_queue *mFrameQueue = nullptr; struct ::wl_event_queue *mFrameQueue = nullptr;
QWaitCondition mFrameSyncWait; QWaitCondition mFrameSyncWait;