Skip to content

Commit f1ea0fd

Browse files
author
超渡法師
committed
fix(cron): atomic write, kill child on timeout, add timeout test
- update_usercron_job: write to .toml.tmp then rename (atomic on POSIX) - check_disable_on_success: use spawn() + wait_with_output() to retain child handle; explicitly kill on timeout to prevent orphan processes - Add disable_on_success_kills_child_on_timeout test (sleep 999 + 1s timeout)
1 parent f8d3d16 commit f1ea0fd

1 file changed

Lines changed: 41 additions & 5 deletions

File tree

src/cron.rs

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -751,24 +751,45 @@ async fn check_disable_on_success(
751751
marker: &str,
752752
) -> DisableOnSuccessResult {
753753
let timeout_secs = job.disable_on_success_timeout_secs.max(1);
754-
let mut child = shell_command(command);
754+
let mut cmd = shell_command(command);
755755
if let Some(dir) = non_empty_opt(job.disable_on_success_working_dir.as_deref()) {
756-
child.current_dir(dir);
756+
cmd.current_dir(dir);
757757
}
758+
cmd.stdout(std::process::Stdio::piped());
759+
cmd.stderr(std::process::Stdio::piped());
758760

759-
let output = match timeout(std::time::Duration::from_secs(timeout_secs), child.output()).await
761+
let mut child = match cmd.spawn() {
762+
Ok(child) => child,
763+
Err(e) => {
764+
warn!(
765+
id = job.id.as_deref().unwrap_or(""),
766+
command,
767+
error = %e,
768+
"disable_on_success command failed to start"
769+
);
770+
return DisableOnSuccessResult::NotAchieved("command failed to start");
771+
}
772+
};
773+
774+
let output = match timeout(
775+
std::time::Duration::from_secs(timeout_secs),
776+
child.wait_with_output(),
777+
)
778+
.await
760779
{
761780
Ok(Ok(output)) => output,
762781
Ok(Err(e)) => {
763782
warn!(
764783
id = job.id.as_deref().unwrap_or(""),
765784
command,
766785
error = %e,
767-
"disable_on_success command failed to start"
786+
"disable_on_success command wait failed"
768787
);
769788
return DisableOnSuccessResult::NotAchieved("command failed to start");
770789
}
771790
Err(_) => {
791+
// Timeout — kill the child to avoid orphan processes.
792+
let _ = child.kill().await;
772793
warn!(
773794
id = job.id.as_deref().unwrap_or(""),
774795
command,
@@ -839,7 +860,10 @@ fn update_usercron_job(
839860
anyhow::bail!("usercron job id {:?} not found", id);
840861
}
841862

842-
std::fs::write(path, doc.to_string())?;
863+
// Atomic write: write to temp file then rename to avoid corruption on crash.
864+
let tmp = path.with_extension("toml.tmp");
865+
std::fs::write(&tmp, doc.to_string())?;
866+
std::fs::rename(&tmp, path)?;
843867
Ok(())
844868
}
845869

@@ -1451,6 +1475,18 @@ message = "a"
14511475
));
14521476
}
14531477

1478+
#[tokio::test]
1479+
async fn disable_on_success_kills_child_on_timeout() {
1480+
let mut job = test_cron_job();
1481+
job.disable_on_success_timeout_secs = 1;
1482+
1483+
let result = check_disable_on_success(&job, "sleep 999", "SUCCESS").await;
1484+
assert!(matches!(
1485+
result,
1486+
DisableOnSuccessResult::NotAchieved("command timed out")
1487+
));
1488+
}
1489+
14541490
fn test_cron_job() -> CronJobConfig {
14551491
CronJobConfig {
14561492
id: Some("goal".into()),

0 commit comments

Comments
 (0)