diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index e82e9d9aa..938553d8c 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -1004,6 +1004,10 @@ Falling back to NAT networking. WSL is finishing an upgrade... + + WSL was updated but a system restart is required to complete the installation. Please reboot your machine and try again. + {Locked="WSL"} + Update failed (exit code: {}). {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated diff --git a/src/windows/common/install.cpp b/src/windows/common/install.cpp index a268f8bf6..8508f7724 100644 --- a/src/windows/common/install.cpp +++ b/src/windows/common/install.cpp @@ -106,6 +106,7 @@ int UpdatePackageImpl(bool preRelease, bool repair) if (exitCode == ERROR_SUCCESS_REBOOT_REQUIRED) { + wsl::windows::common::install::SetRebootRequiredMarker(); PrintSystemError(ERROR_SUCCESS_REBOOT_REQUIRED); } else if (exitCode != 0) @@ -116,6 +117,11 @@ int UpdatePackageImpl(bool preRelease, bool repair) wsl::shared::Localization::MessageUpdateFailed(exitCode) + L"\r\n" + wsl::shared::Localization::MessageSeeLogFile(logFile.c_str())); } + else + { + // Clean install — clear any pending reboot marker from a prior 3010-result install. + wsl::windows::common::install::ClearRebootRequiredMarker(); + } } else { @@ -166,6 +172,16 @@ void WaitForMsiInstall() wprintf(L"\n%ls\n", message.get()); } + if (exitCode == ERROR_SUCCESS_REBOOT_REQUIRED) + { + // The MSI completed but one or more files (typically system.vhd or wslservice.exe) + // were in use and have been scheduled for replacement on the next reboot. Warn + // the user so they understand why WSL may not work, but do not throw — the + // service's _CreateInstance gate will also warn when launching a distro. + EMIT_USER_WARNING(wsl::shared::Localization::MessageUpdateRebootRequired()); + return; + } + if (exitCode != 0) { THROW_HR_WITH_USER_ERROR(HRESULT_FROM_WIN32(exitCode), wsl::shared::Localization::MessageUpdateFailed(exitCode)); @@ -231,10 +247,47 @@ void ConfigureMsiLogging(_In_opt_ LPCWSTR LogFile, _In_ const std::function LxssUserSessionImpl::_CreateInstance(_In_op { ExecutionContext context(Context::CreateInstance); + // If a previous MSI install is pending reboot (files like system.vhd have been + // renamed away and are waiting for delayed replacement), warn the user + // but allow execution to continue. + if (wsl::windows::common::install::IsRebootRequired()) + { + EMIT_USER_WARNING(wsl::shared::Localization::MessageUpdateRebootRequired()); + } + // Validate flags. THROW_HR_IF(E_INVALIDARG, (WI_IsAnyFlagSet(Flags, ~LXSS_CREATE_INSTANCE_FLAGS_ALL))); diff --git a/src/windows/wslinstaller/exe/WslInstaller.cpp b/src/windows/wslinstaller/exe/WslInstaller.cpp index 7ac796d6c..b279d0f76 100644 --- a/src/windows/wslinstaller/exe/WslInstaller.cpp +++ b/src/windows/wslinstaller/exe/WslInstaller.cpp @@ -97,13 +97,28 @@ std::pair InstallMsipackageImpl() auto result = wsl::windows::common::install::UpgradeViaMsi( GetMsiPackagePath().c_str(), L"SKIPMSIX=1", logFile.has_value() ? logFile->path.c_str() : nullptr, messageCallback); - // ERROR_SUCCESS_REBOOT_REQUIRED (3010) means the install succeeded but some files - // will be replaced on the next reboot. Treat as success since the service runs - // silently with no user-facing console. + // ERROR_SUCCESS_REBOOT_REQUIRED (3010) means MSI completed its database changes but + // one or more files (e.g. system.vhd, wslservice.exe) were in use and have been moved + // to .rbf backups under %WINDIR%\Installer\Config.Msi with their replacements scheduled + // via MoveFileEx(MOVEFILE_DELAY_UNTIL_REBOOT). Until the user reboots, the install + // location is in a half-replaced state — notably, the old system.vhd has been renamed + // away and the new one is not yet in place. Propagate this distinctly so the client + // does not proceed to launch WSL against a broken install (which surfaces to users as + // "my system.vhd disappeared after the update"). const bool rebootRequired = (result == ERROR_SUCCESS_REBOOT_REQUIRED); + + // Write a volatile (auto-cleared on reboot) registry marker so subsequent wsl.exe + // invocations know the install is incomplete. Without this, CallMsiPackage() would + // short-circuit and launch against the half-replaced install directory. if (rebootRequired) { - result = ERROR_SUCCESS; + wsl::windows::common::install::SetRebootRequiredMarker(); + } + else if (result == ERROR_SUCCESS) + { + // A clean install means any previously-pending reboot has been resolved (the new + // files are in place). Clear the marker so the user can resume without a reboot. + wsl::windows::common::install::ClearRebootRequiredMarker(); } WSL_LOG( @@ -112,7 +127,10 @@ std::pair InstallMsipackageImpl() TraceLoggingValue(rebootRequired, "rebootRequired"), TraceLoggingValue(errors.c_str(), "errorMessage")); - if (result != ERROR_SUCCESS && result != ERROR_SUCCESS_REBOOT_REQUIRED) + // Preserve MSI logs on anything other than a clean success — including + // ERROR_SUCCESS_REBOOT_REQUIRED, since the log identifies which file(s) forced the + // delayed rename. + if (result != ERROR_SUCCESS) { clearLogs.release(); } diff --git a/test/windows/InstallerTests.cpp b/test/windows/InstallerTests.cpp index 91d9c699c..03260d8d6 100644 --- a/test/windows/InstallerTests.cpp +++ b/test/windows/InstallerTests.cpp @@ -17,6 +17,7 @@ Module Name: #include "Common.h" #include "registry.hpp" +#include "install.h" #include "PluginTests.h" #include "wslcsdk.h" @@ -339,6 +340,90 @@ class InstallerTests // TODO: check wslsupport, wslapi and p9rdr } + // Remove any PendingFileRenameOperations entries whose source path falls under installPath. + // The MSI uses MoveFileEx(MOVEFILE_DELAY_UNTIL_REBOOT) when a file is locked, leaving stale + // entries that prevent a subsequent MSI install from putting the system into a clean state. + void ClearPendingFileRenameOperationsForPath(const std::filesystem::path& installPath) const + { + static constexpr auto c_sessionManagerKey = L"SYSTEM\\CurrentControlSet\\Control\\Session Manager"; + static constexpr auto c_pendingRenameValue = L"PendingFileRenameOperations"; + + auto [key, hr] = wsl::windows::common::registry::OpenKeyNoThrow(HKEY_LOCAL_MACHINE, c_sessionManagerKey, KEY_READ | KEY_WRITE); + if (FAILED(hr)) + { + return; + } + + DWORD size = 0; + if (RegGetValueW(key.get(), nullptr, c_pendingRenameValue, RRF_RT_REG_MULTI_SZ, nullptr, nullptr, &size) != ERROR_SUCCESS || size == 0) + { + return; + } + + std::vector buffer(size / sizeof(WCHAR) + 2); + THROW_IF_WIN32_ERROR(RegGetValueW(key.get(), nullptr, c_pendingRenameValue, RRF_RT_REG_MULTI_SZ, nullptr, buffer.data(), &size)); + + // Entries are stored as null-separated pairs: \0\0\0 + // Sources may carry a \??\ NT-namespace prefix — strip it before comparing. + // Normalize: strip trailing separators so the boundary check is unambiguous. + auto installPathStr = installPath.wstring(); + while (!installPathStr.empty() && (installPathStr.back() == L'\\' || installPathStr.back() == L'/')) + { + installPathStr.pop_back(); + } + + auto matchesInstallPath = [&installPathStr](std::wstring_view src) { + if (wsl::shared::string::StartsWith(src, std::wstring_view{L"\\??\\"}, true)) + { + src = src.substr(4); + } + + if (!wsl::shared::string::StartsWith(src, installPathStr, true)) + { + return false; + } + + // Require a path separator (or exact match) after the prefix so that + // e.g. "C:\Program Files\WSL2" is not matched by "C:\Program Files\WSL". + return src.size() == installPathStr.size() || src[installPathStr.size()] == L'\\' || src[installPathStr.size()] == L'/'; + }; + + std::wstring filtered; + for (auto* p = buffer.data(); *p != UNICODE_NULL;) + { + std::wstring src{p}; + p += src.size() + 1; + std::wstring dst{p}; + p += dst.size() + 1; + + if (!matchesInstallPath(src)) + { + filtered += src; + filtered += UNICODE_NULL; + filtered += dst; + filtered += UNICODE_NULL; + } + } + filtered += UNICODE_NULL; // REG_MULTI_SZ double-null terminator + + if (filtered.size() == 1) + { + // All matching entries removed; delete the value entirely. + auto error = RegDeleteValueW(key.get(), c_pendingRenameValue); + THROW_WIN32_IF(error, error != ERROR_SUCCESS && error != ERROR_FILE_NOT_FOUND); + } + else + { + THROW_IF_WIN32_ERROR(RegSetValueExW( + key.get(), + c_pendingRenameValue, + 0, + REG_MULTI_SZ, + reinterpret_cast(filtered.c_str()), + static_cast(filtered.size() * sizeof(WCHAR)))); + } + } + void DeleteProductCode() const { const auto msiKey = wsl::windows::common::registry::OpenKey(m_lxssKey.get(), L"MSI", KEY_ALL_ACCESS); @@ -1125,4 +1210,113 @@ class InstallerTests SHChangeNotify(SHCNE_ASSOCCHANGED, SHCNF_IDLIST, nullptr, nullptr); VerifyWslSettingsProtocolAssociationExistsWithRetry(); } + + TEST_METHOD(UpgradeWithLockedFileReportsRebootRequired) + { + // Ensure the MSI is installed cleanly first. If a prior test run was + // interrupted, the MSI may be missing — reinstall it. + if (!IsMsiPackageInstalled()) + { + InstallMsi(); + } + + VERIFY_IS_TRUE(IsMsiPackageInstalled()); + + // Stop the WSL service so nothing holds files open. + StopWslService(); + + // Uninstall the MSI. MsiInstallProduct on an already-registered ProductCode + // enters maintenance mode and won't replace files. We need a fresh install + // so the MSI actually writes files and hits the lock. + UninstallMsi(); + + // Create a dummy system.vhd in the install directory so we have something to lock. + // When the MSI does a fresh install it will try to write its real system.vhd here, + // but can't because the dummy is memory-mapped — resulting in 3010. + std::filesystem::create_directories(m_installedPath); + auto systemVhdPath = m_installedPath / L"system.vhd"; + { + wil::unique_hfile dummyHandle{ + CreateFileW(systemVhdPath.c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)}; + VERIFY_IS_TRUE(dummyHandle.is_valid()); + BYTE pad = 0; + DWORD written = 0; + VERIFY_WIN32_BOOL_SUCCEEDED(WriteFile(dummyHandle.get(), &pad, 1, &written, nullptr)); + } + + // Memory-map the dummy to simulate a running VM. A memory-mapped file cannot + // be renamed or deleted regardless of directory permissions — this forces the MSI + // to schedule a delayed rename (MoveFileEx MOVEFILE_DELAY_UNTIL_REBOOT) and return 3010. + wil::unique_hfile lockedHandle{CreateFileW( + systemVhdPath.c_str(), GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr)}; + VERIFY_IS_TRUE(lockedHandle.is_valid()); + + wil::unique_handle mapping{CreateFileMappingW(lockedHandle.get(), nullptr, PAGE_READONLY, 0, 0, nullptr)}; + VERIFY_IS_TRUE(mapping.is_valid()); + + auto* mapView = MapViewOfFile(mapping.get(), FILE_MAP_READ, 0, 0, 1); + VERIFY_IS_NOT_NULL(mapView); + auto unmapOnExit = wil::scope_exit([mapView]() { UnmapViewOfFile(mapView); }); + + // Fake a stale version so the WslInstaller service thinks an upgrade is needed. + RegistryKeyChange version( + HKEY_LOCAL_MACHINE, L"Software\\Microsoft\\Windows\\CurrentVersion\\Lxss\\MSI", L"Version", L"1.0.0"); + + // Remove the MSIX so we can reinstall it to trigger the WslInstaller service. + UninstallMsix(); + VERIFY_IS_FALSE(IsMsixInstalled()); + + // Install the MSIX — this starts the WslInstaller service which detects the + // stale version and runs the MSI. With system.vhd locked, the MSI returns 3010 + // and WslInstaller calls SetRebootRequiredMarker(). + InstallMsix(); + + // Wait for the reboot-required marker — this is the signal that the installer + // completed the MSI install and hit the locked file (3010). + auto waitForMarker = []() { THROW_HR_IF(E_FAIL, !wsl::windows::common::install::IsRebootRequired()); }; + + try + { + wsl::shared::retry::RetryWithTimeout(waitForMarker, std::chrono::seconds(1), std::chrono::minutes(5)); + } + catch (...) + { + VERIFY_FAIL("Timed out waiting for reboot-required marker to be set by WslInstaller"); + } + + // Release the memory map and handle — the file has been renamed to .rbf by MSI. + unmapOnExit.reset(); + mapping.reset(); + lockedHandle.reset(); + + // Verify that launching wsl.exe emits the reboot-required warning on stderr + // but does not fail with an error code attributable to the marker itself. + // (The underlying distro start may still fail because system.vhd is gone, + // but the user gets a clear heads-up about why.) + auto wslCommandLine = LxssGenerateWslCommandLine(L"echo OK"); + auto [output, warnings, wslExitCode] = LxsstuLaunchCommandAndCaptureOutputWithResult(wslCommandLine.data()); + + LogInfo("wsl echo OK output: %ls", output.c_str()); + LogInfo("wsl echo OK warnings: %ls", warnings.c_str()); + + // The reboot-required warning should appear on stderr. + VERIFY_IS_TRUE(warnings.find(wsl::shared::Localization::MessageUpdateRebootRequired()) != std::wstring::npos); + + // Non-distro commands (--version, --list, --shutdown, --update) must keep working + // even with the marker set — they go through CallMsiPackage but don't reach the + // service's _CreateInstance gate, so they should not be blocked. + std::wstring versionCmd = wsl::windows::common::wslutil::GetMsiPackagePath().value_or(L"") + L"\\wsl.exe --version"; + auto [versionOutput, versionWarnings, versionExitCode] = LxsstuLaunchCommandAndCaptureOutputWithResult(versionCmd.data()); + LogInfo("wsl --version output: %ls", versionOutput.c_str()); + VERIFY_ARE_EQUAL(versionExitCode, 0L); + + // Clean up: clear any pending file-rename entries left by the locked-file MSI run, + // delete the volatile marker, then reinstall so subsequent tests start from a clean state. + ClearPendingFileRenameOperationsForPath(m_installedPath); + wsl::windows::common::registry::DeleteKey(OpenLxssMachineKey(KEY_ALL_ACCESS).get(), L"MSI\\RebootPending"); + VERIFY_IS_FALSE(wsl::windows::common::install::IsRebootRequired()); + + InstallMsi(); + ValidatePackageInstalledProperly(); + } };