Surface MSI reboot-required failures instead of silently dropping VHDs

localization/strings/en-US/Resources.resw

@@ -1008,6 +1008,10 @@ Falling back to NAT networking.</value>
   <data name="MessageFinishMsiInstallation" xml:space="preserve">
     <value>WSL is finishing an upgrade...</value>
   </data>
+  <data name="MessageUpdateRebootRequired" xml:space="preserve">
+    <value>WSL was updated but a system restart is required to complete the installation. Please reboot your machine and try again.</value>
+    <comment>{Locked="WSL"}</comment>
+  </data>
   <data name="MessageUpdateFailed" xml:space="preserve">
     <value>Update failed (exit code: {}).</value>
     <comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>

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. 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 +247,47 @@ void ConfigureMsiLogging(_In_opt_ LPCWSTR LogFile, _In_ const std::function<void
 
 } // namespace
 
+static constexpr auto c_rebootPendingSubkey = L"MSI\\RebootPending";
+
+void wsl::windows::common::install::SetRebootRequiredMarker()
+{
+    const auto lxssKey = OpenLxssMachineKey(KEY_ALL_ACCESS);
+    // REG_OPTION_VOLATILE: key is automatically deleted on reboot.
+    auto key = CreateKey(lxssKey.get(), c_rebootPendingSubkey, KEY_SET_VALUE, nullptr, REG_OPTION_VOLATILE);
+    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);
+    if (FAILED(hr))
+    {
+        return false;
+    }
+
+    return ReadDword(key.get(), nullptr, L"RebootRequired", 0) != 0;
+}
+
 int wsl::windows::common::install::CallMsiPackage()
 {
     wsl::windows::common::ExecutionContext context(wsl::windows::common::CallMsi);
 
+    // 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())
     {
@@ -249,6 +302,18 @@ int wsl::windows::common::install::CallMsiPackage()
         {
             LOG_CAUGHT_EXCEPTION();
 
+            // If the install completed but is pending a reboot to finish replacing files
+            // (ERROR_SUCCESS_REBOOT_REQUIRED), the registered MSI install path is already
+            // populated even though some files (e.g. system.vhd) are physically missing
+            // until the user reboots. Do not fall through to the race-recovery
+            // GetMsiPackagePath() retry below — that would silently proceed to launch
+            // wsl.exe against a half-installed package. Surface the reboot-required
+            // error to the user instead.
+            if (wil::ResultFromCaughtException() == HRESULT_FROM_WIN32(ERROR_SUCCESS_REBOOT_REQUIRED))
+            {
+                throw;
+            }
+
             // GetMsiPackagePath() will generate a user error if the registry access fails.
             // Save the error from GetMsiPackagePath() to return a proper 'install failed' message.
             auto savedError = context.ReportedError();

src/windows/common/install.h

@@ -31,4 +31,17 @@ UINT UninstallViaMsi(_In_opt_ LPCWSTR LogFile, _In_ const std::function<void(INS
 
 void WriteInstallLog(const std::string& Content);
 
+// Sets a volatile (auto-cleared on reboot) registry marker indicating the MSI install
+// completed but files are pending replacement until the next reboot (ERROR_SUCCESS_REBOOT_REQUIRED).
+void SetRebootRequiredMarker();
+
+// Returns true if the reboot-required marker is present (i.e. the machine has not rebooted
+// 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

src/windows/service/exe/LxssUserSession.cpp

@@ -13,6 +13,7 @@ Module Name:
 --*/
 
 #include "precomp.h"
+#include "install.h"
 #include "Localization.h"
 #include "LxssUserSession.h"
 #include "LxssInstance.h"
@@ -2494,6 +2495,14 @@ std::shared_ptr<LxssRunningInstance> 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)));
 

src/windows/wslinstaller/exe/WslInstaller.cpp

@@ -97,13 +97,28 @@ std::pair<UINT, std::wstring> 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<UINT, std::wstring> 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();
     }

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,111 @@ 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<std::wstring> 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<void>(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);
+
+        // 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());
+
+        InstallMsi();
+        ValidatePackageInstalledProperly();
+    }
 };