From 0df4406c2cf8df56f90a7a006eb911775a120886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Mon, 11 Apr 2016 09:42:22 +0200 Subject: [PATCH] Fix crash on repainting an invalid sizes decoration Summary: If a window has an invalid size the decoration also has an invalid size. This results in the texture used by the SceneOpenGLDecorationRenderer to be invalid and being reset to null. Of course we shouldn't try to use this texture to render to. The change comes with a test case to simulate the situation. We cannot simulate it with Wayland clients as the geometry can never be empty. Thus we create an X11 client, resize it to an empty size and unmap it. This is the first integration test case which creates an X11 Client! It's also a test case which needs the OpenGL compositor. This will most likely not work on build.kde.org yet - we need to see what to do about it. Will need adjustments to get it at least skip on build.kde.org. BUG: 361551 FIXED-IN: 5.6.3 Reviewers: #plasma Subscribers: plasma-devel Projects: #plasma Differential Revision: https://phabricator.kde.org/D1383 --- autotests/wayland/CMakeLists.txt | 9 + autotests/wayland/dont_crash_empty_deco.cpp | 221 ++++++++++++++++++++ client.h | 2 +- scene_opengl.cpp | 5 + 4 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 autotests/wayland/dont_crash_empty_deco.cpp diff --git a/autotests/wayland/CMakeLists.txt b/autotests/wayland/CMakeLists.txt index d46f6dff88..9c6d5a4536 100644 --- a/autotests/wayland/CMakeLists.txt +++ b/autotests/wayland/CMakeLists.txt @@ -125,3 +125,12 @@ add_executable(testTransientPlacmenet ${testTransientPlacmenet_SRCS}) target_link_libraries( testTransientPlacmenet kwin Qt5::Test) add_test(kwin-testTransientPlacmenet testTransientPlacmenet) ecm_mark_as_test(testTransientPlacmenet) + +######################################################## +# Dont Crash Empty Deco Test +######################################################## +set( testDontCrashEmptyDeco_SRCS dont_crash_empty_deco.cpp kwin_wayland_test.cpp ) +add_executable(testDontCrashEmptyDeco ${testDontCrashEmptyDeco_SRCS}) +target_link_libraries( testDontCrashEmptyDeco kwin Qt5::Test) +add_test(kwin-testDontCrashEmptyDeco testDontCrashEmptyDeco) +ecm_mark_as_test(testDontCrashEmptyDeco) diff --git a/autotests/wayland/dont_crash_empty_deco.cpp b/autotests/wayland/dont_crash_empty_deco.cpp new file mode 100644 index 0000000000..1b74bf8785 --- /dev/null +++ b/autotests/wayland/dont_crash_empty_deco.cpp @@ -0,0 +1,221 @@ +/******************************************************************** +KWin - the KDE window manager +This file is part of the KDE project. + +Copyright (C) 2016 Martin Gräßlin + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*********************************************************************/ +#include "kwin_wayland_test.h" +#include "abstract_backend.h" +#include "client.h" +#include "cursor.h" +#include "screenedge.h" +#include "screens.h" +#include "wayland_server.h" +#include "workspace.h" +#include "shell_client.h" +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include + +namespace KWin +{ + +static const QString s_socketName = QStringLiteral("wayland_test_kwin_dont_crash_empty_decoration-0"); + +class DontCrashEmptyDecorationTest : public QObject +{ + Q_OBJECT +private Q_SLOTS: + void initTestCase(); + void init(); + void cleanup(); + void testBug361551(); + +private: + KWayland::Client::ConnectionThread *m_connection = nullptr; + KWayland::Client::Compositor *m_compositor = nullptr; + KWayland::Client::ServerSideDecorationManager *m_deco = nullptr; + KWayland::Client::Seat *m_seat = nullptr; + KWayland::Client::ShmPool *m_shm = nullptr; + KWayland::Client::Shell *m_shell = nullptr; + KWayland::Client::EventQueue *m_queue = nullptr; + QThread *m_thread = nullptr; +}; + +void DontCrashEmptyDecorationTest::initTestCase() +{ + qRegisterMetaType(); + qRegisterMetaType(); + QSignalSpy workspaceCreatedSpy(kwinApp(), &Application::workspaceCreated); + QVERIFY(workspaceCreatedSpy.isValid()); + waylandServer()->backend()->setInitialWindowSize(QSize(1280, 1024)); + QMetaObject::invokeMethod(waylandServer()->backend(), "setOutputCount", Qt::DirectConnection, Q_ARG(int, 2)); + waylandServer()->init(s_socketName.toLocal8Bit()); + + // this test needs to enforce OpenGL compositing to get into the crashy condition + qputenv("KWIN_COMPOSE", QByteArrayLiteral("O2")); + kwinApp()->start(); + QVERIFY(workspaceCreatedSpy.wait()); + QCOMPARE(screens()->count(), 2); + QCOMPARE(screens()->geometry(0), QRect(0, 0, 1280, 1024)); + QCOMPARE(screens()->geometry(1), QRect(1280, 0, 1280, 1024)); + setenv("QT_QPA_PLATFORM", "wayland", true); + waylandServer()->initWorkspace(); +} + +void DontCrashEmptyDecorationTest::init() +{ + using namespace KWayland::Client; + // setup connection + m_connection = new ConnectionThread; + QSignalSpy connectedSpy(m_connection, &ConnectionThread::connected); + QVERIFY(connectedSpy.isValid()); + m_connection->setSocketName(s_socketName); + + m_thread = new QThread(this); + m_connection->moveToThread(m_thread); + m_thread->start(); + + m_connection->initConnection(); + QVERIFY(connectedSpy.wait()); + + m_queue = new EventQueue(this); + QVERIFY(!m_queue->isValid()); + m_queue->setup(m_connection); + QVERIFY(m_queue->isValid()); + + Registry registry; + registry.setEventQueue(m_queue); + QSignalSpy compositorSpy(®istry, &Registry::compositorAnnounced); + QSignalSpy shmSpy(®istry, &Registry::shmAnnounced); + QSignalSpy shellSpy(®istry, &Registry::shellAnnounced); + QSignalSpy seatSpy(®istry, &Registry::seatAnnounced); + QSignalSpy decorationSpy(®istry, &Registry::serverSideDecorationManagerAnnounced); + QSignalSpy allAnnounced(®istry, &Registry::interfacesAnnounced); + QVERIFY(allAnnounced.isValid()); + QVERIFY(shmSpy.isValid()); + QVERIFY(shellSpy.isValid()); + QVERIFY(compositorSpy.isValid()); + QVERIFY(seatSpy.isValid()); + QVERIFY(decorationSpy.isValid()); + registry.create(m_connection->display()); + QVERIFY(registry.isValid()); + registry.setup(); + QVERIFY(allAnnounced.wait()); + QVERIFY(!compositorSpy.isEmpty()); + QVERIFY(!shmSpy.isEmpty()); + QVERIFY(!shellSpy.isEmpty()); + QVERIFY(!seatSpy.isEmpty()); + QVERIFY(!decorationSpy.isEmpty()); + + m_compositor = registry.createCompositor(compositorSpy.first().first().value(), compositorSpy.first().last().value(), this); + QVERIFY(m_compositor->isValid()); + m_shm = registry.createShmPool(shmSpy.first().first().value(), shmSpy.first().last().value(), this); + QVERIFY(m_shm->isValid()); + m_shell = registry.createShell(shellSpy.first().first().value(), shellSpy.first().last().value(), this); + QVERIFY(m_shell->isValid()); + m_seat = registry.createSeat(seatSpy.first().first().value(), seatSpy.first().last().value(), this); + QVERIFY(m_seat->isValid()); + m_deco = registry.createServerSideDecorationManager(decorationSpy.first().first().value(), decorationSpy.first().last().value()); + QVERIFY(m_deco->isValid()); + QSignalSpy hasPointerSpy(m_seat, &Seat::hasPointerChanged); + QVERIFY(hasPointerSpy.isValid()); + QVERIFY(hasPointerSpy.wait()); + + screens()->setCurrent(0); + Cursor::setPos(QPoint(640, 512)); +} + +void DontCrashEmptyDecorationTest::cleanup() +{ + delete m_compositor; + m_compositor = nullptr; + delete m_deco; + m_deco = nullptr; + delete m_seat; + m_seat = nullptr; + delete m_shm; + m_shm = nullptr; + delete m_shell; + m_shell = nullptr; + delete m_queue; + m_queue = nullptr; + if (m_thread) { + m_connection->deleteLater(); + m_thread->quit(); + m_thread->wait(); + delete m_thread; + m_thread = nullptr; + m_connection = nullptr; + } +} + +void DontCrashEmptyDecorationTest::testBug361551() +{ + // this test verifies that resizing an X11 window to an invalid size does not result in crash on unmap + // when the DecorationRenderer gets copied to the Deleted + // there a repaint is scheduled and the resulting texture is invalid if the window size is invalid + + // create an xcb window + xcb_connection_t *c = xcb_connect(nullptr, nullptr); + QVERIFY(!xcb_connection_has_error(c)); + + xcb_window_t w = xcb_generate_id(c); + xcb_create_window(c, XCB_COPY_FROM_PARENT, w, rootWindow(), 0, 0, 10, 10, 0, XCB_WINDOW_CLASS_INPUT_OUTPUT, XCB_COPY_FROM_PARENT, 0, nullptr); + xcb_map_window(c, w); + xcb_flush(c); + + // we should get a client for it + QSignalSpy windowCreatedSpy(workspace(), &Workspace::clientAdded); + QVERIFY(windowCreatedSpy.isValid()); + QVERIFY(windowCreatedSpy.wait()); + Client *client = windowCreatedSpy.first().first().value(); + QVERIFY(client); + QCOMPARE(client->window(), w); + QVERIFY(client->isDecorated()); + + // let's set a stupid geometry + client->setGeometry(0, 0, 0, 0); + QCOMPARE(client->geometry(), QRect(0, 0, 0, 0)); + + // and destroy the window again + xcb_unmap_window(c, w); + xcb_destroy_window(c, w); + xcb_flush(c); + xcb_disconnect(c); + + QSignalSpy windowClosedSpy(client, &Client::windowClosed); + QVERIFY(windowClosedSpy.isValid()); + QVERIFY(windowClosedSpy.wait()); +} + +} + +WAYLANDTEST_MAIN(KWin::DontCrashEmptyDecorationTest) +#include "dont_crash_empty_deco.moc" diff --git a/client.h b/client.h index 697b6418bf..7a28b547ba 100644 --- a/client.h +++ b/client.h @@ -61,7 +61,7 @@ enum class Predicate { InputIdMatch }; -class Client +class KWIN_EXPORT Client : public AbstractClient { Q_OBJECT diff --git a/scene_opengl.cpp b/scene_opengl.cpp index c81c2b8905..cb43b4819d 100644 --- a/scene_opengl.cpp +++ b/scene_opengl.cpp @@ -2451,6 +2451,11 @@ void SceneOpenGLDecorationRenderer::render() resetImageSizesDirty(); } + if (!m_texture) { + // for invalid sizes we get no texture, see BUG 361551 + return; + } + QRect left, top, right, bottom; client()->client()->layoutDecorationRects(left, top, right, bottom);