fix(logger,config_watcher): align loader-lock leak paths (#76)

* fix(logger,config_watcher): align loader-lock leak paths with HookManager nothrow pattern

Replace static-vector emplace_back leaks with per-call new (std::nothrow)
heap cells in Logger::shutdown_internal and ~ConfigWatcher so the
noexcept destructor contract is honestly upheld under OOM rather than
turning a vector growth bad_alloc into std::terminate. Add the
previously missing pin_current_module() call to Logger's leak branch.
ConfigWatcher falls back to m_impl.release() if the heap cell allocation
fails, leaking the raw Impl pointer without invoking ~Impl.

Refs #75

* test(version): update expectations to 3.2.1

CMakeLists.txt project version bumped to 3.2.1; bring the version
macro asserts in lockstep so the suite stays green.

Author: tkhquang

AGENTS.md

@@ -256,14 +256,14 @@ PATH="/c/msys64/mingw64/bin:$PATH" ./build/mingw-debug/tests/DetourModKit_tests.
 | Module | Thread safety | Hot-path mechanism |
 |--------|--------------|-------------------|
 | Scanner | Stateless -- inherently safe | N/A (startup only) |
-| HookManager | `shared_mutex` (readers) / `unique_lock` (writers); two-phase shutdown (disable under shared lock, clear under exclusive lock); `m_mutator_gate` (shared_mutex) blocks new mutators (including all VMT operations) during teardown; CAS on `m_shutdown_called` serializes shutdown/remove_all_hooks; double-checked fast-fail on `m_shutdown_called` in all mutators; destructor fallback (when `DMK_Shutdown()` was not called) acquires `m_mutator_gate` exclusively, flips `m_shutdown_called`, drains readers via exclusive `m_hooks_mutex`, then clears the maps -- under loader lock it pins the module and swaps each map's contents into heap storage allocated via `new (std::nothrow)` so the storage outlives the destructor without ever draining, mirroring the leak-on-loader-lock discipline used in `Logger::shutdown_internal` | `shared_lock` for `with_inline_hook()` |
-| Logger | `atomic<shared_ptr>` for lock-free async reads; `shutdown_internal` is safe across repeated shutdown / enable_async_mode cycles: when the writer thread has to be detached under loader lock, the `shared_ptr<AsyncLogger>` is appended to a static `std::vector` rather than overwriting a single static slot, so prior handles are never dropped while their writer threads may still be running | Single atomic load on log level check |
+| HookManager | `shared_mutex` (readers) / `unique_lock` (writers); two-phase shutdown (disable under shared lock, clear under exclusive lock); `m_mutator_gate` (shared_mutex) blocks new mutators (including all VMT operations) during teardown; CAS on `m_shutdown_called` serializes shutdown/remove_all_hooks; double-checked fast-fail on `m_shutdown_called` in all mutators; destructor fallback (when `DMK_Shutdown()` was not called) acquires `m_mutator_gate` exclusively, flips `m_shutdown_called`, drains readers via exclusive `m_hooks_mutex`, then clears the maps -- under loader lock it pins the module and swaps each map's contents into heap storage allocated via `new (std::nothrow)` so the storage outlives the destructor without ever draining, mirroring the leak-on-loader-lock discipline used in `Logger::shutdown_internal` and `ConfigWatcher::~ConfigWatcher` | `shared_lock` for `with_inline_hook()` |
+| Logger | `atomic<shared_ptr>` for lock-free async reads; `shutdown_internal` is safe across repeated shutdown / enable_async_mode cycles: when the writer thread has to be detached under loader lock, the module is pinned and the `shared_ptr<AsyncLogger>` is moved into a per-call heap cell allocated via `new (std::nothrow)` rather than appended to a static `std::vector`, so the leak path keeps the noexcept destructor honest under OOM (returns nullptr instead of throwing `bad_alloc`) and prior handles are never dropped while their writer threads may still be running | Single atomic load on log level check |
 | AsyncLogger | Lock-free MPMC queue (Vyukov-style); post-join drain on shutdown (at most one message per producer can be lost in the nanosecond race between drain and force-zero -- accepted trade-off to avoid atomic overhead on every enqueue); timestamp caching in write batches | Atomic sequence numbers per slot |
 | InputPoller | Atomic `active_states_[]` array | `memory_order_relaxed` load per binding |
 | InputManager | `mutex` for lifecycle, `atomic<InputPoller*>` for reads | Lock-free `is_binding_active()` |
 | Memory cache | Sharded `SRWLOCK` + epoch-based shutdown | Shared reader locks per shard |
 | Config | `mutex` for registration; deferred setter invocation outside lock (no reentrancy guard needed -- setters may call back into Config); `reload()` re-runs the registered items against the stashed INI path using the same deferred pattern and short-circuits on FNV-1a 64 hash match of the on-disk bytes to skip no-op reloads; bytes are read once per load/reload and fed to `CSimpleIniA::LoadData`, so the cached hash and the parsed INI state are guaranteed to reflect the same file snapshot (no TOCTOU between hash and parse); `enable_auto_reload()` owns a `ConfigWatcher` behind a separate `std::mutex` so start/stop transitions do not contend with registration traffic; setters invoked by the watcher run on the watcher thread, setters invoked by the reload hotkey run on a dedicated `ReloadServicer` thread (lazily started on first `register_reload_hotkey`, torn down in `clear_registered_items()`) so the `InputManager` poll thread never blocks on INI parsing; the servicer's press-request path takes its internal `m_mutex` around the predicate store before `cv.notify_one` to close the lost-wakeup window; all setters must be reentrant and thread-safe | N/A (startup only) |
-| ConfigWatcher | One `StoppableWorker` per instance; worker opens the parent directory with `FILE_FLAG_BACKUP_SEMANTICS` and `FILE_FLAG_OVERLAPPED`, then pumps `ReadDirectoryChangesW` via `GetOverlappedResultEx` with a 100 ms timeout so `stop_token` is observed promptly; debounce uses `steady_clock`; filename match is case-insensitive; `start()` and `stop()` are idempotent and serialized by an internal `std::mutex` | 100 ms `GetOverlappedResultEx` pump; idle CPU ~0 |
+| ConfigWatcher | One `StoppableWorker` per instance; worker opens the parent directory with `FILE_FLAG_BACKUP_SEMANTICS` and `FILE_FLAG_OVERLAPPED`, then pumps `ReadDirectoryChangesW` via `GetOverlappedResultEx` with a 100 ms timeout so `stop_token` is observed promptly; debounce uses `steady_clock`; filename match is case-insensitive; `start()` and `stop()` are idempotent and serialized by an internal `std::mutex`; under loader lock the destructor pins the module, requests stop on the worker, and moves `Impl` into a per-call heap cell allocated via `new (std::nothrow)` (with a `release()` fallback on OOM that leaks the raw pointer instead of running `~Impl`) so the noexcept destructor stays honest, mirroring the `Logger::shutdown_internal` discipline | 100 ms `GetOverlappedResultEx` pump; idle CPU ~0 |
 | EventDispatcher | Lock-free `emit()` / `emit_safe()` via `std::atomic<std::shared_ptr<const std::vector<Entry>>>` snapshot (copy-on-write publish, acquire-load on read); zero-subscriber fast path skips the snapshot load via an atomic handler counter; writers serialize on a small `std::mutex` that never touches the emit hot path; thread-local reentrancy guard rejects subscribe/unsubscribe from within handlers so the no-mutation-during-emit invariant holds; `emit()` propagates handler exceptions, `emit_safe()` catches and skips them | Atomic acquire-load of a `shared_ptr` snapshot plus linear iteration over a contiguous vector; no reader lock |
 | Profiler | Lock-free ring buffer via atomic `fetch_add` on write position; odd/even sequence counter per sample slot prevents torn reads during concurrent export -- the sequence is opened and closed with unconditional `fetch_add` (never a load-then-store) so concurrent producers racing on the same slot cannot roll the counter backwards; `DMK_PROFILE_SCOPE(name)` requires `name` to be a string literal, enforced at compile time by a `ScopedProfile` constructor that only binds to `const char (&)[N]` | Single atomic increment + sequence-guarded field writes per sample |
 

CMakeLists.txt

@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.25)
 
-project(DetourModKit VERSION 3.2.0 LANGUAGES CXX)
+project(DetourModKit VERSION 3.2.1 LANGUAGES CXX)
 
 # --- Standard and Compiler Options ---
 set(CMAKE_CXX_STANDARD 23)
@@ -146,9 +146,9 @@ configure_file(
 # For brand new files, a manual CMake re-run might still be needed.
 #
 # Notable modules picked up here (alphabetical, not exhaustive):
-#   async_logger, bootstrap, config, config_watcher, event_dispatcher,
-#   filesystem, hook_manager, input, logger, memory, profiler,
-#   scanner, win_file_stream, worker.
+# async_logger, bootstrap, config, config_watcher, event_dispatcher,
+# filesystem, hook_manager, input, logger, memory, profiler,
+# scanner, win_file_stream, worker.
 file(GLOB DETOURMODKIT_SOURCE_FILES
   CONFIGURE_DEPENDS
   "src/*.cpp"
@@ -201,6 +201,7 @@ else()
   # after -O3 on the scanner's compile line is the effective flag because
   # GCC and Clang resolve repeated -O levels by taking the last one.
   set(_dmk_scanner_src "${CMAKE_CURRENT_SOURCE_DIR}/src/scanner.cpp")
+
   if(EXISTS "${_dmk_scanner_src}")
     set_source_files_properties("${_dmk_scanner_src}"
       TARGET_DIRECTORY DetourModKit
@@ -483,8 +484,10 @@ if(DMK_BUILD_TESTS OR DMK_BUILD_BENCHMARKS)
     message(STATUS "Building unit tests...")
     enable_testing()
   endif()
+
   if(DMK_BUILD_BENCHMARKS)
     message(STATUS "Building benchmarks...")
   endif()
+
   add_subdirectory(tests)
 endif()

src/config_watcher.cpp

@@ -20,9 +20,11 @@
 #include <future>
 #include <memory>
 #include <mutex>
+#include <new>
 #include <optional>
 #include <string>
 #include <string_view>
+#include <type_traits>
 #include <utility>
 #include <vector>
 
@@ -174,9 +176,9 @@ namespace DetourModKit
             // 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.
+            // stop, then leak the entire Impl onto the heap so it outlives
+            // the destructor. The same discipline as HookManager::~HookManager
+            // and Logger::shutdown_internal.
             detail::pin_current_module();
 
             if (m_impl->worker)
@@ -189,16 +191,33 @@ namespace DetourModKit
                 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));
+            // Per-call heap leak: each invocation allocates its own cell,
+            // so prior leaked Impls are never overwritten and the leak is
+            // bounded by one cell per ~ConfigWatcher-under-loader-lock
+            // call. 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.
+            //
+            // new (std::nothrow) keeps this noexcept destructor honest by
+            // returning nullptr on OOM rather than turning a container
+            // emplace_back bad_alloc into std::terminate. On allocation
+            // failure, fall back to releasing the unique_ptr so the Impl
+            // storage is leaked directly without invoking ~Impl (which
+            // would tear down the detached StoppableWorker -- safe under
+            // a normal join, but not under loader lock).
+            static_assert(std::is_nothrow_move_constructible_v<std::unique_ptr<Impl>>,
+                          "Leak cell must be nothrow-move-constructible to keep ~ConfigWatcher noexcept honest.");
+
+            if (auto *leaked = new (std::nothrow)
+                    std::unique_ptr<Impl>(std::move(m_impl)))
+            {
+                static_cast<void>(leaked);
+            }
+            else
+            {
+                static_cast<void>(m_impl.release());
+            }
             return;
         }
 

src/logger.cpp

@@ -11,9 +11,10 @@
 #include <chrono>
 #include <iomanip>
 #include <iostream>
+#include <new>
 #include <stdexcept>
 #include <array>
-#include <vector>
+#include <type_traits>
 
 namespace DetourModKit
 {
@@ -200,9 +201,9 @@ namespace DetourModKit
 
         // If the writer thread was detached under loader lock, it may still
         // be accessing AsyncLogger members (queue_, flush_mutex_, etc.).
-        // Transfer ownership to a static so the object outlives the detached
-        // thread. The pinned module keeps code pages valid; this keeps the
-        // heap-allocated state valid.
+        // Transfer ownership to permanent heap storage so the object
+        // outlives the detached thread; pin the module so the code pages
+        // it executes from also remain mapped.
         //
         // The transfer is unconditional when loader lock is held: concurrent
         // log() callers may still own temporary shared_ptrs obtained from
@@ -211,14 +212,24 @@ namespace DetourModKit
         // a temporary outlives us would let the last temporary's destructor
         // race the detached writer thread.
         //
-        // The storage is append-only: a process that re-attaches after a
-        // shutdown (e.g. hot-reload) and hits loader lock again must not
-        // drop the prior handle, because its writer thread may still be
-        // accessing the old AsyncLogger state.
+        // The leak is per-call and append-only: each invocation allocates
+        // its own heap cell, so a process that re-attaches after shutdown
+        // (e.g. hot-reload) and hits loader lock again cannot drop a prior
+        // handle whose writer thread may still be running. Mirrors the
+        // HookManager loader-lock discipline: new (std::nothrow) keeps the
+        // noexcept destructor honest by returning nullptr on OOM rather
+        // than turning a std::vector::emplace_back bad_alloc into
+        // std::terminate inside this noexcept context.
         if (local_logger && detail::is_loader_lock_held())
         {
-            static std::vector<std::shared_ptr<AsyncLogger>> s_leaked_loggers;
-            s_leaked_loggers.emplace_back(std::move(local_logger));
+            static_assert(std::is_nothrow_move_constructible_v<std::shared_ptr<AsyncLogger>>,
+                          "Leak cell must be nothrow-move-constructible to keep ~Logger noexcept honest.");
+
+            detail::pin_current_module();
+
+            auto *leaked = new (std::nothrow)
+                std::shared_ptr<AsyncLogger>(std::move(local_logger));
+            static_cast<void>(leaked);
         }
 
         {

tests/test_config_watcher.cpp

@@ -4,9 +4,11 @@
 #include <chrono>
 #include <filesystem>
 #include <fstream>
+#include <memory>
 #include <process.h>
 #include <string>
 #include <thread>
+#include <type_traits>
 
 #include <windows.h>
 
@@ -15,6 +17,17 @@
 using namespace DetourModKit;
 using namespace std::chrono_literals;
 
+// The loader-lock fallback in ~ConfigWatcher heap-allocates a
+// std::unique_ptr<Impl> via new (std::nothrow) and leaks the cell to
+// outlive the destructor. Impl is a private nested type, so this
+// assertion guards the unique_ptr leak-cell pattern at the type level
+// rather than referencing Impl directly: if std::unique_ptr ever ceased
+// to be nothrow-move-constructible, the leak cell could no longer be
+// constructed from a noexcept context without risking std::terminate.
+static_assert(std::is_nothrow_move_constructible_v<std::unique_ptr<int>>,
+              "std::unique_ptr must remain nothrow-move-constructible for the "
+              "ConfigWatcher loader-lock leak path to keep ~ConfigWatcher noexcept honest.");
+
 namespace
 {
     class ConfigWatcherTest : public ::testing::Test
@@ -427,8 +440,9 @@ namespace
 // 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.
+// joined, the Impl is moved into a per-call heap cell allocated via
+// new (std::nothrow) that outlives the destructor, and the watcher does not
+// deadlock.
 namespace DetourModKit::detail
 {
     extern bool (*g_config_watcher_loader_lock_override)() noexcept;
@@ -505,9 +519,11 @@ namespace
 
     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).
+        // Confirms the per-call heap leak path accepts multiple invocations
+        // without tripping any single-slot overwrite hazards (the bug pattern
+        // that motivated #69's Logger::shutdown_internal fix). Each
+        // teardown allocates its own cell via new (std::nothrow), so prior
+        // leaked Impls are never overwritten.
         for (int i = 0; i < 3; ++i)
         {
             ConfigWatcher watcher(m_ini_path.string(), 50ms, []() {});

tests/test_logger.cpp

@@ -1,8 +1,10 @@
 #include <gtest/gtest.h>
 #include <fstream>
 #include <filesystem>
+#include <memory>
 #include <thread>
 #include <chrono>
+#include <type_traits>
 #include <windows.h>
 #include <atomic>
 
@@ -11,6 +13,15 @@
 
 using namespace DetourModKit;
 
+// The loader-lock fallback in Logger::shutdown_internal heap-allocates a
+// std::shared_ptr<AsyncLogger> via new (std::nothrow) and leaks the cell
+// to outlive the destructor. Guard the leak cell's move constructor stays
+// noexcept so the call cannot turn the noexcept ~Logger contract into
+// std::terminate via a thrown move.
+static_assert(std::is_nothrow_move_constructible_v<std::shared_ptr<AsyncLogger>>,
+              "std::shared_ptr<AsyncLogger> must be nothrow-move-constructible "
+              "for the Logger loader-lock leak path to keep ~Logger noexcept honest.");
+
 class LoggerTest : public ::testing::Test
 {
 protected:

tests/test_version.cpp

@@ -10,20 +10,21 @@ namespace
     {
         EXPECT_EQ(DMK_VERSION_MAJOR, 3);
         EXPECT_EQ(DMK_VERSION_MINOR, 2);
-        EXPECT_EQ(DMK_VERSION_PATCH, 0);
+        EXPECT_EQ(DMK_VERSION_PATCH, 1);
     }
 
     TEST(VersionTest, VersionStringMatchesMacros)
     {
-        EXPECT_STREQ(DMK_VERSION_STRING, "3.2.0");
+        EXPECT_STREQ(DMK_VERSION_STRING, "3.2.1");
     }
 
     TEST(VersionTest, AtLeastComparisonsAreCorrect)
     {
+        EXPECT_TRUE(DMK_VERSION_AT_LEAST(3, 2, 1));
         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, 2, 2));
         EXPECT_FALSE(DMK_VERSION_AT_LEAST(3, 3, 0));
         EXPECT_FALSE(DMK_VERSION_AT_LEAST(4, 0, 0));
     }