Ask for user consent if wsl is active during store updates.#40786
Ask for user consent if wsl is active during store updates.#40786chemwolf6922 wants to merge 2 commits into
Conversation
| return {}; | ||
| } | ||
|
|
||
| if (wsl::windows::common::WslActivityMarker::IsWslActive() && !PromptUserToUpgradeWhileActive()) |
There was a problem hiding this comment.
We're holding the install lock across this prompt, and WTSSendMessageW blocks for up to 60s. Any other Install call will sit on the lock that whole time, and we won't react to g_stopEvent if the service is trying to stop. Worth deciding consent before taking the lock (or try_lock and bail).
| if (wsl::windows::common::WslActivityMarker::IsWslActive() && !PromptUserToUpgradeWhileActive()) | ||
| { | ||
| wsl::windows::common::install::WriteInstallLog("WSL is active; deferring MSI upgrade until WSL is idle."); | ||
| return {}; |
There was a problem hiding this comment.
Deferring returns {} here, which Install() turns into S_OK / exit code 0, the same as 'nothing to install'. So if someone runs an update interactively and clicks No, they get told it succeeded. Can we return a distinct 'deferred' result so callers can tell the difference?
|
|
||
| const auto commandLine = LxssGenerateWslCommandLine(L"sleep infinity"); | ||
| wsl::windows::common::SubProcess process(nullptr, commandLine.c_str()); | ||
| auto processHandle = process.Start(); |
There was a problem hiding this comment.
This sleep infinity never gets killed. SubProcess just closes the handle on destruction, it doesn't terminate the child, and the cleanup only reinstalls the MSI. If that reinstall doesn't actually tear the distro down, it stays Running (and keeps the activity marker set), which can bleed into later installer tests. A wsl --shutdown or TerminateProcess in the scope_exit would make it deterministic.
OneBlue
left a comment
OneBlue
left a comment
There was a problem hiding this comment.
This is an interesting idea. I think having some prompt with a timeout during a WSL upgrade while WSL is being in use could help improve the UX.
However, I don't think we should do it at the WSLInstaller.cpp level. That class is only involved when WSL is being upgraded with MSIX package, which is a path that we want to deprecate.
If we wanted to do this, I think the best way would be to declare a new COM interface that wslservice implements that the MSI can call to ask to "are there any WSL containers / distributions in use ? And then if the answer is yes we can display a notification with a timeout (we should also have an MSI variable to bypass the prompt, so that it doesn't trigger if the user calls wsl --update for instance)
Summary of the Pull Request
Check if WSL is active before installing the msi during a msix installation. If so, ask the user if they would like to shutdown wsl and apply the update. If declined or there is no active user session, the updated will be deferred to the next wslinstaller service start, likely next boot.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
Add headless test without the user prompt:
InstallerTests::MsixUpgradeDefer
User prompt manually tested:
