core: move workspace ptrs to weak (#11194)

Fixes some race conditions that come up in tests. We only clean up workspaces when we render a frame. With this, they are always cleared instantly.

CMakeLists.txt

@@ -85,13 +85,14 @@ set(CXX_STANDARD_REQUIRED ON)
 add_compile_options(
   -Wall
   -Wextra
+  -Wpedantic
   -Wno-unused-parameter
   -Wno-unused-value
   -Wno-missing-field-initializers
+  -Wno-gnu-zero-variadic-macro-arguments
   -Wno-narrowing
   -Wno-pointer-arith
   -Wno-clobbered
-  -Wpedantic
   -fmacro-prefix-map=${CMAKE_SOURCE_DIR}/=)
 
 set(CMAKE_EXECUTABLE_ENABLE_EXPORTS TRUE)

src/Compositor.cpp

@@ -1338,29 +1338,14 @@ PHLWINDOW CCompositor::getWindowFromHandle(uint32_t handle) {
 }
 
 PHLWORKSPACE CCompositor::getWorkspaceByID(const WORKSPACEID& id) {
-    for (auto const& w : m_workspaces) {
+    for (auto const& w : getWorkspaces()) {
         if (w->m_id == id && !w->inert())
-            return w;
+            return w.lock();
     }
 
     return nullptr;
 }
 
-void CCompositor::sanityCheckWorkspaces() {
-    auto it = m_workspaces.begin();
-    while (it != m_workspaces.end()) {
-        const auto& WORKSPACE = *it;
-
-        // If ref == 1, only the compositor holds a ref, which means it's inactive and has no mapped windows.
-        if (!WORKSPACE->m_persistent && WORKSPACE.strongRef() == 1) {
-            it = m_workspaces.erase(it);
-            continue;
-        }
-
-        ++it;
-    }
-}
-
 PHLWINDOW CCompositor::getUrgentWindow() {
     for (auto const& w : m_windows) {
         if (w->m_isMapped && w->m_isUrgent)
@@ -1732,7 +1717,7 @@ PHLWINDOW CCompositor::getWindowCycle(PHLWINDOW cur, bool focusableOnly, std::op
 
 WORKSPACEID CCompositor::getNextAvailableNamedWorkspace() {
     WORKSPACEID lowest = -1337 + 1;
-    for (auto const& w : m_workspaces) {
+    for (auto const& w : getWorkspaces()) {
         if (w->m_id < -1 && w->m_id < lowest)
             lowest = w->m_id;
     }
@@ -1741,9 +1726,9 @@ WORKSPACEID CCompositor::getNextAvailableNamedWorkspace() {
 }
 
 PHLWORKSPACE CCompositor::getWorkspaceByName(const std::string& name) {
-    for (auto const& w : m_workspaces) {
+    for (auto const& w : getWorkspaces()) {
         if (w->m_name == name && !w->inert())
-            return w;
+            return w.lock();
     }
 
     return nullptr;
@@ -2144,7 +2129,7 @@ void CCompositor::moveWorkspaceToMonitor(PHLWORKSPACE pWorkspace, PHLMONITOR pMo
     if (!SWITCHINGISACTIVE)
         nextWorkspaceOnMonitorID = pWorkspace->m_id;
     else {
-        for (auto const& w : m_workspaces) {
+        for (auto const& w : getWorkspaces()) {
             if (w->m_monitor == POLDMON && w->m_id != pWorkspace->m_id && !w->m_isSpecialWorkspace) {
                 nextWorkspaceOnMonitorID = w->m_id;
                 break;
@@ -2258,7 +2243,7 @@ bool CCompositor::workspaceIDOutOfBounds(const WORKSPACEID& id) {
     WORKSPACEID lowestID  = INT64_MAX;
     WORKSPACEID highestID = INT64_MIN;
 
-    for (auto const& w : m_workspaces) {
+    for (auto const& w : getWorkspaces()) {
         if (w->m_isSpecialWorkspace)
             continue;
         lowestID  = std::min(w->m_id, lowestID);
@@ -2660,7 +2645,7 @@ PHLWORKSPACE CCompositor::createNewWorkspace(const WORKSPACEID& id, const MONITO
         return nullptr;
     }
 
-    const auto PWORKSPACE = m_workspaces.emplace_back(CWorkspace::create(id, PMONITOR, NAME, SPECIAL, isEmpty));
+    const auto PWORKSPACE = CWorkspace::create(id, PMONITOR, NAME, SPECIAL, isEmpty);
 
     PWORKSPACE->m_alpha->setValueAndWarp(0);
 
@@ -2694,15 +2679,19 @@ bool CCompositor::isWorkspaceSpecial(const WORKSPACEID& id) {
 
 WORKSPACEID CCompositor::getNewSpecialID() {
     WORKSPACEID highest = SPECIAL_WORKSPACE_START;
-    for (auto const& ws : m_workspaces) {
+    for (auto const& ws : getWorkspaces()) {
-        if (ws->m_isSpecialWorkspace && ws->m_id > highest) {
+        if (ws->m_isSpecialWorkspace && ws->m_id > highest)
             highest = ws->m_id;
-        }
     }
 
     return highest + 1;
 }
 
+void CCompositor::registerWorkspace(PHLWORKSPACE w) {
+    m_workspaces.emplace_back(w);
+    w->m_events.destroy.listenStatic([this, weak = PHLWORKSPACEREF{w}] { std::erase(m_workspaces, weak); });
+}
+
 void CCompositor::performUserChecks() {
     static auto PNOCHECKXDG     = CConfigValue<Hyprlang::INT>("misc:disable_xdg_env_checks");
     static auto PNOCHECKQTUTILS = CConfigValue<Hyprlang::INT>("misc:disable_hyprland_qtutils_check");
@@ -3176,7 +3165,4 @@ void CCompositor::ensurePersistentWorkspacesPresent(const std::vector<SWorkspace
             continue;
         }
     }
-
-    // cleanup old
-    sanityCheckWorkspaces();
 }

src/Compositor.hpp

@@ -2,6 +2,8 @@
 
 #include <sys/resource.h>
 
+#include <ranges>
+
 #include "managers/XWaylandManager.hpp"
 #include "managers/KeybindManager.hpp"
 #include "managers/SessionLockManager.hpp"
@@ -42,7 +44,6 @@ class CCompositor {
     std::vector<PHLMONITOR>                      m_realMonitors; // for all monitors, even those turned off
     std::vector<PHLWINDOW>                       m_windows;
     std::vector<PHLLS>                           m_layers;
-    std::vector<PHLWORKSPACE>                    m_workspaces;
     std::vector<PHLWINDOWREF>                    m_windowsFadingOut;
     std::vector<PHLLSREF>                        m_surfacesFadingOut;
 
@@ -75,6 +76,13 @@ class CCompositor {
 
     // ------------------------------------------------- //
 
+    auto getWorkspaces() {
+        return std::views::filter(m_workspaces, [](const auto& e) { return e; });
+    }
+    void registerWorkspace(PHLWORKSPACE w);
+
+    //
+
     PHLMONITOR             getMonitorFromID(const MONITORID&);
     PHLMONITOR             getMonitorFromName(const std::string&);
     PHLMONITOR             getMonitorFromDesc(const std::string&);
@@ -96,7 +104,6 @@ class CCompositor {
     PHLWORKSPACE           getWorkspaceByID(const WORKSPACEID&);
     PHLWORKSPACE           getWorkspaceByName(const std::string&);
     PHLWORKSPACE           getWorkspaceByString(const std::string&);
-    void                   sanityCheckWorkspaces();
     PHLWINDOW              getUrgentWindow();
     bool                   isWindowActive(PHLWINDOW);
     void                   changeWindowZOrder(PHLWINDOW, bool);
@@ -156,21 +163,23 @@ class CCompositor {
     std::string                         m_explicitConfigPath;
 
   private:
-    void             initAllSignals();
+    void                         initAllSignals();
-    void             removeAllSignals();
+    void                         removeAllSignals();
-    void             cleanEnvironment();
+    void                         cleanEnvironment();
-    void             setRandomSplash();
+    void                         setRandomSplash();
-    void             initManagers(eManagersInitStage stage);
+    void                         initManagers(eManagersInitStage stage);
-    void             prepareFallbackOutput();
+    void                         prepareFallbackOutput();
-    void             createLockFile();
+    void                         createLockFile();
-    void             removeLockFile();
+    void                         removeLockFile();
-    void             setMallocThreshold();
+    void                         setMallocThreshold();
 
-    bool             m_bDrmSyncobjTimelineSupported = false;
+    bool                         m_bDrmSyncobjTimelineSupported = false;
 
-    uint64_t         m_hyprlandPID    = 0;
+    uint64_t                     m_hyprlandPID    = 0;
-    wl_event_source* m_critSigSource  = nullptr;
+    wl_event_source*             m_critSigSource  = nullptr;
-    rlimit           m_originalNofile = {};
+    rlimit                       m_originalNofile = {};
+
+    std::vector<PHLWORKSPACEREF> m_workspaces;
 };
 
 inline UP<CCompositor> g_pCompositor;

src/config/ConfigManager.cpp

@@ -1194,7 +1194,7 @@ void CConfigManager::postConfigReload(const Hyprlang::CParseResult& result) {
         refreshGroupBarGradients();
 
     // Updates dynamic window and workspace rules
-    for (auto const& w : g_pCompositor->m_workspaces) {
+    for (auto const& w : g_pCompositor->getWorkspaces()) {
         if (w->inert())
             continue;
         w->updateWindows();

src/debug/HyprCtl.cpp

@@ -432,16 +432,16 @@ static std::string workspacesRequest(eHyprCtlOutputFormat format, std::string re
 
     if (format == eHyprCtlOutputFormat::FORMAT_JSON) {
         result += "[";
-        for (auto const& w : g_pCompositor->m_workspaces) {
+        for (auto const& w : g_pCompositor->getWorkspaces()) {
-            result += CHyprCtl::getWorkspaceData(w, format);
+            result += CHyprCtl::getWorkspaceData(w.lock(), format);
             result += ",";
         }
 
         trimTrailingComma(result);
         result += "]";
     } else {
-        for (auto const& w : g_pCompositor->m_workspaces) {
+        for (auto const& w : g_pCompositor->getWorkspaces()) {
-            result += CHyprCtl::getWorkspaceData(w, format);
+            result += CHyprCtl::getWorkspaceData(w.lock(), format);
         }
     }
 

src/desktop/Workspace.cpp

@@ -13,6 +13,7 @@ using namespace Hyprutils::String;
 PHLWORKSPACE CWorkspace::create(WORKSPACEID id, PHLMONITOR monitor, std::string name, bool special, bool isEmpty) {
     PHLWORKSPACE workspace = makeShared<CWorkspace>(id, monitor, name, special, isEmpty);
     workspace->init(workspace);
+    g_pCompositor->registerWorkspace(workspace);
     return workspace;
 }
 

src/helpers/MiscFunctions.cpp

@@ -219,7 +219,7 @@ SWorkspaceIDName getWorkspaceIDNameFromString(const std::string& in) {
             std::set<WORKSPACEID> invalidWSes;
 
             // Collect all the workspaces we can't jump to.
-            for (auto const& ws : g_pCompositor->m_workspaces) {
+            for (auto const& ws : g_pCompositor->getWorkspaces()) {
                 if (ws->m_isSpecialWorkspace || (ws->m_monitor != g_pCompositor->m_lastMonitor)) {
                     // Can't jump to this workspace
                     invalidWSes.insert(ws->m_id);
@@ -237,7 +237,7 @@ SWorkspaceIDName getWorkspaceIDNameFromString(const std::string& in) {
 
             // Prepare all named workspaces in case when we need them
             std::vector<WORKSPACEID> namedWSes;
-            for (auto const& ws : g_pCompositor->m_workspaces) {
+            for (auto const& ws : g_pCompositor->getWorkspaces()) {
                 if (ws->m_isSpecialWorkspace || (ws->m_monitor != g_pCompositor->m_lastMonitor) || ws->m_id >= 0)
                     continue;
 
@@ -381,7 +381,7 @@ SWorkspaceIDName getWorkspaceIDNameFromString(const std::string& in) {
             int                      remains = (int)result.id;
 
             std::vector<WORKSPACEID> validWSes;
-            for (auto const& ws : g_pCompositor->m_workspaces) {
+            for (auto const& ws : g_pCompositor->getWorkspaces()) {
                 if (ws->m_isSpecialWorkspace || (ws->m_monitor != g_pCompositor->m_lastMonitor && !onAllMonitors))
                     continue;
 

src/helpers/Monitor.cpp

@@ -216,12 +216,12 @@ void CMonitor::onConnect(bool noRule) {
 
     setupDefaultWS(monitorRule);
 
-    for (auto const& ws : g_pCompositor->m_workspaces) {
+    for (auto const& ws : g_pCompositor->getWorkspaces()) {
-        if (!valid(ws))
+        if (!valid(ws.lock()))
             continue;
 
         if (ws->m_lastMonitor == m_name || g_pCompositor->m_monitors.size() == 1 /* avoid lost workspaces on recover */) {
-            g_pCompositor->moveWorkspaceToMonitor(ws, m_self.lock());
+            g_pCompositor->moveWorkspaceToMonitor(ws.lock(), m_self.lock());
             ws->startAnim(true, true, true);
             ws->m_lastMonitor = "";
         }
@@ -365,9 +365,9 @@ void CMonitor::onDisconnect(bool destroy) {
 
         // move workspaces
         std::vector<PHLWORKSPACE> wspToMove;
-        for (auto const& w : g_pCompositor->m_workspaces) {
+        for (auto const& w : g_pCompositor->getWorkspaces()) {
             if (w->m_monitor == m_self || !w->m_monitor)
-                wspToMove.push_back(w);
+                wspToMove.emplace_back(w.lock());
         }
 
         for (auto const& w : wspToMove) {
@@ -994,7 +994,7 @@ void CMonitor::setupDefaultWS(const SMonitorRule& monitorRule) {
     }
 
     if (wsID == WORKSPACE_INVALID || (wsID >= SPECIAL_WORKSPACE_START && wsID <= -2)) {
-        wsID                    = g_pCompositor->m_workspaces.size() + 1;
+        wsID                    = std::ranges::distance(g_pCompositor->getWorkspaces()) + 1;
         newDefaultWorkspaceName = std::to_string(wsID);
 
         Debug::log(LOG, "Invalid workspace= directive name in monitor parsing, workspace name \"{}\" is invalid.", g_pConfigManager->getDefaultWorkspaceFor(m_name));
@@ -1014,7 +1014,7 @@ void CMonitor::setupDefaultWS(const SMonitorRule& monitorRule) {
         if (newDefaultWorkspaceName.empty())
             newDefaultWorkspaceName = std::to_string(wsID);
 
-        PNEWWORKSPACE = g_pCompositor->m_workspaces.emplace_back(CWorkspace::create(wsID, m_self.lock(), newDefaultWorkspaceName));
+        PNEWWORKSPACE = CWorkspace::create(wsID, m_self.lock(), newDefaultWorkspaceName);
     }
 
     m_activeWorkspace = PNEWWORKSPACE;
@@ -1089,9 +1089,9 @@ void CMonitor::setMirror(const std::string& mirrorOf) {
 
         // move all the WS
         std::vector<PHLWORKSPACE> wspToMove;
-        for (auto const& w : g_pCompositor->m_workspaces) {
+        for (auto const& w : g_pCompositor->getWorkspaces()) {
             if (w->m_monitor == m_self || !w->m_monitor)
-                wspToMove.push_back(w);
+                wspToMove.emplace_back(w.lock());
         }
 
         for (auto const& w : wspToMove) {
@@ -1114,8 +1114,6 @@ void CMonitor::setMirror(const std::string& mirrorOf) {
 
         g_pCompositor->setActiveMonitor(g_pCompositor->m_monitors.front());
 
-        g_pCompositor->sanityCheckWorkspaces();
-
         // Software lock mirrored monitor
         g_pPointerManager->lockSoftwareForMonitor(PMIRRORMON);
     }
@@ -1150,7 +1148,7 @@ static bool shouldWraparound(const WORKSPACEID id1, const WORKSPACEID id2) {
     WORKSPACEID lowestID  = INT64_MAX;
     WORKSPACEID highestID = INT64_MIN;
 
-    for (auto const& w : g_pCompositor->m_workspaces) {
+    for (auto const& w : g_pCompositor->getWorkspaces()) {
         if (w->m_id < 0 || w->m_isSpecialWorkspace)
             continue;
         lowestID  = std::min(w->m_id, lowestID);

src/managers/input/Swipe.cpp

@@ -17,7 +17,7 @@ void CInputManager::onSwipeBegin(IPointer::SSwipeBeginEvent e) {
         return;
 
     int onMonitor = 0;
-    for (auto const& w : g_pCompositor->m_workspaces) {
+    for (auto const& w : g_pCompositor->getWorkspaces()) {
         if (w->m_monitor == g_pCompositor->m_lastMonitor && !g_pCompositor->isWorkspaceSpecial(w->m_id))
             onMonitor++;
     }

src/protocols/ExtWorkspace.cpp

@@ -224,8 +224,8 @@ void CExtWorkspaceManagerResource::init(WP<CExtWorkspaceManagerResource> self) {
         onMonitorCreated(m);
     }
 
-    for (auto const& w : g_pCompositor->m_workspaces) {
+    for (auto const& w : g_pCompositor->getWorkspaces()) {
-        onWorkspaceCreated(w);
+        onWorkspaceCreated(w.lock());
     }
 }
 

src/render/Renderer.cpp

@@ -941,14 +941,14 @@ void CHyprRenderer::renderAllClientsForWorkspace(PHLMONITOR pMonitor, PHLWORKSPA
     }
 
     // special
-    for (auto const& ws : g_pCompositor->m_workspaces) {
+    for (auto const& ws : g_pCompositor->getWorkspaces()) {
         if (ws->m_alpha->value() <= 0.F || !ws->m_isSpecialWorkspace)
             continue;
 
         if (ws->m_hasFullscreenWindow)
-            renderWorkspaceWindowsFullscreen(pMonitor, ws, time);
+            renderWorkspaceWindowsFullscreen(pMonitor, ws.lock(), time);
         else
-            renderWorkspaceWindows(pMonitor, ws, time);
+            renderWorkspaceWindows(pMonitor, ws.lock(), time);
     }
 
     // pinned always above
@@ -1225,7 +1225,6 @@ void CHyprRenderer::renderMonitor(PHLMONITOR pMonitor, bool commit) {
 
     if (pMonitor->m_id == m_mostHzMonitor->m_id ||
         *PVFR == 1) { // unfortunately with VFR we don't have the guarantee mostHz is going to be updated all the time, so we have to ignore that
-        g_pCompositor->sanityCheckWorkspaces();
 
         g_pConfigManager->dispatchExecOnce(); // We exec-once when at least one monitor starts refreshing, meaning stuff has init'd
 
@@ -2192,7 +2191,7 @@ void CHyprRenderer::recheckSolitaryForMonitor(PHLMONITOR pMonitor) {
     if (pMonitor->m_activeSpecialWorkspace)
         return;
 
-    for (auto const& ws : g_pCompositor->m_workspaces) {
+    for (auto const& ws : g_pCompositor->getWorkspaces()) {
         if (ws->m_alpha->value() <= 0.F || !ws->m_isSpecialWorkspace || ws->m_monitor != pMonitor)
             continue;