diff --git a/crates/platform/src/lib.rs b/crates/platform/src/lib.rs index 59e9e6ec..c1c62135 100644 --- a/crates/platform/src/lib.rs +++ b/crates/platform/src/lib.rs @@ -40,14 +40,26 @@ impl Drop for KeepAwake { // shortcut: Windows (a PowerShell helper, since this workspace forbids unsafe FFI) and macOS // (caffeinate) only; add `systemd-inhibit` for Linux when it's needed. pub fn keep_awake() -> Option { + keep_awake_while(std::process::id(), std::time::Duration::from_secs(30)) +} + +/// The helper for [`keep_awake`], which also ends by itself once process `owner` is gone (checked +/// every `every` on Windows), so a process that exits without dropping the guard (the window +/// closed during a build, a crash, a kill) never leaves the computer held awake. +fn keep_awake_while(owner: u32, every: std::time::Duration) -> Option { // ES_CONTINUOUS | ES_SYSTEM_REQUIRED (0x80000001, in decimal: Windows PowerShell reads the hex // literal as a negative Int32), held by the helper's thread until it's stopped. Any failure - // ends the helper, so it never runs without doing its job. - const WINDOWS: &str = "$ErrorActionPreference = 'Stop'; $k = Add-Type -Name P -Namespace CraftAnything -PassThru -MemberDefinition '[DllImport(\"kernel32.dll\")] public static extern uint SetThreadExecutionState(uint f);'; while ($true) { if ($k::SetThreadExecutionState(2147483649) -eq 0) { exit 1 }; Start-Sleep -Seconds 30 }"; + // ends the helper, so it never runs without doing its job. The owner is opened once and its + // handle kept, so Windows can't give its id to another process the helper would then serve. + let windows = format!( + "$ErrorActionPreference = 'Stop'; $k = Add-Type -Name P -Namespace CraftAnything -PassThru -MemberDefinition '[DllImport(\"kernel32.dll\")] public static extern uint SetThreadExecutionState(uint f);'; $o = Get-Process -Id {owner}; $null = $o.Handle; while (-not $o.HasExited) {{ if ($k::SetThreadExecutionState(2147483649) -eq 0) {{ exit 1 }}; Start-Sleep -Seconds {} }}", + every.as_secs().max(1) + ); let p = if cfg!(windows) { - Process::new("powershell").args(["-NoProfile", "-NonInteractive", "-Command", WINDOWS]) + Process::new("powershell").args(["-NoProfile", "-NonInteractive", "-Command", windows.as_str()]) } else if cfg!(target_os = "macos") { - Process::new("caffeinate").arg("-i") + // caffeinate's own -w ends it when that process exits. + Process::new("caffeinate").args(["-i".to_string(), "-w".into(), owner.to_string()]) } else { return None; }; @@ -77,6 +89,23 @@ mod tests { assert_eq!(absent.ok(), Some(true), "dropping the guard stops the helper"); } + /// The window closed during a build: the app ends without dropping the guard, and its helper + /// must not keep the computer awake for a week. + #[cfg(any(windows, target_os = "macos"))] + #[test] + fn the_helper_ends_when_its_owner_is_gone() { + let owner = if cfg!(windows) { + Process::new("powershell").args(["-NoProfile", "-NonInteractive", "-Command", "Start-Sleep -Seconds 60"]) + } else { + Process::new("sleep").arg("60") + }; + let mut owner = owner.timeout(Duration::from_secs(90)).spawn().expect("the stand-in owner starts"); + let mut guard = keep_awake_while(owner.pid(), Duration::from_secs(1)).expect("the helper starts"); + assert!(guard.0.wait_timeout(Duration::from_secs(3)).is_none(), "holding while its owner runs"); + assert!(owner.kill_and_wait(Duration::from_secs(5)), "the stand-in owner stops"); + assert!(guard.0.wait_timeout(Duration::from_secs(15)).is_some(), "the helper ends by itself once its owner is gone"); + } + fn stand_in(name: &str) -> Process { // argv[0] is the test binary even in a sandbox without /proc/self/exe. let exe = std::env::args_os().next().expect("test binary path"); diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 85616672..271db94b 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -167,6 +167,10 @@ only when it's green and a maintainer has fully reviewed it, code as well as wor - [ ] **decision** Replies don't reach people who file through the ticket relay (each such ticket says so), yet the bot asks them for details. About half the open issues came that way. Get replies back to them, or stop asking +- [ ] `which::tests::usual_windows_dirs_come_after_path` (platform) fails on a Windows machine + whose environment adds a folder (10 found, 9 expected, "nothing twice"), on `main` too; and + `capture::console_tests::reads_what_the_program_drew_and_stops_at_the_deadline` failed once + with the whole platform suite running and passes alone. Find what each depends on - [ ] `a_helper_is_judged_by_whether_it_runs` (ai crate) sometimes fails when several crates' tests run at once and passes alone: it writes a script and runs it straight away, which a busy machine can refuse while another test's child still holds the file open @@ -644,9 +648,13 @@ Gaps found while using the app for real projects. - [x] Release the keep-awake helper before a rebuild reports `Finished`, waiting up to five seconds for its supervised cleanup and logging a timeout. Real subprocess stand-ins checked on Linux (#541) — claimed: Afloat16, branch bugfix/r2.9-keep-awake-cleanup-20261103 -- [ ] Recheck keep-awake cleanup with a real Windows CLI `rebuild`: the helper once outlived it by - more than 9 minutes and held the caller's output pipe; not reproduced in 8 more runs. The - bounded cleanup change has been checked with Linux stand-ins, not this Windows incident +- [x] Recheck keep-awake cleanup with a real Windows CLI `rebuild`: the helper once outlived it by + more than 9 minutes and held the caller's output pipe; not reproduced in 8 more runs. Found: + closing the desktop app during a build ends it without dropping the guard, and the helper + (an endless PowerShell loop, started with a week's time limit) carried on. It now ends by + itself once the process it serves is gone: the Windows loop checks that process every 30 s, + and macOS uses `caffeinate -w`. Live on Windows: a CLI build killed outright, its helper + ended 30 s later. Not run on a Mac - [ ] Find why a site capture lost its browser ("the WebSocket is closed", 419 ms into `web_site_capture` for a second page, seen once; the same capture passed 41 s earlier) - [ ] Claude Code's Ultracode switch (a setting, not an effort level; the effort environment diff --git a/docs/pitfalls.md b/docs/pitfalls.md index cd8afa89..87f19254 100644 --- a/docs/pitfalls.md +++ b/docs/pitfalls.md @@ -306,6 +306,12 @@ at the wrong tree (the cargo commands `ci` runs use the folder they're started i worktree its own `CARGO_TARGET_DIR`, and rebuild `xtask` (or delete it from `target/`) after sharing one. +**A helper stopped only by a destructor.** The keep-awake helper was stopped when its guard was +dropped; closing the desktop app during a build ended the process without dropping it, and the +PowerShell loop kept the computer awake (its time limit was a week). A helper that outlives a +crash is a leak: have it watch the process it serves and end by itself (`caffeinate -w`, a loop +that checks the PID), and test by killing the owner, not by dropping the guard. + **Stopping a long run leaves children behind** unless every program goes through `craft_anything_platform::process`. After stopping one, check that its child processes are gone.