From 16a07d5df2ee7433694a18be4383fe5912025fbb Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Tue, 21 Mar 2023 15:03:25 +0200
Subject: [PATCH] wayland: Handle xdg_wm_base being destroyed before surface
 role

If a client owns several windows (for example it can be the case with
plasmashell) and it crashes, it's possible to encounter the following
case:

- xdg_wm_base resources are destroyed
- xdg_toplevel is destroyed
- another xdg_toplevel is destroyed

When kwin processes the destruction of the first xdg_toplevel, it may
ping the second xdg_toplevel. But the xdg_wm_base is already free()d by
that time, so kwin can access already released memory.

In order to prevent that, make the associated XdgSurfaceInterface
objects inert. Since XdgToplevelInterface and XdgPopupInterface will
become useless after destroying XdgSurfaceInterface, make them inert too.

As the spec states, it's illegal to destroy a bound xdg_wm_base object
while there are still alive xdg surfaces created by it so destroying the
surface role objects should be fine.
---
 src/wayland/xdgshell_interface.cpp | 17 +++++++++++++----
 src/wayland/xdgshell_interface_p.h |  1 +
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/wayland/xdgshell_interface.cpp b/src/wayland/xdgshell_interface.cpp
index 50847d9730..f1d74b8104 100644
--- a/src/wayland/xdgshell_interface.cpp
+++ b/src/wayland/xdgshell_interface.cpp
@@ -59,6 +59,12 @@ XdgShellInterfacePrivate *XdgShellInterfacePrivate::get(XdgShellInterface *shell
     return shell->d.get();
 }
 
+void XdgShellInterfacePrivate::xdg_wm_base_destroy_resource(Resource *resource)
+{
+    const QList<XdgSurfaceInterface *> surfaces = xdgSurfaces.keys(resource);
+    qDeleteAll(surfaces);
+}
+
 void XdgShellInterfacePrivate::xdg_wm_base_destroy(Resource *resource)
 {
     if (xdgSurfaces.key(resource)) {
@@ -168,8 +174,6 @@ XdgSurfaceInterfacePrivate *XdgSurfaceInterfacePrivate::get(XdgSurfaceInterface
 
 void XdgSurfaceInterfacePrivate::xdg_surface_destroy_resource(Resource *resource)
 {
-    Q_EMIT q->aboutToBeDestroyed();
-    XdgShellInterfacePrivate::get(shell)->unregisterXdgSurface(q);
     delete q;
 }
 
@@ -256,6 +260,11 @@ XdgSurfaceInterface::XdgSurfaceInterface(XdgShellInterface *shell, SurfaceInterf
 
 XdgSurfaceInterface::~XdgSurfaceInterface()
 {
+    delete d->toplevel;
+    delete d->popup;
+
+    Q_EMIT aboutToBeDestroyed();
+    XdgShellInterfacePrivate::get(d->shell)->unregisterXdgSurface(this);
 }
 
 XdgToplevelInterface *XdgSurfaceInterface::toplevel() const
@@ -342,7 +351,6 @@ void XdgToplevelInterfacePrivate::reset()
 
 void XdgToplevelInterfacePrivate::xdg_toplevel_destroy_resource(Resource *resource)
 {
-    Q_EMIT q->aboutToBeDestroyed();
     delete q;
 }
 
@@ -480,6 +488,7 @@ XdgToplevelInterface::XdgToplevelInterface(XdgSurfaceInterface *surface, ::wl_re
 
 XdgToplevelInterface::~XdgToplevelInterface()
 {
+    Q_EMIT aboutToBeDestroyed();
 }
 
 XdgShellInterface *XdgToplevelInterface::shell() const
@@ -638,7 +647,6 @@ void XdgPopupInterfacePrivate::reset()
 
 void XdgPopupInterfacePrivate::xdg_popup_destroy_resource(Resource *resource)
 {
-    Q_EMIT q->aboutToBeDestroyed();
     delete q;
 }
 
@@ -676,6 +684,7 @@ XdgPopupInterface::XdgPopupInterface(XdgSurfaceInterface *surface, SurfaceInterf
 
 XdgPopupInterface::~XdgPopupInterface()
 {
+    Q_EMIT aboutToBeDestroyed();
 }
 
 SurfaceInterface *XdgPopupInterface::parentSurface() const
diff --git a/src/wayland/xdgshell_interface_p.h b/src/wayland/xdgshell_interface_p.h
index 8619ac83e6..86f31ef293 100644
--- a/src/wayland/xdgshell_interface_p.h
+++ b/src/wayland/xdgshell_interface_p.h
@@ -34,6 +34,7 @@ public:
     QMap<quint32, QTimer *> pings;
 
 protected:
+    void xdg_wm_base_destroy_resource(Resource *resource) override;
     void xdg_wm_base_destroy(Resource *resource) override;
     void xdg_wm_base_create_positioner(Resource *resource, uint32_t id) override;
     void xdg_wm_base_get_xdg_surface(Resource *resource, uint32_t id, ::wl_resource *surface) override;