Fix delayed global removal

Unfortunately, we cannot just simply unset the wl_global's user data.
The compositor still needs to process client requests after the global
has been removed, for example bind requests or the requests that create
new resources.

CCBUG: 435258

src/wayland/blur_interface.cpp

@@ -10,8 +10,6 @@
 #include "display.h"
 #include "surface_interface_p.h"
 
-#include <wayland-server.h>
-
 #include "qwayland-server-blur.h"
 
 namespace KWaylandServer
@@ -27,6 +25,7 @@ public:
     BlurManagerInterface *q;
 
 protected:
+    void org_kde_kwin_blur_manager_destroy_global() override;
     void org_kde_kwin_blur_manager_create(Resource *resource, uint32_t id, wl_resource *surface) override;
     void org_kde_kwin_blur_manager_unset(Resource *resource, wl_resource *surface) override;
 };
@@ -37,6 +36,11 @@ BlurManagerInterfacePrivate::BlurManagerInterfacePrivate(BlurManagerInterface *_
 {
 }
 
+void BlurManagerInterfacePrivate::org_kde_kwin_blur_manager_destroy_global()
+{
+    delete q;
+}
+
 void BlurManagerInterfacePrivate::org_kde_kwin_blur_manager_unset(Resource *resource, wl_resource *surface)
 {
     Q_UNUSED(resource);
@@ -73,6 +77,10 @@ BlurManagerInterface::BlurManagerInterface(Display *display, QObject *parent)
 }
 
 BlurManagerInterface::~BlurManagerInterface()
+{
+}
+
+void BlurManagerInterface::remove()
 {
     d->globalRemove();
 }

src/wayland/blur_interface.h

@@ -35,6 +35,8 @@ public:
     explicit BlurManagerInterface(Display *display, QObject *parent = nullptr);
     ~BlurManagerInterface() override;
 
+    void remove();
+
 private:
     QScopedPointer<BlurManagerInterfacePrivate> d;
 };

src/wayland/contrast_interface.cpp

@@ -9,7 +9,6 @@
 #include "display.h"
 #include "surface_interface_p.h"
 
-#include <wayland-server.h>
 #include <qwayland-server-contrast.h>
 
 namespace KWaylandServer
@@ -20,16 +19,25 @@ static const quint32 s_version = 1;
 class ContrastManagerInterfacePrivate : public QtWaylandServer::org_kde_kwin_contrast_manager
 {
 public:
-    ContrastManagerInterfacePrivate(Display *display);
+    ContrastManagerInterfacePrivate(ContrastManagerInterface *q, Display *display);
+
+    ContrastManagerInterface *q;
 
 protected:
+    void org_kde_kwin_contrast_manager_destroy_global() override;
     void org_kde_kwin_contrast_manager_create(Resource *resource, uint32_t id, wl_resource *surface) override;
     void org_kde_kwin_contrast_manager_unset(Resource *resource, wl_resource *surface) override;
 };
 
-ContrastManagerInterfacePrivate::ContrastManagerInterfacePrivate(Display *display)
+ContrastManagerInterfacePrivate::ContrastManagerInterfacePrivate(ContrastManagerInterface *q, Display *display)
     : QtWaylandServer::org_kde_kwin_contrast_manager(*display, s_version)
+    , q(q)
+{
+}
+
+void ContrastManagerInterfacePrivate::org_kde_kwin_contrast_manager_destroy_global()
 {
+    delete q;
 }
 
 void ContrastManagerInterfacePrivate::org_kde_kwin_contrast_manager_create(Resource *resource, uint32_t id, wl_resource *surface)
@@ -63,11 +71,15 @@ void ContrastManagerInterfacePrivate::org_kde_kwin_contrast_manager_unset(Resour
 
 ContrastManagerInterface::ContrastManagerInterface(Display *display, QObject *parent)
     : QObject(parent)
-    , d(new ContrastManagerInterfacePrivate(display))
+    , d(new ContrastManagerInterfacePrivate(this, display))
 {
 }
 
 ContrastManagerInterface::~ContrastManagerInterface()
+{
+}
+
+void ContrastManagerInterface::remove()
 {
     d->globalRemove();
 }

src/wayland/contrast_interface.h

@@ -35,6 +35,8 @@ public:
     explicit ContrastManagerInterface(Display *display, QObject *parent = nullptr);
     ~ContrastManagerInterface() override;
 
+    void remove();
+
 private:
     QScopedPointer<ContrastManagerInterfacePrivate> d;
 };

src/wayland/output_interface.cpp

@@ -47,6 +47,7 @@ public:
     } dpms;
 
 private:
+    void output_destroy_global() override;
     void output_bind_resource(Resource *resource);
     void output_release(Resource *resource);
 };
@@ -138,6 +139,11 @@ void OutputInterfacePrivate::broadcastGeometry()
     }
 }
 
+void OutputInterfacePrivate::output_destroy_global()
+{
+    delete q;
+}
+
 void OutputInterfacePrivate::output_release(Resource *resource)
 {
     wl_resource_destroy(resource->handle);
@@ -145,6 +151,10 @@ void OutputInterfacePrivate::output_release(Resource *resource)
 
 void OutputInterfacePrivate::output_bind_resource(Resource *resource)
 {
+    if (isGlobalRemoved()) {
+        return; // We are waiting for the wl_output global to be destroyed.
+    }
+
     sendMode(resource);
     sendScale(resource);
     sendGeometry(resource);
@@ -163,7 +173,14 @@ OutputInterface::OutputInterface(Display *display, QObject *parent)
 
 OutputInterface::~OutputInterface()
 {
-    emit aboutToBeDestroyed();
+    remove();
+}
+
+void OutputInterface::remove()
+{
+    if (d->isGlobalRemoved()) {
+        return;
+    }
 
     if (d->display) {
         DisplayPrivate *displayPrivate = DisplayPrivate::get(d->display);
@@ -171,6 +188,7 @@ OutputInterface::~OutputInterface()
     }
 
     d->globalRemove();
+    emit removed();
 }
 
 QSize OutputInterface::pixelSize() const

src/wayland/output_interface.h

@@ -68,6 +68,8 @@ public:
     explicit OutputInterface(Display *display, QObject *parent = nullptr);
     ~OutputInterface() override;
 
+    void remove();
+
     QSize physicalSize() const;
     QPoint globalPosition() const;
     QString manufacturer() const;
@@ -120,7 +122,6 @@ public:
     static OutputInterface *get(wl_resource *native);
 
 Q_SIGNALS:
-    void aboutToBeDestroyed();
     void physicalSizeChanged(const QSize&);
     void globalPositionChanged(const QPoint&);
     void manufacturerChanged(const QString&);
@@ -133,6 +134,7 @@ Q_SIGNALS:
     void modeChanged();
     void dpmsModeChanged();
     void dpmsSupportedChanged();
+    void removed();
 
     /**
      * Change of dpms @p mode is requested.

src/wayland/server/outputdevice_interface.cpp

@@ -23,7 +23,6 @@ class OutputDeviceInterfacePrivate : public QtWaylandServer::org_kde_kwin_output
 {
 public:
     OutputDeviceInterfacePrivate(OutputDeviceInterface *q, Display *display);
-    ~OutputDeviceInterfacePrivate() override;
 
     void updateGeometry();
     void updateUuid();
@@ -65,14 +64,12 @@ public:
     OutputDeviceInterface::ColorCurves colorCurves;
     QList<OutputDeviceInterface::Mode> modes;
     OutputDeviceInterface::Mode currentMode;
-
     QByteArray edid;
     OutputDeviceInterface::Enablement enabled = OutputDeviceInterface::Enablement::Enabled;
     QUuid uuid;
     OutputDeviceInterface::Capabilities capabilities;
     uint32_t overscan = 0;
     OutputDeviceInterface::VrrPolicy vrrPolicy = OutputDeviceInterface::VrrPolicy::Automatic;
-
     QPointer<Display> display;
     OutputDeviceInterface *q;
 
@@ -81,24 +78,20 @@ private:
     int32_t toSubPixel() const;
 
 protected:
+    void org_kde_kwin_outputdevice_destroy_global() override;
     void org_kde_kwin_outputdevice_bind_resource(Resource *resource) override;
 };
 
+void OutputDeviceInterfacePrivate::org_kde_kwin_outputdevice_destroy_global()
+{
+    delete q;
+}
+
 OutputDeviceInterfacePrivate::OutputDeviceInterfacePrivate(OutputDeviceInterface *q, Display *display)
     : QtWaylandServer::org_kde_kwin_outputdevice(*display, s_version)
     , display(display)
     , q(q)
 {
-    DisplayPrivate *displayPrivate = DisplayPrivate::get(display);
-    displayPrivate->outputdevices.append(q);
-}
-
-OutputDeviceInterfacePrivate::~OutputDeviceInterfacePrivate()
-{
-    if (display) {
-        DisplayPrivate *displayPrivate = DisplayPrivate::get(display);
-        displayPrivate->outputdevices.removeOne(q);
-    }
 }
 
 OutputDeviceInterface::OutputDeviceInterface(Display *display, QObject *parent)
@@ -122,10 +115,27 @@ OutputDeviceInterface::OutputDeviceInterface(Display *display, QObject *parent)
     connect(this, &OutputDeviceInterface::manufacturerChanged,   this, [this] { d->updateGeometry(); });
     connect(this, &OutputDeviceInterface::scaleFChanged,         this, [this] { d->updateScale(); });
     connect(this, &OutputDeviceInterface::colorCurvesChanged,    this, [this] { d->updateColorCurves(); });
+
+    DisplayPrivate *displayPrivate = DisplayPrivate::get(display);
+    displayPrivate->outputdevices.append(this);
 }
 
 OutputDeviceInterface::~OutputDeviceInterface()
 {
+    remove();
+}
+
+void OutputDeviceInterface::remove()
+{
+    if (d->isGlobalRemoved()) {
+        return;
+    }
+
+    if (d->display) {
+        DisplayPrivate *displayPrivate = DisplayPrivate::get(d->display);
+        displayPrivate->outputdevices.removeOne(this);
+    }
+
     d->globalRemove();
 }
 

src/wayland/server/outputdevice_interface.h

@@ -105,6 +105,8 @@ public:
     explicit OutputDeviceInterface(Display *display, QObject *parent = nullptr);
     ~OutputDeviceInterface() override;
 
+    void remove();
+
     QSize physicalSize() const;
     QPoint globalPosition() const;
     QString manufacturer() const;

src/wayland/slide_interface.cpp

@@ -7,7 +7,6 @@
 #include "display.h"
 #include "surface_interface_p.h"
 
-#include <wayland-server.h>
 #include <qwayland-server-slide.h>
 
 namespace KWaylandServer
@@ -20,14 +19,19 @@ class SlideManagerInterfacePrivate : public QtWaylandServer::org_kde_kwin_slide_
 public:
     SlideManagerInterfacePrivate(SlideManagerInterface *_q, Display *display);
 
-private:
     SlideManagerInterface *q;
 
 protected:
+    void org_kde_kwin_slide_manager_destroy_global() override;
     void org_kde_kwin_slide_manager_create(Resource *resource, uint32_t id, wl_resource *surface) override;
     void org_kde_kwin_slide_manager_unset(Resource *resource, wl_resource *surface) override;
 };
 
+void SlideManagerInterfacePrivate::org_kde_kwin_slide_manager_destroy_global()
+{
+    delete q;
+}
+
 void SlideManagerInterfacePrivate::org_kde_kwin_slide_manager_create(Resource *resource, uint32_t id, wl_resource *surface)
 {
     SurfaceInterface *s = SurfaceInterface::get(surface);
@@ -43,7 +47,6 @@ void SlideManagerInterfacePrivate::org_kde_kwin_slide_manager_create(Resource *r
     }
 
     auto slide = new SlideInterface(q, slide_resource);
-    
     SurfaceInterfacePrivate *surfacePrivate = SurfaceInterfacePrivate::get(s);
     surfacePrivate->setSlide(QPointer<SlideInterface>(slide));
 }
@@ -72,6 +75,10 @@ SlideManagerInterface::SlideManagerInterface(Display *display, QObject *parent)
 }
 
 SlideManagerInterface::~SlideManagerInterface()
+{
+}
+
+void SlideManagerInterface::remove()
 {
     d->globalRemove();
 }

src/wayland/slide_interface.h

@@ -26,6 +26,8 @@ public:
     explicit SlideManagerInterface(Display *display, QObject *parent = nullptr);
     ~SlideManagerInterface() override;
 
+    void remove();
+
 private:
     QScopedPointer<SlideManagerInterfacePrivate> d;
 };

src/wayland/surface_interface.cpp

@@ -827,7 +827,7 @@ void SurfaceInterface::setOutputs(const QVector<OutputInterface *> &outputs)
         for (wl_resource *outputResource : resources) {
             d->send_enter(outputResource);
         }
-        d->outputDestroyedConnections[o] = connect(o, &OutputInterface::aboutToBeDestroyed, this, [this, o] {
+        d->outputDestroyedConnections[o] = connect(o, &OutputInterface::removed, this, [this, o] {
             auto outputs = d->outputs;
             if (outputs.removeOne(o)) {
                 setOutputs(outputs);

src/wayland/tools/qtwaylandscanner.cpp

@@ -569,6 +569,7 @@ bool Scanner::process()
             printf("        QMultiMap<struct ::wl_client*, Resource*> resourceMap() { return m_resource_map; }\n");
             printf("        const QMultiMap<struct ::wl_client*, Resource*> resourceMap() const { return m_resource_map; }\n");
             printf("\n");
+            printf("        bool isGlobalRemoved() const { return m_globalRemovedEvent; }\n");
             printf("        void globalRemove();\n");
             printf("\n");
             printf("        bool isGlobal() const { return m_global != nullptr; }\n");
@@ -599,6 +600,8 @@ bool Scanner::process()
             printf("    protected:\n");
             printf("        virtual Resource *%s_allocate();\n", interfaceNameStripped);
             printf("\n");
+            printf("        virtual void %s_destroy_global();\n", interfaceNameStripped);
+            printf("\n");
             printf("        virtual void %s_bind_resource(Resource *resource);\n", interfaceNameStripped);
             printf("        virtual void %s_destroy_resource(Resource *resource);\n", interfaceNameStripped);
 
@@ -618,6 +621,7 @@ bool Scanner::process()
             printf("        static void bind_func(struct ::wl_client *client, void *data, uint32_t version, uint32_t id);\n");
             printf("        static void destroy_func(struct ::wl_resource *client_resource);\n");
             printf("        static void display_destroy_func(struct ::wl_listener *listener, void *data);\n");
+            printf("        static int deferred_destroy_global_func(void *data);\n");
             printf("\n");
             printf("        Resource *bind(struct ::wl_client *client, uint32_t id, int version);\n");
             printf("        Resource *bind(struct ::wl_resource *handle);\n");
@@ -640,6 +644,7 @@ bool Scanner::process()
             printf("        Resource *m_resource;\n");
             printf("        struct ::wl_global *m_global;\n");
             printf("        struct ::wl_display *m_display;\n");
+            printf("        struct wl_event_source *m_globalRemovedEvent;\n");
             printf("        uint32_t m_globalVersion;\n");
             printf("        struct DisplayDestroyedListener : ::wl_listener {\n");
             printf("            %s *parent;\n", interfaceName);
@@ -670,21 +675,6 @@ bool Scanner::process()
 
         bool needsNewLine = false;
 
-        printf("\n");
-        printf("    struct deferred_destroy_global_data {\n");
-        printf("        struct wl_global *global;\n");
-        printf("        struct wl_event_source *event_source;\n");
-        printf("    };\n");
-        printf("\n");
-        printf("    static int deferred_destroy_global_func(void *_data) {\n");
-        printf("        deferred_destroy_global_data *data = static_cast<deferred_destroy_global_data *>(_data);\n");
-        printf("        wl_global_destroy(data->global);\n");
-        printf("        wl_event_source_remove(data->event_source);\n");
-        printf("        delete data;\n");
-        printf("        return 0;\n");
-        printf("    }\n");
-        printf("\n");
-
         for (const WaylandInterface &interface : interfaces) {
 
             if (ignoreInterface(interface.name))
@@ -700,11 +690,24 @@ bool Scanner::process()
             QByteArray stripped = stripInterfaceName(interface.name);
             const char *interfaceNameStripped = stripped.constData();
 
+            printf("\n");
+            printf("    int %s::deferred_destroy_global_func(void *data) {\n", interfaceName);
+            printf("        auto object = static_cast<%s *>(data);\n", interfaceName);
+            printf("        wl_global_destroy(object->m_global);\n");
+            printf("        object->m_global = nullptr;\n");
+            printf("        wl_event_source_remove(object->m_globalRemovedEvent);\n");
+            printf("        object->m_globalRemovedEvent = nullptr;\n");
+            printf("        object->%s_destroy_global();\n", interfaceNameStripped);
+            printf("        return 0;\n");
+            printf("    }\n");
+            printf("\n");
+
             printf("    %s::%s(struct ::wl_client *client, int id, int version)\n", interfaceName, interfaceName);
             printf("        : m_resource_map()\n");
             printf("        , m_resource(nullptr)\n");
             printf("        , m_global(nullptr)\n");
             printf("        , m_display(nullptr)\n");
+            printf("        , m_globalRemovedEvent(nullptr)\n");
             printf("    {\n");
             printf("        init(client, id, version);\n");
             printf("    }\n");
@@ -715,6 +718,7 @@ bool Scanner::process()
             printf("        , m_resource(nullptr)\n");
             printf("        , m_global(nullptr)\n");
             printf("        , m_display(nullptr)\n");
+            printf("        , m_globalRemovedEvent(nullptr)\n");
             printf("    {\n");
             printf("        init(display, version);\n");
             printf("    }\n");
@@ -725,6 +729,7 @@ bool Scanner::process()
             printf("        , m_resource(nullptr)\n");
             printf("        , m_global(nullptr)\n");
             printf("        , m_display(nullptr)\n");
+            printf("        , m_globalRemovedEvent(nullptr)\n");
             printf("    {\n");
             printf("        init(resource);\n");
             printf("    }\n");
@@ -734,6 +739,8 @@ bool Scanner::process()
             printf("        : m_resource_map()\n");
             printf("        , m_resource(nullptr)\n");
             printf("        , m_global(nullptr)\n");
+            printf("        , m_display(nullptr)\n");
+            printf("        , m_globalRemovedEvent(nullptr)\n");
             printf("    {\n");
             printf("    }\n");
             printf("\n");
@@ -747,6 +754,8 @@ bool Scanner::process()
             printf("            m_resource->%s_object = nullptr;\n", interfaceNameStripped);
             printf("\n");
             printf("        if (m_global) {\n");
+            printf("            if (m_globalRemovedEvent)\n");
+            printf("                wl_event_source_remove(m_globalRemovedEvent);\n");
             printf("            wl_global_destroy(m_global);\n");
             printf("            wl_list_remove(&m_displayDestroyedListener.link);\n");
             printf("        }\n");
@@ -804,6 +813,11 @@ bool Scanner::process()
             printf("    }\n");
             printf("\n");
 
+            printf("    void %s::%s_destroy_global()\n", interfaceName, interfaceNameStripped);
+            printf("    {\n");
+            printf("    }\n");
+            printf("\n");
+
             printf("    void %s::%s_bind_resource(Resource *)\n", interfaceName, interfaceNameStripped);
             printf("    {\n");
             printf("    }\n");
@@ -826,6 +840,7 @@ bool Scanner::process()
             printf("        Q_UNUSED(data);\n");
             printf("        %s *that = static_cast<%s::DisplayDestroyedListener *>(listener)->parent;\n", interfaceName, interfaceName);
             printf("        that->m_global = nullptr;\n");
+            printf("        that->m_globalRemovedEvent = nullptr;\n");
             printf("    }\n");
             printf("\n");
 
@@ -851,19 +866,14 @@ bool Scanner::process()
             printf("\n");
             printf("    void %s::globalRemove()\n", interfaceName);
             printf("    {\n");
-            printf("        if (!m_global)\n");
+            printf("        if (!m_global || m_globalRemovedEvent)\n");
             printf("            return;\n");
             printf("\n");
             printf("        wl_global_remove(m_global);\n");
-            printf("        wl_global_set_user_data(m_global, nullptr);\n");
             printf("\n");
             printf("        struct wl_event_loop *event_loop = wl_display_get_event_loop(m_display);\n");
-            printf("        struct deferred_destroy_global_data *data = new deferred_destroy_global_data;\n");
-            printf("        data->global = m_global;\n");
-            printf("        data->event_source = wl_event_loop_add_timer(event_loop, deferred_destroy_global_func, data);\n");
-            printf("        wl_event_source_timer_update(data->event_source, 5000);\n");
-            printf("        m_global = nullptr;\n");
-            printf("        wl_list_remove(&m_displayDestroyedListener.link);\n");
+            printf("        m_globalRemovedEvent = wl_event_loop_add_timer(event_loop, deferred_destroy_global_func, this);\n");
+            printf("        wl_event_source_timer_update(m_globalRemovedEvent, 5000);\n");
             printf("    }\n");
             printf("\n");
 

src/wayland/utils.h

@@ -18,6 +18,20 @@ struct wl_resource;
 namespace KWaylandServer
 {
 
+template <typename T>
+struct SafeGlobalDeleter
+{
+    static inline void cleanup(T *global)
+    {
+        if (global) {
+            global->remove();
+        }
+    }
+};
+
+template <typename T>
+using ScopedGlobalPointer = QScopedPointer<T, SafeGlobalDeleter<T>>;
+
 /**
  * Returns an infinite region.
  */