Commit 0d52ce8f authored by Martin Flöser's avatar Martin Flöser
Browse files

[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
parent 6d3e7691
......@@ -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"
......@@ -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();
}
......
......@@ -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);
}
}
}
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment