fix: harden lifecycle and concurrency paths across core modules (#115)

* fix: harden lifecycle and concurrency paths across core modules

- logger: loader-lock-safe disable_async_mode leak path with OOM fallbacks
- hook_manager: reject reentrant mutators/teardown from with_* callbacks; defer batch-toggle logging outside locks
- bootstrap: re-arm detach gate so attach/detach cycles are re-entrant
- worker: read liveness from atomics and signal a copied stop_source
- input_intercept: reconcile the wndproc predecessor against the swap return

* fix: address review feedback and de-flake DropOldest drop-count test

- bootstrap: stop the Config auto-reload watcher before clearing the registry on logic-DLL hot-unload
- hook_manager: warn against nested with_*/try_with_* accessors inside callbacks
- test(worker): latch pollers before shutdown so the concurrency window is exercised
- test(async_logger): force DropOldest overflow with a 100-message burst to remove a timing flake

Author: tkhquang

AGENTS.md

@@ -355,8 +355,8 @@ 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 | SRWLOCK-backed reader/writer locks; two-phase shutdown (disable under shared lock, clear under exclusive lock), both phases walking `m_hook_creation_order` in reverse so inline/mid hooks layered on one address unwind newest-first instead of in bucket order; `m_mutator_gate` 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` SRWLOCK reader 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 |
+| HookManager | SRWLOCK-backed reader/writer locks; two-phase shutdown (disable under shared lock, clear under exclusive lock), both phases walking `m_hook_creation_order` in reverse so inline/mid hooks layered on one address unwind newest-first instead of in bucket order; `m_mutator_gate` 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; every public mutation or teardown entry point also fails closed (`HookError::ReentrantCallRejected`, a false/zero result, or a logged no-op for void lifecycle calls) when the per-thread reentrancy guard is set, so a call from inside a `with_*` callback returns or no-ops instead of recursively acquiring the non-recursive lock; the batch toggles (`enable_hooks`/`disable_hooks`/`*_all`) collect their log lines under the lock and emit them after release so a synchronous sink flush or a blocking async overflow never stalls an exclusive acquirer; 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` SRWLOCK reader for `with_inline_hook()` |
+| Logger | `atomic<shared_ptr>` for lock-free async reads; `shutdown_internal` and `disable_async_mode` are 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 permanent cell (normal path: `new (std::nothrow)`; fallback path: non-CRT permanent storage), so a heap allocation failure cannot drop the last handle while the writer 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 `m_active_states[]` array | `shared_lock` (uncontended SRWLOCK reader) guarding the `m_active_states` pointer swap + `memory_order_relaxed` load per binding |
 | InputManager | `mutex` for lifecycle, `atomic<shared_ptr<InputPoller>>` for reads | `atomic<shared_ptr<InputPoller>>` acquire-load, then the poller's `shared_lock` + relaxed load (not lock-free) |
@@ -417,6 +417,7 @@ These are called at 60+ fps from game hook callbacks. Never add allocations, exc
 - **Do not return** from a memory-writing helper before its post-write cache maintenance (instruction-cache flush and cache-range invalidation) once bytes have been modified -- run the cleanup on every exit path, even when a later step such as restoring page protection fails.
 - **Do not let** a public doc comment describe behavior the implementation no longer has -- lifecycle and ordering claims must match the code, and are best pinned by a test.
 - **Do not take** a shared lock in a const query/accessor that can be called from inside a `with_*`/`try_with_*` callback without first making it reentrancy-guard-aware (skip the lock when the per-thread guard is non-zero, since the callback already holds it). Recursive shared acquisition of a non-recursive reader/writer lock on one thread is undefined behavior and deadlocks if a writer is queued between the two acquisitions (see `HookManager::lock_hooks_shared_reentrant`).
+- **Do not call** a HookManager mutation or teardown entry point (`create_inline_hook` / `create_inline_hook_aob` / `create_mid_hook` / `create_mid_hook_aob` / `create_vmt_hook` / `hook_vmt_method` / `apply_vmt_hook` / `enable_hook` / `disable_hook` / `remove_hook` / `remove_vmt_hook` / `remove_vmt_method` / `remove_vmt_from_object`, the `enable_hooks` / `disable_hooks` / `enable_all_hooks` / `disable_all_hooks` batch toggles, or the `shutdown` / `remove_all_hooks` / `remove_all_vmt_hooks` bulk teardown calls) from inside a `with_*`/`try_with_*` callback. The callback already holds `m_hooks_mutex` shared and the per-thread reentrancy guard is non-zero, so re-acquiring that non-recursive lock (shared for the toggles, exclusive for the create/remove/teardown paths) is undefined behavior and deadlocks. Every such public entry point now checks the guard on entry and fails closed -- `HookError::ReentrantCallRejected` for the `std::expected` mutators, `false` or a zero count for the bool/count mutators, and a logged no-op for void lifecycle calls -- so the misuse is a visible error, not a hang. Defer the mutation until after the callback returns (queue it to a worker).
 - **Do not key** a cache store and its invalidation/eviction by different addresses or shard-selection functions. Eviction must use the same canonical key and the same containment lookup as insertion and read, or entries silently survive invalidation (see `Memory::invalidate_range`, which scans every shard because storage is sharded by query address, not region base).
 - **Do not let** a queue or backlog fed at an external event rate grow without bound -- clamp the pending count to a documented ceiling (see `Input` wheel-notch `MAX_WHEEL_PENDING`).
 - **Do not let** an async/deferred sink diverge from its synchronous counterpart's configuration (timestamp format, level, etc.) -- carry the same settings through, as `Logger::enable_async_mode` does for the timestamp format.

README.md

@@ -918,7 +918,7 @@ void ShutdownMyMod()
     DMKLogger::get_instance().info("MyMod Shutting Down...");
     // DMK_Shutdown() is invoked automatically by on_dll_detach() after this
     // function returns, in the correct order:
-    //   InputManager -> HookManager -> Memory cache -> Config -> Logger
+    //   Config auto-reload watcher -> InputManager -> HookManager -> Memory cache -> Config registry -> Logger
 }
 
 BOOL APIENTRY DllMain(HMODULE hModule, DWORD ul_reason_for_call, LPVOID lpReserved)

include/DetourModKit.hpp

@@ -83,9 +83,9 @@ using DMKProfileSample = DetourModKit::ProfileSample;
 /**
  * @brief Explicitly shuts down all DetourModKit singletons in the correct order.
  * @details This function should be called before process exit or DLL unload to ensure proper cleanup without
- *          use-after-free errors. It shuts down singletons in reverse dependency order: InputManager, HookManager,
- *          Memory cache, Config, then Logger last. After calling this function, the singletons are in a safe state for
- *          destruction.
+ *          use-after-free errors. It shuts down singletons in reverse dependency order: the Config auto-reload watcher
+ *          first, then InputManager, HookManager, Memory cache, the Config registry, then Logger last. After calling
+ *          this function, the singletons are in a safe state for destruction.
  *
  * @note This function is idempotent - calling it multiple times is safe.
  * @note Each subsystem detects if it is running under the Windows loader lock (e.g. from DllMain/DLL_PROCESS_DETACH or
@@ -98,19 +98,23 @@ using DMKProfileSample = DetourModKit::ProfileSample;
 inline void DMK_Shutdown()
 {
     // Shutdown in reverse dependency order:
-    // 1. InputManager first (polling thread may invoke callbacks that log)
+    // 1. Config auto-reload watcher first: its background thread can fire the user on_reload callback at any moment, so
+    //    it must stop before any consumer state that callback may touch is torn down. Idempotent and noexcept.
+    DetourModKit::Config::disable_auto_reload();
+
+    // 2. InputManager (polling thread may invoke callbacks that log)
     DetourModKit::InputManager::get_instance().shutdown();
 
-    // 2. HookManager (may have been logging via Logger)
+    // 3. HookManager (may have been logging via Logger)
     DetourModKit::HookManager::get_instance().shutdown();
 
-    // 3. Memory cache (background cleanup thread must stop before Logger shuts down)
+    // 4. Memory cache (background cleanup thread must stop before Logger shuts down)
     DetourModKit::Memory::shutdown_cache();
 
-    // 4. Clear registered config items (static vector cleanup)
+    // 5. Clear registered config items (static vector cleanup)
     DetourModKit::Config::clear_registered_items();
 
-    // 5. Logger last (no more logging after this)
+    // 6. Logger last (no more logging after this)
     DetourModKit::Logger::get_instance().shutdown();
 }
 

include/DetourModKit/config.hpp

@@ -461,7 +461,9 @@ namespace DetourModKit
 
         /**
          * @brief Clears all currently registered configuration items.
-         * @details Useful if the configuration system needs to be reset without restarting the application.
+         * @details Useful if the configuration system needs to be reset without restarting the application. This does
+         *          NOT stop the auto-reload watcher; call disable_auto_reload() first (DMK_Shutdown() already does so
+         *          in the correct order) so a watcher callback cannot fire against state torn down afterwards.
          */
         void clear_registered_items();