chore: bump version to 3.2.0 (#74)

Author: tkhquang

CMakeLists.txt

@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.25)
 
-project(DetourModKit VERSION 3.1.0 LANGUAGES CXX)
+project(DetourModKit VERSION 3.2.0 LANGUAGES CXX)
 
 # --- Standard and Compiler Options ---
 set(CMAKE_CXX_STANDARD 23)

include/DetourModKit/config.hpp

@@ -313,6 +313,13 @@ namespace DetourModKit
          * @brief Stops the filesystem watcher started by enable_auto_reload().
          * @details Idempotent. Returns only once the watcher thread has exited
          *          (or been detached under the Windows loader lock).
+         * @note When invoked from inside an on_reload callback (i.e. on the
+         *       watcher thread itself) this is a no-op: joining the worker
+         *       from its own thread would raise
+         *       std::system_error(resource_deadlock_would_occur). The error
+         *       is logged and the watcher remains running. Tear the watcher
+         *       down from a different thread, e.g. by posting the disable
+         *       request to a deferred shutdown hook.
          */
         void disable_auto_reload() noexcept;
 

include/DetourModKit/event_dispatcher.hpp

@@ -478,6 +478,8 @@ namespace DetourModKit
         // the class-level doc for details.
         [[nodiscard]] int &emitting_depth() const noexcept
         {
+            // Shared across all dispatcher instances on the same thread.
+            // The reentrancy guard is per-thread (intentional), not per-dispatcher.
             thread_local int depth{0};
             return depth;
         }

include/DetourModKit/memory.hpp

@@ -143,6 +143,10 @@ namespace DetourModKit
          *          zero overhead on the success path. On MinGW, falls back to a single
          *          VirtualQuery guard before dereferencing (no cache interaction).
          *          Suitable for hot paths that already manage their own error recovery.
+         * @note On MinGW the implementation additionally probes the cache via a
+         *       non-blocking try_lock_shared before falling back to VirtualQuery,
+         *       so cache hits avoid a syscall. This is invisible to callers; the
+         *       function still exposes no caller-observable cache state.
          * @param base The base address to read from.
          * @param offset Byte offset added to base before dereferencing.
          * @return The pointer-sized value at the address, or 0 if the read faults.

src/config_watcher.cpp

@@ -7,6 +7,7 @@
 
 #include "DetourModKit/logger.hpp"
 #include "DetourModKit/worker.hpp"
+#include "platform.hpp"
 
 #include <windows.h>
 
@@ -27,13 +28,33 @@
 
 namespace DetourModKit
 {
+    namespace detail
+    {
+        // Test-only override for is_loader_lock_held(). When non-null the
+        // ConfigWatcher destructor consults this hook instead of the real
+        // PEB-based detection, letting the test suite exercise the
+        // detach-and-leak branch from user code. Defined as a plain
+        // function pointer because the override is set/cleared on a single
+        // thread inside a test fixture.
+        bool (*g_config_watcher_loader_lock_override)() noexcept = nullptr;
+    } // namespace detail
+
     namespace
     {
         constexpr DWORD kNotifyFilter =
             FILE_NOTIFY_CHANGE_LAST_WRITE |
             FILE_NOTIFY_CHANGE_FILE_NAME |
             FILE_NOTIFY_CHANGE_SIZE;
 
+        bool loader_lock_held_for_watcher() noexcept
+        {
+            if (auto *override_fn = detail::g_config_watcher_loader_lock_override)
+            {
+                return override_fn();
+            }
+            return detail::is_loader_lock_held();
+        }
+
         // Sized so bursty editor saves do not overflow a single call while
         // still fitting comfortably on the worker's stack.
         constexpr DWORD kBufferBytes = 16 * 1024;
@@ -146,6 +167,41 @@ namespace DetourModKit
 
     ConfigWatcher::~ConfigWatcher() noexcept
     {
+        if (m_impl && loader_lock_held_for_watcher())
+        {
+            // Under loader lock (FreeLibrary path): joining the watcher
+            // would deadlock against ReadDirectoryChangesW's I/O completion,
+            // and tearing down Impl would invalidate the worker_thread_id
+            // pointer the detached lambda still references. Pin the module
+            // so trampoline and worker code pages remain mapped, request
+            // stop, then leak the entire Impl into a static vector that
+            // outlives the destructor. The same discipline as
+            // HookManager::~HookManager and Logger::shutdown_internal.
+            detail::pin_current_module();
+
+            if (m_impl->worker)
+            {
+                // shutdown() takes its own loader-lock branch: it requests
+                // stop and detaches the std::jthread (no join), then sets
+                // joined_ so the eventual ~StoppableWorker run during
+                // static teardown short-circuits without trying to join a
+                // detached handle.
+                m_impl->worker->shutdown();
+            }
+
+            // Releasing into the leak list keeps Impl alive for the rest
+            // of the process. The detached worker thread holds raw
+            // pointers and references into Impl members (worker_thread_id,
+            // captured strings); they must stay valid until the OS thread
+            // either observes the stop_token and exits or the process
+            // tears down. Either way the leaked storage is bounded:
+            // ~ConfigWatcher under loader lock runs at most once per
+            // Config-owned watcher per process.
+            static std::vector<std::unique_ptr<Impl>> s_leaked_impls;
+            s_leaked_impls.emplace_back(std::move(m_impl));
+            return;
+        }
+
         stop();
     }
 

tests/test_config_watcher.cpp

@@ -414,11 +414,110 @@ namespace
 
     TEST_F(ConfigWatcherTest, Construct_InvalidPath_StartReturnsFalse)
     {
-        const std::filesystem::path missing =
-            m_temp_dir / "nonexistent_subdir" / "file.ini";
-
-        ConfigWatcher watcher(missing.string(), 100ms, []() {});
+        ConfigWatcher watcher(
+            (m_temp_dir / "nonexistent_subdir" / "file.ini").string(),
+            100ms, []() {});
         EXPECT_FALSE(watcher.start());
         EXPECT_FALSE(watcher.is_running());
     }
 } // namespace
+
+// Loader-lock detach tests. The real loader-lock branch (detected by reading
+// the PEB inside DllMain) cannot be reached from user code in a normal test
+// process, so the runtime exposes a test-only function pointer override that
+// reports "loader lock held" on demand. These tests exercise the leak-on-
+// loader-lock branch in ~ConfigWatcher: the worker is detached instead of
+// joined, the Impl is moved into a static vector that outlives the
+// destructor, and the watcher does not deadlock.
+namespace DetourModKit::detail
+{
+    extern bool (*g_config_watcher_loader_lock_override)() noexcept;
+} // namespace DetourModKit::detail
+
+namespace
+{
+    using DetourModKit::detail::g_config_watcher_loader_lock_override;
+
+    bool always_true_loader_lock() noexcept
+    {
+        return true;
+    }
+
+    class ConfigWatcherLoaderLockTest : public ConfigWatcherTest
+    {
+    protected:
+        void TearDown() override
+        {
+            g_config_watcher_loader_lock_override = nullptr;
+            ConfigWatcherTest::TearDown();
+        }
+    };
+
+    TEST_F(ConfigWatcherLoaderLockTest, DestructorWithoutLoaderLockJoinsCleanly)
+    {
+        std::atomic<int> hits{0};
+        const auto t_start = std::chrono::steady_clock::now();
+        {
+            ConfigWatcher watcher(m_ini_path.string(), 50ms,
+                                  [&hits]()
+                                  { hits.fetch_add(1); });
+            ASSERT_TRUE(watcher.start());
+            ASSERT_TRUE(wait_until([&]()
+                                   { return watcher.is_running(); },
+                                   1s));
+        }
+        const auto elapsed = std::chrono::steady_clock::now() - t_start;
+
+        // Without the loader-lock override the destructor takes the normal
+        // join path. A clean join completes well under a second; a hang
+        // (e.g. a regression that joined under loader lock) would blow past
+        // the GetOverlappedResultEx pump timeout repeatedly.
+        EXPECT_LT(elapsed, std::chrono::seconds(3));
+    }
+
+    TEST_F(ConfigWatcherLoaderLockTest, DestructorUnderLoaderLockDoesNotHang)
+    {
+        std::atomic<int> hits{0};
+        const auto t_start = std::chrono::steady_clock::now();
+        {
+            ConfigWatcher watcher(m_ini_path.string(), 50ms,
+                                  [&hits]()
+                                  { hits.fetch_add(1); });
+            ASSERT_TRUE(watcher.start());
+            ASSERT_TRUE(wait_until([&]()
+                                   { return watcher.is_running(); },
+                                   1s));
+
+            // Flip the override on so ~ConfigWatcher takes the leak branch.
+            // The detach path must not block, must not call join(), and
+            // must keep the worker's captured pointers valid by leaking
+            // the Impl into a static vector.
+            g_config_watcher_loader_lock_override = &always_true_loader_lock;
+        }
+        const auto elapsed = std::chrono::steady_clock::now() - t_start;
+
+        // Under loader lock the destructor returns essentially immediately:
+        // request_stop on the worker, detach, leak. The OS thread continues
+        // running but no longer blocks the destructor.
+        EXPECT_LT(elapsed, std::chrono::seconds(2))
+            << "Loader-lock detach branch must not join the worker";
+    }
+
+    TEST_F(ConfigWatcherLoaderLockTest, MultipleLoaderLockTeardownsAreSafe)
+    {
+        // Confirms the static leak vector accepts multiple entries without
+        // tripping any single-slot overwrite hazards (the bug pattern that
+        // motivated #69's Logger::shutdown_internal fix).
+        for (int i = 0; i < 3; ++i)
+        {
+            ConfigWatcher watcher(m_ini_path.string(), 50ms, []() {});
+            ASSERT_TRUE(watcher.start());
+            ASSERT_TRUE(wait_until([&]()
+                                   { return watcher.is_running(); },
+                                   1s));
+            g_config_watcher_loader_lock_override = &always_true_loader_lock;
+            // Watcher destructor on scope exit takes the leak path.
+        }
+        SUCCEED();
+    }
+} // namespace

tests/test_version.cpp

@@ -0,0 +1,38 @@
+#include <gtest/gtest.h>
+
+#include "DetourModKit/version.hpp"
+
+#include <cstring>
+
+namespace
+{
+    TEST(VersionTest, MacrosMatchProjectVersion)
+    {
+        EXPECT_EQ(DMK_VERSION_MAJOR, 3);
+        EXPECT_EQ(DMK_VERSION_MINOR, 2);
+        EXPECT_EQ(DMK_VERSION_PATCH, 0);
+    }
+
+    TEST(VersionTest, VersionStringMatchesMacros)
+    {
+        EXPECT_STREQ(DMK_VERSION_STRING, "3.2.0");
+    }
+
+    TEST(VersionTest, AtLeastComparisonsAreCorrect)
+    {
+        EXPECT_TRUE(DMK_VERSION_AT_LEAST(3, 2, 0));
+        EXPECT_TRUE(DMK_VERSION_AT_LEAST(3, 1, 0));
+        EXPECT_TRUE(DMK_VERSION_AT_LEAST(2, 0, 0));
+        EXPECT_FALSE(DMK_VERSION_AT_LEAST(3, 2, 1));
+        EXPECT_FALSE(DMK_VERSION_AT_LEAST(3, 3, 0));
+        EXPECT_FALSE(DMK_VERSION_AT_LEAST(4, 0, 0));
+    }
+
+    TEST(VersionTest, EncodedVersionMatchesComponents)
+    {
+        EXPECT_EQ(DMK_VERSION,
+                  DMK_MAKE_VERSION(DMK_VERSION_MAJOR,
+                                   DMK_VERSION_MINOR,
+                                   DMK_VERSION_PATCH));
+    }
+} // namespace