Stop a debouncing config watcher

src/flexasio/FlexASIO/config.cpp

@@ -177,19 +177,25 @@ namespace flexasio {
 	ConfigLoader::Watcher::~Watcher() noexcept(false) {
 		Log() << "Stopping config watcher";
 		{
-			std::unique_lock lock(mutex);
-			stopRequested = true;
+			std::unique_lock lock(directoryMutex);
 			if (directory != INVALID_HANDLE_VALUE) {
 				Log() << "Cancelling any pending config directory operations";
 				if (::CancelIoEx(directory, NULL) == 0)
 					throw std::system_error(::GetLastError(), std::system_category(), "Unable to cancel directory watch operation");
 			}
 		}
+		stopSemaphore.release();
+
 		Log() << "Waiting for config watcher thread to finish";
 		thread.join();
+
 		Log() << "Joined config watcher thread";
 	}
 
+	void ConfigLoader::Watcher::CheckStopRequested(std::chrono::milliseconds waitFor = {}) {
+		if (stopSemaphore.try_acquire_for(waitFor)) throw StopRequested();
+	}
+
 	void ConfigLoader::Watcher::RunThread() {
 		Log() << "Config watcher thread running";
 
@@ -205,8 +211,7 @@ namespace flexasio {
 				// Another reason to debounce is that it might make it less likely we'll run into file locking issues.
 				// We do this by sleeping for a while, and getting rid of all events that occurred in the mean time.
 				Log() << "Sleeping for debounce";
-				// TODO: ideally the destructor should be able to cut this sleep short. This should be fairly trivial with std::condition_variable::wait_for().
-				::Sleep(250);
+				CheckStopRequested(/*waitFor=*/std::chrono::milliseconds(250));
 			}
 		}
 		catch (StopRequested) {}
@@ -237,13 +242,13 @@ namespace flexasio {
 				throw std::system_error(::GetLastError(), std::system_category(), "Unable to open config directory for watching");
 
 			{
-				std::unique_lock lock(mutex);
+				std::unique_lock lock(directoryMutex);
 				assert(directory == INVALID_HANDLE_VALUE);
 				directory = handle;
 			}
 			const auto directoryDeleter = [&](HANDLE handle) {
 				{
-					std::unique_lock lock(mutex);
+					std::unique_lock lock(directoryMutex);
 					assert(directory == handle);
 					directory = INVALID_HANDLE_VALUE;
 				}
@@ -261,10 +266,7 @@ namespace flexasio {
 
 			// Check for stop requests *after* we start watching, so that we correctly exit
 			// even if the destructor ran just before there was an I/O to cancel.
-			{
-				std::unique_lock lock(mutex);
-				if (stopRequested) throw StopRequested();
-			}
+			CheckStopRequested();
 
 			if (first) {
 				Log() << "Triggering initial config file event";
@@ -273,8 +275,7 @@ namespace flexasio {
 
 			if (OnVariant(operation.Await(),
 				[&](ConfigDirectoryWatchOperation::Aborted) -> bool {
-					std::unique_lock lock(mutex);
-					if (stopRequested) throw StopRequested();
+					CheckStopRequested();
 					throw std::runtime_error("Config directory watch operation was aborted, but we were not requested to stop");
 				},
 				[&](ConfigDirectoryWatchOperation::Overflow) {

src/flexasio/FlexASIO/config.h

@@ -9,6 +9,7 @@
 #include <optional>
 #include <regex>
 #include <string>
+#include <semaphore>
 #include <span>
 #include <thread>
 #include <variant>
@@ -108,6 +109,7 @@ namespace flexasio {
 			// We abuse exception handling to handle stop requests. This is a bit unorthodox, but
 			// it does make the code significantly simpler.
 			struct StopRequested final {};
+			void CheckStopRequested(std::chrono::milliseconds timeout);
 
 			void RunThread();
 			void TriggerConfigFileEventThenWait(OVERLAPPED*, std::span<std::byte> fileNotifyInformationBuffer);
@@ -117,10 +119,10 @@ namespace flexasio {
 			const ConfigLoader& configLoader;
 			const std::function<void()> onConfigChange;
 
-
-			std::mutex mutex;
-			bool stopRequested = false;
+			std::binary_semaphore stopSemaphore{0};
+			std::mutex directoryMutex;
 			HANDLE directory = INVALID_HANDLE_VALUE;
+
 			std::thread thread;
 		};