From 48fde62a6fe1cf4083b0c775f60ee21f2554763e Mon Sep 17 00:00:00 2001 From: Tom Englund Date: Sat, 10 Aug 2024 16:40:09 +0200 Subject: [PATCH] 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. --- src/Compositor.cpp | 3 +++ src/helpers/Watchdog.cpp | 13 ++++++------- src/helpers/Watchdog.hpp | 9 +++++---- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/Compositor.cpp b/src/Compositor.cpp index e3a347fd9dd2..b2dc2cf01872 100644 --- a/src/Compositor.cpp +++ b/src/Compositor.cpp @@ -552,6 +552,9 @@ void CCompositor::initManagers(eManagersInitStage stage) { g_pConfigManager->init(); g_pWatchdog = std::make_unique(); // 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(); diff --git a/src/helpers/Watchdog.cpp b/src/helpers/Watchdog.cpp index b9f654daf1c2..c7ff648ba5f5 100644 --- a/src/helpers/Watchdog.cpp +++ b/src/helpers/Watchdog.cpp @@ -18,15 +18,14 @@ CWatchdog::CWatchdog() { m_pWatchdog = std::make_unique([this] { static auto PTIMEOUT = CConfigValue("debug:watchdog_timeout"); - while (1337) { - std::unique_lock lk(m_mWatchdogMutex); + m_bWatchdogInitialized = true; + while (!m_bExitThread) { + std::unique_lock 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 index 7bb499d69318..a21e692e9ef1 100644 --- a/src/helpers/Watchdog.hpp +++ b/src/helpers/Watchdog.hpp @@ -14,18 +14,19 @@ class CWatchdog { void startWatching(); void endWatching(); + std::atomic 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 m_bWatching = false; + std::atomic m_bWillWatch = false; std::unique_ptr m_pWatchdog; std::mutex m_mWatchdogMutex; - bool m_bNotified = false; - bool m_bExitThread = false; + std::atomic m_bNotified = false; + std::atomic m_bExitThread = false; std::condition_variable m_cvWatchdogCondition; };