fix(#665): kill child on timeout, guard running-for display, drop stale doc ref

This commit is contained in:
damocles 2026-06-01 13:13:51 +02:00 committed by mara
commit 083a100ba1
2 changed files with 31 additions and 29 deletions

View file

@ -14,7 +14,9 @@
//! (the process died with the previous harness). Best-effort wake is still
//! sent so the agent is not silently blocked waiting forever.
//!
//! See `docs/bash-runner.md` for the full design rationale.
//! The runner kills the child process on timeout — `tokio::process::Child::drop()`
//! does not kill children, so we explicitly call `child.kill().await` before
//! collecting the copy tasks.
use std::collections::HashSet;
use std::path::{Path, PathBuf};
@ -279,30 +281,17 @@ async fn run_task(mut task: TaskFile, socket: &Path) {
let err_path = task_err(&id);
let timeout = Duration::from_secs(task.timeout_secs);
let exec_result = tokio::time::timeout(
timeout,
exec_cmd(&task.cmd, &out_path, &err_path),
)
.await;
let timed_out;
let exit_code;
match exec_result {
Ok(Ok(code)) => {
timed_out = false;
exit_code = Some(code);
}
Ok(Err(e)) => {
tracing::warn!(id = %id, error = ?e, "bash_runner: exec error");
timed_out = false;
exit_code = None;
}
Err(_elapsed) => {
let (timed_out, exit_code) = match exec_cmd(&task.cmd, &out_path, &err_path, timeout).await {
Ok((code, false)) => (false, Some(code)),
Ok((_, true)) => {
tracing::warn!(id = %id, "bash_runner: task timed out");
timed_out = true;
exit_code = None;
(true, None)
}
}
Err(e) => {
tracing::warn!(id = %id, error = ?e, "bash_runner: exec error");
(false, None)
}
};
let stdout_tail = tail_file(&out_path, SUMMARY_BYTES);
let stderr_tail = tail_file(&err_path, SUMMARY_BYTES);
@ -327,7 +316,9 @@ async fn run_task(mut task: TaskFile, socket: &Path) {
send_wake(socket, &id, &summary, Some((output_snippet, err_snippet))).await;
}
async fn exec_cmd(cmd: &str, out_path: &Path, err_path: &Path) -> Result<i32> {
/// Run `sh -c cmd`, streaming output to files. Returns `(exit_code, timed_out)`.
/// On timeout the child process is explicitly killed before returning.
async fn exec_cmd(cmd: &str, out_path: &Path, err_path: &Path, timeout: Duration) -> Result<(i32, bool)> {
use tokio::process::Command;
let mut child = Command::new("sh")
.arg("-c")
@ -352,11 +343,22 @@ async fn exec_cmd(cmd: &str, out_path: &Path, err_path: &Path) -> Result<i32> {
err_path,
));
let status = child.wait().await?;
let _ = copy_stdout.await;
let _ = copy_stderr.await;
Ok(status.code().unwrap_or(-1))
match tokio::time::timeout(timeout, child.wait()).await {
Ok(Ok(status)) => {
let _ = copy_stdout.await;
let _ = copy_stderr.await;
Ok((status.code().unwrap_or(-1), false))
}
Ok(Err(e)) => Err(e.into()),
Err(_elapsed) => {
// Kill the child so it doesn't linger after timeout.
let _ = child.kill().await;
let _ = child.wait().await; // reap zombie
let _ = copy_stdout.await;
let _ = copy_stderr.await;
Ok((-1, true))
}
}
}
async fn copy_stream_to_file<R>(mut reader: R, path: PathBuf)

View file

@ -442,7 +442,7 @@ fn format_bash_status(id: &str) -> String {
if let Some(code) = task.exit_code {
let _ = write!(out, ", exit={code}");
}
if let Some(t) = task.started_at {
if let Some(t) = task.started_at && task.completed_at.is_none() {
let age = crate::serve_common::now_unix() - t;
let _ = write!(out, ", running for {age}s");
}