Skip to content

Commit 31cc7f3

Browse files
vaxerskivaxerski
authored
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.
1 parent ecc04e8 commit 31cc7f3

File tree

11 files changed

+69
-75
lines changed

11 files changed

+69
-75
lines changed

‎CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,14 @@ set(CXX_STANDARD_REQUIRED ON)
8585
add_compile_options(
8686
-Wall
8787
-Wextra
88+
-Wpedantic
8889
-Wno-unused-parameter
8990
-Wno-unused-value
9091
-Wno-missing-field-initializers
92+
-Wno-gnu-zero-variadic-macro-arguments
9193
-Wno-narrowing
9294
-Wno-pointer-arith
9395
-Wno-clobbered
94-
-Wpedantic
9596
-fmacro-prefix-map=${CMAKE_SOURCE_DIR}/=)
9697

9798
set(CMAKE_EXECUTABLE_ENABLE_EXPORTS TRUE)

‎src/Compositor.cpp

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,29 +1338,14 @@ PHLWINDOW CCompositor::getWindowFromHandle(uint32_t handle) {
13381338
}
13391339

13401340
PHLWORKSPACE CCompositor::getWorkspaceByID(const WORKSPACEID& id) {
1341-
for (auto const& w : m_workspaces) {
1341+
for (auto const& w : getWorkspaces()) {
13421342
if (w->m_id == id && !w->inert())
1343-
return w;
1343+
return w.lock();
13441344
}
13451345

13461346
return nullptr;
13471347
}
13481348

1349-
void CCompositor::sanityCheckWorkspaces() {
1350-
auto it = m_workspaces.begin();
1351-
while (it != m_workspaces.end()) {
1352-
const auto& WORKSPACE = *it;
1353-
1354-
// If ref == 1, only the compositor holds a ref, which means it's inactive and has no mapped windows.
1355-
if (!WORKSPACE->m_persistent && WORKSPACE.strongRef() == 1) {
1356-
it = m_workspaces.erase(it);
1357-
continue;
1358-
}
1359-
1360-
++it;
1361-
}
1362-
}
1363-
13641349
PHLWINDOW CCompositor::getUrgentWindow() {
13651350
for (auto const& w : m_windows) {
13661351
if (w->m_isMapped && w->m_isUrgent)
@@ -1732,7 +1717,7 @@ PHLWINDOW CCompositor::getWindowCycle(PHLWINDOW cur, bool focusableOnly, std::op
17321717

17331718
WORKSPACEID CCompositor::getNextAvailableNamedWorkspace() {
17341719
WORKSPACEID lowest = -1337 + 1;
1735-
for (auto const& w : m_workspaces) {
1720+
for (auto const& w : getWorkspaces()) {
17361721
if (w->m_id < -1 && w->m_id < lowest)
17371722
lowest = w->m_id;
17381723
}
@@ -1741,9 +1726,9 @@ WORKSPACEID CCompositor::getNextAvailableNamedWorkspace() {
17411726
}
17421727

17431728
PHLWORKSPACE CCompositor::getWorkspaceByName(const std::string& name) {
1744-
for (auto const& w : m_workspaces) {
1729+
for (auto const& w : getWorkspaces()) {
17451730
if (w->m_name == name && !w->inert())
1746-
return w;
1731+
return w.lock();
17471732
}
17481733

17491734
return nullptr;
@@ -2144,7 +2129,7 @@ void CCompositor::moveWorkspaceToMonitor(PHLWORKSPACE pWorkspace, PHLMONITOR pMo
21442129
if (!SWITCHINGISACTIVE)
21452130
nextWorkspaceOnMonitorID = pWorkspace->m_id;
21462131
else {
2147-
for (auto const& w : m_workspaces) {
2132+
for (auto const& w : getWorkspaces()) {
21482133
if (w->m_monitor == POLDMON && w->m_id != pWorkspace->m_id && !w->m_isSpecialWorkspace) {
21492134
nextWorkspaceOnMonitorID = w->m_id;
21502135
break;
@@ -2258,7 +2243,7 @@ bool CCompositor::workspaceIDOutOfBounds(const WORKSPACEID& id) {
22582243
WORKSPACEID lowestID = INT64_MAX;
22592244
WORKSPACEID highestID = INT64_MIN;
22602245

2261-
for (auto const& w : m_workspaces) {
2246+
for (auto const& w : getWorkspaces()) {
22622247
if (w->m_isSpecialWorkspace)
22632248
continue;
22642249
lowestID = std::min(w->m_id, lowestID);
@@ -2660,7 +2645,7 @@ PHLWORKSPACE CCompositor::createNewWorkspace(const WORKSPACEID& id, const MONITO
26602645
return nullptr;
26612646
}
26622647

2663-
const auto PWORKSPACE = m_workspaces.emplace_back(CWorkspace::create(id, PMONITOR, NAME, SPECIAL, isEmpty));
2648+
const auto PWORKSPACE = CWorkspace::create(id, PMONITOR, NAME, SPECIAL, isEmpty);
26642649

26652650
PWORKSPACE->m_alpha->setValueAndWarp(0);
26662651

@@ -2694,15 +2679,19 @@ bool CCompositor::isWorkspaceSpecial(const WORKSPACEID& id) {
26942679

26952680
WORKSPACEID CCompositor::getNewSpecialID() {
26962681
WORKSPACEID highest = SPECIAL_WORKSPACE_START;
2697-
for (auto const& ws : m_workspaces) {
2698-
if (ws->m_isSpecialWorkspace && ws->m_id > highest) {
2682+
for (auto const& ws : getWorkspaces()) {
2683+
if (ws->m_isSpecialWorkspace && ws->m_id > highest)
26992684
highest = ws->m_id;
2700-
}
27012685
}
27022686

27032687
return highest + 1;
27042688
}
27052689

2690+
void CCompositor::registerWorkspace(PHLWORKSPACE w) {
2691+
m_workspaces.emplace_back(w);
2692+
w->m_events.destroy.listenStatic([this, weak = PHLWORKSPACEREF{w}] { std::erase(m_workspaces, weak); });
2693+
}
2694+
27062695
void CCompositor::performUserChecks() {
27072696
static auto PNOCHECKXDG = CConfigValue<Hyprlang::INT>("misc:disable_xdg_env_checks");
27082697
static auto PNOCHECKQTUTILS = CConfigValue<Hyprlang::INT>("misc:disable_hyprland_qtutils_check");
@@ -3176,7 +3165,4 @@ void CCompositor::ensurePersistentWorkspacesPresent(const std::vector<SWorkspace
31763165
continue;
31773166
}
31783167
}
3179-
3180-
// cleanup old
3181-
sanityCheckWorkspaces();
31823168
}

‎src/Compositor.hpp

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
#include <sys/resource.h>
44

5+
#include <ranges>
6+
57
#include "managers/XWaylandManager.hpp"
68
#include "managers/KeybindManager.hpp"
79
#include "managers/SessionLockManager.hpp"
@@ -42,7 +44,6 @@ class CCompositor {
4244
std::vector<PHLMONITOR> m_realMonitors; // for all monitors, even those turned off
4345
std::vector<PHLWINDOW> m_windows;
4446
std::vector<PHLLS> m_layers;
45-
std::vector<PHLWORKSPACE> m_workspaces;
4647
std::vector<PHLWINDOWREF> m_windowsFadingOut;
4748
std::vector<PHLLSREF> m_surfacesFadingOut;
4849

@@ -75,6 +76,13 @@ class CCompositor {
7576

7677
// ------------------------------------------------- //
7778

79+
auto getWorkspaces() {
80+
return std::views::filter(m_workspaces, [](const auto& e) { return e; });
81+
}
82+
void registerWorkspace(PHLWORKSPACE w);
83+
84+
//
85+
7886
PHLMONITOR getMonitorFromID(const MONITORID&);
7987
PHLMONITOR getMonitorFromName(const std::string&);
8088
PHLMONITOR getMonitorFromDesc(const std::string&);
@@ -96,7 +104,6 @@ class CCompositor {
96104
PHLWORKSPACE getWorkspaceByID(const WORKSPACEID&);
97105
PHLWORKSPACE getWorkspaceByName(const std::string&);
98106
PHLWORKSPACE getWorkspaceByString(const std::string&);
99-
void sanityCheckWorkspaces();
100107
PHLWINDOW getUrgentWindow();
101108
bool isWindowActive(PHLWINDOW);
102109
void changeWindowZOrder(PHLWINDOW, bool);
@@ -156,21 +163,23 @@ class CCompositor {
156163
std::string m_explicitConfigPath;
157164

158165
private:
159-
void initAllSignals();
160-
void removeAllSignals();
161-
void cleanEnvironment();
162-
void setRandomSplash();
163-
void initManagers(eManagersInitStage stage);
164-
void prepareFallbackOutput();
165-
void createLockFile();
166-
void removeLockFile();
167-
void setMallocThreshold();
168-
169-
bool m_bDrmSyncobjTimelineSupported = false;
170-
171-
uint64_t m_hyprlandPID = 0;
172-
wl_event_source* m_critSigSource = nullptr;
173-
rlimit m_originalNofile = {};
166+
void initAllSignals();
167+
void removeAllSignals();
168+
void cleanEnvironment();
169+
void setRandomSplash();
170+
void initManagers(eManagersInitStage stage);
171+
void prepareFallbackOutput();
172+
void createLockFile();
173+
void removeLockFile();
174+
void setMallocThreshold();
175+
176+
bool m_bDrmSyncobjTimelineSupported = false;
177+
178+
uint64_t m_hyprlandPID = 0;
179+
wl_event_source* m_critSigSource = nullptr;
180+
rlimit m_originalNofile = {};
181+
182+
std::vector<PHLWORKSPACEREF> m_workspaces;
174183
};
175184

176185
inline UP<CCompositor> g_pCompositor;

‎src/config/ConfigManager.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1194,7 +1194,7 @@ void CConfigManager::postConfigReload(const Hyprlang::CParseResult& result) {
11941194
refreshGroupBarGradients();
11951195

11961196
// Updates dynamic window and workspace rules
1197-
for (auto const& w : g_pCompositor->m_workspaces) {
1197+
for (auto const& w : g_pCompositor->getWorkspaces()) {
11981198
if (w->inert())
11991199
continue;
12001200
w->updateWindows();

‎src/debug/HyprCtl.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -432,16 +432,16 @@ static std::string workspacesRequest(eHyprCtlOutputFormat format, std::string re
432432

433433
if (format == eHyprCtlOutputFormat::FORMAT_JSON) {
434434
result += "[";
435-
for (auto const& w : g_pCompositor->m_workspaces) {
436-
result += CHyprCtl::getWorkspaceData(w, format);
435+
for (auto const& w : g_pCompositor->getWorkspaces()) {
436+
result += CHyprCtl::getWorkspaceData(w.lock(), format);
437437
result += ",";
438438
}
439439

440440
trimTrailingComma(result);
441441
result += "]";
442442
} else {
443-
for (auto const& w : g_pCompositor->m_workspaces) {
444-
result += CHyprCtl::getWorkspaceData(w, format);
443+
for (auto const& w : g_pCompositor->getWorkspaces()) {
444+
result += CHyprCtl::getWorkspaceData(w.lock(), format);
445445
}
446446
}
447447

‎src/desktop/Workspace.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ using namespace Hyprutils::String;
1313
PHLWORKSPACE CWorkspace::create(WORKSPACEID id, PHLMONITOR monitor, std::string name, bool special, bool isEmpty) {
1414
PHLWORKSPACE workspace = makeShared<CWorkspace>(id, monitor, name, special, isEmpty);
1515
workspace->init(workspace);
16+
g_pCompositor->registerWorkspace(workspace);
1617
return workspace;
1718
}
1819

‎src/helpers/MiscFunctions.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ SWorkspaceIDName getWorkspaceIDNameFromString(const std::string& in) {
219219
std::set<WORKSPACEID> invalidWSes;
220220

221221
// Collect all the workspaces we can't jump to.
222-
for (auto const& ws : g_pCompositor->m_workspaces) {
222+
for (auto const& ws : g_pCompositor->getWorkspaces()) {
223223
if (ws->m_isSpecialWorkspace || (ws->m_monitor != g_pCompositor->m_lastMonitor)) {
224224
// Can't jump to this workspace
225225
invalidWSes.insert(ws->m_id);
@@ -237,7 +237,7 @@ SWorkspaceIDName getWorkspaceIDNameFromString(const std::string& in) {
237237

238238
// Prepare all named workspaces in case when we need them
239239
std::vector<WORKSPACEID> namedWSes;
240-
for (auto const& ws : g_pCompositor->m_workspaces) {
240+
for (auto const& ws : g_pCompositor->getWorkspaces()) {
241241
if (ws->m_isSpecialWorkspace || (ws->m_monitor != g_pCompositor->m_lastMonitor) || ws->m_id >= 0)
242242
continue;
243243

@@ -381,7 +381,7 @@ SWorkspaceIDName getWorkspaceIDNameFromString(const std::string& in) {
381381
int remains = (int)result.id;
382382

383383
std::vector<WORKSPACEID> validWSes;
384-
for (auto const& ws : g_pCompositor->m_workspaces) {
384+
for (auto const& ws : g_pCompositor->getWorkspaces()) {
385385
if (ws->m_isSpecialWorkspace || (ws->m_monitor != g_pCompositor->m_lastMonitor && !onAllMonitors))
386386
continue;
387387

‎src/helpers/Monitor.cpp

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -216,12 +216,12 @@ void CMonitor::onConnect(bool noRule) {
216216

217217
setupDefaultWS(monitorRule);
218218

219-
for (auto const& ws : g_pCompositor->m_workspaces) {
220-
if (!valid(ws))
219+
for (auto const& ws : g_pCompositor->getWorkspaces()) {
220+
if (!valid(ws.lock()))
221221
continue;
222222

223223
if (ws->m_lastMonitor == m_name || g_pCompositor->m_monitors.size() == 1 /* avoid lost workspaces on recover */) {
224-
g_pCompositor->moveWorkspaceToMonitor(ws, m_self.lock());
224+
g_pCompositor->moveWorkspaceToMonitor(ws.lock(), m_self.lock());
225225
ws->startAnim(true, true, true);
226226
ws->m_lastMonitor = "";
227227
}
@@ -365,9 +365,9 @@ void CMonitor::onDisconnect(bool destroy) {
365365

366366
// move workspaces
367367
std::vector<PHLWORKSPACE> wspToMove;
368-
for (auto const& w : g_pCompositor->m_workspaces) {
368+
for (auto const& w : g_pCompositor->getWorkspaces()) {
369369
if (w->m_monitor == m_self || !w->m_monitor)
370-
wspToMove.push_back(w);
370+
wspToMove.emplace_back(w.lock());
371371
}
372372

373373
for (auto const& w : wspToMove) {
@@ -994,7 +994,7 @@ void CMonitor::setupDefaultWS(const SMonitorRule& monitorRule) {
994994
}
995995

996996
if (wsID == WORKSPACE_INVALID || (wsID >= SPECIAL_WORKSPACE_START && wsID <= -2)) {
997-
wsID = g_pCompositor->m_workspaces.size() + 1;
997+
wsID = std::ranges::distance(g_pCompositor->getWorkspaces()) + 1;
998998
newDefaultWorkspaceName = std::to_string(wsID);
999999

10001000
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) {
10141014
if (newDefaultWorkspaceName.empty())
10151015
newDefaultWorkspaceName = std::to_string(wsID);
10161016

1017-
PNEWWORKSPACE = g_pCompositor->m_workspaces.emplace_back(CWorkspace::create(wsID, m_self.lock(), newDefaultWorkspaceName));
1017+
PNEWWORKSPACE = CWorkspace::create(wsID, m_self.lock(), newDefaultWorkspaceName);
10181018
}
10191019

10201020
m_activeWorkspace = PNEWWORKSPACE;
@@ -1089,9 +1089,9 @@ void CMonitor::setMirror(const std::string& mirrorOf) {
10891089

10901090
// move all the WS
10911091
std::vector<PHLWORKSPACE> wspToMove;
1092-
for (auto const& w : g_pCompositor->m_workspaces) {
1092+
for (auto const& w : g_pCompositor->getWorkspaces()) {
10931093
if (w->m_monitor == m_self || !w->m_monitor)
1094-
wspToMove.push_back(w);
1094+
wspToMove.emplace_back(w.lock());
10951095
}
10961096

10971097
for (auto const& w : wspToMove) {
@@ -1114,8 +1114,6 @@ void CMonitor::setMirror(const std::string& mirrorOf) {
11141114

11151115
g_pCompositor->setActiveMonitor(g_pCompositor->m_monitors.front());
11161116

1117-
g_pCompositor->sanityCheckWorkspaces();
1118-
11191117
// Software lock mirrored monitor
11201118
g_pPointerManager->lockSoftwareForMonitor(PMIRRORMON);
11211119
}
@@ -1150,7 +1148,7 @@ static bool shouldWraparound(const WORKSPACEID id1, const WORKSPACEID id2) {
11501148
WORKSPACEID lowestID = INT64_MAX;
11511149
WORKSPACEID highestID = INT64_MIN;
11521150

1153-
for (auto const& w : g_pCompositor->m_workspaces) {
1151+
for (auto const& w : g_pCompositor->getWorkspaces()) {
11541152
if (w->m_id < 0 || w->m_isSpecialWorkspace)
11551153
continue;
11561154
lowestID = std::min(w->m_id, lowestID);

‎src/managers/input/Swipe.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ void CInputManager::onSwipeBegin(IPointer::SSwipeBeginEvent e) {
1717
return;
1818

1919
int onMonitor = 0;
20-
for (auto const& w : g_pCompositor->m_workspaces) {
20+
for (auto const& w : g_pCompositor->getWorkspaces()) {
2121
if (w->m_monitor == g_pCompositor->m_lastMonitor && !g_pCompositor->isWorkspaceSpecial(w->m_id))
2222
onMonitor++;
2323
}

‎src/protocols/ExtWorkspace.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,8 @@ void CExtWorkspaceManagerResource::init(WP<CExtWorkspaceManagerResource> self) {
224224
onMonitorCreated(m);
225225
}
226226

227-
for (auto const& w : g_pCompositor->m_workspaces) {
228-
onWorkspaceCreated(w);
227+
for (auto const& w : g_pCompositor->getWorkspaces()) {
228+
onWorkspaceCreated(w.lock());
229229
}
230230
}
231231

0 commit comments

Comments
 (0)