diff --git a/crates/engine/src/select_extra_cmds.rs b/crates/engine/src/select_extra_cmds.rs index 9c8453bf64..9d42cb36c4 100644 --- a/crates/engine/src/select_extra_cmds.rs +++ b/crates/engine/src/select_extra_cmds.rs @@ -376,3 +376,23 @@ mod tests { assert!(!s.is_enabled("select.transformSelection")); } } + +#[cfg(test)] +mod hang_tests { + use super::*; + + #[test] + fn transform_selection_with_extreme_params_terminates() { + // Regression (panic_hunt): a huge off-canvas transform used to allocate a buffer spanning + // the old + new bounds and hang. It must return quickly without crashing. + let mut s = Session::new(); + s.execute("file.new", json!({"width": 24, "height": 16})).unwrap(); + s.execute("select.rect", json!({"x": 1, "y": 1, "width": 6, "height": 5})).unwrap(); + // dx/dy move the selection far off-canvas; scaleX enlarges it — the old code allocated a + // buffer spanning the original + moved bounds (~1e5 × 1e5) and hung. + let r = s.execute("select.transformSelection", json!({"dx": 1e5, "dy": -1e5, "scaleX": 300.0})); + assert!(r.is_ok(), "far transform returns without hanging: {r:?}"); + // The document is still usable. + assert!(s.execute("select.all", json!({})).is_ok()); + } +} diff --git a/crates/engine/src/transform_cmds.rs b/crates/engine/src/transform_cmds.rs index e55ed5be0d..96f3920c47 100644 --- a/crates/engine/src/transform_cmds.rs +++ b/crates/engine/src/transform_cmds.rs @@ -47,10 +47,12 @@ pub(crate) fn warp_gray(s: &Surface, h: &Homography, interp: Interp) -> Surface let v = s.read_region(src); tmp.write_region(src, &v.iter().flat_map(|g| [*g, 1.0]).collect::>()); let w = warp_surface(&tmp, src, h, interp); - // Everything the old content covered becomes default, then the warped content composites over. - let cover = src.union(&w.content_bounds()); - let old: Vec = vec![default; cover.width() as usize * cover.height() as usize]; - out.write_region(cover, &old); + // Clear the old content region, then composite the warped content over it. These are written as + // two sparse regions rather than one dense `src ∪ warped` block: a transform that moves the + // content far away (e.g. a huge translation) would otherwise allocate a buffer spanning both and + // hang/OOM. `Surface` is tile-sparse, so distant regions cost only their own tiles. + let old: Vec = vec![default; src.width() as usize * src.height() as usize]; + out.write_region(src, &old); let b = w.content_bounds(); if !b.is_empty() { let px = w.read_region(b); diff --git a/crates/engine/tests/panic_hunt.rs b/crates/engine/tests/panic_hunt.rs index 406fbfda72..d334ef7c75 100644 --- a/crates/engine/tests/panic_hunt.rs +++ b/crates/engine/tests/panic_hunt.rs @@ -1,9 +1,12 @@ -//! Fuzz every command with adversarial params; a command must return `Err`, never panic. -//! (Rule 9 in AGENTS.md.) Params here keep the canvas small on purpose — validating absurd -//! dimensions is a separate concern; this hunts missing-param / empty-state / bad-index panics. +//! Fuzz every command with adversarial params; a command must return `Err`, never panic or hang. +//! (Rule 9 in AGENTS.md.) Each command runs on its own thread with a timeout, so one bad command +//! can't wedge the run — panics are caught and hangs are reported. Opt-in (slow): +//! `cargo test -p photocraft-engine --test panic_hunt -- --ignored`. use photocraft_engine::{command_specs, Session}; use serde_json::{json, Value}; use std::panic::{catch_unwind, AssertUnwindSafe}; +use std::sync::mpsc; +use std::time::Duration; fn fresh() -> Session { let mut s = Session::new(); @@ -13,36 +16,42 @@ fn fresh() -> Session { s } -// Slow (fresh session per command × params, in debug). Opt-in: `cargo test -p photocraft-engine -// --test panic_hunt -- --ignored`. Fast targeted regressions live beside the fixed commands. +/// Run `id` with `p` on a fresh session, on a worker thread. Returns `Err` with a reason on panic +/// or timeout, `Ok` otherwise (whether the command returned Ok or a graceful Err). +fn run_guarded(id: &str, p: &Value) -> Result<(), String> { + let (tx, rx) = mpsc::channel(); + let (id, p) = (id.to_string(), p.clone()); + std::thread::spawn(move || { + let r = catch_unwind(AssertUnwindSafe(|| { + let mut s = fresh(); + let _ = s.execute(&id, p); + })); + let _ = tx.send(r.is_ok()); + }); + match rx.recv_timeout(Duration::from_secs(4)) { + Ok(true) => Ok(()), + Ok(false) => Err("panicked".into()), + Err(_) => Err("hung (>4s)".into()), + } +} + #[test] #[ignore = "slow full-registry fuzz; run with --ignored"] -fn no_command_panics_on_adversarial_params() { - // None of these resize the canvas, so the fuzz stays fast. +fn no_command_panics_or_hangs_on_adversarial_params() { let params: Vec = vec![ json!({}), - json!({"x": -9, "y": -9, "radius": 0, "amount": 0, "angle": 0, "opacity": 0, "scale": 0, "levels": 0, "tolerance": 0, "gamma": 0, "layer": 999999, "channel": 999, "index": 999999}), - json!({"x": 1e6, "y": -1e6, "radius": 500, "amount": 1000, "angle": 1e6, "opacity": -100, "scale": -5, "gamma": -1}), + json!({"x": -9, "y": -9, "radius": 0, "amount": 0, "angle": 0, "opacity": 0, "scale": 0, "levels": 0, "tolerance": 0, "gamma": 0, "layer": 999999, "channel": 999, "index": 999999, "count": 0}), + json!({"x": 1e5, "y": -1e5, "radius": 300, "amount": 300, "angle": 1e5, "opacity": -100, "scale": -5, "gamma": -1}), json!({"points": [], "from": [0, 0], "to": [0, 0], "stops": [], "matrix": [0, 0, 0, 0, 0, 0], "colors": [], "name": "", "mode": "", "style": ""}), ]; - let prev = std::panic::take_hook(); - std::panic::set_hook(Box::new(|_| {})); - let mut panicked: Vec = Vec::new(); + let mut bad: Vec = Vec::new(); for spec in command_specs() { for p in ¶ms { - let id = spec.id.to_string(); - let pc = p.clone(); - let crashed = catch_unwind(AssertUnwindSafe(|| { - let mut s = fresh(); - let _ = s.execute(&id, pc); - })) - .is_err(); - if crashed { - panicked.push(format!("{} <- {}", spec.id, p)); + if let Err(why) = run_guarded(spec.id, p) { + bad.push(format!("{} [{why}] <- {p}", spec.id)); break; } } } - std::panic::set_hook(prev); - assert!(panicked.is_empty(), "{} commands panicked:\n{}", panicked.len(), panicked.join("\n")); + assert!(bad.is_empty(), "{} commands panicked or hung:\n{}", bad.len(), bad.join("\n")); }