core: fix data race and a unsigned int rollover (#7278)

* keybindmgr: avoid uint rollover on mouse keycode mouse keycode is 0, and the switch case checks for 0 - 8 and rolls over, just return early if keycode is 0. * watchdog: avoid data races in watchdog asan thread sanitizer reported data races in the watchdog from reading and setting the bool variables make them std::atomic bools. also add a atomic bool for the main thread to wait for to avoid data race when reading the config values. * hyprdebug: change non unicode character to name asan created false positives and didnt like this bit, so for the sake of easier debugging rename it to something unicode.
hyprwm · Aug 12, 2024 · 3fa6db1 · 3fa6db1
1 parent d361fcb
commit 3fa6db1
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 30 deletions.
diff --git a/src/Compositor.cpp b/src/Compositor.cpp
@@ -552,6 +552,10 @@ void CCompositor::initManagers(eManagersInitStage stage) {
 
             g_pConfigManager->init();
             g_pWatchdog = std::make_unique<CWatchdog>(); // requires config
+            // wait for watchdog to initialize to not hit data races in reading config values.
+            while (!g_pWatchdog->m_bWatchdogInitialized) {
+                std::this_thread::yield();
+            }
 
             Debug::log(LOG, "Creating the PointerManager!");
             g_pPointerManager = std::make_unique<CPointerManager>();

diff --git a/src/debug/HyprDebugOverlay.cpp b/src/debug/HyprDebugOverlay.cpp
@@ -7,8 +7,8 @@ CHyprDebugOverlay::CHyprDebugOverlay() {
     m_pTexture = makeShared<CTexture>();
 }
 
-void CHyprMonitorDebugOverlay::renderData(CMonitor* pMonitor, float µs) {
-    m_dLastRenderTimes.push_back(µs / 1000.f);
+void CHyprMonitorDebugOverlay::renderData(CMonitor* pMonitor, float durationUs) {
+    m_dLastRenderTimes.push_back(durationUs / 1000.f);
 
     if (m_dLastRenderTimes.size() > (long unsigned int)pMonitor->refreshRate)
         m_dLastRenderTimes.pop_front();
@@ -17,8 +17,8 @@ void CHyprMonitorDebugOverlay::renderData(CMonitor* pMonitor, float µs) {
         m_pMonitor = pMonitor;
 }
 
-void CHyprMonitorDebugOverlay::renderDataNoOverlay(CMonitor* pMonitor, float µs) {
-    m_dLastRenderTimesNoOverlay.push_back(µs / 1000.f);
+void CHyprMonitorDebugOverlay::renderDataNoOverlay(CMonitor* pMonitor, float durationUs) {
+    m_dLastRenderTimesNoOverlay.push_back(durationUs / 1000.f);
 
     if (m_dLastRenderTimesNoOverlay.size() > (long unsigned int)pMonitor->refreshRate)
         m_dLastRenderTimesNoOverlay.pop_front();
@@ -188,12 +188,12 @@ int CHyprMonitorDebugOverlay::draw(int offset) {
     return posY - offset;
 }
 
-void CHyprDebugOverlay::renderData(CMonitor* pMonitor, float µs) {
-    m_mMonitorOverlays[pMonitor].renderData(pMonitor, µs);
+void CHyprDebugOverlay::renderData(CMonitor* pMonitor, float durationUs) {
+    m_mMonitorOverlays[pMonitor].renderData(pMonitor, durationUs);
 }
 
-void CHyprDebugOverlay::renderDataNoOverlay(CMonitor* pMonitor, float µs) {
-    m_mMonitorOverlays[pMonitor].renderDataNoOverlay(pMonitor, µs);
+void CHyprDebugOverlay::renderDataNoOverlay(CMonitor* pMonitor, float durationUs) {
+    m_mMonitorOverlays[pMonitor].renderDataNoOverlay(pMonitor, durationUs);
 }
 
 void CHyprDebugOverlay::frameData(CMonitor* pMonitor) {

diff --git a/src/debug/HyprDebugOverlay.hpp b/src/debug/HyprDebugOverlay.hpp
@@ -13,8 +13,8 @@ class CHyprMonitorDebugOverlay {
   public:
     int  draw(int offset);
 
-    void renderData(CMonitor* pMonitor, float µs);
-    void renderDataNoOverlay(CMonitor* pMonitor, float µs);
+    void renderData(CMonitor* pMonitor, float durationUs);
+    void renderDataNoOverlay(CMonitor* pMonitor, float durationUs);
     void frameData(CMonitor* pMonitor);
 
   private:
@@ -33,8 +33,8 @@ class CHyprDebugOverlay {
   public:
     CHyprDebugOverlay();
     void draw();
-    void renderData(CMonitor*, float µs);
-    void renderDataNoOverlay(CMonitor*, float µs);
+    void renderData(CMonitor*, float durationUs);
+    void renderDataNoOverlay(CMonitor*, float durationUs);
     void frameData(CMonitor*);
 
   private:

diff --git a/src/helpers/Watchdog.cpp b/src/helpers/Watchdog.cpp
@@ -18,15 +18,14 @@ CWatchdog::CWatchdog() {
     m_pWatchdog = std::make_unique<std::thread>([this] {
         static auto PTIMEOUT = CConfigValue<Hyprlang::INT>("debug:watchdog_timeout");
 
-        while (1337) {
-            std::unique_lock lk(m_mWatchdogMutex);
+        m_bWatchdogInitialized = true;
+        while (!m_bExitThread) {
+            std::unique_lock<std::mutex> lk(m_mWatchdogMutex);
 
             if (!m_bWillWatch)
-                m_cvWatchdogCondition.wait(lk, [this] { return m_bNotified; });
-            else {
-                if (m_cvWatchdogCondition.wait_for(lk, std::chrono::milliseconds((int)(*PTIMEOUT * 1000.0)), [this] { return m_bNotified; }) == false)
-                    pthread_kill(m_iMainThreadPID, SIGUSR1);
-            }
+                m_cvWatchdogCondition.wait(lk, [this] { return m_bNotified || m_bExitThread; });
+            else if (m_cvWatchdogCondition.wait_for(lk, std::chrono::milliseconds((int)(*PTIMEOUT * 1000.0)), [this] { return m_bNotified || m_bExitThread; }) == false)
+                pthread_kill(m_iMainThreadPID, SIGUSR1);
 
             if (m_bExitThread)
                 break;

diff --git a/src/helpers/Watchdog.hpp b/src/helpers/Watchdog.hpp
@@ -11,21 +11,23 @@ class CWatchdog {
     CWatchdog();
     ~CWatchdog();
 
-    void startWatching();
-    void endWatching();
+    void              startWatching();
+    void              endWatching();
+
+    std::atomic<bool> m_bWatchdogInitialized{false};
 
   private:
     std::chrono::high_resolution_clock::time_point m_tTriggered;
 
     pthread_t                                      m_iMainThreadPID = 0;
 
-    bool                                           m_bWatching  = false;
-    bool                                           m_bWillWatch = false;
+    std::atomic<bool>                              m_bWatching  = false;
+    std::atomic<bool>                              m_bWillWatch = false;
 
     std::unique_ptr<std::thread>                   m_pWatchdog;
     std::mutex                                     m_mWatchdogMutex;
-    bool                                           m_bNotified   = false;
-    bool                                           m_bExitThread = false;
+    std::atomic<bool>                              m_bNotified   = false;
+    std::atomic<bool>                              m_bExitThread = false;
     std::condition_variable                        m_cvWatchdogCondition;
 };
 

diff --git a/src/managers/KeybindManager.cpp b/src/managers/KeybindManager.cpp
@@ -180,6 +180,9 @@ uint32_t CKeybindManager::stringToModMask(std::string mods) {
 }
 
 uint32_t CKeybindManager::keycodeToModifier(xkb_keycode_t keycode) {
+    if (keycode == 0)
+        return 0;
+
     switch (keycode - 8) {
         case KEY_LEFTMETA: return HL_MODIFIER_META;
         case KEY_RIGHTMETA: return HL_MODIFIER_META;

diff --git a/src/render/Renderer.cpp b/src/render/Renderer.cpp
@@ -1395,15 +1395,15 @@ void CHyprRenderer::renderMonitor(CMonitor* pMonitor) {
 
     pMonitor->pendingFrame = false;
 
-    const float µs = std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::high_resolution_clock::now() - renderStart).count() / 1000.f;
-    g_pDebugOverlay->renderData(pMonitor, µs);
+    const float durationUs = std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::high_resolution_clock::now() - renderStart).count() / 1000.f;
+    g_pDebugOverlay->renderData(pMonitor, durationUs);
 
     if (*PDEBUGOVERLAY == 1) {
         if (pMonitor == g_pCompositor->m_vMonitors.front().get()) {
-            const float µsNoOverlay = µs - std::chrono::duration_cast<std::chrono::nanoseconds>(endRenderOverlay - renderStartOverlay).count() / 1000.f;
-            g_pDebugOverlay->renderDataNoOverlay(pMonitor, µsNoOverlay);
+            const float noOverlayUs = durationUs - std::chrono::duration_cast<std::chrono::nanoseconds>(endRenderOverlay - renderStartOverlay).count() / 1000.f;
+            g_pDebugOverlay->renderDataNoOverlay(pMonitor, noOverlayUs);
         } else {
-            g_pDebugOverlay->renderDataNoOverlay(pMonitor, µs);
+            g_pDebugOverlay->renderDataNoOverlay(pMonitor, durationUs);
         }
     }
 }