forked from 1b-agent/fieldcraft-upstream
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 <[email protected]> Claude-Session: https://claude.ai/code/session_01UhhZMBTviDQxZvwnyP89fo
This commit is contained in:
1 parent
3d38753cad
commit
02b408c499
5 files changed
+170
-2
No files matched your search
Generated
+11
@@ -1284,6 +1284,7 @@ dependencies = [
|
|||||||
"rfd",
|
"rfd",
|
||||||
"rustls",
|
"rustls",
|
||||||
"serde_json",
|
"serde_json",
|
||||||
|
"signal-hook",
|
||||||
"ureq",
|
"ureq",
|
||||||
]
|
]
|
||||||
|
|
||||||
@@ -3677,6 +3678,16 @@ version = "2.0.1"
|
|||||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||||
checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba"
|
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]]
|
[[package]]
|
||||||
name = "signal-hook-registry"
|
name = "signal-hook-registry"
|
||||||
version = "1.4.8"
|
version = "1.4.8"
|
||||||
|
|||||||
@@ -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"] }
|
rustls = { version = "0.23", default-features = false, features = ["ring", "std"] }
|
||||||
getrandom = "0.3"
|
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]
|
[build-dependencies]
|
||||||
# The Windows executable's icon resource (build.rs).
|
# The Windows executable's icon resource (build.rs).
|
||||||
embed-resource = "3.0.12"
|
embed-resource = "3.0.12"
|
||||||
|
|||||||
@@ -12,6 +12,7 @@
|
|||||||
#![forbid(unsafe_code)]
|
#![forbid(unsafe_code)]
|
||||||
|
|
||||||
mod control_server;
|
mod control_server;
|
||||||
|
mod terminate;
|
||||||
mod update;
|
mod update;
|
||||||
mod usage;
|
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);
|
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);
|
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 {
|
if let Some(port) = args.control {
|
||||||
app = app.with_control(control_server::start(port, cc.egui_ctx.clone()));
|
app = app.with_control(control_server::start(port, cc.egui_ctx.clone()));
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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::<Vec<_>>().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);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -50,8 +50,8 @@ error stream.
|
|||||||
- `report.json` (`"format": "fieldcraft-crash/1"`): `kind` (`command_panic`: a command hit an
|
- `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;
|
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
|
`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
|
— an abort, a stack overflow, a crash in a graphics driver, the process being killed outright
|
||||||
the next start), `message`, `location`, `thread`, `backtrace`, `app`, `system`, `command`,
|
(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
|
`recent_commands` (up to 32), `last_activity` (the last control request, such as
|
||||||
`control ui.capture`), `base`, `count`, `first_seen`, `last_seen`, `notes`
|
`control ui.capture`), `base`, `count`, `first_seen`, `last_seen`, `notes`
|
||||||
- `model.fieldcraft`: the model the replay starts from
|
- `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}`
|
**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).
|
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
|
### How it stays cheap and small
|
||||||
|
|
||||||
The app keeps a journal of the window's top-level commands (`crashes/sessions/*.jsonl`, at most
|
The app keeps a journal of the window's top-level commands (`crashes/sessions/*.jsonl`, at most
|
||||||
|
|||||||
Reference in new issue
Block a user