From 06906e0d1163ed84bbdf5b9d67509e19e0fbfa01 Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Thu, 22 Apr 2021 08:42:33 +0200 Subject: [PATCH] client: Gracefully handle shutdown and window hiding When a window is hidden or destroyed, the render thread may already be rendering. We need to properly read-lock the surface pointer when it is in use and exit when it becomes null. Note that there is also a potential crash in the Mesa GL driver where it keeps a proxy to the wl_surface, so if we delete this while we are still rendering, it can crash inside the driver. This is not addressed by this patch, and has not been reproduced on any other drivers so far. [ChangeLog][Client] Fixed a crash that could happen when hiding or closing windows while Qt Quick was actively rendering on a different thread. Pick-to: 6.0 6.1 5.15 Fixes: QTBUG-91264 Fixes: QTBUG-90037 Task-number: QTBUG-92249 Change-Id: I029b123b83c58740321e8b90a463ced748d8bcf4 Reviewed-by: Laszlo Agocs Reviewed-by: David Edmundson --- .../platforms/wayland/qwaylandwindow.cpp | 20 +++++++++++++++++-- .../platforms/wayland/qwaylandwindow_p.h | 2 +- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index 65a91497639..62690fc6f9b 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -429,6 +429,7 @@ void QWaylandWindow::closePopups(QWaylandWindow *parent) QPlatformScreen *QWaylandWindow::calculateScreenFromSurfaceEvents() const { + QReadLocker lock(&mSurfaceLock); if (mSurface) { if (auto *screen = mSurface->oldestEnteredScreen()) return screen; @@ -482,6 +483,7 @@ void QWaylandWindow::setMask(const QRegion &mask) mMask = mask; + QReadLocker locker(&mSurfaceLock); if (!mSurface) return; @@ -567,6 +569,10 @@ void QWaylandWindow::sendRecursiveExposeEvent() void QWaylandWindow::attach(QWaylandBuffer *buffer, int x, int y) { + QReadLocker locker(&mSurfaceLock); + if (mSurface == nullptr) + return; + if (buffer) { Q_ASSERT(!buffer->committed()); handleUpdate(); @@ -586,6 +592,10 @@ void QWaylandWindow::attachOffset(QWaylandBuffer *buffer) void QWaylandWindow::damage(const QRect &rect) { + QReadLocker locker(&mSurfaceLock); + if (mSurface == nullptr) + return; + const qreal s = scale(); if (mSurface->version() >= 4) mSurface->damage_buffer(qFloor(s * rect.x()), qFloor(s * rect.y()), qCeil(s * rect.width()), qCeil(s * rect.height())); @@ -620,6 +630,8 @@ void QWaylandWindow::commit(QWaylandBuffer *buffer, const QRegion &damage) qCDebug(lcWaylandBackingstore) << "Buffer already committed, ignoring."; return; } + + QReadLocker locker(&mSurfaceLock); if (!mSurface) return; @@ -639,7 +651,9 @@ void QWaylandWindow::commit(QWaylandBuffer *buffer, const QRegion &damage) void QWaylandWindow::commit() { - mSurface->commit(); + QReadLocker locker(&mSurfaceLock); + if (mSurface != nullptr) + mSurface->commit(); } const wl_callback_listener QWaylandWindow::callbackListener = { @@ -736,6 +750,7 @@ QPointF QWaylandWindow::mapFromWlSurface(const QPointF &surfacePosition) const wl_surface *QWaylandWindow::wlSurface() { + QReadLocker locker(&mSurfaceLock); return mSurface ? mSurface->object() : nullptr; } @@ -760,7 +775,8 @@ QWaylandScreen *QWaylandWindow::waylandScreen() const void QWaylandWindow::handleContentOrientationChange(Qt::ScreenOrientation orientation) { - if (mSurface->version() < 2) + QReadLocker locker(&mSurfaceLock); + if (mSurface == nullptr || mSurface->version() < 2) return; wl_output_transform transform; diff --git a/src/plugins/platforms/wayland/qwaylandwindow_p.h b/src/plugins/platforms/wayland/qwaylandwindow_p.h index 1d743f4e4a3..314294fcfc6 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow_p.h +++ b/src/plugins/platforms/wayland/qwaylandwindow_p.h @@ -304,7 +304,7 @@ private: static QWaylandWindow *mMouseGrab; - QReadWriteLock mSurfaceLock; + mutable QReadWriteLock mSurfaceLock; friend class QWaylandSubSurface; };