From f9f8dd0c54855dbf6a0873407dff6956e81fc122 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Mon, 25 May 2026 12:55:58 -0400 Subject: [PATCH] process: signal PTY foreground group --- .github/workflows/ci.yml | 1 + src/process.rs | 25 +++++++++++++++++++++---- tests/m6_5_repl_acceptance.rs | 28 ++++++++++++++++++++++------ tests/m6_perf_acceptance.rs | 16 +++++++--------- 4 files changed, 51 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 458ab37..a5df65e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -110,6 +110,7 @@ jobs: m6-perf-gates: name: M6 Perf Gates runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable diff --git a/src/process.rs b/src/process.rs index 8777b63..f990711 100644 --- a/src/process.rs +++ b/src/process.rs @@ -507,6 +507,21 @@ impl ChildHandle { } } +fn signal_target(proc: &ManagedProcess, pid: u32) -> Result { + if let Some(runtime) = proc.runtime.as_ref() + && let ChildHandle::Pty { + _master: master, .. + } = &runtime.child + && let Some(pgrp) = master.process_group_leader() + && pgrp > 0 + { + return Ok(Pid::from_raw(-pgrp)); + } + Ok(Pid::from_raw( + i32::try_from(pid).map_err(|e| e.to_string())?, + )) +} + /// Termination status of one generation. Internal --- the supervisor /// translates this into a [`Termination`] with timing. enum TermStatus { @@ -673,8 +688,10 @@ impl ProcessSupervisor { } /// Send `signal` to `id`. Errors if the id is unknown or the - /// process is not currently running. The signal is applied to - /// the OS pid via [`nix::sys::signal::kill`]; nothing about the + /// process is not currently running. Pipe-mode children are + /// signaled by OS pid; PTY-mode children are signaled via the + /// foreground process group when the kernel reports one, matching + /// terminal C-c behavior for shells and REPLs. Nothing about the /// supervisor's state changes synchronously --- the lifecycle /// transition happens when the supervisor next observes the /// child's exit through `tick`. @@ -687,8 +704,8 @@ impl ProcessSupervisor { else { return Err(format!("process {id} is not running")); }; - let nix_pid = Pid::from_raw(i32::try_from(pid).map_err(|e| e.to_string())?); - nix::sys::signal::kill(nix_pid, Some(signal)).map_err(|e| format!("kill: {e}"))?; + let target = signal_target(proc, pid)?; + nix::sys::signal::kill(target, Some(signal)).map_err(|e| format!("kill: {e}"))?; if matches!(signal, Signal::SIGTERM | Signal::SIGKILL | Signal::SIGHUP) { proc.state = ProcessState::Exiting { pid, diff --git a/tests/m6_5_repl_acceptance.rs b/tests/m6_5_repl_acceptance.rs index 8646ca5..e8f3ec3 100644 --- a/tests/m6_5_repl_acceptance.rs +++ b/tests/m6_5_repl_acceptance.rs @@ -273,7 +273,7 @@ fn m6_5_ctrl_d_on_nonempty_input_deletes_char_forward() { "#); } -/// Acceptance bullet 3: C-c sends SIGINT. We spawn `cat` (which +/// Acceptance bullet 3: C-c sends SIGINT. We spawn `sleep` (which /// terminates on SIGINT) and verify the exit marker reports the /// expected signal. The signal name in the marker is symbolic /// ("SIGINT") rather than a number, per the M6.5 design (the libc @@ -285,12 +285,20 @@ fn m6_5_ctrl_d_on_nonempty_input_deletes_char_forward() { /// profile under `cargo test`'s default parallelism). #[test] fn m6_5_ctrl_c_sends_sigint() { - run_with_pump( + let Some(sleep) = locate_shell("sleep") else { + eprintln!("skipping: sleep not on PATH (set PMACS_TEST_SLEEP to override)"); + return; + }; + let setup = format!( r#" - _G.h = pmacs.repl.spawn { argv = { "cat" } } + _G.h = pmacs.repl.spawn {{ argv = {{ "{sleep}", "30" }} }} _G.sigint_sent = false _G.first_seen_running_at = nil "#, + sleep = sleep.display(), + ); + run_with_pump( + &setup, r#" local h = _G.h if not _G.sigint_sent then @@ -324,13 +332,21 @@ fn m6_5_ctrl_c_sends_sigint() { /// The exit marker uses `basename(argv[0])` (so `/usr/bin/cat` /// renders as `cat`), leads with `\n` (so a process exiting mid-line /// stays readable), and uses symbolic signal names. Verified by -/// spawning `/bin/false`, which exits with code 1. +/// spawning `false`, which exits with code 1. #[test] fn m6_5_exit_marker_uses_basename_with_leading_newline() { - run_with_pump( + let Some(false_bin) = locate_shell("false") else { + eprintln!("skipping: false not on PATH (set PMACS_TEST_FALSE to override)"); + return; + }; + let setup = format!( r#" - _G.h = pmacs.repl.spawn { argv = { "/bin/false" } } + _G.h = pmacs.repl.spawn {{ argv = {{ "{false_bin}" }} }} "#, + false_bin = false_bin.display(), + ); + run_with_pump( + &setup, r#" local h = _G.h local buf = h:buffer_id() diff --git a/tests/m6_perf_acceptance.rs b/tests/m6_perf_acceptance.rs index bf7500c..bb6bd24 100644 --- a/tests/m6_perf_acceptance.rs +++ b/tests/m6_perf_acceptance.rs @@ -86,15 +86,13 @@ //! `yes` at the M6.6 ingest rate). Both signals fire in the same //! tick, so the choice is purely a measurement-cost decision. //! -//! - **Why we cancel `yes` directly, not a shell.** `pmacs.process. -//! signal(_proc_id, "INT")` signals the *spawned PID*. For a -//! shell, that's bash itself, not bash's child (which is what -//! `find /` would be). Real-terminal SIGINT semantics involve -//! foreground-pgrp routing through the PTY layer, which M6.5 -//! does not implement. A shell-foreground-pgrp aware C-c is M6.5+ -//! work; for the M6.6 gate, the cleanest measurement is a -//! single-process target (`yes`), which catches SIGINT and exits -//! directly. +//! - **Why we cancel `yes` directly, not a shell.** PTY-mode +//! `pmacs.process.signal(_proc_id, "INT")` targets the foreground +//! process group. For a shell, that group can include the shell's +//! current foreground job rather than only the shell process. For +//! the M6.6 gate, the cleanest measurement is a single-process +//! target (`yes`), so the foreground group contains the producer +//! being measured and no shell/job-control policy enters the timing. //! //! - **Percentile computation.** Sort the latency samples; p99 is //! `samples[(len * 99) / 100]`, matching M5.9c's exact-integer