From 324f06f6394d9fc1d6f1eef35a7af9bebee23560 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 12:04:02 -0800 Subject: [PATCH] craft-update: wait later for a started process when no thread can wait for it When spawn_detached or the system opener cannot start a thread to wait for the process it started, the process is kept and waited for by a later call once it has exited, so on Unix it does not stay in the process table. --- crates/update/src/process.rs | 73 ++++++++++++++++++++++++++++-------- 1 file changed, 58 insertions(+), 15 deletions(-) diff --git a/crates/update/src/process.rs b/crates/update/src/process.rs index 5aa6d42e..71489934 100644 --- a/crates/update/src/process.rs +++ b/crates/update/src/process.rs @@ -2,7 +2,8 @@ use std::ffi::OsStr; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::{Child, Command, Stdio}; +use std::sync::{Arc, Mutex, PoisonError}; use std::time::{Duration, Instant}; /// A `Command` that won't flash a console window on Windows, and clears AppImage-injected @@ -115,18 +116,47 @@ fn read_all(pipe: Option) -> std::io::Resu } /// Starts `cmd` without waiting for it, and reaps it when it exits. The process keeps running -/// when no thread can be started to reap it; only the wait is skipped. +/// when no thread can be started to reap it; it is then reaped by a later call once it exits. pub fn spawn_detached(mut cmd: Command) -> Result<(), String> { - let mut child = cmd.stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null()).spawn().map_err(|e| format!("couldn't start it ({e})"))?; - let reaper = std::thread::Builder::new().name("reap-child".into()).spawn(move || { - let _ = child.wait(); - }); - if let Err(e) = reaper { - log::warn!("couldn't start a thread to wait for a started process ({e})"); - } + let child = cmd.stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null()).spawn().map_err(|e| format!("couldn't start it ({e})"))?; + reap_in_background(child, "a started process"); Ok(()) } +/// Started processes that no thread waits for, because none could be started. On Unix a process +/// that exits stays in the process table until it is waited for. +static UNREAPED: Mutex> = Mutex::new(Vec::new()); + +/// Waits for `child` on its own thread. When no thread can be started, `child` is kept in +/// [`UNREAPED`] and waited for by a later call once it has exited. +fn reap_in_background(child: Child, what: &str) { + reap_in_background_with(child, what, |wait| std::thread::Builder::new().name("reap-child".into()).spawn(wait).map(drop)); +} + +/// [`reap_in_background`] with `start_thread` in place of starting a thread (tests). +fn reap_in_background_with(child: Child, what: &str, start_thread: impl FnOnce(Box) -> std::io::Result<()>) { + reap_exited(); + let slot = Arc::new(Mutex::new(Some(child))); + let theirs = Arc::clone(&slot); + let wait = Box::new(move || { + let child = theirs.lock().unwrap_or_else(PoisonError::into_inner).take(); + if let Some(mut child) = child { + let _ = child.wait(); + } + }); + if let Err(e) = start_thread(wait) { + log::warn!("couldn't start a thread to wait for {what} ({e})"); + if let Some(child) = slot.lock().unwrap_or_else(PoisonError::into_inner).take() { + UNREAPED.lock().unwrap_or_else(PoisonError::into_inner).push(child); + } + } +} + +/// Waits for the processes in [`UNREAPED`] that have exited, without blocking on the others. +fn reap_exited() { + UNREAPED.lock().unwrap_or_else(PoisonError::into_inner).retain_mut(|child| matches!(child.try_wait(), Ok(None))); +} + /// Waits up to `timeout` for the process `pid` to exit, and says whether it has. A process that /// can't be opened, because it's gone or never was, counts as exited. #[cfg(windows)] @@ -277,12 +307,7 @@ fn open_and_wait_success(target: &str, timeout: Duration) -> bool { Ok(Some(status)) => return status.success(), Ok(None) if start.elapsed() < timeout => std::thread::sleep(Duration::from_millis(25)), Ok(None) => { - let reaper = std::thread::Builder::new().name("reap-child".into()).spawn(move || { - let _ = child.wait(); - }); - if let Err(e) = reaper { - log::warn!("couldn't start a thread to wait for the opener of {target} ({e})"); - } + reap_in_background(child, &format!("the opener of {target}")); return true; } Err(_) => return false, @@ -430,6 +455,24 @@ mod tests { assert_eq!(is_running(Path::new(&requested)), expected, "{requested}"); } } + + /// A process that no thread can wait for is kept, and waited for once it has exited, so it + /// does not stay in the process table. + #[test] + fn a_process_without_a_waiting_thread_is_reaped_later() { + let mut child = command("cat").stdin(Stdio::piped()).stdout(Stdio::null()).stderr(Stdio::null()).spawn().unwrap(); + let stdin = child.stdin.take().unwrap(); + let pid = child.id(); + let kept = || UNREAPED.lock().unwrap_or_else(PoisonError::into_inner).iter().any(|c| c.id() == pid); + reap_in_background_with(child, "a test process", |_wait| Err(std::io::Error::other("no threads"))); + assert!(kept(), "a running process with no thread to wait for it is kept"); + // `cat` exits when its input closes. + drop(stdin); + while kept() { + reap_exited(); + std::thread::yield_now(); + } + } } #[cfg(test)]