From 9da91ed94038a5cddfcd0b4e8e1ba1659a17308d Mon Sep 17 00:00:00 2001 From: Acrux Development Date: Mon, 14 Sep 2026 09:22:14 -0300 Subject: [PATCH] Fix .bat build script. Cleanup .dll.hotreload.live.N binaries on startup. --- scripts/build.bat | 26 +++++++-- source/HotReload/README.md | 14 ++++- source/HotReload/src/HotReloadContext.cpp | 53 ++++++++++++++++++ .../examples/HotReloadLiveDemo/Host/Main.cpp | 4 ++ source/examples/HotReloadLiveDemo/README.md | 54 ++++++++++++++++--- source/tests/HotReloadTests/Main.cpp | 35 ++++++++++++ 6 files changed, 174 insertions(+), 12 deletions(-) diff --git a/scripts/build.bat b/scripts/build.bat index 255e1e0..a060343 100644 --- a/scripts/build.bat +++ b/scripts/build.bat @@ -7,8 +7,7 @@ REM REM BUILD_TYPE Debug^|Release^|RelWithDebInfo^|MinSizeRel (default: Release) REM REM Anything after BUILD_TYPE is passed straight through to cmake's -REM configure step (up to 8 extra arguments -- %2 through %9, batch's own -REM positional-parameter limit), e.g.: +REM configure step, e.g.: REM scripts\build.bat Debug -DBUILD_EXAMPLES=ON -DBUILD_TESTS=ON REM CMake caches -D options across reconfigures, so passing them once and REM then calling scripts\build.bat plain afterward for incremental builds @@ -20,10 +19,27 @@ setlocal enabledelayedexpansion cd /d "%~dp0.." if "%BUILD_DIR%"=="" set "BUILD_DIR=out\build" -set "BUILD_TYPE=%~1" -if "%BUILD_TYPE%"=="" set "BUILD_TYPE=Release" if "%TOOLCACHE_DIR%"=="" set "TOOLCACHE_DIR=.cache\tools" -set "EXTRA_CMAKE_ARGS=%2 %3 %4 %5 %6 %7 %8 %9" + +REM Deliberately NOT %1/%2/... for this: cmd.exe's own parameter +REM parser treats = (along with , and ;) as an argument separator, so +REM "-DBUILD_EXAMPLES=ON" typed as one argument arrives as %2= +REM "-DBUILD_EXAMPLES" and %3="ON" -- two arguments, with the = gone -- +REM before this script ever sees it, corrupting any -D value passed +REM this way and silently breaking the configure step below. %* does +REM not go through that same parser and reflects the command line +REM as typed, and FOR /F's own tokenizer only splits on space/tab, so +REM routing %* through "tokens=1,*" correctly separates just the +REM first word from everything after it, = signs and all -- verified +REM directly (via Wine's cmd.exe) against exactly the failure the +REM old %2 %3 %4... version had, not just reasoned through. +set "BUILD_TYPE=Release" +set "EXTRA_CMAKE_ARGS=" +for /f "tokens=1,*" %%A in ("%*") do ( + set "BUILD_TYPE=%%A" + set "EXTRA_CMAKE_ARGS=%%B" +) +if "%BUILD_TYPE%"=="" set "BUILD_TYPE=Release" where cmake >nul 2>nul if errorlevel 1 ( diff --git a/source/HotReload/README.md b/source/HotReload/README.md index 7a6c456..00bb435 100644 --- a/source/HotReload/README.md +++ b/source/HotReload/README.md @@ -475,4 +475,16 @@ fix this in any shared library, hot-reloaded or not. copy internally specifically so your build system's exact write pattern can't cause a stale reload (a known gotcha for this style of tool; see `HotReloadContext.cpp`'s - `stagePathForLoad()`). + `stagePathForLoad()`). On Windows, each staged copy backing a + still-loaded module can't be deleted for as long as that module + stays mapped -- the rest of the process's lifetime, by design -- + so a long-running session accumulates one such file per reload; + `hot_reload_load()` sweeps up whatever a *previous* run left + behind, so this doesn't grow without bound across sessions, only + (unavoidably) within one very long one. See + `source/examples/HotReloadLiveDemo/README.md`'s own section on + this for the full story, including why "Rebuild Solution" (or any + other full clean) separately fails outright while a host built + from this library is running, for an unrelated reason -- it also + tries to delete the *host's own* executable, which Windows refuses + while it's running, and which has no staging workaround at all. diff --git a/source/HotReload/src/HotReloadContext.cpp b/source/HotReload/src/HotReloadContext.cpp index b5fc450..04d598e 100644 --- a/source/HotReload/src/HotReloadContext.cpp +++ b/source/HotReload/src/HotReloadContext.cpp @@ -4,6 +4,7 @@ #include "MemoryProtection.hpp" #include +#include #include #include @@ -26,6 +27,56 @@ namespace hot_reload out << in.rdbuf(); return static_cast(out); } + + // Best-effort: deletes any staging copy left over from a + // PREVIOUS run of this same host/module -- see + // stagePathForLoad()'s own comment for why those copies exist + // at all. On POSIX this normally finds nothing (an active + // run's own copies are already gone by the time it makes its + // next one -- see the std::remove() calls after each + // ModuleHandle::load() below). On Windows, a copy backing a + // still-loaded module cannot be deleted for as long as that + // module stays mapped -- which, by this library's own design, + // is for the rest of the CURRENT process's lifetime (see + // HotReloadContext.hpp's LoadedModule comment) -- so within a + // single long-running session some accumulation is expected + // and unavoidable; this specifically cleans up the PREVIOUS + // session's leftovers once that process has exited and + // released whatever lock Windows held. Called once, here, at + // the first hot_reload_load() for a given name -- not on every + // reload() -- since a copy from earlier in the SAME session + // would still be locked for the same reason and retrying + // wouldn't change that. + void cleanupOrphanedStagingFiles(const std::string& sourcePath, const std::string& moduleName) + { + namespace fs = std::filesystem; + + fs::path source(sourcePath); + fs::path dir = source.has_parent_path() ? source.parent_path() : fs::path("."); + std::string prefix = source.filename().string() + ".hotreload." + moduleName + "."; + + std::error_code dirError; + fs::directory_iterator entries(dir, dirError); + if (dirError) + { + return; + } + + for (const fs::directory_entry& entry : entries) + { + std::error_code fileTypeError; + if (!entry.is_regular_file(fileTypeError) || fileTypeError) + { + continue; + } + const std::string filename = entry.path().filename().string(); + if (filename.compare(0, prefix.size(), prefix) == 0) + { + std::error_code removeError; + fs::remove(entry.path(), removeError); // best-effort; a failure here just means try again next launch + } + } + } } HotReloadContext::HotReloadContext(const HotReloadOptions* options) @@ -89,6 +140,8 @@ namespace hot_reload return HOT_RELOAD_ERROR_INVALID_ARGUMENT; } + cleanupOrphanedStagingFiles(path, moduleName); + LoadedModule module; std::string staged = stagePathForLoad(module, moduleName, path); if (staged.empty()) diff --git a/source/examples/HotReloadLiveDemo/Host/Main.cpp b/source/examples/HotReloadLiveDemo/Host/Main.cpp index dddd907..9a06cfe 100644 --- a/source/examples/HotReloadLiveDemo/Host/Main.cpp +++ b/source/examples/HotReloadLiveDemo/Host/Main.cpp @@ -95,6 +95,10 @@ int main(int argc, char** argv) std::printf(" (or run watch.sh, in this example's own directory, in another\n"); std::printf(" terminal to rebuild automatically every time you save)\n"); std::printf(" Stop: Ctrl+C\n"); + std::printf(" Note: rebuild the PLUGIN target above, not \"Rebuild Solution\"/a full\n"); + std::printf(" clean -- that also tries to delete this Host's own .exe, which\n"); + std::printf(" Windows refuses to do while it's running (see this example's\n"); + std::printf(" own README for why that's expected, not a bug)\n"); std::printf("================================================================\n\n"); std::signal(SIGINT, &handleSigint); diff --git a/source/examples/HotReloadLiveDemo/README.md b/source/examples/HotReloadLiveDemo/README.md index 7ddb467..f5fefbb 100644 --- a/source/examples/HotReloadLiveDemo/README.md +++ b/source/examples/HotReloadLiveDemo/README.md @@ -73,9 +73,51 @@ HotReloadContext.cpp`) copies a plugin to a fresh, uniquely-named file before ever calling `LoadLibrary`/`dlopen` on it -- built originally to sidestep a dlopen caching gotcha on POSIX (see that function's comment) -- so the path this example's build actually writes to is -never the one Windows locks. This reasoning was not verified against -a real Windows rebuild-while-running cycle specifically (unlike the -rest of this library's Windows behavior, which was exercised under -Wine -- see `source/HotReload/README.md`'s verification notes); if a -rebuild on Windows still fails while the Host is running, that -mismatch between reasoning and reality is worth reporting. +never the one Windows locks. Confirmed directly on real Windows/MSVC: +rebuilding `hotreload_live_plugin` repeatedly while the Host stayed +running worked correctly, hot-reloading each time. + +## "Rebuild Solution" (or any other full clean) fails while the Host is running -- this is expected + +Different from the DLL case just above, and this one genuinely can't +be worked around the same way. Visual Studio's "Rebuild" (and the +equivalent `cmake --build --clean-first`, or deleting the build +directory yourself) deletes *every* build output first, including +`hotreload_live_demo_host.exe` itself -- and Windows will not let you +delete a running process's own executable image, full stop, for as +long as it's running. There's no staging trick that helps here the way +there is for the plugin DLL: the thing being deleted is quite literally +the file currently executing. + +Confirmed directly: this fails with `ninja: error: remove(bin/ +examples/hotreload_live_demo_host.exe): Access is denied.`, and the +"Rebuild failed" it's reported as makes it look worse than it is -- +depending on your build tool, the *subsequent* build step can still +run and still correctly rebuild+reload the plugin regardless (also +confirmed directly), since only the clean step, not the whole +operation, actually failed. + +**The fix is simply not to use "Rebuild"/a full clean while the Host +is running** -- an ordinary incremental "Build", or building just +`hotreload_live_plugin` specifically (exactly what `watch.sh` and the +manual `cmake --build ... --target hotreload_live_plugin` command +above already do), never touches the Host's own executable at all, so +neither is affected by this. Stop the Host first if you ever do want a +genuine full clean. + +## Leftover `hotreload_live_plugin.dll.hotreload.live.N` files + +Each reload stages the plugin through a fresh, uniquely-named copy +before loading it (`HotReloadContext::stagePathForLoad()`, needed for +reasons unrelated to this -- see that function's comment). On Linux/ +macOS, the copy is deleted right after loading and nothing +accumulates. On Windows, that deletion fails for as long as the module +it backs stays loaded -- which, by this library's design, is for the +rest of the Host process's lifetime, so **one leftover file per reload +during a long editing session is expected**, not a leak to chase down. +`hot_reload_load()` sweeps up whatever a *previous* run of the same +Host left behind the next time it starts, so restarting the Host is +enough to clean up between sessions; within a single very long +session, some accumulation is simply the cost of every old version +staying safely addressable. Safe to delete by hand at any point the +Host isn't running, if it bothers you before then. diff --git a/source/tests/HotReloadTests/Main.cpp b/source/tests/HotReloadTests/Main.cpp index 910baf9..132b833 100644 --- a/source/tests/HotReloadTests/Main.cpp +++ b/source/tests/HotReloadTests/Main.cpp @@ -12,6 +12,8 @@ #include #include #include +#include +#include #include #include @@ -182,6 +184,39 @@ int main() CHECK(fixtureError != nullptr && std::strlen(fixtureError) > 0); hot_reload_context_destroy(fixtureCtx); + // --- orphaned staging-file cleanup: a real, reported case. On + // Windows, a staged copy backing a still-loaded module cannot be + // deleted for as long as that module stays mapped (the rest of + // the process's lifetime, by this library's own design), so a + // long-running session accumulates one such file per reload -- + // expected and unavoidable, but hot_reload_load() should still + // sweep up whatever a PREVIOUS run of the same host left behind. + // Simulated here (rather than actually reproducing the Windows + // lock) by dropping a few files matching the staging pattern next + // to a real fixture before loading it, then checking they're gone + // afterward -- this exercises the real cleanup code path + // regardless of platform, even though the scenario it protects + // against is Windows-specific. + { + namespace fs = std::filesystem; + std::string orphanedBase = std::string(PLUGIN_V1_PATH) + ".hotreload.cleanup-test."; + for (int i = 1; i <= 3; ++i) + { + std::ofstream orphan(orphanedBase + std::to_string(i)); + orphan << "leftover from a previous run"; + } + + HotReloadContext* cleanupCtx = hot_reload_context_create(nullptr); + CHECK(cleanupCtx != nullptr); + CHECK(hot_reload_load(cleanupCtx, "cleanup-test", PLUGIN_V1_PATH) == HOT_RELOAD_OK); + + for (int i = 1; i <= 3; ++i) + { + CHECK(!fs::exists(orphanedBase + std::to_string(i))); + } + hot_reload_context_destroy(cleanupCtx); + } + if (g_failures == 0) { std::printf("hotreload_tests: all checks passed\n");