File 0001-server-Don-t-crash-when-a-subsurface-gets-... of Package kwayland

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-server-Don-t-crash-when-a-subsurface-gets-commited-w.patch of Package kwayland (Revision 4fc07e232681236a7a094626975ed3e8)

Currently displaying revision 4fc07e232681236a7a094626975ed3e8 , Show latest

From abfd8c2f896bf38c703755952bb3c955ff83a8f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= <mgraesslin@kde.org>
Date: Tue, 6 Feb 2018 16:57:59 +0100
Subject: [PATCH] [server] Don't crash when a subsurface gets commited whose
 parent surface got destroyed

Summary:
Qt seems to damage and commit child subsurfaces although their parent
got destroyed. This actually doesn't make any sense as without a parent
surface they cannot be shown. But nevertheless we should not crash in
such a situation.

This change guards the places in the commit handling code where the
parent gets accessed.

BUG: 389231

Test Plan: New test case which exposes the problem

Reviewers: #frameworks, #kwin, #plasma, davidedmundson

Reviewed By: #plasma, davidedmundson

Subscribers: plasma-devel

Tags: #frameworks, #plasma_on_wayland

Differential Revision: https://phabricator.kde.org/D10300
---
 autotests/client/test_wayland_subsurface.cpp | 47 ++++++++++++++++++++++++++++
 src/server/subcompositor_interface.cpp       |  3 ++
 src/server/surface_interface.cpp             |  7 +++--
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/autotests/client/test_wayland_subsurface.cpp b/autotests/client/test_wayland_subsurface.cpp
index 42c326a..dc53a1b 100644
--- a/autotests/client/test_wayland_subsurface.cpp
+++ b/autotests/client/test_wayland_subsurface.cpp
@@ -59,6 +59,7 @@ private Q_SLOTS:
     void testMappingOfSurfaceTree();
     void testSurfaceAt();
     void testDestroyAttachedBuffer();
+    void testDestroyParentSurface();
 
 private:
     KWayland::Server::Display *m_display;
@@ -1041,5 +1042,51 @@ void TestSubSurface::testDestroyAttachedBuffer()
     QVERIFY(destroySpy.wait());
 }
 
+void TestSubSurface::testDestroyParentSurface()
+{
+    // this test verifies that destroying a parent surface does not create problems
+    // see BUG 389231
+    using namespace KWayland::Client;
+    using namespace KWayland::Server;
+    // create surface
+    QSignalSpy serverSurfaceCreated(m_compositorInterface, &CompositorInterface::surfaceCreated);
+    QVERIFY(serverSurfaceCreated.isValid());
+    QScopedPointer<Surface> parent(m_compositor->createSurface());
+    QVERIFY(serverSurfaceCreated.wait());
+    SurfaceInterface *serverParentSurface = serverSurfaceCreated.last().first().value<KWayland::Server::SurfaceInterface*>();
+    QScopedPointer<Surface> child(m_compositor->createSurface());
+    QVERIFY(serverSurfaceCreated.wait());
+    SurfaceInterface *serverChildSurface = serverSurfaceCreated.last().first().value<KWayland::Server::SurfaceInterface*>();
+    QScopedPointer<Surface> grandChild(m_compositor->createSurface());
+    QVERIFY(serverSurfaceCreated.wait());
+    SurfaceInterface *serverGrandChildSurface = serverSurfaceCreated.last().first().value<KWayland::Server::SurfaceInterface*>();
+    // create sub-surface in desynchronized mode as Qt uses them
+    auto sub1 = m_subCompositor->createSubSurface(child.data(), parent.data());
+    sub1->setMode(SubSurface::Mode::Desynchronized);
+    auto sub2 = m_subCompositor->createSubSurface(grandChild.data(), child.data());
+    sub2->setMode(SubSurface::Mode::Desynchronized);
+
+    // let's damage this surface
+    // and at the same time delete the parent surface
+    parent.reset();
+    QSignalSpy parentDestroyedSpy(serverParentSurface, &QObject::destroyed);
+    QVERIFY(parentDestroyedSpy.isValid());
+    QVERIFY(parentDestroyedSpy.wait());
+    QImage image(QSize(100, 100), QImage::Format_ARGB32_Premultiplied);
+    image.fill(Qt::red);
+    grandChild->attachBuffer(m_shm->createBuffer(image));
+    grandChild->damage(QRect(0, 0, 100, 100));
+    grandChild->commit(Surface::CommitFlag::None);
+    QSignalSpy damagedSpy(serverGrandChildSurface, &SurfaceInterface::damaged);
+    QVERIFY(damagedSpy.isValid());
+    QVERIFY(damagedSpy.wait());
+
+    // Let's try to destroy it
+    QSignalSpy destroySpy(serverChildSurface, &QObject::destroyed);
+    QVERIFY(destroySpy.isValid());
+    child.reset();
+    QVERIFY(destroySpy.wait());
+}
+
 QTEST_GUILESS_MAIN(TestSubSurface)
 #include "test_wayland_subsurface.moc"
diff --git a/src/server/subcompositor_interface.cpp b/src/server/subcompositor_interface.cpp
index f829ab2..f2b6de9 100644
--- a/src/server/subcompositor_interface.cpp
+++ b/src/server/subcompositor_interface.cpp
@@ -360,6 +360,9 @@ bool SubSurfaceInterface::isSynchronized() const
 QPointer<SurfaceInterface> SubSurfaceInterface::mainSurface() const
 {
     Q_D();
+    if (!d->parent) {
+        return QPointer<SurfaceInterface>();
+    }
     if (d->parent->d_func()->subSurface) {
         return d->parent->d_func()->subSurface->mainSurface();
     }
diff --git a/src/server/surface_interface.cpp b/src/server/surface_interface.cpp
index 5102271..3787794 100644
--- a/src/server/surface_interface.cpp
+++ b/src/server/surface_interface.cpp
@@ -453,8 +453,11 @@ void SurfaceInterface::Private::swapStates(State *source, State *target, bool em
                     emit q->damaged(target->damage);
                     // workaround for https://bugreports.qt.io/browse/QTBUG-52092
                     // if the surface is a sub-surface, but the main surface is not yet mapped, fake frame rendered
-                    if (subSurface && !subSurface->mainSurface()->buffer()) {
-                        q->frameRendered(0);
+                    if (subSurface) {
+                        const auto mainSurface = subSurface->mainSurface();
+                        if (!mainSurface || !mainSurface->buffer()) {
+                            q->frameRendered(0);
+                        }
                     }
                 }
             }
-- 
2.16.1

Places

File 0001-server-Don-t-crash-when-a-subsurface-gets-commited-w.patch of Package kwayland (Revision 4fc07e232681236a7a094626975ed3e8)

Places