Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,41 @@ on:
default: false

jobs:
validate-version:
name: Validate release version matches CMake
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v4
Comment thread
tkhquang marked this conversation as resolved.

- name: Compare release input against project(VERSION) in CMakeLists.txt
run: |
# Single-source the release version from CMakeLists.txt project(VERSION ...): the shipped
# DetourModKitConfigVersion.cmake and the DMK_VERSION_* macros are generated from that number, so a tag
# whose version disagrees with it would publish a package whose find_package check contradicts the tag.
# Fail closed here rather than shipping the mismatch.
INPUT_VERSION="${{ github.event.inputs.version }}"
# Strip any pre-release / build suffix (e.g. 3.7.0-beta -> 3.7.0); CMake project() is numeric MAJOR.MINOR.PATCH.
INPUT_CORE="${INPUT_VERSION%%-*}"
PROJECT_VERSION="$(grep -oP 'project\(DetourModKit\s+VERSION\s+\K[0-9]+\.[0-9]+\.[0-9]+' CMakeLists.txt)"
echo "Release input version : ${INPUT_VERSION} (core: ${INPUT_CORE})"
echo "CMakeLists project ver: ${PROJECT_VERSION}"
if [ -z "${PROJECT_VERSION}" ]; then
echo "::error::Could not parse project(VERSION ...) from CMakeLists.txt"
exit 1
fi
if [ "${INPUT_CORE}" != "${PROJECT_VERSION}" ]; then
echo "::error::Release version '${INPUT_VERSION}' does not match CMakeLists.txt project(VERSION) '${PROJECT_VERSION}'. Bump project(VERSION) first so the shipped ConfigVersion and DMK_VERSION_* macros match the tag."
exit 1
fi
echo "Version check passed."
shell: bash

build-mingw:
name: Build for MinGW (g++)
runs-on: windows-latest
needs: validate-version
outputs:
artifact_name: ${{ steps.determine-artifact-name-mingw.outputs.ARTIFACT_NAME }}
artifact_zip_filename: ${{ steps.determine-artifact-name-mingw.outputs.ARTIFACT_NAME }}
Expand Down Expand Up @@ -138,6 +170,7 @@ jobs:
build-msvc:
name: Build for MSVC (Visual Studio)
runs-on: windows-latest
needs: validate-version
outputs:
artifact_name: ${{ steps.determine-artifact-name-msvc.outputs.ARTIFACT_NAME }}
artifact_zip_filename: ${{ steps.determine-artifact-name-msvc.outputs.ARTIFACT_NAME }}
Expand Down
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ These are called at 60+ fps from game hook callbacks. Never add allocations, exc
- **Do not weaken** atomic memory orderings without proving correctness.
- **Do not skip** running the test suite before committing.
- **Do not publish** release packages before debug tests, release builds, and installed-package smoke tests pass for both MinGW and MSVC.
- **Do not tag** a release whose version differs from `CMakeLists.txt` `project(VERSION ...)`. The release version is single-sourced from there: the generated `DetourModKitConfigVersion.cmake` and the `DMK_VERSION_*` macros derive from it, so a tag that disagrees would ship a package whose `find_package` version check and `DMK_VERSION_AT_LEAST` contradict the tag. The `validate-version` job in `.github/workflows/release.yml` fails closed when the dispatch `version` input does not match, so bump `project(VERSION)` first.
- **Do not add** Windows API calls without `#ifdef _WIN32` guards in headers (implementation files are Windows-only, but headers should remain clean).
- **Do not commit** build artifacts, `.exe`, `.a`, `.lib`, `.obj`, or `.pdb` files.
- **Do not remove** or weaken existing tests. Add new tests for new code.
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
cmake_minimum_required(VERSION 3.25)

project(DetourModKit VERSION 3.6.1 LANGUAGES CXX)
project(DetourModKit VERSION 3.7.0 LANGUAGES CXX)

# GNUInstallDirs defines CMAKE_INSTALL_LIBDIR / BINDIR / INCLUDEDIR / DOCDIR. It must be included before any
# install() rule reads those variables; otherwise they expand to empty and components land at the install-prefix
Expand Down
18 changes: 10 additions & 8 deletions docs/hot-reload/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,9 @@ static void unload_logic_dll()
}

// Brief sleep to allow any in-flight hook callbacks to complete.
// SafetyHook freezes threads during hook removal, but callbacks
// that were already past the hook entry point need time to return.
// SafetyHook does not freeze threads during removal: it relocates only a
// thread that faults on the patched page during the brief rewrite window,
// so a callback already past the hook entry must return on its own.
Sleep(CALLBACK_DRAIN_MS);

FreeLibrary(s_logic_module);
Expand Down Expand Up @@ -653,7 +654,7 @@ With this setup, the workflow is always **build, then press reload key**. The po

**Problem:** A hook callback may be executing on the game's thread when you trigger a reload. If the logic DLL is unloaded while a callback is mid-execution, the game crashes (code page unmapped leads to access violation).

**How DMK handles this:** SafetyHook's `remove_all_hooks()` freezes all threads, patches the original bytes back, then resumes threads. Any thread that was inside a hook trampoline will now execute the original function code. This is safe as long as:
**How DMK handles this:** SafetyHook's `remove_all_hooks()` patches the original bytes back without freezing threads. While it rewrites the prologue it strips execute on the patched page, and a vectored exception handler relocates the instruction pointer of any thread that *faults* on that page during the rewrite window; a thread already running inside the trampoline or detour body is **not** relocated. So removal is safe only if the hooked function is quiescent at that moment, which is why you must drain or quiesce callers before unloading. As long as that holds:

- The hook callback does not store persistent pointers into the logic DLL's code/data segments.
- The hook callback does not spawn threads that outlive the DLL.
Expand Down Expand Up @@ -1216,7 +1217,7 @@ If DMK could only ship one of the two, `DMK_Shutdown` would cover ~90% of consum

### Pre-unload contract: worker-thread quiescence

`Bootstrap::on_logic_dll_unload(_all)` removes hooks and bindings, but it cannot prove that every consumer-owned worker thread has stopped firing those hooks. A worker thread that calls into a detoured function between `remove_hook` returning and `FreeLibrary` reclaiming the Logic DLL's `.text` pages will execute freed code; the resulting access violation often points at an address that no longer maps to anything, which is hard to triage from a crash dump. SafetyHook freezes game threads while it patches the original prologue back, but it has no visibility into worker threads spawned by the consumer.
`Bootstrap::on_logic_dll_unload(_all)` removes hooks and bindings, but it cannot prove that every consumer-owned worker thread has stopped firing those hooks. A worker thread that calls into a detoured function between `remove_hook` returning and `FreeLibrary` reclaiming the Logic DLL's `.text` pages will execute freed code; the resulting access violation often points at an address that no longer maps to anything, which is hard to triage from a crash dump. SafetyHook does not freeze threads while it patches the original prologue back; it only relocates a thread that faults on the patched page during the rewrite window, and has no visibility into worker threads spawned by the consumer, so a worker already inside a detour body is never rescued.

Stop and join every consumer-owned worker BEFORE you call the unload helper. The canonical Logic-DLL `Shutdown()` ordering for the persistent-host topology is:

Expand All @@ -1229,16 +1230,17 @@ extern "C" __declspec(dllexport) void Shutdown()
s_scan_worker.reset();
s_telemetry_worker.reset();

// 2. Now the only remaining callers into the hooks are game threads,
// which SafetyHook will freeze inside remove_hook. Drop the
// DLL-local registrations.
// 2. Now the only remaining callers into the hooks are game threads.
// SafetyHook does not freeze them inside remove_hook; it relocates
// only a thread that faults on the patched page during the rewrite,
// so they must be quiescent here. Drop the DLL-local registrations.
DMKBootstrap::on_logic_dll_unload(hook_names, binding_names);

// 3. Return from Shutdown(). The loader's FreeLibrary call follows.
}
```

A common worker case to watch for: a hook callback runs on a game thread, but a separate consumer-owned thread pool *also* calls into the same detour body (e.g. an off-thread snapshot capture, a deferred re-scan, a periodic poller that touches game state through a hooked accessor). Both paths must be quiet before the unload helper runs. If a worker calls into game-side code that the host module also hooks, joining the worker before unload is sufficient; SafetyHook handles the game-thread side.
A common worker case to watch for: a hook callback runs on a game thread, but a separate consumer-owned thread pool *also* calls into the same detour body (e.g. an off-thread snapshot capture, a deferred re-scan, a periodic poller that touches game state through a hooked accessor). Both paths must be quiet before the unload helper runs. If a worker calls into game-side code that the host module also hooks, joining the worker before unload handles the worker side; the game-thread side still depends on the hooked function being quiescent during removal, since SafetyHook only relocates threads that fault on the patched page and does not drain a thread already inside a detour.

---

Expand Down
4 changes: 4 additions & 0 deletions include/DetourModKit/config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ namespace DetourModKit
* @param log_key_name Human-readable name used in log output.
* @param setter Callback applied with the resolved value. Must be reentrant and thread-safe.
* @param default_value Value used when the key is absent or unparsable.
* @note The INI is parsed as narrow bytes (the underlying SimpleIni uses SetUnicode(false)), so a value is
* delivered to @p setter verbatim as the bytes on disk -- not transcoded. ASCII values (the common case)
* pass through unchanged; a value with non-ASCII characters arrives as raw bytes (e.g. UTF-8 from a
* UTF-8-saved INI), and any encoding interpretation is the consumer's responsibility.
*/
void register_string(std::string_view section, std::string_view ini_key, std::string_view log_key_name,
std::function<void(const std::string &)> setter, std::string default_value);
Expand Down
5 changes: 5 additions & 0 deletions include/DetourModKit/drift_manifest.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ namespace DetourModKit
* @param path Destination file path (UTF-8).
* @param entries The drift entries to serialize.
* @return true on success, false if the file could not be opened or written.
* @note The write is not atomic: it truncates @p path in place, so a crash or power loss mid-write can leave a
* partial manifest. That is acceptable here because the manifest is a regenerable diagnostic/diff
* artifact (offsets are re-healed every session, never loaded as load-bearing state); a torn file is
* reported as MalformedLine / MissingHeader on the next read and overwritten. Do not route load-bearing
* data through this path without first making the write atomic (temp file + replace).
*/
[[nodiscard]] bool write_drift_report_to_file(const std::string &path, std::span<const DriftEntry> entries);

Expand Down
8 changes: 6 additions & 2 deletions include/DetourModKit/hook_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -632,9 +632,13 @@ namespace DetourModKit
* @param name A unique, descriptive name for the VMT hook.
* @param object Pointer to the polymorphic object whose vptr will be replaced.
* @return std::expected<std::string, HookError> The hook name if successful, error code otherwise.
* @note Setup/control-plane only: clones a vtable, allocates, and takes the HookManager exclusive lock. Call
* from init/shutdown or a worker thread, never from a hook or input callback.
* @warning VMT hooks have no enable/disable: creation swaps the object's vptr to the cloned table and removal
* restores it. As with the inline hooks, removal cannot drain a thread that is mid-dispatch through a
* hooked slot, so the caller must guarantee no thread is calling a hooked method on @p object across
* restores it. Removal is a bare vptr write with no thread protection at all -- weaker than inline/mid
* teardown, which at least relocates a thread that faults on the patched page via SafetyHook's
* vectored exception handler. A thread already dispatching through the cloned slot can call into the
* freed clone, so the caller must guarantee no thread is calling a hooked method on @p object across
Comment thread
coderabbitai[bot] marked this conversation as resolved.
* create/remove, and that @p object outlives the hook. The vptr must also stay stable for the hook's
* lifetime; if the game reconstructs the object in place (rewriting its vptr) the hook is silently
* lost.
Expand Down
4 changes: 4 additions & 0 deletions include/DetourModKit/logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ namespace DetourModKit
* lost: the post-join drain can miss at most one in-flight message per producer thread (an accepted
* trade-off documented on AsyncLogger::shutdown). Do not rely on a final diagnostics line reaching the
* file if it is emitted while the logger is being torn down.
* @note In synchronous mode a Warning or Error force-flushes the file stream under the log mutex, so a hook or
* input callback that logs at those levels every frame stalls the game thread on disk I/O. For per-frame
* hot-path logging, enable_async_mode() first (the lock-free queue is non-blocking and callback-safe), or
* keep the hot path at Debug/Trace, which is gated out unless explicitly enabled.
*/
bool log(LogLevel level, std::string_view message);

Expand Down
9 changes: 9 additions & 0 deletions include/DetourModKit/rtti_dissect.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,11 @@ namespace DetourModKit
* - @ref HealError::NoMatch when no slot matched;
* - @ref HealError::Ambiguous when both the @c +d and @c -d slots
* at the nearest matching distance match (an irreducible tie).
* @note For a struct known to hold more than one field of @c expected_mangled's type, prefer @ref
* solve_fingerprint. A single landmark resolves to the uniquely nearest same-typed slot, so a nearer
* same-typed neighbour heals to the wrong field silently (both satisfy the slot shape); the @ref
* HealError::Ambiguous result fires only for an exact +/- distance tie, not for a nearer decoy.
* solve_fingerprint disambiguates structurally because one uniform delta must fit every field at once.
* @warning Init-time / re-heal-on-miss, not per-frame: each probe runs the syscall-heavy prelude up to twice.
* The window cap bounds the worst case. Allocates nothing (one reused stack @ref PointeeType).
*/
Expand Down Expand Up @@ -398,6 +403,10 @@ namespace DetourModKit
* required landmark;
* - @ref HealError::Ambiguous when two or more deltas tie for the
* most optional matches.
* @note Each landmark in @p fp must have a distinct @c nominal_offset. Corroboration is scored by counting the
* required landmarks satisfied at a delta, so two landmarks sharing a nominal_offset probe the same slot
* and would double-count it, reporting stronger agreement than the template provides. Distinct offsets
* are an author-side invariant of the constexpr template, not validated at runtime.
* @warning Init-time only: the probe count is (2 * window_bytes / 8 + 1) * fp.size() prelude walks. Allocates
* nothing.
*/
Expand Down
7 changes: 7 additions & 0 deletions include/DetourModKit/scanner.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,13 @@ namespace DetourModKit
* RipRelative candidates are skipped in the fallback phase since they target instructions deeper than
* the patched prologue and are unaffected by the overwrite.
*
* @note Recovery covers only the E9 near-jump and FF25 indirect-jump trampoline shapes, and never returns a
* wrong address. Two failure modes are distinct and worth handling separately: NoMatch means the direct
* scan and the rebuilt E9/FF25 fallback both ran and matched nothing (the case for a prologue overwritten
* by an unhandled shape such as a push imm32 / ret thunk, an FF15 call thunk, or a prefixed jump);
* PrologueFallbackNotApplicable means no fallback could be formed in the first place (a Direct-mode
* candidate's literal tail was too short to rebuild a unique pattern around the prologue), so nothing was
* retried. Do not assume every unsupported overwrite collapses to NoMatch.
* @param candidates Ordered candidates.
* @param label Human-readable identifier used in log messages.
* @return ResolveHit on success; ResolveError on failure.
Expand Down
34 changes: 23 additions & 11 deletions src/bootstrap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include <windows.h>

#include <atomic>
#include <cstring>
#include <cwchar>
#include <exception>

namespace DetourModKit::Bootstrap
Expand All @@ -35,27 +35,39 @@ namespace DetourModKit::Bootstrap
return true;
}

char exe_path[MAX_PATH]{};
const DWORD len = GetModuleFileNameA(nullptr, exe_path, MAX_PATH);
// Resolve the running executable path as wide rather than via GetModuleFileNameA: the ANSI form maps the
// path through the active code page, so a game whose EXE basename contains non-ASCII characters would be
// mangled and could miss (or false-match) the gate. The rest of bootstrap already works in wide for the
// instance-mutex name, so the wide module path keeps the process gate consistent with the kit's UTF-8
// stance.
wchar_t exe_path[MAX_PATH]{};
const DWORD len = GetModuleFileNameW(nullptr, exe_path, MAX_PATH);
if (len == 0 || len >= MAX_PATH)
{
return false;
}

const char *exe_name = std::strrchr(exe_path, '\\');
const wchar_t *exe_name = std::wcsrchr(exe_path, L'\\');
exe_name = exe_name ? exe_name + 1 : exe_path;

// Copy the caller-supplied name into a bounded stack buffer to get a null-terminated string for _stricmp
// without heap allocation: this helper is noexcept and runs on the bootstrap path, so a throwing allocation
// would call std::terminate. A name that cannot fit a module file name cannot match the running executable.
// Widen the caller-supplied UTF-8 name into a bounded stack buffer for a wide case-insensitive compare.
// This helper is noexcept and runs on the bootstrap path, so it avoids a throwing allocation; a name that
// cannot fit a module file name cannot match the running executable. MultiByteToWideChar with a fixed
// destination and MB_ERR_INVALID_CHARS fails closed (returns 0) on overflow or malformed UTF-8 rather than
// truncating into a false match.
if (expected.size() >= MAX_PATH)
{
return false;
}
char expected_buf[MAX_PATH];
std::memcpy(expected_buf, expected.data(), expected.size());
expected_buf[expected.size()] = '\0';
return _stricmp(exe_name, expected_buf) == 0;
wchar_t expected_buf[MAX_PATH];
const int wide_len = MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, expected.data(),
static_cast<int>(expected.size()), expected_buf, MAX_PATH - 1);
if (wide_len <= 0)
{
return false;
}
expected_buf[wide_len] = L'\0';
return _wcsicmp(exe_name, expected_buf) == 0;
}

bool acquire_instance_mutex(std::string_view prefix) noexcept
Expand Down
Loading
Loading