From 6cccafd1d08f2aee65eba862c7280da912d14de4 Mon Sep 17 00:00:00 2001 From: Stephen Dennis Date: Mon, 27 Jul 2026 18:46:23 -0600 Subject: [PATCH] fix(win32): no module could load under muxscript, for two reasons (#1594) The module preflight added with #1572 turned master red on Windows because none of the three modules smoke.conf configures could load there. Two independent causes; fixing either alone left the run red. 1. comsys and mail built as comsys.dll / mail.dll, while modules/Makefile.am builds comsys_mod.so / mail_mod.so. `cf_module` resolves `module ` to `.\bin\.dll` or `./bin/.so`, so no single config line could name those two modules on both platforms. The vcxproj files were taking their default TargetName; they now set it explicitly. Renaming the Windows side rather than the Unix side because those two DLLs had no DCL_EXPORT until #1569 and so were never loadable on Windows at all -- nothing can depend on the old names, whereas Unix installs use _mod today. 2. exp3 -- whose name already matched on both platforms -- still did not load, silently, and only under muxscript. LoadLibrary() does not treat a relative path as relative to the current directory: it appends the whole relative path to every entry in the DLL search order. The current directory is one of those entries until something calls SetDllDirectory(), which removes it. muxscript calls it in init_com() so engine.dll can find libmux.dll, and cf_module hands LoadLibrary ".\bin\.dll" -- so under muxscript that resolved nowhere. netmux never calls SetDllDirectory(), which is why the identical config worked there and the divergence looked inexplicable. ModuleLoad now resolves the path against the current directory itself, which is what every caller already assumed. Both smoke gates go back to unconditional. They were made advisory on Windows only because the platform could not comply; it can now. Verified on Windows Server 2022, MSVC 14.51, Release x64: all three modules report (loaded) under muxscript from a clean environment, netmux logs "Comsys module loaded" / "Mail module loaded" with the new names, and the preflight passes silently in the smoke run. Measured, and worth recording for #1589/#1614: with the modules finally live on Windows, the suite returns exactly what it returns without them -- 1555 succeeded, 5 failed, same five, same one-verdict deficit. The five are a pre-existing configuration gap (autoconf-win32.h has no HAVE_LIBSSL and no reality levels), not a regression, and they are identical with SMOKE_OMIT_MODULES set. The corpus still cannot tell the two implementations apart. Co-Authored-By: Claude Opus 5 --- mux/lib/libmux.cpp | 22 ++++++++++ mux/modules/comsys/comsys.vcxproj | 4 ++ mux/modules/mail/mail.vcxproj | 4 ++ testcases/tools/Smoke | 68 ++++++++----------------------- win32/TOC.bin.patchable | 4 +- win32/TOC.bin.removed | 2 + 6 files changed, 52 insertions(+), 52 deletions(-) diff --git a/mux/lib/libmux.cpp b/mux/lib/libmux.cpp index 2ccbd93f0..86a8863ad 100644 --- a/mux/lib/libmux.cpp +++ b/mux/lib/libmux.cpp @@ -312,7 +312,29 @@ static void ModuleLoad(Module *pModule) return; } +#if defined(WINDOWS_DYNALIB) + // LoadLibrary() does not treat a relative path as relative to the current + // directory: it appends the whole relative path to every entry in the DLL + // search order. The current directory is one of those entries -- until + // something calls SetDllDirectory(), which removes it. muxscript does + // exactly that so engine.dll can find libmux.dll, and cf_module hands us + // ".\\bin\\.dll", so every module configured under muxscript failed + // to load (#1594). netmux never calls SetDllDirectory(), which is why the + // identical config worked there and the divergence went unexplained. + // + // Resolve against the current directory ourselves. That is what every + // caller already assumes, and it no longer depends on the search order. + // + UTF16 aFullPath[4096]; + DWORD nFullPath = GetFullPathNameW(pModule->pFileName, + sizeof(aFullPath)/sizeof(aFullPath[0]), aFullPath, nullptr); + const UTF16 *pPathName = ( 0 < nFullPath + && nFullPath < sizeof(aFullPath)/sizeof(aFullPath[0])) + ? aFullPath : pModule->pFileName; + pModule->hInst = MOD_OPEN(pPathName); +#else pModule->hInst = MOD_OPEN(pModule->pFileName); +#endif if (nullptr != pModule->hInst) { pModule->fpGetClassObject = reinterpret_cast(MOD_SYM(pModule->hInst, "mux_GetClassObject")); diff --git a/mux/modules/comsys/comsys.vcxproj b/mux/modules/comsys/comsys.vcxproj index 34c857eb6..857ede561 100644 --- a/mux/modules/comsys/comsys.vcxproj +++ b/mux/modules/comsys/comsys.vcxproj @@ -41,11 +41,15 @@ ..\..\bin_release\ $(Platform)\$(Configuration)\$(ProjectName)\ + + comsys_mod C:\tinymux\mux\include;C:\tinymux\mux\sqlite;$(IncludePath) ..\..\bin_debug\ $(Platform)\$(Configuration)\$(ProjectName)\ + comsys_mod C:\tinymux\mux\include;C:\tinymux\mux\sqlite;$(IncludePath) diff --git a/mux/modules/mail/mail.vcxproj b/mux/modules/mail/mail.vcxproj index 4e9e8f58b..3ebb07a01 100644 --- a/mux/modules/mail/mail.vcxproj +++ b/mux/modules/mail/mail.vcxproj @@ -41,11 +41,15 @@ ..\..\bin_release\ $(Platform)\$(Configuration)\$(ProjectName)\ + + mail_mod C:\tinymux\mux\include;C:\tinymux\mux\sqlite;$(IncludePath) ..\..\bin_debug\ $(Platform)\$(Configuration)\$(ProjectName)\ + mail_mod C:\tinymux\mux\include;C:\tinymux\mux\sqlite;$(IncludePath) diff --git a/testcases/tools/Smoke b/testcases/tools/Smoke index dac1fafc6..f234e3dee 100755 --- a/testcases/tools/Smoke +++ b/testcases/tools/Smoke @@ -255,34 +255,16 @@ if [ -n "$WANT_MODULES" ]; then sed -n '/^Modules:/,/^$/p' modprobe.log | sed 's/^/ /' echo - # Fatal where the check is achievable; advisory where it is not yet. + # Unconditional again as of #1594. It was briefly advisory on Windows, + # where no module could load under muxscript at all: comsys/mail built + # as comsys.dll/mail.dll instead of the *_mod names smoke.conf needs, + # and muxscript's SetDllDirectory() call removed the current directory + # from the DLL search order, so cf_module's relative ".\bin\.dll" + # resolved nowhere. Both are fixed; every platform can comply, so a + # module that does not load is a failure everywhere. # - # On Windows none of the three modules can load today, for two causes - # tracked in #1594: comsys/mail build as comsys.dll/mail.dll rather - # than the *_mod names smoke.conf must use for Unix, and exp3 -- whose - # name matches on both platforms -- still does not load under - # muxscript specifically, with no diagnostic. Neither is fixable from - # the harness, and a platform that cannot comply should not have a red - # build held over it: red master blocks everyone (#1564). - # - # This is temporary and should be deleted, not kept. When #1594 - # closes, drop the branch so the check is unconditional again -- - # or set SMOKE_REQUIRE_MODULES=1 to get that behaviour now. - # - case "${SMOKE_REQUIRE_MODULES:-}${MODCHECK_PLATFORM:-$(uname -s 2>/dev/null)}" in - 1*|Linux*|Darwin*|*BSD*|SunOS*) - echo "=== Smoke: FAILED (module preflight) ===" - exit 1 - ;; - *) - echo "WARNING: continuing anyway -- this platform cannot load" - echo " modules under muxscript at all yet (#1594)." - echo " The results below may exercise the engine's" - echo " built-in comsys/mail rather than the modules." - echo " Set SMOKE_REQUIRE_MODULES=1 to make this fatal." - echo - ;; - esac + echo "=== Smoke: FAILED (module preflight) ===" + exit 1 fi rm -rf modprobe.in modprobe.log modprobe.conf modprobe.d fi @@ -423,30 +405,16 @@ esac if [ -n "$IMPL_BAD" ]; then echo "" - # Same platform gate as the load preflight (#1599 / #1594): fatal where - # modules can load; advisory on Windows until that is fixed. Otherwise - # this check undoes the soft preflight -- modules fail to load, the - # built-in answers, and we exit 1 after a long green-looking suite. + # Unconditional, like the load preflight above (#1599 / #1594). # - case "${SMOKE_REQUIRE_MODULES:-}${MODCHECK_PLATFORM:-$(uname -s 2>/dev/null)}" in - 1*|Linux*|Darwin*|*BSD*|SunOS*) - echo "=== Smoke: FAILED (implementation fallback) ===" - echo "smoke.conf asks for the module, but the built-in engine answered" - echo "for:$IMPL_BAD" - echo "" - echo "Either the module never loaded (preflight may have been advisory)" - echo "or discover_comsys_mail_modules() fell back after load. The" - echo "results describe the built-in, not the module under test." - exit 1 - ;; - *) - echo "WARNING: smoke.conf asks for the module, but the built-in" - echo " answered for:$IMPL_BAD (#1594 / #1581)." - echo " Continuing on this platform; set" - echo " SMOKE_REQUIRE_MODULES=1 to make this fatal." - echo - ;; - esac + echo "=== Smoke: FAILED (implementation fallback) ===" + echo "smoke.conf asks for the module, but the built-in engine answered" + echo "for:$IMPL_BAD" + echo "" + echo "Either the module never loaded (the preflight should have caught" + echo "that) or discover_comsys_mail_modules() fell back after load. The" + echo "results describe the built-in, not the module under test." + exit 1 fi echo "" diff --git a/win32/TOC.bin.patchable b/win32/TOC.bin.patchable index 16cb2ec6f..14e5bedb6 100644 --- a/win32/TOC.bin.patchable +++ b/win32/TOC.bin.patchable @@ -12,11 +12,11 @@ docs/LIMITS.md docs/PATCHES.md docs/REALMS.md game/alias.conf -game/bin/comsys.dll +game/bin/comsys_mod.dll game/bin/engine.dll game/bin/exp3.dll game/bin/libmux.dll -game/bin/mail.dll +game/bin/mail_mod.dll game/bin/msvcp140.dll game/bin/muxscript.exe game/bin/netmux.exe diff --git a/win32/TOC.bin.removed b/win32/TOC.bin.removed index beb81fc70..00e48189f 100644 --- a/win32/TOC.bin.removed +++ b/win32/TOC.bin.removed @@ -13,7 +13,9 @@ docs/LIMITS docs/MEMORY docs/PATCHES docs/REALMS +game/bin/comsys.dll game/bin/funcs.dll +game/bin/mail.dll game/bin/sample.dll game/bin/sum.dll src/copyright.h