Protect critical WSL processes under heavy load with cgroup & isolate distro cgroups - #40519
Conversation
Co-authored-by: Copilot <copilot@github.com>
…es-with-memory-cgroup
…es-with-memory-cgroup
…es-with-memory-cgroup
|
If we want this change. Please help determine the reserved memory size. 128M might be too much. |
There was a problem hiding this comment.
Pull request overview
This PR aims to improve WSL2 reliability under heavy memory pressure by placing user/workload processes into a cgroup v2 with a memory cap, while keeping critical WSL system processes outside that cap to reduce the chance of catastrophic failures after OOM events.
Changes:
- Add cgroup path constants for a
wsl-usercgroup and itscgroup.procs/memory.maxcontrol files. - Create and configure the
wsl-usercgroup at mini_init startup, settingmemory.maxtototalram - 128MB. - Move key workload processes (session leaders, boot command, systemd-spawned workload) into
wsl-userby writing0tocgroup.procs.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/linux/init/util.h | Adds constants for the wsl-user cgroup paths. |
| src/linux/init/util.cpp | Moves create-process children into wsl-user cgroup before exec. |
| src/linux/init/main.cpp | Adds cgroup setup routine and invokes it after mounting cgroup2. |
| src/linux/init/init.cpp | Moves session leaders and systemd into wsl-user cgroup. |
| src/linux/init/config.cpp | Moves boot command into wsl-user cgroup. |
Comments suppressed due to low confidence (2)
src/linux/init/main.cpp:3944
- Enabling the memory controller in cgroup v2 via /sys/fs/cgroup/cgroup.subtree_control generally requires the parent cgroup to have no internal processes ("no internal process" rule). At this point mini_init is in the root cgroup, and wsl-user is created before enabling the controller, so the write is likely to fail (e.g., EBUSY) and the feature becomes a no-op. Consider creating a dedicated top-level cgroup hierarchy (e.g., move system processes into a sibling cgroup and leave the root empty), and enable the controller before creating/using children so the memory controller is actually available.
if (UtilMkdir(WSL_USER_CGROUP_PATH, 0755) < 0)
{
LOG_ERROR("Failed to create wsl-user cgroup directory {}", errno);
return;
}
if (WriteToFile(CGROUP_MOUNTPOINT "/cgroup.subtree_control", "+memory") < 0)
{
LOG_ERROR("Failed to enable memory controller {}", errno);
return;
}
src/linux/init/init.cpp:1265
- This cgroup move is relied on to ensure session leaders are subject to the memory cap, but the return value from WriteToFile is ignored. If it fails, the session leader will stay in the root cgroup and can still starve system processes. Consider checking the return and logging a warning/error (or propagating failure) so the protection isn't silently skipped.
"SessionLeader", [ListenSocket = std::move(ListenSocket), &Channel, &Config, Mask = Config.Umask, SocketAddress]() {
// Move session leader into the memory-limited user cgroup.
WriteToFile(WSL_USER_CGROUP_PROCS, "0");
…es-with-memory-cgroup
…es-with-memory-cgroup
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
src/linux/init/init.cpp:1272
- This cgroup move is treated as non-critical but logs failures with LOG_ERROR. To avoid noisy/false errors, consider downgrading to LOG_WARNING/LOG_INFO and stating the impact (session leader not placed in wsl-user cgroup).
// Move session leader into the memory-limited user cgroup.
if (WriteToFile(WSL_USER_CGROUP_PROCS, "0") != 0)
{
// Non-critical.
LOG_ERROR("Failed to move session leader into user cgroup, {}", errno);
}
src/linux/init/init.cpp:2413
- This cgroup move is non-critical but uses LOG_ERROR on failure. Consider logging at warning/info level instead and clarifying that memory protection for systemd (and its subtree) is disabled if the move fails.
// Move systemd into the memory-limited user cgroup.
if (WriteToFile(WSL_USER_CGROUP_PROCS, "0") != 0)
{
// Non-critical.
LOG_ERROR("Failed to move systemd to user cgroup {}", errno);
}
…o user/chemwolf6922/protect-wsl-core-processes-with-memory-cgroup
Blue (OneBlue)
left a comment
There was a problem hiding this comment.
LGTM, couple minor comments that can be addressed in a followup.
Sorry for the delay in review !
WSL can leave user@.service failed when same-UID distributions share systemd cgroups. Omit the hostname to avoid the cross-distro collision and retry transient EBUSY failures during startup. Remove both workarounds after microsoft/WSL#40519 is released and verified locally.
Easy to read the result as broader than it is. The fix is one-directional: it stops this flavour being a cause, it does not make it immune. Any other systemd distribution's shutdown still flushes the shared registry and still breaks interop for everyone left running, us included — `wsl --terminate Ubuntu` while NixOS is up costs NixOS its interop. Also records three things that narrow the exposure, all checkable rather than assumed: - only distributions running systemd can do it, and WSL's own log says which: `init-systemd(NixOS)` can, `init(docker-desktop)` cannot. Docker Desktop's distro starts and stops constantly on this machine and was never a suspect. - creation order and which distro is "main" are irrelevant. The registry is per-VM-boot and WSL's line names /init with P and no F, so it resolves per namespace at exec time and one entry serves everyone. Only runtime start and stop order matters. - `wsl --shutdown` is harmless: it destroys the VM, so the registry is rebuilt from scratch. And notes that the real fix is upstream — WSL isolating binfmt_misc per distribution as microsoft/WSL#40519 does for cgroups, or systemd declining the shutdown flush when it has already detected virtualization wsl. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XoDQiAXKSkzKGDgNmd1Sor
Adds 110-wsl-install.cmd (run_once) and 115-wsl-update.cmd (every apply), and splits the scoop script into 200-scoop-install.cmd (run_once) and 210-scoop-update.cmd (every apply). The WSL install uses `wsl --install --no-distribution` rather than the Microsoft.WSL winget package: wsl.exe's own installer also enables the VirtualMachinePlatform and Microsoft-Windows-Subsystem-Linux optional features, which the store package alone does not. --no-distribution because this repo ships its own images and there is no reason to drag in the default Ubuntu only to unregister it. Presence is tested with `wsl --version`, not `where wsl` -- wsl.exe is a stub present on every Windows 11 install whether or not WSL is actually there. Keeping WSL current is worth an every-apply script rather than a pin: microsoft/WSL#40593, systemd user sessions failing whenever two distros both have a uid-1000 user, is fixed by microsoft/WSL#40519, and running an old WSL left that live indefinitely. Both scripts were run here: install correctly detected an existing WSL and skipped, update no-opped cleanly and exited 0. `wsl --update` returns 0 both when it updates and when already current, so a non-zero really is a failure. It does restart the WSL service when an update lands, terminating running distros -- but only on that path, so an apply against a current WSL disturbs nothing. Everything renumbered to three zero-padded digits. chezmoi orders scripts by plain lexicographic comparison of the stripped name, so a two-digit script would sort after a three-digit one -- "20-scoop" would have run after "115-wsl-update", not before it. Mixing widths silently reorders the suite. Numbering is spaced to leave room. .chezmoiignore's useHeadless entry moved with 60-vencord.cmd to 600-vencord.cmd; no other references to the old names remain. Also fixes a latent bug found while splitting: the mpv portable_config removal was a PowerShell Remove-Item line sitting in a .cmd file, so cmd reported it as an unrecognized command and carried on. The directory was never removed and mpv kept using its portable config instead of the one in .config. Rewritten in cmd, guarded on existence, and hard-failing if the removal itself fails. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Hengqian Ling (linghengqian)
left a comment
There was a problem hiding this comment.
Although the current PR corresponds to commits tagged with 2.9.5, 2.9.6, and 2.9.7, the latest version released on GitHub is still 2.9.4? Am I missing an issue?
I confirm that the tags are already there, but the release descriptions are not yet available. |
Summary of the Pull Request
Issue 1:
When under heavy memory load, the critical WSL processes can fail due to various OOM failures. And may end up in an error state after the memory storm ended. Causing "Catastrophic Failures" afterwards.
Issue 2:
All distro's systemd instances shared the same sets of cgroup. This will cause conflicts. For example, when booting multiple wsl distros at the same time. Only one can create the systemd user session successfully.
Changes
This PR reorganize the wsl cgroup with the follow structure:
The wsl-user cgroup has memory.max and cpu.max set so it can only take max - 32MiB of RAM and max - 0.01 CPU cores. Effectively reserving 32MiB RAM and 0.01 cores for processes not under this cgroup.
Note
Since this uses cgroupv2. Enabling this will not allow the use of cgroupv1. A .wslconfig option IsolateDistroCgroup (default true) is added so users can opt-out of this and keep using cgorupv1.
PR Checklist
poweroffdoes not work properly when multiple systemd enabled distros are running #40865Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
Issue 1
Validated by the OP of this issue: #40458 (comment)
The original issue is not deterministic and is hard to repro. Thus, no new test is added.
Issue 2
The test_distro won't repro the issue. No new test is added.
Validated manually with launching Ubuntu and Debian at the same time.
General
Add tests to validate the cgroup isolation works:
UnitTests::UnitTests::IsolatedCgroupLayout
UnitTests::UnitTests::IsolatedCgroupLayoutSystemd
UnitTests::UnitTests::IsolatedCgroupLayoutDisabled