watchDog: check less often, and not at all when nothing would act

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

src/watchDog.cpp

@@ -27,6 +27,8 @@
 
 #include <unordered_map>
 #include <thread>
+#include <mutex>
+#include <condition_variable>
 #include <string>
 #include <iostream>
 #include <stdlib.h> // std::getenv
@@ -51,6 +53,20 @@
 		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 @@
 			[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<std::chrono::milliseconds>(currentTime - watchInfo.lastAlive).count() > watchInfo.maximumInterval.count())
-								{
-									_observers.notifyObserversMethod<Observer>(&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 @@
 	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,13 +196,52 @@
 		}
 	}
 
+	// 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<std::chrono::system_clock> 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<std::chrono::milliseconds>(currentTime - watchInfo.lastAlive).count() > watchInfo.maximumInterval.count())
+		{
+			_observers.notifyObserversMethod<Observer>(&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<std::string, WatchInfo>;
 
 	// Private members
 	std::mutex _lock{};
 	std::unordered_map<std::thread::id, WatchedMap> _watched{};
 	//WatchedMap _watched{};
 	bool _shouldTerminate{ false };
+	std::condition_variable _wakeCondition{};
 	std::thread _watchThread{};
 	Subject _observers{};
 };