From 9c38b0fdbe32dc2cb81d53c9be90113d114f1cd2 Mon Sep 17 00:00:00 2001 From: Tom Englund Date: Mon, 29 Jul 2024 19:19:47 +0200 Subject: [PATCH] core: add a destructor to CHyprOpenglImpl and avoid wl_container_of undefined behaviour (#7101) * protocols: avoid undefined behaviour in C macro to safely use wl_container_of with a class the class has to be no virtual functions, no inheritance, and uniform access control (e.g all public) work around this by putting this into a destroywrapper struct. * opengl: clean memory on destruction add a destructor and free the allocated memory and close the fd --- src/protocols/GlobalShortcuts.cpp | 13 ++++++++----- src/protocols/GlobalShortcuts.hpp | 8 +++++++- src/protocols/WaylandProtocol.cpp | 13 ++++++++----- src/protocols/WaylandProtocol.hpp | 8 +++++++- src/render/OpenGL.cpp | 16 ++++++++++++++++ src/render/OpenGL.hpp | 1 + 6 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/protocols/GlobalShortcuts.cpp b/src/protocols/GlobalShortcuts.cpp index 898f0e07..7eb84be6 100644 --- a/src/protocols/GlobalShortcuts.cpp +++ b/src/protocols/GlobalShortcuts.cpp @@ -8,13 +8,14 @@ static void bindManagerInt(wl_client* client, void* data, uint32_t version, uint } static void handleDisplayDestroy(struct wl_listener* listener, void* data) { - CGlobalShortcutsProtocolManager* proto = wl_container_of(listener, proto, m_liDisplayDestroy); + CGlobalShortcutsProtocolManagerDestroyWrapper* wrap = wl_container_of(listener, wrap, listener); + CGlobalShortcutsProtocolManager* proto = wrap->parent; proto->displayDestroy(); } void CGlobalShortcutsProtocolManager::displayDestroy() { - wl_list_remove(&m_liDisplayDestroy.link); - wl_list_init(&m_liDisplayDestroy.link); + wl_list_remove(&m_liDisplayDestroy.listener.link); + wl_list_init(&m_liDisplayDestroy.listener.link); wl_global_destroy(m_pGlobal); } @@ -30,8 +31,10 @@ CGlobalShortcutsProtocolManager::CGlobalShortcutsProtocolManager() { return; } - m_liDisplayDestroy.notify = handleDisplayDestroy; - wl_display_add_destroy_listener(g_pCompositor->m_sWLDisplay, &m_liDisplayDestroy); + wl_list_init(&m_liDisplayDestroy.listener.link); + m_liDisplayDestroy.listener.notify = handleDisplayDestroy; + m_liDisplayDestroy.parent = this; + wl_display_add_destroy_listener(g_pCompositor->m_sWLDisplay, &m_liDisplayDestroy.listener); Debug::log(LOG, "GlobalShortcutsManager started successfully!"); } diff --git a/src/protocols/GlobalShortcuts.hpp b/src/protocols/GlobalShortcuts.hpp index 5fd03465..7e512021 100644 --- a/src/protocols/GlobalShortcuts.hpp +++ b/src/protocols/GlobalShortcuts.hpp @@ -14,6 +14,12 @@ struct SShortcutClient { std::vector> shortcuts; }; +class CGlobalShortcutsProtocolManager; +struct CGlobalShortcutsProtocolManagerDestroyWrapper { + wl_listener listener; + CGlobalShortcutsProtocolManager* parent = nullptr; +}; + class CGlobalShortcutsProtocolManager { public: CGlobalShortcutsProtocolManager(); @@ -31,7 +37,7 @@ class CGlobalShortcutsProtocolManager { std::vector getAllShortcuts(); - wl_listener m_liDisplayDestroy; + CGlobalShortcutsProtocolManagerDestroyWrapper m_liDisplayDestroy; private: std::vector> m_vClients; diff --git a/src/protocols/WaylandProtocol.cpp b/src/protocols/WaylandProtocol.cpp index 71c43300..954f160d 100644 --- a/src/protocols/WaylandProtocol.cpp +++ b/src/protocols/WaylandProtocol.cpp @@ -6,13 +6,14 @@ static void bindManagerInternal(wl_client* client, void* data, uint32_t ver, uin } static void displayDestroyInternal(struct wl_listener* listener, void* data) { - IWaylandProtocol* proto = wl_container_of(listener, proto, m_liDisplayDestroy); + IWaylandProtocolDestroyWrapper* wrap = wl_container_of(listener, wrap, listener); + IWaylandProtocol* proto = wrap->parent; proto->onDisplayDestroy(); } void IWaylandProtocol::onDisplayDestroy() { - wl_list_remove(&m_liDisplayDestroy.link); - wl_list_init(&m_liDisplayDestroy.link); + wl_list_remove(&m_liDisplayDestroy.listener.link); + wl_list_init(&m_liDisplayDestroy.listener.link); wl_global_destroy(m_pGlobal); } @@ -24,8 +25,10 @@ IWaylandProtocol::IWaylandProtocol(const wl_interface* iface, const int& ver, co return; } - m_liDisplayDestroy.notify = displayDestroyInternal; - wl_display_add_destroy_listener(g_pCompositor->m_sWLDisplay, &m_liDisplayDestroy); + wl_list_init(&m_liDisplayDestroy.listener.link); + m_liDisplayDestroy.listener.notify = displayDestroyInternal; + m_liDisplayDestroy.parent = this; + wl_display_add_destroy_listener(g_pCompositor->m_sWLDisplay, &m_liDisplayDestroy.listener); protoLog(LOG, "Registered global"); } diff --git a/src/protocols/WaylandProtocol.hpp b/src/protocols/WaylandProtocol.hpp index 6154fa30..4d4e7925 100644 --- a/src/protocols/WaylandProtocol.hpp +++ b/src/protocols/WaylandProtocol.hpp @@ -11,6 +11,12 @@ #define PROTO NProtocols +class IWaylandProtocol; +struct IWaylandProtocolDestroyWrapper { + wl_listener listener; + IWaylandProtocol* parent = nullptr; +}; + class IWaylandProtocol { public: IWaylandProtocol(const wl_interface* iface, const int& ver, const std::string& name); @@ -26,7 +32,7 @@ class IWaylandProtocol { Debug::log(level, std::format("[{}] ", m_szName) + std::vformat(fmt.get(), std::make_format_args(args...))); }; - wl_listener m_liDisplayDestroy; + IWaylandProtocolDestroyWrapper m_liDisplayDestroy; private: std::string m_szName; diff --git a/src/render/OpenGL.cpp b/src/render/OpenGL.cpp index 43a2c3a7..b925fcc9 100644 --- a/src/render/OpenGL.cpp +++ b/src/render/OpenGL.cpp @@ -345,6 +345,22 @@ CHyprOpenGLImpl::CHyprOpenGLImpl() { m_tGlobalTimer.reset(); } +CHyprOpenGLImpl::~CHyprOpenGLImpl() { + if (m_pEglDisplay && m_pEglContext != EGL_NO_CONTEXT) + eglDestroyContext(m_pEglDisplay, m_pEglContext); + + if (m_pEglDisplay) + eglTerminate(m_pEglDisplay); + + eglReleaseThread(); + + if (m_pGbmDevice) + gbm_device_destroy(m_pGbmDevice); + + if (m_iGBMFD >= 0) + close(m_iGBMFD); +} + std::optional> CHyprOpenGLImpl::getModsForFormat(EGLint format) { // TODO: return std::expected when clang supports it diff --git a/src/render/OpenGL.hpp b/src/render/OpenGL.hpp index 41e80ee5..1e8325c1 100644 --- a/src/render/OpenGL.hpp +++ b/src/render/OpenGL.hpp @@ -145,6 +145,7 @@ class CGradientValueData; class CHyprOpenGLImpl { public: CHyprOpenGLImpl(); + ~CHyprOpenGLImpl(); void begin(CMonitor*, const CRegion& damage, CFramebuffer* fb = nullptr, std::optional finalDamage = {}); void beginSimple(CMonitor*, const CRegion& damage, SP rb = nullptr, CFramebuffer* fb = nullptr);