From 0d52ce8f9b20db5b49ba2b5b6f63e9ebed7743d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= Date: Sun, 4 Feb 2018 16:57:23 +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 Subscribers: plasma-devel Tags: #plasma, #frameworks Differential Revision: https://phabricator.kde.org/D10300 --- .../client/test_wayland_subsurface.cpp | 47 +++++++++++++++++++ src/wayland/subcompositor_interface.cpp | 3 ++ src/wayland/surface_interface.cpp | 7 ++- 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/wayland/autotests/client/test_wayland_subsurface.cpp b/src/wayland/autotests/client/test_wayland_subsurface.cpp index 42c326a549..dc53a1b3e8 100644 --- a/src/wayland/autotests/client/test_wayland_subsurface.cpp +++ b/src/wayland/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 parent(m_compositor->createSurface()); + QVERIFY(serverSurfaceCreated.wait()); + SurfaceInterface *serverParentSurface = serverSurfaceCreated.last().first().value(); + QScopedPointer child(m_compositor->createSurface()); + QVERIFY(serverSurfaceCreated.wait()); + SurfaceInterface *serverChildSurface = serverSurfaceCreated.last().first().value(); + QScopedPointer grandChild(m_compositor->createSurface()); + QVERIFY(serverSurfaceCreated.wait()); + SurfaceInterface *serverGrandChildSurface = serverSurfaceCreated.last().first().value(); + // 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/wayland/subcompositor_interface.cpp b/src/wayland/subcompositor_interface.cpp index f829ab21c4..f2b6de9b46 100644 --- a/src/wayland/subcompositor_interface.cpp +++ b/src/wayland/subcompositor_interface.cpp @@ -360,6 +360,9 @@ bool SubSurfaceInterface::isSynchronized() const QPointer SubSurfaceInterface::mainSurface() const { Q_D(); + if (!d->parent) { + return QPointer(); + } if (d->parent->d_func()->subSurface) { return d->parent->d_func()->subSurface->mainSurface(); } diff --git a/src/wayland/surface_interface.cpp b/src/wayland/surface_interface.cpp index 5102271429..3787794c26 100644 --- a/src/wayland/surface_interface.cpp +++ b/src/wayland/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); + } } } }