From 02b408c499aedb3bc12acea587ae16b881a35497 Mon Sep 17 00:00:00 2001 From: jyviko <2501648+jyviko@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:58:56 -0400 Subject: [PATCH] app: a termination signal closes the window the normal way, not as a crash (#248) SIGTERM, SIGINT and SIGHUP ended the process on the spot: no last autosave, and the crash journal left behind became an unclean_exit report at the next start. The report on #248's machine ("ended without a clean exit between commands (last: control ui.capture)") has that exact signature and is reproduced by kill -TERM after a capture; 500 captures of the solved crank-mirror (50 app runs), 30 CLI snapshots and 250 MCP headless screenshots exited cleanly. The first signal now asks the window to close; a second ends the process with the signal's default action. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UhhZMBTviDQxZvwnyP89fo --- Cargo.lock | 11 +++ apps/fieldcraft/Cargo.toml | 4 + apps/fieldcraft/src/main.rs | 9 ++ apps/fieldcraft/src/terminate.rs | 140 +++++++++++++++++++++++++++++++ docs/crash-reports.md | 8 +- 5 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 apps/fieldcraft/src/terminate.rs diff --git a/Cargo.lock b/Cargo.lock index 1854381..5a6c147 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1284,6 +1284,7 @@ dependencies = [ "rfd", "rustls", "serde_json", + "signal-hook", "ureq", ] @@ -3677,6 +3678,16 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba" +[[package]] +name = "signal-hook" +version = "0.3.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d881a16cf4426aa584979d30bd82cb33429027e42122b169753d6ef1085ed6e2" +dependencies = [ + "libc", + "signal-hook-registry", +] + [[package]] name = "signal-hook-registry" version = "1.4.8" diff --git a/apps/fieldcraft/Cargo.toml b/apps/fieldcraft/Cargo.toml index 4153ad1..c88b832 100644 --- a/apps/fieldcraft/Cargo.toml +++ b/apps/fieldcraft/Cargo.toml @@ -25,6 +25,10 @@ ureq = { version = "3.4", default-features = false, features = ["rustls-no-provi rustls = { version = "0.23", default-features = false, features = ["ring", "std"] } getrandom = "0.3" +# A termination signal (SIGTERM, SIGINT, SIGHUP) closes the window the normal way (terminate.rs). +[target.'cfg(unix)'.dependencies] +signal-hook = "0.3" + [build-dependencies] # The Windows executable's icon resource (build.rs). embed-resource = "3.0.12" diff --git a/apps/fieldcraft/src/main.rs b/apps/fieldcraft/src/main.rs index 68f5af2..f114a67 100644 --- a/apps/fieldcraft/src/main.rs +++ b/apps/fieldcraft/src/main.rs @@ -12,6 +12,7 @@ #![forbid(unsafe_code)] mod control_server; +mod terminate; mod update; mod usage; @@ -267,6 +268,14 @@ fn main() -> eframe::Result { log::info!("graphics {:?} {} ({}), MSAA {}x (requested {MSAA}x)", info.backend, info.name, info.driver, rs.msaa_samples); app.set_wgpu(rs, DEPTH_BITS, rs.msaa_samples, &cc.egui_ctx); } + // `kill`, Ctrl-C or a logout closes the window as Close does: autosaved, no crash (#248). + let ctx = cc.egui_ctx.clone(); + if let Err(e) = terminate::watch(move || { + ctx.send_viewport_cmd(egui::ViewportCommand::Close); + ctx.request_repaint(); + }) { + log::warn!("termination signals: {e}"); + } if let Some(port) = args.control { app = app.with_control(control_server::start(port, cc.egui_ctx.clone())); } diff --git a/apps/fieldcraft/src/terminate.rs b/apps/fieldcraft/src/terminate.rs new file mode 100644 index 0000000..6546286 --- /dev/null +++ b/apps/fieldcraft/src/terminate.rs @@ -0,0 +1,140 @@ +//! A request to terminate (SIGTERM, SIGINT, SIGHUP on Unix) closes the window the normal way. +//! +//! Without this, `kill PID`, Ctrl-C in the terminal, a session logout or a test harness's timeout +//! ended the process on the spot: no `on_exit`, so no last autosave and no clean end of the crash +//! journal, and the next start filed an `unclean_exit` crash report for what was no crash (#248: +//! "ended without a clean exit between commands (last: control ui.capture)" was an automation run +//! stopped with SIGTERM after its capture). Now the first signal asks the window to close, which +//! autosaves, ends the journal and exits as a click on Close does; the journal notes the signal, +//! so a run that still dies while closing says why. A second signal, for a window that does not +//! close, ends the process at once with the signal's default action. +//! +//! Windows has no such signals: a window there is closed through its messages, which eframe +//! already turns into a normal close. + +use std::io; + +/// The signals that ask the app to terminate. +#[cfg(unix)] +const SIGNALS: [i32; 3] = [signal_hook::consts::SIGTERM, signal_hook::consts::SIGINT, signal_hook::consts::SIGHUP]; + +/// The signal's usual name, for the log and the crash journal. +#[cfg(unix)] +fn name(signal: i32) -> &'static str { + match signal { + signal_hook::consts::SIGTERM => "SIGTERM", + signal_hook::consts::SIGINT => "SIGINT", + signal_hook::consts::SIGHUP => "SIGHUP", + _ => "signal", + } +} + +/// Call `close` (on a thread of its own) on the first termination signal; a second one ends the +/// process with the signal's default action. +#[cfg(unix)] +pub fn watch(close: impl Fn() + Send + 'static) -> io::Result<()> { + let mut signals = signal_hook::iterator::Signals::new(SIGNALS)?; + std::thread::Builder::new().name("terminate".into()).spawn(move || { + let mut asked = false; + for signal in signals.forever() { + if asked { + log::warn!("{}: the window did not close; ending now", name(signal)); + if let Err(e) = signal_hook::low_level::emulate_default_handler(signal) { + log::error!("{}: {e}", name(signal)); + std::process::exit(128 + signal); + } + continue; + } + asked = true; + log::info!("{}: closing", name(signal)); + fieldcraft_engine::crash::activity(&format!("terminating ({})", name(signal))); + close(); + } + })?; + Ok(()) +} + +/// Nothing to watch without Unix signals. +#[cfg(not(unix))] +pub fn watch(_close: impl Fn() + Send + 'static) -> io::Result<()> { + Ok(()) +} + +#[cfg(all(test, unix))] +mod tests { + use std::path::{Path, PathBuf}; + use std::sync::mpsc; + + use fieldcraft_engine::crash; + + /// The child half: start crash reporting as the app does, then get SIGTERM. With the watcher + /// the "window" (here the main thread) closes the normal way; without it the process dies. + #[test] + fn child() { + let Some(dir) = std::env::var_os("FIELDCRAFT_TERMINATE_CHILD") else { return }; + let build = crash::Build { version: "9.9.9", commit: "abc1234" }; + let fresh = crash::install(PathBuf::from(dir), build); + if std::env::var_os("FIELDCRAFT_TERMINATE_NEXT").is_some() { + // The next start: what it reports of the run before. + println!("FRESH {}", fresh.iter().map(|s| s.message.as_str()).collect::>().join(" | ")); + crash::end_session(); + return; + } + crash::activity("control ui.capture"); + if std::env::var_os("FIELDCRAFT_TERMINATE_WATCH").is_some() { + let (tx, rx) = mpsc::channel(); + super::watch(move || { + let _ = tx.send(()); + }) + .unwrap(); + signal_hook::low_level::raise(signal_hook::consts::SIGTERM).unwrap(); + // What `on_exit` does once the window has closed. + rx.recv().unwrap(); + crash::end_session(); + println!("CLOSED"); + return; + } + signal_hook::low_level::raise(signal_hook::consts::SIGTERM).unwrap(); + // Not reached: SIGTERM's default action ends the process. + std::thread::sleep(std::time::Duration::from_secs(10)); + } + + #[test] + fn sigterm_closes_cleanly_and_leaves_no_crash_report() { + let root = std::env::temp_dir().join(format!("fieldcraft-terminate-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + let run = |dir: &Path, mode: &str| { + let mut c = std::process::Command::new(std::env::current_exe().unwrap()); + c.args(["--exact", "terminate::tests::child", "--test-threads=1", "--nocapture"]).env("FIELDCRAFT_TERMINATE_CHILD", dir); + match mode { + "watch" => c.env("FIELDCRAFT_TERMINATE_WATCH", "1"), + "next" => c.env("FIELDCRAFT_TERMINATE_NEXT", "1"), + _ => &mut c, + }; + c.output().unwrap() + }; + // What the next start in `dir` reports of the run before. + let reported_at_next_start = |dir: &Path| { + let out = run(dir, "next"); + assert!(out.status.success(), "{out:?}"); + let stdout = String::from_utf8_lossy(&out.stdout).to_string(); + stdout.lines().find_map(|l| l.split_once("FRESH ").map(|(_, r)| r.trim().to_string())).unwrap_or_else(|| panic!("{stdout}")) + }; + + // Without the watcher (the bug): killed, and the journal it leaves reads as a crash. + let unwatched = root.join("unwatched"); + let out = run(&unwatched, "kill"); + use std::os::unix::process::ExitStatusExt; + assert_eq!(out.status.signal(), Some(signal_hook::consts::SIGTERM), "{out:?}"); + let reported = reported_at_next_start(&unwatched); + assert!(reported.contains("ended without a clean exit") && reported.contains("control ui.capture"), "{reported}"); + + // With it: the close runs, the journal ends, and nothing is reported as a crash. + let watched = root.join("watched"); + let out = run(&watched, "watch"); + assert!(out.status.success(), "{out:?}"); + assert!(String::from_utf8_lossy(&out.stdout).contains("CLOSED"), "{out:?}"); + assert_eq!(reported_at_next_start(&watched), ""); + let _ = std::fs::remove_dir_all(&root); + } +} diff --git a/docs/crash-reports.md b/docs/crash-reports.md index 16d0d84..de052a8 100644 --- a/docs/crash-reports.md +++ b/docs/crash-reports.md @@ -50,8 +50,8 @@ error stream. - `report.json` (`"format": "fieldcraft-crash/1"`): `kind` (`command_panic`: a command hit an internal error, caught, the model left unchanged; `recovered_panic`: one caught inside a command; `panic`: an internal error outside commands; `unclean_exit`: the app ended without a clean exit - — an abort, a stack overflow, a crash in a graphics driver, the process being killed — found at - the next start), `message`, `location`, `thread`, `backtrace`, `app`, `system`, `command`, + — an abort, a stack overflow, a crash in a graphics driver, the process being killed outright + (SIGKILL) — found at the next start), `message`, `location`, `thread`, `backtrace`, `app`, `system`, `command`, `recent_commands` (up to 32), `last_activity` (the last control request, such as `control ui.capture`), `base`, `count`, `first_seen`, `last_seen`, `notes` - `model.fieldcraft`: the model the replay starts from @@ -78,6 +78,10 @@ diagnostics.bug_report` opens the form, `ui.prompt {"answer": "set", …}` sets **Check that reporting works:** with `FIELDCRAFT_CRASH_TEST=1`, `diagnostics.crash_test {mode}` crashes on purpose: `panic`, `recovered` or `abort` (reported at the next start). +A request to terminate is no crash: on Linux and macOS, SIGTERM (`kill PID`), SIGINT (Ctrl-C) +and SIGHUP close the window as **Close** does (unsaved work is autosaved for recovery, the +journal ends, no report). A second signal ends the app at once. + ### How it stays cheap and small The app keeps a journal of the window's top-level commands (`crashes/sessions/*.jsonl`, at most