diff --git a/src-tauri/src/commands/mod.rs b/src-tauri/src/commands/mod.rs index 9972e600..3b9a636e 100644 --- a/src-tauri/src/commands/mod.rs +++ b/src-tauri/src/commands/mod.rs @@ -27,6 +27,7 @@ pub mod log_secrets; pub mod maa_agent; pub mod maa_core; pub mod notification; +mod pre_action_stops; mod run_completion; pub mod state; pub mod system; diff --git a/src-tauri/src/commands/pre_action_stops.rs b/src-tauri/src/commands/pre_action_stops.rs new file mode 100644 index 00000000..1dfeebb7 --- /dev/null +++ b/src-tauri/src/commands/pre_action_stops.rs @@ -0,0 +1,63 @@ +use std::collections::HashMap; +use std::sync::Mutex; +use tokio::sync::watch; + +/// A Stop belongs to the processes already waiting on this instance. Reset opens +/// the next run without clearing cancellation on their existing receivers. +#[derive(Default)] +pub struct PreActionStops(Mutex>>); + +impl PreActionStops { + pub fn subscribe(&self, instance_id: &str) -> watch::Receiver { + let mut requests = self.0.lock().unwrap_or_else(|e| e.into_inner()); + requests.retain(|_, tx| tx.receiver_count() > 0 || *tx.borrow()); + requests + .entry(instance_id.to_owned()) + .or_insert_with(|| watch::Sender::new(false)) + .subscribe() + } + + pub fn set(&self, instance_id: String, stop: bool) { + let mut requests = self.0.lock().unwrap_or_else(|e| e.into_inner()); + if stop { + requests + .entry(instance_id) + .or_insert_with(|| watch::Sender::new(false)) + .send_replace(true); + } else if requests + .get(&instance_id) + .is_some_and(|tx| *tx.borrow() || tx.receiver_count() == 0) + { + requests.remove(&instance_id); + } + } +} + +#[cfg(test)] +mod tests { + use super::PreActionStops; + + #[test] + fn reset_does_not_clear_stop_for_an_older_waiter() { + let requests = PreActionStops::default(); + let first = requests.subscribe("A"); + let other = requests.subscribe("B"); + requests.set("A".into(), true); + let already_stopped = requests.subscribe("A"); + requests.set("A".into(), false); + let next = requests.subscribe("A"); + assert!(*first.borrow()); + assert!(*already_stopped.borrow()); + assert!(!*next.borrow()); + assert!(!*other.borrow()); + } + + #[test] + fn repeated_reset_does_not_detach_an_active_waiter() { + let requests = PreActionStops::default(); + let waiter = requests.subscribe("A"); + requests.set("A".into(), false); + requests.set("A".into(), true); + assert!(*waiter.borrow()); + } +} diff --git a/src-tauri/src/commands/system.rs b/src-tauri/src/commands/system.rs index 5d840a9d..dc3a5b17 100644 --- a/src-tauri/src/commands/system.rs +++ b/src-tauri/src/commands/system.rs @@ -545,15 +545,7 @@ pub fn set_pre_action_stop( instance_id: String, stop: bool, ) -> Result<(), String> { - let mut requests = state - .pre_action_stop_requests - .lock() - .map_err(|e| e.to_string())?; - if stop { - requests.insert(instance_id); - } else { - requests.remove(&instance_id); - } + state.pre_action_stop_requests.set(instance_id, stop); Ok(()) } @@ -596,6 +588,10 @@ pub async fn run_action( } if wait_for_exit { + let stop = state.pre_action_stop_requests.subscribe(&instance_id); + if *stop.borrow() { + return Err("MXU_PRE_ACTION_CANCELLED".to_string()); + } let mut child = cmd .spawn() .map_err(|e| format!("Failed to run action: {} - {}", program, e))?; @@ -610,15 +606,7 @@ pub async fn run_action( return Ok(exit_code); } - let stop_requested = { - let requests = state - .pre_action_stop_requests - .lock() - .map_err(|e| e.to_string())?; - requests.contains(&instance_id) - }; - - if stop_requested { + if *stop.borrow() { info!("run_action wait cancelled by stop request: {}", instance_id); // 停止只中断等待,不强制终止前置程序;交给后台线程 wait 以避免子进程泄漏。 std::thread::spawn(move || { @@ -705,6 +693,10 @@ pub async fn run_pretask_impl( } } + let stop = state.pre_action_stop_requests.subscribe(&instance_id); + if *stop.borrow() { + return Err("MXU_PRE_ACTION_CANCELLED".to_string()); + } let mut child = cmd .spawn() .map_err(|e| format!("Failed to run pretask: {} - {}", program, e))?; @@ -743,6 +735,7 @@ pub async fn run_pretask_impl( )); } + let mut kill_requested = false; loop { if let Some(status) = child .try_wait() @@ -759,30 +752,28 @@ pub async fn run_pretask_impl( } output_batcher.flush(); + if *stop.borrow() { + return Err("MXU_PRE_ACTION_CANCELLED".to_string()); + } return Ok(exit_code); } - let stop_requested = { - let requests = state - .pre_action_stop_requests - .lock() - .map_err(|e| e.to_string())?; - requests.contains(&instance_id) - }; - - if stop_requested { + if !kill_requested && *stop.borrow() { info!( "run_pretask wait cancelled by stop request: {}", instance_id ); // pretask 由项目声明、受 MXU 管理,与前置程序不同,停止时直接终止,避免其在后台继续操作 if let Err(e) = child.kill() { - warn!("Failed to kill pretask {}: {}", program, e); + // The process can exit between try_wait and kill. + if !matches!(child.try_wait(), Ok(Some(_))) { + std::thread::spawn(move || { + let _ = child.wait(); + }); + return Err(format!("Failed to kill pretask {}: {}", program, e)); + } } - std::thread::spawn(move || { - let _ = child.wait(); - }); - return Err("MXU_PRE_ACTION_CANCELLED".to_string()); + kill_requested = true; } sleep(Duration::from_millis(100)).await; diff --git a/src-tauri/src/commands/types.rs b/src-tauri/src/commands/types.rs index ff32e2a1..70397928 100644 --- a/src-tauri/src/commands/types.rs +++ b/src-tauri/src/commands/types.rs @@ -2,7 +2,7 @@ //! //! 包含 Tauri 命令使用的数据结构和枚举 -use std::collections::{BTreeMap, HashMap, HashSet, VecDeque}; +use std::collections::{BTreeMap, HashMap, VecDeque}; use std::path::PathBuf; use std::process::Child; use std::sync::{Arc, Mutex}; @@ -778,7 +778,7 @@ pub struct MaaState { pub resource_dir: Mutex>, pub instances: Mutex>, /// 前置程序停止请求(用于中断等待退出) - pub pre_action_stop_requests: Mutex>, + pub pre_action_stop_requests: super::pre_action_stops::PreActionStops, /// Controller 连接池:相同配置的 Controller 复用同一个 MaaControllerHandle pub controller_pool: Mutex>, pub controller_connection_gates: super::callback_lifecycle::ConnectionGates, diff --git a/src-tauri/src/web_server.rs b/src-tauri/src/web_server.rs index 99093315..cb2662d6 100644 --- a/src-tauri/src/web_server.rs +++ b/src-tauri/src/web_server.rs @@ -1410,16 +1410,10 @@ async fn handle_stop_pretask( axum::extract::Path(instance_id): axum::extract::Path, Json(body): Json, ) -> impl IntoResponse { - let mut requests = state + state .maa_state .pre_action_stop_requests - .lock() - .unwrap_or_else(|e| e.into_inner()); - if body.stop { - requests.insert(instance_id); - } else { - requests.remove(&instance_id); - } + .set(instance_id, body.stop); Json(serde_json::json!({ "ok": true })) } diff --git a/tests/pretask/README.md b/tests/pretask/README.md new file mode 100644 index 00000000..57fcffcb --- /dev/null +++ b/tests/pretask/README.md @@ -0,0 +1,33 @@ +# Pretask cancellation regression + +From the repository root, with Python 3.11+ and Rust 1.89+: + +```sh +python tests/pretask/run.py +# Use cached Cargo dependencies without accessing the network: +python tests/pretask/run.py --offline +# Check a revision before the fix; cancellation assertions should fail: +python tests/pretask/run.py --offline --ref +``` + +The runner copies the selected checkout's actual `run_pretask_impl`, `run_action`, +desktop/Web stop setters, cancellation registry, and launch-command builder into +a temporary Rust crate. It uses the direct dependency versions in that checkout's +Cargo.lock. App handles, event/log sinks, and HTTP wrappers are stubbed; command +logic and child-process operations are unchanged. This does not exercise Tauri, +HTTP transport, or a real Maa resource. + +The child is the fixture executable itself. A ready file and a manually polled +future hold the first command inside its wait loop while Stop, reset, and a new +command complete. This deterministically checks that resetting the next run does +not erase the first run's cancellation. Other cases cover ordinary Stop, +redundant reset, stopping before spawn, instance isolation, normal exit codes, +and cancellation of a user-configured pre-action wait without killing its program. +An exclusive file lock checks that a canceled managed child has exited before +the command returns. Each waiting child also exits on its own after four seconds; +the runner has build and execution timeouts and keeps artifacts in a temporary +directory. + +The registry's unit tests also run in the fixture and remain available through +the application's normal `cargo test --lib` suite. This standalone fixture is +useful when app linking or Windows app-manifest setup prevents running that suite. diff --git a/tests/pretask/harness.rs b/tests/pretask/harness.rs new file mode 100644 index 00000000..062a85f5 --- /dev/null +++ b/tests/pretask/harness.rs @@ -0,0 +1,275 @@ +// Real run_pretask_impl, its desktop stop command and build_launch_command are included +// from the selected checkout. Stub the app/logging boundary; stdout/stderr are drained. +#![allow(dead_code, unused_imports, unused_macros, private_interfaces)] +use std::collections::HashSet; +use std::future::{poll_fn, Future}; +use std::path::{Path, PathBuf}; +use std::pin::Pin; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::{Arc, Mutex}; +use std::task::Poll; +use std::time::{Duration, Instant}; + +mod pre_action_stops { + include!("control.rs"); +} +mod state { + use super::*; + include!("state.rs"); +} +use state::MaaState; + +mod tauri { + pub struct AppHandle; +} +mod chrono { + pub struct Local; + impl Local { + pub fn now() -> Self { + Self + } + pub fn format(&self, _: &str) -> &str { + "fixture" + } + } +} +macro_rules! info { ($($item:tt)*) => {{ let _ = format_args!($($item)*); }} } +macro_rules! warn { ($($item:tt)*) => {{ eprintln!($($item)*); }} } +mod utils { + include!("launch.rs"); +} +mod maa_agent { + use super::*; + pub fn resolve_child_exec_path(program: &str, cwd: &str) -> PathBuf { + Path::new(cwd).join(program) + } +} +static SPAWN_COUNT: AtomicUsize = AtomicUsize::new(0); +struct AgentOutputBatcher; +impl AgentOutputBatcher { + fn new(_: tauri::AppHandle, _: String) -> Arc { + SPAWN_COUNT.fetch_add(1, Ordering::SeqCst); + Arc::new(Self) + } + fn flush(&self) {} +} +struct ChildOutputLog; +impl ChildOutputLog { + fn new(_: &str) -> Arc { + Arc::new(Self) + } +} +fn spawn_child_output_reader( + mut pipe: impl std::io::Read + Send + 'static, + _: &str, + _: String, + _: Arc, + _: Arc, +) -> std::thread::JoinHandle<()> { + std::thread::spawn(move || { + let _ = std::io::copy(&mut pipe, &mut std::io::sink()); + }) +} +mod system { + use super::*; + use tokio::time::sleep; + type State<'a, T> = &'a T; + const CREATE_NO_WINDOW: u32 = 0x08000000; + include!("commands.rs"); +} + +mod web { + use super::*; + struct State(T); + struct Json(T); + struct WebState { + maa_state: Arc, + } + struct PretaskStopRequest { + stop: bool, + } + trait IntoResponse {} + impl IntoResponse for Json {} + mod axum { + pub mod extract { + pub struct Path(pub T); + } + } + include!("web.rs"); + pub async fn stop(state: &Arc, id: &str, value: bool) { + handle_stop_pretask( + State(WebState { + maa_state: state.clone(), + }), + axum::extract::Path(id.into()), + Json(PretaskStopRequest { stop: value }), + ) + .await; + } +} + +fn stop(state: &Arc, id: &str, value: bool) { + system::set_pre_action_stop(state, id.to_owned(), value).unwrap() +} +async fn start(state: &Arc, id: &str, marker: &str, mode: &str) -> Result { + system::run_pretask_impl( + tauri::AppHandle, + state, + id.to_owned(), + std::env::current_exe().unwrap().to_str().unwrap().into(), + vec!["--child".into(), marker.into(), mode.into()], + Some( + std::env::current_dir() + .unwrap() + .to_str() + .unwrap() + .to_owned(), + ), + ) + .await +} + +// Advance only to the actual wait loop's first Pending; the future is deliberately not +// scheduled again until both stop/reset requests and the replacement start have completed. +async fn start_and_hold(mut command: Pin<&mut F>, marker: &str) { + poll_fn(|cx| { + assert!(command.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; + tokio::time::timeout(Duration::from_secs(2), async { + while !Path::new(&format!("{marker}.ready")).is_file() { + tokio::time::sleep(Duration::from_millis(5)).await; + } + }) + .await + .expect("child fixture did not signal readiness"); +} +fn report(ok: bool, name: &str, details: impl std::fmt::Debug) -> usize { + println!("{} {name} {details:?}", if ok { "PASS" } else { "FAIL" }); + usize::from(!ok) +} +fn child_released_lock(marker: &str) -> bool { + let lock = std::fs::OpenOptions::new() + .read(true) + .write(true) + .open(format!("{marker}.lock")) + .unwrap(); + lock.try_lock().is_ok() +} +async fn run() -> usize { + let mut failures = 0; + let state = Arc::new(MaaState::default()); + let cancelled = Err(String::from("MXU_PRE_ACTION_CANCELLED")); + + let mut first = Box::pin(start(&state, "A", "first", "wait")); + start_and_hold(first.as_mut(), "first").await; + web::stop(&state, "A", true).await; + stop(&state, "A", false); + let replacement = start(&state, "A", "second", "quick").await; + failures += report( + replacement == Ok(23), + "replacement completes with its own exit code", + replacement, + ); + // At this point cancellation must still win for the old child. The control file lets + // a buggy child exit naturally, so even the expected baseline failure stays bounded. + let old = tokio::time::timeout(Duration::from_millis(800), first.as_mut()).await; + let old = match old { + Ok(value) => value, + Err(_) => { + std::fs::write("first.release", []).unwrap(); + first.await + } + }; + failures += report( + old == cancelled && !Path::new("first.done").exists() && child_released_lock("first"), + "stop then immediate reset cancels the old process", + (&old, Path::new("first.done").exists()), + ); + + let mut direct = Box::pin(start(&state, "A", "direct", "wait")); + start_and_hold(direct.as_mut(), "direct").await; + stop(&state, "A", false); + stop(&state, "A", true); + let result = direct.await; + let child_exited = child_released_lock("direct"); + failures += report( + result == cancelled && !Path::new("direct.done").exists() && child_exited, + "ordinary stop cancels the child", + (result, child_exited), + ); + let before = SPAWN_COUNT.load(Ordering::SeqCst); + let blocked = start(&state, "A", "blocked", "quick").await; + let spawns = SPAWN_COUNT.load(Ordering::SeqCst) - before; + failures += report( + blocked == cancelled && spawns == 0, + "already-stopped pretask never spawns", + (blocked, spawns), + ); + let other = start(&state, "B", "other", "quick").await; + failures += report(other == Ok(23), "unrelated instance runs normally", other); + stop(&state, "A", false); + let retry = start(&state, "A", "retry", "quick").await; + failures += report(retry == Ok(23), "reset allows next launch", retry); + + // User-configured pre-actions intentionally detach on Stop. Verify the shared + // cancellation registry still interrupts just the wait for those programs. + let mut action = Box::pin(system::run_action( + &state, + "A".into(), + std::env::current_exe().unwrap().to_str().unwrap().into(), + "--child action wait".into(), + Some(std::env::current_dir().unwrap().to_str().unwrap().into()), + true, + Some(false), + )); + start_and_hold(action.as_mut(), "action").await; + stop(&state, "A", true); + web::stop(&state, "A", false).await; + let stopped = tokio::time::timeout(Duration::from_millis(800), action.as_mut()).await; + std::fs::write("action.release", []).unwrap(); + let stopped = match stopped { + Ok(value) => value, + Err(_) => action.await, + }; + let survived = tokio::time::timeout(Duration::from_secs(2), async { + while !Path::new("action.done").exists() || !child_released_lock("action") { + tokio::time::sleep(Duration::from_millis(5)).await; + } + }) + .await + .is_ok(); + failures += report( + stopped == cancelled && survived, + "pre-action wait cancelled; program finishes on fixture release", + (stopped, survived), + ); + failures +} + +fn main() { + let args: Vec<_> = std::env::args().collect(); + if args.get(1).map(String::as_str) == Some("--child") { + let marker = &args[2]; + let lock = std::fs::File::create(format!("{marker}.lock")).unwrap(); + lock.lock().unwrap(); + std::fs::write(format!("{marker}.ready"), []).unwrap(); + if args[3] == "wait" { + let start = Instant::now(); + while !Path::new(&format!("{marker}.release")).exists() + && start.elapsed() < Duration::from_secs(4) + { + std::thread::sleep(Duration::from_millis(5)); + } + } + std::fs::write(format!("{marker}.done"), []).unwrap(); + std::process::exit(23) + } + let failures = tokio::runtime::Builder::new_current_thread() + .enable_time() + .build() + .unwrap() + .block_on(run()); + std::process::exit(if failures == 0 { 0 } else { 1 }); +} diff --git a/tests/pretask/run.py b/tests/pretask/run.py new file mode 100644 index 00000000..85dd5d6d --- /dev/null +++ b/tests/pretask/run.py @@ -0,0 +1,96 @@ +"""Run MXU's pretask commands with real child processes, without starting the app.""" + +import argparse +import os +from pathlib import Path +import shutil +import subprocess +import tempfile +import tomllib + + +fixture = Path(__file__).resolve().parent +parser = argparse.ArgumentParser(description=__doc__) +parser.add_argument("repo", nargs="?", type=Path, default=fixture.parents[1], + help="MXU checkout (defaults to this script's repository)") +parser.add_argument("--ref", help="Read source from a Git revision instead of the working tree") +parser.add_argument("--offline", action="store_true", help="Use only cached Cargo dependencies") +args = parser.parse_args() +repo = args.repo.resolve() + + +def read_repo(path): + if args.ref: + return subprocess.check_output( + ["git", "-C", str(repo), "show", f"{args.ref}:{path}"], encoding="utf-8" + ) + return (repo / path).read_text(encoding="utf-8") + + +def source(name): + return read_repo("src-tauri/src/" + name) + + +def function(code, marker): + start = code.index(marker) + # MXU formats each top-level function's closing brace at column zero. + return code[start:code.index("\n}", start) + 2] + + +packages = tomllib.loads(read_repo("src-tauri/Cargo.lock"))["package"] + + +def locked_version(name): + versions = {package["version"] for package in packages if package["name"] == name} + if len(versions) != 1: + raise ValueError(f"Expected one locked version of {name}, found {versions}") + return versions.pop() + + +system = source("commands/system.rs") +types = source("commands/types.rs") +field = next(line for line in types.splitlines() if "pub pre_action_stop_requests:" in line) +control_types = "" +if "PreActionStops" in field: + control_types = source("commands/pre_action_stops.rs") + +with tempfile.TemporaryDirectory(prefix="mxu-pretask-regression-") as temp: + project = Path(temp) + src = project / "src" + src.mkdir() + shutil.copy(fixture / "harness.rs", src / "main.rs") + (src / "control.rs").write_text(control_types, encoding="utf-8") + (src / "state.rs").write_text( + "#[derive(Default)] pub struct MaaState {\n" + field + "\n}", encoding="utf-8" + ) + (src / "commands.rs").write_text( + "\n\n".join(function(system, marker) for marker in ( + "pub fn set_pre_action_stop(", "pub async fn run_action(", "pub async fn run_pretask_impl(" + )), encoding="utf-8" + ) + (src / "web.rs").write_text( + function(source("web_server.rs"), "async fn handle_stop_pretask("), encoding="utf-8" + ) + (src / "launch.rs").write_text( + function(source("commands/utils.rs"), "pub fn build_launch_command("), encoding="utf-8" + ) + (project / "Cargo.toml").write_text( + '[package]\nname="pretask-fixture"\nversion="0.1.0"\nedition="2021"\n' + '[dependencies]\n' + f'tokio={{version="={locked_version("tokio")}", features=["rt","time","sync"]}}\n' + f'serde_json="={locked_version("serde_json")}"\n' + f'shell-words="={locked_version("shell-words")}"\n', + encoding="utf-8" + ) + env = os.environ.copy() + env["CARGO_TARGET_DIR"] = str(project / "target") + cargo_flags = ["--quiet"] + (["--offline"] if args.offline else []) + print("Source:", args.ref or "working tree", repo, flush=True) + subprocess.run(["cargo", "build", *cargo_flags], cwd=project, env=env, + check=True, timeout=180) + if control_types: + subprocess.run(["cargo", "test", *cargo_flags], cwd=project, env=env, + check=True, timeout=180) + binary = project / "target/debug" / ("pretask-fixture.exe" if os.name == "nt" else "pretask-fixture") + result = subprocess.run([str(binary)], cwd=project, timeout=30, check=False) + raise SystemExit(result.returncode)