From 2093820aba4bbb26130b2b6d788b73584a5a6ebd Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Wed, 14 Oct 2020 18:04:57 +0300
Subject: [PATCH] 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
---
 autotests/integration/xwaylandserver_crash_test.cpp   |  7 +++++++
 autotests/integration/xwaylandserver_restart_test.cpp |  7 +++++++
 main.h                                                | 11 +++++++++++
 workspace.cpp                                         |  2 +-
 xwl/xwayland.cpp                                      |  4 ++++
 5 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/autotests/integration/xwaylandserver_crash_test.cpp b/autotests/integration/xwaylandserver_crash_test.cpp
index 8739b66c4e..77b4160773 100644
--- a/autotests/integration/xwaylandserver_crash_test.cpp
+++ b/autotests/integration/xwaylandserver_crash_test.cpp
@@ -5,8 +5,10 @@
 */
 
 #include "kwin_wayland_test.h"
+#include "composite.h"
 #include "main.h"
 #include "platform.h"
+#include "scene.h"
 #include "screens.h"
 #include "unmanaged.h"
 #include "wayland_server.h"
@@ -124,6 +126,11 @@ void XwaylandServerCrashTest::testCrash()
     QCOMPARE(kwinApp()->x11DefaultScreen(), nullptr);
     QCOMPARE(kwinApp()->x11RootWindow(), XCB_WINDOW_NONE);
     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
diff --git a/autotests/integration/xwaylandserver_restart_test.cpp b/autotests/integration/xwaylandserver_restart_test.cpp
index 83895fd33e..d268320307 100644
--- a/autotests/integration/xwaylandserver_restart_test.cpp
+++ b/autotests/integration/xwaylandserver_restart_test.cpp
@@ -5,8 +5,10 @@
 */
 
 #include "kwin_wayland_test.h"
+#include "composite.h"
 #include "main.h"
 #include "platform.h"
+#include "scene.h"
 #include "screens.h"
 #include "wayland_server.h"
 #include "workspace.h"
@@ -103,6 +105,11 @@ void XwaylandServerRestartTest::testRestart()
     QCOMPARE(client->windowId(), window);
     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.
     xcb_destroy_window(c.data(), window);
     xcb_flush(c.data());
diff --git a/main.h b/main.h
index 43ffb6a909..4f4409fa56 100644
--- a/main.h
+++ b/main.h
@@ -159,6 +159,13 @@ public:
         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
     bool usesKActivities() const {
         return m_useKActivities;
@@ -234,6 +241,9 @@ protected:
     void setTerminating() {
         m_terminating = true;
     }
+    void setClosingX11Connection(bool set) {
+        m_isClosingX11Connection = set;
+    }
 
 protected:
     static int crashes;
@@ -253,6 +263,7 @@ private:
 #endif
     Platform *m_platform = nullptr;
     bool m_terminating = false;
+    bool m_isClosingX11Connection = false;
 };
 
 inline static Application *kwinApp()
diff --git a/workspace.cpp b/workspace.cpp
index 0ac1483038..ebce832d1c 100644
--- a/workspace.cpp
+++ b/workspace.cpp
@@ -1788,7 +1788,7 @@ bool Workspace::compositing() const
 void Workspace::markXStackingOrderAsDirty()
 {
     m_xStackingDirty = true;
-    if (kwinApp()->x11Connection()) {
+    if (kwinApp()->x11Connection() && !kwinApp()->isClosingX11Connection()) {
         m_xStackingQueryTree.reset(new Xcb::Tree(kwinApp()->x11RootWindow()));
     }
 }
diff --git a/xwl/xwayland.cpp b/xwl/xwayland.cpp
index bc6a69f5af..5bb08b1620 100644
--- a/xwl/xwayland.cpp
+++ b/xwl/xwayland.cpp
@@ -149,6 +149,8 @@ void Xwayland::stop()
         return;
     }
 
+    m_app->setClosingX11Connection(true);
+
     // 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...
     uninstallSocketNotifier();
@@ -169,6 +171,8 @@ void Xwayland::stop()
     m_xwaylandProcess = nullptr;
 
     waylandServer()->destroyXWaylandConnection(); // This one must be destroyed last!
+
+    m_app->setClosingX11Connection(false);
 }
 
 void Xwayland::restart()