merge master -> feature/wsl-for-apps#40317
merge master -> feature/wsl-for-apps#40317benhillis wants to merge 2 commits intofeature/wsl-for-appsfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR merges master into feature/wsl-for-apps, bringing in upstream changes across WSL networking, host/guest messaging, installer diagnostics, and the WSL Settings UI.
Changes:
- Introduces a transaction-based protocol for
SocketChannelmessaging (replacing the prior sequence-number-only behavior) and updates host + guest components to use it. - Adds WSL Settings “pending changes” staging + an “Apply changes” flow that commits settings and optionally shuts down WSL.
- Adds/updates networking behavior and tests (mirrored fallback to NAT when IPv6 is registry-disabled; port-0 bind/rebind coverage).
Reviewed changes
Copilot reviewed 83 out of 86 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/NetworkTests.cpp | Adds tests for port-0 rebind and mirrored fallback behavior. |
| src/windows/wslsettings/directory.build.targets.in | Disables Roslyn shared compilation to avoid intermittent build failures. |
| src/windows/wslsettings/Views/Settings/ShellPage.xaml.cs | Adds INotifyPropertyChanged and wires pending-changes UI updates. |
| src/windows/wslsettings/Views/Settings/ShellPage.xaml | Adds “Apply changes” button bound to pending-changes state. |
| src/windows/wslsettings/Views/Settings/SettingsApplyHelper.cs | New helper to display/apply pending config changes and optionally shutdown WSL. |
| src/windows/wslsettings/Views/Settings/OptionalFeaturesPage.xaml.cs | Formatting-only change (closing brace alignment). |
| src/windows/wslsettings/Services/WslConfigService.cs | Implements pending-change staging, commit API, and change notification events. |
| src/windows/wslsettings/Contracts/Services/IWslConfigService.cs | Extends service contract with pending-change APIs and value-kind helper. |
| src/windows/wslsettings/CMakeLists.txt | Adds new SettingsApplyHelper source to the build. |
| src/windows/wslrelay/localhost.cpp | Updates relay receive/send to use the new transaction abstraction. |
| src/windows/wslinstaller/exe/WslInstaller.cpp | Adds default MSI log location + deletes logs on success (keeps on failure). |
| src/windows/service/exe/WslCoreVm.cpp | Converts various init messages to transactions; improves crash dump name parsing; adds mirrored IPv6-disabled fallback to NAT. |
| src/windows/service/exe/WslCoreInstance.cpp | Converts init channel interactions to transaction API. |
| src/windows/service/exe/LxssUserSession.cpp | Fixes context enum usage; adjusts VHD resize logic and impersonation handling. |
| src/windows/service/exe/LxssCreateProcess.h | Uses transaction for create-process request/response exchange. |
| src/windows/service/exe/DistributionRegistration.cpp | Removes duplicate registry write of Flags. |
| src/windows/service/exe/BridgedNetworking.cpp | Fixes integer division in DHCP timeout computation. |
| src/windows/common/wslutil.cpp | Prepends a warning when Admin Protection may explain distro-not-found errors. |
| src/windows/common/socket.cpp | Fixes telemetry field name typo (MessagSize → MessageSize). |
| src/windows/common/relay.cpp | Fixes WaitForMultipleObjects upper-bound check. |
| src/windows/common/helpers.cpp | Fixes wait upper-bound check; fixes timezone buffer resize bug. |
| src/windows/common/filesystem.cpp | Adjusts hostname string sizing logic. |
| src/windows/common/WslInstall.cpp | Formatting-only change (blank line). |
| src/windows/common/WslCoreNetworkingSupport.cpp | Fixes dynamic-function load variable misuse. |
| src/windows/common/HandleConsoleProgressBar.cpp | Fixes incorrect FAILED() usage with Win32 API (GetFileSizeEx). |
| src/windows/common/GnsPortTrackerChannel.cpp | Uses transaction API for GNS port-tracker channel handling. |
| src/shared/inc/stringshared.h | Adjusts hostname cleaning to truncate before stripping trailing ./-. |
| src/shared/inc/socketshared.h | Adds message-size cap and updates header logging fields for transactions. |
| src/shared/inc/prettyprintshared.h | Adds bounded printing for flexible buffers; improves pretty-print formatting. |
| src/shared/inc/message.h | Fixes relative-index bounds validation; removes unused member. |
| src/shared/inc/lxinitshared.h | Replaces sequence number with transaction fields; updates pretty-print to avoid OOB reads. |
| src/shared/inc/SocketChannel.h | Introduces Transaction abstraction and transaction-aware send/receive logic. |
| src/shared/configfile/configfile.cpp | Fixes quote-expected warning and adjusts malformed overwrite handling logic. |
| src/linux/plan9/p9util.cpp | Fixes getpwuid_r / getgrnam_r return-value checks. |
| src/linux/plan9/p9readdir.cpp | Fixes missing semicolon. |
| src/linux/plan9/p9io.cpp | Fixes AIO completion/error reporting semantics and cancellation handling. |
| src/linux/plan9/p9file.h | Fixes getpwuid_r return-value check. |
| src/linux/plan9/p9file.cpp | Uses auto for find_last_of result type. |
| src/linux/netlinkutil/NetlinkMessage.hxx | Fixes sizeof() used in an error message. |
| src/linux/netlinkutil/Interface.cpp | Fixes IFNAMSIZ length check and avoids writing trailing NUL to sysctl files. |
| src/linux/mountutil/mountutil.c | Adds missing break in state machine. |
| src/linux/init/wslinfo.cpp | Fixes file header comment text. |
| src/linux/init/util.h | Updates forward declarations and switches create-process handler to Transaction. |
| src/linux/init/util.cpp | Converts various init queries to transactions; adjusts create-process handler signature. |
| src/linux/init/plan9.cpp | Converts stop-plan9 message loop to transactions; fixes log formatting. |
| src/linux/init/main.cpp | Converts message processing to transactions; fixes pthread return check and parsing edge cases. |
| src/linux/init/localhost.cpp | Uses transactions for relay start/stop messages. |
| src/linux/init/init.cpp | Propagates transaction usage through session/init message handlers; adds child-exit early handling. |
| src/linux/init/drvfs.cpp | Converts drvfs elevated query to transaction. |
| src/linux/init/config.h | Switches response path to transactions / callbacks for replies. |
| src/linux/init/config.cpp | Implements transaction-based replies and callback-based init responses; fixes log formatting. |
| src/linux/init/binfmt.cpp | Sends create-NT-process message via transaction. |
| src/linux/init/SecCompDispatcher.cpp | Fixes format specifiers for logging. |
| src/linux/init/GnsPortTracker.h | Removes deferred background resolution queue and exposes synchronous resolve result. |
| src/linux/init/GnsPortTracker.cpp | Makes port-0 resolution synchronous in main loop; reduces retry delay. |
| src/linux/init/GnsEngine.h | Extends engine callbacks to accept a transaction; stores channel reference. |
| src/linux/init/GnsEngine.cpp | Processes one transaction per message; returns status using transaction. |
| src/linux/init/DnsServer.cpp | Fixes epoll event data union usage. |
| src/linux/inc/lxwil.h | Fixes macro typo (THROW_UNEXCEPTED → THROW_UNEXPECTED) and pipe2 error check. |
| localization/strings/zh-TW/Resources.resw | Adds Admin Protection + mirrored IPv6-disabled strings; adds Settings apply-changes strings. |
| localization/strings/zh-CN/Resources.resw | Same as above (Simplified Chinese). |
| localization/strings/tr-TR/Resources.resw | Same as above (Turkish). |
| localization/strings/sv-SE/Resources.resw | Same as above (Swedish). |
| localization/strings/ru-RU/Resources.resw | Same as above (Russian). |
| localization/strings/pt-PT/Resources.resw | Same as above (Portuguese - Portugal). |
| localization/strings/pt-BR/Resources.resw | Same as above (Portuguese - Brazil). |
| localization/strings/pl-PL/Resources.resw | Same as above (Polish). |
| localization/strings/nl-NL/Resources.resw | Same as above (Dutch). |
| localization/strings/nb-NO/Resources.resw | Same as above (Norwegian Bokmål). |
| localization/strings/ko-KR/Resources.resw | Same as above (Korean). |
| localization/strings/ja-JP/Resources.resw | Same as above (Japanese). |
| localization/strings/it-IT/Resources.resw | Same as above (Italian). |
| localization/strings/hu-HU/Resources.resw | Same as above (Hungarian). |
| localization/strings/fr-FR/Resources.resw | Same as above (French). |
| localization/strings/fi-FI/Resources.resw | Same as above (Finnish). |
| localization/strings/es-ES/Resources.resw | Same as above (Spanish - Spain). |
| localization/strings/en-US/Resources.resw | Adds Admin Protection + mirrored IPv6-disabled strings; adds Settings apply-changes strings. |
| localization/strings/en-GB/Resources.resw | Same as above (UK English). |
| localization/strings/de-DE/Resources.resw | Same as above (German). |
| localization/strings/da-DK/Resources.resw | Same as above (Danish). |
| localization/strings/cs-CZ/Resources.resw | Same as above (Czech). |
| distributions/DistributionInfo.json | Updates openSUSE Tumbleweed URLs/hashes and package URLs. |
| diagnostics/collect-wsl-logs.ps1 | Adds collection of install + MSI verbose logs. |
| cgmanifest.json | Formatting normalization in component list. |
| .pipelines/package-stage.yml | Fixes SDK artifact copy path formatting. |
| .pipelines/nuget-stage.yml | Adjusts artifact download location and NuGet push path. |
| App.GetService<IWslConfigService>().PendingChangesChanged += OnPendingChangesChanged; | ||
| } | ||
|
|
||
| private void OnPendingChangesChanged() | ||
| { | ||
| _dispatcherQueue.TryEnqueue(() => | ||
| { | ||
| OnPropertyChanged(nameof(HasPendingChanges)); | ||
| }); | ||
| } |
There was a problem hiding this comment.
ShellPage subscribes to IWslConfigService.PendingChangesChanged in the constructor but never unsubscribes. Since the service likely outlives the page, this can keep ShellPage alive and leak memory/handlers across navigation/window lifetime. Consider unsubscribing on Unloaded (and re-subscribing on Loaded if needed) or implementing IDisposable to detach the event handler deterministically.
| protected virtual void Dispose(bool disposing) | ||
| { | ||
| lock (_wslCoreConfigInterfaceLockObj!) | ||
| { | ||
| if (disposing && _wslConfigFileSystemWatcher != null) | ||
| { | ||
| _wslConfigFileSystemWatcher.EnableRaisingEvents = false; | ||
| _wslConfigFileSystemWatcher.Changed -= OnWslConfigFileChanged; | ||
| _wslConfigFileSystemWatcher.Deleted -= OnWslConfigFileChanged; | ||
| _wslConfigFileSystemWatcher.Renamed -= OnWslConfigFileChanged; | ||
| _wslConfigFileSystemWatcher.Dispose(); | ||
| _wslConfigFileSystemWatcher = null; | ||
| } | ||
|
|
||
| WslCoreConfigInterface.FreeWslConfig(_wslConfig); | ||
| WslCoreConfigInterface.FreeWslConfig(_wslConfigDefaults); | ||
| } |
There was a problem hiding this comment.
WslConfigService.Dispose() calls FreeWslConfig on _wslConfig/_wslConfigDefaults but does not null out these fields or guard against multiple Dispose() calls. If Dispose() is invoked twice (or Dispose(true) followed by Dispose(false) from the finalizer if SuppressFinalize is missed), this can attempt to free the same native config pointer more than once. Add a disposed flag and set _wslConfig/_wslConfigDefaults to null after freeing (and consider making Dispose(bool) idempotent).
| DWORD size = 0; | ||
| WI_VERIFY(GetComputerNameExA(ComputerNamePhysicalDnsHostname, nullptr, &size) == FALSE); | ||
| std::string hostName(size, '\0'); | ||
| std::string hostName(size - 1, '\0'); | ||
| THROW_LAST_ERROR_IF(!GetComputerNameExA(ComputerNamePhysicalDnsHostname, hostName.data(), &size)); | ||
|
|
||
| WI_ASSERT((size <= LX_HOST_NAME_MAX) && (hostName.size() == size + 1)); | ||
| WI_ASSERT((size <= LX_HOST_NAME_MAX) && (hostName.size() == size)); | ||
|
|
There was a problem hiding this comment.
GetLinuxHostName() allocates hostName with length (size - 1) but then passes the original size value to GetComputerNameExA as the buffer capacity. That means the API may write up to size characters into a buffer that is only size - 1 long, which is a potential buffer overrun. Allocate the string with length size (or set size to match the allocated length) and then resize the string after the call using the returned character count.
| #elif defined(__GNUC__) | ||
| THROW_UNEXPECTED(); | ||
| #endif | ||
| } |
There was a problem hiding this comment.
socketshared.h contains the "MessageSize > 4 MiB" validation block twice in a row. This duplication is dead code and makes future edits error-prone; remove the redundant second check.
effedb9 to
1c0a783
Compare
The merge from master introduced a transaction-based message protocol in SocketChannel.h, but the WSLC code paths were not updated. This caused crashes when creating WSLC sessions because the Linux-side init expected non-transaction messages while the Windows side sent transaction messages. Changes: - WSLCInit.cpp: Update all 13 HandleMessageImpl handlers to accept Transaction& and reply via Transaction.Send/SendResultMessage instead of Channel.SendMessage. Update ProcessMessages loop to use Channel.ReceiveTransaction(). - WSLCVirtualMachine.cpp: Convert remaining non-transaction SendMessage calls (WSLC_WATCH_PROCESSES, WSLC_TTY_RELAY, WSLC_EXEC) to use StartTransaction. Fix ConnectSocket Fd=-1 path to receive the second reply within the same transaction instead of a non-transaction receive. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ba5485b to
28d5f8e
Compare
No description provided.