Compositor xdg-decoration: Fix crash when clients disconnect

If clients were disconnected without shutting down nicely, e.g. if Qt clients
were terminated with SIGINT, then QWaylandXdgToplevelDecorationV1 would be
deleted before its Wayland resource, causing a crash in the resource destroy
handler.

This switches to a raw pointer in the toplevel, which is now cleared when the
decoration is deleted. Deletion order is also ensured by conditionally destroying
the resource in the toplevel destructor.

Warnings are also printed when clients destroy objects in the wrong order
(protocol errors).

Change-Id: I8c24becb82e298e93cd45d70a004d8d23ead382b
Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>

1 files changed, 13 insertions, 0 deletions

diff --git a/src/compositor/extensions/qwaylandxdgshell.cpp b/src/compositor/extensions/qwaylandxdgshell.cpp
index eaa443e87..ce6add3de 100644
--- a/src/compositor/extensions/qwaylandxdgshell.cpp
+++ b/src/compositor/extensions/qwaylandxdgshell.cpp
@@ -731,6 +731,16 @@ QWaylandXdgToplevel::QWaylandXdgToplevel(QWaylandXdgSurface *xdgSurface, QWaylan
     sendConfigure({0, 0}, states);
 }
 
+QWaylandXdgToplevel::~QWaylandXdgToplevel()
+{
+    Q_D(QWaylandXdgToplevel);
+    // Usually, the decoration is destroyed by the client (according to the protocol),
+    // but if the client misbehaves, or is shut down, we need to clean up here.
+    if (Q_UNLIKELY(d->m_decoration))
+        wl_resource_destroy(d->m_decoration->resource()->handle);
+    Q_ASSERT(!d->m_decoration);
+}
+
 /*!
  * \qmlproperty XdgToplevel QtWaylandCompositor::XdgToplevel::parentToplevel
  *
@@ -1331,6 +1341,9 @@ void QWaylandXdgToplevelPrivate::xdg_toplevel_destroy_resource(QtWaylandServer::
 
 void QWaylandXdgToplevelPrivate::xdg_toplevel_destroy(QtWaylandServer::xdg_toplevel::Resource *resource)
 {
+    if (Q_UNLIKELY(m_decoration))
+        qWarning() << "Client error: xdg_toplevel destroyed before its decoration object";
+
     wl_resource_destroy(resource->handle);
     //TODO: Should the xdg surface be desroyed as well? Or is it allowed to recreate a new toplevel for it?
 }