From 679df78a1e74ba6413ce43dccd330f74dbaba58e Mon Sep 17 00:00:00 2001 From: Copilot App <223556219+Copilot@users.noreply.github.com> Date: Wed, 7 Oct 2026 08:59:49 -0700 Subject: [PATCH 1/6] [WSLC] Add host probes for the multi-session store model Spike PR 0 for AB#64133563. Four ignore-gated host probes that answer the questions the multi-session store design rests on, so the design is settled against a live WSL2 host rather than against assumptions. Findings on the vendored SDK 2.9.9: - Two sessions with separate stores coexist in one process on one MTA thread; each stays usable while the other is live, and one survives the other's release. - A second session on a store another session already holds is NOT refused at create time. The create succeeds and the violation surfaces only on first use, as HRESULT 0x80070020. - A released store keeps its cache: a store warmed by a real pull still lists the image after the session is dropped and the store reopened. - A freshly minted store starts empty, so a per-session store under the default no-egress posture cannot resolve an uncached image. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02c25756-75fe-4b3f-84be-6e5c972ad967 --- src/mxc-sdk/src/backends/wslc/common/mod.rs | 2 + .../backends/wslc/common/store_model_probe.rs | 313 ++++++++++++++++++ 2 files changed, 315 insertions(+) create mode 100644 src/mxc-sdk/src/backends/wslc/common/store_model_probe.rs diff --git a/src/mxc-sdk/src/backends/wslc/common/mod.rs b/src/mxc-sdk/src/backends/wslc/common/mod.rs index cb707799e..1bf0cff65 100644 --- a/src/mxc-sdk/src/backends/wslc/common/mod.rs +++ b/src/mxc-sdk/src/backends/wslc/common/mod.rs @@ -26,6 +26,8 @@ pub mod registry_policy; pub mod sandbox; pub mod sdk_init; pub mod state_aware; +#[cfg(all(test, windows))] +mod store_model_probe; mod stream_buffer; pub mod wsl_container_runner; pub mod wslc_bindings; diff --git a/src/mxc-sdk/src/backends/wslc/common/store_model_probe.rs b/src/mxc-sdk/src/backends/wslc/common/store_model_probe.rs new file mode 100644 index 000000000..5b6094b14 --- /dev/null +++ b/src/mxc-sdk/src/backends/wslc/common/store_model_probe.rs @@ -0,0 +1,313 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Host probes for the multi-session store model (AB#64133563, spike PR 0). +//! +//! These answer four questions that decide whether the state-aware daemon can +//! hold more than one WSLc session, and what each session's image store must +//! look like. They need a live WSL2 host, so they are `#[ignore]`d and run +//! explicitly: +//! +//! ```text +//! cargo test -p mxc-sdk --features wslc store_model_probe -- --ignored --nocapture --test-threads=1 +//! ``` +//! +//! `--test-threads=1` matters: every probe drives the same per-user WSL +//! service, and two of them deliberately contend for one store. + +use std::fmt::Write as _; +use std::path::PathBuf; +use std::ptr; + +use super::container_steps::{create_daemon_session, sdk_error}; +use crate::mxc_common::logger::{Logger, Mode}; +use crate::mxc_common::models::ScriptResponse; +use crate::wslc_common::image::pull_image; +use crate::wslc_common::sdk_init::init_and_load_sdk; +use crate::wslc_common::wslc_bindings::{WslcImageInfo, WslcSdk, WslcSession, S_OK}; + +/// `ERROR_SHARING_VIOLATION` as an `HRESULT`. The symptom the work item reports +/// when a second live session opens a store another session already holds. +const E_SHARING_VIOLATION: i32 = 0x8007_0020u32 as i32; + +/// Image the cache-persistence probe pulls. Small, and the same default the +/// state-aware backend uses, so a developer's warm cache likely already has it. +const PROBE_IMAGE: &str = "alpine:latest"; + +/// A scratch store directory that deletes itself on drop, so a probe run does +/// not leave VHDs behind in `%TEMP%`. +struct ScratchStore { + path: PathBuf, +} + +impl ScratchStore { + fn new(tag: &str) -> Self { + let path = std::env::temp_dir().join(format!( + "mxc-store-probe-{tag}-{}", + uuid::Uuid::new_v4().simple() + )); + std::fs::create_dir_all(&path).expect("create scratch store"); + Self { path } + } + + fn as_str(&self) -> String { + self.path.to_string_lossy().to_string() + } +} + +impl Drop for ScratchStore { + fn drop(&mut self) { + // Best-effort: the WSL service may still hold the VHD briefly after + // release, and a probe must not fail during unwind. + let _ = std::fs::remove_dir_all(&self.path); + } +} + +/// Names of the images a session reports in its store. +/// +/// This is the cheapest call that proves a session handle is actually usable +/// rather than merely non-null, and it is the exact call the work item names as +/// failing with `ERROR_SHARING_VIOLATION`. +/// +/// # Safety +/// `sdk` must hold valid function pointers and `session` must be a live handle. +unsafe fn probe_list_images( + sdk: &WslcSdk, + session: WslcSession, +) -> Result, ScriptResponse> { + let mut images: *mut WslcImageInfo = ptr::null_mut(); + let mut count: u32 = 0; + let hr = sdk.WslcListSessionImages(session, &mut images, &mut count); + if hr != S_OK { + return Err(sdk_error("WslcListSessionImages failed", hr, "")); + } + if images.is_null() { + return Ok(Vec::new()); + } + + let mut names = Vec::with_capacity(count as usize); + for info in std::slice::from_raw_parts(images, count as usize) { + let bytes = std::slice::from_raw_parts(info.name.as_ptr().cast::(), info.name.len()); + let end = bytes.iter().position(|&b| b == 0).unwrap_or(bytes.len()); + if let Ok(name) = std::str::from_utf8(&bytes[..end]) { + names.push(name.to_string()); + } + } + windows::Win32::System::Com::CoTaskMemFree(Some(images as *const std::ffi::c_void)); + Ok(names) +} + +fn logger() -> Logger { + Logger::new(Mode::Console) +} + +/// Q1 — can one process, on one MTA thread, hold two live sessions with +/// separate stores, use both, and release both cleanly? +/// +/// This is the premise the whole multi-session design rests on. The work item +/// claims a probe confirmed it on SDK 2.9.3; this re-confirms it on the 2.9.9 +/// the repo now vendors, through the daemon's own `create_daemon_session`. +#[test] +#[ignore = "requires a WSL2 host with the WSLc SDK installed"] +fn two_sessions_with_separate_stores_coexist_in_one_process() { + let mut log = logger(); + let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); + + let store_a = ScratchStore::new("a"); + let store_b = ScratchStore::new("b"); + + // SAFETY: `sdk` is live and both stores are distinct empty directories. + let session_a = + unsafe { create_daemon_session(sdk, "mxc-probe-a", &store_a.as_str(), &mut log) } + .expect("first session must create"); + + let session_b = + unsafe { create_daemon_session(sdk, "mxc-probe-b", &store_b.as_str(), &mut log) } + .expect("second concurrent session with its own store must create"); + + // Both handles must be usable *while the other is live*, not merely + // created: a handle that cannot list its store is not a working session. + let images_a = unsafe { probe_list_images(sdk, session_a.as_raw()) } + .expect("session A must stay usable while B is live"); + let images_b = unsafe { probe_list_images(sdk, session_b.as_raw()) } + .expect("session B must be usable while A is live"); + + let mut out = String::new(); + let _ = writeln!( + out, + "[probe] A store={} images={:?}", + store_a.as_str(), + images_a + ); + let _ = writeln!( + out, + "[probe] B store={} images={:?}", + store_b.as_str(), + images_b + ); + println!("{out}"); + + // Interleave once more after both have been exercised, to catch a handle + // that only works until the other session touches the SDK. + unsafe { probe_list_images(sdk, session_a.as_raw()) } + .expect("session A must still be usable after B was used"); + + drop(session_b); + // A must survive B's release — per-session teardown releasing the wrong + // session is the specific hazard the daemon's current single-session + // `shutdown()` cannot express. + unsafe { probe_list_images(sdk, session_a.as_raw()) } + .expect("session A must survive session B's release"); + drop(session_a); +} + +/// Q2 — does a second live session on the *same* store really fail, and where? +/// +/// The work item reports `WslcListSessionImages failed: HRESULT 0x80070020`. +/// Whether that fires at create time or only on first use decides whether the +/// daemon can detect the collision before it mints a sandbox id. +#[test] +#[ignore = "requires a WSL2 host with the WSLc SDK installed"] +fn a_second_live_session_on_one_store_is_a_sharing_violation() { + let mut log = logger(); + let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); + + let shared = ScratchStore::new("shared"); + + // SAFETY: `sdk` is live; both calls target the same store on purpose. + let first = unsafe { create_daemon_session(sdk, "mxc-probe-1", &shared.as_str(), &mut log) } + .expect("first session on an unheld store must create"); + unsafe { probe_list_images(sdk, first.as_raw()) }.expect("first session must be usable"); + + let second = unsafe { create_daemon_session(sdk, "mxc-probe-2", &shared.as_str(), &mut log) }; + + match second { + Err(resp) => { + println!("[probe] second create FAILED: {}", resp.error_message); + println!("[probe] => collision is detectable at CREATE time"); + } + Ok(guard) => { + println!("[probe] second create SUCCEEDED; probing first use"); + let used = unsafe { probe_list_images(sdk, guard.as_raw()) }; + match used { + Err(resp) => { + println!("[probe] first use FAILED: {}", resp.error_message); + println!("[probe] => collision only surfaces on USE"); + assert!( + resp.error_message.contains("80070020") + || resp + .error_message + .contains(&format!("{E_SHARING_VIOLATION:#x}")), + "expected ERROR_SHARING_VIOLATION, got: {}", + resp.error_message + ); + } + Ok(images) => { + println!("[probe] first use SUCCEEDED: images={images:?}"); + println!( + "[probe] => NO collision on this SDK/host. The store-exclusivity \ + premise does not hold as stated; re-scope the store design." + ); + } + } + drop(guard); + } + } + drop(first); +} + +/// Q3 — THE store-model question. After a session releases its store, can a +/// later session reopen that same store and still see the images the first one +/// cached? +/// +/// Reopenability alone is not enough: a slot pool is only worth building if the +/// *cache survives* the lease. So this warms the store with a real pull, drops +/// the session, retakes the same store, and requires the image to still be +/// there. +/// +/// If it survives, a bounded pool of reusable store slots works: a session +/// leases a warm slot and returns it, so N sessions cost N stores but not N +/// *cold* stores, and `--setup-wslc` can warm the pool ahead of time. +/// +/// If it does not, every session is permanently cold and the only offline path +/// is `imageTarPath` — which would make per-session stores a serious regression +/// for the default no-egress posture. +#[test] +#[ignore = "requires a WSL2 host and network egress to pull alpine:latest"] +fn a_released_store_keeps_its_cache_for_the_next_session() { + let mut log = logger(); + let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); + + let slot = ScratchStore::new("slot"); + let store = slot.as_str(); + + // Lease 1: warm the slot with a real pull. + // SAFETY: `sdk` is live and the store is a fresh directory. + let first = unsafe { create_daemon_session(sdk, "mxc-probe-lease-1", &store, &mut log) } + .expect("first lease must create"); + let cold = unsafe { probe_list_images(sdk, first.as_raw()) }.expect("list before pull"); + println!("[probe] lease 1 cold images={cold:?}"); + assert!(cold.is_empty(), "a fresh slot must start cold"); + + unsafe { + pull_image( + sdk, + first.as_raw(), + PROBE_IMAGE, + Some(&store), + "[probe]", + &mut log, + ) + } + .expect("pull must succeed; this probe needs egress"); + let warmed = unsafe { probe_list_images(sdk, first.as_raw()) }.expect("list after pull"); + println!("[probe] lease 1 warmed images={warmed:?}"); + assert!( + !warmed.is_empty(), + "the pull must have populated the store; got {warmed:?}" + ); + + // Return the slot. + drop(first); + + // Lease 2: take the same slot again, as a pool would. + let second = unsafe { create_daemon_session(sdk, "mxc-probe-lease-2", &store, &mut log) } + .expect("a released store must be reopenable by a later session"); + let reopened = unsafe { probe_list_images(sdk, second.as_raw()) } + .expect("the reopened store must be readable"); + println!("[probe] lease 2 reopened images={reopened:?}"); + + assert_eq!( + warmed, reopened, + "a reopened store must present the cache it had at release; losing it \ + would make a warm slot pool pointless and leave every new session cold" + ); + drop(second); +} + +/// Q4 — baseline: a brand-new store really is empty. +/// +/// This is the premise behind `select_action`'s `RefuseNoEgress` arm: an +/// uncached image with no tar and no egress is refused. If a freshly minted +/// per-session store starts empty, then under the default deny-egress posture +/// every provision into a new session fails before it ever reaches the VM. +#[test] +#[ignore = "requires a WSL2 host with the WSLc SDK installed"] +fn a_fresh_store_starts_empty() { + let mut log = logger(); + let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); + + let fresh = ScratchStore::new("fresh"); + // SAFETY: `sdk` is live and the store is a fresh directory. + let session = + unsafe { create_daemon_session(sdk, "mxc-probe-fresh", &fresh.as_str(), &mut log) } + .expect("fresh store session must create"); + let images = unsafe { probe_list_images(sdk, session.as_raw()) }.expect("list fresh store"); + + println!("[probe] fresh store images={images:?}"); + assert!( + images.is_empty(), + "a newly minted per-session store must start cold; found {images:?}" + ); + drop(session); +} From 8f04840012cf700abf6ef086f1c0604edbfeae10 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Mon, 12 Oct 2026 13:59:47 -0700 Subject: [PATCH 2/6] [WSLC] Move the state-aware daemon off the create-and-run image store Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 763fd4b3-1dff-4398-8b27-e82d89a02a35 --- .../wslc/wsl-container-getting-started.md | 21 +- docs/backends/wslc/wslc-state-aware.md | 68 +- scripts/setup-wslc.ps1 | 30 +- .../backends/wslc/common/container_steps.rs | 96 ++- src/mxc-sdk/src/backends/wslc/common/image.rs | 66 +- src/mxc-sdk/src/backends/wslc/common/mod.rs | 3 +- .../backends/wslc/common/store_model_probe.rs | 313 -------- .../src/backends/wslc/common/store_slots.rs | 666 ++++++++++++++++++ .../src/bin/wslc_daemon/session_manager.rs | 73 +- src/mxc-sdk/tests/wslc_daemon_ipc.rs | 7 +- src/tools/wxc/src/main.rs | 11 + tests/scripts/run_wslc_all_tests.ps1 | 5 +- tests/scripts/run_wslc_state_aware_tests.ps1 | 6 +- 13 files changed, 1004 insertions(+), 361 deletions(-) delete mode 100644 src/mxc-sdk/src/backends/wslc/common/store_model_probe.rs create mode 100644 src/mxc-sdk/src/backends/wslc/common/store_slots.rs diff --git a/docs/backends/wslc/wsl-container-getting-started.md b/docs/backends/wslc/wsl-container-getting-started.md index 9cdfee1c1..2553ed71e 100644 --- a/docs/backends/wslc/wsl-container-getting-started.md +++ b/docs/backends/wslc/wsl-container-getting-started.md @@ -141,6 +141,15 @@ cost once per image, not once per run. > `wxc-exec.exe`), otherwise the runner will not find what you just > pulled and will pull it again under its own path. +> **Container lifecycle operations use a different store.** The state-aware +> daemon leases its store from a pool under +> `%LOCALAPPDATA%\mxc\wslc-store-slots`, which `-StoragePath` does not reach. +> `setup-wslc.ps1` warms the first slot as well by default; `-DaemonSlots 0` +> skips it, and a larger count warms more slots at a pull and a multi-GiB store +> each. On `wxc-exec.exe` the equivalent is `--wslc-daemon-slots `, which +> defaults to `0`. See +> [the lifecycle guide](wslc-state-aware.md#image-store). + > **Bring-up reaches the network.** A cache miss makes the host fetch > from the image's registry before the container starts. That fetch is > outside the sandbox's own network policy, so a config declaring @@ -312,7 +321,7 @@ WSLC-specific settings go under `wslc` in the JSON config: | `cpuCount` | number | Host default | Number of CPU cores for the container | | `memoryMb` | number | Host default | Memory limit in MB | | `gpu` | boolean | `false` | Enable GPU passthrough | -| `storagePath` | string | System default | Host path for container storage (VHD) | +| `storagePath` | string | System default | Host path for container storage (VHD). Create-and-run execution only | | `imageTarPath` | string | — | Path to a local tar file to import as the image | | `portMappings` | array | `[]` | Host→container TCP forwards; each entry takes `windowsPort` and `containerPort` | @@ -350,8 +359,11 @@ The separate `provision` operation takes the same list under The lifecycle daemon creates one container per sandbox, so a forward belongs to the sandbox that declared it. The session-wide `cpuCount` / `memoryMb` / -`gpu` / `storagePath` settings remain available only for create-and-run execution, because that daemon shares a -single WSL session across every sandbox. +`gpu` settings remain available only for create-and-run execution, because that daemon shares a +single WSL session across every sandbox. `storagePath` is create-and-run-only +for a different reason: the daemon leases its store from a slot pool rather +than taking a caller-named path, so warming its cache is a separate step. See +[the lifecycle guide](wslc-state-aware.md#image-store). ### Image sources @@ -633,7 +645,8 @@ images — cannot be used. | `Failed to load wslcsdk.dll` | DLL not in same directory as `wxc-exec.exe` | Copy `wslcsdk.dll` next to the binary | | `WSLC runtime unavailable` | WSL runtime package is missing, older than 2.9.9, or the Virtual Machine Platform optional component is disabled | Update WSL with `wsl --update --pre-release`, verify the installed version with `wsl --version`, and enable the Virtual Machine Platform optional component if required. The WSLC SDK DLL is a separate dependency and does not replace the WSL runtime package. | | `WSLC runtime unavailable. Missing components: SdkNeedsUpdate` | The opposite direction: your installed WSL is **newer** than the WSLc SDK this MXC build ships (specified by `WSLC_SDK_VERSION` in `src/mxc-sdk/build/build_wslc_common.rs`) | Update MXC to a build that includes a newer SDK. Do **not** update WSL — it is already ahead, and updating it further will not clear this. | -| `WSLC image '' is not cached, and this sandbox declares no egress` | An isolated config named an image the store does not have | Warm the cache with `--setup-wslc`, set `imageTarPath`, or allow egress | +| `WSLC image '' is not cached, and this sandbox declares no egress` | An isolated config named an image the store does not have | Warm the cache with `--setup-wslc`, set `imageTarPath`, or allow egress. For a lifecycle operation the store is a daemon slot, so warm it with `setup-wslc.ps1 -DaemonSlots 1` | +| `WSLc store unavailable: the image store at is already held by another live session` | Two processes reached the same store, which admits one live session | Wait for the holder to release it. A running `wxc-wslc-daemon.exe` releases its slot after `MXC_WSLC_DAEMON_IDLE_TIMEOUT_SECS` (default 300s); a create-and-run execution releases its store when the run ends | | `WSLC image '' cannot be pulled: '' is not in the administrative registry allowlist` | Machine policy restricts which registries may be used | Use a permitted registry, set `imageTarPath`, or ask an administrator to widen `WslcAllowedImageRegistries` | | `WSLC image '' did not finish pulling within s and was stopped` | The pull exceeded its budget and was aborted | Retry, raise `MXC_WSLC_PULL_TIMEOUT_SECS`, or warm the cache from a faster network | | `WSLC image '' did not finish pulling within s. The transfer was abandoned` | The registry stopped responding, so the pull was given up on rather than ended | Retry, or warm the cache from a faster network. For a separate `provision` operation, raise `MXC_WSLC_DAEMON_CALL_TIMEOUT_SECS` alongside `MXC_WSLC_PULL_TIMEOUT_SECS`, since the pull must finish before the daemon-call deadline | diff --git a/docs/backends/wslc/wslc-state-aware.md b/docs/backends/wslc/wslc-state-aware.md index 6358056b4..a0d33628b 100644 --- a/docs/backends/wslc/wslc-state-aware.md +++ b/docs/backends/wslc/wslc-state-aware.md @@ -55,6 +55,7 @@ waits for the run, because deleting the container would free a handle the run is | Lifecycle backend | `src/mxc-sdk/src/backends/wslc/common/state_aware.rs` (`WslcStateAwareRunner`) | Translates runtime `WslcProvisionConfig` and shared policy fields into daemon protocol frames; implements `StatefulSandboxBackend` (`ID_PREFIX`/`BACKEND_KEY` = `wslc`). | | Policy support checks | `src/mxc-sdk/src/backends/wslc/common/policy.rs` | Per-phase validation of which policy fields are enforced or rejected. | | Daemon client | `src/mxc-sdk/src/backends/wslc/common/daemon_client.rs` | Discovers / spawns the daemon, connects the control pipe, sends `DaemonRequest` frames, reads responses; typed `DaemonError`. | +| Store slot pool | `src/mxc-sdk/src/backends/wslc/common/store_slots.rs` | Owns the daemon's image stores: slot paths, leasing, and the capacity knob. See [Image store](#image-store). | | Daemon | `src/mxc-sdk/src/bin/wslc_daemon/` (`wxc-wslc-daemon.exe`) | Long-lived host process holding `WslcSession` / `WslcContainer`; worker thread drives the SDK; idle-timeout watchdog tears the session down when unused. | | Engine routing | `src/mxc-sdk/src/core/mxc_engine/state_aware.rs` | Selects the WSLc lifecycle backend (Windows + `wslc` feature). | | Prefix registration | `src/mxc-sdk/src/tools/mxc_common/state_aware_dispatch.rs` (`backend_from_prefix`) | Maps the `wslc:` id prefix back to the WSLc backend for post-provision phases. | @@ -99,8 +100,10 @@ The field requires development contract `1.1.0-alpha`. Requests declaring `0.9.0-alpha` cannot include it and must select `1.1.0-alpha` explicitly. The daemon shares one session (VM) but creates a separate container for each provision, so a mapping applies only to the container that declared it, unlike -the session-wide `cpuCount` / `memoryMb` / `gpu` / `storagePath` knobs that -remain available only for create-and-run execution. +the session-wide `cpuCount` / `memoryMb` / `gpu` knobs that +remain available only for create-and-run execution. `storagePath` is not among them either way: +the daemon leases its store from the slot pool rather than taking a caller-named +path (see [Image store](#image-store)). The create-and-run `wslc.portMappings` list and `wslc.provision.portMappings` share one duplicate check, so two entries claiming the same `windowsPort` are @@ -130,6 +133,65 @@ as `--container-id`; the dispatcher derives the backend from the `wslc:` prefix | stop | `WslcStopContainer`. | | deprovision | `WslcDeleteContainer`; release the session + SDK when the last container is gone (daemon may then exit / idle-time out). | +## Image store + +A WSLc store is a host directory handed to `WslcCreateSession`; the SDK creates +`storage.vhdx` inside it and mounts that VHDX into the session's WSL2 utility +VM, where it holds the session's pulled images. A warmed store runs to a couple +of GiB. + +The daemon leases its store from a fixed pool rather than taking a caller-named +path: + +```text +%LOCALAPPDATA%\mxc\wslc-store-slots\slot-0 .. slot-3 +``` + +`%TEMP%\mxc\wslc-store-slots` is used instead when `%LOCALAPPDATA%` is +unresolvable. A session leases the lowest free slot when it boots and returns it +when it is released, so a daemon holding one session always runs on `slot-0`. +Create-and-run execution keeps its own default of `%TEMP%\mxc-wslc-sessions` and +never touches the pool. + +Separating the two is what lets both surfaces run at once. A store's VHDX admits +one live session, so while the daemon held `%TEMP%\mxc-wslc-sessions` every +create-and-run WSLc execution on the same host failed with +`WslcListSessionImages failed: HRESULT 0x80070020` until the daemon idled out. + +### Slot contents and disk + +A returned slot keeps its images — that is the point of leasing rather than +minting. A slot that has never been leased holds nothing at all, and an uncached +image under the isolated posture is refused before the VM is reached, so a cold +slot fails a provision outright rather than pulling. Warm the pool with +`scripts\setup-wslc.ps1 -DaemonSlots ` (see +[the getting-started guide](wsl-container-getting-started.md#step-3--container-images-optional-pre-pull)). + +Slots are identified by index and are never repurposed, so nothing accumulates +and no startup collection is needed; disk is bounded by one VHDX per slot. +Reclaim it by deleting the pool directory while no daemon is running. Lowering +`MXC_WSLC_STORE_SLOTS` leaves the higher-numbered directories in place, inert +and safe to delete. + +### Store exclusivity + +The SDK does not report a store conflict at create time: `WslcCreateSession` +against a store another live session holds returns a healthy-looking handle, and +the violation appears only on first use. The daemon therefore probes the store +itself immediately after create (`WslcListSessionImages`) and treats a failure +as a *create* failure, so a conflict can never mint a sandbox id that dies at an +arbitrary later phase. + +Lease tracking covers one process, so two sessions inside one daemon can never +be handed the same slot. A holder outside that bookkeeping — typically a daemon +from a previous build still inside its idle window — fails the provision with an +error naming the slot, rather than relocating the session onto a cold slot and +reporting a missing image instead. + +| Variable | Default | Meaning | +|----------|---------|---------| +| `MXC_WSLC_STORE_SLOTS` | `4` | Slots the pool offers. Unset, empty, non-numeric, or zero falls back to the default. | + ### exec output semantics `provision` / `start` / `stop` / `deprovision` return a JSON object containing `result` or `error` on stdout. @@ -303,7 +365,7 @@ it can observe idle-teardown within seconds. ## Testing `tests/scripts/run_wslc_state_aware_tests.ps1` is the multi-invocation E2E harness (requires a WSL2 -host that can reach a registry or already has the image cached, and `wxc-wslc-daemon.exe` staged next to `wxc-exec.exe`). It exercises +host that can reach a registry or already has the image cached in daemon store slot 0, and `wxc-wslc-daemon.exe` staged next to `wxc-exec.exe`). It exercises core lifecycle, warm-reuse (a marker written by one `exec` is read back by a separate `exec` process — only possible if the container stayed warm), filesystem volumes, bridged networking + proxy, validation rejections, exec concurrency, and idle teardown. Fixtures live in diff --git a/scripts/setup-wslc.ps1 b/scripts/setup-wslc.ps1 index 51e203e84..821418fd4 100644 --- a/scripts/setup-wslc.ps1 +++ b/scripts/setup-wslc.ps1 @@ -19,6 +19,11 @@ (the `wslc.storagePath` field of the config, or the runner's default of `%TEMP%\mxc-wslc-sessions` when omitted). + Container lifecycle operations do not read that store. The state-aware + daemon leases its store from a pool under + %LOCALAPPDATA%\mxc\wslc-store-slots, so -DaemonSlots warms those slots + separately. + .PARAMETER Image One or more image references to pre-pull. Defaults to a small starter set (alpine:latest, python:3.12-alpine) suitable for a quick @@ -33,7 +38,16 @@ .PARAMETER StoragePath WSLC storage path to populate. When omitted, the runner default (`%TEMP%\mxc-wslc-sessions`) is used. Set this if your runtime configs - override `wslc.storagePath`. + override `wslc.storagePath`. This reaches create-and-run execution only; + use -DaemonSlots for the container-lifecycle store. + +.PARAMETER DaemonSlots + How many container-lifecycle store slots to warm as well, from slot 0 up. + The state-aware daemon leases its store from a separate pool under + %LOCALAPPDATA%\mxc\wslc-store-slots, which -StoragePath does not reach, and + a slot that has never been leased holds no images. Defaults to 1, which is + what a daemon holding one session uses. Pass 0 to warm the create-and-run + cache only; each slot costs another pull and another multi-GiB store. .PARAMETER DebugLogs Enable verbose logging from wxc-exec (passes `--debug`). @@ -52,6 +66,10 @@ .EXAMPLE .\setup-wslc.ps1 -StoragePath C:\wslc-cache Pulls into a non-default cache directory. + +.EXAMPLE + .\setup-wslc.ps1 -DaemonSlots 0 + Pulls for create-and-run execution only, leaving the lifecycle slots alone. #> [CmdletBinding()] @@ -59,6 +77,8 @@ param( [string[]]$Image = @("alpine:latest", "python:3.12-alpine"), [string]$WxcExecPath, [string]$StoragePath, + [ValidateRange(0, 64)] + [int]$DaemonSlots = 1, [switch]$DebugLogs, [switch]$Force ) @@ -99,6 +119,11 @@ if ($StoragePath) { } else { Write-Host "Storage path : (default) %TEMP%\mxc-wslc-sessions" -ForegroundColor Gray } +if ($DaemonSlots -gt 0) { + Write-Host "Daemon slots : $DaemonSlots (%LOCALAPPDATA%\mxc\wslc-store-slots)" -ForegroundColor Gray +} else { + Write-Host "Daemon slots : none" -ForegroundColor Gray +} Write-Host "Images : $($Image -join ', ')`n" -ForegroundColor Gray $failed = @() @@ -108,6 +133,9 @@ foreach ($img in $Image) { if ($StoragePath) { $wxcArgs += @("--storage-path", $StoragePath) } + if ($DaemonSlots -gt 0) { + $wxcArgs += @("--wslc-daemon-slots", "$DaemonSlots") + } if ($DebugLogs) { $wxcArgs += "--debug" } diff --git a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs index 1d888795b..2ce4cb6a4 100644 --- a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs +++ b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs @@ -56,6 +56,10 @@ use crate::wslc_common::wslc_bindings::*; // Shared error helper // --------------------------------------------------------------------------- +/// `ERROR_SHARING_VIOLATION`, which the SDK reports when a call reaches a store +/// whose VHDX another live session already has mounted. +const E_SHARING_VIOLATION: HRESULT = 0x8007_0020_u32 as HRESULT; + /// Build a `ScriptResponse` error from an HRESULT failure with an optional /// SDK-provided message. Thin wrapper over [`WslcError::Sdk`], which owns the /// message formatting and the `LaunchFailed` phase attribution. @@ -729,13 +733,99 @@ pub unsafe fn create_daemon_session( let msg = err_msg.to_string_lossy(); return Err(sdk_error("WslcCreateSession failed", hr, &msg)); } - let _ = writeln!(logger, "[WSLC][daemon] Session created"); - Ok(WslcSessionGuard::from_raw( + let guard = WslcSessionGuard::from_raw( session, sdk.terminate_session_fn(), sdk.release_session_fn(), - )) + ); + validate_store_access(sdk, guard.as_raw(), storage_path)?; + + let _ = writeln!( + logger, + "[WSLC][daemon] Session created on store {storage_path}" + ); + + Ok(guard) +} + +/// Confirm a session just created can reach the store it was given. +/// +/// A store's VHDX admits one live session, and the SDK does not report a +/// conflict at create time: `WslcCreateSession` against a held store returns a +/// healthy-looking handle, and the violation appears on first use as +/// `HRESULT 0x80070020`. Probing here turns that into a create failure, so a +/// caller never mints a sandbox id against a session that would die at an +/// arbitrary later phase. +/// +/// # Safety +/// `sdk` must hold valid function pointers and `session` must be a live handle. +pub unsafe fn validate_store_access( + sdk: &WslcSdk, + session: WslcSession, + storage_path: &str, +) -> Result<(), ScriptResponse> { + list_image_names(sdk, session).map(|_| ()).map_err(|hr| { + if hr == E_SHARING_VIOLATION { + sdk_error( + "WSLc store unavailable", + hr, + &format!( + "the image store at {storage_path} is already held by another live session. \ + A running wxc-wslc-daemon.exe releases it after its idle timeout; a \ + create-and-run execution releases it when the run ends" + ), + ) + } else { + sdk_error( + "WslcListSessionImages failed", + hr, + &format!("could not read the image store at {storage_path}"), + ) + } + }) +} + +/// Names of the images a session's store holds. +/// +/// This is the cheapest call that proves a session handle is usable rather than +/// merely non-null, since it has to reach the store's VHDX to answer. +/// +/// # Safety +/// `sdk` must hold valid function pointers and `session` must be a live handle. +pub unsafe fn list_session_image_names( + sdk: &WslcSdk, + session: WslcSession, +) -> Result, ScriptResponse> { + list_image_names(sdk, session).map_err(|hr| sdk_error("WslcListSessionImages failed", hr, "")) +} + +/// # Safety +/// `sdk` must hold valid function pointers and `session` must be a live handle. +unsafe fn list_image_names(sdk: &WslcSdk, session: WslcSession) -> Result, HRESULT> { + let mut images: *mut WslcImageInfo = ptr::null_mut(); + let mut count: u32 = 0; + let hr = sdk.WslcListSessionImages(session, &mut images, &mut count); + if hr != S_OK { + return Err(hr); + } + if images.is_null() { + return Ok(Vec::new()); + } + + let mut names = Vec::with_capacity(count as usize); + for info in std::slice::from_raw_parts(images, count as usize) { + // `info.name` is a fixed-size, possibly-unterminated C buffer; read up + // to the first NUL, or the whole buffer if there is none. + let bytes = std::slice::from_raw_parts(info.name.as_ptr().cast::(), info.name.len()); + let end = bytes.iter().position(|&b| b == 0).unwrap_or(bytes.len()); + if let Ok(name) = std::str::from_utf8(&bytes[..end]) { + names.push(name.to_string()); + } + } + windows::Win32::System::Com::CoTaskMemFree(Some(images as *const c_void)); + + Ok(names) } /// Create a daemon-owned container with `keepalive` as its init process, so it diff --git a/src/mxc-sdk/src/backends/wslc/common/image.rs b/src/mxc-sdk/src/backends/wslc/common/image.rs index 653bb4880..5ec1d3a2a 100644 --- a/src/mxc-sdk/src/backends/wslc/common/image.rs +++ b/src/mxc-sdk/src/backends/wslc/common/image.rs @@ -13,10 +13,11 @@ use crate::mxc_common::logger::Logger; use crate::mxc_common::models::ScriptResponse; use crate::mxc_common::string_util::{to_wide, CoTaskMemPWSTR}; -use crate::wslc_common::container_steps::{cstr_bytes, sdk_error}; +use crate::wslc_common::container_steps::{cstr_bytes, sdk_error, validate_store_access}; use crate::wslc_common::error::WslcError; use crate::wslc_common::registry_policy; use crate::wslc_common::sdk_init; +use crate::wslc_common::store_slots; use crate::wslc_common::wslc_bindings::*; /// How long a single pull may run before it is abandoned. @@ -943,8 +944,14 @@ fn with_implicit_tag(name: &str) -> String { } } -/// Warm a storage path's image cache with `image_name` and release the session, -/// backing `wxc-exec.exe --setup-wslc`. +/// Warm the create-and-run store's image cache with `image_name`, and the first +/// `daemon_slots` state-aware store slots with it too, backing +/// `wxc-exec.exe --setup-wslc`. +/// +/// The state-aware daemon leases its store from a separate pool, so warming the +/// create-and-run store does not warm the daemon's. A slot that has never been +/// leased holds no images at all, and an uncached image under the default +/// deny-egress posture is refused before the VM is reached. /// /// # Safety /// Must be called once per process before any other WSLC SDK functions @@ -952,6 +959,7 @@ fn with_implicit_tag(name: &str) -> String { pub unsafe fn setup_pull_image( image_name: &str, storage_path: Option<&str>, + daemon_slots: usize, logger: &mut Logger, ) -> Result<(), String> { let sdk = match sdk_init::init_and_load_sdk(logger) { @@ -959,6 +967,50 @@ pub unsafe fn setup_pull_image( Err(resp) => return Err(resp.error_message), }; + warm_store( + sdk, + image_name, + storage_path, + StoreKind::CreateAndRun, + logger, + )?; + + for slot in store_slots::slot_paths(daemon_slots) { + std::fs::create_dir_all(&slot) + .map_err(|e| format!("could not create the WSLc store slot at {slot:?}: {e}"))?; + warm_store( + sdk, + image_name, + Some(&slot.to_string_lossy()), + StoreKind::DaemonSlot, + logger, + )?; + } + + Ok(()) +} + +/// Which store a warm pass is filling. +enum StoreKind { + /// The store create-and-run execution reads, caller-named or defaulted. + CreateAndRun, + + /// A slot from the state-aware pool, which one live session holds at a time. + DaemonSlot, +} + +/// Pull `image_name` into one store and release the session. +/// +/// # Safety +/// `sdk` must hold valid function pointers and COM must be initialised on this +/// thread. +unsafe fn warm_store( + sdk: &'static WslcSdk, + image_name: &str, + storage_path: Option<&str>, + kind: StoreKind, + logger: &mut Logger, +) -> Result<(), String> { let storage_path_str = storage_path.map(|s| s.to_string()).unwrap_or_else(|| { std::env::temp_dir() .join("mxc-wslc-sessions") @@ -997,6 +1049,14 @@ pub unsafe fn setup_pull_image( sdk.release_session_fn(), ); + // A slot admits one live session, and a conflict is invisible until the + // store is touched, so a held slot would otherwise surface partway through + // the pull. + if matches!(kind, StoreKind::DaemonSlot) { + validate_store_access(sdk, session, &storage_path_str) + .map_err(|resp| resp.error_message)?; + } + let _ = writeln!(logger, "[WSLC setup] Target store: {}", storage_path_str); let budget = pull_budget(); let (done, finished) = std::sync::mpsc::channel(); diff --git a/src/mxc-sdk/src/backends/wslc/common/mod.rs b/src/mxc-sdk/src/backends/wslc/common/mod.rs index 1bf0cff65..f8f07dd0c 100644 --- a/src/mxc-sdk/src/backends/wslc/common/mod.rs +++ b/src/mxc-sdk/src/backends/wslc/common/mod.rs @@ -26,8 +26,7 @@ pub mod registry_policy; pub mod sandbox; pub mod sdk_init; pub mod state_aware; -#[cfg(all(test, windows))] -mod store_model_probe; +pub mod store_slots; mod stream_buffer; pub mod wsl_container_runner; pub mod wslc_bindings; diff --git a/src/mxc-sdk/src/backends/wslc/common/store_model_probe.rs b/src/mxc-sdk/src/backends/wslc/common/store_model_probe.rs deleted file mode 100644 index 5b6094b14..000000000 --- a/src/mxc-sdk/src/backends/wslc/common/store_model_probe.rs +++ /dev/null @@ -1,313 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -//! Host probes for the multi-session store model (AB#64133563, spike PR 0). -//! -//! These answer four questions that decide whether the state-aware daemon can -//! hold more than one WSLc session, and what each session's image store must -//! look like. They need a live WSL2 host, so they are `#[ignore]`d and run -//! explicitly: -//! -//! ```text -//! cargo test -p mxc-sdk --features wslc store_model_probe -- --ignored --nocapture --test-threads=1 -//! ``` -//! -//! `--test-threads=1` matters: every probe drives the same per-user WSL -//! service, and two of them deliberately contend for one store. - -use std::fmt::Write as _; -use std::path::PathBuf; -use std::ptr; - -use super::container_steps::{create_daemon_session, sdk_error}; -use crate::mxc_common::logger::{Logger, Mode}; -use crate::mxc_common::models::ScriptResponse; -use crate::wslc_common::image::pull_image; -use crate::wslc_common::sdk_init::init_and_load_sdk; -use crate::wslc_common::wslc_bindings::{WslcImageInfo, WslcSdk, WslcSession, S_OK}; - -/// `ERROR_SHARING_VIOLATION` as an `HRESULT`. The symptom the work item reports -/// when a second live session opens a store another session already holds. -const E_SHARING_VIOLATION: i32 = 0x8007_0020u32 as i32; - -/// Image the cache-persistence probe pulls. Small, and the same default the -/// state-aware backend uses, so a developer's warm cache likely already has it. -const PROBE_IMAGE: &str = "alpine:latest"; - -/// A scratch store directory that deletes itself on drop, so a probe run does -/// not leave VHDs behind in `%TEMP%`. -struct ScratchStore { - path: PathBuf, -} - -impl ScratchStore { - fn new(tag: &str) -> Self { - let path = std::env::temp_dir().join(format!( - "mxc-store-probe-{tag}-{}", - uuid::Uuid::new_v4().simple() - )); - std::fs::create_dir_all(&path).expect("create scratch store"); - Self { path } - } - - fn as_str(&self) -> String { - self.path.to_string_lossy().to_string() - } -} - -impl Drop for ScratchStore { - fn drop(&mut self) { - // Best-effort: the WSL service may still hold the VHD briefly after - // release, and a probe must not fail during unwind. - let _ = std::fs::remove_dir_all(&self.path); - } -} - -/// Names of the images a session reports in its store. -/// -/// This is the cheapest call that proves a session handle is actually usable -/// rather than merely non-null, and it is the exact call the work item names as -/// failing with `ERROR_SHARING_VIOLATION`. -/// -/// # Safety -/// `sdk` must hold valid function pointers and `session` must be a live handle. -unsafe fn probe_list_images( - sdk: &WslcSdk, - session: WslcSession, -) -> Result, ScriptResponse> { - let mut images: *mut WslcImageInfo = ptr::null_mut(); - let mut count: u32 = 0; - let hr = sdk.WslcListSessionImages(session, &mut images, &mut count); - if hr != S_OK { - return Err(sdk_error("WslcListSessionImages failed", hr, "")); - } - if images.is_null() { - return Ok(Vec::new()); - } - - let mut names = Vec::with_capacity(count as usize); - for info in std::slice::from_raw_parts(images, count as usize) { - let bytes = std::slice::from_raw_parts(info.name.as_ptr().cast::(), info.name.len()); - let end = bytes.iter().position(|&b| b == 0).unwrap_or(bytes.len()); - if let Ok(name) = std::str::from_utf8(&bytes[..end]) { - names.push(name.to_string()); - } - } - windows::Win32::System::Com::CoTaskMemFree(Some(images as *const std::ffi::c_void)); - Ok(names) -} - -fn logger() -> Logger { - Logger::new(Mode::Console) -} - -/// Q1 — can one process, on one MTA thread, hold two live sessions with -/// separate stores, use both, and release both cleanly? -/// -/// This is the premise the whole multi-session design rests on. The work item -/// claims a probe confirmed it on SDK 2.9.3; this re-confirms it on the 2.9.9 -/// the repo now vendors, through the daemon's own `create_daemon_session`. -#[test] -#[ignore = "requires a WSL2 host with the WSLc SDK installed"] -fn two_sessions_with_separate_stores_coexist_in_one_process() { - let mut log = logger(); - let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); - - let store_a = ScratchStore::new("a"); - let store_b = ScratchStore::new("b"); - - // SAFETY: `sdk` is live and both stores are distinct empty directories. - let session_a = - unsafe { create_daemon_session(sdk, "mxc-probe-a", &store_a.as_str(), &mut log) } - .expect("first session must create"); - - let session_b = - unsafe { create_daemon_session(sdk, "mxc-probe-b", &store_b.as_str(), &mut log) } - .expect("second concurrent session with its own store must create"); - - // Both handles must be usable *while the other is live*, not merely - // created: a handle that cannot list its store is not a working session. - let images_a = unsafe { probe_list_images(sdk, session_a.as_raw()) } - .expect("session A must stay usable while B is live"); - let images_b = unsafe { probe_list_images(sdk, session_b.as_raw()) } - .expect("session B must be usable while A is live"); - - let mut out = String::new(); - let _ = writeln!( - out, - "[probe] A store={} images={:?}", - store_a.as_str(), - images_a - ); - let _ = writeln!( - out, - "[probe] B store={} images={:?}", - store_b.as_str(), - images_b - ); - println!("{out}"); - - // Interleave once more after both have been exercised, to catch a handle - // that only works until the other session touches the SDK. - unsafe { probe_list_images(sdk, session_a.as_raw()) } - .expect("session A must still be usable after B was used"); - - drop(session_b); - // A must survive B's release — per-session teardown releasing the wrong - // session is the specific hazard the daemon's current single-session - // `shutdown()` cannot express. - unsafe { probe_list_images(sdk, session_a.as_raw()) } - .expect("session A must survive session B's release"); - drop(session_a); -} - -/// Q2 — does a second live session on the *same* store really fail, and where? -/// -/// The work item reports `WslcListSessionImages failed: HRESULT 0x80070020`. -/// Whether that fires at create time or only on first use decides whether the -/// daemon can detect the collision before it mints a sandbox id. -#[test] -#[ignore = "requires a WSL2 host with the WSLc SDK installed"] -fn a_second_live_session_on_one_store_is_a_sharing_violation() { - let mut log = logger(); - let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); - - let shared = ScratchStore::new("shared"); - - // SAFETY: `sdk` is live; both calls target the same store on purpose. - let first = unsafe { create_daemon_session(sdk, "mxc-probe-1", &shared.as_str(), &mut log) } - .expect("first session on an unheld store must create"); - unsafe { probe_list_images(sdk, first.as_raw()) }.expect("first session must be usable"); - - let second = unsafe { create_daemon_session(sdk, "mxc-probe-2", &shared.as_str(), &mut log) }; - - match second { - Err(resp) => { - println!("[probe] second create FAILED: {}", resp.error_message); - println!("[probe] => collision is detectable at CREATE time"); - } - Ok(guard) => { - println!("[probe] second create SUCCEEDED; probing first use"); - let used = unsafe { probe_list_images(sdk, guard.as_raw()) }; - match used { - Err(resp) => { - println!("[probe] first use FAILED: {}", resp.error_message); - println!("[probe] => collision only surfaces on USE"); - assert!( - resp.error_message.contains("80070020") - || resp - .error_message - .contains(&format!("{E_SHARING_VIOLATION:#x}")), - "expected ERROR_SHARING_VIOLATION, got: {}", - resp.error_message - ); - } - Ok(images) => { - println!("[probe] first use SUCCEEDED: images={images:?}"); - println!( - "[probe] => NO collision on this SDK/host. The store-exclusivity \ - premise does not hold as stated; re-scope the store design." - ); - } - } - drop(guard); - } - } - drop(first); -} - -/// Q3 — THE store-model question. After a session releases its store, can a -/// later session reopen that same store and still see the images the first one -/// cached? -/// -/// Reopenability alone is not enough: a slot pool is only worth building if the -/// *cache survives* the lease. So this warms the store with a real pull, drops -/// the session, retakes the same store, and requires the image to still be -/// there. -/// -/// If it survives, a bounded pool of reusable store slots works: a session -/// leases a warm slot and returns it, so N sessions cost N stores but not N -/// *cold* stores, and `--setup-wslc` can warm the pool ahead of time. -/// -/// If it does not, every session is permanently cold and the only offline path -/// is `imageTarPath` — which would make per-session stores a serious regression -/// for the default no-egress posture. -#[test] -#[ignore = "requires a WSL2 host and network egress to pull alpine:latest"] -fn a_released_store_keeps_its_cache_for_the_next_session() { - let mut log = logger(); - let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); - - let slot = ScratchStore::new("slot"); - let store = slot.as_str(); - - // Lease 1: warm the slot with a real pull. - // SAFETY: `sdk` is live and the store is a fresh directory. - let first = unsafe { create_daemon_session(sdk, "mxc-probe-lease-1", &store, &mut log) } - .expect("first lease must create"); - let cold = unsafe { probe_list_images(sdk, first.as_raw()) }.expect("list before pull"); - println!("[probe] lease 1 cold images={cold:?}"); - assert!(cold.is_empty(), "a fresh slot must start cold"); - - unsafe { - pull_image( - sdk, - first.as_raw(), - PROBE_IMAGE, - Some(&store), - "[probe]", - &mut log, - ) - } - .expect("pull must succeed; this probe needs egress"); - let warmed = unsafe { probe_list_images(sdk, first.as_raw()) }.expect("list after pull"); - println!("[probe] lease 1 warmed images={warmed:?}"); - assert!( - !warmed.is_empty(), - "the pull must have populated the store; got {warmed:?}" - ); - - // Return the slot. - drop(first); - - // Lease 2: take the same slot again, as a pool would. - let second = unsafe { create_daemon_session(sdk, "mxc-probe-lease-2", &store, &mut log) } - .expect("a released store must be reopenable by a later session"); - let reopened = unsafe { probe_list_images(sdk, second.as_raw()) } - .expect("the reopened store must be readable"); - println!("[probe] lease 2 reopened images={reopened:?}"); - - assert_eq!( - warmed, reopened, - "a reopened store must present the cache it had at release; losing it \ - would make a warm slot pool pointless and leave every new session cold" - ); - drop(second); -} - -/// Q4 — baseline: a brand-new store really is empty. -/// -/// This is the premise behind `select_action`'s `RefuseNoEgress` arm: an -/// uncached image with no tar and no egress is refused. If a freshly minted -/// per-session store starts empty, then under the default deny-egress posture -/// every provision into a new session fails before it ever reaches the VM. -#[test] -#[ignore = "requires a WSL2 host with the WSLc SDK installed"] -fn a_fresh_store_starts_empty() { - let mut log = logger(); - let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); - - let fresh = ScratchStore::new("fresh"); - // SAFETY: `sdk` is live and the store is a fresh directory. - let session = - unsafe { create_daemon_session(sdk, "mxc-probe-fresh", &fresh.as_str(), &mut log) } - .expect("fresh store session must create"); - let images = unsafe { probe_list_images(sdk, session.as_raw()) }.expect("list fresh store"); - - println!("[probe] fresh store images={images:?}"); - assert!( - images.is_empty(), - "a newly minted per-session store must start cold; found {images:?}" - ); - drop(session); -} diff --git a/src/mxc-sdk/src/backends/wslc/common/store_slots.rs b/src/mxc-sdk/src/backends/wslc/common/store_slots.rs new file mode 100644 index 000000000..7a373b8fb --- /dev/null +++ b/src/mxc-sdk/src/backends/wslc/common/store_slots.rs @@ -0,0 +1,666 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! A bounded pool of reusable WSLc image stores for the state-aware daemon. +//! +//! A WSLc "store" is a host directory the caller hands to `WslcCreateSession`; +//! the SDK creates `storage.vhdx` inside it and mounts that VHDX into the +//! session's WSL2 utility VM, where it holds the session's pulled images. Two +//! facts about that VHDX shape this module: +//! +//! - Only one live session may hold it, and the SDK does not report the +//! conflict at create time. `WslcCreateSession` against a held store returns +//! a healthy-looking handle, and the violation surfaces on first use as +//! `HRESULT 0x80070020` (`ERROR_SHARING_VIOLATION`). +//! - Releasing a session only unmounts the file, so the images a lease pulled +//! are still there for the next one. +//! +//! The second fact is what makes a fixed set of leased slots work. A fresh +//! directory has no VHDX and therefore no images, and an uncached image under +//! the default deny-egress posture is refused before the VM is ever reached — +//! so minting a new directory per session would leave every session cold and +//! break isolated provisioning outright. Slots are leased and returned instead, +//! which also bounds disk at one multi-GiB VHDX per slot. +//! +//! Lease tracking is per-process and so cannot see a store another process +//! holds. The first fact means that cannot be delegated to the SDK either, so a +//! caller must probe the store itself after create — see +//! [`container_steps::validate_store_access`](super::container_steps::validate_store_access). + +use std::collections::BTreeSet; +use std::ffi::OsStr; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; + +/// Number of store slots the pool offers when the environment does not say. +pub const DEFAULT_SLOT_COUNT: usize = 4; + +/// Environment override for the slot count, as a positive whole number. +pub const SLOT_COUNT_ENV: &str = "MXC_WSLC_STORE_SLOTS"; + +/// Directory holding the slots, beneath the per-user application data root. +const POOL_LEAF: &str = "wslc-store-slots"; + +/// Resolve the pool root beneath `local_app_data`, falling back to `temp`. +/// +/// The fallback stays clear of `mxc-wslc-sessions`, the create-and-run runner's +/// default store: a daemon holding that directory fails every create-and-run +/// execution for as long as a sandbox is provisioned. +fn pool_root_in(local_app_data: Option<&OsStr>, temp: &Path) -> PathBuf { + match local_app_data.filter(|dir| !dir.is_empty()) { + Some(dir) => Path::new(dir).join("mxc").join(POOL_LEAF), + None => temp.join("mxc").join(POOL_LEAF), + } +} + +/// The per-user directory holding this host's store slots. +pub fn pool_root() -> PathBuf { + pool_root_in( + std::env::var_os("LOCALAPPDATA").as_deref(), + &std::env::temp_dir(), + ) +} + +/// Read a slot count from `raw`, falling back to [`DEFAULT_SLOT_COUNT`] when it +/// is unset, empty, non-numeric, or zero. +fn slot_count_from(raw: Option<&OsStr>) -> usize { + raw.and_then(OsStr::to_str) + .and_then(|text| text.trim().parse::().ok()) + .filter(|count| *count > 0) + .unwrap_or(DEFAULT_SLOT_COUNT) +} + +/// The slot count this process uses, honouring [`SLOT_COUNT_ENV`]. +pub fn configured_slot_count() -> usize { + slot_count_from(std::env::var_os(SLOT_COUNT_ENV).as_deref()) +} + +/// The path of slot `index` beneath `root`. +fn slot_path_in(root: &Path, index: usize) -> PathBuf { + root.join(format!("slot-{index}")) +} + +/// The paths of the first `count` slots, lowest index first. +/// +/// Warming opens each store in turn rather than holding one for a session's +/// lifetime, so it takes paths without leasing. +pub fn slot_paths(count: usize) -> Vec { + let root = pool_root(); + (0..count).map(|index| slot_path_in(&root, index)).collect() +} + +/// Why a slot could not be leased. +#[derive(Debug)] +pub enum StoreSlotError { + /// Every slot in the pool is already leased by this process. + Exhausted { capacity: usize }, + + /// The slot directory could not be created. + Io { + path: PathBuf, + source: std::io::Error, + }, + + /// The caller refused the slot, typically because another process holds it. + Refused { + index: usize, + path: PathBuf, + source: E, + }, +} + +impl std::fmt::Display for StoreSlotError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + StoreSlotError::Exhausted { capacity } => write!( + f, + "all {capacity} WSLc store slots are in use; raise {SLOT_COUNT_ENV} to offer more" + ), + StoreSlotError::Io { path, source } => write!( + f, + "could not create the WSLc store slot at {}: {source}", + path.display() + ), + + // The refusal itself is in `source`, which carries the caller's own + // error type and is reported by whoever asked for the lease. + StoreSlotError::Refused { index, path, .. } => write!( + f, + "the WSLc store slot {index} at {} could not be opened", + path.display() + ), + } + } +} + +impl std::error::Error for StoreSlotError {} + +/// A fixed set of reusable image stores, leased one at a time. +#[derive(Debug)] +pub struct StoreSlotPool { + root: PathBuf, + capacity: usize, + leased: Arc>>, +} + +impl StoreSlotPool { + /// A pool over the per-user slot directory at the configured capacity. + pub fn new() -> Self { + Self::with_root(pool_root(), configured_slot_count()) + } + + /// A pool over an explicit root, for tests and tools that place slots + /// somewhere other than the per-user directory. + pub fn with_root(root: PathBuf, capacity: usize) -> Self { + Self { + root, + capacity, + leased: Arc::new(Mutex::new(BTreeSet::new())), + } + } + + /// How many slots this pool can lease at once. + pub fn capacity(&self) -> usize { + self.capacity + } + + /// The path of slot `index`, whether or not it is leased. + pub fn slot_path(&self, index: usize) -> PathBuf { + slot_path_in(&self.root, index) + } + + /// Lease the lowest free slot and open it with `open`. + /// + /// A refusal fails the lease rather than moving to another slot: the + /// remaining slots are cold, and an uncached image under the default + /// deny-egress posture is refused, so relocating would report a missing + /// image instead of the held store that actually caused it. + pub fn lease_with( + &self, + open: impl FnOnce(&Path) -> Result, + ) -> Result<(StoreSlotLease, T), StoreSlotError> { + let index = self.take_lowest_free().ok_or(StoreSlotError::Exhausted { + capacity: self.capacity, + })?; + let path = self.slot_path(index); + + // Built before anything can fail, so every error path below returns the + // slot as it unwinds. + let lease = StoreSlotLease { + index, + path: path.clone(), + leased: Arc::clone(&self.leased), + }; + + if let Err(source) = std::fs::create_dir_all(&path) { + return Err(StoreSlotError::Io { path, source }); + } + + match open(&path) { + Ok(opened) => Ok((lease, opened)), + Err(source) => Err(StoreSlotError::Refused { + index, + path, + source, + }), + } + } + + fn take_lowest_free(&self) -> Option { + let mut leased = self.leased.lock().ok()?; + let free = (0..self.capacity).find(|index| !leased.contains(index))?; + leased.insert(free); + Some(free) + } +} + +impl Default for StoreSlotPool { + fn default() -> Self { + Self::new() + } +} + +/// A slot held for the lifetime of one session, returned to the pool on drop. +/// +/// Forgetting a lease keeps its slot held. A caller that leaks the session +/// holding the store must leak the lease with it, since the VHDX stays mounted +/// and reissuing the slot would hand out a store nothing can open. +#[derive(Debug)] +pub struct StoreSlotLease { + index: usize, + path: PathBuf, + leased: Arc>>, +} + +impl StoreSlotLease { + /// Which slot this lease holds. + pub fn index(&self) -> usize { + self.index + } + + /// The store directory this lease holds. + pub fn path(&self) -> &Path { + &self.path + } +} + +impl Drop for StoreSlotLease { + fn drop(&mut self) { + if let Ok(mut leased) = self.leased.lock() { + leased.remove(&self.index); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A pool of `capacity` slots under a directory that deletes itself. + fn pool(capacity: usize) -> (tempfile::TempDir, StoreSlotPool) { + let root = tempfile::tempdir().expect("create scratch pool root"); + let pool = StoreSlotPool::with_root(root.path().to_path_buf(), capacity); + (root, pool) + } + + /// Accepts whatever slot it is offered. + fn accept(path: &Path) -> Result { + Ok(path.to_path_buf()) + } + + #[test] + fn a_lease_takes_the_lowest_free_slot() { + let (_root, pool) = pool(4); + + let (first, _) = pool.lease_with(accept).expect("first lease"); + let (second, _) = pool.lease_with(accept).expect("second lease"); + let (third, _) = pool.lease_with(accept).expect("third lease"); + + assert_eq!( + [first.index(), second.index(), third.index()], + [0, 1, 2], + "slots must be handed out lowest first, so warming slot 0 is enough \ + for a daemon that holds one session" + ); + } + + #[test] + fn a_live_lease_is_never_handed_out_twice() { + let (_root, pool) = pool(4); + + let (held, _) = pool.lease_with(accept).expect("first lease"); + let (other, _) = pool.lease_with(accept).expect("second lease"); + + assert_ne!( + held.path(), + other.path(), + "two live sessions sharing one store is the sharing violation this \ + pool exists to prevent" + ); + } + + #[test] + fn a_returned_lease_frees_its_slot_for_the_next_caller() { + let (_root, pool) = pool(4); + + let (first, _) = pool.lease_with(accept).expect("first lease"); + let returned = first.path().to_path_buf(); + drop(first); + + let (second, _) = pool.lease_with(accept).expect("re-lease after return"); + + assert_eq!( + second.path(), + returned, + "a returned slot must be re-leasable; its images survive the \ + release, and reusing it is what keeps a slot warm" + ); + } + + #[test] + fn a_forgotten_lease_keeps_its_slot_held() { + let (_root, pool) = pool(2); + + let (leaked, _) = pool.lease_with(accept).expect("first lease"); + let leaked_path = leaked.path().to_path_buf(); + std::mem::forget(leaked); + + let (next, _) = pool.lease_with(accept).expect("second lease"); + + assert_ne!( + next.path(), + leaked_path, + "teardown leaks a lease when it leaks the session holding the store, \ + and that store stays mounted" + ); + } + + #[test] + fn a_full_pool_reports_exhaustion() { + let (_root, pool) = pool(2); + + let (_first, _) = pool.lease_with(accept).expect("first lease"); + let (_second, _) = pool.lease_with(accept).expect("second lease"); + + match pool.lease_with(accept) { + Err(StoreSlotError::Exhausted { capacity }) => assert_eq!(capacity, 2), + other => panic!("a full pool must report exhaustion, got {other:?}"), + } + } + + #[test] + fn a_refused_slot_fails_the_lease_rather_than_moving_to_another() { + let (_root, pool) = pool(4); + + let refusal = pool.lease_with(|_| Err::<(), _>("store is held".to_string())); + + match refusal { + Err(StoreSlotError::Refused { index, source, .. }) => { + assert_eq!(index, 0); + assert_eq!(source, "store is held"); + } + other => panic!("a refused slot must fail the lease, got {other:?}"), + } + + let (next, _) = pool.lease_with(accept).expect("lease after a refusal"); + assert_eq!( + next.index(), + 0, + "a refused slot must be returned to the pool" + ); + } + + #[test] + fn a_lease_creates_its_slot_directory() { + let (_root, pool) = pool(1); + + let (lease, _) = pool.lease_with(accept).expect("lease"); + + assert!( + lease.path().is_dir(), + "the SDK is handed this path to create storage.vhdx in" + ); + } + + #[test] + fn an_io_failure_returns_the_slot() { + let root = tempfile::tempdir().expect("create scratch pool root"); + let blocker = root.path().join("blocker"); + std::fs::write(&blocker, b"").expect("write blocker"); + + // Rooting the pool at a regular file makes `create_dir_all` fail + // without depending on permissions a test cannot rely on. + let pool = StoreSlotPool::with_root(blocker, 1); + + match pool.lease_with(accept) { + Err(StoreSlotError::Io { .. }) => {} + other => panic!("an uncreatable slot must report the io failure, got {other:?}"), + } + + match pool.lease_with(accept) { + Err(StoreSlotError::Io { .. }) => {} + other => panic!("the slot must have been returned, got {other:?}"), + } + } + + #[test] + fn the_pool_root_lives_beside_the_users_other_application_data() { + let root = pool_root_in( + Some(OsStr::new(r"C:\Users\u\AppData\Local")), + Path::new(r"C:\Temp"), + ); + + assert_eq!( + root, + PathBuf::from(r"C:\Users\u\AppData\Local\mxc\wslc-store-slots") + ); + } + + #[test] + fn the_pool_root_falls_back_without_local_application_data() { + for unusable in [None, Some(OsStr::new(""))] { + let root = pool_root_in(unusable, Path::new(r"C:\Temp")); + + assert_eq!(root, PathBuf::from(r"C:\Temp\mxc\wslc-store-slots")); + assert!( + !root.ends_with("mxc-wslc-sessions"), + "falling back onto the create-and-run store would reintroduce the \ + collision the pool exists to remove" + ); + } + } + + #[test] + fn the_slot_count_falls_back_for_a_value_it_cannot_use() { + for unusable in [ + None, + Some(""), + Some(" "), + Some("four"), + Some("0"), + Some("-1"), + ] { + assert_eq!( + slot_count_from(unusable.map(OsStr::new)), + DEFAULT_SLOT_COUNT, + "{unusable:?} must not change the slot count" + ); + } + + assert_eq!(slot_count_from(Some(OsStr::new("6"))), 6); + assert_eq!(slot_count_from(Some(OsStr::new(" 2 "))), 2); + } + + #[test] + fn slot_paths_are_numbered_from_zero() { + let root = PathBuf::from(r"C:\pool"); + + let paths: Vec<_> = (0..3).map(|index| slot_path_in(&root, index)).collect(); + + assert_eq!( + paths, + [ + PathBuf::from(r"C:\pool\slot-0"), + PathBuf::from(r"C:\pool\slot-1"), + PathBuf::from(r"C:\pool\slot-2"), + ] + ); + } +} + +/// Pool behaviour that only a live WSL2 host can answer. +/// +/// These drive the real SDK, so they are `#[ignore]`d and run explicitly: +/// +/// ```text +/// cargo test -p mxc-sdk --features wslc --lib store_slots -- --ignored --nocapture --test-threads=1 +/// ``` +/// +/// `--test-threads=1` matters: every test here drives the same per-user WSL +/// service, and one of them deliberately contends for a store. +#[cfg(test)] +mod host_tests { + use super::*; + + use crate::mxc_common::logger::{Logger, Mode}; + use crate::wslc_common::container_steps::{create_daemon_session, list_session_image_names}; + use crate::wslc_common::image::pull_image; + use crate::wslc_common::sdk_init::init_and_load_sdk; + + /// Small, and the default the state-aware backend uses, so a developer's + /// warm cache likely already has it. + const CACHE_IMAGE: &str = "alpine:latest"; + + /// A pool rooted somewhere disposable, so a run leaves no multi-GiB stores + /// behind in the real per-user pool. + fn scratch_pool(capacity: usize) -> (tempfile::TempDir, StoreSlotPool) { + let root = tempfile::tempdir().expect("create scratch pool root"); + let pool = StoreSlotPool::with_root(root.path().to_path_buf(), capacity); + (root, pool) + } + + #[test] + #[ignore = "requires a WSL2 host with the WSLc SDK installed"] + fn two_leased_slots_host_two_live_sessions() { + let mut log = Logger::new(Mode::Console); + let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); + let (_root, pool) = scratch_pool(4); + + let (lease_a, session_a) = pool + .lease_with(|path| { + // SAFETY: `sdk` is live and the pool hands out a distinct store. + unsafe { + create_daemon_session( + sdk, + "mxc-slot-probe-a", + &path.to_string_lossy(), + &mut log, + ) + } + }) + .expect("first lease"); + let (lease_b, session_b) = pool + .lease_with(|path| { + // SAFETY: `sdk` is live and the pool hands out a distinct store. + unsafe { + create_daemon_session( + sdk, + "mxc-slot-probe-b", + &path.to_string_lossy(), + &mut log, + ) + } + }) + .expect("second concurrent lease"); + + assert_ne!(lease_a.path(), lease_b.path()); + + // Both handles have to be usable *while the other is live*, not merely + // created: a session that cannot read its store is not a working one. + // SAFETY: both sessions are live. + unsafe { list_session_image_names(sdk, session_a.as_raw()) } + .expect("session A must stay usable while B is live"); + // SAFETY: both sessions are live. + unsafe { list_session_image_names(sdk, session_b.as_raw()) } + .expect("session B must be usable while A is live"); + // SAFETY: session A is live. + unsafe { list_session_image_names(sdk, session_a.as_raw()) } + .expect("session A must still be usable after B was used"); + + drop(session_b); + drop(lease_b); + + // SAFETY: session A is live. + unsafe { list_session_image_names(sdk, session_a.as_raw()) } + .expect("session A must survive session B's release"); + } + + #[test] + #[ignore = "requires a WSL2 host with the WSLc SDK installed"] + fn a_session_is_refused_when_another_process_holds_its_store() { + let mut log = Logger::new(Mode::Console); + let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); + let (_root, pool) = scratch_pool(1); + + let (lease, holder) = pool + .lease_with(|path| { + // SAFETY: `sdk` is live and the store is a fresh directory. + unsafe { + create_daemon_session(sdk, "mxc-slot-holder", &path.to_string_lossy(), &mut log) + } + }) + .expect("first lease"); + let store = lease.path().to_string_lossy().to_string(); + + // The pool would never hand this slot out again, so take it the way a + // process outside this pool's bookkeeping would. + // SAFETY: `sdk` is live; this targets the held store on purpose. + let second = unsafe { create_daemon_session(sdk, "mxc-slot-intruder", &store, &mut log) }; + + let refusal = match second { + Err(resp) => resp, + Ok(_) => panic!( + "a session on a held store must be refused at create; letting it \ + through mints a sandbox id that dies at an arbitrary later phase" + ), + }; + assert!( + refusal.error_message.contains("80070020"), + "the refusal must carry the sharing violation: {}", + refusal.error_message + ); + assert!( + refusal.error_message.contains(&store), + "the refusal must name the store that is held: {}", + refusal.error_message + ); + + // SAFETY: the holder is still live. + unsafe { list_session_image_names(sdk, holder.as_raw()) } + .expect("refusing the second session must leave the holder usable"); + } + + #[test] + #[ignore = "requires a WSL2 host and network egress to pull alpine:latest"] + fn a_returned_slot_keeps_its_cache_for_the_next_lease() { + let mut log = Logger::new(Mode::Console); + let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); + let (_root, pool) = scratch_pool(1); + + let (lease, session) = pool + .lease_with(|path| { + // SAFETY: `sdk` is live and the pool hands out a distinct store. + unsafe { + create_daemon_session(sdk, "mxc-slot-lease", &path.to_string_lossy(), &mut log) + } + }) + .expect("first lease"); + let store = lease.path().to_string_lossy().to_string(); + + // SAFETY: the session is live. + let cold = unsafe { list_session_image_names(sdk, session.as_raw()) }.expect("list cold"); + assert!( + cold.is_empty(), + "a slot that has never been leased holds no images, which is why a \ + fresh store breaks deny-egress provisioning; got {cold:?}" + ); + + // SAFETY: the session is live and the store is the one it holds. + unsafe { + pull_image( + sdk, + session.as_raw(), + CACHE_IMAGE, + Some(&store), + "[slot]", + &mut log, + ) + } + .expect("pull must succeed; this test needs egress"); + // SAFETY: the session is live. + let warmed = unsafe { list_session_image_names(sdk, session.as_raw()) }.expect("list warm"); + assert!(!warmed.is_empty(), "the pull must have filled the store"); + + drop(session); + drop(lease); + + let (next_lease, next_session) = pool + .lease_with(|path| { + // SAFETY: `sdk` is live and the pool hands out a distinct store. + unsafe { + create_daemon_session(sdk, "mxc-slot-lease", &path.to_string_lossy(), &mut log) + } + }) + .expect("re-lease after return"); + assert_eq!(next_lease.path().to_string_lossy(), store); + // SAFETY: the session is live. + let reopened = + unsafe { list_session_image_names(sdk, next_session.as_raw()) }.expect("list reopened"); + + assert_eq!( + warmed, reopened, + "a re-leased slot must present the cache it had at release; losing it \ + would leave every lease cold and make the pool pointless" + ); + } +} diff --git a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs index 6fc36d026..b17dbc6e3 100644 --- a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs +++ b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs @@ -16,7 +16,9 @@ //! utility VM, booted lazily on first provision and amortised across all //! sandboxes) and a refcounted `sandbox_id -> container` map. The session is //! released when the last container is deprovisioned and the idle timeout -//! elapses. +//! elapses. Its image store is leased from +//! [`mxc_sdk::wslc_common::store_slots`], which keeps the daemon off the store +//! create-and-run execution defaults to. //! //! Each phase drives the real WSLc SDK via the reusable steps in //! [`mxc_sdk::wslc_common::container_steps`] and [`mxc_sdk::wslc_common::image`]: `provision` @@ -47,6 +49,7 @@ use mxc_sdk::wslc_common::daemon_protocol::{ use mxc_sdk::wslc_common::image; use mxc_sdk::wslc_common::policy_mapping; use mxc_sdk::wslc_common::process_env::EnvScope; +use mxc_sdk::wslc_common::store_slots::{StoreSlotError, StoreSlotLease, StoreSlotPool}; use mxc_sdk::wslc_common::wslc_bindings::{ WslcContainer, WslcContainerGuard, WslcContainerNetworkingMode, WslcSdk, WslcSessionGuard, }; @@ -76,15 +79,6 @@ fn wait_for_execs_in_flight(in_flight: &AtomicUsize, budget: Duration) -> bool { true } -/// Default on-disk WSLc image/session store. Matches the one-shot runner's -/// default so both surfaces read and fill the same cache. -fn default_storage_path() -> String { - std::env::temp_dir() - .join("mxc-wslc-sessions") - .to_string_lossy() - .to_string() -} - /// Convert a step helper's `failure_phase` into a typed failure, since the wire /// carries only a message and an [`ErrKind`]. fn sr_err(resp: ScriptResponse) -> WorkerError { @@ -818,6 +812,12 @@ struct Worker { next_pull_token: u64, session: Option, + /// The store the live session holds, returned to the pool when the session + /// is released. Teardown that leaks the session leaks this with it, since + /// the leaked session keeps the store's VHDX mounted. + store_lease: Option, + store_slots: StoreSlotPool, + /// Boxed so the address an off-worker pull or exec was handed survives this /// struct being moved or the field being taken, which teardown does to leak /// the SDK rather than unload it under a running thread. @@ -831,6 +831,8 @@ impl Worker { start_run: start_run_on_sdk, sdk: None, session: None, + store_lease: None, + store_slots: StoreSlotPool::new(), containers: HashMap::new(), pending: HashMap::new(), exec_in_flight: HashMap::new(), @@ -848,19 +850,33 @@ impl Worker { self.sdk = Some(Box::new(sdk)); } if self.session.is_none() { - let storage = default_storage_path(); - let sdk = self.sdk.as_ref().expect("sdk loaded above"); - // SAFETY: `sdk` holds valid pointers and COM is initialised. - let session = unsafe { - container_steps::create_daemon_session( - sdk, - SESSION_NAME, - &storage, - &mut self.logger, - ) - } - .map_err(sr_err)?; + let Self { + store_slots, + sdk, + logger, + .. + } = self; + let sdk = sdk.as_ref().expect("sdk loaded above"); + + let (lease, session) = store_slots + .lease_with(|store| { + // SAFETY: `sdk` holds valid pointers and COM is initialised. + unsafe { + container_steps::create_daemon_session( + sdk, + SESSION_NAME, + &store.to_string_lossy(), + logger, + ) + } + }) + .map_err(|e| match e { + StoreSlotError::Refused { source, .. } => sr_err(source), + exhausted_or_io => WorkerError::Backend(anyhow::anyhow!("{exhausted_or_io}")), + })?; + self.session = Some(session); + self.store_lease = Some(lease); } Ok(()) } @@ -1380,6 +1396,7 @@ impl Worker { if execs_in_flight > 0 || pull_outstanding { std::mem::forget(std::mem::take(&mut self.containers)); std::mem::forget(self.session.take()); + std::mem::forget(self.store_lease.take()); std::mem::forget(self.sdk.take()); } @@ -1481,6 +1498,7 @@ impl Worker { ); std::mem::forget(std::mem::take(&mut self.containers)); std::mem::forget(self.session.take()); + std::mem::forget(self.store_lease.take()); std::mem::forget(self.sdk.take()); return; } @@ -1517,12 +1535,14 @@ impl Worker { releasing a handle it is using", ); std::mem::forget(self.session.take()); + std::mem::forget(self.store_lease.take()); std::mem::forget(self.sdk.take()); return; } // Session guard drops before the SDK unloads the DLL. self.session = None; + self.store_lease = None; self.sdk = None; } } @@ -3070,11 +3090,12 @@ mod tests { // Exercises the real SDK path end to end: provision (boot VM + create // container) → start → exec → stop → deprovision → refcount back to 0. It // provisions with the default isolated posture, which refuses a registry - // pull, so `alpine:latest` has to be in the daemon session cache already - // (`%TEMP%\mxc-wslc-sessions`). It is `#[ignore]`d and run explicitly with - // `cargo test -p wxc_wslc_daemon -- --ignored`. + // pull, so `alpine:latest` has to be cached in the store slot this worker + // leases — slot 0 of the pool, since it holds one session. Warm it with + // `scripts\setup-wslc.ps1 -DaemonSlots 1`. It is `#[ignore]`d and run + // explicitly with `cargo test -p mxc-sdk --features wslc --lib -- --ignored`. #[tokio::test] - #[ignore = "requires a WSL2 host with alpine:latest already in the daemon session cache"] + #[ignore = "requires a WSL2 host with alpine:latest cached in daemon store slot 0"] async fn full_lifecycle_on_wsl_host() { let handle = spawn().unwrap(); assert_eq!(count(&handle).await, 0); diff --git a/src/mxc-sdk/tests/wslc_daemon_ipc.rs b/src/mxc-sdk/tests/wslc_daemon_ipc.rs index e85c2bdad..814e36d66 100644 --- a/src/mxc-sdk/tests/wslc_daemon_ipc.rs +++ b/src/mxc-sdk/tests/wslc_daemon_ipc.rs @@ -276,11 +276,12 @@ fn ping_round_trip_over_pipe() { /// Full state-aware lifecycle over the pipe: provision -> start -> exec -> stop /// -> deprovision. Provisions with the default isolated posture, which refuses -/// a registry pull, so `alpine:latest` has to be in the daemon session cache -/// already (`%TEMP%\mxc-wslc-sessions`). Run explicitly with +/// a registry pull, so `alpine:latest` has to be cached in the store slot the +/// daemon leases — slot 0 of the pool, since it holds one session. Warm it with +/// `scripts\setup-wslc.ps1 -DaemonSlots 1`. Run explicitly with /// `cargo test -p mxc-sdk --features wslc --test wslc_daemon_ipc -- --ignored`. #[test] -#[ignore = "requires a WSL2 host with alpine:latest already in the daemon session cache"] +#[ignore = "requires a WSL2 host with alpine:latest cached in daemon store slot 0"] fn full_lifecycle_over_pipe() { use mxc_sdk::wslc_common::daemon_protocol::{ DeprovisionConfig, ExecConfig, ProvisionConfig, StartConfig, StopConfig, diff --git a/src/tools/wxc/src/main.rs b/src/tools/wxc/src/main.rs index be8a0ea9c..4ed41b832 100644 --- a/src/tools/wxc/src/main.rs +++ b/src/tools/wxc/src/main.rs @@ -72,6 +72,7 @@ struct Cli { "setup_wslc", "image", "storage_path", + "wslc_daemon_slots", "probe", "force_reclaim" ] @@ -135,6 +136,14 @@ struct Cli { #[arg(long = "storage-path", requires = "setup_wslc")] storage_path: Option, + /// Also warm the first N container-lifecycle store slots, which the + /// state-aware daemon leases from and `--storage-path` does not reach. + /// Defaults to 0 so warming a create-and-run cache costs one pull per + /// image; each slot warmed adds a pull and a multi-GiB store. + /// Requires `--setup-wslc`. + #[arg(long = "wslc-daemon-slots", value_name = "N", requires = "setup_wslc")] + wslc_daemon_slots: Option, + /// Run the fallback detector and emit JSON, without spawning a sandbox. #[arg(long)] probe: bool, @@ -157,6 +166,7 @@ struct Cli { "setup_wslc", "image", "storage_path", + "wslc_daemon_slots", "probe", "force_reclaim", "operation", @@ -1215,6 +1225,7 @@ fn main() { mxc_sdk::wslc_common::image::setup_pull_image( image, cli.storage_path.as_deref(), + cli.wslc_daemon_slots.unwrap_or(0), &mut logger, ) }; diff --git a/tests/scripts/run_wslc_all_tests.ps1 b/tests/scripts/run_wslc_all_tests.ps1 index 8c18efd39..09370fc90 100644 --- a/tests/scripts/run_wslc_all_tests.ps1 +++ b/tests/scripts/run_wslc_all_tests.ps1 @@ -77,7 +77,8 @@ if (-not $WxcExec -or -not (Test-Path $WxcExec)) { # Preflight: warm the WSLC image cache so per-test timings stay clear of one-off # download cost. The runner pulls on a miss by itself, so -SkipSetup is a -# supported way to exercise that path instead. +# supported way to exercise that path instead. These are create-and-run tests, +# so the lifecycle store slots are left alone. if (-not $SkipSetup) { $SetupScript = Join-Path $RepoRoot "scripts\setup-wslc.ps1" if (Test-Path $SetupScript) { @@ -92,7 +93,7 @@ if (-not $SkipSetup) { "ghcr.io/linuxserver/baseimage-alpine:3.21", "quay.io/fedora/fedora-minimal:latest" ) - & $SetupScript -WxcExecPath $WxcExec -Image $images -Force + & $SetupScript -WxcExecPath $WxcExec -Image $images -DaemonSlots 0 -Force if ($LASTEXITCODE -ne 0) { Write-Host "WARN: setup-wslc.ps1 reported failures; continuing with tests anyway." -ForegroundColor Yellow } diff --git a/tests/scripts/run_wslc_state_aware_tests.ps1 b/tests/scripts/run_wslc_state_aware_tests.ps1 index c2c7e03b2..5d1ff83d0 100644 --- a/tests/scripts/run_wslc_state_aware_tests.ps1 +++ b/tests/scripts/run_wslc_state_aware_tests.ps1 @@ -124,11 +124,15 @@ if (-not $ConfigDir) { # ---------------- Image pre-pull preflight ---------------- +# The daemon leases its store from the slot pool, not from +# %TEMP%\mxc-wslc-sessions, and the fixtures below provision with the isolated +# posture, which refuses a registry pull. Slot 0 is the one a daemon holding a +# single session leases, so that is the store these images have to be in. if (-not $SkipSetup) { $SetupScript = Join-Path $RepoRoot "scripts\setup-wslc.ps1" if (Test-Path $SetupScript) { Write-Host "Pre-pulling WSLc images (pass -SkipSetup to skip)..." -ForegroundColor Cyan - & $SetupScript -WxcExecPath $WxcExec -Image @("alpine:latest", "python:3.12-alpine") -Force + & $SetupScript -WxcExecPath $WxcExec -Image @("alpine:latest", "python:3.12-alpine") -DaemonSlots 1 -Force if ($LASTEXITCODE -ne 0) { Write-Host "WARN: setup-wslc.ps1 reported failures; continuing anyway." -ForegroundColor Yellow } From eb4ff62191b8d3dc46786d4c5f32bcae4e447402 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Mon, 12 Oct 2026 15:39:00 -0700 Subject: [PATCH 3/6] More changes Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 763fd4b3-1dff-4398-8b27-e82d89a02a35 --- .../wslc/wsl-container-getting-started.md | 12 +- docs/backends/wslc/wslc-state-aware.md | 14 +- scripts/setup-wslc.ps1 | 19 +- .../backends/wslc/common/container_steps.rs | 109 ++++-- src/mxc-sdk/src/backends/wslc/common/image.rs | 320 +++++++++++------- .../src/backends/wslc/common/store_slots.rs | 32 +- .../wslc/common/wsl_container_runner.rs | 2 +- .../src/bin/wslc_daemon/session_manager.rs | 11 +- src/tools/wxc/src/main.rs | 9 +- tests/scripts/README.md | 3 +- tests/scripts/run_wslc_all_tests.ps1 | 12 +- tests/scripts/run_wslc_state_aware_tests.ps1 | 36 ++ 12 files changed, 399 insertions(+), 180 deletions(-) diff --git a/docs/backends/wslc/wsl-container-getting-started.md b/docs/backends/wslc/wsl-container-getting-started.md index 2553ed71e..331807fab 100644 --- a/docs/backends/wslc/wsl-container-getting-started.md +++ b/docs/backends/wslc/wsl-container-getting-started.md @@ -141,13 +141,13 @@ cost once per image, not once per run. > `wxc-exec.exe`), otherwise the runner will not find what you just > pulled and will pull it again under its own path. -> **Container lifecycle operations use a different store.** The state-aware +> **Container lifecycle operations use a different store.** The lifecycle > daemon leases its store from a pool under -> `%LOCALAPPDATA%\mxc\wslc-store-slots`, which `-StoragePath` does not reach. -> `setup-wslc.ps1` warms the first slot as well by default; `-DaemonSlots 0` -> skips it, and a larger count warms more slots at a pull and a multi-GiB store -> each. On `wxc-exec.exe` the equivalent is `--wslc-daemon-slots `, which -> defaults to `0`. See +> `%LOCALAPPDATA%\mxc\wslc-store-slots` (or `%TEMP%\mxc\wslc-store-slots` when +> `%LOCALAPPDATA%` is unresolvable), which `-StoragePath` does not reach. +> Warm it with `setup-wslc.ps1 -DaemonSlots 1`, or `--wslc-daemon-slots 1` on +> `wxc-exec.exe`; both default to 0, and each slot warmed costs another pull +> and another multi-GiB store. See > [the lifecycle guide](wslc-state-aware.md#image-store). > **Bring-up reaches the network.** A cache miss makes the host fetch diff --git a/docs/backends/wslc/wslc-state-aware.md b/docs/backends/wslc/wslc-state-aware.md index a0d33628b..476316e41 100644 --- a/docs/backends/wslc/wslc-state-aware.md +++ b/docs/backends/wslc/wslc-state-aware.md @@ -163,9 +163,17 @@ create-and-run WSLc execution on the same host failed with A returned slot keeps its images — that is the point of leasing rather than minting. A slot that has never been leased holds nothing at all, and an uncached image under the isolated posture is refused before the VM is reached, so a cold -slot fails a provision outright rather than pulling. Warm the pool with -`scripts\setup-wslc.ps1 -DaemonSlots ` (see -[the getting-started guide](wsl-container-getting-started.md#step-3--container-images-optional-pre-pull)). +slot fails a provision outright rather than pulling. Warm the pool with either: + +```powershell +.\scripts\setup-wslc.ps1 -Image alpine:latest -DaemonSlots 1 +wxc-exec.exe --setup-wslc --image alpine:latest --wslc-daemon-slots 1 +``` + +Both default to 0 and warm slots from 0 up, so `1` is what a daemon holding one +session needs. A count above the pool capacity is clamped to it. See +[the getting-started guide](wsl-container-getting-started.md#step-3--container-images-optional-pre-pull) +for the create-and-run store, which these do not replace. Slots are identified by index and are never repurposed, so nothing accumulates and no startup collection is needed; disk is bounded by one VHDX per slot. diff --git a/scripts/setup-wslc.ps1 b/scripts/setup-wslc.ps1 index 821418fd4..dfca643cc 100644 --- a/scripts/setup-wslc.ps1 +++ b/scripts/setup-wslc.ps1 @@ -19,7 +19,7 @@ (the `wslc.storagePath` field of the config, or the runner's default of `%TEMP%\mxc-wslc-sessions` when omitted). - Container lifecycle operations do not read that store. The state-aware + Container lifecycle operations do not read that store. The lifecycle daemon leases its store from a pool under %LOCALAPPDATA%\mxc\wslc-store-slots, so -DaemonSlots warms those slots separately. @@ -40,14 +40,15 @@ (`%TEMP%\mxc-wslc-sessions`) is used. Set this if your runtime configs override `wslc.storagePath`. This reaches create-and-run execution only; use -DaemonSlots for the container-lifecycle store. - .PARAMETER DaemonSlots How many container-lifecycle store slots to warm as well, from slot 0 up. - The state-aware daemon leases its store from a separate pool under + The lifecycle daemon leases its store from a separate pool under %LOCALAPPDATA%\mxc\wslc-store-slots, which -StoragePath does not reach, and - a slot that has never been leased holds no images. Defaults to 1, which is - what a daemon holding one session uses. Pass 0 to warm the create-and-run - cache only; each slot costs another pull and another multi-GiB store. + a slot that has never been leased holds no images. Defaults to 0, matching + `wxc-exec.exe --wslc-daemon-slots`, so warming a create-and-run cache stays + one pull per image. Pass 1 before running container lifecycle operations + against an isolated posture; each slot costs another pull and another + multi-GiB store. .PARAMETER DebugLogs Enable verbose logging from wxc-exec (passes `--debug`). @@ -68,8 +69,8 @@ Pulls into a non-default cache directory. .EXAMPLE - .\setup-wslc.ps1 -DaemonSlots 0 - Pulls for create-and-run execution only, leaving the lifecycle slots alone. + .\setup-wslc.ps1 -DaemonSlots 1 + Also warms the store slot the container-lifecycle daemon leases. #> [CmdletBinding()] @@ -78,7 +79,7 @@ param( [string]$WxcExecPath, [string]$StoragePath, [ValidateRange(0, 64)] - [int]$DaemonSlots = 1, + [int]$DaemonSlots = 0, [switch]$DebugLogs, [switch]$Force ) diff --git a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs index 2ce4cb6a4..54153c3ca 100644 --- a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs +++ b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs @@ -765,25 +765,31 @@ pub unsafe fn validate_store_access( session: WslcSession, storage_path: &str, ) -> Result<(), ScriptResponse> { - list_image_names(sdk, session).map(|_| ()).map_err(|hr| { - if hr == E_SHARING_VIOLATION { - sdk_error( - "WSLc store unavailable", - hr, - &format!( - "the image store at {storage_path} is already held by another live session. \ - A running wxc-wslc-daemon.exe releases it after its idle timeout; a \ - create-and-run execution releases it when the run ends" - ), - ) - } else { - sdk_error( - "WslcListSessionImages failed", - hr, - &format!("could not read the image store at {storage_path}"), - ) - } - }) + list_stored_images(sdk, session) + .map(|_| ()) + .map_err(|hr| store_access_failure(hr, storage_path)) +} + +/// Render a failed store probe, telling a sharing violation apart from any +/// other reason the store could not be read. +fn store_access_failure(hr: HRESULT, storage_path: &str) -> ScriptResponse { + if hr == E_SHARING_VIOLATION { + sdk_error( + "WSLc store unavailable", + hr, + &format!( + "the image store at {storage_path} is already held by another live session. \ + A running wxc-wslc-daemon.exe releases it after its idle timeout; a \ + create-and-run execution releases it when the run ends" + ), + ) + } else { + sdk_error( + "WslcListSessionImages failed", + hr, + &format!("could not read the image store at {storage_path}"), + ) + } } /// Names of the images a session's store holds. @@ -797,12 +803,26 @@ pub unsafe fn list_session_image_names( sdk: &WslcSdk, session: WslcSession, ) -> Result, ScriptResponse> { - list_image_names(sdk, session).map_err(|hr| sdk_error("WslcListSessionImages failed", hr, "")) + list_stored_images(sdk, session) + .map(|stored| stored.into_iter().map(|entry| entry.name).collect()) + .map_err(|hr| sdk_error("WslcListSessionImages failed", hr, "")) } +/// One entry of a session's image store. +pub(crate) struct StoredImage { + pub(crate) name: String, + pub(crate) sha256: [u8; 32], +} + +/// Read a session's image store, preserving the `HRESULT` so a caller can tell +/// a sharing violation from any other failure. +/// /// # Safety /// `sdk` must hold valid function pointers and `session` must be a live handle. -unsafe fn list_image_names(sdk: &WslcSdk, session: WslcSession) -> Result, HRESULT> { +pub(crate) unsafe fn list_stored_images( + sdk: &WslcSdk, + session: WslcSession, +) -> Result, HRESULT> { let mut images: *mut WslcImageInfo = ptr::null_mut(); let mut count: u32 = 0; let hr = sdk.WslcListSessionImages(session, &mut images, &mut count); @@ -813,19 +833,22 @@ unsafe fn list_image_names(sdk: &WslcSdk, session: WslcSession) -> Result(), info.name.len()); let end = bytes.iter().position(|&b| b == 0).unwrap_or(bytes.len()); if let Ok(name) = std::str::from_utf8(&bytes[..end]) { - names.push(name.to_string()); + stored.push(StoredImage { + name: name.to_string(), + sha256: info.sha256, + }); } } windows::Win32::System::Com::CoTaskMemFree(Some(images as *const c_void)); - Ok(names) + Ok(stored) } /// Create a daemon-owned container with `keepalive` as its init process, so it @@ -1309,6 +1332,44 @@ pub unsafe fn delete_daemon_container( #[cfg(test)] mod tests { use super::*; + use crate::mxc_common::models::FailurePhase; + + #[test] + fn a_held_store_is_reported_as_held_rather_than_as_a_failed_call() { + let resp = store_access_failure(E_SHARING_VIOLATION, r"C:\slots\slot-0"); + + assert!( + resp.error_message + .contains("already held by another live session"), + "a sharing violation is the one store failure a caller can wait out: {}", + resp.error_message + ); + assert!(resp.error_message.contains(r"C:\slots\slot-0")); + assert!(resp.error_message.contains("idle timeout")); + assert!(resp.error_message.contains("80070020")); + } + + #[test] + fn any_other_store_failure_keeps_the_call_that_produced_it() { + let resp = store_access_failure(-2147467259, r"C:\slots\slot-0"); + + assert!( + resp.error_message.contains("WslcListSessionImages failed"), + "an unrecognised failure must not be attributed to another holder: {}", + resp.error_message + ); + assert!(!resp.error_message.contains("already held")); + assert!(resp.error_message.contains(r"C:\slots\slot-0")); + } + + #[test] + fn a_store_failure_is_attributed_to_bring_up() { + assert_eq!( + store_access_failure(E_SHARING_VIOLATION, "store").failure_phase, + FailurePhase::LaunchFailed, + "a held store frees up on its own, so this must not read as a rejection" + ); + } #[test] fn cstr_bytes_nul_terminates_a_plain_value() { diff --git a/src/mxc-sdk/src/backends/wslc/common/image.rs b/src/mxc-sdk/src/backends/wslc/common/image.rs index 5ec1d3a2a..2cc89bca3 100644 --- a/src/mxc-sdk/src/backends/wslc/common/image.rs +++ b/src/mxc-sdk/src/backends/wslc/common/image.rs @@ -3,7 +3,6 @@ //! Container image resolution shared by the one-shot runner and the state-aware daemon. -use std::ffi::c_void; use std::fmt::Write; use std::ptr; use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; @@ -13,7 +12,7 @@ use crate::mxc_common::logger::Logger; use crate::mxc_common::models::ScriptResponse; use crate::mxc_common::string_util::{to_wide, CoTaskMemPWSTR}; -use crate::wslc_common::container_steps::{cstr_bytes, sdk_error, validate_store_access}; +use crate::wslc_common::container_steps::{self, cstr_bytes, sdk_error, validate_store_access}; use crate::wslc_common::error::WslcError; use crate::wslc_common::registry_policy; use crate::wslc_common::sdk_init; @@ -148,7 +147,7 @@ pub unsafe fn pull_image( sdk: &WslcSdk, session: WslcSession, image: &str, - storage_path: Option<&str>, + store: &StoreHint, log_prefix: &str, logger: &mut Logger, ) -> Result<(), ScriptResponse> { @@ -189,7 +188,7 @@ pub unsafe fn pull_image( if hr != S_OK { return Err(pull_failure( image, - storage_path, + store, hr, &pull_err.to_string_lossy(), budget, @@ -211,7 +210,7 @@ struct PullJob { sdk: *const WslcSdk, session: WslcSession, image: String, - storage_path: Option, + store: StoreHint, } // SAFETY: the SDK is apartment-affine, not thread-affine. Both this thread and @@ -238,7 +237,7 @@ pub unsafe fn start_pull( sdk: &WslcSdk, session: WslcSession, image: &str, - storage_path: Option<&str>, + store: &StoreHint, log_prefix: &str, on_done: F, ) -> Result<(), ScriptResponse> @@ -249,7 +248,7 @@ where sdk: sdk as *const WslcSdk, session, image: image.to_string(), - storage_path: storage_path.map(str::to_string), + store: store.clone(), }; let prefix = log_prefix.to_string(); @@ -270,7 +269,7 @@ where &*job.sdk, job.session, &job.image, - job.storage_path.as_deref(), + &job.store, &prefix, &mut log, ) @@ -354,7 +353,7 @@ pub fn pull_deadline_expired(image: &str, budget: Duration) -> ScriptResponse { /// Classify a `WslcPullSessionImage` failure and render it for the caller. fn pull_failure( image: &str, - storage_path: Option<&str>, + store: &StoreHint, hr: HRESULT, sdk_msg: &str, budget: Duration, @@ -377,7 +376,7 @@ fn pull_failure( budget.as_secs(), PULL_TIMEOUT_ENV, image, - storage_arg(storage_path), + store.warm_args(), )), WSLC_E_IMAGE_NOT_FOUND => WslcError::Rejected(format!( "WSLC image '{}' could not be pulled: {}. Check the image name and tag. \ @@ -403,7 +402,7 @@ fn pull_failure( image, detail, image, - storage_arg(storage_path), + store.warm_args(), )), } .into_response() @@ -428,12 +427,25 @@ fn sanitize_sdk_message(sdk_msg: &str) -> String { } } -/// Render a storage-path override as a `--storage-path` argument for the -/// suggested command, or nothing when the run uses the default path. -fn storage_arg(storage_path: Option<&str>) -> String { - match storage_path { - None => String::new(), - Some(sp) => format!(" --storage-path \"{}\"", sp), +/// Which store a resolution is reading, so a failure can name the command that +/// warms *that* store. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum StoreHint { + /// The create-and-run store, carrying any `wslc.storagePath` override. + CreateAndRun(Option), + + /// Store slot `index` of the container-lifecycle pool. + DaemonSlot(usize), +} + +impl StoreHint { + /// The arguments that point `--setup-wslc` at this store, if it needs any. + fn warm_args(&self) -> String { + match self { + StoreHint::CreateAndRun(None) => String::new(), + StoreHint::CreateAndRun(Some(path)) => format!(" --storage-path \"{}\"", path), + StoreHint::DaemonSlot(index) => format!(" --wslc-daemon-slots {}", index + 1), + } } } @@ -667,7 +679,7 @@ pub unsafe fn begin_resolve( session: WslcSession, image: &str, image_tar_path: Option<&str>, - storage_path: Option<&str>, + store: &StoreHint, registry: RegistryAccess, log_prefix: &str, logger: &mut Logger, @@ -714,21 +726,26 @@ pub unsafe fn begin_resolve( ImageAction::RefuseByPolicy => { Err(WslcError::Rejected(policy.refusal(image)).into_response()) } - ImageAction::RefuseNoEgress => Err(WslcError::Rejected(format!( - "WSLC image '{}' is not cached, and this sandbox declares no egress. \ - Pulling it would reach the registry on the host's network, before the \ - container exists and outside the policy the request declares. Warm the \ - cache first with wxc-exec.exe --setup-wslc --image {}{}, set \ - wslc.imageTarPath to a local tar, or allow egress. \ - See docs/backends/wslc/wsl-container-getting-started.md.", - image, - image, - storage_arg(storage_path), - )) - .into_response()), + ImageAction::RefuseNoEgress => Err(no_egress_refusal(image, store)), } } +/// The refusal for an uncached image a sandbox declaring no egress cannot pull. +fn no_egress_refusal(image: &str, store: &StoreHint) -> ScriptResponse { + WslcError::Rejected(format!( + "WSLC image '{}' is not cached, and this sandbox declares no egress. \ + Pulling it would reach the registry on the host's network, before the \ + container exists and outside the policy the request declares. Warm the \ + cache first with wxc-exec.exe --setup-wslc --image {}{}, set \ + wslc.imageTarPath to a local tar, or allow egress. \ + See docs/backends/wslc/wsl-container-getting-started.md.", + image, + image, + store.warm_args(), + )) + .into_response() +} + /// Record which content a completed pull produced. /// /// A tag is mutable, so the digest is the only record of what this run @@ -776,7 +793,7 @@ pub unsafe fn resolve_image( session: WslcSession, image: &str, image_tar_path: Option<&str>, - storage_path: Option<&str>, + store: &StoreHint, registry: RegistryAccess, log_prefix: &str, logger: &mut Logger, @@ -786,7 +803,7 @@ pub unsafe fn resolve_image( session, image, image_tar_path, - storage_path, + store, registry, log_prefix, logger, @@ -797,16 +814,9 @@ pub unsafe fn resolve_image( // deadline cannot fire on a registry that sends no further chunk. let budget = pull_budget(); let (done, finished) = std::sync::mpsc::channel(); - start_pull( - sdk, - session, - image, - storage_path, - log_prefix, - move |outcome| { - let _ = done.send(outcome); - }, - )?; + start_pull(sdk, session, image, store, log_prefix, move |outcome| { + let _ = done.send(outcome); + })?; match finished.recv_timeout(budget) { Ok(outcome) => outcome?, @@ -861,35 +871,17 @@ unsafe fn list_cached_images( sdk: &WslcSdk, session: WslcSession, ) -> Result, ScriptResponse> { - let mut images: *mut WslcImageInfo = ptr::null_mut(); - let mut image_count: u32 = 0; - let hr = sdk.WslcListSessionImages(session, &mut images, &mut image_count); - if hr != S_OK { - return Err(sdk_error("WslcListSessionImages failed", hr, "")); - } - if images.is_null() { - return Ok(Vec::new()); - } - - let mut cached = Vec::with_capacity(image_count as usize); - for info in std::slice::from_raw_parts(images, image_count as usize) { - // `info.name` is a fixed-size, possibly-unterminated C buffer; read up - // to the first NUL, or the whole buffer if there is none. - let name_bytes = - std::slice::from_raw_parts(info.name.as_ptr().cast::(), info.name.len()); - let end = name_bytes - .iter() - .position(|&b| b == 0) - .unwrap_or(name_bytes.len()); - if let Ok(name) = std::str::from_utf8(&name_bytes[..end]) { - cached.push(CachedImage { - name: name.to_string(), - sha256: info.sha256, - }); - } - } - windows::Win32::System::Com::CoTaskMemFree(Some(images as *const c_void)); - Ok(cached) + container_steps::list_stored_images(sdk, session) + .map(|stored| { + stored + .into_iter() + .map(|entry| CachedImage { + name: entry.name, + sha256: entry.sha256, + }) + .collect() + }) + .map_err(|hr| sdk_error("WslcListSessionImages failed", hr, "")) } /// How `resolve_image` satisfies the request once the cache has been consulted. @@ -945,14 +937,17 @@ fn with_implicit_tag(name: &str) -> String { } /// Warm the create-and-run store's image cache with `image_name`, and the first -/// `daemon_slots` state-aware store slots with it too, backing +/// `daemon_slots` container-lifecycle store slots with it too, backing /// `wxc-exec.exe --setup-wslc`. /// -/// The state-aware daemon leases its store from a separate pool, so warming the +/// The lifecycle daemon leases its store from a separate pool, so warming the /// create-and-run store does not warm the daemon's. A slot that has never been /// leased holds no images at all, and an uncached image under the default /// deny-egress posture is refused before the VM is reached. /// +/// `daemon_slots` is clamped to the pool's capacity, since warming a slot the +/// pool will never hand out costs a pull and a multi-GiB store for nothing. +/// /// # Safety /// Must be called once per process before any other WSLC SDK functions /// (it initialises COM via `init_and_load_sdk`). @@ -967,39 +962,46 @@ pub unsafe fn setup_pull_image( Err(resp) => return Err(resp.error_message), }; + let capacity = store_slots::configured_slot_count(); + let wanted = daemon_slots.min(capacity); + if daemon_slots > capacity { + let _ = writeln!( + logger, + "[WSLC setup] {daemon_slots} slots requested but the pool offers {capacity}; \ + warming {wanted}" + ); + } + warm_store( sdk, image_name, + &StoreHint::CreateAndRun(storage_path.map(str::to_string)), storage_path, - StoreKind::CreateAndRun, logger, )?; - for slot in store_slots::slot_paths(daemon_slots) { - std::fs::create_dir_all(&slot) - .map_err(|e| format!("could not create the WSLc store slot at {slot:?}: {e}"))?; + for index in 0..wanted { + let slot = store_slots::slot_path(index); + std::fs::create_dir_all(&slot).map_err(|e| { + format!( + "could not create the WSLc store slot at {}: {e}", + slot.display() + ) + })?; warm_store( sdk, image_name, + &StoreHint::DaemonSlot(index), Some(&slot.to_string_lossy()), - StoreKind::DaemonSlot, logger, - )?; + ) + .map_err(|e| format!("{e} (slot {index} of {wanted}; earlier slots were warmed)"))?; } Ok(()) } -/// Which store a warm pass is filling. -enum StoreKind { - /// The store create-and-run execution reads, caller-named or defaulted. - CreateAndRun, - - /// A slot from the state-aware pool, which one live session holds at a time. - DaemonSlot, -} - -/// Pull `image_name` into one store and release the session. +/// Pull `image_name` into the store at `storage_path` and release the session. /// /// # Safety /// `sdk` must hold valid function pointers and COM must be initialised on this @@ -1007,8 +1009,8 @@ enum StoreKind { unsafe fn warm_store( sdk: &'static WslcSdk, image_name: &str, + store: &StoreHint, storage_path: Option<&str>, - kind: StoreKind, logger: &mut Logger, ) -> Result<(), String> { let storage_path_str = storage_path.map(|s| s.to_string()).unwrap_or_else(|| { @@ -1052,7 +1054,7 @@ unsafe fn warm_store( // A slot admits one live session, and a conflict is invisible until the // store is touched, so a held slot would otherwise surface partway through // the pull. - if matches!(kind, StoreKind::DaemonSlot) { + if matches!(store, StoreHint::DaemonSlot(_)) { validate_store_access(sdk, session, &storage_path_str) .map_err(|resp| resp.error_message)?; } @@ -1064,7 +1066,7 @@ unsafe fn warm_store( sdk, session, image_name, - storage_path, + store, "[WSLC setup]", move |outcome| { let _ = done.send(outcome); @@ -1095,13 +1097,8 @@ mod tests { const E_FAIL: HRESULT = -2147467259; /// `pull_failure` at the production budget, which most cases do not care about. - fn failure( - image: &str, - storage_path: Option<&str>, - hr: HRESULT, - sdk_msg: &str, - ) -> ScriptResponse { - pull_failure(image, storage_path, hr, sdk_msg, PULL_TIMEOUT) + fn failure(image: &str, store: &StoreHint, hr: HRESULT, sdk_msg: &str) -> ScriptResponse { + pull_failure(image, store, hr, sdk_msg, PULL_TIMEOUT) } #[test] @@ -1149,7 +1146,7 @@ mod tests { fn an_abort_is_reported_as_a_budget_overrun_not_a_registry_failure() { let resp = pull_failure( "alpine:latest", - None, + &StoreHint::CreateAndRun(None), E_ABORT, "operation aborted", Duration::from_secs(90), @@ -1429,7 +1426,12 @@ mod tests { #[test] fn a_refused_reference_is_not_worth_retrying() { - let resp = failure("ghcr.io/nope:1", None, WSLC_E_IMAGE_NOT_FOUND, "denied"); + let resp = failure( + "ghcr.io/nope:1", + &StoreHint::CreateAndRun(None), + WSLC_E_IMAGE_NOT_FOUND, + "denied", + ); assert_eq!(resp.failure_phase, FailurePhase::Rejected); } @@ -1437,7 +1439,7 @@ mod tests { fn a_blocked_registry_is_not_worth_retrying() { let resp = failure( "ghcr.io/nope:1", - None, + &StoreHint::CreateAndRun(None), WSLC_E_REGISTRY_BLOCKED_BY_POLICY, "blocked", ); @@ -1446,13 +1448,23 @@ mod tests { #[test] fn an_unreachable_registry_is_retryable() { - let resp = failure("alpine:latest", None, E_FAIL, "no such host"); + let resp = failure( + "alpine:latest", + &StoreHint::CreateAndRun(None), + E_FAIL, + "no such host", + ); assert_eq!(resp.failure_phase, FailurePhase::LaunchFailed); } #[test] fn an_unreachable_registry_says_how_to_work_offline() { - let resp = failure("alpine:latest", None, E_FAIL, "no such host"); + let resp = failure( + "alpine:latest", + &StoreHint::CreateAndRun(None), + E_FAIL, + "no such host", + ); assert!(resp.error_message.contains("retry once it is available")); assert!(resp.error_message.contains("imageTarPath")); } @@ -1461,7 +1473,12 @@ mod tests { fn an_unrecognised_failure_does_not_diagnose_the_network() { // A full disk reaches this arm too, so the wording offers the network // as one possibility rather than asserting it. - let resp = failure("alpine:latest", None, E_FAIL, "no space left on device"); + let resp = failure( + "alpine:latest", + &StoreHint::CreateAndRun(None), + E_FAIL, + "no space left on device", + ); assert!( resp.error_message .contains("If the registry was unreachable"), @@ -1473,7 +1490,12 @@ mod tests { #[test] fn registry_text_cannot_smuggle_control_characters() { let hostile = "denied\u{1b}[2Jcleared\u{7}\nnext"; - let resp = failure("alpine:latest", None, E_FAIL, hostile); + let resp = failure( + "alpine:latest", + &StoreHint::CreateAndRun(None), + E_FAIL, + hostile, + ); assert!(!resp.error_message.contains('\u{1b}')); assert!(!resp.error_message.contains('\u{7}')); assert!(!resp.error_message.contains('\n')); @@ -1483,7 +1505,12 @@ mod tests { #[test] fn registry_text_is_capped() { let flood = "x".repeat(5_000); - let resp = failure("alpine:latest", None, E_FAIL, &flood); + let resp = failure( + "alpine:latest", + &StoreHint::CreateAndRun(None), + E_FAIL, + &flood, + ); assert!(resp.error_message.contains('…')); assert!( resp.error_message.len() < 1_500, @@ -1496,14 +1523,24 @@ mod tests { fn a_multibyte_registry_message_is_cut_on_a_character_boundary() { // Slicing by byte offset here would panic mid-character. let flood = "é".repeat(5_000); - let resp = failure("alpine:latest", None, E_FAIL, &flood); + let resp = failure( + "alpine:latest", + &StoreHint::CreateAndRun(None), + E_FAIL, + &flood, + ); assert!(resp.error_message.contains('…')); } #[test] fn an_unreachable_registry_does_not_prescribe_the_pull_that_just_failed() { // `--setup-wslc` shares `pull_image`, so it fails the same way here. - let resp = failure("alpine:latest", None, E_FAIL, "no such host"); + let resp = failure( + "alpine:latest", + &StoreHint::CreateAndRun(None), + E_FAIL, + "no such host", + ); let advice = &resp.error_message; let setup = advice .find("--setup-wslc") @@ -1519,20 +1556,72 @@ mod tests { #[test] fn an_overridden_store_travels_with_the_suggested_command() { - let resp = failure("alpine:latest", Some(r"C:\store"), E_FAIL, "down"); + let resp = failure( + "alpine:latest", + &StoreHint::CreateAndRun(Some(r"C:\store".to_string())), + E_FAIL, + "down", + ); assert!(resp.error_message.contains(r#"--storage-path "C:\store""#)); } #[test] fn a_default_store_adds_no_path_argument() { - let resp = failure("alpine:latest", None, E_FAIL, "down"); + let resp = failure( + "alpine:latest", + &StoreHint::CreateAndRun(None), + E_FAIL, + "down", + ); assert!(!resp.error_message.contains("--storage-path")); } + #[test] + fn a_lifecycle_store_is_warmed_by_slot_rather_than_by_path() { + let resp = failure("alpine:latest", &StoreHint::DaemonSlot(0), E_FAIL, "down"); + + assert!( + resp.error_message.contains("--wslc-daemon-slots 1"), + "the daemon leases its own store, so --storage-path would warm the \ + wrong one: {}", + resp.error_message + ); + assert!(!resp.error_message.contains("--storage-path")); + } + + #[test] + fn an_uncached_image_without_egress_names_the_store_it_needs_warmed() { + let create_and_run = no_egress_refusal("alpine:latest", &StoreHint::CreateAndRun(None)); + assert!(!create_and_run.error_message.contains("--wslc-daemon-slots")); + + let lifecycle = no_egress_refusal("alpine:latest", &StoreHint::DaemonSlot(0)); + + assert!( + lifecycle.error_message.contains("--wslc-daemon-slots 1"), + "a lifecycle caller sent to warm the create-and-run store retries \ + against the same cold slot: {}", + lifecycle.error_message + ); + } + + #[test] + fn a_higher_slot_asks_for_enough_slots_to_reach_it() { + assert_eq!( + StoreHint::DaemonSlot(2).warm_args(), + " --wslc-daemon-slots 3", + "warming is counted from slot 0, so reaching slot 2 takes 3 slots" + ); + } + #[test] fn every_rejection_names_a_way_forward() { for hr in [WSLC_E_IMAGE_NOT_FOUND, WSLC_E_REGISTRY_BLOCKED_BY_POLICY] { - let resp = failure("ghcr.io/nope:1", None, hr, "denied"); + let resp = failure( + "ghcr.io/nope:1", + &StoreHint::CreateAndRun(None), + hr, + "denied", + ); assert!( resp.error_message.contains("imageTarPath"), "a rejection that only diagnoses leaves the caller stuck: {}", @@ -1543,7 +1632,12 @@ mod tests { #[test] fn a_failure_without_an_sdk_message_still_reports_the_code() { - let resp = failure("alpine:latest", None, WSLC_E_IMAGE_NOT_FOUND, ""); + let resp = failure( + "alpine:latest", + &StoreHint::CreateAndRun(None), + WSLC_E_IMAGE_NOT_FOUND, + "", + ); assert!(resp.error_message.contains("0x80040601")); } diff --git a/src/mxc-sdk/src/backends/wslc/common/store_slots.rs b/src/mxc-sdk/src/backends/wslc/common/store_slots.rs index 7a373b8fb..284f00a05 100644 --- a/src/mxc-sdk/src/backends/wslc/common/store_slots.rs +++ b/src/mxc-sdk/src/backends/wslc/common/store_slots.rs @@ -1,7 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -//! A bounded pool of reusable WSLc image stores for the state-aware daemon. +//! A bounded pool of reusable WSLc image stores for the container-lifecycle +//! daemon. //! //! A WSLc "store" is a host directory the caller hands to `WslcCreateSession`; //! the SDK creates `storage.vhdx` inside it and mounts that VHDX into the @@ -80,13 +81,9 @@ fn slot_path_in(root: &Path, index: usize) -> PathBuf { root.join(format!("slot-{index}")) } -/// The paths of the first `count` slots, lowest index first. -/// -/// Warming opens each store in turn rather than holding one for a session's -/// lifetime, so it takes paths without leasing. -pub fn slot_paths(count: usize) -> Vec { - let root = pool_root(); - (0..count).map(|index| slot_path_in(&root, index)).collect() +/// The path of slot `index` in this host's pool. +pub fn slot_path(index: usize) -> PathBuf { + slot_path_in(&pool_root(), index) } /// Why a slot could not be leased. @@ -466,6 +463,19 @@ mod tests { ] ); } + + #[test] + fn warming_targets_the_same_store_leasing_takes() { + let pool = StoreSlotPool::new(); + + for index in 0..pool.capacity() { + assert_eq!( + slot_path(index), + pool.slot_path(index), + "warming a slot the pool would not hand out leaves the daemon cold" + ); + } + } } /// Pool behaviour that only a live WSL2 host can answer. @@ -487,8 +497,8 @@ mod host_tests { use crate::wslc_common::image::pull_image; use crate::wslc_common::sdk_init::init_and_load_sdk; - /// Small, and the default the state-aware backend uses, so a developer's - /// warm cache likely already has it. + /// Small, and the default the container-lifecycle backend uses, so a + /// developer's warm cache likely already has it. const CACHE_IMAGE: &str = "alpine:latest"; /// A pool rooted somewhere disposable, so a run leaves no multi-GiB stores @@ -631,7 +641,7 @@ mod host_tests { sdk, session.as_raw(), CACHE_IMAGE, - Some(&store), + &crate::wslc_common::image::StoreHint::DaemonSlot(lease.index()), "[slot]", &mut log, ) diff --git a/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs b/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs index 4037824c1..31a560e76 100644 --- a/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs +++ b/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs @@ -845,7 +845,7 @@ impl WSLContainerRunner { session_guard.as_raw(), &self.config.image, self.config.image_tar_path.as_deref(), - self.config.storage_path.as_deref(), + &image::StoreHint::CreateAndRun(self.config.storage_path.clone()), registry, "[WSLC]", logger, diff --git a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs index b17dbc6e3..e2998e6a0 100644 --- a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs +++ b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs @@ -49,7 +49,7 @@ use mxc_sdk::wslc_common::daemon_protocol::{ use mxc_sdk::wslc_common::image; use mxc_sdk::wslc_common::policy_mapping; use mxc_sdk::wslc_common::process_env::EnvScope; -use mxc_sdk::wslc_common::store_slots::{StoreSlotError, StoreSlotLease, StoreSlotPool}; +use mxc_sdk::wslc_common::store_slots::{self, StoreSlotError, StoreSlotLease, StoreSlotPool}; use mxc_sdk::wslc_common::wslc_bindings::{ WslcContainer, WslcContainerGuard, WslcContainerNetworkingMode, WslcSdk, WslcSessionGuard, }; @@ -899,6 +899,11 @@ impl Worker { let sdk = self.sdk.as_ref().expect("session ensured"); let session = self.session.as_ref().expect("session ensured").as_raw(); + let store = image::StoreHint::DaemonSlot( + self.store_lease + .as_ref() + .map_or(0, store_slots::StoreSlotLease::index), + ); // SAFETY: `sdk`/`session` are valid. let step = unsafe { @@ -907,7 +912,7 @@ impl Worker { session, &config.image, config.image_tar_path.as_deref(), - None, + &store, match config.network { NetworkMode::None => image::RegistryAccess::Denied, NetworkMode::Bridged => image::RegistryAccess::Allowed, @@ -938,7 +943,7 @@ impl Worker { sdk, session, &config.image, - None, + &store, "[WSLC][daemon]", move |outcome| { let _ = done.send(WorkerCommand::PullFinished { token, outcome }); diff --git a/src/tools/wxc/src/main.rs b/src/tools/wxc/src/main.rs index 4ed41b832..239c2a357 100644 --- a/src/tools/wxc/src/main.rs +++ b/src/tools/wxc/src/main.rs @@ -137,10 +137,11 @@ struct Cli { storage_path: Option, /// Also warm the first N container-lifecycle store slots, which the - /// state-aware daemon leases from and `--storage-path` does not reach. - /// Defaults to 0 so warming a create-and-run cache costs one pull per - /// image; each slot warmed adds a pull and a multi-GiB store. - /// Requires `--setup-wslc`. + /// lifecycle daemon leases from and `--storage-path` does not reach. + /// Clamped to the pool capacity (`MXC_WSLC_STORE_SLOTS`, default 4), since + /// a slot the pool never hands out costs a pull and a multi-GiB store for + /// nothing. Defaults to 0 so warming a create-and-run cache stays one pull + /// per image. Requires `--setup-wslc`. #[arg(long = "wslc-daemon-slots", value_name = "N", requires = "setup_wslc")] wslc_daemon_slots: Option, diff --git a/tests/scripts/README.md b/tests/scripts/README.md index 2ccffdb3e..b9e2173fe 100644 --- a/tests/scripts/README.md +++ b/tests/scripts/README.md @@ -45,7 +45,8 @@ Linux / macOS (`.sh`): | `run_windows_sandbox_state_aware_tests.ps1` | Windows Sandbox state-aware lifecycle E2E (single VM held across provision/start/exec*/stop/deprovision) | Windows Sandbox enabled | | `run_isolation_session_tests.ps1` | IsolationSession one-shot E2E suite | Interactive local session; OS-side IsolationSession service | | `run_isolation_session_state_aware_tests.ps1` | IsolationSession provision/start/exec/stop/deprovision E2E suite | Interactive local session; OS-side IsolationSession service | -| `run_wslc_all_tests.ps1` | All WSLC one-shot and state-aware E2E tests | WSL2, WSLC SDK, staged daemon, and registry access for the image preflight (`-SkipSetup` skips it and needs an already-warm cache, because the state-aware fixtures deny egress) | +| `run_wslc_all_tests.ps1` | All WSLC create-and-run and container-lifecycle E2E tests; delegates the lifecycle half to `run_wslc_state_aware_tests.ps1` | WSL2, WSLC SDK, staged daemon, and registry access for the image preflight, which warms both the create-and-run store and daemon store slot 0 (`-SkipSetup` skips it and needs both already warm, because the lifecycle fixtures deny egress) | +| `run_wslc_state_aware_tests.ps1` | WSLC provision/start/exec/stop/deprovision lifecycle E2E suite, plus warm reuse, exec concurrency and idle teardown | WSL2, WSLC SDK, `wxc-wslc-daemon.exe` staged beside `wxc-exec.exe`, and `alpine:latest` / `python:3.12-alpine` cached in daemon store slot 0 (warm it with `scripts\setup-wslc.ps1 -DaemonSlots 1`) | | `run_processcontainer_all_tests.ps1` | Process container (AppContainer / BaseContainer) primitives suite — tier probes, rw/ro/denied matrix, enumeration-only grants, UI mitigations, DACL restore, crash recovery, directional networking. Dispatches to the per-area `run_processcontainer_*_test.ps1` scripts | `wxc-exec.exe`, `wxc-ui-probe.exe`, `plm.exe` and `winhttp-proxy-shim.exe` beside `wxc-exec.exe` | | `T3-Workloads.ps1` | Real workloads (pwsh, git, node, python, cmd) on top of the T3 primitives. A missing interpreter is reported as a skip, not a failure | `wxc-exec.exe`; `pwsh` / `git` / `node` / `python` each optional, gating their own cases | | `run_telemetry_consent_smoke_test.ps1` | Consent maintenance, presentation, policy, and exit-code smoke tests | Debug `wxc-exec.exe` built with `test-support` | diff --git a/tests/scripts/run_wslc_all_tests.ps1 b/tests/scripts/run_wslc_all_tests.ps1 index 09370fc90..839c0b670 100644 --- a/tests/scripts/run_wslc_all_tests.ps1 +++ b/tests/scripts/run_wslc_all_tests.ps1 @@ -77,8 +77,9 @@ if (-not $WxcExec -or -not (Test-Path $WxcExec)) { # Preflight: warm the WSLC image cache so per-test timings stay clear of one-off # download cost. The runner pulls on a miss by itself, so -SkipSetup is a -# supported way to exercise that path instead. These are create-and-run tests, -# so the lifecycle store slots are left alone. +# supported way to exercise that path instead. The lifecycle fixtures provision +# with the isolated posture, which refuses a registry pull, so the daemon's slot +# has to be warmed here too or the delegated suite below starts cold. if (-not $SkipSetup) { $SetupScript = Join-Path $RepoRoot "scripts\setup-wslc.ps1" if (Test-Path $SetupScript) { @@ -93,7 +94,7 @@ if (-not $SkipSetup) { "ghcr.io/linuxserver/baseimage-alpine:3.21", "quay.io/fedora/fedora-minimal:latest" ) - & $SetupScript -WxcExecPath $WxcExec -Image $images -DaemonSlots 0 -Force + & $SetupScript -WxcExecPath $WxcExec -Image $images -DaemonSlots 1 -Force if ($LASTEXITCODE -ne 0) { Write-Host "WARN: setup-wslc.ps1 reported failures; continuing with tests anyway." -ForegroundColor Yellow } @@ -467,8 +468,9 @@ $null = $results.Add((Run-WslcTest "wslc_destroy_on_exit_false_rejected.json" ` Write-Host "`n--- State-Aware Lifecycle Tests ---" -ForegroundColor Cyan # Delegate the multi-invocation provision/start/exec/stop/deprovision lifecycle # to its owning harness (it needs the daemon binary, mints + threads sandbox ids, -# and drives the idle-teardown watchdog). Images are already pre-pulled above, so -# skip its redundant preflight. Fold its exit code into the summary. +# and drives the idle-teardown watchdog). The preflight above warmed the daemon +# store slot as well, so skip its redundant preflight. Fold its exit code into +# the summary. $saScript = Join-Path $PSScriptRoot "run_wslc_state_aware_tests.ps1" $saArgs = @{ WxcExecPath = $WxcExec; SkipSetup = $true } if ($Debug) { $saArgs.Debug = $true } diff --git a/tests/scripts/run_wslc_state_aware_tests.ps1 b/tests/scripts/run_wslc_state_aware_tests.ps1 index 5d1ff83d0..aa5174260 100644 --- a/tests/scripts/run_wslc_state_aware_tests.ps1 +++ b/tests/scripts/run_wslc_state_aware_tests.ps1 @@ -1863,6 +1863,42 @@ try { } } +# ---------------- Lifecycle S: store isolation from create-and-run ---------------- + +# A store's VHDX admits one live session, and the daemon used to default to the +# create-and-run store, so this combination is what failed. Must stay ahead of +# test H, which drives the daemon to exit. +Run-StateAwareTest "S: create-and-run execution succeeds while a sandbox is live" { + $script:storeSandbox = Provision-Sandbox + Assert-True ($null -ne $script:storeSandbox) "provisioned a sandbox to hold the daemon's store" + if ($null -eq $script:storeSandbox) { return } + + try { + Assert-True (Test-DaemonRunning) "the daemon is live and holding its store slot" + + $oneShotConfig = Join-Path $RepoRoot "tests\examples\wslc_hello_world.json" + if (-not (Test-Path $oneShotConfig)) { + Assert-True $false "create-and-run fixture not found at $oneShotConfig" + return + } + + $prevPref = $ErrorActionPreference + $ErrorActionPreference = "Continue" + $output = & $WxcExec --config $oneShotConfig 2>&1 | Out-String + $exitCode = $LASTEXITCODE + $ErrorActionPreference = $prevPref + + Assert-True ($exitCode -eq 0) "create-and-run run exits 0 while a sandbox is provisioned" + Assert-True ($output -notmatch '0x80070020') ` + "create-and-run run reports no sharing violation (got: $($output.Trim()))" + Assert-True ($output -match 'Hello from WSL Container') ` + "create-and-run run produced its own output" + } finally { + $null = Invoke-StateAware -ConfigFile 'wslc_state_aware_deprovision.json' ` + -SandboxId $script:storeSandbox + } +} + # ---------------- Lifecycle H: idle teardown ---------------- # All sandboxes above have been deprovisioned, so the daemon's live-container From 4a08e12af6e8eacb2657cf3cca8df8d84dc8da39 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Mon, 12 Oct 2026 16:02:23 -0700 Subject: [PATCH 4/6] Remove the slot-count bound from the WSLc store pool Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 763fd4b3-1dff-4398-8b27-e82d89a02a35 --- docs/backends/wslc/wslc-state-aware.md | 27 ++-- src/mxc-sdk/src/backends/wslc/common/image.rs | 17 +-- .../src/backends/wslc/common/store_slots.rs | 142 ++++++------------ .../src/bin/wslc_daemon/session_manager.rs | 2 +- src/tools/wxc/src/main.rs | 18 ++- 5 files changed, 75 insertions(+), 131 deletions(-) diff --git a/docs/backends/wslc/wslc-state-aware.md b/docs/backends/wslc/wslc-state-aware.md index 476316e41..96f9722b8 100644 --- a/docs/backends/wslc/wslc-state-aware.md +++ b/docs/backends/wslc/wslc-state-aware.md @@ -55,7 +55,7 @@ waits for the run, because deleting the container would free a handle the run is | Lifecycle backend | `src/mxc-sdk/src/backends/wslc/common/state_aware.rs` (`WslcStateAwareRunner`) | Translates runtime `WslcProvisionConfig` and shared policy fields into daemon protocol frames; implements `StatefulSandboxBackend` (`ID_PREFIX`/`BACKEND_KEY` = `wslc`). | | Policy support checks | `src/mxc-sdk/src/backends/wslc/common/policy.rs` | Per-phase validation of which policy fields are enforced or rejected. | | Daemon client | `src/mxc-sdk/src/backends/wslc/common/daemon_client.rs` | Discovers / spawns the daemon, connects the control pipe, sends `DaemonRequest` frames, reads responses; typed `DaemonError`. | -| Store slot pool | `src/mxc-sdk/src/backends/wslc/common/store_slots.rs` | Owns the daemon's image stores: slot paths, leasing, and the capacity knob. See [Image store](#image-store). | +| Store slot pool | `src/mxc-sdk/src/backends/wslc/common/store_slots.rs` | Owns the daemon's image stores: slot paths and leasing. See [Image store](#image-store). | | Daemon | `src/mxc-sdk/src/bin/wslc_daemon/` (`wxc-wslc-daemon.exe`) | Long-lived host process holding `WslcSession` / `WslcContainer`; worker thread drives the SDK; idle-timeout watchdog tears the session down when unused. | | Engine routing | `src/mxc-sdk/src/core/mxc_engine/state_aware.rs` | Selects the WSLc lifecycle backend (Windows + `wslc` feature). | | Prefix registration | `src/mxc-sdk/src/tools/mxc_common/state_aware_dispatch.rs` (`backend_from_prefix`) | Maps the `wslc:` id prefix back to the WSLc backend for post-provision phases. | @@ -140,11 +140,10 @@ A WSLc store is a host directory handed to `WslcCreateSession`; the SDK creates VM, where it holds the session's pulled images. A warmed store runs to a couple of GiB. -The daemon leases its store from a fixed pool rather than taking a caller-named -path: +The daemon leases its store from a pool rather than taking a caller-named path: ```text -%LOCALAPPDATA%\mxc\wslc-store-slots\slot-0 .. slot-3 +%LOCALAPPDATA%\mxc\wslc-store-slots\slot-0, slot-1, … ``` `%TEMP%\mxc\wslc-store-slots` is used instead when `%LOCALAPPDATA%` is @@ -158,6 +157,11 @@ one live session, so while the daemon held `%TEMP%\mxc-wslc-sessions` every create-and-run WSLc execution on the same host failed with `WslcListSessionImages failed: HRESULT 0x80070020` until the daemon idled out. +The pool does not cap how many sessions a host may run, and a slot count would +be the wrong control for that: a session carries its own cpu and memory, so the +same count can mean very different host load. Admission belongs with the work +that lets a caller request a second session. + ### Slot contents and disk A returned slot keeps its images — that is the point of leasing rather than @@ -171,15 +175,14 @@ wxc-exec.exe --setup-wslc --image alpine:latest --wslc-daemon-slots 1 ``` Both default to 0 and warm slots from 0 up, so `1` is what a daemon holding one -session needs. A count above the pool capacity is clamped to it. See +session needs. See [the getting-started guide](wsl-container-getting-started.md#step-3--container-images-optional-pre-pull) for the create-and-run store, which these do not replace. -Slots are identified by index and are never repurposed, so nothing accumulates -and no startup collection is needed; disk is bounded by one VHDX per slot. -Reclaim it by deleting the pool directory while no daemon is running. Lowering -`MXC_WSLC_STORE_SLOTS` leaves the higher-numbered directories in place, inert -and safe to delete. +A slot directory is created when it is first leased, so an unused index costs +nothing. Slots are identified by index and never repurposed, so nothing +accumulates beyond the number of sessions the host has actually run at once. +Reclaim the disk by deleting the pool directory while no daemon is running. ### Store exclusivity @@ -196,10 +199,6 @@ from a previous build still inside its idle window — fails the provision with error naming the slot, rather than relocating the session onto a cold slot and reporting a missing image instead. -| Variable | Default | Meaning | -|----------|---------|---------| -| `MXC_WSLC_STORE_SLOTS` | `4` | Slots the pool offers. Unset, empty, non-numeric, or zero falls back to the default. | - ### exec output semantics `provision` / `start` / `stop` / `deprovision` return a JSON object containing `result` or `error` on stdout. diff --git a/src/mxc-sdk/src/backends/wslc/common/image.rs b/src/mxc-sdk/src/backends/wslc/common/image.rs index 2cc89bca3..5ff2c1797 100644 --- a/src/mxc-sdk/src/backends/wslc/common/image.rs +++ b/src/mxc-sdk/src/backends/wslc/common/image.rs @@ -945,9 +945,6 @@ fn with_implicit_tag(name: &str) -> String { /// leased holds no images at all, and an uncached image under the default /// deny-egress posture is refused before the VM is reached. /// -/// `daemon_slots` is clamped to the pool's capacity, since warming a slot the -/// pool will never hand out costs a pull and a multi-GiB store for nothing. -/// /// # Safety /// Must be called once per process before any other WSLC SDK functions /// (it initialises COM via `init_and_load_sdk`). @@ -962,16 +959,6 @@ pub unsafe fn setup_pull_image( Err(resp) => return Err(resp.error_message), }; - let capacity = store_slots::configured_slot_count(); - let wanted = daemon_slots.min(capacity); - if daemon_slots > capacity { - let _ = writeln!( - logger, - "[WSLC setup] {daemon_slots} slots requested but the pool offers {capacity}; \ - warming {wanted}" - ); - } - warm_store( sdk, image_name, @@ -980,7 +967,7 @@ pub unsafe fn setup_pull_image( logger, )?; - for index in 0..wanted { + for index in 0..daemon_slots { let slot = store_slots::slot_path(index); std::fs::create_dir_all(&slot).map_err(|e| { format!( @@ -995,7 +982,7 @@ pub unsafe fn setup_pull_image( Some(&slot.to_string_lossy()), logger, ) - .map_err(|e| format!("{e} (slot {index} of {wanted}; earlier slots were warmed)"))?; + .map_err(|e| format!("{e} (slot {index} of {daemon_slots}; earlier slots were warmed)"))?; } Ok(()) diff --git a/src/mxc-sdk/src/backends/wslc/common/store_slots.rs b/src/mxc-sdk/src/backends/wslc/common/store_slots.rs index 284f00a05..7731652d3 100644 --- a/src/mxc-sdk/src/backends/wslc/common/store_slots.rs +++ b/src/mxc-sdk/src/backends/wslc/common/store_slots.rs @@ -1,8 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -//! A bounded pool of reusable WSLc image stores for the container-lifecycle -//! daemon. +//! A pool of reusable WSLc image stores for the container-lifecycle daemon. //! //! A WSLc "store" is a host directory the caller hands to `WslcCreateSession`; //! the SDK creates `storage.vhdx` inside it and mounts that VHDX into the @@ -16,12 +15,15 @@ //! - Releasing a session only unmounts the file, so the images a lease pulled //! are still there for the next one. //! -//! The second fact is what makes a fixed set of leased slots work. A fresh -//! directory has no VHDX and therefore no images, and an uncached image under -//! the default deny-egress posture is refused before the VM is ever reached — -//! so minting a new directory per session would leave every session cold and -//! break isolated provisioning outright. Slots are leased and returned instead, -//! which also bounds disk at one multi-GiB VHDX per slot. +//! The second fact is what makes reusable slots work. A fresh directory has no +//! VHDX and therefore no images, and an uncached image under the default +//! deny-egress posture is refused before the VM is ever reached — so minting a +//! new directory per session would leave every session cold and break isolated +//! provisioning outright. Slots are leased and returned instead. +//! +//! How many sessions a host may run at once is not decided here. A slot count +//! cannot express it: a session carries its own cpu and memory, so equal counts +//! can mean very different host load. //! //! Lease tracking is per-process and so cannot see a store another process //! holds. The first fact means that cannot be delegated to the SDK either, so a @@ -33,12 +35,6 @@ use std::ffi::OsStr; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; -/// Number of store slots the pool offers when the environment does not say. -pub const DEFAULT_SLOT_COUNT: usize = 4; - -/// Environment override for the slot count, as a positive whole number. -pub const SLOT_COUNT_ENV: &str = "MXC_WSLC_STORE_SLOTS"; - /// Directory holding the slots, beneath the per-user application data root. const POOL_LEAF: &str = "wslc-store-slots"; @@ -62,20 +58,6 @@ pub fn pool_root() -> PathBuf { ) } -/// Read a slot count from `raw`, falling back to [`DEFAULT_SLOT_COUNT`] when it -/// is unset, empty, non-numeric, or zero. -fn slot_count_from(raw: Option<&OsStr>) -> usize { - raw.and_then(OsStr::to_str) - .and_then(|text| text.trim().parse::().ok()) - .filter(|count| *count > 0) - .unwrap_or(DEFAULT_SLOT_COUNT) -} - -/// The slot count this process uses, honouring [`SLOT_COUNT_ENV`]. -pub fn configured_slot_count() -> usize { - slot_count_from(std::env::var_os(SLOT_COUNT_ENV).as_deref()) -} - /// The path of slot `index` beneath `root`. fn slot_path_in(root: &Path, index: usize) -> PathBuf { root.join(format!("slot-{index}")) @@ -89,8 +71,9 @@ pub fn slot_path(index: usize) -> PathBuf { /// Why a slot could not be leased. #[derive(Debug)] pub enum StoreSlotError { - /// Every slot in the pool is already leased by this process. - Exhausted { capacity: usize }, + /// The pool's own bookkeeping is unusable, so no slot can be handed out + /// without risking handing one out twice. + Unavailable, /// The slot directory could not be created. Io { @@ -109,10 +92,9 @@ pub enum StoreSlotError { impl std::fmt::Display for StoreSlotError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - StoreSlotError::Exhausted { capacity } => write!( - f, - "all {capacity} WSLc store slots are in use; raise {SLOT_COUNT_ENV} to offer more" - ), + StoreSlotError::Unavailable => { + write!(f, "the WSLc store slot pool is no longer usable") + } StoreSlotError::Io { path, source } => write!( f, "could not create the WSLc store slot at {}: {source}", @@ -132,35 +114,28 @@ impl std::fmt::Display for StoreSlotError { impl std::error::Error for StoreSlotError {} -/// A fixed set of reusable image stores, leased one at a time. +/// Reusable image stores, one per live session, minted on demand. #[derive(Debug)] pub struct StoreSlotPool { root: PathBuf, - capacity: usize, leased: Arc>>, } impl StoreSlotPool { - /// A pool over the per-user slot directory at the configured capacity. + /// A pool over the per-user slot directory. pub fn new() -> Self { - Self::with_root(pool_root(), configured_slot_count()) + Self::with_root(pool_root()) } /// A pool over an explicit root, for tests and tools that place slots /// somewhere other than the per-user directory. - pub fn with_root(root: PathBuf, capacity: usize) -> Self { + pub fn with_root(root: PathBuf) -> Self { Self { root, - capacity, leased: Arc::new(Mutex::new(BTreeSet::new())), } } - /// How many slots this pool can lease at once. - pub fn capacity(&self) -> usize { - self.capacity - } - /// The path of slot `index`, whether or not it is leased. pub fn slot_path(&self, index: usize) -> PathBuf { slot_path_in(&self.root, index) @@ -176,9 +151,7 @@ impl StoreSlotPool { &self, open: impl FnOnce(&Path) -> Result, ) -> Result<(StoreSlotLease, T), StoreSlotError> { - let index = self.take_lowest_free().ok_or(StoreSlotError::Exhausted { - capacity: self.capacity, - })?; + let index = self.take_lowest_free().ok_or(StoreSlotError::Unavailable)?; let path = self.slot_path(index); // Built before anything can fail, so every error path below returns the @@ -205,7 +178,7 @@ impl StoreSlotPool { fn take_lowest_free(&self) -> Option { let mut leased = self.leased.lock().ok()?; - let free = (0..self.capacity).find(|index| !leased.contains(index))?; + let free = (0..).find(|index| !leased.contains(index))?; leased.insert(free); Some(free) } @@ -253,10 +226,9 @@ impl Drop for StoreSlotLease { mod tests { use super::*; - /// A pool of `capacity` slots under a directory that deletes itself. - fn pool(capacity: usize) -> (tempfile::TempDir, StoreSlotPool) { + fn pool() -> (tempfile::TempDir, StoreSlotPool) { let root = tempfile::tempdir().expect("create scratch pool root"); - let pool = StoreSlotPool::with_root(root.path().to_path_buf(), capacity); + let pool = StoreSlotPool::with_root(root.path().to_path_buf()); (root, pool) } @@ -267,7 +239,7 @@ mod tests { #[test] fn a_lease_takes_the_lowest_free_slot() { - let (_root, pool) = pool(4); + let (_root, pool) = pool(); let (first, _) = pool.lease_with(accept).expect("first lease"); let (second, _) = pool.lease_with(accept).expect("second lease"); @@ -283,7 +255,7 @@ mod tests { #[test] fn a_live_lease_is_never_handed_out_twice() { - let (_root, pool) = pool(4); + let (_root, pool) = pool(); let (held, _) = pool.lease_with(accept).expect("first lease"); let (other, _) = pool.lease_with(accept).expect("second lease"); @@ -298,7 +270,7 @@ mod tests { #[test] fn a_returned_lease_frees_its_slot_for_the_next_caller() { - let (_root, pool) = pool(4); + let (_root, pool) = pool(); let (first, _) = pool.lease_with(accept).expect("first lease"); let returned = first.path().to_path_buf(); @@ -316,7 +288,7 @@ mod tests { #[test] fn a_forgotten_lease_keeps_its_slot_held() { - let (_root, pool) = pool(2); + let (_root, pool) = pool(); let (leaked, _) = pool.lease_with(accept).expect("first lease"); let leaked_path = leaked.path().to_path_buf(); @@ -333,21 +305,24 @@ mod tests { } #[test] - fn a_full_pool_reports_exhaustion() { - let (_root, pool) = pool(2); + fn slots_keep_being_minted_past_the_ones_already_leased() { + let (_root, pool) = pool(); - let (_first, _) = pool.lease_with(accept).expect("first lease"); - let (_second, _) = pool.lease_with(accept).expect("second lease"); + let leases: Vec<_> = (0..6) + .map(|_| pool.lease_with(accept).expect("lease").0) + .collect(); - match pool.lease_with(accept) { - Err(StoreSlotError::Exhausted { capacity }) => assert_eq!(capacity, 2), - other => panic!("a full pool must report exhaustion, got {other:?}"), - } + assert_eq!( + leases.iter().map(StoreSlotLease::index).collect::>(), + [0, 1, 2, 3, 4, 5], + "a session carries its own cpu and memory, so a slot count is not \ + the unit that bounds how many may run" + ); } #[test] fn a_refused_slot_fails_the_lease_rather_than_moving_to_another() { - let (_root, pool) = pool(4); + let (_root, pool) = pool(); let refusal = pool.lease_with(|_| Err::<(), _>("store is held".to_string())); @@ -369,7 +344,7 @@ mod tests { #[test] fn a_lease_creates_its_slot_directory() { - let (_root, pool) = pool(1); + let (_root, pool) = pool(); let (lease, _) = pool.lease_with(accept).expect("lease"); @@ -387,7 +362,7 @@ mod tests { // Rooting the pool at a regular file makes `create_dir_all` fail // without depending on permissions a test cannot rely on. - let pool = StoreSlotPool::with_root(blocker, 1); + let pool = StoreSlotPool::with_root(blocker); match pool.lease_with(accept) { Err(StoreSlotError::Io { .. }) => {} @@ -427,27 +402,6 @@ mod tests { } } - #[test] - fn the_slot_count_falls_back_for_a_value_it_cannot_use() { - for unusable in [ - None, - Some(""), - Some(" "), - Some("four"), - Some("0"), - Some("-1"), - ] { - assert_eq!( - slot_count_from(unusable.map(OsStr::new)), - DEFAULT_SLOT_COUNT, - "{unusable:?} must not change the slot count" - ); - } - - assert_eq!(slot_count_from(Some(OsStr::new("6"))), 6); - assert_eq!(slot_count_from(Some(OsStr::new(" 2 "))), 2); - } - #[test] fn slot_paths_are_numbered_from_zero() { let root = PathBuf::from(r"C:\pool"); @@ -468,7 +422,7 @@ mod tests { fn warming_targets_the_same_store_leasing_takes() { let pool = StoreSlotPool::new(); - for index in 0..pool.capacity() { + for index in 0..4 { assert_eq!( slot_path(index), pool.slot_path(index), @@ -503,9 +457,9 @@ mod host_tests { /// A pool rooted somewhere disposable, so a run leaves no multi-GiB stores /// behind in the real per-user pool. - fn scratch_pool(capacity: usize) -> (tempfile::TempDir, StoreSlotPool) { + fn scratch_pool() -> (tempfile::TempDir, StoreSlotPool) { let root = tempfile::tempdir().expect("create scratch pool root"); - let pool = StoreSlotPool::with_root(root.path().to_path_buf(), capacity); + let pool = StoreSlotPool::with_root(root.path().to_path_buf()); (root, pool) } @@ -514,7 +468,7 @@ mod host_tests { fn two_leased_slots_host_two_live_sessions() { let mut log = Logger::new(Mode::Console); let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); - let (_root, pool) = scratch_pool(4); + let (_root, pool) = scratch_pool(); let (lease_a, session_a) = pool .lease_with(|path| { @@ -570,7 +524,7 @@ mod host_tests { fn a_session_is_refused_when_another_process_holds_its_store() { let mut log = Logger::new(Mode::Console); let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); - let (_root, pool) = scratch_pool(1); + let (_root, pool) = scratch_pool(); let (lease, holder) = pool .lease_with(|path| { @@ -615,7 +569,7 @@ mod host_tests { fn a_returned_slot_keeps_its_cache_for_the_next_lease() { let mut log = Logger::new(Mode::Console); let sdk = unsafe { init_and_load_sdk(&mut log) }.expect("load SDK"); - let (_root, pool) = scratch_pool(1); + let (_root, pool) = scratch_pool(); let (lease, session) = pool .lease_with(|path| { diff --git a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs index e2998e6a0..bee67b299 100644 --- a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs +++ b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs @@ -872,7 +872,7 @@ impl Worker { }) .map_err(|e| match e { StoreSlotError::Refused { source, .. } => sr_err(source), - exhausted_or_io => WorkerError::Backend(anyhow::anyhow!("{exhausted_or_io}")), + pool_failure => WorkerError::Backend(anyhow::anyhow!("{pool_failure}")), })?; self.session = Some(session); diff --git a/src/tools/wxc/src/main.rs b/src/tools/wxc/src/main.rs index 239c2a357..b150fd308 100644 --- a/src/tools/wxc/src/main.rs +++ b/src/tools/wxc/src/main.rs @@ -138,12 +138,16 @@ struct Cli { /// Also warm the first N container-lifecycle store slots, which the /// lifecycle daemon leases from and `--storage-path` does not reach. - /// Clamped to the pool capacity (`MXC_WSLC_STORE_SLOTS`, default 4), since - /// a slot the pool never hands out costs a pull and a multi-GiB store for - /// nothing. Defaults to 0 so warming a create-and-run cache stays one pull - /// per image. Requires `--setup-wslc`. - #[arg(long = "wslc-daemon-slots", value_name = "N", requires = "setup_wslc")] - wslc_daemon_slots: Option, + /// Defaults to 0 so warming a create-and-run cache stays one pull per + /// image; each slot warmed adds a pull and a multi-GiB store. + /// Requires `--setup-wslc`. + #[arg( + long = "wslc-daemon-slots", + value_name = "N", + value_parser = clap::value_parser!(u16).range(0..=64), + requires = "setup_wslc" + )] + wslc_daemon_slots: Option, /// Run the fallback detector and emit JSON, without spawning a sandbox. #[arg(long)] @@ -1226,7 +1230,7 @@ fn main() { mxc_sdk::wslc_common::image::setup_pull_image( image, cli.storage_path.as_deref(), - cli.wslc_daemon_slots.unwrap_or(0), + cli.wslc_daemon_slots.unwrap_or(0) as usize, &mut logger, ) }; From 81efa5342a9e1f690bbc0390b334a83c60726ba3 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Tue, 13 Oct 2026 13:51:52 -0700 Subject: [PATCH 5/6] Addressed PR comments Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 763fd4b3-1dff-4398-8b27-e82d89a02a35 --- .../wslc/wsl-container-getting-started.md | 27 ++ docs/backends/wslc/wslc-state-aware.md | 7 +- scripts/setup-wslc.ps1 | 69 ++-- .../backends/wslc/common/container_steps.rs | 16 - src/mxc-sdk/src/backends/wslc/common/image.rs | 310 +++++++++++++----- .../src/backends/wslc/common/store_slots.rs | 27 +- src/tools/wxc/src/main.rs | 25 +- tests/scripts/run_wslc_all_tests.ps1 | 8 +- tests/scripts/run_wslc_state_aware_tests.ps1 | 9 +- 9 files changed, 342 insertions(+), 156 deletions(-) diff --git a/docs/backends/wslc/wsl-container-getting-started.md b/docs/backends/wslc/wsl-container-getting-started.md index 331807fab..72f4cbcc6 100644 --- a/docs/backends/wslc/wsl-container-getting-started.md +++ b/docs/backends/wslc/wsl-container-getting-started.md @@ -131,6 +131,14 @@ Or pull a single image directly: --setup-wslc --image alpine:latest ``` +Repeat `--image` to pull several in one go, which opens each store once instead +of once per image: + +```powershell +.\src\target\x86_64-pc-windows-msvc\release\wxc-exec.exe ` + --setup-wslc --image alpine:latest --image python:3.12-alpine +``` + Pulled images persist in the cache until you remove them — pay the cost once per image, not once per run. @@ -222,6 +230,25 @@ prepare a machine that will run offline — warm the cache first: .\scripts\setup-wslc.ps1 -Image ``` +### How warming works + +A store is a directory holding a `storage.vhdx`, and the SDK offers no way to +write into one directly — a pull needs a live session. So warming opens a +short-lived WSLc session against each target store, pulls every requested image +into it, and releases it. The session is only a way to reach the file; what +persists is the VHDX, which a later run or a lifecycle daemon reads. + +One session is opened per *store*, not per image, so warming three images into +two stores costs two sessions rather than six. Passing every image to one +invocation is therefore cheaper than invoking the tool once per image: + +```powershell +.\scripts\setup-wslc.ps1 -Image alpine:latest, python:3.12-alpine -DaemonSlots 1 +``` + +That warms two stores — the create-and-run store and lifecycle slot 0 — with +both images, booting two utility VMs in total. + ## Usage ### TypeScript SDK diff --git a/docs/backends/wslc/wslc-state-aware.md b/docs/backends/wslc/wslc-state-aware.md index 96f9722b8..74bbede4a 100644 --- a/docs/backends/wslc/wslc-state-aware.md +++ b/docs/backends/wslc/wslc-state-aware.md @@ -175,8 +175,11 @@ wxc-exec.exe --setup-wslc --image alpine:latest --wslc-daemon-slots 1 ``` Both default to 0 and warm slots from 0 up, so `1` is what a daemon holding one -session needs. See -[the getting-started guide](wsl-container-getting-started.md#step-3--container-images-optional-pre-pull) +session needs. Each store is opened once and filled with every image given, so +pass them all to one invocation. See +[the getting-started guide](wsl-container-getting-started.md#how-warming-works) +for how warming works, and +[Step 3](wsl-container-getting-started.md#step-3--container-images-optional-pre-pull) for the create-and-run store, which these do not replace. A slot directory is created when it is first leased, so an unused index costs diff --git a/scripts/setup-wslc.ps1 b/scripts/setup-wslc.ps1 index dfca643cc..d1a7ff0fc 100644 --- a/scripts/setup-wslc.ps1 +++ b/scripts/setup-wslc.ps1 @@ -9,11 +9,11 @@ Warms the WSLC SDK image cache ahead of time. A run pulls on a cache miss by itself, so this is an optimisation rather than a prerequisite — it moves the download off the critical path of a later run, and it is how you - populate a cache for a host that cannot reach a registry. Each image is - pulled via `wxc-exec.exe --setup-wslc --image `, which opens a minimal - WSLC session against the configured `storage_path` and invokes - `WslcPullSessionImage`. The pulled images persist in that storage path - and become visible to subsequent runtime executions. + populate a cache for a host that cannot reach a registry. All images are + pulled via a single `wxc-exec.exe --setup-wslc --image ...` + invocation per store, which opens one WSLC session against that store and + invokes `WslcPullSessionImage` for each image. The pulled images persist in + that storage path and become visible to subsequent runtime executions. The storage path you pass here MUST match the value used at run time (the `wslc.storagePath` field of the config, or the @@ -54,7 +54,9 @@ Enable verbose logging from wxc-exec (passes `--debug`). .PARAMETER Force - Continue pulling subsequent images even if one fails. + Retained for compatibility. Every image is now pulled in one `wxc-exec` + invocation per store, which always attempts them all and reports each + failure, so there is no longer a run to continue past. .EXAMPLE .\setup-wslc.ps1 @@ -128,37 +130,36 @@ if ($DaemonSlots -gt 0) { Write-Host "Images : $($Image -join ', ')`n" -ForegroundColor Gray $failed = @() +Write-Host " pulling $($Image.Count) image(s) ... " -NoNewline + +$wxcArgs = @("--setup-wslc") foreach ($img in $Image) { - Write-Host " $img ... " -NoNewline - $wxcArgs = @("--setup-wslc", "--image", $img) - if ($StoragePath) { - $wxcArgs += @("--storage-path", $StoragePath) - } - if ($DaemonSlots -gt 0) { - $wxcArgs += @("--wslc-daemon-slots", "$DaemonSlots") - } - if ($DebugLogs) { - $wxcArgs += "--debug" - } + $wxcArgs += @("--image", $img) +} +if ($StoragePath) { + $wxcArgs += @("--storage-path", $StoragePath) +} +if ($DaemonSlots -gt 0) { + $wxcArgs += @("--wslc-daemon-slots", "$DaemonSlots") +} +if ($DebugLogs) { + $wxcArgs += "--debug" +} + +$prevPref = $ErrorActionPreference +$ErrorActionPreference = "Continue" +$output = & $WxcExec @wxcArgs 2>&1 | Out-String +$exitCode = $LASTEXITCODE +$ErrorActionPreference = $prevPref - $prevPref = $ErrorActionPreference - $ErrorActionPreference = "Continue" - $output = & $WxcExec @wxcArgs 2>&1 | Out-String - $exitCode = $LASTEXITCODE - $ErrorActionPreference = $prevPref - - if ($exitCode -eq 0) { - Write-Host "OK" -ForegroundColor Green - } else { - Write-Host "FAIL (exit $exitCode)" -ForegroundColor Red - $output -split "`n" | Where-Object { $_.Trim() -ne "" } | ForEach-Object { - Write-Host " > $($_.TrimEnd())" -ForegroundColor Gray - } - $failed += $img - if (-not $Force) { - break - } +if ($exitCode -eq 0) { + Write-Host "OK" -ForegroundColor Green +} else { + Write-Host "FAIL (exit $exitCode)" -ForegroundColor Red + $output -split "`n" | Where-Object { $_.Trim() -ne "" } | ForEach-Object { + Write-Host " > $($_.TrimEnd())" -ForegroundColor Gray } + $failed += $Image } Write-Host "" diff --git a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs index 54153c3ca..291dec2d4 100644 --- a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs +++ b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs @@ -792,22 +792,6 @@ fn store_access_failure(hr: HRESULT, storage_path: &str) -> ScriptResponse { } } -/// Names of the images a session's store holds. -/// -/// This is the cheapest call that proves a session handle is usable rather than -/// merely non-null, since it has to reach the store's VHDX to answer. -/// -/// # Safety -/// `sdk` must hold valid function pointers and `session` must be a live handle. -pub unsafe fn list_session_image_names( - sdk: &WslcSdk, - session: WslcSession, -) -> Result, ScriptResponse> { - list_stored_images(sdk, session) - .map(|stored| stored.into_iter().map(|entry| entry.name).collect()) - .map_err(|hr| sdk_error("WslcListSessionImages failed", hr, "")) -} - /// One entry of a session's image store. pub(crate) struct StoredImage { pub(crate) name: String, diff --git a/src/mxc-sdk/src/backends/wslc/common/image.rs b/src/mxc-sdk/src/backends/wslc/common/image.rs index 5ff2c1797..ded0aab5d 100644 --- a/src/mxc-sdk/src/backends/wslc/common/image.rs +++ b/src/mxc-sdk/src/backends/wslc/common/image.rs @@ -12,7 +12,9 @@ use crate::mxc_common::logger::Logger; use crate::mxc_common::models::ScriptResponse; use crate::mxc_common::string_util::{to_wide, CoTaskMemPWSTR}; -use crate::wslc_common::container_steps::{self, cstr_bytes, sdk_error, validate_store_access}; +use crate::wslc_common::container_steps::{ + self, cstr_bytes, sdk_error, validate_store_access, StoredImage, +}; use crate::wslc_common::error::WslcError; use crate::wslc_common::registry_policy; use crate::wslc_common::sdk_init; @@ -439,6 +441,20 @@ pub enum StoreHint { } impl StoreHint { + /// The store directory this names. + fn store_path(&self) -> String { + match self { + StoreHint::CreateAndRun(Some(path)) => path.clone(), + StoreHint::CreateAndRun(None) => std::env::temp_dir() + .join("mxc-wslc-sessions") + .to_string_lossy() + .to_string(), + StoreHint::DaemonSlot(index) => { + store_slots::slot_path(*index).to_string_lossy().to_string() + } + } + } + /// The arguments that point `--setup-wslc` at this store, if it needs any. fn warm_args(&self) -> String { match self { @@ -827,13 +843,7 @@ pub unsafe fn resolve_image( } } -/// An image the session's store already holds. -pub struct CachedImage { - name: String, - sha256: [u8; 32], -} - -impl CachedImage { +impl StoredImage { /// Whether this entry satisfies a request for `requested`. /// /// A reference pinning a digest never does. The store reports the config @@ -870,17 +880,8 @@ fn digest_of(reference: &str) -> Option<&str> { unsafe fn list_cached_images( sdk: &WslcSdk, session: WslcSession, -) -> Result, ScriptResponse> { +) -> Result, ScriptResponse> { container_steps::list_stored_images(sdk, session) - .map(|stored| { - stored - .into_iter() - .map(|entry| CachedImage { - name: entry.name, - sha256: entry.sha256, - }) - .collect() - }) .map_err(|hr| sdk_error("WslcListSessionImages failed", hr, "")) } @@ -936,20 +937,22 @@ fn with_implicit_tag(name: &str) -> String { } } -/// Warm the create-and-run store's image cache with `image_name`, and the first -/// `daemon_slots` container-lifecycle store slots with it too, backing +/// Warm the create-and-run store's image cache with `images`, and the first +/// `daemon_slots` container-lifecycle store slots with them too, backing /// `wxc-exec.exe --setup-wslc`. /// +/// Each store is opened once and filled with every image, so warming M images +/// across N slots costs N+1 sessions rather than M×(N+1). +/// /// The lifecycle daemon leases its store from a separate pool, so warming the /// create-and-run store does not warm the daemon's. A slot that has never been /// leased holds no images at all, and an uncached image under the default -/// deny-egress posture is refused before the VM is reached. -/// +/// deny-egress posture is refused before the VM is reached./// /// # Safety /// Must be called once per process before any other WSLC SDK functions /// (it initialises COM via `init_and_load_sdk`). pub unsafe fn setup_pull_image( - image_name: &str, + images: &[&str], storage_path: Option<&str>, daemon_slots: usize, logger: &mut Logger, @@ -959,53 +962,94 @@ pub unsafe fn setup_pull_image( Err(resp) => return Err(resp.error_message), }; - warm_store( - sdk, - image_name, - &StoreHint::CreateAndRun(storage_path.map(str::to_string)), - storage_path, - logger, - )?; - - for index in 0..daemon_slots { - let slot = store_slots::slot_path(index); - std::fs::create_dir_all(&slot).map_err(|e| { - format!( - "could not create the WSLc store slot at {}: {e}", - slot.display() - ) - })?; - warm_store( - sdk, - image_name, - &StoreHint::DaemonSlot(index), - Some(&slot.to_string_lossy()), - logger, - ) - .map_err(|e| format!("{e} (slot {index} of {daemon_slots}; earlier slots were warmed)"))?; + let mut targets = vec![StoreHint::CreateAndRun(storage_path.map(str::to_string))]; + targets.extend((0..daemon_slots).map(StoreHint::DaemonSlot)); + + warm_each(&targets, |target| { + if let StoreHint::DaemonSlot(index) = target { + let slot = store_slots::slot_path(*index); + std::fs::create_dir_all(&slot).map_err(|e| { + format!( + "could not create the WSLc store slot at {}: {e}", + slot.display() + ) + })?; + } + warm_store(sdk, images, target, logger) + }) +} + +/// Warm every target, reporting what failed rather than stopping at the first. +/// +/// The create-and-run store and the lifecycle slots are independent, so one +/// being held says nothing about whether the others can be filled. +fn warm_each( + targets: &[StoreHint], + mut warm: impl FnMut(&StoreHint) -> Result<(), String>, +) -> Result<(), String> { + let mut failures = Vec::new(); + + for target in targets { + if let Err(e) = warm(target) { + failures.push(match target { + StoreHint::CreateAndRun(_) => e, + StoreHint::DaemonSlot(index) => format!("daemon store slot {index}: {e}"), + }); + } } - Ok(()) + if failures.is_empty() { + Ok(()) + } else { + Err(failures.join("; ")) + } +} + +/// What one warm attempt leaves behind. +enum PullStep { + /// The image landed, or failed in a way that leaves the session usable. + Continue(Result<(), String>), + + /// The pull was abandoned and may still be writing into the session, so + /// nothing further may be pulled on it. + Abandoned(String), +} + +/// Pull every image on one session, stopping only once the session is no longer +/// usable. +fn pull_each(images: &[&str], mut pull: impl FnMut(&str) -> PullStep) -> Result<(), String> { + let mut failures = Vec::new(); + + for image in images { + match pull(image) { + PullStep::Continue(Ok(())) => {} + PullStep::Continue(Err(e)) => failures.push(e), + PullStep::Abandoned(e) => { + failures.push(e); + break; + } + } + } + + if failures.is_empty() { + Ok(()) + } else { + Err(failures.join("; ")) + } } -/// Pull `image_name` into the store at `storage_path` and release the session. +/// Pull `images` into the store `store` names, on one session. /// /// # Safety /// `sdk` must hold valid function pointers and COM must be initialised on this /// thread. unsafe fn warm_store( sdk: &'static WslcSdk, - image_name: &str, + images: &[&str], store: &StoreHint, - storage_path: Option<&str>, logger: &mut Logger, ) -> Result<(), String> { - let storage_path_str = storage_path.map(|s| s.to_string()).unwrap_or_else(|| { - std::env::temp_dir() - .join("mxc-wslc-sessions") - .to_string_lossy() - .to_string() - }); + let storage_path_str = store.store_path(); let session_name: Vec = to_wide("mxc-setup-wslc"); let storage_path_wide: Vec = to_wide(&storage_path_str); @@ -1047,31 +1091,32 @@ unsafe fn warm_store( } let _ = writeln!(logger, "[WSLC setup] Target store: {}", storage_path_str); - let budget = pull_budget(); - let (done, finished) = std::sync::mpsc::channel(); - start_pull( - sdk, - session, - image_name, - store, - "[WSLC setup]", - move |outcome| { + + // Taken when a pull is abandoned, so the session is leaked rather than + // released. + let mut session_guard = Some(session_guard); + + pull_each(images, |image| { + let budget = pull_budget(); + let (done, finished) = std::sync::mpsc::channel(); + if let Err(resp) = start_pull(sdk, session, image, store, "[WSLC setup]", move |outcome| { let _ = done.send(outcome); - }, - ) - .map_err(|resp| resp.error_message)?; - - match finished.recv_timeout(budget) { - Ok(outcome) => outcome.map_err(|resp| resp.error_message), - Err(_) => { - // Leak the session if the pull never drained: dropping the guard - // closes a session the SDK is still writing into. - if !wait_for_pulls_in_flight(PULL_DRAIN_TIMEOUT) { - std::mem::forget(session_guard); + }) { + return PullStep::Continue(Err(resp.error_message)); + } + + match finished.recv_timeout(budget) { + Ok(outcome) => PullStep::Continue(outcome.map_err(|resp| resp.error_message)), + Err(_) => { + if !wait_for_pulls_in_flight(PULL_DRAIN_TIMEOUT) { + if let Some(guard) = session_guard.take() { + std::mem::forget(guard); + } + } + PullStep::Abandoned(pull_deadline_expired(image, budget).error_message) } - Err(pull_deadline_expired(image_name, budget).error_message) } - } + }) } #[cfg(test)] @@ -1166,8 +1211,8 @@ mod tests { ); } - fn cached(name: &str) -> CachedImage { - CachedImage { + fn cached(name: &str) -> StoredImage { + StoredImage { name: name.to_string(), sha256: [0u8; 32], } @@ -1265,7 +1310,7 @@ mod tests { let mut sha = [0u8; 32]; sha[0] = 0x0e; sha[31] = 0xff; - let hex = CachedImage { + let hex = StoredImage { name: "x".to_string(), sha256: sha, } @@ -1600,6 +1645,107 @@ mod tests { ); } + #[test] + fn a_busy_create_and_run_store_still_lets_the_slots_warm() { + let targets = [ + StoreHint::CreateAndRun(None), + StoreHint::DaemonSlot(0), + StoreHint::DaemonSlot(1), + ]; + let mut warmed = Vec::new(); + + let outcome = warm_each(&targets, |target| match target { + StoreHint::CreateAndRun(_) => Err("store is held".to_string()), + StoreHint::DaemonSlot(index) => { + warmed.push(*index); + Ok(()) + } + }); + + assert_eq!( + warmed, + [0, 1], + "the two stores are independent, so a held create-and-run store \ + must not leave a free lifecycle slot cold" + ); + assert_eq!( + outcome, + Err("store is held".to_string()), + "an unreachable store is still a failure, not a silent skip" + ); + } + + #[test] + fn every_store_that_failed_is_named() { + let targets = [StoreHint::CreateAndRun(None), StoreHint::DaemonSlot(1)]; + + let outcome = warm_each(&targets, |target| match target { + StoreHint::CreateAndRun(_) => Err("temp store is held".to_string()), + StoreHint::DaemonSlot(_) => Err("slot is held".to_string()), + }); + + assert_eq!( + outcome, + Err("temp store is held; daemon store slot 1: slot is held".to_string()) + ); + } + + #[test] + fn warming_every_store_reports_success() { + let targets = [StoreHint::CreateAndRun(None), StoreHint::DaemonSlot(0)]; + + assert_eq!(warm_each(&targets, |_| Ok(())), Ok(())); + } + + #[test] + fn one_image_failing_still_pulls_the_rest() { + let mut attempted = Vec::new(); + + let outcome = pull_each(&["alpine:latest", "nope:1", "python:3"], |image| { + attempted.push(image.to_string()); + if image == "nope:1" { + PullStep::Continue(Err("not found".to_string())) + } else { + PullStep::Continue(Ok(())) + } + }); + + assert_eq!(attempted, ["alpine:latest", "nope:1", "python:3"]); + assert_eq!(outcome, Err("not found".to_string())); + } + + #[test] + fn an_abandoned_pull_stops_the_rest() { + let mut attempted = Vec::new(); + + let outcome = pull_each(&["alpine:latest", "slow:1", "python:3"], |image| { + attempted.push(image.to_string()); + if image == "slow:1" { + PullStep::Abandoned("gave up".to_string()) + } else { + PullStep::Continue(Ok(())) + } + }); + + assert_eq!( + attempted, + ["alpine:latest", "slow:1"], + "an abandoned pull may still be writing into the session, so nothing \ + further may be pulled on it" + ); + assert_eq!(outcome, Err("gave up".to_string())); + } + + #[test] + fn pulling_every_image_reports_success() { + assert_eq!( + pull_each(&["alpine:latest", "python:3"], |_| PullStep::Continue(Ok( + () + ))), + Ok(()) + ); + } + #[test] fn every_rejection_names_a_way_forward() { for hr in [WSLC_E_IMAGE_NOT_FOUND, WSLC_E_REGISTRY_BLOCKED_BY_POLICY] { diff --git a/src/mxc-sdk/src/backends/wslc/common/store_slots.rs b/src/mxc-sdk/src/backends/wslc/common/store_slots.rs index 7731652d3..118ef788a 100644 --- a/src/mxc-sdk/src/backends/wslc/common/store_slots.rs +++ b/src/mxc-sdk/src/backends/wslc/common/store_slots.rs @@ -447,10 +447,14 @@ mod host_tests { use super::*; use crate::mxc_common::logger::{Logger, Mode}; - use crate::wslc_common::container_steps::{create_daemon_session, list_session_image_names}; + use crate::wslc_common::container_steps::{create_daemon_session, list_stored_images}; use crate::wslc_common::image::pull_image; use crate::wslc_common::sdk_init::init_and_load_sdk; + fn image_names(stored: Vec) -> Vec { + stored.into_iter().map(|entry| entry.name).collect() + } + /// Small, and the default the container-lifecycle backend uses, so a /// developer's warm cache likely already has it. const CACHE_IMAGE: &str = "alpine:latest"; @@ -502,20 +506,20 @@ mod host_tests { // Both handles have to be usable *while the other is live*, not merely // created: a session that cannot read its store is not a working one. // SAFETY: both sessions are live. - unsafe { list_session_image_names(sdk, session_a.as_raw()) } + unsafe { list_stored_images(sdk, session_a.as_raw()) } .expect("session A must stay usable while B is live"); // SAFETY: both sessions are live. - unsafe { list_session_image_names(sdk, session_b.as_raw()) } + unsafe { list_stored_images(sdk, session_b.as_raw()) } .expect("session B must be usable while A is live"); // SAFETY: session A is live. - unsafe { list_session_image_names(sdk, session_a.as_raw()) } + unsafe { list_stored_images(sdk, session_a.as_raw()) } .expect("session A must still be usable after B was used"); drop(session_b); drop(lease_b); // SAFETY: session A is live. - unsafe { list_session_image_names(sdk, session_a.as_raw()) } + unsafe { list_stored_images(sdk, session_a.as_raw()) } .expect("session A must survive session B's release"); } @@ -560,7 +564,7 @@ mod host_tests { ); // SAFETY: the holder is still live. - unsafe { list_session_image_names(sdk, holder.as_raw()) } + unsafe { list_stored_images(sdk, holder.as_raw()) } .expect("refusing the second session must leave the holder usable"); } @@ -582,7 +586,8 @@ mod host_tests { let store = lease.path().to_string_lossy().to_string(); // SAFETY: the session is live. - let cold = unsafe { list_session_image_names(sdk, session.as_raw()) }.expect("list cold"); + let cold = + image_names(unsafe { list_stored_images(sdk, session.as_raw()) }.expect("list cold")); assert!( cold.is_empty(), "a slot that has never been leased holds no images, which is why a \ @@ -602,7 +607,8 @@ mod host_tests { } .expect("pull must succeed; this test needs egress"); // SAFETY: the session is live. - let warmed = unsafe { list_session_image_names(sdk, session.as_raw()) }.expect("list warm"); + let warmed = + image_names(unsafe { list_stored_images(sdk, session.as_raw()) }.expect("list warm")); assert!(!warmed.is_empty(), "the pull must have filled the store"); drop(session); @@ -618,8 +624,9 @@ mod host_tests { .expect("re-lease after return"); assert_eq!(next_lease.path().to_string_lossy(), store); // SAFETY: the session is live. - let reopened = - unsafe { list_session_image_names(sdk, next_session.as_raw()) }.expect("list reopened"); + let reopened = image_names( + unsafe { list_stored_images(sdk, next_session.as_raw()) }.expect("list reopened"), + ); assert_eq!( warmed, reopened, diff --git a/src/tools/wxc/src/main.rs b/src/tools/wxc/src/main.rs index b150fd308..96b231547 100644 --- a/src/tools/wxc/src/main.rs +++ b/src/tools/wxc/src/main.rs @@ -125,9 +125,11 @@ struct Cli { setup_wslc: bool, /// Image reference to pre-pull (e.g. `alpine:latest`, - /// `ghcr.io/owner/image:tag`). Required with `--setup-wslc`. + /// `ghcr.io/owner/image:tag`). Repeat to pull several images, which fills + /// each store in one session rather than one per image. Required with + /// `--setup-wslc`. #[arg(long = "image", requires = "setup_wslc")] - image: Option, + image: Vec, /// Optional WSLC storage path. When omitted the runner default is used /// (`%TEMP%\mxc-wslc-sessions`). Pass the same value here that your @@ -1210,13 +1212,16 @@ fn main() { if cli.setup_wslc { #[cfg(feature = "wslc")] { - let image = match cli.image.as_deref() { - Some(s) if !s.is_empty() => s, - _ => { - eprintln!("Error: --setup-wslc requires --image "); - process::exit(1); - } - }; + let images: Vec<&str> = cli + .image + .iter() + .map(String::as_str) + .filter(|name| !name.is_empty()) + .collect(); + if images.is_empty() { + eprintln!("Error: --setup-wslc requires --image "); + process::exit(1); + } let mut logger = Logger::new(if cli.debug { Mode::Console } else { @@ -1228,7 +1233,7 @@ fn main() { // `init_and_load_sdk`. let result = unsafe { mxc_sdk::wslc_common::image::setup_pull_image( - image, + &images, cli.storage_path.as_deref(), cli.wslc_daemon_slots.unwrap_or(0) as usize, &mut logger, diff --git a/tests/scripts/run_wslc_all_tests.ps1 b/tests/scripts/run_wslc_all_tests.ps1 index 839c0b670..922b498ef 100644 --- a/tests/scripts/run_wslc_all_tests.ps1 +++ b/tests/scripts/run_wslc_all_tests.ps1 @@ -96,7 +96,13 @@ if (-not $SkipSetup) { ) & $SetupScript -WxcExecPath $WxcExec -Image $images -DaemonSlots 1 -Force if ($LASTEXITCODE -ne 0) { - Write-Host "WARN: setup-wslc.ps1 reported failures; continuing with tests anyway." -ForegroundColor Yellow + # The create-and-run half pulls on a miss, but the delegated + # lifecycle suite provisions with the isolated posture and cannot, + # so a cold slot 0 surfaces much later as unrelated failures. + Write-Host "ERROR: setup-wslc.ps1 failed; daemon store slot 0 may be cold." -ForegroundColor Red + Write-Host " The delegated lifecycle suite refuses registry pulls, so it cannot recover." -ForegroundColor Yellow + Write-Host " Resolve the setup failure above, or pass -SkipSetup if both stores are warm." -ForegroundColor Yellow + exit 1 } Write-Host "" } else { diff --git a/tests/scripts/run_wslc_state_aware_tests.ps1 b/tests/scripts/run_wslc_state_aware_tests.ps1 index aa5174260..587fb32fe 100644 --- a/tests/scripts/run_wslc_state_aware_tests.ps1 +++ b/tests/scripts/run_wslc_state_aware_tests.ps1 @@ -134,7 +134,14 @@ if (-not $SkipSetup) { Write-Host "Pre-pulling WSLc images (pass -SkipSetup to skip)..." -ForegroundColor Cyan & $SetupScript -WxcExecPath $WxcExec -Image @("alpine:latest", "python:3.12-alpine") -DaemonSlots 1 -Force if ($LASTEXITCODE -ne 0) { - Write-Host "WARN: setup-wslc.ps1 reported failures; continuing anyway." -ForegroundColor Yellow + # Continuing would leave slot 0 cold, and the isolated fixtures + # cannot pull, so every later provision fails for a reason that + # looks nothing like this preflight. + Write-Host "ERROR: setup-wslc.ps1 failed to warm daemon store slot 0." -ForegroundColor Red + Write-Host " The lifecycle fixtures provision with the isolated posture, which refuses a" -ForegroundColor Yellow + Write-Host " registry pull, so a cold slot fails every provision rather than pulling." -ForegroundColor Yellow + Write-Host " Resolve the setup failure above, or pass -SkipSetup if slot 0 is already warm." -ForegroundColor Yellow + exit 1 } Write-Host "" } else { From 34187f5be25e3197276fff247570c1ecacbc6627 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Wed, 14 Oct 2026 13:44:36 -0700 Subject: [PATCH 6/6] Addressed PR comments Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 763fd4b3-1dff-4398-8b27-e82d89a02a35 --- docs/backends/wslc/wslc-state-aware.md | 5 ++ src/mxc-sdk/src/backends/wslc/common/image.rs | 54 ++++++++++++++----- .../src/bin/wslc_daemon/session_manager.rs | 3 +- 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/docs/backends/wslc/wslc-state-aware.md b/docs/backends/wslc/wslc-state-aware.md index 74bbede4a..8d89dbe3a 100644 --- a/docs/backends/wslc/wslc-state-aware.md +++ b/docs/backends/wslc/wslc-state-aware.md @@ -152,6 +152,11 @@ when it is released, so a daemon holding one session always runs on `slot-0`. Create-and-run execution keeps its own default of `%TEMP%\mxc-wslc-sessions` and never touches the pool. +Both the daemon and the command that warms the pool resolve these variables from +their own environment, so warm the pool as the user the daemon runs as. A +refusal names the directory it needs filled, so a warm that reports success +against a different directory has filled the wrong store. + Separating the two is what lets both surfaces run at once. A store's VHDX admits one live session, so while the daemon held `%TEMP%\mxc-wslc-sessions` every create-and-run WSLc execution on the same host failed with diff --git a/src/mxc-sdk/src/backends/wslc/common/image.rs b/src/mxc-sdk/src/backends/wslc/common/image.rs index ded0aab5d..33f666c7f 100644 --- a/src/mxc-sdk/src/backends/wslc/common/image.rs +++ b/src/mxc-sdk/src/backends/wslc/common/image.rs @@ -378,7 +378,7 @@ fn pull_failure( budget.as_secs(), PULL_TIMEOUT_ENV, image, - store.warm_args(), + store.warm_instruction(), )), WSLC_E_IMAGE_NOT_FOUND => WslcError::Rejected(format!( "WSLC image '{}' could not be pulled: {}. Check the image name and tag. \ @@ -404,7 +404,7 @@ fn pull_failure( image, detail, image, - store.warm_args(), + store.warm_instruction(), )), } .into_response() @@ -455,12 +455,18 @@ impl StoreHint { } } - /// The arguments that point `--setup-wslc` at this store, if it needs any. - fn warm_args(&self) -> String { + /// The arguments that point `--setup-wslc` at this store, naming the + /// directory when the arguments leave it to be re-resolved from the + /// environment of whoever runs them. + fn warm_instruction(&self) -> String { match self { - StoreHint::CreateAndRun(None) => String::new(), StoreHint::CreateAndRun(Some(path)) => format!(" --storage-path \"{}\"", path), - StoreHint::DaemonSlot(index) => format!(" --wslc-daemon-slots {}", index + 1), + StoreHint::CreateAndRun(None) => format!(" (which fills {})", self.store_path()), + StoreHint::DaemonSlot(index) => format!( + " --wslc-daemon-slots {} (which fills {})", + index + 1, + self.store_path() + ), } } } @@ -757,7 +763,7 @@ fn no_egress_refusal(image: &str, store: &StoreHint) -> ScriptResponse { See docs/backends/wslc/wsl-container-getting-started.md.", image, image, - store.warm_args(), + store.warm_instruction(), )) .into_response() } @@ -947,7 +953,8 @@ fn with_implicit_tag(name: &str) -> String { /// The lifecycle daemon leases its store from a separate pool, so warming the /// create-and-run store does not warm the daemon's. A slot that has never been /// leased holds no images at all, and an uncached image under the default -/// deny-egress posture is refused before the VM is reached./// +/// deny-egress posture is refused before the VM is reached. +/// /// # Safety /// Must be called once per process before any other WSLC SDK functions /// (it initialises COM via `init_and_load_sdk`). @@ -1598,7 +1605,7 @@ mod tests { } #[test] - fn a_default_store_adds_no_path_argument() { + fn a_default_store_names_its_directory_without_a_path_argument() { let resp = failure( "alpine:latest", &StoreHint::CreateAndRun(None), @@ -1606,6 +1613,13 @@ mod tests { "down", ); assert!(!resp.error_message.contains("--storage-path")); + assert!( + resp.error_message + .contains(&StoreHint::CreateAndRun(None).store_path()), + "a reader whose TEMP differs warms a different store unless the \ + default is named: {}", + resp.error_message + ); } #[test] @@ -1638,10 +1652,24 @@ mod tests { #[test] fn a_higher_slot_asks_for_enough_slots_to_reach_it() { - assert_eq!( - StoreHint::DaemonSlot(2).warm_args(), - " --wslc-daemon-slots 3", - "warming is counted from slot 0, so reaching slot 2 takes 3 slots" + let instruction = StoreHint::DaemonSlot(2).warm_instruction(); + assert!( + instruction.starts_with(" --wslc-daemon-slots 3"), + "warming is counted from slot 0, so reaching slot 2 takes 3 slots: \ + {instruction}" + ); + } + + #[test] + fn a_slot_remediation_names_the_store_it_fills() { + let slot = StoreHint::DaemonSlot(1); + let instruction = slot.warm_instruction(); + + assert!( + instruction.contains(&slot.store_path()), + "--wslc-daemon-slots re-resolves the pool from the environment of \ + whoever runs it, so a remediation that does not name the store can \ + warm a different one: {instruction}" ); } diff --git a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs index bee67b299..1e5ec024b 100644 --- a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs +++ b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs @@ -3098,7 +3098,8 @@ mod tests { // pull, so `alpine:latest` has to be cached in the store slot this worker // leases — slot 0 of the pool, since it holds one session. Warm it with // `scripts\setup-wslc.ps1 -DaemonSlots 1`. It is `#[ignore]`d and run - // explicitly with `cargo test -p mxc-sdk --features wslc --lib -- --ignored`. + // explicitly with: + // `cargo test -p mxc-sdk --features wslc --bin wxc-wslc-daemon -- --ignored` #[tokio::test] #[ignore = "requires a WSL2 host with alpine:latest cached in daemon store slot 0"] async fn full_lifecycle_on_wsl_host() {