internal: Wayland Protocol impl improvements (#2944)

3 files changed, 87 insertions, 30 deletions

diff --git a/src/protocols/WaylandProtocol.cpp b/src/protocols/WaylandProtocol.cpp
index b5acffea..1ddf6f4b 100644
--- a/src/protocols/WaylandProtocol.cpp
+++ b/src/protocols/WaylandProtocol.cpp
@@ -1,7 +1,12 @@
 #include "WaylandProtocol.hpp"
 #include "../Compositor.hpp"
 
-CWaylandResource::CWaylandResource(wl_client* client, const wl_interface* wlInterface, uint32_t version, uint32_t id, bool destroyInDestructor) {
+static void resourceDestroyNotify(wl_listener* listener, void* data) {
+    CWaylandResource* pResource = wl_container_of(listener, pResource, m_liResourceDestroy);
+    pResource->markDefunct();
+}
+
+CWaylandResource::CWaylandResource(wl_client* client, const wl_interface* wlInterface, uint32_t version, uint32_t id) {
     m_pWLResource = wl_resource_create(client, wlInterface, version, id);
 
     if (!m_pWLResource) {
@@ -9,21 +14,42 @@ CWaylandResource::CWaylandResource(wl_client* client, const wl_interface* wlInte
         return;
     }
 
-    m_pWLClient            = client;
-    m_bDestroyInDestructor = destroyInDestructor;
+    wl_resource_set_user_data(m_pWLResource, this);
+
+    m_pWLClient = client;
 
-    Debug::log(LOG, "[wl res %lx] created", m_pWLResource);
+    wl_list_init(&m_liResourceDestroy.link);
+    m_liResourceDestroy.notify = resourceDestroyNotify;
+    wl_resource_add_destroy_listener(m_pWLResource, &m_liResourceDestroy);
+
+    Debug::log(TRACE, "[wl res %lx] created", m_pWLResource);
+}
+
+void CWaylandResource::markDefunct() {
+    if (m_bDefunct)
+        return;
+
+    Debug::log(TRACE, "[wl res %lx] now defunct", m_pWLResource);
+    m_bDefunct = true;
 }
 
 CWaylandResource::~CWaylandResource() {
-    if (m_pWLResource && m_bDestroyInDestructor)
-        wl_resource_destroy(m_pWLResource);
+    const bool DESTROY = m_pWLResource && !m_bDefunct;
 
-    Debug::log(LOG, "[wl res %lx] destroyed (wl_resource_destroy %s)", m_pWLResource, (m_pWLResource && m_bDestroyInDestructor ? "sent" : "not sent"));
+    wl_list_remove(&m_liResourceDestroy.link);
+    wl_list_init(&m_liResourceDestroy.link);
+
+    if (m_pWLResource)
+        wl_resource_set_user_data(m_pWLResource, nullptr);
+
+    Debug::log(TRACE, "[wl res %lx] destroying (wl_resource_destroy will be %s)", m_pWLResource, (DESTROY ? "sent" : "not sent"));
+
+    if (DESTROY)
+        wl_resource_destroy(m_pWLResource);
 }
 
 bool CWaylandResource::good() {
-    return resource();
+    return m_pWLResource && !m_bDefunct;
 }
 
 wl_resource* CWaylandResource::resource() {
@@ -31,19 +57,31 @@ wl_resource* CWaylandResource::resource() {
 }
 
 uint32_t CWaylandResource::version() {
+    RASSERT(good(), "Attempted to call version() on a bad resource");
+
     return wl_resource_get_version(m_pWLResource);
 }
 
-void CWaylandResource::setImplementation(const void* impl, void* data, wl_resource_destroy_func_t df) {
+void CWaylandResource::setImplementation(const void* impl, wl_resource_destroy_func_t df) {
+    RASSERT(good(), "Attempted to call setImplementation() on a bad resource");
     RASSERT(!m_bImplementationSet, "Wayland Resource %lx already has an implementation, cannot re-set!", m_pWLResource);
 
-    wl_resource_set_implementation(m_pWLResource, impl, data, df);
+    wl_resource_set_implementation(m_pWLResource, impl, this, df);
 
-    Debug::log(LOG, "[wl res %lx] set impl to %lx", m_pWLResource, impl);
+    Debug::log(TRACE, "[wl res %lx] set impl to %lx", m_pWLResource, impl);
 
     m_bImplementationSet = true;
 }
 
+void CWaylandResource::setData(void* data) {
+    Debug::log(TRACE, "[wl res %lx] set data to %lx", m_pWLResource, data);
+    m_pData = data;
+}
+
+void* CWaylandResource::data() {
+    return m_pData;
+}
+
 static void bindManagerInternal(wl_client* client, void* data, uint32_t ver, uint32_t id) {
     ((IWaylandProtocol*)data)->bindManager(client, data, ver, id);
 }
diff --git a/src/protocols/WaylandProtocol.hpp b/src/protocols/WaylandProtocol.hpp
index 904a3053..a87bd519 100644
--- a/src/protocols/WaylandProtocol.hpp
+++ b/src/protocols/WaylandProtocol.hpp
@@ -2,22 +2,34 @@
 
 #include "../defines.hpp"
 
+#define RESOURCE_OR_BAIL(resname)                                                                                                                                                  \
+    const auto resname = (CWaylandResource*)wl_resource_get_user_data(resource);                                                                                                   \
+    if (!resname)                                                                                                                                                                  \
+        return;
+
 class CWaylandResource {
   public:
-    CWaylandResource(wl_client* client, const wl_interface* wlInterface, uint32_t version, uint32_t id, bool destroyInDestructor = false);
+    CWaylandResource(wl_client* client, const wl_interface* wlInterface, uint32_t version, uint32_t id);
     ~CWaylandResource();
 
     bool         good();
     wl_resource* resource();
     uint32_t     version();
 
-    void         setImplementation(const void* impl, void* data, wl_resource_destroy_func_t df);
+    void         setImplementation(const void* impl, wl_resource_destroy_func_t df);
+
+    wl_listener  m_liResourceDestroy; // private but has to be public
+    void         markDefunct();
+
+    void*        data();
+    void         setData(void* data);
 
   private:
-    bool         m_bDestroyInDestructor = false;
-    bool         m_bImplementationSet   = false;
-    wl_client*   m_pWLClient            = nullptr;
-    wl_resource* m_pWLResource          = nullptr;
+    bool         m_bImplementationSet = false;
+    bool         m_bDefunct           = false; // m_liResourceDestroy fired
+    wl_client*   m_pWLClient          = nullptr;
+    wl_resource* m_pWLResource        = nullptr;
+    void*        m_pData              = nullptr;
 };
 
 class IWaylandProtocol {
diff --git a/src/protocols/XDGOutput.cpp b/src/protocols/XDGOutput.cpp
index 0d3aa9b1..8d9bbd84 100644
--- a/src/protocols/XDGOutput.cpp
+++ b/src/protocols/XDGOutput.cpp
@@ -8,21 +8,18 @@
 #define OUTPUT_DESCRIPTION_MUTABLE_SINCE_VERSION 3
 
 static void destroyManagerResource(wl_client* client, wl_resource* resource) {
-    ((CXDGOutputProtocol*)wl_resource_get_user_data(resource))->onManagerResourceDestroy(resource);
-    wl_resource_destroy(resource);
+    RESOURCE_OR_BAIL(PRESOURCE);
+    reinterpret_cast<CXDGOutputProtocol*>(PRESOURCE->data())->onManagerResourceDestroy(resource);
 }
 
 static void destroyOutputResource(wl_client* client, wl_resource* resource) {
-    ((CXDGOutputProtocol*)wl_resource_get_user_data(resource))->onOutputResourceDestroy(resource);
-    wl_resource_destroy(resource);
-}
-
-static void destroyOutputResourceOnly(wl_resource* resource) {
-    ((CXDGOutputProtocol*)wl_resource_get_user_data(resource))->onOutputResourceDestroy(resource);
+    RESOURCE_OR_BAIL(PRESOURCE);
+    reinterpret_cast<CXDGOutputProtocol*>(PRESOURCE->data())->onOutputResourceDestroy(resource);
 }
 
 static void getXDGOutput(wl_client* client, wl_resource* resource, uint32_t id, wl_resource* outputResource) {
-    ((CXDGOutputProtocol*)wl_resource_get_user_data(resource))->onManagerGetXDGOutput(client, resource, id, outputResource);
+    RESOURCE_OR_BAIL(PRESOURCE);
+    reinterpret_cast<CXDGOutputProtocol*>(PRESOURCE->data())->onManagerGetXDGOutput(client, resource, id, outputResource);
 }
 
 //
@@ -41,18 +38,23 @@ void CXDGOutputProtocol::onManagerResourceDestroy(wl_resource* res) {
 }
 
 void CXDGOutputProtocol::onOutputResourceDestroy(wl_resource* res) {
-    std::erase_if(m_vXDGOutputs, [&](const auto& other) { return !other->resource || other->resource->resource() == res; });
+    std::erase_if(m_vXDGOutputs, [&](const auto& other) {
+        if (!other->resource)
+            return false; // ???
+        return other->resource->resource() == res;
+    });
 }
 
 void CXDGOutputProtocol::bindManager(wl_client* client, void* data, uint32_t ver, uint32_t id) {
-    const auto RESOURCE = m_vManagerResources.emplace_back(std::make_unique<CWaylandResource>(client, &zxdg_output_manager_v1_interface, ver, id, false)).get();
+    const auto RESOURCE = m_vManagerResources.emplace_back(std::make_unique<CWaylandResource>(client, &zxdg_output_manager_v1_interface, ver, id)).get();
 
     if (!RESOURCE->good()) {
         Debug::log(LOG, "Couldn't bind XDGOutputMgr");
         return;
     }
 
-    RESOURCE->setImplementation(&MANAGER_IMPL, this, nullptr);
+    RESOURCE->setImplementation(&MANAGER_IMPL, nullptr);
+    RESOURCE->setData(this);
 }
 
 CXDGOutputProtocol::CXDGOutputProtocol(const wl_interface* iface, const int& ver, const std::string& name) : IWaylandProtocol(iface, ver, name) {
@@ -86,10 +88,12 @@ void CXDGOutputProtocol::onManagerGetXDGOutput(wl_client* client, wl_resource* r
 
     if (!pXDGOutput->resource->good()) {
         pXDGOutput->resource.release();
+        m_vXDGOutputs.pop_back();
         return;
     }
 
-    pXDGOutput->resource->setImplementation(&OUTPUT_IMPL, this, destroyOutputResourceOnly);
+    pXDGOutput->resource->setImplementation(&OUTPUT_IMPL, nullptr);
+    pXDGOutput->resource->setData(this);
     const auto XDGVER = pXDGOutput->resource->version();
 
     if (XDGVER >= ZXDG_OUTPUT_V1_NAME_SINCE_VERSION)
@@ -106,6 +110,9 @@ void CXDGOutputProtocol::onManagerGetXDGOutput(wl_client* client, wl_resource* r
 
 void CXDGOutputProtocol::updateOutputDetails(SXDGOutput* pOutput) {
     static auto* const PXWLFORCESCALEZERO = &g_pConfigManager->getConfigValuePtr("xwayland:force_zero_scaling")->intValue;
+  
+    if (!pOutput->resource->good())
+        return;
 
     const auto         POS = pOutput->isXWayland ? pOutput->monitor->vecXWaylandPosition : pOutput->monitor->vecPosition;
     zxdg_output_v1_send_logical_position(pOutput->resource->resource(), POS.x, POS.y);