gnu: qtwayland: Fix crashes from excessive number of frame callbacks.
QWaylandWindow::handleUpdate could create thousands of pending frame callbacks, causing compositor to terminate client connection. https://bugreports.qt.io/browse/QTBUG-81504 * gnu/packages/patches/qtwayland-dont-recreate-callbacks.patch: New file. * gnu/packages/patches/qtwayland-cleanup-callbacks.patch: New file. * gnu/local.mk (dist_patch_DATA): Adjust accordingly. * gnu/packages/qt.scm (qtwayland)[source](patches): Add patches.
parent
ae071e21f3
commit
a43c524252
|
@ -1775,6 +1775,8 @@ dist_patch_DATA = \
|
||||||
%D%/packages/patches/quagga-reproducible-build.patch \
|
%D%/packages/patches/quagga-reproducible-build.patch \
|
||||||
%D%/packages/patches/quickswitch-fix-dmenu-check.patch \
|
%D%/packages/patches/quickswitch-fix-dmenu-check.patch \
|
||||||
%D%/packages/patches/qtwayland-gcc-11.patch \
|
%D%/packages/patches/qtwayland-gcc-11.patch \
|
||||||
|
%D%/packages/patches/qtwayland-dont-recreate-callbacks.patch \
|
||||||
|
%D%/packages/patches/qtwayland-cleanup-callbacks.patch \
|
||||||
%D%/packages/patches/qtwebkit-pbutils-include.patch \
|
%D%/packages/patches/qtwebkit-pbutils-include.patch \
|
||||||
%D%/packages/patches/qtwebkit-fix-building-with-bison-3.7.patch \
|
%D%/packages/patches/qtwebkit-fix-building-with-bison-3.7.patch \
|
||||||
%D%/packages/patches/qtwebkit-fix-building-with-python-3.9.patch \
|
%D%/packages/patches/qtwebkit-fix-building-with-python-3.9.patch \
|
||||||
|
|
|
@ -0,0 +1,52 @@
|
||||||
|
From 42cdc61a93cf2acb09936aebb5e431fdbc0a26c6 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Georges Basile Stavracas Neto <gbsneto@gnome.org>
|
||||||
|
Date: Thu, 27 May 2021 20:02:53 -0300
|
||||||
|
Subject: [PATCH] Client: Always destroy frame callback in the actual callback
|
||||||
|
|
||||||
|
It's good hygiene to destroy all frame callbacks. Destroy the
|
||||||
|
frame callback and cleanup the mFrameCallback class member in
|
||||||
|
the callback itself. The callback destruction happens before
|
||||||
|
calling handleFrameCallback() to avoid the theoretical case
|
||||||
|
where another frame callback is queued by handleFrameCallback(),
|
||||||
|
and then immediately destroyed in the callback handler.
|
||||||
|
|
||||||
|
Change-Id: Ide6dc95e3402932c58bfc088a9d471fda821e9a1
|
||||||
|
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>
|
||||||
|
---
|
||||||
|
src/client/qwaylandwindow.cpp | 14 +++++---------
|
||||||
|
1 file changed, 5 insertions(+), 9 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp
|
||||||
|
index d83d51695..5561f58f7 100644
|
||||||
|
--- a/src/client/qwaylandwindow.cpp
|
||||||
|
+++ b/src/client/qwaylandwindow.cpp
|
||||||
|
@@ -659,9 +659,13 @@ void QWaylandWindow::commit()
|
||||||
|
|
||||||
|
const wl_callback_listener QWaylandWindow::callbackListener = {
|
||||||
|
[](void *data, wl_callback *callback, uint32_t time) {
|
||||||
|
- Q_UNUSED(callback);
|
||||||
|
Q_UNUSED(time);
|
||||||
|
auto *window = static_cast<QWaylandWindow*>(data);
|
||||||
|
+
|
||||||
|
+ Q_ASSERT(callback == window->mFrameCallback);
|
||||||
|
+ wl_callback_destroy(callback);
|
||||||
|
+ window->mFrameCallback = nullptr;
|
||||||
|
+
|
||||||
|
window->handleFrameCallback();
|
||||||
|
}
|
||||||
|
};
|
||||||
|
@@ -1366,11 +1370,6 @@ void QWaylandWindow::handleUpdate()
|
||||||
|
if (!mSurface)
|
||||||
|
return;
|
||||||
|
|
||||||
|
- if (mFrameCallback) {
|
||||||
|
- wl_callback_destroy(mFrameCallback);
|
||||||
|
- mFrameCallback = nullptr;
|
||||||
|
- }
|
||||||
|
-
|
||||||
|
QMutexLocker locker(mFrameQueue.mutex);
|
||||||
|
struct ::wl_surface *wrappedSurface = reinterpret_cast<struct ::wl_surface *>(wl_proxy_create_wrapper(mSurface->object()));
|
||||||
|
wl_proxy_set_queue(reinterpret_cast<wl_proxy *>(wrappedSurface), mFrameQueue.queue);
|
||||||
|
--
|
||||||
|
2.38.1
|
||||||
|
|
|
@ -0,0 +1,76 @@
|
||||||
|
From cbc74ba6d7186457d8d07183272e952dee5f34f9 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Georges Basile Stavracas Neto <gbsneto@gnome.org>
|
||||||
|
Date: Thu, 27 May 2021 19:55:04 -0300
|
||||||
|
Subject: [PATCH] Client: Don't always recreate frame callbacks
|
||||||
|
|
||||||
|
The main QWaylandWindow method that is executed when handling updates is
|
||||||
|
QWaylandWindow::handleUpdate(). This method always, unconditionally queues
|
||||||
|
a frame callback, regardless of whether any other one is already queued.
|
||||||
|
|
||||||
|
On some circumstances, e.g. when a window is hidden or completely obscured
|
||||||
|
by other windows, it stops receiving frame callbacks from the compositor.
|
||||||
|
However, QWaylandWindow would continue to request for them, which eventually
|
||||||
|
fills up the Wayland socket, and causes the application to crash.
|
||||||
|
|
||||||
|
This can be avoided by checking if the platform window is already waiting
|
||||||
|
for a frame callback, before queueing another one.
|
||||||
|
|
||||||
|
In QWaylandWindow::handleUpdate(), check if mWaitingForFrameCallback is true
|
||||||
|
before queueing frame callbacks, and early return if that's the case.
|
||||||
|
|
||||||
|
The XDG-shell test needed to be updated for this: The mock compositor is
|
||||||
|
not responding to any frame callbacks, so the window will be unexposed,
|
||||||
|
no longer get paint events and therefore not trigger any commit. This
|
||||||
|
worked by accident before because we were issuing updates quickly enough
|
||||||
|
to reset the timer before it had a chance to unexpose the window. The
|
||||||
|
easiest fix is just to disable the dependency on frame callbacks in
|
||||||
|
this test, since that is clearly not what it's testing.
|
||||||
|
|
||||||
|
Task-number: QTBUG-81504
|
||||||
|
Change-Id: Ieacb05c7d5a5fcf662243d9177ebcc308cb9ca84
|
||||||
|
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
|
||||||
|
Reviewed-by: Georges Basile Stavracas Neto <gbsneto@gnome.org>
|
||||||
|
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>
|
||||||
|
---
|
||||||
|
src/client/qwaylandwindow.cpp | 4 ++++
|
||||||
|
tests/auto/client/xdgshell/tst_xdgshell.cpp | 2 ++
|
||||||
|
2 files changed, 6 insertions(+)
|
||||||
|
|
||||||
|
diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp
|
||||||
|
index a708afce..d83d5169 100644
|
||||||
|
--- a/src/client/qwaylandwindow.cpp
|
||||||
|
+++ b/src/client/qwaylandwindow.cpp
|
||||||
|
@@ -1357,6 +1357,10 @@ void QWaylandWindow::requestUpdate()
|
||||||
|
void QWaylandWindow::handleUpdate()
|
||||||
|
{
|
||||||
|
qCDebug(lcWaylandBackingstore) << "handleUpdate" << QThread::currentThread();
|
||||||
|
+
|
||||||
|
+ if (mWaitingForFrameCallback)
|
||||||
|
+ return;
|
||||||
|
+
|
||||||
|
// TODO: Should sync subsurfaces avoid requesting frame callbacks?
|
||||||
|
QReadLocker lock(&mSurfaceLock);
|
||||||
|
if (!mSurface)
|
||||||
|
diff --git a/tests/auto/client/xdgshell/tst_xdgshell.cpp b/tests/auto/client/xdgshell/tst_xdgshell.cpp
|
||||||
|
index 1d2a2014..962093c7 100644
|
||||||
|
--- a/tests/auto/client/xdgshell/tst_xdgshell.cpp
|
||||||
|
+++ b/tests/auto/client/xdgshell/tst_xdgshell.cpp
|
||||||
|
@@ -138,6 +138,7 @@ void tst_xdgshell::configureSize()
|
||||||
|
|
||||||
|
void tst_xdgshell::configureStates()
|
||||||
|
{
|
||||||
|
+ QVERIFY(qputenv("QT_WAYLAND_FRAME_CALLBACK_TIMEOUT", "0"));
|
||||||
|
QRasterWindow window;
|
||||||
|
window.resize(64, 48);
|
||||||
|
window.show();
|
||||||
|
@@ -186,6 +187,7 @@ void tst_xdgshell::configureStates()
|
||||||
|
QCOMPARE(window.windowStates(), Qt::WindowNoState);
|
||||||
|
QCOMPARE(window.frameGeometry().size(), windowedSize);
|
||||||
|
// QCOMPARE(window.frameGeometry().topLeft(), QPoint()); // TODO: this doesn't currently work when window decorations are enabled
|
||||||
|
+ QVERIFY(qunsetenv("QT_WAYLAND_FRAME_CALLBACK_TIMEOUT"));
|
||||||
|
}
|
||||||
|
|
||||||
|
void tst_xdgshell::popup()
|
||||||
|
--
|
||||||
|
2.38.1
|
||||||
|
|
|
@ -1522,7 +1522,9 @@ set of plugins for interacting with pulseaudio and GStreamer.")
|
||||||
(source (origin
|
(source (origin
|
||||||
(method url-fetch)
|
(method url-fetch)
|
||||||
(uri (qt-urls name version))
|
(uri (qt-urls name version))
|
||||||
(patches (search-patches "qtwayland-gcc-11.patch"))
|
(patches (search-patches "qtwayland-gcc-11.patch"
|
||||||
|
"qtwayland-dont-recreate-callbacks.patch"
|
||||||
|
"qtwayland-cleanup-callbacks.patch"))
|
||||||
(sha256
|
(sha256
|
||||||
(base32
|
(base32
|
||||||
"0yy8qf9kn15iqsxi2r7jbcsc0vsdyfz7bbxmfn4i9qmz1yvg0jgr"))))
|
"0yy8qf9kn15iqsxi2r7jbcsc0vsdyfz7bbxmfn4i9qmz1yvg0jgr"))))
|
||||||
|
|
Reference in New Issue