Skip to content

install: emit warning instead of error when reboot is required after MSI update#40622

Draft
benhillis wants to merge 9 commits into
masterfrom
reboot-warning
Draft

install: emit warning instead of error when reboot is required after MSI update#40622
benhillis wants to merge 9 commits into
masterfrom
reboot-warning

Conversation

@benhillis

Copy link
Copy Markdown
Member

Summary

When the MSI installer returns ERROR_SUCCESS_REBOOT_REQUIRED (files in use, delayed rename pending), WaitForMsiInstall previously threw an error that blocked all subsequent wsl.exe invocations. This was unnecessarily harsh - the service's _CreateInstance gate already emits the same reboot-required warning when a distro launch is attempted.

Changes

  • WaitForMsiInstall(): emit EMIT_USER_WARNING and return instead of THROW_HR_WITH_USER_ERROR
  • CallMsiPackage(): remove the now-dead reboot-specific rethrow

Validation

  • All 25 InstallerTests pass, including UpgradeWithLockedFileReportsRebootRequired

Ben Hillis and others added 3 commits May 20, 2026 13:48
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\<hex>.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>
…er 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>
…MSI update

When the MSI installer returns ERROR_SUCCESS_REBOOT_REQUIRED (files in use,
delayed rename pending), WaitForMsiInstall previously threw an error that
blocked all subsequent wsl.exe invocations. This was unnecessarily harsh —
the service's _CreateInstance gate already emits the same reboot-required
warning when a distro launch is attempted.

Change WaitForMsiInstall to emit a user warning and return normally instead
of throwing. Remove the now-dead reboot-specific rethrow in CallMsiPackage.

Verified by InstallerTests::UpgradeWithLockedFileReportsRebootRequired
(25/25 passing).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 21, 2026 19:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines MSI-upgrade handling when msiexec returns ERROR_SUCCESS_REBOOT_REQUIRED (3010), aiming to avoid permanently breaking subsequent wsl.exe invocations while still preventing distro launches against a half-updated install.

Changes:

  • Add a volatile registry marker (MSI\\RebootPending) to persist “reboot required” state across wsl.exe invocations until reboot.
  • Update WaitForMsiInstall() to emit a user warning (instead of throwing) when the MSI reports reboot required.
  • Gate distro instance creation in the service (_CreateInstance) when the reboot-required marker is present, using a localized reboot-required message.
  • Add an installer test to validate behavior under a locked system.vhd scenario.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/windows/InstallerTests.cpp Adds a new upgrade test that forces a reboot-required MSI result and validates distro vs non-distro command behavior.
src/windows/wslinstaller/exe/WslInstaller.cpp Preserves 3010 as a distinct outcome and sets/clears the reboot-required marker accordingly; preserves logs on non-clean success.
src/windows/service/exe/LxssUserSession.cpp Blocks distro instance creation when the reboot-required marker is set, returning a user-facing reboot-required error.
src/windows/common/install.h Declares APIs to set/clear/check the reboot-required registry marker.
src/windows/common/install.cpp Implements the marker APIs and changes MSI install waiting behavior to warn (not throw) on 3010.
localization/strings/en-US/Resources.resw Adds the localized MessageUpdateRebootRequired string.

// 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";
Comment thread test/windows/InstallerTests.cpp Outdated
Comment on lines +1166 to +1167
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)};
// 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.
Without FILE_SHARE_DELETE, MSI cannot open system.vhd with DELETE intent,
making the 3010 (ERROR_SUCCESS_REBOOT_REQUIRED) outcome more reliable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dkbennett
dkbennett previously approved these changes May 21, 2026

@dkbennett dkbennett left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add similar checking of the reboot key to the WSLC CLI, but perhaps check the list of files that are holding up the install if possible (if they are WSLC-related, note it in the CLI and/or SDK API). Though I have never seen any issues with something of this nature with the CLI so probably very low priority for htat.

Ben Hillis and others added 2 commits May 21, 2026 13:44
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 22, 2026 13:30
After the locked-file scenario, MoveFileEx DELAY_UNTIL_REBOOT entries are
still pending so distro launches fail until an actual reboot. Using
ValidateInstalledVersion (wsl --version) instead of ValidatePackageInstalledProperly
(wsl echo OK) for the cleanup check avoids a spurious failure.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Comment on lines +252 to +267
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);
}
Comment thread test/windows/InstallerTests.cpp Outdated
Comment on lines +1218 to +1220
// The warning 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";
Ben Hillis and others added 2 commits May 22, 2026 09:20
After the locked-file MSI scenario, MoveFileEx DELAY_UNTIL_REBOOT entries
remain in PendingFileRenameOperations. Add ClearPendingFileRenameOperationsForPath()
to strip WSL install path entries from that registry value before reinstalling,
allowing ValidatePackageInstalledProperly() (wsl echo OK) to succeed.

Also tightens the path match to require a separator boundary after the prefix
so sibling directories (e.g. 'WSL2') are not inadvertently matched.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rtion

- SetRebootRequiredMarker: KEY_ALL_ACCESS -> KEY_CREATE_SUB_KEY
- ClearRebootRequiredMarker: KEY_ALL_ACCESS -> KEY_WRITE
- Test assertion: replace brittle L"restart" substring check with
  Localization::MessageUpdateRebootRequired() for locale robustness

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 22, 2026 17:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Comment on lines +1247 to +1251
// 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)};
Comment on lines +1314 to +1316
// 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");
Comment on lines +265 to +266
const auto lxssKey = OpenLxssMachineKey(KEY_WRITE);
wsl::windows::common::registry::DeleteKey(lxssKey.get(), c_rebootPendingSubkey);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants