From 224b9858bb2c5772f923aea23c9fecb2431208cf Mon Sep 17 00:00:00 2001 From: nullStack65 Date: Sat, 26 Sep 2026 21:06:09 -0400 Subject: [PATCH 1/7] feat(service): add minimal Windows SCM service host prototype Add native/windows-service-host, a T3-owned SCM entry point that runs the pinned t3.exe __service-launcher under a job object and reports service state. The portable config/control/restart-budget/drain core is unit tested; the SCM dispatch and job-object tree are Windows-only and cross-checked. This is a prototype only: the launcher control adaptation and BootService adapter remain a later single integration owner, and Windows background support stays unqualified until a real SCM run is executed. --- docs/internals/windows-background-service.md | 247 ++++++++ native/windows-service-host/Cargo.lock | 25 + native/windows-service-host/Cargo.toml | 42 ++ native/windows-service-host/README.md | 28 + native/windows-service-host/src/config.rs | 543 ++++++++++++++++++ native/windows-service-host/src/control.rs | 142 +++++ native/windows-service-host/src/host.rs | 202 +++++++ native/windows-service-host/src/lib.rs | 24 + native/windows-service-host/src/main.rs | 146 +++++ native/windows-service-host/src/run.rs | 353 ++++++++++++ native/windows-service-host/src/supervise.rs | 498 ++++++++++++++++ .../windows-service-host/src/windows/job.rs | 322 +++++++++++ .../windows-service-host/src/windows/mod.rs | 10 + .../src/windows/service.rs | 263 +++++++++ .../tests/portable_host.rs | 122 ++++ 15 files changed, 2967 insertions(+) create mode 100644 docs/internals/windows-background-service.md create mode 100644 native/windows-service-host/Cargo.lock create mode 100644 native/windows-service-host/Cargo.toml create mode 100644 native/windows-service-host/README.md create mode 100644 native/windows-service-host/src/config.rs create mode 100644 native/windows-service-host/src/control.rs create mode 100644 native/windows-service-host/src/host.rs create mode 100644 native/windows-service-host/src/lib.rs create mode 100644 native/windows-service-host/src/main.rs create mode 100644 native/windows-service-host/src/run.rs create mode 100644 native/windows-service-host/src/supervise.rs create mode 100644 native/windows-service-host/src/windows/job.rs create mode 100644 native/windows-service-host/src/windows/mod.rs create mode 100644 native/windows-service-host/src/windows/service.rs create mode 100644 native/windows-service-host/tests/portable_host.rs diff --git a/docs/internals/windows-background-service.md b/docs/internals/windows-background-service.md new file mode 100644 index 000000000000..abfc715aa82a --- /dev/null +++ b/docs/internals/windows-background-service.md @@ -0,0 +1,247 @@ +# Windows background service + +Status: prototype source only. Windows background support stays disabled and +unqualified until this host, the launcher control adaptation, the BootService +adapter, packaged artifacts and a real SCM run are joined and tested. + +The implementation is a [small Rust SCM host](../../native/windows-service-host/src/main.rs). +The rest of T3's background service is platform-neutral: [BootService](../../apps/server/src/cloud/bootService.ts) +owns install/status/restart/uninstall, and [serviceLauncher.ts](../../apps/server/src/serviceLauncher.ts) +owns the server child, remote updates and rollback. This host replaces neither. +It is the SCM entry point that starts the existing pinned `t3.exe +__service-launcher` and reports service state. + +Windows currently has no native background service at all. A desktop-owned +child and a WSL systemd unit are different lifecycles; neither is SCM support. +The tracked `native/resource-monitor` is a process monitor, not a service host. + +## SCM contract + +A real service is registered with `sc.exe create` (or the SCM API) and started +by the service control manager. Registering an ordinary console `t3 serve` is +not enough. The host implements: + +- `StartServiceCtrlDispatcherW` on the process main thread, with the service + name as a `SERVICE_TABLE_ENTRYW` row. +- `ServiceMain` on an SCM thread: `RegisterServiceCtrlHandlerExW`, then the + supervisor loop. +- `SetServiceStatus` with `START_PENDING`, `RUNNING`, `STOP_PENDING` and + `STOPPED`, `dwCheckPoint`/`dwWaitHint` while pending, and + `SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN` only while running. +- A control handler that records intent and wakes the supervisor; it never + blocks on the child. Control handling cannot hang indefinitely. + +The single Windows-specific trap is where the arguments live. Auto-start +services receive their command line through the process entry point; they are +**not** delivered to `ServiceMain`'s `argv`. The host parses +`std::env::args_os()` before it calls the dispatcher and never reads +`ServiceMain`'s arguments. + +## Identity and launch + +The host refuses to start without an explicit, canonical T3 home and the pinned +runtime. There is no `~/.t3` fallback, so an omitted argument can never point +the workload at an interactive user's profile. + +- `--home` must be absolute and must not be a drive root or a system directory. +- `--runtime` must be the pinned `t3.exe`. The host appends `__service-launcher` + and launches it once per service start. Remote updates replace the launcher's + server child; they do not re-exec the host or change its command line. +- `T3CODE_HOME` is set explicitly on the child; child stdout/stderr go to + `--log` when supplied. +- No credential flag is accepted. A password or token argument is refused + rather than forwarded. The account password is registered with SCM + (`sc.exe create T3Code ... obj= ".\t3service" password= "..."`) and stored by + LSA; the host never sees it. +- LocalSystem is refused unless `--allow-local-system` is passed explicitly. + `--expected-account` pins the account the process must run as. + +Account constraints: use a dedicated account, ideally the virtual +`NT SERVICE\T3Code` service SID, or a dedicated local/domain user. Do not run +the T3 workload as LocalSystem and do not reuse an interactive user's home or +profile. A virtual service account has no user profile or `HKCU`, so features +that need one are unavailable (below). + +## Shutdown and process ownership + +The child is created suspended, assigned to a job object with +`JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`, then resumed. Suspending closes the race +where a fast child could spawn grandchildren before assignment. The job is +created for the child; the host is not in it, so `TerminateJobObject` can kill +the owned tree while the host stays alive to report `SERVICE_STOPPED`. + +A stop is bounded and two-stage: + +1. On `SERVICE_CONTROL_STOP`/`SHUTDOWN`, the supervisor writes the launcher stop + marker and asks the launcher to stop, then reports `STOP_PENDING` with + checkpoints. +2. If the child has not exited by `--drain-timeout-ms` (default 30s), the job + tree is terminated and the service reports `STOPPED`. + +This is deliberately different from a normal launcher replacement. During an +update the launcher terminates its own server child but stays alive and starts +the replacement; the host must not touch the job during that handoff. The host +only terminates the tree for a whole-service stop. It does not know about +launcher protocol upgrades and does not manage updates or rollback. + +Ownership is verified before any stop or termination: the host re-opens the +recorded PID and compares its creation time with the one captured at spawn, +because a held handle stays valid after the child exits and cannot detect PID +reuse. `Ok(false)` means the PID is foreign and is left alone; a failed query +means ownership is **unknown** and is never read as a successful stop. Only a +verified-owned tree is stopped or terminated, so a stale or foreign PID is +never cleaned up. + +## Required launcher control (not implemented here) + +`serviceLauncher.ts` is owned by its current writer and is not edited by this +slice. Two small adaptations are required before the host can stop gracefully: + +- **A parent control channel.** Spawn the launcher with an IPC channel and have + `Launcher.run()` treat a `{ "type": "stop" }` message from its parent (the + host) like a `SIGTERM`: call `this.stop("SIGTERM")`, which writes the stop + marker before it terminates the child. This belongs next to the existing + `serviceProtocol.ts` messages as an additive type. The host already restores + `T3CODE_HOME` and starts `t3.exe __service-launcher`; it only needs to be + given the channel. +- **Graceful child shutdown on Windows.** Windows has no POSIX signals. Node's + `child.kill("SIGTERM")` calls `TerminateProcess`, so the server child cannot + read the stop marker in its shutdown finalizer. The launcher must deliver a + graceful shutdown over the child IPC channel it already opens, then fall back + to `terminateChild` after its own grace period. + +Until both land, the host's stop degrades to writing the marker and then +force-terminating the job after the drain. That is honest, bounded, and still +correct for the service tree, but it is not graceful server shutdown. + +Because the launcher is not adapted yet, the host writes the stop marker +directly. A later integration owner should decide whether the host keeps that +fallback or relies solely on the control message. + +## Honest failure states + +- **Unexpected exit:** the supervisor restarts the child while a restart budget + allows it (`--max-restarts` inside `--restart-window-ms`, default 5 in 300s, + mirroring the systemd unit), reporting `START_PENDING` between attempts. +- **Planned stop:** a child exit while `STOP_PENDING` is success, not a failure; + a non-zero exit code is ignored so a crashed-but-stopping child does not look + like an unexpected stop. +- **Repeated failure:** once the budget is exhausted the service stops with + `ERROR_SERVICE_SPECIFIC_ERROR` and a specific code instead of respawning + forever. There is no Windows analog of systemd's finite start limit, so the + budget lives here. +- **Slow drain:** reports `STOP_PENDING` with checkpoints and forces the tree + after the deadline. It never hangs. +- **Stale/foreign PID:** as above, verified and left alone; unknown is not + stopped. + +## Platform limitations + +- **Session 0.** The service runs in session 0 with no interactive desktop. + Anything that needs a GUI, a visible browser window, a desktop keychain or an + interactive-only credential is unavailable. +- **Browser auth.** OAuth and provider logins that open a browser cannot + complete in session 0. Credentials must be pre-seeded for the service + account, or the feature is unsupported in service mode. +- **Profile.** A virtual service account has no profile directory or `HKCU`. + A dedicated user account can have one, but it must be seeded and owned by + that account; it must not be an interactive user's profile. +- **A temporary login Scheduled Task is not SCM parity.** A task with "run only + when the user is logged on" runs in that user's interactive profile and stops + at logout. It is at most a declared temporary profile, never proof of the + unattended service. Do not present it as equivalent. +- **No wrapper dependency chosen.** NSSM/WinSW are not adopted. A wrapper would + have to justify a concrete maintenance or correctness advantage over this + ~1-file host; it would also become a fleet-wide runtime dependency and a + generic supervisor T3 does not otherwise need. No such comparison has been + made, so no wrapper is selected. + +## Integration boundary + +A later single integration owner joins these parts; this slice writes only +`native/windows-service-host/**` and this document. + +Adapter (`apps/server/src/cloud/bootService.ts`, owned by R6-T3-STATUS): +- Add `"scm"` to the manager union and a `windowsManager(...)` sibling of + `systemdManager`/`launchdManager`. `render` produces the host command + (`hostPath`, `--home`, `--runtime /t3.exe`, `--log`, + `--service-name`, optional `--expected-account`). The steps are `sc.exe + create/start/stop/delete/config` with `obj=` and the account, not a unit file. +- `selectBootServiceManager` returns it for `platform === "win32"` when the home + and account are known, instead of `undefined`. +- `BootServiceStatus` learns the SCM state; installation, registration and + observed running state stay separate, as they already are for Linux. + +Launcher (`apps/server/src/serviceLauncher.ts`, `serviceProtocol.ts`): the two +adaptations above. + +Packaging (`packaging/**`, root workspaces, release workflows): compile the host +and ship it beside the pinned runtime. Root Cargo/package workspaces and the +release pipeline are outside this slice. + +## Artifact and provenance inputs + +- The host binary: pinned fork commit and toolchain, signed, with a recorded + SHA-256. The SCM `ImagePath` must bind that exact binary. +- The pinned runtime archive and its `.install-complete` sentinel; the active + version comes from `runtime/service-state.json`, which the launcher owns. +- The `sc.exe qc T3Code` registration record: `ImagePath`, `obj=`, `start=`. +- `/5`'s frozen release branch and build source are untouched by this slice. + +## Rollback and native acceptance checklist + +Rollback: `sc.exe stop T3Code` then `sc.exe delete T3Code`, or restore the +previous `ImagePath` with `sc.exe config`. Existing T3 data under the home is +never touched by install or uninstall. If a previous non-SCM supervisor owns +the backend, do not attach a second one. + +Native acceptance (not executed here; see the recipe): + +1. `sc.exe query` reports `STOPPED` before start and `RUNNING` after; the + control handler answers interrogate without hanging. +2. A planned stop reaches `STOPPED` within the drain bound; the job tree is + empty afterwards. +3. Killing the dummy child produces a bounded restart sequence, then a specific + failure code; no restart storm. +4. A child that ignores the stop marker is force-terminated at the deadline and + reported `STOPPED`. +5. A stale PID and an unrelated process are never terminated. +6. Registration and cleanup remove exactly the synthetic service, its + processes, its home under the disposable test root, and nothing else. + +## Native test recipe (unexecuted) + +Only an environment already reserved for disposable Windows tests may run this, +with unique names, dummy children, a bounded runtime and verified cleanup. Do +not run it on an active workstation or against real T3 state. + +```powershell +# Build (developer host, Windows target): +cargo build --locked --release --manifest-path native/windows-service-host/Cargo.toml +# For dummy children instead of the pinned launcher, build with the +# development-only feature: +# cargo build --locked --release --features test-child ... + +$svc = "T3WinSvcProbe$([guid]::NewGuid().ToString('N').Substring(0,8))" +$root = Join-Path $env:TEMP $svc +$home = Join-Path $root "home"; New-Item -ItemType Directory -Force $home | Out-Null +# Dummy child: a script that ignores the stop marker and sleeps, plus a +# grandchild, to prove tree termination. +$dummy = Join-Path $root "dummy.cmd" +"@echo off`r`nstart /b ping -n 600 127.0.0.1 >nul`r`nping -n 600 127.0.0.1 >nul" | Set-Content $dummy + +sc.exe create $svc binPath= "`"$PWD\target\release\t3-windows-service-host.exe`" --home `"$home`" --service-name $svc --exec cmd.exe --exec-arg /c --exec-arg `"$dummy`" --drain-timeout-ms 3000" ` + obj= "NT AUTHORITY\LocalService" start= demand +sc.exe start $svc +sc.exe query $svc # expect RUNNING +sc.exe control $svc 4 # interrogate +sc.exe stop $svc # expect STOPPED within the drain bound +Get-Process -Name ping -ErrorAction SilentlyContinue # must list none for this probe +sc.exe delete $svc +Remove-Item -Recurse -Force $root +``` + +`--exec` exists only under `--features test-child` and is not part of the +production child selection. Portable and cross-compiled checks prove the +portable core, the SCM FFI's type surface and the quoting logic; they do not +prove a real service run, job-object ownership or graceful shutdown. diff --git a/native/windows-service-host/Cargo.lock b/native/windows-service-host/Cargo.lock new file mode 100644 index 000000000000..11e66424f235 --- /dev/null +++ b/native/windows-service-host/Cargo.lock @@ -0,0 +1,25 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "t3-windows-service-host" +version = "0.1.0" +dependencies = [ + "windows-sys", +] + +[[package]] +name = "windows-link" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" + +[[package]] +name = "windows-sys" +version = "0.61.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ae137229bcbd6cdf0f7b80a31df61766145077ddf49416a728b02cb3921ff3fc" +dependencies = [ + "windows-link", +] diff --git a/native/windows-service-host/Cargo.toml b/native/windows-service-host/Cargo.toml new file mode 100644 index 000000000000..03cabfb07268 --- /dev/null +++ b/native/windows-service-host/Cargo.toml @@ -0,0 +1,42 @@ +[package] +name = "t3-windows-service-host" +version = "0.1.0" +edition = "2024" +license = "MIT" +publish = false +description = "Minimal T3-owned Windows SCM host for the pinned service launcher." + +[lib] +name = "t3_windows_service_host" +path = "src/lib.rs" + +[[bin]] +name = "t3-windows-service-host" +path = "src/main.rs" + +[features] +# Development and test only. Enables `--exec` so the host can spawn an arbitrary +# dummy child instead of the pinned `t3.exe __service-launcher`. A packaged +# build must never enable this: the production child is selected by `--runtime`. +test-child = [] + +# SCM dispatch, job objects and process creation are Windows-only. Gating the +# dependency by target keeps `cargo test` on a developer host dependency-free +# and lets the portable core compile and run without a Windows toolchain. +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.61.2", features = [ + "Win32_Foundation", + "Win32_Security", + "Win32_Storage_FileSystem", + "Win32_System_Console", + "Win32_System_JobObjects", + "Win32_System_Services", + "Win32_System_Threading", + "Win32_System_WindowsProgramming", +] } + +[profile.release] +codegen-units = 1 +lto = "thin" +panic = "abort" +strip = true diff --git a/native/windows-service-host/README.md b/native/windows-service-host/README.md new file mode 100644 index 000000000000..478a2cf9719d --- /dev/null +++ b/native/windows-service-host/README.md @@ -0,0 +1,28 @@ +# t3-windows-service-host + +Minimal T3-owned Windows SCM host. It runs the pinned `t3.exe +__service-launcher` under a job object and reports service state to the +service control manager. It is not a general-purpose service wrapper and does +not manage T3 updates; the launcher owns those. + +See [docs/internals/windows-background-service.md](../../docs/internals/windows-background-service.md) +for the SCM contract, account constraints, the required launcher control +adaptation, and the native acceptance checklist. + +## Build and test + +```sh +# Portable core (developer host, no Windows toolchain needed): +cargo test --locked --manifest-path native/windows-service-host/Cargo.toml + +# Type-check the Windows-only SCM and job-object module: +cargo check --locked --target x86_64-pc-windows-msvc \ + --manifest-path native/windows-service-host/Cargo.toml + +# Windows release binary (Windows host): +cargo build --locked --release --manifest-path native/windows-service-host/Cargo.toml +``` + +The SCM dispatch path is unqualified until the native recipe in the design doc +runs on a disposable Windows environment. `--console` exercises the same +supervisor against a terminal and is not SCM proof. diff --git a/native/windows-service-host/src/config.rs b/native/windows-service-host/src/config.rs new file mode 100644 index 000000000000..9d81ae4e4a39 --- /dev/null +++ b/native/windows-service-host/src/config.rs @@ -0,0 +1,543 @@ +//! Service configuration and Windows-correct command-line parsing. +//! +//! The SCM starts the service process from the registered `ImagePath` command +//! line. Those arguments reach the process's ordinary entry point; they are +//! **not** delivered to `ServiceMain` for an auto-start service. The host +//! therefore parses `std::env::args_os()` in `main` before it calls +//! `StartServiceCtrlDispatcherW`, and never reads `ServiceMain`'s argv. +//! +//! Two rules matter for safety: +//! +//! - The T3 home is mandatory and must be canonical. There is no +//! `~/.t3` fallback, so the host can never silently run a workload against +//! an interactive user's profile because an argument was omitted. +//! - No credential may travel on the command line. SCM stores the service +//! account password in the LSA secret store; the host refuses obvious +//! password/token/secret flags instead of accepting them. + +use std::ffi::OsString; +use std::path::{Path, PathBuf}; +use std::time::Duration; + +/// Bounded wait for the launcher to release its child after a stop request. +pub const DEFAULT_DRAIN_TIMEOUT: Duration = Duration::from_secs(30); +/// Restart budget window. Mirrors the systemd unit's `StartLimitIntervalSec`. +pub const DEFAULT_RESTART_WINDOW: Duration = Duration::from_secs(300); +/// Mirrors the systemd unit's `StartLimitBurst`. +pub const DEFAULT_MAX_RESTARTS: u32 = 5; +/// How often the supervisor re-reads the child while running. +pub const DEFAULT_POLL_INTERVAL: Duration = Duration::from_millis(200); + +const FORBIDDEN_HOME_COMPONENTS: &[&str] = &[ + "windows", + "system32", + "program files", + "program files (x86)", + "programdata", +]; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum LaunchMode { + /// Production: run the pinned runtime's `__service-launcher` subcommand. + ServiceLauncher, + /// Feature-gated development mode: run an arbitrary dummy child. + #[cfg(feature = "test-child")] + TestChild { + program: PathBuf, + args: Vec, + }, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ServiceConfig { + /// Canonical T3 home. Also exported to the child as `T3CODE_HOME`. + pub home: PathBuf, + /// The pinned `t3.exe` the service launcher runs. Empty in test-child mode. + pub runtime: PathBuf, + /// Optional service log. When set, the child's stdout/stderr are redirected. + pub log: Option, + /// SCM service name used for `RegisterServiceCtrlHandlerExW`. + pub service_name: String, + /// Bounded wait after a graceful stop request before force-terminating. + pub drain_timeout: Duration, + /// Restart budget window. + pub restart_window: Duration, + /// Maximum child starts inside `restart_window` before the host gives up. + pub max_restarts: u32, + /// How often the running child is polled. + pub poll_interval: Duration, + /// Expected service account. When set, the Windows layer refuses to run as + /// any other account. + pub expected_account: Option, + /// LocalSystem is refused unless this is explicitly set. + pub allow_local_system: bool, + pub mode: LaunchMode, +} + +impl ServiceConfig { + /// The exact child command this host is allowed to launch. + pub fn child_command(&self) -> (PathBuf, Vec) { + match &self.mode { + LaunchMode::ServiceLauncher => ( + self.runtime.clone(), + vec![OsString::from("__service-launcher")], + ), + #[cfg(feature = "test-child")] + LaunchMode::TestChild { program, args } => (program.clone(), args.clone()), + } + } + + /// Path the SCM host writes before asking the launcher to stop its child. + /// Matches `SERVICE_STOP_MARKER_FILE` in the server's `serviceProtocol.ts`. + pub fn stop_marker(&self) -> PathBuf { + self.home.join("runtime").join(".service-stopping") + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Invocation { + /// Dispatch through the Windows SCM. Windows only. + Service(ServiceConfig), + /// Run the supervisor directly against a terminal. Never proof of SCM. + Console(ServiceConfig), + Help, + Version, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ConfigError { + MissingHome, + HomeNotAbsolute, + HomeNotCanonical, + MissingRuntime, + RuntimeNotAbsolute, + RuntimeNotPinned, + EmptyServiceName, + UnknownFlag(String), + MissingValue(String), + InvalidNumber(String), + CredentialArgument(String), + MissingTestChild, +} + +impl std::fmt::Display for ConfigError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ConfigError::MissingHome => write!( + formatter, + "an explicit --home is required; the host has no default home" + ), + ConfigError::HomeNotAbsolute => { + write!(formatter, "--home must be an absolute path") + } + ConfigError::HomeNotCanonical => write!( + formatter, + "--home must name a T3 data directory, not a drive root or system directory" + ), + ConfigError::MissingRuntime => write!( + formatter, + "an explicit --runtime is required" + ), + ConfigError::RuntimeNotAbsolute => { + write!(formatter, "--runtime must be an absolute path") + } + ConfigError::RuntimeNotPinned => write!( + formatter, + "--runtime must name the pinned t3.exe executable, not another program" + ), + ConfigError::EmptyServiceName => write!(formatter, "--service-name must not be empty"), + ConfigError::UnknownFlag(flag) => write!(formatter, "unknown argument '{flag}'"), + ConfigError::MissingValue(flag) => write!(formatter, "{flag} requires a value"), + ConfigError::InvalidNumber(flag) => write!(formatter, "{flag} requires an integer"), + ConfigError::CredentialArgument(flag) => write!( + formatter, + "refusing credential argument '{flag}': the service account is registered with SCM, not passed on the command line" + ), + ConfigError::MissingTestChild => write!( + formatter, + "test-child mode requires --exec [--exec-arg ]" + ), + } + } +} + +impl std::error::Error for ConfigError {} + +fn next_value( + args: &mut std::vec::IntoIter, + flag: &str, +) -> Result { + args.next() + .ok_or_else(|| ConfigError::MissingValue(flag.to_owned())) +} + +fn parse_duration_ms(flag: &str, value: &OsString) -> Result { + value + .to_str() + .and_then(|raw| raw.parse::().ok()) + .map(Duration::from_millis) + .ok_or_else(|| ConfigError::InvalidNumber(flag.to_owned())) +} + +fn parse_u32(flag: &str, value: &OsString) -> Result { + value + .to_str() + .and_then(|raw| raw.parse::().ok()) + .ok_or_else(|| ConfigError::InvalidNumber(flag.to_owned())) +} + +/// Absolute on either POSIX or Windows rules. The host is cross-checked from a +/// developer host, so Windows drive and UNC prefixes are recognized without +/// relying on the running platform's `Path` semantics. +pub fn is_absolute_path(path: &Path) -> bool { + if path.is_absolute() { + return true; + } + let raw = path.to_string_lossy(); + let bytes = raw.as_bytes(); + let drive_absolute = bytes.len() >= 3 + && bytes[0].is_ascii_alphabetic() + && bytes[1] == b':' + && (bytes[2] == b'\\' || bytes[2] == b'/'); + drive_absolute || raw.starts_with("\\\\") +} + +/// Split a path into lower-cased name segments on either separator, dropping +/// the drive prefix and empty segments. +fn name_segments(path: &Path) -> Vec { + path.to_string_lossy() + .split(['\\', '/']) + .filter(|segment| !segment.is_empty() && *segment != "." && !segment.ends_with(':')) + .map(str::to_ascii_lowercase) + .collect() +} + +/// Reject drive roots and well-known system directories. This is a guardrail, +/// not a security boundary: an administrator registering the service still +/// chooses the home, and the account constraint is what actually protects +/// other users' data. +pub fn is_canonical_t3_home(path: &Path) -> bool { + if !is_absolute_path(path) { + return false; + } + let raw = path.to_string_lossy(); + if raw.split(['\\', '/']).any(|segment| segment == "..") { + return false; + } + let normal = name_segments(path); + if normal.is_empty() { + return false; + } + !normal + .iter() + .any(|part| FORBIDDEN_HOME_COMPONENTS.contains(&part.as_str())) +} + +fn is_pinned_runtime(path: &Path) -> bool { + path.to_string_lossy() + .split(['\\', '/']) + .next_back() + .is_some_and(|name| name.eq_ignore_ascii_case("t3.exe")) +} + +/// Parse the process command line. `args` is the full `args_os()` iterator, +/// including argv[0]. +pub fn parse(args: impl IntoIterator) -> Result { + let mut args = args.into_iter().collect::>().into_iter(); + let _program = args.next(); + + let mut home: Option = None; + let mut runtime: Option = None; + let mut log: Option = None; + let mut service_name = String::from("t3code"); + let mut service_name_set = false; + let mut drain_timeout = DEFAULT_DRAIN_TIMEOUT; + let mut restart_window = DEFAULT_RESTART_WINDOW; + let mut max_restarts = DEFAULT_MAX_RESTARTS; + let mut expected_account: Option = None; + let mut allow_local_system = false; + let mut console = false; + let mut help = false; + let mut version = false; + #[cfg(feature = "test-child")] + let mut test_program: Option = None; + #[cfg(feature = "test-child")] + let mut test_args: Vec = Vec::new(); + + while let Some(raw) = args.next() { + let flag = raw.to_string_lossy().into_owned(); + let (name, inline) = match flag.split_once('=') { + Some((name, value)) => (name.to_owned(), Some(OsString::from(value))), + None => (flag.clone(), None), + }; + let mut take = |flag: &str| -> Result { + match inline.clone() { + Some(value) => Ok(value), + None => next_value(&mut args, flag), + } + }; + + match name.as_str() { + "--home" => home = Some(PathBuf::from(take("--home")?)), + "--runtime" => runtime = Some(PathBuf::from(take("--runtime")?)), + "--log" => log = Some(PathBuf::from(take("--log")?)), + "--service-name" => { + let value = take("--service-name")?; + service_name = value.to_string_lossy().into_owned(); + service_name_set = true; + } + "--drain-timeout-ms" => { + drain_timeout = + parse_duration_ms("--drain-timeout-ms", &take("--drain-timeout-ms")?)? + } + "--restart-window-ms" => { + restart_window = + parse_duration_ms("--restart-window-ms", &take("--restart-window-ms")?)? + } + "--max-restarts" => { + max_restarts = parse_u32("--max-restarts", &take("--max-restarts")?)? + } + "--expected-account" => { + expected_account = Some(take("--expected-account")?.to_string_lossy().into_owned()) + } + "--allow-local-system" => allow_local_system = true, + "--console" => console = true, + "--help" | "-h" => help = true, + "--version" | "-V" => version = true, + #[cfg(feature = "test-child")] + "--exec" => test_program = Some(PathBuf::from(take("--exec")?)), + #[cfg(feature = "test-child")] + "--exec-arg" => test_args.push(take("--exec-arg")?), + other => { + let lowered = other.to_ascii_lowercase(); + if lowered.contains("password") + || lowered.contains("token") + || lowered.contains("secret") + { + return Err(ConfigError::CredentialArgument(other.to_owned())); + } + return Err(ConfigError::UnknownFlag(other.to_owned())); + } + } + } + + if help { + return Ok(Invocation::Help); + } + if version { + return Ok(Invocation::Version); + } + + let home = home.ok_or(ConfigError::MissingHome)?; + if !is_absolute_path(&home) { + return Err(ConfigError::HomeNotAbsolute); + } + if !is_canonical_t3_home(&home) { + return Err(ConfigError::HomeNotCanonical); + } + if service_name_set && service_name.is_empty() { + return Err(ConfigError::EmptyServiceName); + } + + #[cfg(feature = "test-child")] + let mode = if let Some(program) = test_program { + LaunchMode::TestChild { + program, + args: test_args, + } + } else { + LaunchMode::ServiceLauncher + }; + #[cfg(not(feature = "test-child"))] + let mode = LaunchMode::ServiceLauncher; + + let runtime_path = runtime.unwrap_or_default(); + match &mode { + LaunchMode::ServiceLauncher => { + if runtime_path.as_os_str().is_empty() { + return Err(ConfigError::MissingRuntime); + } + if !is_absolute_path(&runtime_path) { + return Err(ConfigError::RuntimeNotAbsolute); + } + if !is_pinned_runtime(&runtime_path) { + return Err(ConfigError::RuntimeNotPinned); + } + } + #[cfg(feature = "test-child")] + LaunchMode::TestChild { .. } => {} + } + + let config = ServiceConfig { + home, + runtime: runtime_path, + log, + service_name, + drain_timeout, + restart_window, + max_restarts, + poll_interval: DEFAULT_POLL_INTERVAL, + expected_account, + allow_local_system, + mode, + }; + + Ok(if console { + Invocation::Console(config) + } else { + Invocation::Service(config) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn parse_str(args: &[&str]) -> Result { + parse(args.iter().map(OsString::from)) + } + + fn windows_home() -> &'static str { + r"C:\Users\t3service\.t3" + } + + fn windows_runtime() -> &'static str { + r"C:\Users\t3service\.t3\runtime\versions\0.0.42\t3.exe" + } + + #[test] + fn requires_explicit_home_and_runtime() { + assert_eq!( + parse_str(&["t3-windows-service-host"]).unwrap_err(), + ConfigError::MissingHome + ); + assert_eq!( + parse_str(&["t3-windows-service-host", "--home", windows_home()]).unwrap_err(), + ConfigError::MissingRuntime + ); + } + + #[test] + fn accepts_a_canonical_home_and_pinned_runtime() { + let invocation = parse_str(&[ + "t3-windows-service-host", + "--home", + windows_home(), + "--runtime", + windows_runtime(), + ]) + .expect("valid configuration"); + match invocation { + Invocation::Service(config) => { + assert_eq!(config.home, PathBuf::from(windows_home())); + assert_eq!(config.service_name, "t3code"); + assert_eq!(config.mode, LaunchMode::ServiceLauncher); + } + other => panic!("expected service invocation, got {other:?}"), + } + } + + #[test] + fn supports_equals_and_os_native_paths_with_spaces() { + let invocation = parse_str(&[ + "t3-windows-service-host", + r"--home=C:\Users\T3 Service\.t3", + r"--runtime=C:\Users\T3 Service\.t3\runtime\versions\0.0.42\t3.exe", + "--service-name=T3 Code", + ]) + .expect("valid configuration"); + match invocation { + Invocation::Service(config) => { + assert!(config.home.to_string_lossy().contains("T3 Service")); + assert_eq!(config.service_name, "T3 Code"); + } + other => panic!("expected service invocation, got {other:?}"), + } + } + + #[test] + fn rejects_relative_home() { + assert_eq!( + parse_str(&["host", "--home", r"relative\.t3"]).unwrap_err(), + ConfigError::HomeNotAbsolute + ); + } + + #[test] + fn rejects_drive_root_and_system_directories() { + assert_eq!( + parse_str(&["host", "--home", r"C:\"]).unwrap_err(), + ConfigError::HomeNotCanonical + ); + assert_eq!( + parse_str(&["host", "--home", r"C:\Windows"]).unwrap_err(), + ConfigError::HomeNotCanonical + ); + } + + #[test] + fn rejects_an_unpinned_runtime() { + let error = parse_str(&[ + "host", + "--home", + windows_home(), + "--runtime", + r"C:\Users\t3service\.t3\runtime\versions\0.0.42\other.exe", + ]) + .unwrap_err(); + assert_eq!(error, ConfigError::RuntimeNotPinned); + } + + #[test] + fn refuses_credential_arguments() { + let error = parse_str(&[ + "host", + "--home", + windows_home(), + "--runtime", + windows_runtime(), + "--password", + "hunter2", + ]) + .unwrap_err(); + assert!(matches!(error, ConfigError::CredentialArgument(_))); + } + + #[test] + fn refuses_unknown_flags() { + let error = parse_str(&["host", "--frobnicate"]).unwrap_err(); + assert_eq!(error, ConfigError::UnknownFlag("--frobnicate".to_owned())); + } + + #[test] + fn console_mode_keeps_the_same_configuration() { + let invocation = parse_str(&[ + "host", + "--console", + "--home", + windows_home(), + "--runtime", + windows_runtime(), + ]) + .expect("valid configuration"); + assert!(matches!(invocation, Invocation::Console(_))); + } + + #[test] + fn stop_marker_matches_the_launcher_protocol() { + let config = ServiceConfig { + home: PathBuf::from(windows_home()), + runtime: PathBuf::from(windows_runtime()), + log: None, + service_name: "t3code".to_owned(), + drain_timeout: DEFAULT_DRAIN_TIMEOUT, + restart_window: DEFAULT_RESTART_WINDOW, + max_restarts: DEFAULT_MAX_RESTARTS, + poll_interval: DEFAULT_POLL_INTERVAL, + expected_account: None, + allow_local_system: false, + mode: LaunchMode::ServiceLauncher, + }; + assert!(config.stop_marker().ends_with("runtime/.service-stopping")); + } +} diff --git a/native/windows-service-host/src/control.rs b/native/windows-service-host/src/control.rs new file mode 100644 index 000000000000..e22af17beef8 --- /dev/null +++ b/native/windows-service-host/src/control.rs @@ -0,0 +1,142 @@ +//! Portable mapping from SCM control events to supervisor intent. +//! +//! The Windows control handler runs on an SCM-owned thread and must return +//! promptly. It therefore never performs the stop itself; it records the +//! intent and wakes the supervisor thread. These functions are the pure part +//! of that mapping so they can be tested without Windows. + +/// States reported through `SetServiceStatus`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ServiceState { + StartPending, + Running, + StopPending, + Stopped, +} + +impl ServiceState { + /// SCM `dwCurrentState` value. + pub fn win32_state(self) -> u32 { + match self { + ServiceState::StartPending => 2, + ServiceState::Running => 4, + ServiceState::StopPending => 3, + ServiceState::Stopped => 1, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Control { + Stop, + Shutdown, + Interrogate, + Other, +} + +impl Control { + /// Map a raw `SERVICE_CONTROL_*` value. Unknown values stay `Other` rather + /// than being treated as a stop. + pub fn from_win32(code: u32) -> Control { + match code { + 1 => Control::Stop, + 5 => Control::Shutdown, + 4 => Control::Interrogate, + _ => Control::Other, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ControlOutcome { + /// Begin a planned stop. + StopRequested, + /// A stop is already in flight; do not start a second one. + AlreadyStopping, + /// Report the current status without changing it. + Interrogated, + /// Ignore a control this service does not act on. + Ignored, +} + +/// Exactly the session-controls this service acts on. Interrogate has no accept +/// bit; SCM always allows it. +pub fn accepts_control(control: Control) -> bool { + matches!(control, Control::Stop | Control::Shutdown) +} + +pub fn handle_control(state: ServiceState, control: Control) -> ControlOutcome { + match control { + Control::Stop | Control::Shutdown => match state { + ServiceState::Stopped | ServiceState::StopPending => ControlOutcome::AlreadyStopping, + ServiceState::StartPending | ServiceState::Running => ControlOutcome::StopRequested, + }, + Control::Interrogate => ControlOutcome::Interrogated, + Control::Other => ControlOutcome::Ignored, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn maps_raw_control_codes() { + assert_eq!(Control::from_win32(1), Control::Stop); + assert_eq!(Control::from_win32(5), Control::Shutdown); + assert_eq!(Control::from_win32(4), Control::Interrogate); + assert_eq!(Control::from_win32(255), Control::Other); + } + + #[test] + fn advertising_stop_and_shutdown_but_not_unknown_controls() { + assert!(accepts_control(Control::Stop)); + assert!(accepts_control(Control::Shutdown)); + assert!(!accepts_control(Control::Other)); + assert!(!accepts_control(Control::Interrogate)); + } + + #[test] + fn a_running_service_requests_a_stop_once() { + assert_eq!( + handle_control(ServiceState::Running, Control::Stop), + ControlOutcome::StopRequested + ); + assert_eq!( + handle_control(ServiceState::StopPending, Control::Stop), + ControlOutcome::AlreadyStopping + ); + assert_eq!( + handle_control(ServiceState::Stopped, Control::Shutdown), + ControlOutcome::AlreadyStopping + ); + } + + #[test] + fn shutdown_is_treated_like_stop() { + assert_eq!( + handle_control(ServiceState::Running, Control::Shutdown), + ControlOutcome::StopRequested + ); + } + + #[test] + fn interrogate_never_stops_the_service() { + assert_eq!( + handle_control(ServiceState::Running, Control::Interrogate), + ControlOutcome::Interrogated + ); + assert_eq!( + handle_control(ServiceState::StopPending, Control::Interrogate), + ControlOutcome::Interrogated + ); + } + + #[test] + fn unknown_controls_are_ignored() { + assert_eq!( + handle_control(ServiceState::Running, Control::Other), + ControlOutcome::Ignored + ); + } +} diff --git a/native/windows-service-host/src/host.rs b/native/windows-service-host/src/host.rs new file mode 100644 index 000000000000..ae0dfc67a603 --- /dev/null +++ b/native/windows-service-host/src/host.rs @@ -0,0 +1,202 @@ +//! Child process ownership. +//! +//! The Windows service uses a job object so the whole tree the launcher starts +//! is owned and can be terminated together. The portable host exists so the +//! same supervisor can be exercised on a developer host and under `--console`; +//! it does not claim the Windows tree guarantees. + +use std::fs::OpenOptions; +use std::path::PathBuf; +use std::process::{Child, Command, Stdio}; + +use crate::config::{LaunchMode, ServiceConfig}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ProcessIdentity { + pub pid: u32, + /// Optional process creation time in milliseconds. The Windows host uses it + /// to tell its own child from a foreign process that reused the PID; the + /// portable host leaves it zero. + pub created_at_ms: u64, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SpawnError { + /// The configuration is wrong; retrying cannot help. + Config(String), + /// A transient launch failure. + Launch(String), +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct QueryError; + +pub trait ChildHandle { + fn identity(&self) -> ProcessIdentity; + /// `Ok(true)` means the recorded identity is still ours, `Ok(false)` means + /// the PID belongs to another process, and `Err` means the query failed. + fn verify_identity(&mut self) -> Result; + fn try_wait(&mut self) -> Result, QueryError>; + /// Ask the launcher to stop its server child. The portable host writes the + /// stop marker the launcher reads; the production launcher adaptation + /// (documented in the scoped design) is what makes that marker actionable. + fn request_graceful_stop(&mut self) -> Result<(), QueryError>; + fn terminate_tree(&mut self); +} + +pub trait ChildHost { + type Child: ChildHandle; + fn spawn(&mut self, config: &ServiceConfig) -> Result; +} + +/// Portable host built on `std::process::Command`. +#[derive(Debug)] +pub struct CommandChildHost { + stop_marker: PathBuf, +} + +impl CommandChildHost { + pub fn new(config: &ServiceConfig) -> Self { + Self { + stop_marker: config.stop_marker(), + } + } +} + +pub struct CommandChild { + child: Child, + id: ProcessIdentity, + stop_marker: PathBuf, +} + +impl ChildHost for CommandChildHost { + type Child = CommandChild; + + fn spawn(&mut self, config: &ServiceConfig) -> Result { + if matches!(config.mode, LaunchMode::ServiceLauncher) + && !std::path::Path::new(&config.home).is_dir() + { + return Err(SpawnError::Config(format!( + "T3 home {} does not exist", + config.home.display() + ))); + } + let (program, args) = config.child_command(); + let mut command = Command::new(&program); + command + .args(&args) + .env("T3CODE_HOME", &config.home) + .stdin(Stdio::null()); + if config.home.is_dir() { + command.current_dir(&config.home); + } + if let Some(log_path) = &config.log { + if let Some(parent) = log_path.parent() { + std::fs::create_dir_all(parent).map_err(|error| { + SpawnError::Config(format!("cannot create log directory: {error}")) + })?; + } + let stdout = OpenOptions::new() + .create(true) + .append(true) + .open(log_path) + .map_err(|error| SpawnError::Config(format!("cannot open log file: {error}")))?; + let stderr = stdout + .try_clone() + .map_err(|error| SpawnError::Config(format!("cannot clone log handle: {error}")))?; + command + .stdout(Stdio::from(stdout)) + .stderr(Stdio::from(stderr)); + } + let child = command.spawn().map_err(|error| { + if error.kind() == std::io::ErrorKind::NotFound { + SpawnError::Config(format!("child program not found: {}", program.display())) + } else { + SpawnError::Launch(format!("could not start child: {error}")) + } + })?; + let id = ProcessIdentity { + pid: child.id(), + created_at_ms: 0, + }; + Ok(CommandChild { + child, + id, + stop_marker: self.stop_marker.clone(), + }) + } +} + +impl ChildHandle for CommandChild { + fn identity(&self) -> ProcessIdentity { + self.id + } + + fn verify_identity(&mut self) -> Result { + // The OS handle is authoritative while it is retained; PID reuse alone + // cannot redirect it. Creation-time comparison is Windows-only. + match self.child.try_wait() { + Ok(None) => Ok(true), + Ok(Some(_)) => Ok(false), + Err(_) => Err(QueryError), + } + } + + fn try_wait(&mut self) -> Result, QueryError> { + match self.child.try_wait() { + Ok(None) => Ok(None), + Ok(Some(status)) => Ok(Some(status.code().unwrap_or(-1))), + Err(_) => Err(QueryError), + } + } + + fn request_graceful_stop(&mut self) -> Result<(), QueryError> { + if let Some(parent) = self.stop_marker.parent() { + std::fs::create_dir_all(parent).map_err(|_| QueryError)?; + } + std::fs::write(&self.stop_marker, b"").map_err(|_| QueryError) + } + + fn terminate_tree(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::{DEFAULT_DRAIN_TIMEOUT, LaunchMode, ServiceConfig}; + use std::time::Duration; + + fn config() -> ServiceConfig { + ServiceConfig { + home: PathBuf::from("."), + runtime: PathBuf::new(), + log: None, + service_name: "t3code".to_owned(), + drain_timeout: DEFAULT_DRAIN_TIMEOUT, + restart_window: Duration::from_secs(300), + max_restarts: 3, + poll_interval: Duration::from_millis(50), + expected_account: None, + allow_local_system: false, + mode: LaunchMode::ServiceLauncher, + } + } + + #[test] + fn rejects_a_missing_home_for_the_service_launcher() { + let mut host = CommandChildHost { + stop_marker: PathBuf::from(".t3-test-stop-marker"), + }; + let scoped = ServiceConfig { + home: PathBuf::from("does-not-exist-t3-home"), + ..config() + }; + assert!(matches!( + host.spawn(&scoped), + Err(SpawnError::Config(message)) if message.contains("does not exist") + )); + } +} diff --git a/native/windows-service-host/src/lib.rs b/native/windows-service-host/src/lib.rs new file mode 100644 index 000000000000..fcdd75a9e5b2 --- /dev/null +++ b/native/windows-service-host/src/lib.rs @@ -0,0 +1,24 @@ +//! Minimal T3-owned Windows SCM host. +//! +//! This crate is a prototype of the SCM entry point, control handler and +//! process-tree ownership that a real Windows background service needs. The +//! production child is the pinned `t3.exe __service-launcher` subcommand; this +//! host never re-implements the launcher's update or rollback logic. +//! +//! The SCM dispatch loop and the job-object process tree are Windows-only +//! (`windows`). The configuration parser, control mapping, restart budget and +//! stop/drain state machine are portable so they can be unit tested on any +//! developer host that has no Windows toolchain. + +pub mod config; +pub mod control; +pub mod host; +pub mod run; +pub mod supervise; + +#[cfg(windows)] +pub mod windows; + +pub use config::{Invocation, LaunchMode, ServiceConfig}; +pub use control::{Control, ControlOutcome, ServiceState}; +pub use supervise::{ExitCode, IdentityVerdict, Supervisor, SupervisorAction}; diff --git a/native/windows-service-host/src/main.rs b/native/windows-service-host/src/main.rs new file mode 100644 index 000000000000..10ae492cc17f --- /dev/null +++ b/native/windows-service-host/src/main.rs @@ -0,0 +1,146 @@ +//! Entry point. +//! +//! The command line is parsed here, before any SCM call, because the service +//! arguments arrive through the process command line and not through +//! `ServiceMain`. `--console` runs the same supervisor against a terminal for +//! development; it is never proof that SCM integration works. + +use std::io::BufRead; +use std::process::ExitCode as StdExitCode; +use std::sync::mpsc; +use std::thread; + +use t3_windows_service_host::config::{Invocation, ServiceConfig, parse}; +use t3_windows_service_host::control::{Control, ServiceState}; +use t3_windows_service_host::host::CommandChildHost; +use t3_windows_service_host::run::{ChannelControlInput, Reporter, run}; +use t3_windows_service_host::supervise::ExitCode; + +fn main() -> StdExitCode { + match parse(std::env::args_os()) { + Ok(Invocation::Help) => { + print_help(); + StdExitCode::SUCCESS + } + Ok(Invocation::Version) => { + println!("t3-windows-service-host {}", env!("CARGO_PKG_VERSION")); + StdExitCode::SUCCESS + } + Ok(Invocation::Console(config)) => exit_code(run_console(&config)), + Ok(Invocation::Service(config)) => exit_code(run_service(config)), + Err(error) => { + eprintln!("t3-windows-service-host: {error}"); + StdExitCode::from(2) + } + } +} + +fn exit_code(result: ExitCode) -> StdExitCode { + match result { + ExitCode::Clean => StdExitCode::SUCCESS, + _ => StdExitCode::from(1), + } +} + +#[cfg(windows)] +fn run_service(config: ServiceConfig) -> ExitCode { + match t3_windows_service_host::windows::run_service(config) { + Ok(exit) => exit, + Err(reason) => { + eprintln!("t3-windows-service-host: {reason}"); + ExitCode::LaunchFailure + } + } +} + +#[cfg(not(windows))] +fn run_service(_config: ServiceConfig) -> ExitCode { + eprintln!( + "t3-windows-service-host: SCM dispatch is only available on Windows; pass --console for the portable supervisor" + ); + ExitCode::LaunchFailure +} + +struct ConsoleReporter; + +impl Reporter for ConsoleReporter { + fn report(&mut self, state: ServiceState, exit: ExitCode, checkpoint: u32) { + println!("[t3-service] state={state:?} exit={exit:?} checkpoint={checkpoint}"); + } +} + +fn run_console(config: &ServiceConfig) -> ExitCode { + let (sender, receiver) = mpsc::channel::(); + let reader = thread::spawn(move || { + let stdin = std::io::stdin(); + for line in stdin.lock().lines() { + let command = match line { + Ok(line) => line.trim().to_ascii_lowercase(), + Err(_) => break, + }; + let control = match command.as_str() { + "stop" | "shutdown" | "quit" | "exit" => Control::Stop, + "interrogate" | "status" => Control::Interrogate, + "" => continue, + _ => { + eprintln!("[console] unknown command '{command}'; use stop or status"); + continue; + } + }; + if sender.send(control).is_err() { + return; + } + } + // EOF (for example a closed pipe) is treated as a stop request. + let _ = sender.send(Control::Stop); + }); + + let mut host = CommandChildHost::new(config); + let mut controls = ChannelControlInput { receiver }; + let mut reporter = ConsoleReporter; + let mut log = |message: &str| eprintln!("[t3-service] {message}"); + let outcome = run(config, &mut host, &mut controls, &mut reporter, &mut log); + drop(controls); + let _ = reader.join(); + println!( + "[t3-service] exited exit={:?} forced={} observed={}", + outcome.exit, outcome.forced, outcome.exit_observed + ); + outcome.exit +} + +fn print_help() { + println!( + "\ +t3-windows-service-host {version} + +A minimal T3-owned Windows SCM host. It runs the pinned `t3.exe +__service-launcher` under a job object and reports service state to the service +control manager. Not a general-purpose service wrapper. + +USAGE: + t3-windows-service-host --home --runtime [options] + t3-windows-service-host --console --home --runtime [options] + +REQUIRED: + --home Canonical T3 home. Exported to the child as + T3CODE_HOME. There is no default. + --runtime Pinned runtime executable. The host appends + `__service-launcher`; it never manages updates. + +OPTIONS: + --log Redirect child stdout/stderr. Defaults to inherit. + --service-name SCM registration name (default: t3code). + --expected-account Refuse to run as any other account. + --allow-local-system Allow LocalSystem (discouraged; off by default). + --drain-timeout-ms Bounded wait after a stop request (default: 30000). + --restart-window-ms Restart budget window (default: 300000). + --max-restarts Restarts allowed inside the window (default: 5). + --console Run against a terminal, not SCM (development only). + --help, --version + +The service account and its password are registered with SCM (`sc.exe create +... obj= ...`), never passed here.", + version = env!("CARGO_PKG_VERSION") + ); +} diff --git a/native/windows-service-host/src/run.rs b/native/windows-service-host/src/run.rs new file mode 100644 index 000000000000..70d0e09ca7b4 --- /dev/null +++ b/native/windows-service-host/src/run.rs @@ -0,0 +1,353 @@ +//! The supervision event loop. +//! +//! This loop is shared by `--console`, the portable integration test and the +//! Windows SCM service. It executes `SupervisorAction`s, but every process +//! operation goes through the `ChildHost`, and every stop or termination first +//! verifies that the recorded process identity is still the one this host +//! spawned. A failed query leaves ownership unknown and is never read as a +//! successful stop. + +use std::sync::mpsc; +use std::time::{Duration, Instant}; + +use crate::config::ServiceConfig; +use crate::control::{Control, ServiceState}; +use crate::host::{ChildHandle, ChildHost, SpawnError}; +use crate::supervise::{ExitCode, IdentityVerdict, Supervisor, SupervisorAction, identity_verdict}; + +pub trait ControlInput { + /// Wait up to `timeout` for the next control. `None` means it timed out. + fn wait(&mut self, timeout: Duration) -> Option; +} + +/// Channel-backed control input. The Windows control handler and the console +/// reader both send into this channel from their own thread. +pub struct ChannelControlInput { + pub receiver: mpsc::Receiver, +} + +impl ControlInput for ChannelControlInput { + fn wait(&mut self, timeout: Duration) -> Option { + self.receiver.recv_timeout(timeout).ok() + } +} + +/// Deterministic control input for tests: one entry per loop iteration, where +/// `None` yields a timeout and lets the loop poll the child. +#[derive(Debug, Default)] +pub struct ScriptedControl { + pub script: std::collections::VecDeque>, +} + +impl ControlInput for ScriptedControl { + fn wait(&mut self, _timeout: Duration) -> Option { + self.script.pop_front().flatten() + } +} + +pub trait Reporter { + fn report(&mut self, state: ServiceState, exit: ExitCode, checkpoint: u32); +} + +pub struct RunOutcome { + pub exit: ExitCode, + pub forced: bool, + /// True when the exit came from observing the child, not from a forced + /// termination. Useful to keep "stop completed" honest in logs. + pub exit_observed: bool, +} + +pub fn run( + config: &ServiceConfig, + host: &mut H, + controls: &mut C, + reporter: &mut R, + log: &mut dyn FnMut(&str), +) -> RunOutcome +where + H: ChildHost, + C: ControlInput, + R: Reporter, +{ + let start = Instant::now(); + let now = move || start.elapsed(); + let mut supervisor = Supervisor::new(config.clone()); + let mut child: Option = None; + let mut exit_observed = false; + + let mut queue = supervisor.begin(now()); + loop { + while !queue.is_empty() { + let actions = std::mem::take(&mut queue); + queue = execute( + actions, + config, + &mut supervisor, + host, + &mut child, + reporter, + now(), + log, + ); + } + if supervisor.finished() { + break; + } + + if let Some(control) = controls.wait(config.poll_interval) { + queue = supervisor.on_control(now(), control); + continue; + } + + if let Some(handle) = child.as_mut() { + match handle.try_wait() { + Ok(Some(code)) => { + child = None; + exit_observed = true; + queue = supervisor.on_child_exited(now(), code); + continue; + } + Ok(None) => {} + Err(_) => log("child query failed; status remains unknown, not stopped"), + } + } + + queue = supervisor.tick(now()); + } + + RunOutcome { + exit: supervisor.exit_code(), + forced: supervisor.forced(), + exit_observed, + } +} + +#[allow(clippy::too_many_arguments)] +fn execute( + actions: Vec, + config: &ServiceConfig, + supervisor: &mut Supervisor, + host: &mut H, + child: &mut Option, + reporter: &mut R, + now: Duration, + log: &mut dyn FnMut(&str), +) -> Vec { + let mut follow_up = Vec::new(); + for action in actions { + match action { + SupervisorAction::SpawnChild => match host.spawn(config) { + Ok(spawned) => { + let identity = spawned.identity(); + *child = Some(spawned); + supervisor.on_child_spawned(identity); + } + Err(SpawnError::Config(message)) => { + log(&format!("refusing to launch: {message}")); + follow_up.extend(supervisor.on_spawn_failed(now, true)); + } + Err(SpawnError::Launch(message)) => { + log(&format!("launch failed: {message}")); + follow_up.extend(supervisor.on_spawn_failed(now, false)); + } + }, + SupervisorAction::RequestGracefulStop => match child.as_mut() { + Some(handle) => match identity_verdict(handle.verify_identity()) { + IdentityVerdict::Owned => { + if handle.request_graceful_stop().is_err() { + log("stop request could not be delivered; bounded termination remains"); + } + } + IdentityVerdict::Foreign => { + log("refusing to stop: recorded PID now belongs to another process") + } + IdentityVerdict::Unknown => { + log("process identity unknown; not assuming the child stopped") + } + }, + None => follow_up.extend(supervisor.on_forced_termination()), + }, + SupervisorAction::ForceTerminateTree => { + if let Some(handle) = child.as_mut() { + match identity_verdict(handle.verify_identity()) { + IdentityVerdict::Owned => handle.terminate_tree(), + _ => log("refusing to terminate an unverified process tree"), + } + } + *child = None; + follow_up.extend(supervisor.on_forced_termination()); + } + SupervisorAction::Report { state, exit } => { + reporter.report(state, exit, supervisor.checkpoint()); + } + } + } + follow_up +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::{LaunchMode, ServiceConfig}; + use crate::host::{ChildHandle, ProcessIdentity, QueryError, SpawnError}; + use std::collections::VecDeque; + use std::path::PathBuf; + use std::sync::{Arc, Mutex}; + + type Events = Arc>>; + + #[derive(Clone)] + struct FakeChild { + alive: bool, + owned: bool, + query_fails: bool, + events: Events, + } + + impl ChildHandle for FakeChild { + fn identity(&self) -> ProcessIdentity { + ProcessIdentity { + pid: 7, + created_at_ms: 1, + } + } + fn verify_identity(&mut self) -> Result { + if self.query_fails { + Err(QueryError) + } else { + Ok(self.owned) + } + } + fn try_wait(&mut self) -> Result, QueryError> { + if self.query_fails { + Err(QueryError) + } else if self.alive { + Ok(None) + } else { + Ok(Some(0)) + } + } + fn request_graceful_stop(&mut self) -> Result<(), QueryError> { + self.events.lock().unwrap().push("graceful".to_owned()); + Ok(()) + } + fn terminate_tree(&mut self) { + self.events.lock().unwrap().push("terminate".to_owned()); + self.alive = false; + } + } + + struct FakeHost { + child: FakeChild, + } + + impl ChildHost for FakeHost { + type Child = FakeChild; + fn spawn(&mut self, _config: &ServiceConfig) -> Result { + Ok(self.child.clone()) + } + } + + struct Recorder { + events: Events, + } + + impl Reporter for Recorder { + fn report(&mut self, state: ServiceState, exit: ExitCode, _checkpoint: u32) { + self.events + .lock() + .unwrap() + .push(format!("report:{state:?}:{exit:?}")); + } + } + + fn config() -> ServiceConfig { + ServiceConfig { + home: PathBuf::from("."), + runtime: PathBuf::new(), + log: None, + service_name: "t3code".to_owned(), + drain_timeout: Duration::from_millis(20), + restart_window: Duration::from_secs(300), + max_restarts: 3, + poll_interval: Duration::from_millis(1), + expected_account: None, + allow_local_system: false, + mode: LaunchMode::ServiceLauncher, + } + } + + fn contains(events: &Events, needle: &str) -> bool { + events + .lock() + .unwrap() + .iter() + .any(|event| event.contains(needle)) + } + + fn run_with(child: FakeChild) -> (Events, RunOutcome) { + let events: Events = Arc::new(Mutex::new(Vec::new())); + let child = FakeChild { + events: events.clone(), + ..child + }; + let mut host = FakeHost { child }; + let mut controls = ScriptedControl { + script: VecDeque::from([Some(Control::Stop)]), + }; + let mut recorder = Recorder { + events: events.clone(), + }; + let mut log = |_message: &str| {}; + let outcome = run(&config(), &mut host, &mut controls, &mut recorder, &mut log); + (events, outcome) + } + + #[test] + fn foreign_pid_is_reported_stopped_but_never_terminated() { + let (events, _outcome) = run_with(FakeChild { + alive: true, + owned: false, + query_fails: false, + events: Arc::new(Mutex::new(Vec::new())), + }); + assert!(contains(&events, "report:Stopped"), "service must not hang"); + assert!( + !contains(&events, "terminate"), + "a foreign process must not be terminated" + ); + assert!( + !contains(&events, "graceful"), + "a foreign process must not receive a stop request" + ); + } + + #[test] + fn query_failure_is_not_treated_as_an_owned_tree() { + let (events, _outcome) = run_with(FakeChild { + alive: true, + owned: true, + query_fails: true, + events: Arc::new(Mutex::new(Vec::new())), + }); + assert!(contains(&events, "report:Stopped"), "service must not hang"); + assert!( + !contains(&events, "terminate"), + "an unverifiable tree must not be terminated" + ); + } + + #[test] + fn owned_tree_receives_graceful_stop_then_bounded_termination() { + let (events, outcome) = run_with(FakeChild { + alive: true, + owned: true, + query_fails: false, + events: Arc::new(Mutex::new(Vec::new())), + }); + assert!(contains(&events, "graceful")); + assert!(contains(&events, "terminate")); + assert!(contains(&events, "report:Stopped")); + assert!(outcome.forced); + } +} diff --git a/native/windows-service-host/src/supervise.rs b/native/windows-service-host/src/supervise.rs new file mode 100644 index 000000000000..a86599010428 --- /dev/null +++ b/native/windows-service-host/src/supervise.rs @@ -0,0 +1,498 @@ +//! Portable supervision state machine. +//! +//! The Windows layer owns the SCM status handle and the job object. This module +//! owns the decisions: when to spawn, when an exit is unexpected, when the +//! restart budget is exhausted, and when a stop has drained or must be forced. +//! Keeping it free of OS calls makes the focused cases deterministic. +//! +//! Two rules are encoded here: +//! +//! - A stop request changes the state to `StopPending` and asks the launcher to +//! stop, but the process tree is only force-terminated after the drain +//! deadline. A query failure is never treated as proof that the child +//! stopped; the runner verifies identity before it acts. +//! - The restart budget is bounded. After `max_restarts` restarts inside +//! `restart_window`, the next unexpected exit stops the service instead of +//! respawning forever. + +use std::collections::VecDeque; +use std::time::Duration; + +use crate::config::ServiceConfig; +use crate::control::{Control, ControlOutcome, ServiceState, handle_control}; +use crate::host::{ProcessIdentity, QueryError}; + +/// Monotonic time since the host started. +pub type Monotonic = Duration; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ExitCode { + /// A planned stop completed. + Clean, + /// The child exited on its own with this code. + Child(i32), + /// The restart budget was exhausted. + RepeatedFailure, + /// The child could not be spawned at all. + LaunchFailure, + /// The cause could not be established. + Unknown, +} + +impl ExitCode { + /// `(dwWin32ExitCode, dwServiceSpecificExitCode)` for `SetServiceStatus`. + pub fn win32(self) -> (u32, u32) { + // ERROR_SERVICE_SPECIFIC_ERROR tells SCM to read the specific code. + const ERROR_SERVICE_SPECIFIC_ERROR: u32 = 1066; + match self { + ExitCode::Clean => (0, 0), + ExitCode::Child(_) => (ERROR_SERVICE_SPECIFIC_ERROR, 1), + ExitCode::RepeatedFailure => (ERROR_SERVICE_SPECIFIC_ERROR, 2), + ExitCode::LaunchFailure => (ERROR_SERVICE_SPECIFIC_ERROR, 3), + ExitCode::Unknown => (ERROR_SERVICE_SPECIFIC_ERROR, 4), + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum IdentityVerdict { + /// The recorded identity is still the process this host spawned. + Owned, + /// The PID now belongs to another process. + Foreign, + /// The query failed; ownership is unknown. + Unknown, +} + +/// Map a `verify_identity` result. `Err` means the query failed and must not be +/// read as either ownership or a successful stop. +pub fn identity_verdict(result: Result) -> IdentityVerdict { + match result { + Ok(true) => IdentityVerdict::Owned, + Ok(false) => IdentityVerdict::Foreign, + Err(QueryError) => IdentityVerdict::Unknown, + } +} + +/// Only a verified-owned process tree may be terminated. This is what keeps the +/// host from cleaning up a foreign process that reused a stale PID. +pub fn may_cleanup(verdict: IdentityVerdict) -> bool { + matches!(verdict, IdentityVerdict::Owned) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SupervisorAction { + SpawnChild, + Report { state: ServiceState, exit: ExitCode }, + RequestGracefulStop, + ForceTerminateTree, +} + +#[derive(Debug)] +pub struct Supervisor { + config: ServiceConfig, + state: ServiceState, + identity: Option, + restart_times: VecDeque, + stop_requested_at: Option, + terminate_issued: bool, + checkpoint: u32, + last_exit: ExitCode, + finished: bool, + forced: bool, +} + +impl Supervisor { + pub fn new(config: ServiceConfig) -> Self { + Self { + config, + state: ServiceState::Stopped, + identity: None, + restart_times: VecDeque::new(), + stop_requested_at: None, + terminate_issued: false, + checkpoint: 0, + last_exit: ExitCode::Clean, + finished: false, + forced: false, + } + } + + pub fn state(&self) -> ServiceState { + self.state + } + + pub fn exit_code(&self) -> ExitCode { + self.last_exit + } + + pub fn checkpoint(&self) -> u32 { + self.checkpoint + } + + pub fn finished(&self) -> bool { + self.finished + } + + pub fn forced(&self) -> bool { + self.forced + } + + pub fn child_identity(&self) -> Option { + self.identity + } + + /// Begin service startup: report pending, then ask the runner to spawn. + pub fn begin(&mut self, _now: Monotonic) -> Vec { + self.state = ServiceState::StartPending; + vec![ + SupervisorAction::Report { + state: ServiceState::StartPending, + exit: ExitCode::Clean, + }, + SupervisorAction::SpawnChild, + ] + } + + pub fn on_child_spawned(&mut self, identity: ProcessIdentity) { + self.identity = Some(identity); + self.state = ServiceState::Running; + } + + /// The runner could not spawn the child. `fatal` marks a configuration + /// error that a retry cannot fix. + pub fn on_spawn_failed(&mut self, now: Monotonic, fatal: bool) -> Vec { + self.identity = None; + if fatal || !self.try_restart(now) { + let code = if fatal { + ExitCode::LaunchFailure + } else { + ExitCode::RepeatedFailure + }; + return self.finish(code); + } + vec![ + SupervisorAction::Report { + state: ServiceState::StartPending, + exit: ExitCode::Clean, + }, + SupervisorAction::SpawnChild, + ] + } + + /// The child exited. A `StopPending` exit is the planned stop completing; + /// anything else is unexpected and consumes the restart budget. + pub fn on_child_exited(&mut self, now: Monotonic, _code: i32) -> Vec { + self.identity = None; + if matches!(self.state, ServiceState::StopPending) { + return self.finish(ExitCode::Clean); + } + if self.try_restart(now) { + self.state = ServiceState::StartPending; + return vec![ + SupervisorAction::Report { + state: ServiceState::StartPending, + exit: ExitCode::Clean, + }, + SupervisorAction::SpawnChild, + ]; + } + self.finish(ExitCode::RepeatedFailure) + } + + /// Feed an SCM control. Stop and shutdown are the only controls that + /// change state, and they are idempotent. + pub fn on_control(&mut self, now: Monotonic, control: Control) -> Vec { + match handle_control(self.state, control) { + ControlOutcome::StopRequested => { + self.state = ServiceState::StopPending; + self.stop_requested_at = Some(now); + vec![ + SupervisorAction::Report { + state: ServiceState::StopPending, + exit: ExitCode::Clean, + }, + SupervisorAction::RequestGracefulStop, + ] + } + ControlOutcome::AlreadyStopping + | ControlOutcome::Interrogated + | ControlOutcome::Ignored => Vec::new(), + } + } + + /// Periodic heartbeat. While stopping, refresh the SCM checkpoint until the + /// drain deadline, then force the tree down exactly once. + pub fn tick(&mut self, now: Monotonic) -> Vec { + if !matches!(self.state, ServiceState::StopPending) { + return Vec::new(); + } + if self.identity.is_some() + && !self.terminate_issued + && self + .stop_requested_at + .is_some_and(|since| now.saturating_sub(since) >= self.config.drain_timeout) + { + self.terminate_issued = true; + self.forced = true; + return vec![SupervisorAction::ForceTerminateTree]; + } + self.checkpoint = self.checkpoint.wrapping_add(1); + vec![SupervisorAction::Report { + state: ServiceState::StopPending, + exit: ExitCode::Clean, + }] + } + + /// After a forced termination the runner has no exit event to feed. Finish + /// the stop directly. + pub fn on_forced_termination(&mut self) -> Vec { + self.identity = None; + self.finish(ExitCode::Clean) + } + + fn try_restart(&mut self, now: Monotonic) -> bool { + self.restart_times + .retain(|started| now.saturating_sub(*started) < self.config.restart_window); + if self.restart_times.len() as u32 >= self.config.max_restarts { + return false; + } + self.restart_times.push_back(now); + true + } + + fn finish(&mut self, code: ExitCode) -> Vec { + self.state = ServiceState::Stopped; + self.last_exit = code; + self.identity = None; + self.finished = true; + vec![SupervisorAction::Report { + state: ServiceState::Stopped, + exit: code, + }] + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::{DEFAULT_DRAIN_TIMEOUT, LaunchMode, ServiceConfig}; + use std::path::PathBuf; + + fn config() -> ServiceConfig { + ServiceConfig { + home: PathBuf::from(r"C:\Users\t3service\.t3"), + runtime: PathBuf::from(r"C:\Users\t3service\.t3\runtime\versions\0.0.42\t3.exe"), + log: None, + service_name: "t3code".to_owned(), + drain_timeout: Duration::from_secs(30), + restart_window: Duration::from_secs(300), + max_restarts: 3, + poll_interval: Duration::from_millis(200), + expected_account: None, + allow_local_system: false, + mode: LaunchMode::ServiceLauncher, + } + } + + fn identity(pid: u32) -> ProcessIdentity { + ProcessIdentity { + pid, + created_at_ms: 1_000, + } + } + + fn ms(value: u64) -> Monotonic { + Duration::from_millis(value) + } + + fn spawn_count(actions: &[SupervisorAction]) -> usize { + actions + .iter() + .filter(|action| matches!(action, SupervisorAction::SpawnChild)) + .count() + } + + fn reported(actions: &[SupervisorAction], state: ServiceState) -> bool { + actions.iter().any(|action| { + matches!( + action, + SupervisorAction::Report { state: found, .. } if *found == state + ) + }) + } + + #[test] + fn start_reports_pending_then_runs() { + let mut supervisor = Supervisor::new(config()); + let actions = supervisor.begin(ms(0)); + assert!(reported(&actions, ServiceState::StartPending)); + assert_eq!(spawn_count(&actions), 1); + + supervisor.on_child_spawned(identity(42)); + assert_eq!(supervisor.state(), ServiceState::Running); + assert!(supervisor.child_identity().is_some()); + } + + #[test] + fn planned_stop_drains_to_stopped() { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + supervisor.on_child_spawned(identity(42)); + + let actions = supervisor.on_control(ms(100), Control::Stop); + assert!(reported(&actions, ServiceState::StopPending)); + assert!(actions.contains(&SupervisorAction::RequestGracefulStop)); + + // The child exits on its own inside the drain window. + let actions = supervisor.on_child_exited(ms(200), 0); + assert!(reported(&actions, ServiceState::Stopped)); + assert!(supervisor.finished()); + assert_eq!(supervisor.exit_code(), ExitCode::Clean); + } + + #[test] + fn repeated_stop_is_ignored() { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + supervisor.on_child_spawned(identity(42)); + supervisor.on_control(ms(100), Control::Stop); + let actions = supervisor.on_control(ms(110), Control::Stop); + assert!(actions.is_empty()); + } + + #[test] + fn planned_stop_is_not_reported_as_a_failure() { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + supervisor.on_child_spawned(identity(42)); + supervisor.on_control(ms(100), Control::Stop); + supervisor.on_child_exited(ms(200), 1); + assert_eq!(supervisor.exit_code(), ExitCode::Clean); + } + + #[test] + fn unexpected_exit_restarts_within_budget() { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + supervisor.on_child_spawned(identity(42)); + let actions = supervisor.on_child_exited(ms(1_000), 7); + assert_eq!(spawn_count(&actions), 1); + assert!(reported(&actions, ServiceState::StartPending)); + assert!(!supervisor.finished()); + } + + #[test] + fn repeated_failure_exhausts_the_budget() { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + supervisor.on_child_spawned(identity(42)); + // budget is 3 restarts inside the window + for index in 0..3 { + supervisor.on_child_spawned(identity(42)); + let actions = supervisor.on_child_exited(ms(1_000 + index), 7); + assert_eq!(spawn_count(&actions), 1); + assert!(!supervisor.finished()); + } + supervisor.on_child_spawned(identity(42)); + let actions = supervisor.on_child_exited(ms(2_000), 7); + assert!(reported(&actions, ServiceState::Stopped)); + assert!(supervisor.finished()); + assert_eq!(supervisor.exit_code(), ExitCode::RepeatedFailure); + } + + #[test] + fn the_budget_window_forgets_old_restarts() { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + for index in 0..3 { + supervisor.on_child_spawned(identity(42)); + supervisor.on_child_exited(ms(1_000 + index), 7); + } + // Outside the 300s window the earlier restarts fall away. + supervisor.on_child_spawned(identity(42)); + let actions = supervisor.on_child_exited(ms(500_000), 7); + assert_eq!(spawn_count(&actions), 1); + assert!(!supervisor.finished()); + } + + #[test] + fn delayed_drain_forces_termination_once() { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + supervisor.on_child_spawned(identity(42)); + supervisor.on_control(ms(100), Control::Stop); + + // Before the deadline: periodic pending reports, no termination. + let actions = supervisor.tick(ms(200)); + assert!(reported(&actions, ServiceState::StopPending)); + assert!(!actions.contains(&SupervisorAction::ForceTerminateTree)); + + let deadline = 100 + DEFAULT_DRAIN_TIMEOUT.as_millis() as u64; + let actions = supervisor.tick(ms(deadline)); + assert!(actions.contains(&SupervisorAction::ForceTerminateTree)); + + // Once issued, it is not issued again. + let actions = supervisor.tick(ms(deadline + 1_000)); + assert!(!actions.contains(&SupervisorAction::ForceTerminateTree)); + + let actions = supervisor.on_forced_termination(); + assert!(reported(&actions, ServiceState::Stopped)); + assert!(supervisor.forced()); + assert_eq!(supervisor.exit_code(), ExitCode::Clean); + } + + #[test] + fn drain_does_not_force_terminate_while_the_child_is_gone() { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + supervisor.on_child_spawned(identity(42)); + supervisor.on_control(ms(100), Control::Stop); + supervisor.on_child_exited(ms(150), 0); + assert!(supervisor.finished()); + + // A tick after the child is gone and the service is stopped is a no-op. + assert!(supervisor.tick(ms(100_000)).is_empty()); + } + + #[test] + fn fatal_launch_configuration_does_not_restart() { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + let actions = supervisor.on_spawn_failed(ms(10), true); + assert_eq!(spawn_count(&actions), 0); + assert!(supervisor.finished()); + assert_eq!(supervisor.exit_code(), ExitCode::LaunchFailure); + } + + #[test] + fn transient_launch_failure_restarts_then_eventually_stops() { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + for index in 0..3 { + let actions = supervisor.on_spawn_failed(ms(index), false); + assert_eq!(spawn_count(&actions), 1); + } + let actions = supervisor.on_spawn_failed(ms(4), false); + assert_eq!(spawn_count(&actions), 0); + assert_eq!(supervisor.exit_code(), ExitCode::RepeatedFailure); + } + + #[test] + fn identity_verdicts_gate_cleanup() { + assert_eq!(identity_verdict(Ok(true)), IdentityVerdict::Owned); + assert_eq!(identity_verdict(Ok(false)), IdentityVerdict::Foreign); + assert_eq!(identity_verdict(Err(QueryError)), IdentityVerdict::Unknown); + assert!(may_cleanup(IdentityVerdict::Owned)); + assert!(!may_cleanup(IdentityVerdict::Foreign)); + assert!(!may_cleanup(IdentityVerdict::Unknown)); + } + + #[test] + fn exit_codes_map_to_service_specific_errors() { + assert_eq!(ExitCode::Clean.win32(), (0, 0)); + assert_eq!(ExitCode::Child(7).win32(), (1066, 1)); + assert_eq!(ExitCode::RepeatedFailure.win32(), (1066, 2)); + assert_eq!(ExitCode::LaunchFailure.win32(), (1066, 3)); + } +} diff --git a/native/windows-service-host/src/windows/job.rs b/native/windows-service-host/src/windows/job.rs new file mode 100644 index 000000000000..ef70ac4b214a --- /dev/null +++ b/native/windows-service-host/src/windows/job.rs @@ -0,0 +1,322 @@ +//! Windows process-tree ownership through a job object. +//! +//! The child is created suspended, assigned to a job with +//! `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`, and then resumed. Suspending closes +//! the race where a fast child could spawn grandchildren before the job +//! assignment and escape termination. Creating the job here (rather than +//! putting this host in one) means only the descendants of the launcher are +//! owned; the host stays alive to report `SERVICE_STOPPED`. +//! +//! `verify_identity` re-opens the PID and compares its creation time with the +//! one recorded at spawn. A handle we already hold stays valid after the child +//! exits, so it cannot detect PID reuse; a fresh query can. A failed query is +//! returned as an error (`unknown`), never as ownership. + +use std::os::windows::ffi::OsStrExt; +use std::path::Path; + +use windows_sys::Win32::Foundation::{ + CloseHandle, FILETIME, GetLastError, HANDLE, HANDLE_FLAG_INHERIT, INVALID_HANDLE_VALUE, + SetHandleInformation, WAIT_OBJECT_0, WAIT_TIMEOUT, +}; +use windows_sys::Win32::Storage::FileSystem::{ + CreateFileW, FILE_ATTRIBUTE_NORMAL, FILE_SHARE_READ, FILE_SHARE_WRITE, OPEN_ALWAYS, + SetFilePointer, +}; +use windows_sys::Win32::System::JobObjects::{ + AssignProcessToJobObject, CreateJobObjectW, JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, + JOBOBJECT_EXTENDED_LIMIT_INFORMATION, JobObjectExtendedLimitInformation, + SetInformationJobObject, TerminateJobObject, +}; +use windows_sys::Win32::System::Threading::{ + CREATE_NEW_PROCESS_GROUP, CREATE_SUSPENDED, CreateProcessW, GetExitCodeProcess, + GetProcessTimes, OpenProcess, PROCESS_INFORMATION, PROCESS_QUERY_LIMITED_INFORMATION, + ResumeThread, STARTF_USESTDHANDLES, STARTUPINFOW, WaitForSingleObject, +}; + +use crate::config::{LaunchMode, ServiceConfig}; +use crate::host::{ChildHandle, ChildHost, ProcessIdentity, QueryError, SpawnError}; + +const GENERIC_WRITE: u32 = 0x4000_0000; + +struct Handle(HANDLE); + +impl Drop for Handle { + fn drop(&mut self) { + if !self.0.is_null() && self.0 != INVALID_HANDLE_VALUE { + unsafe { CloseHandle(self.0) }; + } + } +} + +pub struct WindowsChildHost; + +impl Default for WindowsChildHost { + fn default() -> Self { + Self + } +} + +impl WindowsChildHost { + pub fn new() -> Self { + Self + } +} + +pub struct WindowsChild { + process: Handle, + job: Handle, + id: ProcessIdentity, + stop_marker: std::path::PathBuf, +} + +impl WindowsChild { + fn creation_time(process: HANDLE) -> Option { + let mut creation = FILETIME { + dwLowDateTime: 0, + dwHighDateTime: 0, + }; + let mut exit = creation; + let mut kernel = creation; + let mut user = creation; + let ok = + unsafe { GetProcessTimes(process, &mut creation, &mut exit, &mut kernel, &mut user) }; + (ok != 0).then(|| ((creation.dwHighDateTime as u64) << 32) | creation.dwLowDateTime as u64) + } +} + +impl ChildHandle for WindowsChild { + fn identity(&self) -> ProcessIdentity { + self.id + } + + fn verify_identity(&mut self) -> Result { + let fresh = unsafe { OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, 0, self.id.pid) }; + if fresh.is_null() { + // Query failed. Ownership is unknown, not owned and not stopped. + return Err(QueryError); + } + let fresh_handle = Handle(fresh); + match WindowsChild::creation_time(fresh_handle.0) { + Some(created) => Ok(created == self.id.created_at_ms), + None => Err(QueryError), + } + } + + fn try_wait(&mut self) -> Result, QueryError> { + let waited = unsafe { WaitForSingleObject(self.process.0, 0) }; + if waited == WAIT_TIMEOUT { + return Ok(None); + } + if waited != WAIT_OBJECT_0 { + return Err(QueryError); + } + let mut code: u32 = 0; + let ok = unsafe { GetExitCodeProcess(self.process.0, &mut code) }; + if ok == 0 { + return Err(QueryError); + } + Ok(Some(code as i32)) + } + + fn request_graceful_stop(&mut self) -> Result<(), QueryError> { + if let Some(parent) = self.stop_marker.parent() { + std::fs::create_dir_all(parent).map_err(|_| QueryError)?; + } + std::fs::write(&self.stop_marker, b"").map_err(|_| QueryError) + } + + fn terminate_tree(&mut self) { + unsafe { + TerminateJobObject(self.job.0, 1); + } + let _ = unsafe { WaitForSingleObject(self.process.0, 5_000) }; + } +} + +impl ChildHost for WindowsChildHost { + type Child = WindowsChild; + + fn spawn(&mut self, config: &ServiceConfig) -> Result { + if matches!(config.mode, LaunchMode::ServiceLauncher) && !config.home.is_dir() { + return Err(SpawnError::Config(format!( + "T3 home {} does not exist", + config.home.display() + ))); + } + + let job = unsafe { CreateJobObjectW(std::ptr::null(), std::ptr::null()) }; + if job.is_null() { + return Err(SpawnError::Launch(format!( + "CreateJobObjectW failed ({})", + unsafe { GetLastError() } + ))); + } + let job = Handle(job); + let limit = extended_limit(); + let ok = unsafe { + SetInformationJobObject( + job.0, + JobObjectExtendedLimitInformation, + &limit as *const _ as *const core::ffi::c_void, + std::mem::size_of::() as u32, + ) + }; + if ok == 0 { + return Err(SpawnError::Launch(format!( + "SetInformationJobObject failed ({})", + unsafe { GetLastError() } + ))); + } + + let mut command_line = build_command_line(config); + let mut startup: STARTUPINFOW = unsafe { std::mem::zeroed() }; + startup.cb = std::mem::size_of::() as u32; + + let mut log_handle: Option = None; + let inherit = if let Some(log_path) = &config.log { + if let Some(parent) = log_path.parent() { + std::fs::create_dir_all(parent).map_err(|error| { + SpawnError::Config(format!("cannot create log dir: {error}")) + })?; + } + let wide = wide_null(log_path); + let handle = unsafe { + CreateFileW( + wide.as_ptr(), + GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, + std::ptr::null(), + OPEN_ALWAYS, + FILE_ATTRIBUTE_NORMAL, + std::ptr::null_mut(), + ) + }; + if handle == INVALID_HANDLE_VALUE { + return Err(SpawnError::Config(format!( + "cannot open log file {} ({})", + log_path.display(), + unsafe { GetLastError() } + ))); + } + unsafe { + SetHandleInformation(handle, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT); + SetFilePointer(handle, 0, std::ptr::null_mut(), 2); + } + startup.dwFlags = STARTF_USESTDHANDLES; + startup.hStdInput = std::ptr::null_mut(); + startup.hStdOutput = handle; + startup.hStdError = handle; + log_handle = Some(Handle(handle)); + true + } else { + false + }; + + let home_wide = wide_null(&config.home); + let mut info: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; + let created = unsafe { + CreateProcessW( + std::ptr::null(), + command_line.as_mut_ptr(), + std::ptr::null(), + std::ptr::null(), + inherit as i32, + CREATE_SUSPENDED | CREATE_NEW_PROCESS_GROUP, + std::ptr::null(), + home_wide.as_ptr(), + &startup, + &mut info, + ) + }; + if created == 0 { + return Err(SpawnError::Launch(format!( + "CreateProcessW failed ({})", + unsafe { GetLastError() } + ))); + } + // The child has inherited its own copy of the log handle; close ours. + drop(log_handle); + let process = Handle(info.hProcess); + let thread = Handle(info.hThread); + + let assigned = unsafe { AssignProcessToJobObject(job.0, process.0) }; + if assigned == 0 { + let error = unsafe { GetLastError() }; + unsafe { + TerminateJobObject(job.0, 1); + } + return Err(SpawnError::Launch(format!( + "AssignProcessToJobObject failed ({error})" + ))); + } + + unsafe { ResumeThread(thread.0) }; + + let created_at_ms = WindowsChild::creation_time(process.0).unwrap_or(0); + Ok(WindowsChild { + process, + job, + id: ProcessIdentity { + pid: info.dwProcessId, + created_at_ms, + }, + stop_marker: config.stop_marker(), + }) + } +} + +fn extended_limit() -> JOBOBJECT_EXTENDED_LIMIT_INFORMATION { + let mut limit: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = unsafe { std::mem::zeroed() }; + limit.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + limit +} + +fn wide_null(value: &Path) -> Vec { + value + .as_os_str() + .encode_wide() + .chain(std::iter::once(0)) + .collect() +} + +fn build_command_line(config: &ServiceConfig) -> Vec { + let (program, args) = config.child_command(); + let mut line = Vec::new(); + push_quoted( + &mut line, + &program.as_os_str().encode_wide().collect::>(), + ); + for arg in &args { + line.push(b' ' as u16); + push_quoted(&mut line, &arg.encode_wide().collect::>()); + } + line.push(0); + line +} + +/// Windows command-line quoting: backslashes are doubled before a quote, and a +/// literal quote needs `2n + 1` backslashes in front of it. +fn push_quoted(out: &mut Vec, value: &[u16]) { + out.push(b'"' as u16); + let mut backslashes = 0usize; + for &character in value { + if character == b'\\' as u16 { + backslashes += 1; + out.push(character); + } else if character == b'"' as u16 { + for _ in 0..backslashes + 1 { + out.push(b'\\' as u16); + } + out.push(character); + backslashes = 0; + } else { + backslashes = 0; + out.push(character); + } + } + for _ in 0..backslashes { + out.push(b'\\' as u16); + } + out.push(b'"' as u16); +} diff --git a/native/windows-service-host/src/windows/mod.rs b/native/windows-service-host/src/windows/mod.rs new file mode 100644 index 000000000000..fece982b6646 --- /dev/null +++ b/native/windows-service-host/src/windows/mod.rs @@ -0,0 +1,10 @@ +//! Windows-only pieces: the job-object child host and the SCM service loop. +//! +//! The portable core in the crate root has no Windows dependency; this module +//! is compiled only when targeting Windows so the crate can be unit tested on +//! a developer host. + +pub mod job; +pub mod service; + +pub use service::run_service; diff --git a/native/windows-service-host/src/windows/service.rs b/native/windows-service-host/src/windows/service.rs new file mode 100644 index 000000000000..7430a3e7a3de --- /dev/null +++ b/native/windows-service-host/src/windows/service.rs @@ -0,0 +1,263 @@ +//! SCM dispatch, the control handler and status reporting. +//! +//! `StartServiceCtrlDispatcherW` connects this process to the service control +//! manager. It must run on the process's main thread. `ServiceMain` runs on an +//! SCM-owned thread and must not block: it registers the extended control +//! handler, starts the supervisor, and returns when the service stops. +//! +//! The control handler only records intent and wakes the supervisor through a +//! channel; it never waits on the child. This is what keeps control handling +//! from hanging when the launcher is slow to drain. + +use std::path::PathBuf; +use std::sync::OnceLock; +use std::sync::atomic::{AtomicIsize, AtomicU32, Ordering}; +use std::sync::mpsc; + +use windows_sys::Win32::Foundation::GetLastError; +use windows_sys::Win32::System::Services::{ + RegisterServiceCtrlHandlerExW, SERVICE_STATUS, SERVICE_STATUS_HANDLE, SERVICE_TABLE_ENTRYW, + SetServiceStatus, StartServiceCtrlDispatcherW, +}; + +use super::job::WindowsChildHost; +use crate::config::ServiceConfig; +use crate::control::{Control, ServiceState}; +use crate::run::{ChannelControlInput, Reporter, run}; +use crate::supervise::ExitCode; + +const SERVICE_WIN32_OWN_PROCESS: u32 = 0x0000_0010; +const SERVICE_ACCEPT_STOP: u32 = 0x0000_0001; +const SERVICE_ACCEPT_SHUTDOWN: u32 = 0x0000_0004; +const NO_ERROR: u32 = 0; +const WAIT_HINT_MS: u32 = 30_000; + +struct Shared { + sender: mpsc::Sender, + status_handle: AtomicIsize, + state: AtomicU32, + win32_exit: AtomicU32, + specific_exit: AtomicU32, + checkpoint: AtomicU32, + service_name: Vec, +} + +static CONFIG: OnceLock = OnceLock::new(); +static SHARED: OnceLock = OnceLock::new(); +static SETTLED: OnceLock = OnceLock::new(); + +/// Connect to SCM and run until the service stops. Windows only. +pub fn run_service(config: ServiceConfig) -> Result { + if CONFIG.set(config).is_err() { + return Err("the service host was initialized twice".to_owned()); + } + let config = CONFIG.get().expect("configuration just set"); + if let Err(reason) = check_service_account(config) { + return Err(reason); + } + let name = wide_null(&config.service_name); + let name = Box::leak(name.into_boxed_slice()); + let dispatch_table = [ + SERVICE_TABLE_ENTRYW { + lpServiceName: name.as_mut_ptr(), + lpServiceProc: Some(service_main), + }, + SERVICE_TABLE_ENTRYW { + lpServiceName: std::ptr::null_mut(), + lpServiceProc: None, + }, + ]; + let started = unsafe { StartServiceCtrlDispatcherW(dispatch_table.as_ptr()) }; + if started == 0 { + return Err(format!("StartServiceCtrlDispatcherW failed ({})", unsafe { + GetLastError() + })); + } + Ok(SETTLED.get().copied().unwrap_or(ExitCode::Unknown)) +} + +unsafe extern "system" fn service_main(_argc: u32, _argv: *mut *mut u16) { + let config = match CONFIG.get() { + Some(config) => config, + None => return, + }; + let (sender, receiver) = mpsc::channel(); + let shared = Shared { + sender, + status_handle: AtomicIsize::new(0), + state: AtomicU32::new(ServiceState::StartPending.win32_state()), + win32_exit: AtomicU32::new(0), + specific_exit: AtomicU32::new(0), + checkpoint: AtomicU32::new(0), + service_name: wide_null(&config.service_name), + }; + if SHARED.set(shared).is_err() { + return; + } + let shared = SHARED.get().expect("shared state just set"); + let handle = unsafe { + RegisterServiceCtrlHandlerExW( + shared.service_name.as_ptr(), + Some(control_handler), + shared as *const Shared as *mut core::ffi::c_void, + ) + }; + if handle as isize == 0 { + let _ = SETTLED.set(ExitCode::Unknown); + return; + } + shared + .status_handle + .store(handle as isize, Ordering::SeqCst); + emit_status(shared); + + let mut host = WindowsChildHost::new(); + let mut controls = ChannelControlInput { receiver }; + let mut reporter = ScmReporter; + let log_path = config.log.clone(); + let mut log = |message: &str| append_log(log_path.as_ref(), message); + let outcome = run(config, &mut host, &mut controls, &mut reporter, &mut log); + set_status(ServiceState::Stopped, outcome.exit, 0); + let _ = SETTLED.set(outcome.exit); +} + +unsafe extern "system" fn control_handler( + control: u32, + _event_type: u32, + _event_data: *mut core::ffi::c_void, + context: *mut core::ffi::c_void, +) -> u32 { + let shared = unsafe { &*(context as *const Shared) }; + match Control::from_win32(control) { + Control::Stop | Control::Shutdown => { + // Unbounded channel: this never blocks the SCM thread. + let _ = shared.sender.send(Control::from_win32(control)); + } + Control::Interrogate => emit_status(shared), + Control::Other => {} + } + NO_ERROR +} + +struct ScmReporter; + +impl Reporter for ScmReporter { + fn report(&mut self, state: ServiceState, exit: ExitCode, checkpoint: u32) { + set_status(state, exit, checkpoint); + } +} + +fn set_status(state: ServiceState, exit: ExitCode, checkpoint: u32) { + let Some(shared) = SHARED.get() else { + return; + }; + shared.state.store(state.win32_state(), Ordering::SeqCst); + let (win32, specific) = exit.win32(); + shared.win32_exit.store(win32, Ordering::SeqCst); + shared.specific_exit.store(specific, Ordering::SeqCst); + shared.checkpoint.store(checkpoint, Ordering::SeqCst); + emit_status(shared); +} + +fn emit_status(shared: &Shared) { + let raw = shared.status_handle.load(Ordering::SeqCst); + if raw == 0 { + return; + } + let state = shared.state.load(Ordering::SeqCst); + let pending = state == ServiceState::StartPending.win32_state() + || state == ServiceState::StopPending.win32_state(); + let mut status = SERVICE_STATUS { + dwServiceType: SERVICE_WIN32_OWN_PROCESS, + dwCurrentState: state, + dwControlsAccepted: if state == ServiceState::Running.win32_state() { + SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN + } else { + 0 + }, + dwWin32ExitCode: shared.win32_exit.load(Ordering::SeqCst), + dwServiceSpecificExitCode: shared.specific_exit.load(Ordering::SeqCst), + dwCheckPoint: shared.checkpoint.load(Ordering::SeqCst), + dwWaitHint: if pending { WAIT_HINT_MS } else { 0 }, + }; + unsafe { SetServiceStatus(raw as SERVICE_STATUS_HANDLE, &mut status) }; +} + +/// Refuse the default LocalSystem workload and enforce an explicit expected +/// account. The account password is never here: SCM stores it in LSA and +/// starts the process under that token. +fn check_service_account(config: &ServiceConfig) -> Result<(), String> { + let account = current_user_name().ok_or_else(|| { + "could not read the service account; refusing to start without an identity".to_owned() + })?; + if !config.allow_local_system && account.eq_ignore_ascii_case("SYSTEM") { + return Err( + "refusing to run the T3 workload as LocalSystem; register the service with a dedicated account" + .to_owned(), + ); + } + if let Some(expected) = &config.expected_account { + let matches = account.eq_ignore_ascii_case(expected) + || account + .rsplit('\\') + .next() + .is_some_and(|name| name.eq_ignore_ascii_case(expected)); + if !matches { + return Err(format!( + "service account '{account}' does not match the expected account '{expected}'" + )); + } + } + Ok(()) +} + +fn current_user_name() -> Option { + let mut size: u32 = 0; + unsafe { + // First call sizes the buffer; the expected ERROR_INSUFFICIENT_BUFFER is + // not an error for this probe. + windows_sys::Win32::System::WindowsProgramming::GetUserNameW( + std::ptr::null_mut(), + &mut size, + ); + } + if size == 0 { + return None; + } + let mut buffer = vec![0u16; size as usize]; + let ok = unsafe { + windows_sys::Win32::System::WindowsProgramming::GetUserNameW(buffer.as_mut_ptr(), &mut size) + }; + if ok == 0 { + return None; + } + let end = buffer + .iter() + .position(|&unit| unit == 0) + .unwrap_or(buffer.len()); + Some(String::from_utf16_lossy(&buffer[..end])) +} + +fn append_log(log_path: Option<&PathBuf>, message: &str) { + use std::io::Write; + let line = format!("[t3-windows-service-host] {message}\n"); + match log_path { + Some(path) => { + if let Some(parent) = path.parent() { + let _ = std::fs::create_dir_all(parent); + } + if let Ok(mut file) = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(path) + { + let _ = file.write_all(line.as_bytes()); + } + } + None => eprint!("{line}"), + } +} + +fn wide_null(value: &str) -> Vec { + value.encode_utf16().chain(std::iter::once(0)).collect() +} diff --git a/native/windows-service-host/tests/portable_host.rs b/native/windows-service-host/tests/portable_host.rs new file mode 100644 index 000000000000..14a777f8360a --- /dev/null +++ b/native/windows-service-host/tests/portable_host.rs @@ -0,0 +1,122 @@ +//! Portable integration test for the real `CommandChildHost`. +//! +//! This runs on a Unix developer host: it spawns a real dummy child through the +//! production `ServiceLauncher` code path, requests a graceful stop, lets the +//! drain deadline pass, and asserts the child is terminated. It does not prove +//! SCM dispatch or Windows job-object ownership; those are the unexecuted +//! native gate in the scoped design. +#![cfg(unix)] + +use std::collections::VecDeque; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; + +use t3_windows_service_host::config::{LaunchMode, ServiceConfig}; +use t3_windows_service_host::control::{Control, ServiceState}; +use t3_windows_service_host::host::CommandChildHost; +use t3_windows_service_host::run::{Reporter, ScriptedControl, run}; +use t3_windows_service_host::supervise::ExitCode; + +#[derive(Clone)] +struct Capturing { + events: Arc>>, +} + +impl Reporter for Capturing { + fn report(&mut self, state: ServiceState, exit: ExitCode, _checkpoint: u32) { + self.events + .lock() + .unwrap() + .push(format!("{state:?}:{exit:?}")); + } +} + +fn unique_dir() -> PathBuf { + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + std::env::temp_dir().join(format!("t3-winsvc-{}-{nanos}", std::process::id())) +} + +fn write_runtime(home: &Path) -> PathBuf { + use std::os::unix::fs::PermissionsExt; + let runtime = home.join("t3.exe"); + let pid_file = home.join("child.pid"); + let script = format!( + "#!/bin/sh\necho $$ > '{}'\nexec sleep 30\n", + pid_file.display() + ); + std::fs::write(&runtime, script).unwrap(); + std::fs::set_permissions(&runtime, std::fs::Permissions::from_mode(0o755)).unwrap(); + runtime +} + +#[test] +fn real_child_is_stopped_and_terminated_after_drain() { + let dir = unique_dir(); + let home = dir.join("home"); + std::fs::create_dir_all(&home).unwrap(); + let runtime = write_runtime(&home); + + let config = ServiceConfig { + home: home.clone(), + runtime, + log: Some(home.join("service.log")), + service_name: "t3code-test".to_owned(), + drain_timeout: Duration::from_millis(150), + restart_window: Duration::from_secs(300), + max_restarts: 3, + poll_interval: Duration::from_millis(2), + expected_account: None, + allow_local_system: false, + mode: LaunchMode::ServiceLauncher, + }; + + let events = Arc::new(Mutex::new(Vec::new())); + let mut host = CommandChildHost::new(&config); + let mut controls = ScriptedControl { + script: VecDeque::from([None, None, Some(Control::Stop), None, None, None, None]), + }; + let mut reporter = Capturing { + events: events.clone(), + }; + let mut log = |_message: &str| {}; + + let outcome = run(&config, &mut host, &mut controls, &mut reporter, &mut log); + + let reports = events.lock().unwrap().clone(); + assert!( + reports + .iter() + .any(|event| event.starts_with("Stopped:Clean")) + ); + assert!(outcome.forced, "slow drain must be force-terminated"); + assert!( + config.stop_marker().exists(), + "the launcher stop marker must be written before termination" + ); + assert!( + config.log.as_ref().is_some_and(|log| log.exists()), + "child output must be redirected to the configured log" + ); + + let pid: i32 = std::fs::read_to_string(home.join("child.pid")) + .unwrap() + .trim() + .parse() + .unwrap(); + let alive = std::process::Command::new("/bin/sh") + .arg("-c") + .arg(format!("kill -0 {pid} 2>/dev/null")) + .status() + .map(|status| status.success()) + .unwrap_or(false); + assert!( + !alive, + "the child process must be gone after the service stops" + ); + + std::fs::remove_dir_all(&dir).ok(); +} From 9b3bc5f50a545848a1d0af17fdad1019f900591a Mon Sep 17 00:00:00 2001 From: nullStack65 Date: Sat, 26 Sep 2026 21:08:06 -0400 Subject: [PATCH 2/7] docs(service): record the CI native crate-list touch point --- docs/internals/windows-background-service.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/internals/windows-background-service.md b/docs/internals/windows-background-service.md index abfc715aa82a..94beab773da0 100644 --- a/docs/internals/windows-background-service.md +++ b/docs/internals/windows-background-service.md @@ -179,6 +179,11 @@ Packaging (`packaging/**`, root workspaces, release workflows): compile the host and ship it beside the pinned runtime. Root Cargo/package workspaces and the release pipeline are outside this slice. +CI (`.github/workflows/ci.yml`): the `Rust` job hardcodes the native crate list +(`resource-monitor kde-snap-shot hyprland-snap-shot`). Add `windows-service-host` +there so its portable tests and `cargo fmt --check` run on every PR. The host +already passes both locally; the workflow itself is outside this slice. + ## Artifact and provenance inputs - The host binary: pinned fork commit and toolchain, signed, with a recorded From b4ab6c78e382b2d4cd0086533a257c54d0a0a695 Mon Sep 17 00:00:00 2001 From: nullStack65 Date: Sat, 26 Sep 2026 23:22:36 -0400 Subject: [PATCH 3/7] fix(service): repair the SCM lifecycle, native home binding, admission unwind and stop outcomes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the F1-F4 source review of the Windows SCM host at 9b3bc5f5. F1 — publish the real SCM lifecycle. A successful spawn now returns and publishes the RUNNING report, so the SCM leaves START_PENDING and accepts STOP/SHUTDOWN. The loop remains the single authority for the final STOPPED: service_main no longer emits a second stopped status after run returns. Reporter::report is fallible; a SetServiceStatus failure is surfaced, aborts the run once, cleans the owned child and ends with an unknown cause. F2 — bind the native child environment. CreateProcessW now receives a Unicode environment block with the selected home as T3CODE_HOME, overriding an absent or conflicting ambient value, while preserving the rest of the environment. The block builder is portable and unit tested; the pinned launcher's required variable is now proven at the native spawn request. Expected-account binding uses qualified GetUserNameExW identities (SAM/UPN); a bare --expected-account is refused. The plaintext-password recipe is removed. F3 — scoped, checked process admission. Assignment, creation-time identity capture and ResumeThread are checked through a portable admission seam; any failure terminates the freshly created process and reports the cleanup outcome. A failed identity query is unknown, not a fabricated zero, and a suspended child is never left orphaned. F4 — honest stop outcomes and drop effects. terminate_tree returns an explicit CleanupOutcome; only a confirmed cleanup is a clean stop. Failed, unknown and refused cleanups report STOPPED with an unknown cause. Dropping the kill-on-close job is treated as the termination effect it is, bounded and explicit, and only touches this host's owned job members. The dummy native recipe is corrected: matching test-child build, actual binary path, explicit test-account ACL grant, uniquely captured child/grandchild PIDs and try/finally cleanup. Portable tests: 56 unit + 1 integration passed. Windows-target cargo check (with and without test-child) and rustfmt are clean. No native SCM run was possible; that gate remains unrun. --- docs/internals/windows-background-service.md | 149 +++++-- native/windows-service-host/Cargo.toml | 1 + native/windows-service-host/src/account.rs | 68 +++ native/windows-service-host/src/admission.rs | 171 ++++++++ native/windows-service-host/src/config.rs | 40 ++ .../windows-service-host/src/environment.rs | 149 +++++++ native/windows-service-host/src/host.rs | 37 +- native/windows-service-host/src/lib.rs | 4 + native/windows-service-host/src/main.rs | 10 +- native/windows-service-host/src/run.rs | 387 +++++++++++++++--- native/windows-service-host/src/supervise.rs | 84 +++- .../windows-service-host/src/windows/job.rs | 172 ++++++-- .../src/windows/service.rs | 109 +++-- .../tests/portable_host.rs | 10 +- 14 files changed, 1209 insertions(+), 182 deletions(-) create mode 100644 native/windows-service-host/src/account.rs create mode 100644 native/windows-service-host/src/admission.rs create mode 100644 native/windows-service-host/src/environment.rs diff --git a/docs/internals/windows-background-service.md b/docs/internals/windows-background-service.md index 94beab773da0..ac76457773d8 100644 --- a/docs/internals/windows-background-service.md +++ b/docs/internals/windows-background-service.md @@ -27,7 +27,13 @@ not enough. The host implements: supervisor loop. - `SetServiceStatus` with `START_PENDING`, `RUNNING`, `STOP_PENDING` and `STOPPED`, `dwCheckPoint`/`dwWaitHint` while pending, and - `SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN` only while running. + `SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN` only while running. A + successful spawn publishes the actual `RUNNING` transition: until it is + published the SCM stays `START_PENDING` and accepts no stop control. Exactly + one final `STOPPED` is published, after the child resources are released, and + a `SetServiceStatus` failure is propagated rather than ignored (Microsoft's + SetServiceStatus contract allows no later status call once the service is + stopped). - A control handler that records intent and wakes the supervisor; it never blocks on the child. Control handling cannot hang indefinitely. @@ -47,28 +53,45 @@ the workload at an interactive user's profile. - `--runtime` must be the pinned `t3.exe`. The host appends `__service-launcher` and launches it once per service start. Remote updates replace the launcher's server child; they do not re-exec the host or change its command line. -- `T3CODE_HOME` is set explicitly on the child; child stdout/stderr go to - `--log` when supplied. +- The native spawn builds a Unicode environment block for `CreateProcessW` with + the selected `--home` as `T3CODE_HOME`, overriding an absent or conflicting + ambient value; cwd alone is not enough because the pinned launcher reads + `process.env.T3CODE_HOME`. The rest of the intended host environment is + preserved. Child stdout/stderr go to `--log` when supplied. - No credential flag is accepted. A password or token argument is refused - rather than forwarded. The account password is registered with SCM - (`sc.exe create T3Code ... obj= ".\t3service" password= "..."`) and stored by - LSA; the host never sees it. + rather than forwarded. The account password, if any, is registered with SCM + out of band and kept in LSA; the host never sees it and no password belongs + on the command line. - LocalSystem is refused unless `--allow-local-system` is passed explicitly. - `--expected-account` pins the account the process must run as. + `--expected-account` must be qualified (`DOMAIN\user` or `user@domain`); a + bare name is refused in configuration because it cannot prove the domain. The + running identity is read with `GetUserNameExW` (SAM and UPN forms), never the + ambiguous bare `GetUserNameW` name. Account constraints: use a dedicated account, ideally the virtual `NT SERVICE\T3Code` service SID, or a dedicated local/domain user. Do not run the T3 workload as LocalSystem and do not reuse an interactive user's home or profile. A virtual service account has no user profile or `HKCU`, so features -that need one are unavailable (below). +that need one are unavailable (below). Dedicated-account provisioning or reuse +of an existing credential is not selected by this source repair; installer +identity selection remains later work. ## Shutdown and process ownership The child is created suspended, assigned to a job object with -`JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`, then resumed. Suspending closes the race -where a fast child could spawn grandchildren before assignment. The job is -created for the child; the host is not in it, so `TerminateJobObject` can kill -the owned tree while the host stays alive to report `SERVICE_STOPPED`. +`JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`, its identity captured, then resumed. +Suspending closes the race where a fast child could spawn grandchildren before +assignment. The job is created for the child; the host is not in it, so +`TerminateJobObject` can kill the owned tree while the host stays alive to +report `SERVICE_STOPPED`. + +Every step after `CreateProcessW` succeeds is checked and scoped: assignment to +the job, creation-time identity capture, and `ResumeThread`. If any of them +fails, the freshly created process is terminated explicitly — the created +process handle is retained until that cleanup outcome is known — and the +failure is reported together with the cleanup result. A suspended child is never +left orphaned outside the job, and a failed creation-time query is reported as +unknown rather than a fabricated zero identity. A stop is bounded and two-stage: @@ -76,7 +99,13 @@ A stop is bounded and two-stage: marker and asks the launcher to stop, then reports `STOP_PENDING` with checkpoints. 2. If the child has not exited by `--drain-timeout-ms` (default 30s), the job - tree is terminated and the service reports `STOPPED`. + tree is terminated and the service reports `STOPPED`. The termination outcome + is explicit: only a confirmed child exit is a clean stop. A refused + (foreign or unverified) tree and a failed or unconfirmed termination report + `STOPPED` with an unknown cause. Closing the kill-on-close job is itself a + termination effect, so it is treated as an implicit, bounded cleanup rather + than as evidence of a clean shutdown; the job only ever contains this host's + owned members, so no PID or process name is matched. This is deliberately different from a normal launcher replacement. During an update the launcher terminates its own server child but stays alive and starts @@ -132,6 +161,9 @@ fallback or relies solely on the control message. budget lives here. - **Slow drain:** reports `STOP_PENDING` with checkpoints and forces the tree after the deadline. It never hangs. +- **Publication failure:** if `SetServiceStatus` fails for any state, the failure + is surfaced (not ignored); the host abandons the run, cleans the owned child + once and reports an unknown cause rather than claiming a clean stop. - **Stale/foreign PID:** as above, verified and left alone; unknown is not stopped. @@ -216,37 +248,74 @@ Native acceptance (not executed here; see the recipe): ## Native test recipe (unexecuted) -Only an environment already reserved for disposable Windows tests may run this, -with unique names, dummy children, a bounded runtime and verified cleanup. Do -not run it on an active workstation or against real T3 state. +Only an environment already reserved for disposable Windows tests may run this. +Do not run it on an active workstation or against real T3 state. The recipe +builds the host with the development-only `test-child` feature, runs a uniquely +named dummy child that records its own and its grandchild's PID inside the probe +home, and cleans up in a `finally` block. Process cleanup is proven by those +captured PIDs, not by a process-name listing. ```powershell -# Build (developer host, Windows target): -cargo build --locked --release --manifest-path native/windows-service-host/Cargo.toml -# For dummy children instead of the pinned launcher, build with the -# development-only feature: -# cargo build --locked --release --features test-child ... - -$svc = "T3WinSvcProbe$([guid]::NewGuid().ToString('N').Substring(0,8))" +# Build the exact binary the recipe runs. The crate is standalone, so the binary +# lands under native/windows-service-host/target; build WITH the test-child +# feature so --exec exists, or the launch below will fail. +cargo build --locked --release --features test-child ` + --manifest-path native/windows-service-host/Cargo.toml +$hostExe = Join-Path $PWD "native/windows-service-host/target/release/t3-windows-service-host.exe" +if (-not (Test-Path $hostExe)) { throw "host binary not built at $hostExe" } + +$svc = "T3WinSvcProbe$([guid]::NewGuid().ToString('N').Substring(0,8))" $root = Join-Path $env:TEMP $svc -$home = Join-Path $root "home"; New-Item -ItemType Directory -Force $home | Out-Null -# Dummy child: a script that ignores the stop marker and sleeps, plus a -# grandchild, to prove tree termination. -$dummy = Join-Path $root "dummy.cmd" -"@echo off`r`nstart /b ping -n 600 127.0.0.1 >nul`r`nping -n 600 127.0.0.1 >nul" | Set-Content $dummy - -sc.exe create $svc binPath= "`"$PWD\target\release\t3-windows-service-host.exe`" --home `"$home`" --service-name $svc --exec cmd.exe --exec-arg /c --exec-arg `"$dummy`" --drain-timeout-ms 3000" ` - obj= "NT AUTHORITY\LocalService" start= demand -sc.exe start $svc -sc.exe query $svc # expect RUNNING -sc.exe control $svc 4 # interrogate -sc.exe stop $svc # expect STOPPED within the drain bound -Get-Process -Name ping -ErrorAction SilentlyContinue # must list none for this probe -sc.exe delete $svc -Remove-Item -Recurse -Force $root +$home = Join-Path $root "home" +New-Item -ItemType Directory -Force $home | Out-Null + +# The dummy records its own PID and spawns a sleeping grandchild that records +# its PID too, so cleanup can be checked against exactly these two processes. +$dummy = Join-Path $root "dummy.ps1" +@' +$pidFile = Join-Path $env:T3CODE_HOME "dummy.pid" +$PID | Set-Content $pidFile +$gcFile = Join-Path $env:T3CODE_HOME "grandchild.pid" +Start-Process powershell -WindowStyle Hidden -ArgumentList ` + "-NoProfile","-Command","$PID | Set-Content '$gcFile'; Start-Sleep -Seconds 600" +Start-Sleep -Seconds 600 +'@ | Set-Content $dummy + +# A service account cannot read the interactive user's TEMP: grant the selected +# test account explicit access to only this probe root. +icacls $root /grant "NT AUTHORITY\LocalService:(OI)(CI)F" /T | Out-Null + +$created = $false +try { + sc.exe create $svc binPath= "`"$hostExe`" --home `"$home`" --service-name $svc --exec powershell.exe --exec-arg -NoProfile --exec-arg -File --exec-arg `"$dummy`" --drain-timeout-ms 3000" ` + obj= "NT AUTHORITY\LocalService" start= demand + $created = $true + sc.exe start $svc + sc.exe query $svc # expect RUNNING + sc.exe control $svc 4 # interrogate; must return + sc.exe stop $svc # expect STOPPED within the drain bound + + $probePids = @( + (Join-Path $home "dummy.pid"), + (Join-Path $home "grandchild.pid") + ) | Where-Object { Test-Path $_ } | ForEach-Object { [int](Get-Content $_) } + foreach ($probePid in $probePids) { + if (Get-Process -Id $probePid -ErrorAction SilentlyContinue) { + throw "probe-owned process $probePid survived the stop" + } + } +} finally { + if ($created) { + sc.exe stop $svc | Out-Null + sc.exe delete $svc | Out-Null + } + Remove-Item -Recurse -Force $root -ErrorAction SilentlyContinue +} ``` `--exec` exists only under `--features test-child` and is not part of the production child selection. Portable and cross-compiled checks prove the -portable core, the SCM FFI's type surface and the quoting logic; they do not -prove a real service run, job-object ownership or graceful shutdown. +portable core, the SCM FFI's type surface, the environment/argument construction +and the quoting logic; they do not prove a real service run, job-object +ownership or graceful shutdown. Running this recipe on the reserved environment +is the native gate; its absence leaves that gate unrun. diff --git a/native/windows-service-host/Cargo.toml b/native/windows-service-host/Cargo.toml index 03cabfb07268..f1f201b37aea 100644 --- a/native/windows-service-host/Cargo.toml +++ b/native/windows-service-host/Cargo.toml @@ -27,6 +27,7 @@ test-child = [] windows-sys = { version = "0.61.2", features = [ "Win32_Foundation", "Win32_Security", + "Win32_Security_Authentication_Identity", "Win32_Storage_FileSystem", "Win32_System_Console", "Win32_System_JobObjects", diff --git a/native/windows-service-host/src/account.rs b/native/windows-service-host/src/account.rs new file mode 100644 index 000000000000..64897d00a4e7 --- /dev/null +++ b/native/windows-service-host/src/account.rs @@ -0,0 +1,68 @@ +//! Qualified service-account identity matching. +//! +//! `GetUserNameW` returns a bare account name with no domain, so comparing a +//! potentially qualified `--expected-account` against it is ambiguous: the same +//! name can exist in several domains. When the binding is supplied the host +//! therefore proves identity against qualified forms (`DOMAIN\user` from +//! `NameSamCompatible`, `user@domain` from `NameUserPrincipal`). A bare expected +//! name is rejected at configuration time because it cannot prove which domain +//! the process runs as. + +/// A qualified account carries a domain component. +pub fn is_qualified(account: &str) -> bool { + account.contains('\\') || account.contains('@') +} + +/// True when the expected account matches either qualified identity form. +/// At least one identity form must be present; otherwise there is no proof. +pub fn qualified_match(expected: &str, sam: Option<&str>, upn: Option<&str>) -> bool { + [sam, upn] + .into_iter() + .flatten() + .any(|identity| identity.eq_ignore_ascii_case(expected)) +} + +/// True when a SAM account is LocalSystem. Used to refuse the default +/// LocalSystem workload; `--allow-local-system` is the explicit override. +pub fn is_local_system(sam: Option<&str>) -> bool { + sam.and_then(|sam| sam.rsplit('\\').next()) + .is_some_and(|name| name.eq_ignore_ascii_case("SYSTEM")) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn bare_names_are_not_qualified() { + assert!(!is_qualified("t3service")); + assert!(is_qualified(r"CORP\t3service")); + assert!(is_qualified("t3service@corp.example")); + } + + #[test] + fn matches_sam_and_upn_case_insensitively() { + assert!(qualified_match( + r"CORP\t3service", + Some(r"corp\T3Service"), + None + )); + assert!(qualified_match( + "t3service@corp.example", + None, + Some("T3Service@Corp.Example") + )); + assert!(!qualified_match( + r"CORP\t3service", + Some(r"OTHER\t3service"), + None + )); + } + + #[test] + fn detects_local_system_from_the_sam_form() { + assert!(is_local_system(Some(r"NT AUTHORITY\SYSTEM"))); + assert!(!is_local_system(Some(r"NT AUTHORITY\LocalService"))); + assert!(!is_local_system(None)); + } +} diff --git a/native/windows-service-host/src/admission.rs b/native/windows-service-host/src/admission.rs new file mode 100644 index 000000000000..de798df60484 --- /dev/null +++ b/native/windows-service-host/src/admission.rs @@ -0,0 +1,171 @@ +//! Scoped, checked process admission. +//! +//! `CreateProcessW` returns a *suspended* process. Three steps must all succeed +//! before the host may own it: assign it to the job object, capture its +//! creation-time identity, and resume its primary thread. A failure at any step +//! must reclaim the freshly created process explicitly — a suspended child left +//! outside the job is an orphan, and closing its handle does not terminate it. +//! +//! The native implementation retains the created process handle until the +//! cleanup outcome is known and reports that outcome with the failure. This +//! module is portable so each failure can be injected without Windows. + +use crate::host::{CleanupOutcome, ProcessIdentity}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AdmissionStage { + AssignToJob, + CaptureIdentity, + Resume, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AdmissionFailure { + pub stage: AdmissionStage, + pub reason: String, + /// What happened when the freshly created process was reclaimed. A failure + /// to confirm cleanup is itself part of the admission failure. + pub cleanup: CleanupOutcome, +} + +/// The native operations an admission needs. Implemented over the real handles +/// in `windows::job`; tests inject each failure through a portable fake. +pub trait AdmissionOps { + fn assign_to_job(&mut self) -> Result<(), String>; + fn capture_identity(&mut self) -> Result; + fn resume(&mut self) -> Result<(), String>; + /// Terminate the freshly created process and report whether that is confirmed. + fn terminate_created(&mut self) -> CleanupOutcome; +} + +pub fn admit(ops: &mut O) -> Result { + if let Err(reason) = ops.assign_to_job() { + return Err(AdmissionFailure { + stage: AdmissionStage::AssignToJob, + reason, + cleanup: ops.terminate_created(), + }); + } + let identity = match ops.capture_identity() { + Ok(identity) => identity, + Err(reason) => { + return Err(AdmissionFailure { + stage: AdmissionStage::CaptureIdentity, + reason, + cleanup: ops.terminate_created(), + }); + } + }; + if let Err(reason) = ops.resume() { + return Err(AdmissionFailure { + stage: AdmissionStage::Resume, + reason, + cleanup: ops.terminate_created(), + }); + } + Ok(identity) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[derive(Default)] + struct FakeOps { + fail_assign: bool, + fail_identity: bool, + fail_resume: bool, + cleanup: Option, + terminated: bool, + } + + impl AdmissionOps for FakeOps { + fn assign_to_job(&mut self) -> Result<(), String> { + if self.fail_assign { + Err("assign failed".to_owned()) + } else { + Ok(()) + } + } + fn capture_identity(&mut self) -> Result { + if self.fail_identity { + Err("creation time unavailable".to_owned()) + } else { + Ok(ProcessIdentity { + pid: 42, + created_at_ms: 1_000, + }) + } + } + fn resume(&mut self) -> Result<(), String> { + if self.fail_resume { + Err("resume failed".to_owned()) + } else { + Ok(()) + } + } + fn terminate_created(&mut self) -> CleanupOutcome { + self.terminated = true; + self.cleanup.unwrap_or(CleanupOutcome::Confirmed) + } + } + + #[test] + fn success_does_not_terminate_the_created_process() { + let mut ops = FakeOps::default(); + let identity = admit(&mut ops).expect("admission succeeds"); + assert_eq!(identity.pid, 42); + assert!( + !ops.terminated, + "a successfully admitted process is not cleaned up" + ); + } + + #[test] + fn assignment_failure_reclaims_the_created_process() { + let mut ops = FakeOps { + fail_assign: true, + ..FakeOps::default() + }; + let failure = admit(&mut ops).unwrap_err(); + assert_eq!(failure.stage, AdmissionStage::AssignToJob); + assert!( + ops.terminated, + "the suspended child must not be left orphaned" + ); + assert_eq!(failure.cleanup, CleanupOutcome::Confirmed); + } + + #[test] + fn identity_failure_reclaims_the_created_process() { + let mut ops = FakeOps { + fail_identity: true, + ..FakeOps::default() + }; + let failure = admit(&mut ops).unwrap_err(); + assert_eq!(failure.stage, AdmissionStage::CaptureIdentity); + assert!(ops.terminated); + } + + #[test] + fn resume_failure_reclaims_the_created_process() { + let mut ops = FakeOps { + fail_resume: true, + ..FakeOps::default() + }; + let failure = admit(&mut ops).unwrap_err(); + assert_eq!(failure.stage, AdmissionStage::Resume); + assert!(ops.terminated); + } + + #[test] + fn an_unconfirmed_cleanup_is_reported_with_the_failure() { + let mut ops = FakeOps { + fail_assign: true, + cleanup: Some(CleanupOutcome::Failed), + ..FakeOps::default() + }; + let failure = admit(&mut ops).unwrap_err(); + assert_eq!(failure.cleanup, CleanupOutcome::Failed); + } +} diff --git a/native/windows-service-host/src/config.rs b/native/windows-service-host/src/config.rs index 9d81ae4e4a39..c20c5ed59d2a 100644 --- a/native/windows-service-host/src/config.rs +++ b/native/windows-service-host/src/config.rs @@ -118,6 +118,7 @@ pub enum ConfigError { InvalidNumber(String), CredentialArgument(String), MissingTestChild, + ExpectedAccountUnqualified(String), } impl std::fmt::Display for ConfigError { @@ -157,6 +158,10 @@ impl std::fmt::Display for ConfigError { formatter, "test-child mode requires --exec [--exec-arg ]" ), + ConfigError::ExpectedAccountUnqualified(account) => write!( + formatter, + "--expected-account '{account}' is not qualified; use DOMAIN\\user or user@domain so the account can be proven unambiguously" + ), } } } @@ -338,6 +343,11 @@ pub fn parse(args: impl IntoIterator) -> Result Vec { + let mut entries: Vec<(String, OsString)> = Vec::with_capacity(ambient.len() + 1); + for (name, value) in ambient { + let name = name.to_string_lossy().into_owned(); + if name.eq_ignore_ascii_case(HOME_KEY) { + continue; + } + entries.push((name, value.clone())); + } + entries.push((HOME_KEY.to_owned(), home.as_os_str().to_os_string())); + entries.sort_by_key(|(name, _)| name.to_ascii_lowercase()); + + let mut block = Vec::new(); + for (name, value) in entries { + block.extend(name.encode_utf16()); + block.push(u16::from(b'=')); + block.extend(value.to_string_lossy().encode_utf16()); + block.push(0); + } + block.push(0); + block +} + +/// The production launch request's environment: the host's ambient environment +/// with the selected home overriding any absent/conflicting `T3CODE_HOME`. +pub fn host_environment(home: &Path) -> Vec { + let ambient: Vec<(OsString, OsString)> = std::env::vars_os().collect(); + build_environment_block(&ambient, home) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + fn vars(block: &[u16]) -> Vec { + let mut result = Vec::new(); + let mut current = String::new(); + for &unit in block { + if unit == 0 { + if !current.is_empty() { + result.push(std::mem::take(&mut current)); + } + } else if let Some(character) = char::from_u32(unit as u32) { + current.push(character); + } + } + result + } + + fn ambient(pairs: &[(&str, &str)]) -> Vec<(OsString, OsString)> { + pairs + .iter() + .map(|(name, value)| (OsString::from(name), OsString::from(value))) + .collect() + } + + #[test] + fn supplies_the_selected_home_when_absent() { + let block = build_environment_block( + &ambient(&[("PATH", r"C:\Windows")]), + &PathBuf::from(r"D:\t3\.t3"), + ); + let values = vars(&block); + assert!(values.contains(&r"T3CODE_HOME=D:\t3\.t3".to_owned())); + assert!(values.contains(&r"PATH=C:\Windows".to_owned())); + } + + #[test] + fn overrides_a_conflicting_ambient_home_case_insensitively() { + let block = build_environment_block( + &ambient(&[ + ("t3code_home", r"C:\Users\someone else\.t3"), + ("PATH", r"C:\Windows"), + ]), + &PathBuf::from(r"D:\t3\.t3"), + ); + let values = vars(&block); + assert_eq!( + values + .iter() + .filter(|value| value.to_ascii_lowercase().starts_with("t3code_home=")) + .count(), + 1 + ); + assert!(values.contains(&r"T3CODE_HOME=D:\t3\.t3".to_owned())); + } + + #[test] + fn preserves_unicode_values_and_terminates_the_block() { + let block = build_environment_block( + &ambient(&[("GREETING", "\u{4f60}\u{597d}")]), + &PathBuf::from(r"D:\t3\.t3"), + ); + assert_eq!(block.last(), Some(&0)); + let entries = vars(&block); + assert!( + entries + .iter() + .any(|value| value == "GREETING=\u{4f60}\u{597d}") + ); + } + + #[test] + fn sorts_names_case_insensitively() { + let block = build_environment_block( + &ambient(&[("zeta", "1"), ("Alpha", "2")]), + &PathBuf::from(r"D:\t3\.t3"), + ); + let entries = vars(&block); + let alpha = entries + .iter() + .position(|value| value.starts_with("Alpha=")) + .unwrap(); + let home = entries + .iter() + .position(|value| value.starts_with("T3CODE_HOME=")) + .unwrap(); + let zeta = entries + .iter() + .position(|value| value.starts_with("zeta=")) + .unwrap(); + assert!(alpha < home && home < zeta); + } +} diff --git a/native/windows-service-host/src/host.rs b/native/windows-service-host/src/host.rs index ae0dfc67a603..764d5cb55151 100644 --- a/native/windows-service-host/src/host.rs +++ b/native/windows-service-host/src/host.rs @@ -31,6 +31,31 @@ pub enum SpawnError { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct QueryError; +/// Outcome of an explicit termination attempt against an owned process tree. +/// +/// `Confirmed` and `Failed` are the two outcomes of an *attempt*; `Unknown` and +/// `Refused` mean no termination was attempted at all. A bounded supervisor exit +/// must never map `Unknown`/`Refused`/`Failed` to a clean stop. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CleanupOutcome { + /// Termination was attempted and the owned process was observed to exit. + Confirmed, + /// Termination was attempted but not confirmed: an API call failed or the + /// bounded wait timed out. + Failed, + /// Identity could not be verified, so nothing was terminated. + Unknown, + /// The recorded identity was verified as foreign; deliberately not terminated. + Refused, +} + +impl CleanupOutcome { + /// A confirmed stop is the only outcome that may be reported as clean. + pub fn is_clean(self) -> bool { + matches!(self, CleanupOutcome::Confirmed) + } +} + pub trait ChildHandle { fn identity(&self) -> ProcessIdentity; /// `Ok(true)` means the recorded identity is still ours, `Ok(false)` means @@ -41,7 +66,8 @@ pub trait ChildHandle { /// stop marker the launcher reads; the production launcher adaptation /// (documented in the scoped design) is what makes that marker actionable. fn request_graceful_stop(&mut self) -> Result<(), QueryError>; - fn terminate_tree(&mut self); + /// Force the owned tree down and report whether its exit was confirmed. + fn terminate_tree(&mut self) -> CleanupOutcome; } pub trait ChildHost { @@ -157,9 +183,14 @@ impl ChildHandle for CommandChild { std::fs::write(&self.stop_marker, b"").map_err(|_| QueryError) } - fn terminate_tree(&mut self) { + fn terminate_tree(&mut self) -> CleanupOutcome { + // Kill first; the subsequent wait both reaps the child and confirms the + // exit. A failed kill of an already-exiting child still confirms via wait. let _ = self.child.kill(); - let _ = self.child.wait(); + match self.child.wait() { + Ok(_) => CleanupOutcome::Confirmed, + Err(_) => CleanupOutcome::Failed, + } } } diff --git a/native/windows-service-host/src/lib.rs b/native/windows-service-host/src/lib.rs index fcdd75a9e5b2..456431b0ec8f 100644 --- a/native/windows-service-host/src/lib.rs +++ b/native/windows-service-host/src/lib.rs @@ -10,8 +10,11 @@ //! stop/drain state machine are portable so they can be unit tested on any //! developer host that has no Windows toolchain. +pub mod account; +pub mod admission; pub mod config; pub mod control; +pub mod environment; pub mod host; pub mod run; pub mod supervise; @@ -21,4 +24,5 @@ pub mod windows; pub use config::{Invocation, LaunchMode, ServiceConfig}; pub use control::{Control, ControlOutcome, ServiceState}; +pub use host::CleanupOutcome; pub use supervise::{ExitCode, IdentityVerdict, Supervisor, SupervisorAction}; diff --git a/native/windows-service-host/src/main.rs b/native/windows-service-host/src/main.rs index 10ae492cc17f..c8d592b626d8 100644 --- a/native/windows-service-host/src/main.rs +++ b/native/windows-service-host/src/main.rs @@ -13,7 +13,7 @@ use std::thread; use t3_windows_service_host::config::{Invocation, ServiceConfig, parse}; use t3_windows_service_host::control::{Control, ServiceState}; use t3_windows_service_host::host::CommandChildHost; -use t3_windows_service_host::run::{ChannelControlInput, Reporter, run}; +use t3_windows_service_host::run::{ChannelControlInput, PublishError, Reporter, run}; use t3_windows_service_host::supervise::ExitCode; fn main() -> StdExitCode { @@ -64,8 +64,14 @@ fn run_service(_config: ServiceConfig) -> ExitCode { struct ConsoleReporter; impl Reporter for ConsoleReporter { - fn report(&mut self, state: ServiceState, exit: ExitCode, checkpoint: u32) { + fn report( + &mut self, + state: ServiceState, + exit: ExitCode, + checkpoint: u32, + ) -> Result<(), PublishError> { println!("[t3-service] state={state:?} exit={exit:?} checkpoint={checkpoint}"); + Ok(()) } } diff --git a/native/windows-service-host/src/run.rs b/native/windows-service-host/src/run.rs index 70d0e09ca7b4..36248974cab1 100644 --- a/native/windows-service-host/src/run.rs +++ b/native/windows-service-host/src/run.rs @@ -6,13 +6,17 @@ //! verifies that the recorded process identity is still the one this host //! spawned. A failed query leaves ownership unknown and is never read as a //! successful stop. +//! +//! The `Reporter` is the actual SCM/runtime status boundary, so the loop treats +//! publication as fallible: a failed `report` is surfaced, not ignored, and the +//! run never claims a clean stop when its cleanup could not be confirmed. use std::sync::mpsc; use std::time::{Duration, Instant}; use crate::config::ServiceConfig; use crate::control::{Control, ServiceState}; -use crate::host::{ChildHandle, ChildHost, SpawnError}; +use crate::host::{ChildHandle, ChildHost, CleanupOutcome, SpawnError}; use crate::supervise::{ExitCode, IdentityVerdict, Supervisor, SupervisorAction, identity_verdict}; pub trait ControlInput { @@ -45,8 +49,20 @@ impl ControlInput for ScriptedControl { } } +/// A status publication failure. `win32_error` is the `GetLastError` value the +/// SCM reporter observed; tests use a synthetic value. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct PublishError { + pub win32_error: u32, +} + pub trait Reporter { - fn report(&mut self, state: ServiceState, exit: ExitCode, checkpoint: u32); + fn report( + &mut self, + state: ServiceState, + exit: ExitCode, + checkpoint: u32, + ) -> Result<(), PublishError>; } pub struct RunOutcome { @@ -55,6 +71,9 @@ pub struct RunOutcome { /// True when the exit came from observing the child, not from a forced /// termination. Useful to keep "stop completed" honest in logs. pub exit_observed: bool, + /// True when at least one status publication failed. The failure is + /// surfaced here and the run does not claim a clean stop. + pub publish_failed: bool, } pub fn run( @@ -74,12 +93,14 @@ where let mut supervisor = Supervisor::new(config.clone()); let mut child: Option = None; let mut exit_observed = false; + let mut publish_failed = false; + let mut aborting = false; let mut queue = supervisor.begin(now()); loop { while !queue.is_empty() { let actions = std::mem::take(&mut queue); - queue = execute( + let (follow_up, failed) = execute( actions, config, &mut supervisor, @@ -89,7 +110,23 @@ where now(), log, ); + queue = follow_up; + publish_failed |= failed; + } + + // A failed publication means the SCM no longer tracks our true state + // (a failed RUNNING report advertises no stop controls). Abandon the run + // once, cleaning the owned child, and report an unknown cause. + if publish_failed && !aborting && !supervisor.finished() { + aborting = true; + log("status publication failed; abandoning the run"); + let cleanup = terminate_child::(&mut child); + child = None; + log(&format!("abandon cleanup outcome: {cleanup:?}")); + queue = supervisor.on_publish_failure(); + continue; } + if supervisor.finished() { break; } @@ -119,6 +156,19 @@ where exit: supervisor.exit_code(), forced: supervisor.forced(), exit_observed, + publish_failed, + } +} + +/// Terminate the held child if its identity is still verified-owned. +fn terminate_child(child: &mut Option) -> CleanupOutcome { + match child.as_mut() { + Some(handle) => match identity_verdict(handle.verify_identity()) { + IdentityVerdict::Owned => handle.terminate_tree(), + IdentityVerdict::Foreign => CleanupOutcome::Refused, + IdentityVerdict::Unknown => CleanupOutcome::Unknown, + }, + None => CleanupOutcome::Confirmed, } } @@ -132,15 +182,16 @@ fn execute( reporter: &mut R, now: Duration, log: &mut dyn FnMut(&str), -) -> Vec { +) -> (Vec, bool) { let mut follow_up = Vec::new(); + let mut publish_failed = false; for action in actions { match action { SupervisorAction::SpawnChild => match host.spawn(config) { Ok(spawned) => { let identity = spawned.identity(); *child = Some(spawned); - supervisor.on_child_spawned(identity); + follow_up.extend(supervisor.on_child_spawned(identity)); } Err(SpawnError::Config(message)) => { log(&format!("refusing to launch: {message}")); @@ -165,24 +216,30 @@ fn execute( log("process identity unknown; not assuming the child stopped") } }, - None => follow_up.extend(supervisor.on_forced_termination()), + None => { + follow_up.extend(supervisor.on_forced_termination(CleanupOutcome::Confirmed)) + } }, SupervisorAction::ForceTerminateTree => { - if let Some(handle) = child.as_mut() { - match identity_verdict(handle.verify_identity()) { - IdentityVerdict::Owned => handle.terminate_tree(), - _ => log("refusing to terminate an unverified process tree"), - } - } + let cleanup = terminate_child::(child); + // Releasing the held job handle is itself a termination effect + // for a kill-on-close job; the outcome above is what we report, + // not the implicit drop. *child = None; - follow_up.extend(supervisor.on_forced_termination()); + follow_up.extend(supervisor.on_forced_termination(cleanup)); } SupervisorAction::Report { state, exit } => { - reporter.report(state, exit, supervisor.checkpoint()); + if let Err(error) = reporter.report(state, exit, supervisor.checkpoint()) { + log(&format!( + "status publication failed (win32 {}); service state is not represented", + error.win32_error + )); + publish_failed = true; + } } } } - follow_up + (follow_up, publish_failed) } #[cfg(test)] @@ -192,6 +249,7 @@ mod tests { use crate::host::{ChildHandle, ProcessIdentity, QueryError, SpawnError}; use std::collections::VecDeque; use std::path::PathBuf; + use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; type Events = Arc>>; @@ -201,7 +259,26 @@ mod tests { alive: bool, owned: bool, query_fails: bool, + graceful_kills: bool, + terminate_outcome: CleanupOutcome, events: Events, + unrelated_alive: Arc, + } + + impl Drop for FakeChild { + fn drop(&mut self) { + debug_assert!( + self.unrelated_alive.load(Ordering::SeqCst), + "host cleanup must never touch an unrelated process" + ); + // Closing a kill-on-close job terminates its owned members. A held + // child dropped while still alive is therefore still a termination + // effect, even when no terminate method was called. + if self.alive { + self.events.lock().unwrap().push("job-drop".to_owned()); + self.alive = false; + } + } } impl ChildHandle for FakeChild { @@ -229,22 +306,28 @@ mod tests { } fn request_graceful_stop(&mut self) -> Result<(), QueryError> { self.events.lock().unwrap().push("graceful".to_owned()); + if self.graceful_kills { + self.alive = false; + } Ok(()) } - fn terminate_tree(&mut self) { + fn terminate_tree(&mut self) -> CleanupOutcome { self.events.lock().unwrap().push("terminate".to_owned()); - self.alive = false; + if self.terminate_outcome.is_clean() { + self.alive = false; + } + self.terminate_outcome } } struct FakeHost { - child: FakeChild, + template: FakeChild, } impl ChildHost for FakeHost { type Child = FakeChild; fn spawn(&mut self, _config: &ServiceConfig) -> Result { - Ok(self.child.clone()) + Ok(self.template.clone()) } } @@ -253,11 +336,41 @@ mod tests { } impl Reporter for Recorder { - fn report(&mut self, state: ServiceState, exit: ExitCode, _checkpoint: u32) { + fn report( + &mut self, + state: ServiceState, + exit: ExitCode, + _checkpoint: u32, + ) -> Result<(), PublishError> { self.events .lock() .unwrap() .push(format!("report:{state:?}:{exit:?}")); + Ok(()) + } + } + + struct FailingReporter { + fail_on: ServiceState, + events: Events, + } + + impl Reporter for FailingReporter { + fn report( + &mut self, + state: ServiceState, + exit: ExitCode, + _checkpoint: u32, + ) -> Result<(), PublishError> { + self.events + .lock() + .unwrap() + .push(format!("report:{state:?}:{exit:?}")); + if state == self.fail_on { + Err(PublishError { win32_error: 1066 }) + } else { + Ok(()) + } } } @@ -267,7 +380,7 @@ mod tests { runtime: PathBuf::new(), log: None, service_name: "t3code".to_owned(), - drain_timeout: Duration::from_millis(20), + drain_timeout: Duration::ZERO, restart_window: Duration::from_secs(300), max_restarts: 3, poll_interval: Duration::from_millis(1), @@ -277,23 +390,48 @@ mod tests { } } - fn contains(events: &Events, needle: &str) -> bool { - events - .lock() - .unwrap() - .iter() - .any(|event| event.contains(needle)) + fn child(options: ChildOptions, events: &Events) -> FakeChild { + FakeChild { + alive: options.alive, + owned: options.owned, + query_fails: options.query_fails, + graceful_kills: options.graceful_kills, + terminate_outcome: options.terminate_outcome, + events: events.clone(), + unrelated_alive: options.unrelated_alive, + } } - fn run_with(child: FakeChild) -> (Events, RunOutcome) { + #[derive(Clone)] + struct ChildOptions { + alive: bool, + owned: bool, + query_fails: bool, + graceful_kills: bool, + terminate_outcome: CleanupOutcome, + unrelated_alive: Arc, + } + + impl Default for ChildOptions { + fn default() -> Self { + Self { + alive: true, + owned: true, + query_fails: false, + graceful_kills: false, + terminate_outcome: CleanupOutcome::Confirmed, + unrelated_alive: Arc::new(AtomicBool::new(true)), + } + } + } + + fn run_events(options: ChildOptions, controls: Vec>) -> (Events, RunOutcome) { let events: Events = Arc::new(Mutex::new(Vec::new())); - let child = FakeChild { - events: events.clone(), - ..child + let mut host = FakeHost { + template: child(options, &events), }; - let mut host = FakeHost { child }; let mut controls = ScriptedControl { - script: VecDeque::from([Some(Control::Stop)]), + script: VecDeque::from(controls), }; let mut recorder = Recorder { events: events.clone(), @@ -303,15 +441,95 @@ mod tests { (events, outcome) } + fn contains(events: &Events, needle: &str) -> bool { + events + .lock() + .unwrap() + .iter() + .any(|event| event.contains(needle)) + } + + fn trace(events: &Events) -> Vec { + events.lock().unwrap().clone() + } + + fn stop_and_force() -> Vec> { + vec![None, Some(Control::Stop)] + } + #[test] - fn foreign_pid_is_reported_stopped_but_never_terminated() { - let (events, _outcome) = run_with(FakeChild { - alive: true, + fn successful_spawn_publishes_running_before_stop() { + let (events, _outcome) = run_events(ChildOptions::default(), stop_and_force()); + let trace = trace(&events); + let running = trace + .iter() + .position(|event| event == "report:Running:Clean") + .expect("a successful spawn must publish RUNNING"); + let stopping = trace + .iter() + .position(|event| event == "report:StopPending:Clean") + .expect("a stop must publish STOP_PENDING"); + assert!(running < stopping, "RUNNING must precede the stop"); + } + + #[test] + fn there_is_exactly_one_final_stopped_report() { + let (events, _outcome) = run_events(ChildOptions::default(), stop_and_force()); + let trace = trace(&events); + let stopped = trace + .iter() + .filter(|event| event.starts_with("report:Stopped")) + .count(); + assert_eq!( + stopped, 1, + "SCM requires exactly one final STOPPED: {trace:?}" + ); + let last_report = trace + .iter() + .filter(|event| event.starts_with("report:")) + .next_back() + .expect("at least one report"); + assert!(last_report.starts_with("report:Stopped")); + } + + #[test] + fn graceful_child_exit_publishes_running_then_one_stopped() { + let options = ChildOptions { + graceful_kills: true, + ..ChildOptions::default() + }; + let (events, outcome) = run_events(options, stop_and_force()); + assert!(contains(&events, "report:Running:Clean")); + let trace = trace(&events); + assert_eq!( + trace + .iter() + .filter(|event| event.starts_with("report:Stopped")) + .count(), + 1 + ); + assert_eq!(outcome.exit, ExitCode::Clean); + assert!(!outcome.publish_failed); + } + + #[test] + fn owned_tree_receives_graceful_stop_then_confirmed_termination() { + let (events, outcome) = run_events(ChildOptions::default(), stop_and_force()); + assert!(contains(&events, "graceful")); + assert!(contains(&events, "terminate")); + assert!(contains(&events, "report:Stopped:Clean")); + assert!(outcome.forced); + } + + #[test] + fn foreign_pid_is_not_terminated_and_not_reported_clean() { + let options = ChildOptions { owned: false, - query_fails: false, - events: Arc::new(Mutex::new(Vec::new())), - }); - assert!(contains(&events, "report:Stopped"), "service must not hang"); + ..ChildOptions::default() + }; + let (events, outcome) = run_events(options, stop_and_force()); + assert!(contains(&events, "report:Stopped:Unknown")); + assert!(!outcome.publish_failed); assert!( !contains(&events, "terminate"), "a foreign process must not be terminated" @@ -320,17 +538,37 @@ mod tests { !contains(&events, "graceful"), "a foreign process must not receive a stop request" ); + // Dropping the still-held kill-on-close job is itself an effect, so the + // test records it rather than treating "no terminate call" as clean. + assert!( + contains(&events, "job-drop"), + "the held job drop must be observable, not hidden" + ); + } + + #[test] + fn a_refused_tree_does_not_touch_unrelated_processes() { + let unrelated_alive = Arc::new(AtomicBool::new(true)); + let options = ChildOptions { + owned: false, + unrelated_alive: unrelated_alive.clone(), + ..ChildOptions::default() + }; + let (_events, _outcome) = run_events(options, stop_and_force()); + assert!( + unrelated_alive.load(Ordering::SeqCst), + "no unrelated process may be cleaned up" + ); } #[test] fn query_failure_is_not_treated_as_an_owned_tree() { - let (events, _outcome) = run_with(FakeChild { - alive: true, - owned: true, + let options = ChildOptions { query_fails: true, - events: Arc::new(Mutex::new(Vec::new())), - }); - assert!(contains(&events, "report:Stopped"), "service must not hang"); + ..ChildOptions::default() + }; + let (events, _outcome) = run_events(options, stop_and_force()); + assert!(contains(&events, "report:Stopped:Unknown")); assert!( !contains(&events, "terminate"), "an unverifiable tree must not be terminated" @@ -338,16 +576,51 @@ mod tests { } #[test] - fn owned_tree_receives_graceful_stop_then_bounded_termination() { - let (events, outcome) = run_with(FakeChild { - alive: true, - owned: true, - query_fails: false, - events: Arc::new(Mutex::new(Vec::new())), - }); - assert!(contains(&events, "graceful")); - assert!(contains(&events, "terminate")); - assert!(contains(&events, "report:Stopped")); - assert!(outcome.forced); + fn failed_termination_is_not_reported_clean() { + let options = ChildOptions { + terminate_outcome: CleanupOutcome::Failed, + ..ChildOptions::default() + }; + let (events, outcome) = run_events(options, stop_and_force()); + assert!(contains(&events, "report:Stopped:Unknown")); + assert_ne!(outcome.exit, ExitCode::Clean); + assert!(!outcome.exit_observed, "no clean child exit was observed"); + } + + #[test] + fn publish_failure_is_surfaced_and_does_not_claim_clean() { + let events: Events = Arc::new(Mutex::new(Vec::new())); + let options = ChildOptions::default(); + let mut host = FakeHost { + template: child(options, &events), + }; + let mut controls = ScriptedControl { + script: VecDeque::from(stop_and_force()), + }; + let mut reporter = FailingReporter { + fail_on: ServiceState::Running, + events: events.clone(), + }; + let mut log = |_message: &str| {}; + let outcome = run(&config(), &mut host, &mut controls, &mut reporter, &mut log); + + assert!( + outcome.publish_failed, + "the failed RUNNING report must surface" + ); + assert_ne!( + outcome.exit, + ExitCode::Clean, + "no clean stop without publication" + ); + let trace = trace(&events); + assert_eq!( + trace + .iter() + .filter(|event| event.starts_with("report:Stopped")) + .count(), + 1, + "abort must still end in exactly one STOPPED: {trace:?}" + ); } } diff --git a/native/windows-service-host/src/supervise.rs b/native/windows-service-host/src/supervise.rs index a86599010428..c5ae99d79efa 100644 --- a/native/windows-service-host/src/supervise.rs +++ b/native/windows-service-host/src/supervise.rs @@ -20,7 +20,7 @@ use std::time::Duration; use crate::config::ServiceConfig; use crate::control::{Control, ControlOutcome, ServiceState, handle_control}; -use crate::host::{ProcessIdentity, QueryError}; +use crate::host::{CleanupOutcome, ProcessIdentity, QueryError}; /// Monotonic time since the host started. pub type Monotonic = Duration; @@ -154,9 +154,17 @@ impl Supervisor { ] } - pub fn on_child_spawned(&mut self, identity: ProcessIdentity) { + /// The child started. The real SCM is still `START_PENDING` at this point: + /// the internal state change is not enough, so this returns the actual + /// `RUNNING` report the runner must publish. Without it the SCM never advertises + /// STOP/SHUTDOWN controls. + pub fn on_child_spawned(&mut self, identity: ProcessIdentity) -> Vec { self.identity = Some(identity); self.state = ServiceState::Running; + vec![SupervisorAction::Report { + state: ServiceState::Running, + exit: ExitCode::Clean, + }] } /// The runner could not spawn the child. `fatal` marks a configuration @@ -244,11 +252,26 @@ impl Supervisor { }] } - /// After a forced termination the runner has no exit event to feed. Finish - /// the stop directly. - pub fn on_forced_termination(&mut self) -> Vec { + /// After a forced termination the runner has no exit event to feed. Finish the + /// stop, but only claim a clean shutdown when the cleanup was confirmed. + /// `Failed`, `Unknown` and `Refused` are a bounded exit without evidence, not + /// a clean stop. + pub fn on_forced_termination(&mut self, cleanup: CleanupOutcome) -> Vec { + self.identity = None; + let code = if cleanup.is_clean() { + ExitCode::Clean + } else { + ExitCode::Unknown + }; + self.finish(code) + } + + /// A status publication failed, so the SCM no longer knows the true state. + /// Finish with an unknown cause: even a confirmed child cleanup is not a + /// clean service stop when the SCM was never told the service was running. + pub fn on_publish_failure(&mut self) -> Vec { self.identity = None; - self.finish(ExitCode::Clean) + self.finish(ExitCode::Unknown) } fn try_restart(&mut self, now: Monotonic) -> bool { @@ -329,9 +352,54 @@ mod tests { assert!(reported(&actions, ServiceState::StartPending)); assert_eq!(spawn_count(&actions), 1); - supervisor.on_child_spawned(identity(42)); + let actions = supervisor.on_child_spawned(identity(42)); assert_eq!(supervisor.state(), ServiceState::Running); assert!(supervisor.child_identity().is_some()); + assert!( + reported(&actions, ServiceState::Running), + "a successful spawn must publish the RUNNING transition" + ); + } + + #[test] + fn restart_publishes_running_again() { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + supervisor.on_child_spawned(identity(42)); + supervisor.on_child_exited(ms(1_000), 7); // unexpected, budget allows restart + let actions = supervisor.on_child_spawned(identity(43)); + assert!(reported(&actions, ServiceState::Running)); + assert!(!supervisor.finished()); + } + + #[test] + fn cleanup_outcomes_map_to_honest_exit_codes() { + for (cleanup, expected) in [ + (CleanupOutcome::Confirmed, ExitCode::Clean), + (CleanupOutcome::Failed, ExitCode::Unknown), + (CleanupOutcome::Unknown, ExitCode::Unknown), + (CleanupOutcome::Refused, ExitCode::Unknown), + ] { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + supervisor.on_child_spawned(identity(42)); + supervisor.on_control(ms(100), Control::Stop); + let actions = supervisor.on_forced_termination(cleanup); + assert!(reported(&actions, ServiceState::Stopped)); + assert_eq!(supervisor.exit_code(), expected, "cleanup {cleanup:?}"); + assert!(supervisor.finished()); + } + } + + #[test] + fn a_publish_failure_finishes_with_an_unknown_cause() { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + supervisor.on_child_spawned(identity(42)); + let actions = supervisor.on_publish_failure(); + assert!(reported(&actions, ServiceState::Stopped)); + assert_eq!(supervisor.exit_code(), ExitCode::Unknown); + assert!(supervisor.finished()); } #[test] @@ -436,7 +504,7 @@ mod tests { let actions = supervisor.tick(ms(deadline + 1_000)); assert!(!actions.contains(&SupervisorAction::ForceTerminateTree)); - let actions = supervisor.on_forced_termination(); + let actions = supervisor.on_forced_termination(CleanupOutcome::Confirmed); assert!(reported(&actions, ServiceState::Stopped)); assert!(supervisor.forced()); assert_eq!(supervisor.exit_code(), ExitCode::Clean); diff --git a/native/windows-service-host/src/windows/job.rs b/native/windows-service-host/src/windows/job.rs index ef70ac4b214a..d8619eba3e72 100644 --- a/native/windows-service-host/src/windows/job.rs +++ b/native/windows-service-host/src/windows/job.rs @@ -1,11 +1,13 @@ //! Windows process-tree ownership through a job object. //! //! The child is created suspended, assigned to a job with -//! `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`, and then resumed. Suspending closes -//! the race where a fast child could spawn grandchildren before the job -//! assignment and escape termination. Creating the job here (rather than -//! putting this host in one) means only the descendants of the launcher are -//! owned; the host stays alive to report `SERVICE_STOPPED`. +//! `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`, its identity captured, and then it is +//! resumed. Suspending closes the race where a fast child could spawn +//! grandchildren before assignment and escape termination. Every step after a +//! successful `CreateProcessW` is checked and unwound: a failure reclaims the +//! freshly created process rather than leaving a suspended orphan. Creating the +//! job here (rather than putting this host in one) means only the descendants of +//! the launcher are owned; the host stays alive to report `SERVICE_STOPPED`. //! //! `verify_identity` re-opens the PID and compares its creation time with the //! one recorded at spawn. A handle we already hold stays valid after the child @@ -29,15 +31,21 @@ use windows_sys::Win32::System::JobObjects::{ SetInformationJobObject, TerminateJobObject, }; use windows_sys::Win32::System::Threading::{ - CREATE_NEW_PROCESS_GROUP, CREATE_SUSPENDED, CreateProcessW, GetExitCodeProcess, - GetProcessTimes, OpenProcess, PROCESS_INFORMATION, PROCESS_QUERY_LIMITED_INFORMATION, - ResumeThread, STARTF_USESTDHANDLES, STARTUPINFOW, WaitForSingleObject, + CREATE_NEW_PROCESS_GROUP, CREATE_SUSPENDED, CREATE_UNICODE_ENVIRONMENT, CreateProcessW, + GetExitCodeProcess, GetProcessTimes, OpenProcess, PROCESS_INFORMATION, + PROCESS_QUERY_LIMITED_INFORMATION, ResumeThread, STARTF_USESTDHANDLES, STARTUPINFOW, + TerminateProcess, WaitForSingleObject, }; +use crate::admission::{AdmissionFailure, AdmissionOps, AdmissionStage, admit}; use crate::config::{LaunchMode, ServiceConfig}; -use crate::host::{ChildHandle, ChildHost, ProcessIdentity, QueryError, SpawnError}; +use crate::host::{ + ChildHandle, ChildHost, CleanupOutcome, ProcessIdentity, QueryError, SpawnError, +}; const GENERIC_WRITE: u32 = 0x4000_0000; +/// Bounded wait for an explicitly terminated process to be observed as gone. +const TERMINATE_WAIT_MS: u32 = 5_000; struct Handle(HANDLE); @@ -68,6 +76,10 @@ pub struct WindowsChild { job: Handle, id: ProcessIdentity, stop_marker: std::path::PathBuf, + /// True once an explicit termination was confirmed. While false, dropping + /// this holder still closes a kill-on-close job, which is itself a + /// termination effect; `Drop` makes that effect explicit and bounded. + terminated: bool, } impl WindowsChild { @@ -85,6 +97,21 @@ impl WindowsChild { } } +impl Drop for WindowsChild { + fn drop(&mut self) { + if !self.terminated { + // Releasing a kill-on-close job terminates its members. Make that + // implicit termination explicit and bounded here rather than + // pretending no termination happened. Only this job's owned members + // are affected; no PID or process name is matched. + unsafe { + TerminateJobObject(self.job.0, 1); + let _ = WaitForSingleObject(self.process.0, TERMINATE_WAIT_MS); + } + } + } +} + impl ChildHandle for WindowsChild { fn identity(&self) -> ProcessIdentity { self.id @@ -126,12 +153,89 @@ impl ChildHandle for WindowsChild { std::fs::write(&self.stop_marker, b"").map_err(|_| QueryError) } - fn terminate_tree(&mut self) { - unsafe { - TerminateJobObject(self.job.0, 1); + fn terminate_tree(&mut self) -> CleanupOutcome { + let terminated = unsafe { TerminateJobObject(self.job.0, 1) }; + if terminated == 0 { + return CleanupOutcome::Failed; + } + match unsafe { WaitForSingleObject(self.process.0, TERMINATE_WAIT_MS) } { + WAIT_OBJECT_0 => { + self.terminated = true; + CleanupOutcome::Confirmed + } + _ => CleanupOutcome::Failed, + } + } +} + +/// A process created suspended but not yet admitted. It owns the created +/// process and thread handles until cleanup has been confirmed; the job is +/// already created, so assignment is the first admission step. +struct PendingProcess { + process: Handle, + thread: Handle, + job: Handle, + pid: u32, +} + +impl AdmissionOps for PendingProcess { + fn assign_to_job(&mut self) -> Result<(), String> { + let ok = unsafe { AssignProcessToJobObject(self.job.0, self.process.0) }; + if ok == 0 { + Err(format!("AssignProcessToJobObject failed ({})", unsafe { + GetLastError() + })) + } else { + Ok(()) + } + } + + fn capture_identity(&mut self) -> Result { + match WindowsChild::creation_time(self.process.0) { + Some(created_at_ms) => Ok(ProcessIdentity { + pid: self.pid, + created_at_ms, + }), + None => Err(format!( + "GetProcessTimes failed ({}); identity is unknown, not zero", + unsafe { GetLastError() } + )), + } + } + + fn resume(&mut self) -> Result<(), String> { + let previous = unsafe { ResumeThread(self.thread.0) }; + if previous == u32::MAX { + Err(format!("ResumeThread failed ({})", unsafe { + GetLastError() + })) + } else { + Ok(()) } - let _ = unsafe { WaitForSingleObject(self.process.0, 5_000) }; } + + fn terminate_created(&mut self) -> CleanupOutcome { + let terminated = unsafe { TerminateProcess(self.process.0, 1) }; + if terminated == 0 { + return CleanupOutcome::Failed; + } + match unsafe { WaitForSingleObject(self.process.0, TERMINATE_WAIT_MS) } { + WAIT_OBJECT_0 => CleanupOutcome::Confirmed, + _ => CleanupOutcome::Failed, + } + } +} + +fn describe_admission_failure(failure: &AdmissionFailure) -> String { + let stage = match failure.stage { + AdmissionStage::AssignToJob => "AssignProcessToJobObject", + AdmissionStage::CaptureIdentity => "GetProcessTimes", + AdmissionStage::Resume => "ResumeThread", + }; + format!( + "{stage}: {} (created-process cleanup: {:?})", + failure.reason, failure.cleanup + ) } impl ChildHost for WindowsChildHost { @@ -170,6 +274,10 @@ impl ChildHost for WindowsChildHost { } let mut command_line = build_command_line(config); + // Bind the native child to the selected home. Passing NULL here would + // inherit an ambient T3CODE_HOME (absent or pointing elsewhere); cwd is + // not what the pinned launcher reads. + let environment = crate::environment::host_environment(&config.home); let mut startup: STARTUPINFOW = unsafe { std::mem::zeroed() }; startup.cb = std::mem::size_of::() as u32; @@ -222,8 +330,8 @@ impl ChildHost for WindowsChildHost { std::ptr::null(), std::ptr::null(), inherit as i32, - CREATE_SUSPENDED | CREATE_NEW_PROCESS_GROUP, - std::ptr::null(), + CREATE_SUSPENDED | CREATE_NEW_PROCESS_GROUP | CREATE_UNICODE_ENVIRONMENT, + environment.as_ptr() as *const core::ffi::c_void, home_wide.as_ptr(), &startup, &mut info, @@ -237,31 +345,27 @@ impl ChildHost for WindowsChildHost { } // The child has inherited its own copy of the log handle; close ours. drop(log_handle); - let process = Handle(info.hProcess); - let thread = Handle(info.hThread); - let assigned = unsafe { AssignProcessToJobObject(job.0, process.0) }; - if assigned == 0 { - let error = unsafe { GetLastError() }; - unsafe { - TerminateJobObject(job.0, 1); - } - return Err(SpawnError::Launch(format!( - "AssignProcessToJobObject failed ({error})" - ))); - } - - unsafe { ResumeThread(thread.0) }; + let mut pending = PendingProcess { + process: Handle(info.hProcess), + thread: Handle(info.hThread), + job, + pid: info.dwProcessId, + }; + let id = match admit(&mut pending) { + Ok(id) => id, + Err(failure) => return Err(SpawnError::Launch(describe_admission_failure(&failure))), + }; - let created_at_ms = WindowsChild::creation_time(process.0).unwrap_or(0); + let process = pending.process; + let job = pending.job; + drop(pending.thread); Ok(WindowsChild { process, job, - id: ProcessIdentity { - pid: info.dwProcessId, - created_at_ms, - }, + id, stop_marker: config.stop_marker(), + terminated: false, }) } } diff --git a/native/windows-service-host/src/windows/service.rs b/native/windows-service-host/src/windows/service.rs index 7430a3e7a3de..2a4fc036a850 100644 --- a/native/windows-service-host/src/windows/service.rs +++ b/native/windows-service-host/src/windows/service.rs @@ -15,15 +15,19 @@ use std::sync::atomic::{AtomicIsize, AtomicU32, Ordering}; use std::sync::mpsc; use windows_sys::Win32::Foundation::GetLastError; +use windows_sys::Win32::Security::Authentication::Identity::{ + EXTENDED_NAME_FORMAT, GetUserNameExW, NameSamCompatible, NameUserPrincipal, +}; use windows_sys::Win32::System::Services::{ RegisterServiceCtrlHandlerExW, SERVICE_STATUS, SERVICE_STATUS_HANDLE, SERVICE_TABLE_ENTRYW, SetServiceStatus, StartServiceCtrlDispatcherW, }; use super::job::WindowsChildHost; +use crate::account; use crate::config::ServiceConfig; use crate::control::{Control, ServiceState}; -use crate::run::{ChannelControlInput, Reporter, run}; +use crate::run::{ChannelControlInput, PublishError, Reporter, run}; use crate::supervise::ExitCode; const SERVICE_WIN32_OWN_PROCESS: u32 = 0x0000_0010; @@ -109,16 +113,39 @@ unsafe extern "system" fn service_main(_argc: u32, _argv: *mut *mut u16) { shared .status_handle .store(handle as isize, Ordering::SeqCst); - emit_status(shared); + if let Err(error) = emit_status(shared) { + append_log( + config.log.as_ref(), + &format!( + "could not publish the initial status ({})", + error.win32_error + ), + ); + let _ = SETTLED.set(ExitCode::Unknown); + return; + } let mut host = WindowsChildHost::new(); let mut controls = ChannelControlInput { receiver }; let mut reporter = ScmReporter; let log_path = config.log.clone(); let mut log = |message: &str| append_log(log_path.as_ref(), message); + // `run` publishes the single final STOPPED itself. Reporting STOPPED here as + // well would be a second status update after the SCM may already have + // released this service's context (Microsoft SetServiceStatus contract). let outcome = run(config, &mut host, &mut controls, &mut reporter, &mut log); - set_status(ServiceState::Stopped, outcome.exit, 0); - let _ = SETTLED.set(outcome.exit); + if outcome.publish_failed { + append_log( + log_path.as_ref(), + "a status publication failed; the SCM may not have observed the final state", + ); + } + let settled = if outcome.publish_failed { + ExitCode::Unknown + } else { + outcome.exit + }; + let _ = SETTLED.set(settled); } unsafe extern "system" fn control_handler( @@ -133,7 +160,9 @@ unsafe extern "system" fn control_handler( // Unbounded channel: this never blocks the SCM thread. let _ = shared.sender.send(Control::from_win32(control)); } - Control::Interrogate => emit_status(shared), + Control::Interrogate => { + let _ = emit_status(shared); + } Control::Other => {} } NO_ERROR @@ -142,27 +171,32 @@ unsafe extern "system" fn control_handler( struct ScmReporter; impl Reporter for ScmReporter { - fn report(&mut self, state: ServiceState, exit: ExitCode, checkpoint: u32) { - set_status(state, exit, checkpoint); + fn report( + &mut self, + state: ServiceState, + exit: ExitCode, + checkpoint: u32, + ) -> Result<(), PublishError> { + set_status(state, exit, checkpoint) } } -fn set_status(state: ServiceState, exit: ExitCode, checkpoint: u32) { +fn set_status(state: ServiceState, exit: ExitCode, checkpoint: u32) -> Result<(), PublishError> { let Some(shared) = SHARED.get() else { - return; + return Err(PublishError { win32_error: 0 }); }; shared.state.store(state.win32_state(), Ordering::SeqCst); let (win32, specific) = exit.win32(); shared.win32_exit.store(win32, Ordering::SeqCst); shared.specific_exit.store(specific, Ordering::SeqCst); shared.checkpoint.store(checkpoint, Ordering::SeqCst); - emit_status(shared); + emit_status(shared) } -fn emit_status(shared: &Shared) { +fn emit_status(shared: &Shared) -> Result<(), PublishError> { let raw = shared.status_handle.load(Ordering::SeqCst); if raw == 0 { - return; + return Err(PublishError { win32_error: 0 }); } let state = shared.state.load(Ordering::SeqCst); let pending = state == ServiceState::StartPending.win32_state() @@ -180,55 +214,58 @@ fn emit_status(shared: &Shared) { dwCheckPoint: shared.checkpoint.load(Ordering::SeqCst), dwWaitHint: if pending { WAIT_HINT_MS } else { 0 }, }; - unsafe { SetServiceStatus(raw as SERVICE_STATUS_HANDLE, &mut status) }; + let accepted = unsafe { SetServiceStatus(raw as SERVICE_STATUS_HANDLE, &mut status) }; + if accepted == 0 { + return Err(PublishError { + win32_error: unsafe { GetLastError() }, + }); + } + Ok(()) } -/// Refuse the default LocalSystem workload and enforce an explicit expected +/// Refuse the default LocalSystem workload and enforce a qualified expected /// account. The account password is never here: SCM stores it in LSA and /// starts the process under that token. fn check_service_account(config: &ServiceConfig) -> Result<(), String> { - let account = current_user_name().ok_or_else(|| { - "could not read the service account; refusing to start without an identity".to_owned() - })?; - if !config.allow_local_system && account.eq_ignore_ascii_case("SYSTEM") { + let sam = qualified_user_name(NameSamCompatible); + let upn = qualified_user_name(NameUserPrincipal); + if sam.is_none() && upn.is_none() { + return Err( + "could not read a qualified service account identity; refusing to start".to_owned(), + ); + } + if !config.allow_local_system && account::is_local_system(sam.as_deref()) { return Err( "refusing to run the T3 workload as LocalSystem; register the service with a dedicated account" .to_owned(), ); } if let Some(expected) = &config.expected_account { - let matches = account.eq_ignore_ascii_case(expected) - || account - .rsplit('\\') - .next() - .is_some_and(|name| name.eq_ignore_ascii_case(expected)); - if !matches { + if !account::qualified_match(expected, sam.as_deref(), upn.as_deref()) { + let observed = sam.or(upn).unwrap_or_else(|| "unknown".to_owned()); return Err(format!( - "service account '{account}' does not match the expected account '{expected}'" + "service account '{observed}' does not match the expected account '{expected}'" )); } } Ok(()) } -fn current_user_name() -> Option { +/// A qualified account name from `GetUserNameExW`. The bare-name API is not +/// used: without a domain qualifier it cannot prove the account binding. +fn qualified_user_name(format: EXTENDED_NAME_FORMAT) -> Option { let mut size: u32 = 0; unsafe { - // First call sizes the buffer; the expected ERROR_INSUFFICIENT_BUFFER is - // not an error for this probe. - windows_sys::Win32::System::WindowsProgramming::GetUserNameW( - std::ptr::null_mut(), - &mut size, - ); + // First call sizes the buffer; the expected ERROR_MORE_DATA is not an + // error for this probe. + GetUserNameExW(format, std::ptr::null_mut(), &mut size); } if size == 0 { return None; } let mut buffer = vec![0u16; size as usize]; - let ok = unsafe { - windows_sys::Win32::System::WindowsProgramming::GetUserNameW(buffer.as_mut_ptr(), &mut size) - }; - if ok == 0 { + let ok = unsafe { GetUserNameExW(format, buffer.as_mut_ptr(), &mut size) }; + if !ok { return None; } let end = buffer diff --git a/native/windows-service-host/tests/portable_host.rs b/native/windows-service-host/tests/portable_host.rs index 14a777f8360a..9f3065f72741 100644 --- a/native/windows-service-host/tests/portable_host.rs +++ b/native/windows-service-host/tests/portable_host.rs @@ -15,7 +15,7 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use t3_windows_service_host::config::{LaunchMode, ServiceConfig}; use t3_windows_service_host::control::{Control, ServiceState}; use t3_windows_service_host::host::CommandChildHost; -use t3_windows_service_host::run::{Reporter, ScriptedControl, run}; +use t3_windows_service_host::run::{PublishError, Reporter, ScriptedControl, run}; use t3_windows_service_host::supervise::ExitCode; #[derive(Clone)] @@ -24,11 +24,17 @@ struct Capturing { } impl Reporter for Capturing { - fn report(&mut self, state: ServiceState, exit: ExitCode, _checkpoint: u32) { + fn report( + &mut self, + state: ServiceState, + exit: ExitCode, + _checkpoint: u32, + ) -> Result<(), PublishError> { self.events .lock() .unwrap() .push(format!("{state:?}:{exit:?}")); + Ok(()) } } From f9dc35e333ce58aaaa07b5817b2b686f8e2d4ee8 Mon Sep 17 00:00:00 2001 From: nullStack65 Date: Sat, 26 Sep 2026 23:33:07 -0400 Subject: [PATCH 4/7] test(service): prove the production command line portably and harden the subprocess test Move the Windows command-line quoting into a portable module so the exact production launch request ("" "__service-launcher", spaces, trailing backslashes, embedded quotes and Unicode) is asserted on the developer host. Bound the portable subprocess test's PID read so its child's asynchronous write cannot race the assertion. --- .../windows-service-host/src/command_line.rs | 89 +++++++++++++++++++ native/windows-service-host/src/lib.rs | 1 + .../windows-service-host/src/windows/job.rs | 40 +-------- .../tests/portable_host.rs | 20 +++-- 4 files changed, 108 insertions(+), 42 deletions(-) create mode 100644 native/windows-service-host/src/command_line.rs diff --git a/native/windows-service-host/src/command_line.rs b/native/windows-service-host/src/command_line.rs new file mode 100644 index 000000000000..64a046f3a95c --- /dev/null +++ b/native/windows-service-host/src/command_line.rs @@ -0,0 +1,89 @@ +//! Windows command-line construction for the launched child. +//! +//! `CreateProcessW` receives a single command-line string, not an argv array, so +//! the host must apply the Windows quoting rules itself. The pinned launcher is +//! passed as `"" "__service-launcher"`, and paths may contain spaces or +//! Unicode. Keeping this portable lets the exact production launch request be +//! asserted on a developer host without a Windows toolchain. + +/// Build a NUL-terminated command line from a UTF-16 program and arguments. +pub fn build_command_line(program: &[u16], args: &[Vec]) -> Vec { + let mut line = Vec::new(); + push_quoted(&mut line, program); + for arg in args { + line.push(u16::from(b' ')); + push_quoted(&mut line, arg); + } + line.push(0); + line +} + +/// Quote one argument: backslashes are doubled before a closing quote, and a +/// literal quote is escaped as `2n + 1` backslashes followed by the quote. +fn push_quoted(out: &mut Vec, value: &[u16]) { + out.push(u16::from(b'"')); + let mut backslashes = 0usize; + for &character in value { + if character == u16::from(b'\\') { + backslashes += 1; + out.push(character); + } else if character == u16::from(b'"') { + for _ in 0..backslashes + 1 { + out.push(u16::from(b'\\')); + } + out.push(character); + backslashes = 0; + } else { + backslashes = 0; + out.push(character); + } + } + for _ in 0..backslashes { + out.push(u16::from(b'\\')); + } + out.push(u16::from(b'"')); +} + +#[cfg(test)] +mod tests { + use super::*; + + fn utf16(value: &str) -> Vec { + value.encode_utf16().collect() + } + + fn render(line: &[u16]) -> String { + String::from_utf16_lossy(&line[..line.len() - 1]) + } + + #[test] + fn quotes_the_program_and_launcher_subcommand() { + let line = build_command_line(&utf16(r"C:\t3\t3.exe"), &[utf16("__service-launcher")]); + assert_eq!(render(&line), r#""C:\t3\t3.exe" "__service-launcher""#); + assert_eq!(line.last(), Some(&0)); + } + + #[test] + fn keeps_spaces_inside_one_argument() { + let line = build_command_line(&utf16(r"C:\Program Files\t3\t3.exe"), &[]); + assert_eq!(render(&line), r#""C:\Program Files\t3\t3.exe""#); + } + + #[test] + fn doubles_trailing_backslashes_before_the_closing_quote() { + let line = build_command_line(&utf16(r"C:\t3\"), &[]); + assert_eq!(render(&line), r#""C:\t3\\""#); + } + + #[test] + fn escapes_embedded_quotes() { + let line = build_command_line(&utf16("a\"b"), &[]); + assert_eq!(render(&line), r#""a\"b""#); + } + + #[test] + fn preserves_unicode_arguments() { + let line = build_command_line(&utf16("t3.exe"), &[utf16("\u{4f60}\u{597d}")]); + assert_eq!(render(&line), "\"t3.exe\" \"\u{4f60}\u{597d}\""); + } +} diff --git a/native/windows-service-host/src/lib.rs b/native/windows-service-host/src/lib.rs index 456431b0ec8f..0e991d369c00 100644 --- a/native/windows-service-host/src/lib.rs +++ b/native/windows-service-host/src/lib.rs @@ -12,6 +12,7 @@ pub mod account; pub mod admission; +pub mod command_line; pub mod config; pub mod control; pub mod environment; diff --git a/native/windows-service-host/src/windows/job.rs b/native/windows-service-host/src/windows/job.rs index d8619eba3e72..673aac2ca319 100644 --- a/native/windows-service-host/src/windows/job.rs +++ b/native/windows-service-host/src/windows/job.rs @@ -386,41 +386,7 @@ fn wide_null(value: &Path) -> Vec { fn build_command_line(config: &ServiceConfig) -> Vec { let (program, args) = config.child_command(); - let mut line = Vec::new(); - push_quoted( - &mut line, - &program.as_os_str().encode_wide().collect::>(), - ); - for arg in &args { - line.push(b' ' as u16); - push_quoted(&mut line, &arg.encode_wide().collect::>()); - } - line.push(0); - line -} - -/// Windows command-line quoting: backslashes are doubled before a quote, and a -/// literal quote needs `2n + 1` backslashes in front of it. -fn push_quoted(out: &mut Vec, value: &[u16]) { - out.push(b'"' as u16); - let mut backslashes = 0usize; - for &character in value { - if character == b'\\' as u16 { - backslashes += 1; - out.push(character); - } else if character == b'"' as u16 { - for _ in 0..backslashes + 1 { - out.push(b'\\' as u16); - } - out.push(character); - backslashes = 0; - } else { - backslashes = 0; - out.push(character); - } - } - for _ in 0..backslashes { - out.push(b'\\' as u16); - } - out.push(b'"' as u16); + let program: Vec = program.as_os_str().encode_wide().collect(); + let args: Vec> = args.iter().map(|arg| arg.encode_wide().collect()).collect(); + crate::command_line::build_command_line(&program, &args) } diff --git a/native/windows-service-host/tests/portable_host.rs b/native/windows-service-host/tests/portable_host.rs index 9f3065f72741..6aa173f356dd 100644 --- a/native/windows-service-host/tests/portable_host.rs +++ b/native/windows-service-host/tests/portable_host.rs @@ -46,6 +46,19 @@ fn unique_dir() -> PathBuf { std::env::temp_dir().join(format!("t3-winsvc-{}-{nanos}", std::process::id())) } +fn read_pid_within(path: &Path, timeout: Duration) -> Option { + let deadline = std::time::Instant::now() + timeout; + while std::time::Instant::now() < deadline { + if let Ok(text) = std::fs::read_to_string(path) { + if let Ok(pid) = text.trim().parse::() { + return Some(pid); + } + } + std::thread::sleep(Duration::from_millis(5)); + } + None +} + fn write_runtime(home: &Path) -> PathBuf { use std::os::unix::fs::PermissionsExt; let runtime = home.join("t3.exe"); @@ -108,11 +121,8 @@ fn real_child_is_stopped_and_terminated_after_drain() { "child output must be redirected to the configured log" ); - let pid: i32 = std::fs::read_to_string(home.join("child.pid")) - .unwrap() - .trim() - .parse() - .unwrap(); + let pid = read_pid_within(&home.join("child.pid"), Duration::from_millis(500)) + .expect("the dummy child records its pid"); let alive = std::process::Command::new("/bin/sh") .arg("-c") .arg(format!("kill -0 {pid} 2>/dev/null")) From 6ab38b23f6266691c06f5c4a0967f289529c587e Mon Sep 17 00:00:00 2001 From: nullStack65 Date: Sat, 26 Sep 2026 23:36:36 -0400 Subject: [PATCH 5/7] docs(service): state that the account password never passes on the command line --- native/windows-service-host/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/native/windows-service-host/src/main.rs b/native/windows-service-host/src/main.rs index c8d592b626d8..9ff765591e9f 100644 --- a/native/windows-service-host/src/main.rs +++ b/native/windows-service-host/src/main.rs @@ -145,8 +145,8 @@ OPTIONS: --console Run against a terminal, not SCM (development only). --help, --version -The service account and its password are registered with SCM (`sc.exe create -... obj= ...`), never passed here.", +The service account is registered with SCM out of band (`sc.exe create ... obj= +...`); its password, if any, stays in LSA and is never passed here.", version = env!("CARGO_PKG_VERSION") ); } From dac8a3dab7652a4ad674a250034208f99789ea7e Mon Sep 17 00:00:00 2001 From: nullStack65 Date: Sun, 27 Sep 2026 03:00:39 -0400 Subject: [PATCH 6/7] fix(service): stop retrying unconfirmed admission cleanup; confirm whole-job cleanup Carry the admission cleanup outcome structurally (SpawnError::Admission) instead of flattening it into a launch string. run only takes the ordinary bounded retry when the created process was confirmed reclaimed; a failed, unknown or timed-out reclaim finishes non-retrying with a recovery-required cause so a second child cannot leak behind an unresolved process. The created handles are retained through the explicit cleanup decision. Treat a root-process exit as ownership evidence only for that root. Before a natural exit may finish clean or start a replacement, WindowsChild reclaims the owned job through the retained job handle and confirms emptiness with job accounting (QueryInformationJobObject/ActiveProcesses) under a bounded drain. TerminateJobObject is a request, so a still-nonempty job, an ineffective request or a failed query is never Confirmed. Drop stays a last-resort safeguard, not evidence. --- docs/internals/windows-background-service.md | 68 ++++-- native/windows-service-host/src/admission.rs | 18 ++ native/windows-service-host/src/host.rs | 23 ++ native/windows-service-host/src/run.rs | 212 +++++++++++++++++- native/windows-service-host/src/supervise.rs | 83 ++++++- .../windows-service-host/src/windows/job.rs | 110 ++++++--- 6 files changed, 457 insertions(+), 57 deletions(-) diff --git a/docs/internals/windows-background-service.md b/docs/internals/windows-background-service.md index ac76457773d8..244cc9f0330a 100644 --- a/docs/internals/windows-background-service.md +++ b/docs/internals/windows-background-service.md @@ -88,10 +88,16 @@ report `SERVICE_STOPPED`. Every step after `CreateProcessW` succeeds is checked and scoped: assignment to the job, creation-time identity capture, and `ResumeThread`. If any of them fails, the freshly created process is terminated explicitly — the created -process handle is retained until that cleanup outcome is known — and the -failure is reported together with the cleanup result. A suspended child is never -left orphaned outside the job, and a failed creation-time query is reported as -unknown rather than a fabricated zero identity. +process handle is retained until that cleanup outcome is known — and the failure +is reported together with the cleanup result. The cleanup outcome is carried +structurally (`SpawnError::Admission`), not flattened into a string: only a +*confirmed* reclaim may take the ordinary bounded launch retry. A failed, +unknown or timed-out reclaim — for example assignment failure plus a failed or +timed-out `TerminateProcess` — stops without a second spawn and reports a +recovery-required cause, because starting another child could leak a second +process outside the job while the real ownership is unresolved. A suspended +child is never silently orphaned outside the job, and a failed creation-time +query is reported as unknown rather than a fabricated zero identity. A stop is bounded and two-stage: @@ -107,19 +113,37 @@ A stop is bounded and two-stage: than as evidence of a clean shutdown; the job only ever contains this host's owned members, so no PID or process name is matched. +Whole-job completion is not the same as the root process exiting. Before a +natural root exit can finish clean or start a replacement, the host explicitly +reclaims the owned job through the retained job handle and confirms it is empty. +`TerminateJobObject` is only a *request*; emptiness is proven with job +accounting (`QueryInformationJobObject` / `JobObjectBasicAccountingInformation`, +`ActiveProcesses`), with a bounded drain poll. A still-nonempty job, a +termination request that does not take effect, or a failed job query is never +`Confirmed`: a root exit with a surviving grandchild, an unexpected exit, or a +planned stop all report `STOPPED` with a recovery-required cause instead of a +clean stop, and no replacement child is started over an uncleared tree. The +retained process/job handles are the ownership authority here; the host never +re-opens a possibly-reused PID for this cleanup. `Drop` remains only a +last-resort safeguard that closes the job; it is never the source of a +`Confirmed` result. + This is deliberately different from a normal launcher replacement. During an update the launcher terminates its own server child but stays alive and starts the replacement; the host must not touch the job during that handoff. The host only terminates the tree for a whole-service stop. It does not know about launcher protocol upgrades and does not manage updates or rollback. -Ownership is verified before any stop or termination: the host re-opens the -recorded PID and compares its creation time with the one captured at spawn, -because a held handle stays valid after the child exits and cannot detect PID -reuse. `Ok(false)` means the PID is foreign and is left alone; a failed query -means ownership is **unknown** and is never read as a successful stop. Only a -verified-owned tree is stopped or terminated, so a stale or foreign PID is -never cleaned up. +For a graceful stop request and a forced termination while the root may still be +alive, ownership is verified first: the host re-opens the recorded PID and +compares its creation time with the one captured at spawn, because a held handle +stays valid after the child exits and cannot detect PID reuse. `Ok(false)` means +the PID is foreign and is left alone; a failed query means ownership is +**unknown** and is never read as a successful stop. Whole-job cleanup after the +root has already exited instead uses the retained job handle, which only ever +contains this host's assigned members; a stale or foreign PID is never the +target. Only owned members and confirmed-empty job accounting may be reported as +a clean stop. ## Required launcher control (not implemented here) @@ -151,16 +175,23 @@ fallback or relies solely on the control message. - **Unexpected exit:** the supervisor restarts the child while a restart budget allows it (`--max-restarts` inside `--restart-window-ms`, default 5 in 300s, - mirroring the systemd unit), reporting `START_PENDING` between attempts. + mirroring the systemd unit), reporting `START_PENDING` between attempts. The + restart only happens when the whole owned job was confirmed empty first. - **Planned stop:** a child exit while `STOP_PENDING` is success, not a failure; a non-zero exit code is ignored so a crashed-but-stopping child does not look - like an unexpected stop. + like an unexpected stop. An unconfirmed whole-tree cleanup during a planned + stop is not reported clean. - **Repeated failure:** once the budget is exhausted the service stops with `ERROR_SERVICE_SPECIFIC_ERROR` and a specific code instead of respawning forever. There is no Windows analog of systemd's finite start limit, so the budget lives here. - **Slow drain:** reports `STOP_PENDING` with checkpoints and forces the tree after the deadline. It never hangs. +- **Unconfirmed cleanup (specific code 5):** a created process that could not be + admitted and could not be confirmed reclaimed, or an owned job that stayed + nonempty (or could not be queried) after termination, stops the service with a + recovery-required specific code. The host does not retry and does not claim a + clean stop; it reports the unresolved ownership rather than promising removal. - **Publication failure:** if `SetServiceStatus` fails for any state, the failure is surfaced (not ignored); the host abandons the run, cleans the owned child once and reports an unknown cause rather than claiming a clean stop. @@ -237,12 +268,15 @@ Native acceptance (not executed here; see the recipe): 1. `sc.exe query` reports `STOPPED` before start and `RUNNING` after; the control handler answers interrogate without hanging. 2. A planned stop reaches `STOPPED` within the drain bound; the job tree is - empty afterwards. -3. Killing the dummy child produces a bounded restart sequence, then a specific - failure code; no restart storm. + empty afterwards, including any grandchild. +3. Killing the dummy root while its grandchild is still alive does **not** + restart over the survivor: the host reclaims the owned job, confirms it + empty, and only then reports `STOPPED` (or a recovery-required specific code + if the job cannot be confirmed empty); no restart storm. 4. A child that ignores the stop marker is force-terminated at the deadline and reported `STOPPED`. -5. A stale PID and an unrelated process are never terminated. +5. A stale PID and an unrelated process are never terminated, and a failed job + query is reported as unknown rather than confirmed empty. 6. Registration and cleanup remove exactly the synthetic service, its processes, its home under the disposable test root, and nothing else. diff --git a/native/windows-service-host/src/admission.rs b/native/windows-service-host/src/admission.rs index de798df60484..c0d802167087 100644 --- a/native/windows-service-host/src/admission.rs +++ b/native/windows-service-host/src/admission.rs @@ -168,4 +168,22 @@ mod tests { let failure = admit(&mut ops).unwrap_err(); assert_eq!(failure.cleanup, CleanupOutcome::Failed); } + + #[test] + fn a_timed_out_or_unknown_cleanup_is_reported_with_the_failure() { + for cleanup in [CleanupOutcome::Failed, CleanupOutcome::Unknown] { + let mut ops = FakeOps { + fail_resume: true, + cleanup: Some(cleanup), + ..FakeOps::default() + }; + let failure = admit(&mut ops).unwrap_err(); + assert_eq!(failure.stage, AdmissionStage::Resume); + assert!( + ops.terminated, + "the created process is always reclaimed explicitly first" + ); + assert_eq!(failure.cleanup, cleanup); + } + } } diff --git a/native/windows-service-host/src/host.rs b/native/windows-service-host/src/host.rs index 764d5cb55151..456743e6f9f1 100644 --- a/native/windows-service-host/src/host.rs +++ b/native/windows-service-host/src/host.rs @@ -9,6 +9,7 @@ use std::fs::OpenOptions; use std::path::PathBuf; use std::process::{Child, Command, Stdio}; +use crate::admission::AdmissionFailure; use crate::config::{LaunchMode, ServiceConfig}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -26,6 +27,10 @@ pub enum SpawnError { Config(String), /// A transient launch failure. Launch(String), + /// The child was created but could not be admitted. The cleanup outcome is + /// carried structurally so the supervisor can decide whether a retry is + /// safe: a confirmed reclaim may retry, an unconfirmed one must not. + Admission(AdmissionFailure), } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -68,6 +73,11 @@ pub trait ChildHandle { fn request_graceful_stop(&mut self) -> Result<(), QueryError>; /// Force the owned tree down and report whether its exit was confirmed. fn terminate_tree(&mut self) -> CleanupOutcome; + /// The root process has been observed to exit. Reclaim any remaining members + /// of the owned tree through the retained handle and report whether the whole + /// owned set is confirmed empty. The retained handle is the ownership + /// authority, so this does not re-verify a possibly-reused PID. + fn cleanup_after_exit(&mut self) -> CleanupOutcome; } pub trait ChildHost { @@ -192,6 +202,19 @@ impl ChildHandle for CommandChild { Err(_) => CleanupOutcome::Failed, } } + + fn cleanup_after_exit(&mut self) -> CleanupOutcome { + // `try_wait` already reaped the root. The portable host has no job object + // to observe for grandchildren, so a reaped root is the limit of what it + // can confirm; it never claims more than that. + match self.child.try_wait() { + Ok(Some(_)) => CleanupOutcome::Confirmed, + // Called only after the root was observed to exit; still running is + // not something this host can call confirmed-empty. + Ok(None) => CleanupOutcome::Unknown, + Err(_) => CleanupOutcome::Unknown, + } + } } #[cfg(test)] diff --git a/native/windows-service-host/src/run.rs b/native/windows-service-host/src/run.rs index 36248974cab1..129a1ad7f153 100644 --- a/native/windows-service-host/src/run.rs +++ b/native/windows-service-host/src/run.rs @@ -139,9 +139,16 @@ where if let Some(handle) = child.as_mut() { match handle.try_wait() { Ok(Some(code)) => { + // The root exiting is not proof the whole owned tree is gone. + // Reclaim the job through its retained handle and use that + // evidence before finishing clean or starting a replacement. + let cleanup = handle.cleanup_after_exit(); + log(&format!( + "child exited (code {code}); owned-tree cleanup: {cleanup:?}" + )); child = None; exit_observed = true; - queue = supervisor.on_child_exited(now(), code); + queue = supervisor.on_child_exited(now(), code, cleanup); continue; } Ok(None) => {} @@ -201,6 +208,25 @@ fn execute( log(&format!("launch failed: {message}")); follow_up.extend(supervisor.on_spawn_failed(now, false)); } + Err(SpawnError::Admission(failure)) => { + log(&format!( + "admission failed at {:?}: {} (created-process cleanup: {:?})", + failure.stage, failure.reason, failure.cleanup + )); + if failure.cleanup.is_clean() { + // The created process was confirmed reclaimed, so this is + // the ordinary bounded transient-launch retry. + follow_up.extend(supervisor.on_spawn_failed(now, false)); + } else { + // The created process may still exist outside the job. + // Starting another could leak a second orphan, so stop + // and report the unresolved ownership instead. + log( + "admission cleanup could not be confirmed; not retrying and reporting unresolved ownership", + ); + follow_up.extend(supervisor.on_admission_cleanup_unconfirmed()); + } + } }, SupervisorAction::RequestGracefulStop => match child.as_mut() { Some(handle) => match identity_verdict(handle.verify_identity()) { @@ -261,6 +287,7 @@ mod tests { query_fails: bool, graceful_kills: bool, terminate_outcome: CleanupOutcome, + after_exit_outcome: CleanupOutcome, events: Events, unrelated_alive: Arc, } @@ -318,6 +345,16 @@ mod tests { } self.terminate_outcome } + fn cleanup_after_exit(&mut self) -> CleanupOutcome { + self.events + .lock() + .unwrap() + .push("cleanup-after-exit".to_owned()); + if self.after_exit_outcome.is_clean() { + self.alive = false; + } + self.after_exit_outcome + } } struct FakeHost { @@ -327,10 +364,36 @@ mod tests { impl ChildHost for FakeHost { type Child = FakeChild; fn spawn(&mut self, _config: &ServiceConfig) -> Result { + self.template + .events + .lock() + .unwrap() + .push("spawn".to_owned()); Ok(self.template.clone()) } } + /// A host whose spawn always fails admission, carrying the injected cleanup + /// outcome. Used to prove the spawn→run retry decision structurally. + struct AdmissionFailHost { + cleanup: CleanupOutcome, + events: Events, + attempts: usize, + } + + impl ChildHost for AdmissionFailHost { + type Child = FakeChild; + fn spawn(&mut self, _config: &ServiceConfig) -> Result { + self.attempts += 1; + self.events.lock().unwrap().push("spawn".to_owned()); + Err(SpawnError::Admission(crate::admission::AdmissionFailure { + stage: crate::admission::AdmissionStage::AssignToJob, + reason: "AssignProcessToJobObject failed (5)".to_owned(), + cleanup: self.cleanup, + })) + } + } + struct Recorder { events: Events, } @@ -397,6 +460,7 @@ mod tests { query_fails: options.query_fails, graceful_kills: options.graceful_kills, terminate_outcome: options.terminate_outcome, + after_exit_outcome: options.after_exit_outcome, events: events.clone(), unrelated_alive: options.unrelated_alive, } @@ -409,6 +473,7 @@ mod tests { query_fails: bool, graceful_kills: bool, terminate_outcome: CleanupOutcome, + after_exit_outcome: CleanupOutcome, unrelated_alive: Arc, } @@ -420,6 +485,7 @@ mod tests { query_fails: false, graceful_kills: false, terminate_outcome: CleanupOutcome::Confirmed, + after_exit_outcome: CleanupOutcome::Confirmed, unrelated_alive: Arc::new(AtomicBool::new(true)), } } @@ -453,6 +519,15 @@ mod tests { events.lock().unwrap().clone() } + fn spawn_count(events: &Events) -> usize { + events + .lock() + .unwrap() + .iter() + .filter(|event| event.as_str() == "spawn") + .count() + } + fn stop_and_force() -> Vec> { vec![None, Some(Control::Stop)] } @@ -587,6 +662,141 @@ mod tests { assert!(!outcome.exit_observed, "no clean child exit was observed"); } + #[test] + fn successful_termination_request_with_a_nonempty_job_is_not_clean() { + // `TerminateJobObject` is only a request; a job still nonempty after the + // bounded wait is a failed cleanup, never a clean stop. + let options = ChildOptions { + terminate_outcome: CleanupOutcome::Failed, + ..ChildOptions::default() + }; + let (events, outcome) = run_events(options, stop_and_force()); + assert!(contains(&events, "terminate")); + assert_ne!(outcome.exit, ExitCode::Clean); + assert!(contains(&events, "report:Stopped:Unknown")); + } + + #[test] + fn root_exit_with_a_living_grandchild_is_not_clean() { + // The root exited during a planned stop, but the job stayed nonempty, so + // whole-tree cleanup could not be confirmed. + let options = ChildOptions { + alive: false, + after_exit_outcome: CleanupOutcome::Failed, + ..ChildOptions::default() + }; + let (events, outcome) = run_events(options, vec![Some(Control::Stop), None]); + assert!( + contains(&events, "cleanup-after-exit"), + "the post-exit cleanup must be explicit" + ); + assert!(contains(&events, "report:Stopped:RecoveryRequired")); + assert_ne!(outcome.exit, ExitCode::Clean); + assert_eq!(spawn_count(&events), 1, "no replacement tree is started"); + } + + #[test] + fn natural_exit_during_a_planned_stop_is_clean_only_when_the_job_is_empty() { + let options = ChildOptions { + alive: false, + after_exit_outcome: CleanupOutcome::Confirmed, + ..ChildOptions::default() + }; + let (events, outcome) = run_events(options, vec![Some(Control::Stop), None]); + assert!(contains(&events, "report:Stopped:Clean")); + assert_eq!(outcome.exit, ExitCode::Clean); + } + + #[test] + fn unexpected_exit_with_an_unconfirmed_job_does_not_replace_the_child() { + let options = ChildOptions { + alive: false, + after_exit_outcome: CleanupOutcome::Unknown, + ..ChildOptions::default() + }; + let (events, outcome) = run_events(options, vec![None]); + assert_eq!(spawn_count(&events), 1, "the tree is not replaced"); + assert_eq!(outcome.exit, ExitCode::RecoveryRequired); + assert!(contains(&events, "report:Stopped:RecoveryRequired")); + } + + #[test] + fn job_query_failure_on_a_natural_exit_is_not_clean() { + let options = ChildOptions { + alive: false, + after_exit_outcome: CleanupOutcome::Unknown, + ..ChildOptions::default() + }; + let (events, outcome) = run_events(options, stop_and_force()); + assert_eq!(spawn_count(&events), 1); + assert_ne!(outcome.exit, ExitCode::Clean); + } + + #[test] + fn a_natural_exit_does_not_touch_an_unrelated_process() { + let unrelated_alive = Arc::new(AtomicBool::new(true)); + let options = ChildOptions { + alive: false, + after_exit_outcome: CleanupOutcome::Failed, + unrelated_alive: unrelated_alive.clone(), + ..ChildOptions::default() + }; + let _ = run_events(options, vec![Some(Control::Stop), None]); + assert!( + unrelated_alive.load(Ordering::SeqCst), + "job cleanup must never reach an unrelated process" + ); + } + + #[test] + fn unconfirmed_admission_cleanup_stops_without_a_second_spawn() { + // Fault-inject assignment failure plus a failed or unknown/timeout + // reclaim through the real spawn→run decision. + for cleanup in [CleanupOutcome::Failed, CleanupOutcome::Unknown] { + let events: Events = Arc::new(Mutex::new(Vec::new())); + let mut host = AdmissionFailHost { + cleanup, + events: events.clone(), + attempts: 0, + }; + let mut controls = ScriptedControl { + script: VecDeque::new(), + }; + let mut recorder = Recorder { + events: events.clone(), + }; + let mut log = |_message: &str| {}; + let outcome = run(&config(), &mut host, &mut controls, &mut recorder, &mut log); + assert_eq!( + host.attempts, 1, + "an unconfirmed admission cleanup ({cleanup:?}) must not retry" + ); + assert_eq!(outcome.exit, ExitCode::RecoveryRequired, "{cleanup:?}"); + assert!(contains(&events, "report:Stopped:RecoveryRequired")); + } + } + + #[test] + fn confirmed_admission_cleanup_takes_the_bounded_retry() { + let events: Events = Arc::new(Mutex::new(Vec::new())); + let mut host = AdmissionFailHost { + cleanup: CleanupOutcome::Confirmed, + events: events.clone(), + attempts: 0, + }; + let mut controls = ScriptedControl { + script: VecDeque::new(), + }; + let mut recorder = Recorder { + events: events.clone(), + }; + let mut log = |_message: &str| {}; + let outcome = run(&config(), &mut host, &mut controls, &mut recorder, &mut log); + // config() allows 3 restarts, so 1 initial attempt + 3 retries. + assert_eq!(host.attempts, 4); + assert_eq!(outcome.exit, ExitCode::RepeatedFailure); + } + #[test] fn publish_failure_is_surfaced_and_does_not_claim_clean() { let events: Events = Arc::new(Mutex::new(Vec::new())); diff --git a/native/windows-service-host/src/supervise.rs b/native/windows-service-host/src/supervise.rs index c5ae99d79efa..9dc91e7d1388 100644 --- a/native/windows-service-host/src/supervise.rs +++ b/native/windows-service-host/src/supervise.rs @@ -37,6 +37,9 @@ pub enum ExitCode { LaunchFailure, /// The cause could not be established. Unknown, + /// A created process or owned tree could not be confirmed reclaimed. The + /// host stops without retrying and reports that recovery is required. + RecoveryRequired, } impl ExitCode { @@ -50,6 +53,7 @@ impl ExitCode { ExitCode::RepeatedFailure => (ERROR_SERVICE_SPECIFIC_ERROR, 2), ExitCode::LaunchFailure => (ERROR_SERVICE_SPECIFIC_ERROR, 3), ExitCode::Unknown => (ERROR_SERVICE_SPECIFIC_ERROR, 4), + ExitCode::RecoveryRequired => (ERROR_SERVICE_SPECIFIC_ERROR, 5), } } } @@ -188,10 +192,30 @@ impl Supervisor { ] } + /// A freshly created process could not be admitted and its reclaim could not + /// be confirmed. Retrying could leak another orphan while the real ownership + /// is unresolved, so finish without a retry and report that recovery is + /// required. This is deliberately not the ordinary transient-launch retry. + pub fn on_admission_cleanup_unconfirmed(&mut self) -> Vec { + self.identity = None; + self.finish(ExitCode::RecoveryRequired) + } + /// The child exited. A `StopPending` exit is the planned stop completing; - /// anything else is unexpected and consumes the restart budget. - pub fn on_child_exited(&mut self, now: Monotonic, _code: i32) -> Vec { + /// anything else is unexpected and consumes the restart budget. `cleanup` is + /// the explicit whole-owned-tree result: the root exiting is not proof the + /// tree is empty, so an unconfirmed cleanup may neither finish clean nor + /// start a replacement. + pub fn on_child_exited( + &mut self, + now: Monotonic, + _code: i32, + cleanup: CleanupOutcome, + ) -> Vec { self.identity = None; + if !cleanup.is_clean() { + return self.finish(ExitCode::RecoveryRequired); + } if matches!(self.state, ServiceState::StopPending) { return self.finish(ExitCode::Clean); } @@ -366,7 +390,7 @@ mod tests { let mut supervisor = Supervisor::new(config()); supervisor.begin(ms(0)); supervisor.on_child_spawned(identity(42)); - supervisor.on_child_exited(ms(1_000), 7); // unexpected, budget allows restart + supervisor.on_child_exited(ms(1_000), 7, CleanupOutcome::Confirmed); // unexpected, budget allows restart let actions = supervisor.on_child_spawned(identity(43)); assert!(reported(&actions, ServiceState::Running)); assert!(!supervisor.finished()); @@ -413,7 +437,7 @@ mod tests { assert!(actions.contains(&SupervisorAction::RequestGracefulStop)); // The child exits on its own inside the drain window. - let actions = supervisor.on_child_exited(ms(200), 0); + let actions = supervisor.on_child_exited(ms(200), 0, CleanupOutcome::Confirmed); assert!(reported(&actions, ServiceState::Stopped)); assert!(supervisor.finished()); assert_eq!(supervisor.exit_code(), ExitCode::Clean); @@ -435,7 +459,7 @@ mod tests { supervisor.begin(ms(0)); supervisor.on_child_spawned(identity(42)); supervisor.on_control(ms(100), Control::Stop); - supervisor.on_child_exited(ms(200), 1); + supervisor.on_child_exited(ms(200), 1, CleanupOutcome::Confirmed); assert_eq!(supervisor.exit_code(), ExitCode::Clean); } @@ -444,7 +468,7 @@ mod tests { let mut supervisor = Supervisor::new(config()); supervisor.begin(ms(0)); supervisor.on_child_spawned(identity(42)); - let actions = supervisor.on_child_exited(ms(1_000), 7); + let actions = supervisor.on_child_exited(ms(1_000), 7, CleanupOutcome::Confirmed); assert_eq!(spawn_count(&actions), 1); assert!(reported(&actions, ServiceState::StartPending)); assert!(!supervisor.finished()); @@ -458,12 +482,13 @@ mod tests { // budget is 3 restarts inside the window for index in 0..3 { supervisor.on_child_spawned(identity(42)); - let actions = supervisor.on_child_exited(ms(1_000 + index), 7); + let actions = + supervisor.on_child_exited(ms(1_000 + index), 7, CleanupOutcome::Confirmed); assert_eq!(spawn_count(&actions), 1); assert!(!supervisor.finished()); } supervisor.on_child_spawned(identity(42)); - let actions = supervisor.on_child_exited(ms(2_000), 7); + let actions = supervisor.on_child_exited(ms(2_000), 7, CleanupOutcome::Confirmed); assert!(reported(&actions, ServiceState::Stopped)); assert!(supervisor.finished()); assert_eq!(supervisor.exit_code(), ExitCode::RepeatedFailure); @@ -475,11 +500,11 @@ mod tests { supervisor.begin(ms(0)); for index in 0..3 { supervisor.on_child_spawned(identity(42)); - supervisor.on_child_exited(ms(1_000 + index), 7); + supervisor.on_child_exited(ms(1_000 + index), 7, CleanupOutcome::Confirmed); } // Outside the 300s window the earlier restarts fall away. supervisor.on_child_spawned(identity(42)); - let actions = supervisor.on_child_exited(ms(500_000), 7); + let actions = supervisor.on_child_exited(ms(500_000), 7, CleanupOutcome::Confirmed); assert_eq!(spawn_count(&actions), 1); assert!(!supervisor.finished()); } @@ -516,7 +541,7 @@ mod tests { supervisor.begin(ms(0)); supervisor.on_child_spawned(identity(42)); supervisor.on_control(ms(100), Control::Stop); - supervisor.on_child_exited(ms(150), 0); + supervisor.on_child_exited(ms(150), 0, CleanupOutcome::Confirmed); assert!(supervisor.finished()); // A tick after the child is gone and the service is stopped is a no-op. @@ -562,5 +587,41 @@ mod tests { assert_eq!(ExitCode::Child(7).win32(), (1066, 1)); assert_eq!(ExitCode::RepeatedFailure.win32(), (1066, 2)); assert_eq!(ExitCode::LaunchFailure.win32(), (1066, 3)); + assert_eq!(ExitCode::Unknown.win32(), (1066, 4)); + assert_eq!(ExitCode::RecoveryRequired.win32(), (1066, 5)); + } + + #[test] + fn an_unconfirmed_post_exit_cleanup_is_recovery_required_not_clean_or_restarted() { + for cleanup in [CleanupOutcome::Failed, CleanupOutcome::Unknown] { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + supervisor.on_child_spawned(identity(42)); + // Unexpected exit, but the whole owned tree is not confirmed empty. + let actions = supervisor.on_child_exited(ms(1_000), 7, cleanup); + assert_eq!(spawn_count(&actions), 0, "no replacement after {cleanup:?}"); + assert!(reported(&actions, ServiceState::Stopped)); + assert_eq!(supervisor.exit_code(), ExitCode::RecoveryRequired); + + // A planned stop with an unconfirmed tree is not a clean stop either. + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + supervisor.on_child_spawned(identity(42)); + supervisor.on_control(ms(100), Control::Stop); + let actions = supervisor.on_child_exited(ms(200), 0, cleanup); + assert_eq!(spawn_count(&actions), 0); + assert_eq!(supervisor.exit_code(), ExitCode::RecoveryRequired); + } + } + + #[test] + fn an_unconfirmed_admission_cleanup_finishes_without_retrying() { + let mut supervisor = Supervisor::new(config()); + supervisor.begin(ms(0)); + let actions = supervisor.on_admission_cleanup_unconfirmed(); + assert_eq!(spawn_count(&actions), 0); + assert!(reported(&actions, ServiceState::Stopped)); + assert_eq!(supervisor.exit_code(), ExitCode::RecoveryRequired); + assert!(supervisor.finished()); } } diff --git a/native/windows-service-host/src/windows/job.rs b/native/windows-service-host/src/windows/job.rs index 673aac2ca319..5822ea7db445 100644 --- a/native/windows-service-host/src/windows/job.rs +++ b/native/windows-service-host/src/windows/job.rs @@ -16,6 +16,7 @@ use std::os::windows::ffi::OsStrExt; use std::path::Path; +use std::time::{Duration, Instant}; use windows_sys::Win32::Foundation::{ CloseHandle, FILETIME, GetLastError, HANDLE, HANDLE_FLAG_INHERIT, INVALID_HANDLE_VALUE, @@ -27,8 +28,9 @@ use windows_sys::Win32::Storage::FileSystem::{ }; use windows_sys::Win32::System::JobObjects::{ AssignProcessToJobObject, CreateJobObjectW, JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, - JOBOBJECT_EXTENDED_LIMIT_INFORMATION, JobObjectExtendedLimitInformation, - SetInformationJobObject, TerminateJobObject, + JOBOBJECT_BASIC_ACCOUNTING_INFORMATION, JOBOBJECT_EXTENDED_LIMIT_INFORMATION, + JobObjectBasicAccountingInformation, JobObjectExtendedLimitInformation, + QueryInformationJobObject, SetInformationJobObject, TerminateJobObject, }; use windows_sys::Win32::System::Threading::{ CREATE_NEW_PROCESS_GROUP, CREATE_SUSPENDED, CREATE_UNICODE_ENVIRONMENT, CreateProcessW, @@ -37,15 +39,17 @@ use windows_sys::Win32::System::Threading::{ TerminateProcess, WaitForSingleObject, }; -use crate::admission::{AdmissionFailure, AdmissionOps, AdmissionStage, admit}; +use crate::admission::{AdmissionOps, admit}; use crate::config::{LaunchMode, ServiceConfig}; use crate::host::{ ChildHandle, ChildHost, CleanupOutcome, ProcessIdentity, QueryError, SpawnError, }; const GENERIC_WRITE: u32 = 0x4000_0000; -/// Bounded wait for an explicitly terminated process to be observed as gone. +/// Bounded wait for an explicitly terminated process tree to be observed empty. const TERMINATE_WAIT_MS: u32 = 5_000; +/// Poll interval while waiting for the owned job to drain. +const JOB_DRAIN_POLL_MS: u64 = 20; struct Handle(HANDLE); @@ -95,6 +99,70 @@ impl WindowsChild { unsafe { GetProcessTimes(process, &mut creation, &mut exit, &mut kernel, &mut user) }; (ok != 0).then(|| ((creation.dwHighDateTime as u64) << 32) | creation.dwLowDateTime as u64) } + + /// Number of processes still active in the owned job. This is the whole-tree + /// membership evidence: a wait on the root process alone says nothing about + /// surviving descendants. + fn active_process_count(&self) -> Result { + let mut info: JOBOBJECT_BASIC_ACCOUNTING_INFORMATION = unsafe { std::mem::zeroed() }; + let ok = unsafe { + QueryInformationJobObject( + self.job.0, + JobObjectBasicAccountingInformation, + &mut info as *mut _ as *mut core::ffi::c_void, + std::mem::size_of::() as u32, + std::ptr::null_mut(), + ) + }; + if ok == 0 { + Err(QueryError) + } else { + Ok(info.ActiveProcesses) + } + } + + /// Poll job accounting until the owned set is empty or the bounded wait + /// expires. `TerminateJobObject` is only a request; membership is what proves + /// cleanup, so a still-nonempty job or a failed query is never `Confirmed`. + fn wait_for_empty_job(&self) -> CleanupOutcome { + let deadline = Instant::now() + Duration::from_millis(TERMINATE_WAIT_MS as u64); + loop { + match self.active_process_count() { + Ok(0) => return CleanupOutcome::Confirmed, + Ok(_) if Instant::now() >= deadline => return CleanupOutcome::Failed, + Ok(_) => std::thread::sleep(Duration::from_millis(JOB_DRAIN_POLL_MS)), + Err(_) => return CleanupOutcome::Unknown, + } + } + } + + /// Request termination of the whole owned job, then confirm emptiness. Used + /// while the root may still be alive. + fn terminate_owned_job(&mut self) -> CleanupOutcome { + let terminated = unsafe { TerminateJobObject(self.job.0, 1) }; + if terminated == 0 { + return CleanupOutcome::Failed; + } + let outcome = self.wait_for_empty_job(); + if outcome.is_clean() { + self.terminated = true; + } + outcome + } + + /// Reclaim a job whose root has already exited. The retained job handle — not + /// a re-opened PID — is the ownership authority, so a surviving grandchild is + /// terminated and confirmed through accounting rather than a root-only wait. + fn reclaim_after_root_exit(&mut self) -> CleanupOutcome { + match self.active_process_count() { + Ok(0) => { + self.terminated = true; + CleanupOutcome::Confirmed + } + Ok(_) => self.terminate_owned_job(), + Err(_) => CleanupOutcome::Unknown, + } + } } impl Drop for WindowsChild { @@ -154,17 +222,11 @@ impl ChildHandle for WindowsChild { } fn terminate_tree(&mut self) -> CleanupOutcome { - let terminated = unsafe { TerminateJobObject(self.job.0, 1) }; - if terminated == 0 { - return CleanupOutcome::Failed; - } - match unsafe { WaitForSingleObject(self.process.0, TERMINATE_WAIT_MS) } { - WAIT_OBJECT_0 => { - self.terminated = true; - CleanupOutcome::Confirmed - } - _ => CleanupOutcome::Failed, - } + self.terminate_owned_job() + } + + fn cleanup_after_exit(&mut self) -> CleanupOutcome { + self.reclaim_after_root_exit() } } @@ -226,18 +288,6 @@ impl AdmissionOps for PendingProcess { } } -fn describe_admission_failure(failure: &AdmissionFailure) -> String { - let stage = match failure.stage { - AdmissionStage::AssignToJob => "AssignProcessToJobObject", - AdmissionStage::CaptureIdentity => "GetProcessTimes", - AdmissionStage::Resume => "ResumeThread", - }; - format!( - "{stage}: {} (created-process cleanup: {:?})", - failure.reason, failure.cleanup - ) -} - impl ChildHost for WindowsChildHost { type Child = WindowsChild; @@ -354,7 +404,11 @@ impl ChildHost for WindowsChildHost { }; let id = match admit(&mut pending) { Ok(id) => id, - Err(failure) => return Err(SpawnError::Launch(describe_admission_failure(&failure))), + // `pending` stays alive until this point: its process, thread and job + // handles must survive the explicit cleanup decision `admit` made. + // The cleanup outcome travels structurally so the supervisor never + // treats an unconfirmed reclaim as an ordinary retryable launch. + Err(failure) => return Err(SpawnError::Admission(failure)), }; let process = pending.process; From 7455a7b6c12f01644b874bdf9f790846f6d88a59 Mon Sep 17 00:00:00 2001 From: nullStack65 Date: Sun, 27 Sep 2026 03:06:54 -0400 Subject: [PATCH 7/7] test(service): exercise the real admit decision in the spawn to run retry proof --- native/windows-service-host/src/run.rs | 34 ++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/native/windows-service-host/src/run.rs b/native/windows-service-host/src/run.rs index 129a1ad7f153..390de9c83ec5 100644 --- a/native/windows-service-host/src/run.rs +++ b/native/windows-service-host/src/run.rs @@ -374,23 +374,47 @@ mod tests { } /// A host whose spawn always fails admission, carrying the injected cleanup - /// outcome. Used to prove the spawn→run retry decision structurally. + /// outcome through the real `admit` decision. Used to prove the spawn→run + /// retry decision structurally rather than by hand-building the failure. struct AdmissionFailHost { cleanup: CleanupOutcome, events: Events, attempts: usize, } + /// Fails the first admission step; `terminate_created` returns the injected + /// reclaim outcome (for example a failed termination or a bounded timeout). + struct FailingAdmission { + cleanup: CleanupOutcome, + } + + impl crate::admission::AdmissionOps for FailingAdmission { + fn assign_to_job(&mut self) -> Result<(), String> { + Err("AssignProcessToJobObject failed (5)".to_owned()) + } + fn capture_identity(&mut self) -> Result { + Err("capture_identity must not run after a failed assignment".to_owned()) + } + fn resume(&mut self) -> Result<(), String> { + Err("resume must not run after a failed assignment".to_owned()) + } + fn terminate_created(&mut self) -> CleanupOutcome { + self.cleanup + } + } + impl ChildHost for AdmissionFailHost { type Child = FakeChild; fn spawn(&mut self, _config: &ServiceConfig) -> Result { self.attempts += 1; self.events.lock().unwrap().push("spawn".to_owned()); - Err(SpawnError::Admission(crate::admission::AdmissionFailure { - stage: crate::admission::AdmissionStage::AssignToJob, - reason: "AssignProcessToJobObject failed (5)".to_owned(), + let mut ops = FailingAdmission { cleanup: self.cleanup, - })) + }; + match crate::admission::admit(&mut ops) { + Ok(_) => unreachable!("a failing admission never succeeds"), + Err(failure) => Err(SpawnError::Admission(failure)), + } } }