wslc: idle-terminate per-user session VMs when inactive#40781
wslc: idle-terminate per-user session VMs when inactive#40781benhillis wants to merge 31 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds on-demand creation and idle-termination of per-user WSLC session VMs (for sessions with persistent storage), so memory can be reclaimed while keeping the session object and storage intact. It also introduces VM-liveness/activity bookkeeping to prevent teardown during in-flight operations and adds new E2E coverage around VM lifecycle behavior.
Changes:
- Implement lazy VM bring-up and idle shutdown in
wslcsessionvia an idle worker, activity counting/tokens, and aVmLeaseused by VM-requiring operations. - Add client-side “operation keep-alive” usage in
wslc.execontainer operations to prevent VM teardown betweenOpenContainerand subsequent calls/streaming. - Add a new E2E test suite validating lazy start, idle stop, persistence across restarts, keep-alive for root-namespace processes, and teardown/recreate races.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/wslc/e2e/WSLCE2EVmIdleTests.cpp | New E2E tests covering lazy VM start, idle stop, persistence, keep-alive, and race scenarios. |
| test/windows/wslc/e2e/WSLCE2EHelpers.h | Exposes the underlying IWSLCSession* for diagnostics/test-only calls. |
| src/windows/wslcsession/WSLCSession.h | Adds VM lifecycle state, idle worker/tokens/lease declarations, and new session methods. |
| src/windows/wslcsession/WSLCSession.cpp | Implements lazy VM creation, idle teardown, activity tokens, and VM diagnostics reporting. |
| src/windows/wslcsession/WSLCProcessControl.cpp | Preserves a real exit code when signaling container release, only synthesizing SIGKILL when needed. |
| src/windows/wslcsession/WSLCProcess.h | Stores a keep-alive token on root-namespace processes to keep the VM alive for their lifetime. |
| src/windows/wslcsession/WSLCContainer.cpp | Signals idle re-checks on terminal container transitions; holds a VM lease during delete. |
| src/windows/wslcsession/IORelay.h | Adds IsRelayThread() to safely avoid destroying the relay on its own thread. |
| src/windows/wslcsession/IORelay.cpp | Co-initializes the relay thread into the MTA; implements IsRelayThread(). |
| src/windows/wslc/services/SessionModel.h | Adds a helper to acquire/hold a keep-alive token for client-side container operations. |
| src/windows/wslc/services/ContainerService.cpp | Uses the keep-alive token across container operations (attach/start/stop/kill/delete/exec/etc.). |
| src/windows/service/inc/wslc.idl | Adds VM diagnostics type + new session methods for diagnostics and operation keep-alive. |
| src/windows/service/exe/WSLCSessionManager.cpp | Updates comments to reflect on-demand VM creation and recreation after idle termination. |
c12d7e1 to
fa2eb47
Compare
fa2eb47 to
ea2254c
Compare
b870044 to
4bcd87f
Compare
4bcd87f to
348e2e1
Compare
benhillis
left a comment
There was a problem hiding this comment.
Reviewed VM-related comments - all have been addressed:
Comment on WSLCSession.h:84: Already correct - lines 77-78 say "IWSLCVirtualMachineFactory" and "lazily on first use"
Comment on WSLCSession.cpp:571: Fixed by AddRef/Release activity tracking - idle worker checks ActivityCount (line 779), and container proxies increment it on AddRef 1→2 transition. VM will not idle-terminate while clients hold container proxies. See lines 672-674 comment.
Comment on WSLCSession.cpp:380: Already has exception handling - IdleWorker() is wrapped in CATCH_LOG() at lines 375-379
Comment on Session.cpp:65: Already correct - wil::unique_threadpool_wait (line 56) calls WaitForThreadpoolWaitCallbacks in destructor automatically
Session-level WSLC plugin hooks (OnSessionCreated/OnSessionStopping) assumed session == running VM. With idle termination (PR #40781) the VM is created lazily, torn down when idle, and transparently recreated while the session persists, so plugins had no notification of actual VM lifecycle. Add VM-level hooks OnWslcVmStarted/OnWslcVmStopping that fire on every VM (re)start and teardown, in addition to the once-per-session hooks. Both are best-effort (errors logged and ignored). OnWslcVmStarted fires after releasing the runtime exclusive lock so a plugin may reentrantly call back into the session (e.g. WSLCCreateProcess) without deadlocking; OnWslcVmStopping fires under the lock during teardown, gated on a fire-once flag so a failed bring-up emits no spurious stopping. Adds a PluginTests::WslcVmRestart case that drives first start, forced idle teardown (TriggerIdleTermination), and lazy restart, asserting the hook sequence and proving reentrancy is deadlock-free. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f1c36ca0-19d0-46a7-82b3-10f1e1ee3e4c
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
- Session-scope DockerEventTracker; rebind per VM start, preserve subscriptions - Fire OnVmStopping with m_lock dropped to avoid plugin-reentrancy deadlock - Keep containers alive on idle teardown; clear only on permanent shutdown - Redesign OnIdleTimer to commit teardown unconditionally after notifying, matching the test-trigger path; removes phantom OnVmStopping->OnVmStarted and the concurrent-Terminate is_signaled crash - Dedup containerd storage mount point into WSLCSessionDefaults.h - Mark WslcVmStarted/WslcVmStopping hooks as introduced in 2.9.5
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 38 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/windows/wslcsession/WSLCContainer.cpp:2712
- UploadArchive/DownloadArchive still call into WSLCContainerImpl (which uses Docker HTTP APIs) without holding a VmLease. With on-demand VM lifecycle, these calls can now occur while the VM is idle-terminated, leading to failures (e.g. runtime has no Docker client) or races where the idle worker tears the VM down mid-call. Other container entrypoints in this file were updated to acquire a lease; these two should be updated as well for consistent behavior.
CATCH_RETURN();
HRESULT WSLCContainer::UploadArchive(WSLCHandle TarHandle, LPCSTR DestPath, ULONGLONG ContentSize)
{
WSLCExecutionContext context(&m_session);
NotifyVmStarted/NotifyVmStopping invoked the plugin hook while holding m_notifyLock. During idle teardown a plugin may reentrantly restart the VM (WSLCCreateProcess), re-entering these notifications and self-deadlocking on the non-recursive mutex. Flip the pairing state under the lock, copy the hook, then invoke it after releasing the lock. Also clarify Shutdown's lock parameter (runtimeLock is the exclusive hold on m_lock) per review feedback.
The VM-restart plugin test already validates a reentrant process launch and mount+unmount from OnVmStopping. Extend OnVmStarted to also mount+unmount so both lifecycle notifications validate reentrant mount management does not deadlock, and assert the new log lines.
UploadArchive and DownloadArchive streamed data without holding a VmLease, so idle teardown could race and tear down the docker client mid-transfer. Acquire a VmLease and wrap in try/CATCH_RETURN to match Export/Logs and the other VM-dependent container operations.
…tion TriggerIdleTerminationConcurrentWithOperations bounded the worker join with a std::async future, but on timeout the failed VERIFY would unwind and block in the future destructor until the (deadlocked) task completed -- hanging the test host. Fail fast with a dump on timeout so a real deadlock fails cleanly.
OnIdleTimer dropped the runtime lock to fire OnVmStopping, then unconditionally tore the VM down after reacquiring. A lease that started in that window found the VM still Running, so it did not restart and could leave a long-lived activity token (e.g. a process keep-alive), and the teardown would then kill it. Re-check the activity count after reacquiring the lock and, if non-zero, abandon the stop and re-pair the notification with a fresh OnVmStarted. Also set m_vmStartNotified whenever the VM starts rather than only when an OnVmStarted hook is installed, so a hooks user that sets only OnVmStopping still receives paired stop notifications.
ReleaseRuntimeResources unconditionally called UnmountWindowsFolder via Vm() during container teardown. After an unexpected VM exit the guest is already gone, so each call only blocks on the RPC timeout and emits a spurious unmount-failed warning. A dead VM has already dropped every guest mount, so mark the mounts inactive locally instead. Add WSLCSessionRuntime::VmExited() (mirrors TearDownVmLockHeld's VM-dead check) so the container can detect this via the runtime it already references.
VmExited() read m_vmExitedEvent without the runtime lock, but StartVmLockHeld and TearDownVmLockHeld reset/replace that event under the lock, so a container teardown running on another thread could observe the handle mid-reset and fail-fast. Mirror the state in a std::atomic<bool>: cleared when a VM instance starts and latched when the guest is observed dead at the top of teardown (before session-state cleanup, so ReleaseRuntimeResources sees it). VmExited() now returns the atomic.
OnIdleTimer's abort path re-paired OnVmStarted without checking whether the VM died while m_lock was dropped for OnVmStopping. OnVmExited() declines that exit (the expected-stop claim is held) and the exit handle is one-shot, so the session was left with a dead VM marked Running until a later idle cycle. Tear the VM down in that case so a waiting lease restarts a fresh instance, matching the commit path. EnsureVmRunning's running fast path returned before the terminating gate, so a reentrant plugin lease during Shutdown's OnVmStopping (lock dropped) could run work against a VM being permanently torn down. Enforce the gate before the fast path, as its comment already documents.
|
Closing this in favor of #41077, which supersedes this approach with the same idle-terminate behavior plus a WSLCSessionRuntime encapsulation cleanup and two additional concurrency fixes found during review (notify-lock race between start/stop hooks, and a StopRequested disposition being clobbered before a spontaneous-exit check). Thanks @OneBlue for reviewing here -- please take a look at #41077 when you get a chance. |
Fixes #40961
Summary
Idle-terminates a per-user WSLC session's backing VM when it has been inactive, freeing memory while the session object (and its persistent storage) lives on. The VM is transparently recreated on the next operation.
Builds on #40770 (IWSLCVirtualMachineFactory).
Behavior
VM plugin hooks
Idle-terminate breaks the old "session == running VM" assumption (the VM is lazily created, idle-terminated, and transparently recreated while the session lives on), so plugins gain VM-level hooks that fire on every VM (re)start/teardown, alongside the existing per-session
OnSessionCreated/OnSessionStoppinghooks:WSLPluginAPI_OnWslcVmStarted/WSLPluginAPI_OnWslcVmStopping(WslPluginApi.h) withWslcVmStarted/WslcVmStoppingfields inWSLPluginHooksV1.IWSLCPluginNotifier::OnVmStarted()/OnVmStopping()(new methods, no new IID — service and per-user session ship together).OnWslcVmStartedfires after releasingm_lockinEnsureVmRunning(the sole caller holds an activity reference across the gap, so idle teardown can't race the VM down; firing under the lock could deadlock on a reentrantWSLCCreateProcess).OnWslcVmStoppingfires under the lock inTearDownVmLockHeld, with fire-once pairing viastd::atomic<bool> m_vmStartNotifiedso a failed bring-up emits no spurious stopping.Testing
WSLCE2EVmIdleTestsE2E suite (5 tests) includingWSLCE2E_VmIdle_RootProcessKeepsVmAlive.PluginTests::WslcVmRestartasserting the hook sequence: Session created -> VM started -> VM stopping -> VM started -> VM stopping -> Session stopping.WSLCTests::CreateRootNamespaceProcessstill passes.Notes / follow-ups (deferred)
Note
Draft for early review.