branch master updated: gnu: qtwayland: Fix crashes from excessive number of frame callbacks.

[guix-commits]

This is an automated email from the git hooks/post-receive script.

abcdw pushed a commit to branch master
in repository guix.

The following commit(s) were added to refs/heads/master by this push:
     new a43c524252 gnu: qtwayland: Fix crashes from excessive number of frame 
callbacks.
a43c524252 is described below

commit a43c5242522f19bca67a6762916f236004d569df
Author: Andrew Tropin <andrew@trop.in>
AuthorDate: Wed Jan 18 14:16:31 2023 +0400

    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.
---
 gnu/local.mk                                       |  2 +
 .../patches/qtwayland-cleanup-callbacks.patch      | 52 +++++++++++++++
 .../qtwayland-dont-recreate-callbacks.patch        | 76 ++++++++++++++++++++++
 gnu/packages/qt.scm                                |  4 +-
 4 files changed, 133 insertions(+), 1 deletion(-)

diff --git a/gnu/local.mk b/gnu/local.mk
index dfe0d92a2f..408f7c376b 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1775,6 +1775,8 @@ dist_patch_DATA =                                         
\
   %D%/packages/patches/quagga-reproducible-build.patch          \
   %D%/packages/patches/quickswitch-fix-dmenu-check.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-fix-building-with-bison-3.7.patch \
   %D%/packages/patches/qtwebkit-fix-building-with-python-3.9.patch     \
diff --git a/gnu/packages/patches/qtwayland-cleanup-callbacks.patch 
b/gnu/packages/patches/qtwayland-cleanup-callbacks.patch
new file mode 100644
index 0000000000..b7618432cb
--- /dev/null
+++ b/gnu/packages/patches/qtwayland-cleanup-callbacks.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
+
diff --git a/gnu/packages/patches/qtwayland-dont-recreate-callbacks.patch 
b/gnu/packages/patches/qtwayland-dont-recreate-callbacks.patch
new file mode 100644
index 0000000000..dda2b99844
--- /dev/null
+++ b/gnu/packages/patches/qtwayland-dont-recreate-callbacks.patch
@@ -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
+
diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
index 6406fd5c49..14fc73ef28 100644
--- a/gnu/packages/qt.scm
+++ b/gnu/packages/qt.scm
@@ -1522,7 +1522,9 @@ set of plugins for interacting with pulseaudio and 
GStreamer.")
     (source (origin
              (method url-fetch)
              (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
               (base32
                "0yy8qf9kn15iqsxi2r7jbcsc0vsdyfz7bbxmfn4i9qmz1yvg0jgr"))))