xwayland: Avoid creating a tree query on crash

If Xwayland has crashed, the Workspace will block stacking order updates
and start destroying all X11 clients.

Once stacking order updates are unblocked, the Workspace will mark the X
stacking order as dirty and create a new Xcb::Tree object.

We don't want to create that Xcb::Tree object because accessing it
after the XCB connection has been shut down will lead to a crash.

BUG: 427688
FIXED-IN: 5.20.1
This commit is contained in:
Vlad Zahorodnii 2020-10-14 18:04:57 +03:00
parent 37ffba8b0f
commit 2093820aba
5 changed files with 30 additions and 1 deletions

View file

@ -5,8 +5,10 @@
*/ */
#include "kwin_wayland_test.h" #include "kwin_wayland_test.h"
#include "composite.h"
#include "main.h" #include "main.h"
#include "platform.h" #include "platform.h"
#include "scene.h"
#include "screens.h" #include "screens.h"
#include "unmanaged.h" #include "unmanaged.h"
#include "wayland_server.h" #include "wayland_server.h"
@ -124,6 +126,11 @@ void XwaylandServerCrashTest::testCrash()
QCOMPARE(kwinApp()->x11DefaultScreen(), nullptr); QCOMPARE(kwinApp()->x11DefaultScreen(), nullptr);
QCOMPARE(kwinApp()->x11RootWindow(), XCB_WINDOW_NONE); QCOMPARE(kwinApp()->x11RootWindow(), XCB_WINDOW_NONE);
QCOMPARE(kwinApp()->x11ScreenNumber(), -1); QCOMPARE(kwinApp()->x11ScreenNumber(), -1);
// Render a frame to ensure that the compositor doesn't crash.
Compositor::self()->addRepaintFull();
QSignalSpy frameRenderedSpy(Compositor::self()->scene(), &Scene::frameRendered);
QVERIFY(frameRenderedSpy.wait());
} }
} // namespace KWin } // namespace KWin

View file

@ -5,8 +5,10 @@
*/ */
#include "kwin_wayland_test.h" #include "kwin_wayland_test.h"
#include "composite.h"
#include "main.h" #include "main.h"
#include "platform.h" #include "platform.h"
#include "scene.h"
#include "screens.h" #include "screens.h"
#include "wayland_server.h" #include "wayland_server.h"
#include "workspace.h" #include "workspace.h"
@ -103,6 +105,11 @@ void XwaylandServerRestartTest::testRestart()
QCOMPARE(client->windowId(), window); QCOMPARE(client->windowId(), window);
QVERIFY(client->isDecorated()); QVERIFY(client->isDecorated());
// Render a frame to ensure that the compositor doesn't crash.
Compositor::self()->addRepaintFull();
QSignalSpy frameRenderedSpy(Compositor::self()->scene(), &Scene::frameRendered);
QVERIFY(frameRenderedSpy.wait());
// Destroy the test window. // Destroy the test window.
xcb_destroy_window(c.data(), window); xcb_destroy_window(c.data(), window);
xcb_flush(c.data()); xcb_flush(c.data());

11
main.h
View file

@ -159,6 +159,13 @@ public:
return m_defaultScreen; return m_defaultScreen;
} }
/**
* Returns @c true if we're in the middle of destroying the X11 connection.
*/
bool isClosingX11Connection() const {
return m_isClosingX11Connection;
}
#ifdef KWIN_BUILD_ACTIVITIES #ifdef KWIN_BUILD_ACTIVITIES
bool usesKActivities() const { bool usesKActivities() const {
return m_useKActivities; return m_useKActivities;
@ -234,6 +241,9 @@ protected:
void setTerminating() { void setTerminating() {
m_terminating = true; m_terminating = true;
} }
void setClosingX11Connection(bool set) {
m_isClosingX11Connection = set;
}
protected: protected:
static int crashes; static int crashes;
@ -253,6 +263,7 @@ private:
#endif #endif
Platform *m_platform = nullptr; Platform *m_platform = nullptr;
bool m_terminating = false; bool m_terminating = false;
bool m_isClosingX11Connection = false;
}; };
inline static Application *kwinApp() inline static Application *kwinApp()

View file

@ -1788,7 +1788,7 @@ bool Workspace::compositing() const
void Workspace::markXStackingOrderAsDirty() void Workspace::markXStackingOrderAsDirty()
{ {
m_xStackingDirty = true; m_xStackingDirty = true;
if (kwinApp()->x11Connection()) { if (kwinApp()->x11Connection() && !kwinApp()->isClosingX11Connection()) {
m_xStackingQueryTree.reset(new Xcb::Tree(kwinApp()->x11RootWindow())); m_xStackingQueryTree.reset(new Xcb::Tree(kwinApp()->x11RootWindow()));
} }
} }

View file

@ -149,6 +149,8 @@ void Xwayland::stop()
return; return;
} }
m_app->setClosingX11Connection(true);
// If Xwayland has crashed, we must deactivate the socket notifier and ensure that no X11 // If Xwayland has crashed, we must deactivate the socket notifier and ensure that no X11
// events will be dispatched before blocking; otherwise we will simply hang... // events will be dispatched before blocking; otherwise we will simply hang...
uninstallSocketNotifier(); uninstallSocketNotifier();
@ -169,6 +171,8 @@ void Xwayland::stop()
m_xwaylandProcess = nullptr; m_xwaylandProcess = nullptr;
waylandServer()->destroyXWaylandConnection(); // This one must be destroyed last! waylandServer()->destroyXWaylandConnection(); // This one must be destroyed last!
m_app->setClosingX11Connection(false);
} }
void Xwayland::restart() void Xwayland::restart()