From 1f8c04c1aeafacb5531ac11b96040c1904a64601 Mon Sep 17 00:00:00 2001 From: zamuwelle <226855307+zamuwelle@users.noreply.github.com> Date: Mon, 19 Oct 2026 04:53:54 +0800 Subject: [PATCH] fix(session): forward termination signals to running tasks Signals sent to the runner alone (kill , a supervisor, or docker stop) do not reach the task processes it started: the runner exits and the tasks keep running, holding ports and files. Ctrl-C in a terminal reaches them only because the terminal driver signals the whole foreground process group. The session now forwards SIGINT, SIGTERM, and SIGHUP to the registered task processes and cancels the run's tokens, replacing the terminal Ctrl-C handler. The run ends through the regular wait paths once the tasks exit. Each spawned task process is registered while it runs, so a signal is never forwarded to a reused pid (fspy::TrackedChild now exposes the child pid). Refs voidzero-dev/vite-plus#3090. --- CHANGELOG.md | 1 + Cargo.lock | 2 + Cargo.toml | 1 + crates/fspy/src/lib.rs | 4 + crates/fspy/src/unix/mod.rs | 1 + crates/fspy/src/windows/mod.rs | 1 + crates/vt/Cargo.toml | 3 +- crates/vt/src/session/execute/spawn.rs | 16 ++ crates/vt/src/session/mod.rs | 41 +---- crates/vt/src/session/signals.rs | 174 ++++++++++++++++++ crates/vt_bin/Cargo.toml | 3 + crates/vt_bin/src/vtt/exit_on_sigterm.rs | 25 +++ crates/vt_bin/src/vtt/main.rs | 4 +- .../fixtures/signal_forwarding/package.json | 4 + .../signal_forwarding/packages/a/package.json | 6 + .../signal_forwarding/pnpm-workspace.yaml | 2 + .../fixtures/signal_forwarding/snapshots.toml | 36 ++++ .../sigterm_terminates_running_task.md | 20 ++ ...igterm_terminates_running_task__cached_.md | 18 ++ .../fixtures/signal_forwarding/vite-task.json | 3 + crates/vt_bin/tests/e2e_snapshots/main.rs | 70 +++++++ crates/vt_pty_terminal/src/terminal.rs | 12 +- 22 files changed, 413 insertions(+), 34 deletions(-) create mode 100644 crates/vt/src/session/signals.rs create mode 100644 crates/vt_bin/src/vtt/exit_on_sigterm.rs create mode 100644 crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/package.json create mode 100644 crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/packages/a/package.json create mode 100644 crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/pnpm-workspace.yaml create mode 100644 crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/snapshots.toml create mode 100644 crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/snapshots/sigterm_terminates_running_task.md create mode 100644 crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/snapshots/sigterm_terminates_running_task__cached_.md create mode 100644 crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/vite-task.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bca0d5da..464bf94a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- **Fixed** When the task runner itself is stopped with a termination signal (`kill`, a supervisor, or `docker stop`), it now forwards SIGINT, SIGTERM, and SIGHUP to the running tasks so they can shut down cleanly, and waits for them to exit instead of leaving them running behind it. Ctrl-C in the terminal already reached the tasks and keeps working as before ([vite-plus#3090](https://github.com/voidzero-dev/vite-plus/issues/3090), [#863](https://github.com/voidzero-dev/vite-task/pull/863)). - **Fixed** A mistyped or misplaced setting in a task now gets an error that names it. For example, `cache: { outputs: [...] }` now fails with an `unknown field` error that names `outputs` and lists the accepted settings, instead of `data did not match any variant of untagged enum UserTaskDefinition` ([vite-plus#2635](https://github.com/voidzero-dev/vite-plus/issues/2635), [#815](https://github.com/voidzero-dev/vite-task/pull/815)). - **Fixed** In cached tasks on Linux and macOS, programs that pass `-1` as a directory descriptor to calls such as `openat` no longer abort. For example, `ps -p ` from procps-ng 4.0.6 or later aborted when the process had already exited ([#862](https://github.com/voidzero-dev/vite-task/pull/862)). - **Fixed** `vp run` no longer panics before running a script that contains non-ASCII characters, such as `echo こんにちは`, and no longer cuts off such commands where it displays them ([#861](https://github.com/voidzero-dev/vite-task/pull/861)). diff --git a/Cargo.lock b/Cargo.lock index 40dad5d5e..cd50daf08 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4969,6 +4969,7 @@ dependencies = [ "rustc-hash", "serde", "serde_json", + "signal-hook 0.3.18", "supports-color 3.0.2", "tar", "tempfile", @@ -5030,6 +5031,7 @@ dependencies = [ "serde", "serde_json", "shell-escape", + "signal-hook 0.3.18", "tempfile", "tokio", "toml", diff --git a/Cargo.toml b/Cargo.toml index 74ef3590f..eeb661b3a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -136,6 +136,7 @@ serde_json = "1.0.140" serde_norway = "0.9.42" sha2 = "0.11.0" shell-escape = "0.1.5" +signal-hook = "0.3.18" fspy_nostd = { path = "crates/fspy_nostd" } fspy_nostd_alloc = { path = "crates/fspy_nostd_alloc" } similar = "3.0.0" diff --git a/crates/fspy/src/lib.rs b/crates/fspy/src/lib.rs index 0bab440a4..41b7dcfc7 100644 --- a/crates/fspy/src/lib.rs +++ b/crates/fspy/src/lib.rs @@ -49,6 +49,10 @@ pub struct TrackedChild { /// The future that resolves to exit status and path accesses when the process exits. pub wait_handle: BoxFuture<'static, io::Result>, + /// The child's process id, so callers can forward termination signals to + /// it. `None` until the platform reports it. + pub pid: Option, + /// A duplicated process handle of the child, captured before the tokio `Child` /// is moved into the background wait task. This is an independently owned handle /// (via `DuplicateHandle`) so it remains valid even after tokio closes its copy. diff --git a/crates/fspy/src/unix/mod.rs b/crates/fspy/src/unix/mod.rs index e5a8f70c5..1c9a1746a 100644 --- a/crates/fspy/src/unix/mod.rs +++ b/crates/fspy/src/unix/mod.rs @@ -157,6 +157,7 @@ impl SpyImpl { .map_err(SpawnError::OsSpawn)?; Ok(TrackedChild { + pid: child.id(), stdin: child.stdin.take(), stdout: child.stdout.take(), stderr: child.stderr.take(), diff --git a/crates/fspy/src/windows/mod.rs b/crates/fspy/src/windows/mod.rs index 15533b7bb..98d2833bf 100644 --- a/crates/fspy/src/windows/mod.rs +++ b/crates/fspy/src/windows/mod.rs @@ -152,6 +152,7 @@ impl SpyImpl { })?; Ok(TrackedChild { + pid: child.id(), stdin: child.stdin.take(), stdout: child.stdout.take(), stderr: child.stderr.take(), diff --git a/crates/vt/Cargo.toml b/crates/vt/Cargo.toml index 1be50ccaf..4173bafea 100644 --- a/crates/vt/Cargo.toml +++ b/crates/vt/Cargo.toml @@ -73,7 +73,8 @@ tempfile = { workspace = true } fspy = { workspace = true } [target.'cfg(unix)'.dependencies] -nix = { workspace = true, features = ["dir", "poll"] } +nix = { workspace = true, features = ["dir", "poll", "signal"] } +signal-hook = { workspace = true } [target.'cfg(windows)'.dependencies] winapi = { workspace = true, features = ["handleapi", "jobapi2", "winnt"] } diff --git a/crates/vt/src/session/execute/spawn.rs b/crates/vt/src/session/execute/spawn.rs index 7e2abb59e..fa03f9a6a 100644 --- a/crates/vt/src/session/execute/spawn.rs +++ b/crates/vt/src/session/execute/spawn.rs @@ -14,6 +14,8 @@ use tokio::process::{ChildStderr, ChildStdout}; use tokio_util::sync::CancellationToken; use vt_plan::SpawnCommand; +use crate::session::signals::register_child; + /// How the child's stdin/stdout/stderr are configured. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SpawnStdio { @@ -125,6 +127,10 @@ where } let mut tracked = fspy_cmd.spawn(cancellation_token).await?; + // Forward termination signals received by the runner to the task process + // (see `crate::session::signals`). The registration lives until the wait + // future completes. + let registration = register_child(tracked.pid); // On Windows, assign the child to a Job Object so that killing the child // also kills all descendant processes (e.g., node.exe via a .cmd shim). @@ -139,6 +145,9 @@ where let wait_handle = tracked.wait_handle; let wait = async move { + // Keep the task process registered for signal forwarding until it has + // exited, so a termination signal received meanwhile reaches it. + let _registration = registration; let termination = wait_handle.await?; // Drop order: `job` drops here, KILL_ON_JOB_CLOSE kills any descendants // still alive. fspy's wait handle already watched the cancellation @@ -160,6 +169,10 @@ fn spawn_tokio( cancellation_token: CancellationToken, ) -> anyhow::Result { let mut child = cmd.spawn()?; + // Forward termination signals received by the runner to the task process + // (see `crate::session::signals`). The registration lives until the wait + // future completes. + let registration = register_child(child.id()); #[cfg(windows)] let job = { @@ -175,6 +188,9 @@ fn spawn_tokio( let stderr = child.stderr.take(); let wait = async move { + // Keep the task process registered for signal forwarding until it has + // exited, so a termination signal received meanwhile reaches it. + let _registration = registration; let exit_status = tokio::select! { status = child.wait() => status?, () = cancellation_token.cancelled() => { diff --git a/crates/vt/src/session/mod.rs b/crates/vt/src/session/mod.rs index 5d449279b..eb32be6d1 100644 --- a/crates/vt/src/session/mod.rs +++ b/crates/vt/src/session/mod.rs @@ -2,6 +2,7 @@ mod cache; mod event; mod execute; pub(crate) mod reporter; +mod signals; // Re-export types that are part of the public API use std::{ffi::OsStr, fmt::Debug, io::IsTerminal, sync::Arc}; @@ -379,40 +380,18 @@ impl<'a> Session<'a> { self.program_name.clone(), color_support, )); - // Don't let SIGINT/CTRL_C kill the runner. Child tasks receive - // the signal directly from the terminal driver and handle it - // themselves. Cancelling the cancel token prevents scheduling - // new tasks and caching results of in-flight tasks, and stops - // remote cache lookups. It's a child of the fast-fail token, - // so fast-fail cancels it too. Only Ctrl-C cancels the - // interrupt token, which cancels remote cache uploads. - // - // On Windows, an ancestor process (e.g. cargo) may have been - // created with CREATE_NEW_PROCESS_GROUP, which sets a per-process - // flag that silently drops CTRL_C_EVENT before it reaches - // registered handlers. Clear it so our handler fires. - // - // SAFETY: Passing (None, FALSE) clears the inherited - // CTRL_C ignore flag. - #[cfg(windows)] - unsafe { - unsafe extern "system" { - fn SetConsoleCtrlHandler( - handler: Option i32>, - add: i32, - ) -> i32; - } - SetConsoleCtrlHandler(None, 0); - } + // Termination signals are forwarded to the running task + // processes, and cancel the cancel token: see `signals`. As + // before, an in-terminal Ctrl-C is delivered to the tasks by + // the terminal driver, and cancelling the cancel token + // prevents scheduling new tasks and caching results of + // in-flight tasks, and stops remote cache lookups. The + // interrupt token, which only this handler cancels, cancels + // remote cache uploads. let fast_fail_token = tokio_util::sync::CancellationToken::new(); let cancel_token = fast_fail_token.child_token(); let interrupt_token = tokio_util::sync::CancellationToken::new(); - let ct = cancel_token.clone(); - let it = interrupt_token.clone(); - ctrlc::set_handler(move || { - ct.cancel(); - it.cancel(); - })?; + signals::install(cancel_token.clone(), interrupt_token.clone())?; self.execute_graph(graph, builder, fast_fail_token, cancel_token, interrupt_token) .await diff --git a/crates/vt/src/session/signals.rs b/crates/vt/src/session/signals.rs new file mode 100644 index 000000000..185340067 --- /dev/null +++ b/crates/vt/src/session/signals.rs @@ -0,0 +1,174 @@ +//! Forwarding of termination signals to running task processes. +//! +//! A signal sent to the runner alone (`kill `, a supervisor, an editor's +//! stop button) does not reach the task processes that runner started. +//! Without forwarding, the runner exits and the tasks are orphaned, holding +//! ports and files behind it. The terminal is the only sender that reaches +//! tasks on its own, because Ctrl-C goes to the whole foreground process +//! group; this module makes directed signals behave the same way by +//! forwarding SIGINT, SIGTERM and SIGHUP to every registered task process. +//! +//! The handler also cancels the run's tokens, exactly like an in-terminal +//! Ctrl-C: no new tasks are scheduled and in-flight tasks aren't cached. +//! The run then ends through the regular wait paths, once the tasks exit. +//! +//! Forwarding happens on a dedicated thread fed by a self-pipe +//! ([`signal_hook`]), never inside the signal handler itself, which only +//! writes to the pipe (the one async-signal-safe operation available). +//! +//! Task processes stay in the runner's process group, so on a terminal they +//! receive Ctrl-C from the terminal driver *and* through this forwarding. The +//! duplicate is harmless because handling a termination signal is idempotent, +//! and keeping tasks in the foreground group is what lets interactive tasks +//! read from the terminal at all (a separate process group would stop them +//! with SIGTTIN). +//! +//! On Windows there are no Unix-style signals. Ctrl-C console events reach +//! every process attached to the console, and cancelling the run's tokens +//! terminates the task trees through their Job Objects, so the handler only +//! cancels the tokens there. + +use std::io; + +use tokio_util::sync::CancellationToken; + +/// Registers a spawned task process so that termination signals received by +/// this process are forwarded to it. +/// +/// Callers keep the registration for as long as the process runs and drop it +/// once the process has exited, so signals are never forwarded to a process +/// id that has been reaped and possibly reused. +pub struct ChildRegistration { + #[cfg(unix)] + pid: Option, +} + +#[cfg(unix)] +impl Drop for ChildRegistration { + fn drop(&mut self) { + let Some(pid) = self.pid else { + return; + }; + CHILDREN.lock().unwrap_or_else(std::sync::PoisonError::into_inner).remove(&pid); + } +} + +/// Registers `pid` for signal forwarding. `None` (the process id is unknown) +/// registers nothing. +pub fn register_child(pid: Option) -> ChildRegistration { + #[cfg(unix)] + if let Some(pid) = pid { + CHILDREN.lock().unwrap_or_else(std::sync::PoisonError::into_inner).insert(pid); + } + #[cfg(not(unix))] + let _ = pid; + ChildRegistration { + #[cfg(unix)] + pid, + } +} + +/// Installs the signal handler and points it at the current run's tokens. +/// +/// The handler thread is started once per process; later calls only update the +/// tokens. Every call registers the handler for its tokens before returning, +/// so the caller can rely on signals being handled from here on. +#[cfg(unix)] +pub fn install(cancel: CancellationToken, interrupt: CancellationToken) -> io::Result<()> { + use std::{ + sync::{OnceLock, PoisonError}, + thread, + }; + + use signal_hook::{ + consts::{SIGHUP, SIGINT, SIGTERM}, + iterator::Signals, + }; + + static STARTED: OnceLock<()> = OnceLock::new(); + + *TARGETS.lock().unwrap_or_else(PoisonError::into_inner) = Some(Targets { cancel, interrupt }); + + if STARTED.get().is_some() { + return Ok(()); + } + let mut signals = Signals::new([SIGINT, SIGTERM, SIGHUP])?; + let _ = STARTED.set(()); + thread::Builder::new().name("vt-signal-forward".to_string()).spawn(move || { + for signal in signals.forever() { + handle(signal); + } + })?; + Ok(()) +} + +/// Cancels the current run's tokens and forwards `signal` to every registered +/// task process. +#[cfg(unix)] +fn handle(signal: i32) { + use std::sync::PoisonError; + + use nix::{ + sys::signal::{Signal, kill}, + unistd::Pid, + }; + + if let Some(targets) = TARGETS.lock().unwrap_or_else(PoisonError::into_inner).as_ref() { + targets.cancel.cancel(); + targets.interrupt.cancel(); + } + + let Ok(signal) = Signal::try_from(signal) else { + return; + }; + let children: Vec = + CHILDREN.lock().unwrap_or_else(PoisonError::into_inner).iter().copied().collect(); + for pid in children { + let _ = kill(Pid::from_raw(pid.cast_signed()), signal); + } +} + +/// On Windows, console Ctrl-C events already reach every process attached to +/// the console, and the Job Object assigned to each task kills its whole +/// process tree when the run's tokens are cancelled. The handler therefore +/// only cancels the tokens. +#[cfg(windows)] +pub(crate) fn install(cancel: CancellationToken, interrupt: CancellationToken) -> io::Result<()> { + // An ancestor process (e.g. cargo) may have been created with + // CREATE_NEW_PROCESS_GROUP, which sets a per-process flag that silently + // drops CTRL_C_EVENT before it reaches registered handlers. Clear it so + // our handler fires. + // + // SAFETY: Passing (None, FALSE) clears the inherited CTRL_C ignore flag. + unsafe { + unsafe extern "system" { + fn SetConsoleCtrlHandler( + handler: Option i32>, + add: i32, + ) -> i32; + } + SetConsoleCtrlHandler(None, 0); + } + ctrlc::set_handler(move || { + cancel.cancel(); + interrupt.cancel(); + }) + .map_err(io::Error::other) +} + +/// Task processes to forward termination signals to. +#[cfg(unix)] +static CHILDREN: std::sync::Mutex> = + std::sync::Mutex::new(std::collections::BTreeSet::new()); + +/// The tokens of the run in progress. +#[cfg(unix)] +static TARGETS: std::sync::Mutex> = std::sync::Mutex::new(None); + +/// The tokens a forwarded signal cancels, mirroring the in-terminal Ctrl-C +/// handler this module replaced. +#[cfg(unix)] +struct Targets { + cancel: CancellationToken, + interrupt: CancellationToken, +} diff --git a/crates/vt_bin/Cargo.toml b/crates/vt_bin/Cargo.toml index 0b39cb66a..2054b9126 100644 --- a/crates/vt_bin/Cargo.toml +++ b/crates/vt_bin/Cargo.toml @@ -31,6 +31,9 @@ vt_str = { workspace = true } vt = { workspace = true } which = { workspace = true } +[target.'cfg(unix)'.dependencies] +signal-hook = { workspace = true } + [target.'cfg(target_os = "linux")'.dependencies] nix = { workspace = true, features = ["mount", "sched", "user"] } diff --git a/crates/vt_bin/src/vtt/exit_on_sigterm.rs b/crates/vt_bin/src/vtt/exit_on_sigterm.rs new file mode 100644 index 000000000..43149abc4 --- /dev/null +++ b/crates/vt_bin/src/vtt/exit_on_sigterm.rs @@ -0,0 +1,25 @@ +/// exit-on-sigterm +/// +/// Sets up a SIGTERM handler, emits a "ready" milestone, then waits. +/// When SIGTERM is received, prints "sigterm received" and exits. +#[cfg(unix)] +pub fn run() -> Result<(), Box> { + use signal_hook::{consts::SIGTERM, iterator::Signals}; + + let mut signals = Signals::new([SIGTERM])?; + + vt_pty_terminal_test_client::mark_milestone("ready"); + + for signal in signals.forever() { + if signal == SIGTERM { + break; + } + } + println!("sigterm received"); + Ok(()) +} + +#[cfg(not(unix))] +pub fn run() -> Result<(), Box> { + Err("vtt exit-on-sigterm is only supported on Unix".into()) +} diff --git a/crates/vt_bin/src/vtt/main.rs b/crates/vt_bin/src/vtt/main.rs index e77c719df..18f7c3411 100644 --- a/crates/vt_bin/src/vtt/main.rs +++ b/crates/vt_bin/src/vtt/main.rs @@ -11,6 +11,7 @@ mod check_tty; mod cp; mod exit; mod exit_on_ctrlc; +mod exit_on_sigterm; mod grep_file; mod list_dir; mod mkdir; @@ -36,7 +37,7 @@ fn main() { if args.len() < 2 { eprintln!("Usage: vtt [args...]"); eprintln!( - "Subcommands: barrier, check-tty, cp, exit, exit-on-ctrlc, grep-file, list-dir, mkdir, pipe-stdin, print, print-color, print-cwd, print-env, print-file, read-stdin, replace-file-content, rm, small_dev_shm, stalled-remote-cache, stat-file, stat-many, touch-file, write-file" + "Subcommands: barrier, check-tty, cp, exit, exit-on-ctrlc, exit-on-sigterm, grep-file, list-dir, mkdir, pipe-stdin, print, print-color, print-cwd, print-env, print-file, read-stdin, replace-file-content, rm, small_dev_shm, stalled-remote-cache, stat-file, stat-many, touch-file, write-file" ); std::process::exit(1); } @@ -50,6 +51,7 @@ fn main() { "cp" => cp::run(&args[2..]), "exit" => exit::run(&args[2..]), "exit-on-ctrlc" => exit_on_ctrlc::run(), + "exit-on-sigterm" => exit_on_sigterm::run(), "grep-file" => { grep_file::run(&args[2..]); Ok(()) diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/package.json b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/package.json new file mode 100644 index 000000000..e9d49a47d --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/package.json @@ -0,0 +1,4 @@ +{ + "name": "signal-forwarding-test", + "private": true +} diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/packages/a/package.json b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/packages/a/package.json new file mode 100644 index 000000000..3223471c7 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/packages/a/package.json @@ -0,0 +1,6 @@ +{ + "name": "@signal-forwarding/a", + "scripts": { + "dev": "vtt exit-on-sigterm" + } +} diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/pnpm-workspace.yaml b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/pnpm-workspace.yaml new file mode 100644 index 000000000..924b55f42 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/pnpm-workspace.yaml @@ -0,0 +1,2 @@ +packages: + - packages/* diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/snapshots.toml b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/snapshots.toml new file mode 100644 index 000000000..aedc9b4d7 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/snapshots.toml @@ -0,0 +1,36 @@ +[[e2e]] +name = "sigterm_terminates_running_task" +comment = """ +SIGTERM sent to the runner alone (as `kill `, a supervisor, or a container +runtime would) must reach the running task: the task receives SIGTERM, exits, +and the runner waits for it instead of exiting immediately and orphaning it. +""" +cfg = "unix" +steps = [ + { argv = [ + "vt", + "run", + "--no-cache", + "@signal-forwarding/a#dev", + ], interactions = [ + { "expect-milestone" = "ready" }, + { signal = "sigterm" }, + ] }, +] + +[[e2e]] +name = "sigterm_terminates_running_task__cached_" +comment = """ +Same as above, but under the cached execution path (piped stdio + fspy tracking). +""" +cfg = "unix" +steps = [ + { argv = [ + "vt", + "run", + "@signal-forwarding/a#dev", + ], interactions = [ + { "expect-milestone" = "ready" }, + { signal = "sigterm" }, + ] }, +] diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/snapshots/sigterm_terminates_running_task.md b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/snapshots/sigterm_terminates_running_task.md new file mode 100644 index 000000000..ca3f9facd --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/snapshots/sigterm_terminates_running_task.md @@ -0,0 +1,20 @@ +# sigterm_terminates_running_task + +SIGTERM sent to the runner alone (as `kill `, a supervisor, or a container +runtime would) must reach the running task: the task receives SIGTERM, exits, +and the runner waits for it instead of exiting immediately and orphaning it. + +## `vt run --no-cache @signal-forwarding/a#dev` + +**→ expect-milestone:** `ready` + +``` +~/packages/a$ vtt exit-on-sigterm ⊘ cache disabled +``` + +**← signal:** `sigterm` + +``` +~/packages/a$ vtt exit-on-sigterm ⊘ cache disabled +sigterm received +``` diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/snapshots/sigterm_terminates_running_task__cached_.md b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/snapshots/sigterm_terminates_running_task__cached_.md new file mode 100644 index 000000000..86c92a101 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/snapshots/sigterm_terminates_running_task__cached_.md @@ -0,0 +1,18 @@ +# sigterm_terminates_running_task__cached_ + +Same as above, but under the cached execution path (piped stdio + fspy tracking). + +## `vt run @signal-forwarding/a#dev` + +**→ expect-milestone:** `ready` + +``` +~/packages/a$ vtt exit-on-sigterm +``` + +**← signal:** `sigterm` + +``` +~/packages/a$ vtt exit-on-sigterm +sigterm received +``` diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/vite-task.json b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/vite-task.json new file mode 100644 index 000000000..d548edfac --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/signal_forwarding/vite-task.json @@ -0,0 +1,3 @@ +{ + "cache": true +} diff --git a/crates/vt_bin/tests/e2e_snapshots/main.rs b/crates/vt_bin/tests/e2e_snapshots/main.rs index 0e4d1c819..d11483e77 100644 --- a/crates/vt_bin/tests/e2e_snapshots/main.rs +++ b/crates/vt_bin/tests/e2e_snapshots/main.rs @@ -157,6 +157,7 @@ enum Interaction { Write(WriteInteraction), WriteLine(WriteLineInteraction), WriteKey(WriteKeyInteraction), + Signal(SignalInteraction), } #[derive(serde::Deserialize, Debug, Clone)] @@ -186,6 +187,64 @@ struct WriteKeyInteraction { write_key: WriteKey, } +/// Sends `signal` directly to the PTY child process (the runner under test), +/// as `kill `, a supervisor, or a container runtime would. Unlike +/// `write-key = "ctrl-c"`, which the terminal driver turns into a +/// process-group-wide SIGINT, this targets only the runner, so task processes +/// observe the signal only if the runner forwards it. +#[derive(serde::Deserialize, Debug, Clone)] +#[serde(deny_unknown_fields)] +struct SignalInteraction { + signal: SignalName, +} + +#[derive(serde::Deserialize, Debug, Clone, Copy)] +#[serde(rename_all = "kebab-case")] +enum SignalName { + Sigint, + Sigterm, + Sighup, +} + +impl SignalName { + const fn as_str(self) -> &'static str { + match self { + Self::Sigint => "sigint", + Self::Sigterm => "sigterm", + Self::Sighup => "sighup", + } + } + + #[cfg(unix)] + const fn number(self) -> i32 { + match self { + Self::Sigint => libc::SIGINT, + Self::Sigterm => libc::SIGTERM, + Self::Sighup => libc::SIGHUP, + } + } +} + +/// Delivers `signal` to `pid`, the PTY child spawned by the harness. +#[cfg(unix)] +fn send_signal(pid: u32, signal: SignalName) { + // SAFETY: `pid` identifies a live process spawned by the harness that has + // not been waited on yet, and `kill` only posts a signal to it. + let result = unsafe { libc::kill(pid.cast_signed(), signal.number()) }; + assert_eq!( + result, + 0, + "kill({pid}, {}) failed: {}", + signal.as_str(), + std::io::Error::last_os_error() + ); +} + +#[cfg(not(unix))] +fn send_signal(_pid: u32, signal: SignalName) { + panic!("the 'signal: {}' interaction is only supported on Unix", signal.as_str()); +} + #[derive(serde::Deserialize, Debug, Clone, Copy)] #[serde(rename_all = "kebab-case")] enum WriteKey { @@ -594,6 +653,17 @@ fn run_case( terminal.writer.write_all(write_key.write_key.bytes()).unwrap(); terminal.writer.flush().unwrap(); } + Interaction::Signal(signal) => { + output_for_thread.lock().unwrap().push_str( + vt_str::format!("**← signal:** `{}`\n\n", signal.signal.as_str()) + .as_str(), + ); + let pid = terminal + .child_handle + .pid() + .expect("the child process reports no pid to signal"); + send_signal(pid, signal.signal); + } } } diff --git a/crates/vt_pty_terminal/src/terminal.rs b/crates/vt_pty_terminal/src/terminal.rs index 3382cfff4..39867177f 100644 --- a/crates/vt_pty_terminal/src/terminal.rs +++ b/crates/vt_pty_terminal/src/terminal.rs @@ -35,6 +35,7 @@ pub struct PtyWriter { pub struct ChildHandle { child_killer: Box, exit_status: Arc>, + pid: Option, } impl Clone for ChildHandle { @@ -42,6 +43,7 @@ impl Clone for ChildHandle { Self { child_killer: self.child_killer.clone_killer(), exit_status: Arc::clone(&self.exit_status), + pid: self.pid, } } } @@ -291,6 +293,13 @@ impl ChildHandle { self.child_killer.kill()?; Ok(()) } + + /// Returns the process id of the child process, when the platform reports + /// one. + #[must_use] + pub const fn pid(&self) -> Option { + self.pid + } } impl Terminal { @@ -322,6 +331,7 @@ impl Terminal { Arc::new(Mutex::new(Some(pty_pair.master.take_writer()?))); let mut child = pty_pair.slave.spawn_command(cmd)?; let child_killer = child.clone_killer(); + let pid = child.process_id(); let master = Arc::new(Mutex::new(Some(pty_pair.master))); let exit_status: Arc> = Arc::new(OnceLock::new()); @@ -373,7 +383,7 @@ impl Terminal { Ok(Self { pty_reader: PtyReader { reader, parser: Arc::clone(&parser) }, pty_writer: PtyWriter { writer, parser, master }, - child_handle: ChildHandle { child_killer, exit_status }, + child_handle: ChildHandle { child_killer, exit_status, pid }, }) } }