From ec14e7e788d0a150b12d5e65d64f51acd19ce8da Mon Sep 17 00:00:00 2001 From: Ben Hillis Date: Wed, 20 May 2026 13:48:57 -0700 Subject: [PATCH 1/3] Surface MSI reboot-required failures instead of silently dropping VHDs When the embedded MSI in the Microsoft Store MSIX cannot replace a locked file (most commonly system.vhd, which is mounted whenever a WSL2 instance is running), Windows Installer renames the existing file to C:\Windows\Installer\Config.Msi\.rbf, schedules the new file via MoveFileEx(MOVEFILE_DELAY_UNTIL_REBOOT), and returns ERROR_SUCCESS_REBOOT_REQUIRED (3010). InstallMsipackageImpl in wslinstaller silently converted 3010 to ERROR_SUCCESS and returned success to its caller. The user was told nothing; their next wsl invocation hit a now-empty C:\Program Files\WSL install dir (system.vhd physically gone until reboot) and produced a confusing "vhd missing" failure - perceived as data loss. This change: * Stops swallowing 3010 in InstallMsipackageImpl. The MSI log is now preserved on 3010 (previously deleted) to aid diagnostics. * Sets a volatile registry marker (HKLM\Software\Microsoft\Windows\CurrentVersion\Lxss\MSI\RebootPending) using REG_OPTION_VOLATILE so the kernel auto-clears it on reboot. No cleanup path is needed; the marker is gone iff the user has rebooted. * Adds a marker check in LxssUserSession::_CreateInstance (service side) which throws a localized user-facing error (MessageUpdateRebootRequired) any time a client tries to launch a distro while the reboot is pending. This catches all distro-launching client paths through the single service entry point: wsl.exe (lifted and MSI-installed), wslg.exe, bash.exe, VS Code Remote-WSL, etc. * Also checks the marker on entry to CallMsiPackage and throws on 3010 in WaitForMsiInstall, so the wsl --update / lifted-client paths surface the same error. User-visible behavior: > wsl WSL was updated but a system restart is required to complete the installation. Please reboot your machine and try again. Error code: Wsl/Service/CreateInstance/0x80070bc2 The user reboots; the volatile key is destroyed by the kernel; Windows' pending file-rename queue swaps the staged file into place; WSL works again. Adds an integration test, InstallerTests::UpgradeWithLockedFileReportsRebootRequired, that exercises the full path: uninstalls the MSI, memory-maps a dummy file at the install path to make it unrenameable, runs the MSIX installer to drive the WslInstaller service, polls for the marker, then verifies wsl echo OK fails with the expected message before cleaning up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- localization/strings/en-US/Resources.resw | 4 + src/windows/common/install.cpp | 51 +++++++++ src/windows/common/install.h | 8 ++ src/windows/service/exe/LxssUserSession.cpp | 9 ++ src/windows/wslinstaller/exe/WslInstaller.cpp | 22 +++- test/windows/InstallerTests.cpp | 100 ++++++++++++++++++ 6 files changed, 189 insertions(+), 5 deletions(-) diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index 5a8097efca..577c9a7c84 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -1008,6 +1008,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 a268f8bf63..117984c2ca 100644 --- a/src/windows/common/install.cpp +++ b/src/windows/common/install.cpp @@ -166,6 +166,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. Surface + // this distinctly so the caller does not proceed to launch WSL against a + // half-installed package — notably, the previous system.vhd has been renamed away + // to %WINDIR%\Installer\Config.Msi\*.rbf and the new one is not yet in place. + THROW_HR_WITH_USER_ERROR(HRESULT_FROM_WIN32(ERROR_SUCCESS_REBOOT_REQUIRED), wsl::shared::Localization::MessageUpdateRebootRequired()); + } + if (exitCode != 0) { THROW_HR_WITH_USER_ERROR(HRESULT_FROM_WIN32(exitCode), wsl::shared::Localization::MessageUpdateFailed(exitCode)); @@ -231,10 +241,39 @@ 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), block instance creation + // with a clear error rather than launching against a broken install. + if (wsl::windows::common::install::IsRebootRequired()) + { + THROW_HR_WITH_USER_ERROR(HRESULT_FROM_WIN32(ERROR_SUCCESS_REBOOT_REQUIRED), 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 7ac796d6c9..f50353c7c6 100644 --- a/src/windows/wslinstaller/exe/WslInstaller.cpp +++ b/src/windows/wslinstaller/exe/WslInstaller.cpp @@ -97,13 +97,22 @@ 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(); } WSL_LOG( @@ -112,7 +121,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 91d9c699c1..3d323bf2e1 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" @@ -1125,4 +1126,103 @@ 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 | FILE_SHARE_DELETE, 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 (a command that goes through CallMsiPackage) fails + // with the reboot-required error. + 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()); + VERIFY_ARE_NOT_EQUAL(wslExitCode, 0); + + // The error message should mention a restart is required. + auto combined = output + warnings; + VERIFY_IS_TRUE(combined.find(L"restart") != std::wstring::npos); + + // Clean up: delete the volatile marker and reinstall cleanly. + wsl::windows::common::registry::DeleteKey(OpenLxssMachineKey(KEY_ALL_ACCESS).get(), L"MSI\\RebootPending"); + VERIFY_IS_FALSE(wsl::windows::common::install::IsRebootRequired()); + + InstallMsi(); + ValidatePackageInstalledProperly(); + } }; From 32fc03cc62186eb2a0dd18d44d1dd879ff21977b Mon Sep 17 00:00:00 2001 From: Ben Hillis Date: Wed, 20 May 2026 14:32:16 -0700 Subject: [PATCH 2/3] Address PR feedback: don't block non-distro commands; auto-clear marker on success The original CallMsiPackage() early-throw blocked every MSIX-lifted wsl.exe invocation (including --version, --list, --shutdown, --update) regardless of whether it would touch the half-installed files. Remove the early throw and rely on the service-side _CreateInstance check, which already gates exactly the distro-launching paths that actually depend on the broken install dir. Also add ClearRebootRequiredMarker() and call it from any MSI install path that completes with ERROR_SUCCESS, so a 'wsl --shutdown; wsl --update' flow can self-recover without requiring an actual reboot. Extend the integration test to verify wsl --version still succeeds with the marker set, and that the marker is cleared after a clean reinstall. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/windows/common/install.cpp | 28 ++++++++++++++----- src/windows/common/install.h | 5 ++++ src/windows/wslinstaller/exe/WslInstaller.cpp | 6 ++++ test/windows/InstallerTests.cpp | 9 ++++++ 4 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/windows/common/install.cpp b/src/windows/common/install.cpp index 117984c2ca..46e75271f0 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 { @@ -251,6 +257,15 @@ void wsl::windows::common::install::SetRebootRequiredMarker() WriteDword(key.get(), nullptr, L"RebootRequired", 1); } +void wsl::windows::common::install::ClearRebootRequiredMarker() +{ + // Best-effort. registry::DeleteKey treats ERROR_FILE_NOT_FOUND as a no-op, + // so this is safe to call on any successful install path even if no marker + // was previously set. + const auto lxssKey = OpenLxssMachineKey(KEY_ALL_ACCESS); + wsl::windows::common::registry::DeleteKey(lxssKey.get(), c_rebootPendingSubkey); +} + bool wsl::windows::common::install::IsRebootRequired() { auto [key, hr] = OpenKeyNoThrow(OpenLxssMachineKey(KEY_READ).get(), c_rebootPendingSubkey, KEY_READ); @@ -266,13 +281,12 @@ int wsl::windows::common::install::CallMsiPackage() { wsl::windows::common::ExecutionContext context(wsl::windows::common::CallMsi); - // If a previous MSI install returned ERROR_SUCCESS_REBOOT_REQUIRED, files like system.vhd - // are pending delayed-rename and the install directory is incomplete. Block early rather - // than launching against a broken install. - if (IsRebootRequired()) - { - THROW_HR_WITH_USER_ERROR(HRESULT_FROM_WIN32(ERROR_SUCCESS_REBOOT_REQUIRED), wsl::shared::Localization::MessageUpdateRebootRequired()); - } + // N.B. We intentionally do not block here on IsRebootRequired(). CallMsiPackage() + // is the bootstrap forwarder used by every MSIX-lifted wsl.exe invocation, + // including read-only commands like `--version`, `--list`, and recovery commands + // like `--shutdown` and `--update`. Blocking those would be user-hostile. + // The service-side check in LxssUserSession::_CreateInstance gates the + // distro-launching paths that actually depend on the half-installed files. auto msiPath = GetMsiPackagePath(); if (!msiPath.has_value()) diff --git a/src/windows/common/install.h b/src/windows/common/install.h index 0fe7669f65..aa15c206af 100644 --- a/src/windows/common/install.h +++ b/src/windows/common/install.h @@ -39,4 +39,9 @@ void SetRebootRequiredMarker(); // since a 3010-result MSI install). bool IsRebootRequired(); +// Clears the reboot-required marker. Should be called after any MSI install path that +// completes successfully without ERROR_SUCCESS_REBOOT_REQUIRED, so a user who shuts WSL +// down and runs `wsl --update` can self-recover without an additional reboot. +void ClearRebootRequiredMarker(); + } // namespace wsl::windows::common::install diff --git a/src/windows/wslinstaller/exe/WslInstaller.cpp b/src/windows/wslinstaller/exe/WslInstaller.cpp index f50353c7c6..b279d0f76f 100644 --- a/src/windows/wslinstaller/exe/WslInstaller.cpp +++ b/src/windows/wslinstaller/exe/WslInstaller.cpp @@ -114,6 +114,12 @@ std::pair InstallMsipackageImpl() { 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( "MSIUpgradeResult", diff --git a/test/windows/InstallerTests.cpp b/test/windows/InstallerTests.cpp index 3d323bf2e1..e3068e3a68 100644 --- a/test/windows/InstallerTests.cpp +++ b/test/windows/InstallerTests.cpp @@ -1218,6 +1218,15 @@ class InstallerTests auto combined = output + warnings; VERIFY_IS_TRUE(combined.find(L"restart") != 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: delete the volatile marker and reinstall cleanly. wsl::windows::common::registry::DeleteKey(OpenLxssMachineKey(KEY_ALL_ACCESS).get(), L"MSI\\RebootPending"); VERIFY_IS_FALSE(wsl::windows::common::install::IsRebootRequired()); From aed3fab695e7c31dd59cfc572c43ecd7381128a8 Mon Sep 17 00:00:00 2001 From: Ben Hillis Date: Wed, 10 Jun 2026 10:47:56 -0700 Subject: [PATCH 3/3] Apply clang-format to InstallerTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/windows/InstallerTests.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/windows/InstallerTests.cpp b/test/windows/InstallerTests.cpp index e3068e3a68..19cdb94320 100644 --- a/test/windows/InstallerTests.cpp +++ b/test/windows/InstallerTests.cpp @@ -1222,8 +1222,7 @@ class InstallerTests // 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()); + auto [versionOutput, versionWarnings, versionExitCode] = LxsstuLaunchCommandAndCaptureOutputWithResult(versionCmd.data()); LogInfo("wsl --version output: %ls", versionOutput.c_str()); VERIFY_ARE_EQUAL(versionExitCode, 0L);