Revert "wayland: Terminate client connections before Workspace is destroyed"

Commit 826b9742e9 breaks a lot of other
things. Need further investigation before fixing heap-use-after-free.

autotests/integration/kwin_wayland_test.cpp

@@ -83,23 +83,18 @@ WaylandTestApplication::~WaylandTestApplication()
     if (effects) {
         static_cast<EffectsHandlerImpl*>(effects)->unloadAllEffects();
     }
-
-    // Kill Xwayland before terminating its connection.
-    delete m_xwayland;
-    m_xwayland = nullptr;
-
-    // Terminate all client connections before Workspace is destroyed.
-    // Shell clients need to access RuleBook whose lifetime is bound
-    // to the Workspace class.
-    waylandServer()->terminateClientConnections();
-    waylandServer()->dispatch();
-
+    if (m_xwayland) {
+        // needs to be done before workspace gets destroyed
+        m_xwayland->prepareDestroy();
+    }
     destroyWorkspace();
-
+    waylandServer()->dispatch();
     if (QStyle *s = style()) {
         s->unpolish(this);
     }
-
+    // kill Xwayland before terminating its connection
+    delete m_xwayland;
+    waylandServer()->terminateClientConnections();
     destroyCompositor();
 }
 

main_wayland.cpp

@@ -128,23 +128,20 @@ ApplicationWayland::~ApplicationWayland()
     if (effects) {
         static_cast<EffectsHandlerImpl*>(effects)->unloadAllEffects();
     }
-
-    // Kill Xwayland before terminating its connection.
-    delete m_xwayland;
-    m_xwayland = nullptr;
-
-    // Terminate all client connections before Workspace is destroyed.
-    // Shell clients need to access RuleBook whose lifetime is bound
-    // to the Workspace class.
-    waylandServer()->terminateClientConnections();
-    waylandServer()->dispatch();
-
+    if (m_xwayland) {
+        // needs to be done before workspace gets destroyed
+        m_xwayland->prepareDestroy();
+    }
     destroyWorkspace();
+    waylandServer()->dispatch();
 
     if (QStyle *s = style()) {
         s->unpolish(this);
     }
-
+    // kill Xwayland before terminating its connection
+    delete m_xwayland;
+    m_xwayland = nullptr;
+    waylandServer()->terminateClientConnections();
     destroyCompositor();
 }
 

shell_client.cpp

@@ -425,9 +425,11 @@ void ShellClient::destroyClient()
     if (isMoveResize()) {
         leaveMoveResize();
     }
-
-    Deleted *deleted = Deleted::create(this);
-    emit windowClosed(this, deleted);
+    Deleted *del = nullptr;
+    if (workspace()) {
+        del = Deleted::create(this);
+    }
+    emit windowClosed(this, del);
 
     // Remove Force Temporarily rules.
     RuleBook::self()->discardUsed(this, true);
@@ -435,22 +437,25 @@ void ShellClient::destroyClient()
     destroyWindowManagementInterface();
     destroyDecoration();
 
-    StackingUpdatesBlocker blocker(workspace());
-    if (transientFor()) {
-        transientFor()->removeTransient(this);
-    }
-    for (auto it = transients().constBegin(); it != transients().constEnd();) {
-        if ((*it)->transientFor() == this) {
-            removeTransient(*it);
-            it = transients().constBegin(); // restart, just in case something more has changed with the list
-        } else {
-            ++it;
+    if (workspace()) {
+        StackingUpdatesBlocker blocker(workspace());
+        if (transientFor()) {
+            transientFor()->removeTransient(this);
+        }
+        for (auto it = transients().constBegin(); it != transients().constEnd();) {
+            if ((*it)->transientFor() == this) {
+                removeTransient(*it);
+                it = transients().constBegin(); // restart, just in case something more has changed with the list
+            } else {
+                ++it;
+            }
         }
     }
-
     waylandServer()->removeClient(this);
 
-    deleted->unrefWindow();
+    if (del) {
+        del->unrefWindow();
+    }
     m_shellSurface = nullptr;
     m_xdgShellSurface = nullptr;
     m_xdgShellPopup = nullptr;

xwl/xwayland.cpp

@@ -88,9 +88,6 @@ Xwayland::Xwayland(ApplicationWaylandAbstract *app, QObject *parent)
 
 Xwayland::~Xwayland()
 {
-    delete m_dataBridge;
-    m_dataBridge = nullptr;
-
     disconnect(m_xwaylandFailConnection);
     if (m_app->x11Connection()) {
         Xcb::setInputFocus(XCB_INPUT_FOCUS_POINTER_ROOT);
@@ -181,6 +178,12 @@ void Xwayland::init()
     close(pipeFds[1]);
 }
 
+void Xwayland::prepareDestroy()
+{
+    delete m_dataBridge;
+    m_dataBridge = nullptr;
+}
+
 void Xwayland::createX11Connection()
 {
     int screenNumber = 0;

xwl/xwayland.h

@@ -47,6 +47,7 @@ public:
     ~Xwayland() override;
 
     void init();
+    void prepareDestroy();
 
     xcb_screen_t *xcbScreen() const {
         return m_xcbScreen;