From cafe6a4ff728a6c66f5646f5c35182f81e941300 Mon Sep 17 00:00:00 2001 From: Luke Howard Date: Mon, 14 Sep 2026 14:48:54 +1000 Subject: [PATCH] watchDog: check less often, and not at all when nothing would act The watchdog woke every 10 msec, and on every pass asked for each watch whether a debugger is attached, which on Linux opens and reads /proc/self/status. With the pcap interface's state machines running, that was about 100 opens and reads of that file a second, and as many wakeups, to enforce timeouts of 500 msec and more. Check the watches every 100 msec, look for a debugger at most once a second, and wait on a condition variable so that shutting down doesn't have to wait out the interval. The terminate flag is now only read and written under the lock. Without DEBUG or COMPILE_AVDECC_ASSERT defined, a missed watch doesn't assert and is only reported to observers, so with none registered there is nothing to check: the thread then waits until an observer registers or the watchdog terminates. registerObserver wakes the thread without taking the lock, so that an observer can still register from within a notification, and the idle thread looks again every 10 seconds in case it missed that wakeup. Once an observer is registered, checking resumes, debugger check included, so a process stopped in a debugger still doesn't read as a deadlock. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012i5Dd3gCyYqp4X7vxhSmxh --- CHANGELOG-avdecc.md | 1 + src/watchDog.cpp | 123 ++++++++++++++++++++++++++++++++------------ 2 files changed, 91 insertions(+), 33 deletions(-) diff --git a/CHANGELOG-avdecc.md b/CHANGELOG-avdecc.md index 637daf91..fc82f0fb 100644 --- a/CHANGELOG-avdecc.md +++ b/CHANGELOG-avdecc.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Better debugger support for MacOs and Linux builds - [Crash in MacOSNative ProtocolInterface when an entity goes offline while an AECP command is inflight (semaphore disposed while in use)](https://github.com/L-Acoustics/avdecc/issues/177) - MacOSNative ProtocolInterface not invoking the result handler when the macOS framework refuses to send an AECP or ACMP command +- WatchDog waking every 10 msec and looking for a debugger for every watch (reading /proc/self/status on Linux), even when nothing would act on a missed watch ## [4.3.1] - 2025-12-19 ### Added diff --git a/src/watchDog.cpp b/src/watchDog.cpp index 8fb70db9..d39d983a 100644 --- a/src/watchDog.cpp +++ b/src/watchDog.cpp @@ -27,6 +27,8 @@ #include #include +#include +#include #include #include #include // std::getenv @@ -51,6 +53,20 @@ class WatchDogImpl final : public WatchDog bool ignore{ false }; }; + // Interval between two checks of all watches (the shortest maximumInterval registered is 500 msec) + static constexpr auto CheckInterval = std::chrono::milliseconds{ 100u }; + // Interval between two checks for a debugger, which can be costly (on Linux, it reads /proc/self/status) + static constexpr auto DebuggerCheckInterval = std::chrono::milliseconds{ 1000u }; + // Without asserts compiled in, a missed watch is only reported to observers, so with none registered nothing needs checking. + // registerObserver wakes the idle thread without taking the lock (so that an observer can register from a notification), + // so the idle thread also looks again this often, in case it missed that wakeup + static constexpr auto IdleInterval = std::chrono::milliseconds{ 10000u }; +#if defined(DEBUG) || defined(COMPILE_AVDECC_ASSERT) + static constexpr auto IsAssertCompiled = true; +#else // !DEBUG && !COMPILE_AVDECC_ASSERT + static constexpr auto IsAssertCompiled = false; +#endif // DEBUG || COMPILE_AVDECC_ASSERT + public: WatchDogImpl() noexcept { @@ -59,50 +75,49 @@ class WatchDogImpl final : public WatchDog [this] { utils::setCurrentThreadName("avdecc::watchDog"); + + auto isDebuggerPresent = false; + auto lastDebuggerCheck = std::chrono::steady_clock::time_point{}; + + auto lock = std::unique_lock{ _lock }; while (!_shouldTerminate) { - // Check all watch + // Nothing would act on a missed watch: wait for an observer rather than check + if (!IsAssertCompiled && _observers.countObservers() == 0) { - auto const lg = std::lock_guard{ _lock }; - - auto const currentTime = std::chrono::system_clock::now(); - for (auto& [threadId, watchedMap] : _watched) - { - for (auto& [name, watchInfo] : watchedMap) + _wakeCondition.wait_for(lock, IdleInterval, + [this] { - // If debugger is present, update the last alive time and don't check the timeout - if (utils::isDebuggerPresent()) - { - watchInfo.lastAlive = currentTime; - } - - // Check if we timed out - if (!watchInfo.ignore && std::chrono::duration_cast(currentTime - watchInfo.lastAlive).count() > watchInfo.maximumInterval.count()) - { - _observers.notifyObserversMethod(&Observer::onIntervalExceeded, name, watchInfo.maximumInterval); - - // Only print message if "AVDECC_NO_WATCHDOG_ASSERT" is not defined - if (std::getenv("AVDECC_NO_WATCHDOG_ASSERT") == nullptr) - { - auto stream = std::stringstream{}; - stream << "WatchDog event '" << name << "' exceeded the maximum allowed time (ThreadId: 0x" << std::hex << watchInfo.threadId << "). Deadlock?"; - AVDECC_ASSERT(false, stream.str()); - } - - watchInfo.ignore = true; - } - } - } + return _shouldTerminate || _observers.countObservers() != 0; + }); + continue; + } + + // Look for a debugger once in a while, rather than for every watch on every check + if (!_watched.empty() && std::chrono::steady_clock::now() - lastDebuggerCheck >= DebuggerCheckInterval) + { + isDebuggerPresent = utils::isDebuggerPresent(); + lastDebuggerCheck = std::chrono::steady_clock::now(); } - // Wait a little bit so we don't burn the CPU - std::this_thread::sleep_for(std::chrono::milliseconds(10)); + checkWatches(isDebuggerPresent); + + // Wait until the next check, unless asked to terminate first + _wakeCondition.wait_for(lock, CheckInterval, + [this] + { + return _shouldTerminate; + }); } }); } virtual ~WatchDogImpl() noexcept override { // Notify the thread we are shutting down - _shouldTerminate = true; + { + auto const lg = std::scoped_lock{ _lock }; + _shouldTerminate = true; + } + _wakeCondition.notify_all(); // Wait for the thread to complete its pending tasks if (_watchThread.joinable()) @@ -120,6 +135,9 @@ class WatchDogImpl final : public WatchDog virtual void registerObserver(Observer* const observer) noexcept override { _observers.registerObserver(observer); + + // Wake the thread, which may be idle for want of an observer (not under the lock, see IdleInterval) + _wakeCondition.notify_all(); } virtual void unregisterObserver(Observer* const observer) noexcept override @@ -178,6 +196,44 @@ class WatchDogImpl final : public WatchDog } } + // Checks all watches, with the lock held + void checkWatches(bool const isDebuggerPresent) noexcept + { + auto const currentTime = std::chrono::system_clock::now(); + for (auto& [threadId, watchedMap] : _watched) + { + for (auto& [name, watchInfo] : watchedMap) + { + checkWatch(name, watchInfo, currentTime, isDebuggerPresent); + } + } + } + + void checkWatch(std::string const& name, WatchInfo& watchInfo, std::chrono::time_point const currentTime, bool const isDebuggerPresent) noexcept + { + // If debugger is present, update the last alive time and don't check the timeout + if (isDebuggerPresent) + { + watchInfo.lastAlive = currentTime; + } + + // Check if we timed out + if (!watchInfo.ignore && std::chrono::duration_cast(currentTime - watchInfo.lastAlive).count() > watchInfo.maximumInterval.count()) + { + _observers.notifyObserversMethod(&Observer::onIntervalExceeded, name, watchInfo.maximumInterval); + + // Only print message if "AVDECC_NO_WATCHDOG_ASSERT" is not defined + if (std::getenv("AVDECC_NO_WATCHDOG_ASSERT") == nullptr) + { + auto stream = std::stringstream{}; + stream << "WatchDog event '" << name << "' exceeded the maximum allowed time (ThreadId: 0x" << std::hex << watchInfo.threadId << "). Deadlock?"; + AVDECC_ASSERT(false, stream.str()); + } + + watchInfo.ignore = true; + } + } + using WatchedMap = std::unordered_map; // Private members @@ -185,6 +241,7 @@ class WatchDogImpl final : public WatchDog std::unordered_map _watched{}; //WatchedMap _watched{}; bool _shouldTerminate{ false }; + std::condition_variable _wakeCondition{}; std::thread _watchThread{}; Subject _observers{}; };