diff --git a/.changes/shell-process-group.md b/.changes/shell-process-group.md new file mode 100644 index 0000000000..3a9bc68323 --- /dev/null +++ b/.changes/shell-process-group.md @@ -0,0 +1,6 @@ +--- +"shell": minor:feat +"shell-js": minor +--- + +Add `processGroup` option to spawn commands in a new process group (POSIX) or job object (Windows), allowing the entire process tree to be killed when calling `kill()` on the child process. diff --git a/Cargo.lock b/Cargo.lock index f4c95b5faf..1dd84d7852 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4608,6 +4608,18 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "process-wrap" +version = "8.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a3ef4f2f0422f23a82ec9f628ea2acd12871c81a9362b02c43c1aa86acfc3ba1" +dependencies = [ + "indexmap 2.11.4", + "nix 0.30.1", + "tracing", + "windows 0.61.1", +] + [[package]] name = "psl-types" version = "2.0.11" @@ -5633,16 +5645,6 @@ dependencies = [ "digest", ] -[[package]] -name = "shared_child" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "09fa9338aed9a1df411814a5b2252f7cd206c55ae9bf2fa763f8de84603aa60c" -dependencies = [ - "libc", - "windows-sys 0.59.0", -] - [[package]] name = "shlex" version = "1.3.0" @@ -6769,14 +6771,15 @@ name = "tauri-plugin-shell" version = "2.3.5" dependencies = [ "encoding_rs", + "libc", "log", "open", "os_pipe", + "process-wrap", "regex", "schemars", "serde", "serde_json", - "shared_child", "tauri", "tauri-plugin", "thiserror 2.0.12", diff --git a/plugins/shell/Cargo.toml b/plugins/shell/Cargo.toml index fbbd51f42c..0775b53c4e 100644 --- a/plugins/shell/Cargo.toml +++ b/plugins/shell/Cargo.toml @@ -28,11 +28,20 @@ tauri = { workspace = true } tokio = { version = "1", features = ["time"] } log = { workspace = true } thiserror = { workspace = true } -shared_child = "1" regex = "1" open = { version = "5", features = ["shellexecute-on-windows"] } encoding_rs = "0.8" os_pipe = "1" +process-wrap = { version = "8.2", default-features = false, features = [ + "std", + "process-group", + "creation-flags", + "job-object", + "tracing", +] } + +[target.'cfg(unix)'.dependencies] +libc = "0.2" [target.'cfg(target_os = "ios")'.dependencies] tauri = { workspace = true, features = ["wry"] } diff --git a/plugins/shell/guest-js/index.ts b/plugins/shell/guest-js/index.ts index 081d54c132..7bb9153572 100644 --- a/plugins/shell/guest-js/index.ts +++ b/plugins/shell/guest-js/index.ts @@ -79,6 +79,16 @@ interface SpawnOptions { * @since 2.0.0 * */ encoding?: string + /** + * When enabled, spawns the child process in its own process group (POSIX) + * or job object (Windows). This allows killing the entire process tree + * when calling `kill()` on the child process. + * + * Useful for programs that spawn child processes, such as PyInstaller wrappers. + * + * Defaults to `false`. + */ + processGroup?: boolean } /** @ignore */ diff --git a/plugins/shell/src/commands.rs b/plugins/shell/src/commands.rs index 0facce7193..882d274e08 100644 --- a/plugins/shell/src/commands.rs +++ b/plugins/shell/src/commands.rs @@ -88,6 +88,10 @@ pub struct CommandOptions { env: Option>, // Character encoding for stdout/stderr encoding: Option, + // Spawn the child in a new process group (POSIX) or job object (Windows). + // When enabled, killing the child also kills all processes in the group. + #[serde(default)] + process_group: bool, } #[allow(clippy::unnecessary_wraps)] @@ -154,6 +158,9 @@ fn prepare_cmd( } else { command = command.env_clear(); } + if options.process_group { + command = command.set_process_group(true); + } let encoding = match options.encoding { Option::None => EncodingWrapper::Text(None), diff --git a/plugins/shell/src/process/mod.rs b/plugins/shell/src/process/mod.rs index 3d29162d66..828af298c4 100644 --- a/plugins/shell/src/process/mod.rs +++ b/plugins/shell/src/process/mod.rs @@ -7,7 +7,7 @@ use std::{ io::{BufRead, BufReader, Write}, path::{Path, PathBuf}, process::{Command as StdCommand, Stdio}, - sync::{Arc, RwLock}, + sync::{Arc, Mutex, RwLock}, thread::spawn, }; @@ -25,7 +25,6 @@ use tauri::async_runtime::{block_on as block_on_task, channel, Receiver, Sender} pub use encoding_rs::Encoding; use os_pipe::{pipe, PipeReader, PipeWriter}; use serde::Serialize; -use shared_child::SharedChild; use tauri::utils::platform; /// Payload for the [`CommandEvent::Terminated`] command event. @@ -58,12 +57,13 @@ pub enum CommandEvent { pub struct Command { cmd: StdCommand, raw_out: bool, + process_group: bool, } /// Spawned child process. -#[derive(Debug)] pub struct CommandChild { - inner: Arc, + inner: Arc>>, + pid: u32, stdin_writer: PipeWriter, } @@ -74,15 +74,18 @@ impl CommandChild { Ok(()) } - /// Sends a kill signal to the child. + /// Sends a kill signal to the child, then waits for it to exit. + /// When the child was spawned with `process_group` enabled, this kills the + /// entire process group (POSIX) or job object (Windows), reaping every + /// member before returning. pub fn kill(self) -> crate::Result<()> { - self.inner.kill()?; + self.inner.lock().unwrap().kill()?; Ok(()) } /// Returns the process pid. pub fn pid(&self) -> u32 { - self.inner.id() + self.pid } } @@ -175,6 +178,7 @@ impl Command { Self { cmd: command, raw_out: false, + process_group: false, } } @@ -243,6 +247,16 @@ impl Command { self } + /// Configures the command to spawn in a new process group (POSIX) or job object (Windows). + /// + /// When enabled, killing the child process will also kill all processes in the group, + /// which is useful for programs that spawn child processes (e.g. PyInstaller wrappers). + #[must_use] + pub fn set_process_group(mut self, process_group: bool) -> Self { + self.process_group = process_group; + self + } + /// Spawns the command. /// /// # Examples @@ -304,6 +318,7 @@ impl Command { /// ``` pub fn spawn(self) -> crate::Result<(Receiver, CommandChild)> { let raw = self.raw_out; + let process_group = self.process_group; let mut command: StdCommand = self.into(); let (stdout_reader, stdout_writer) = pipe()?; let (stderr_reader, stderr_writer) = pipe()?; @@ -312,11 +327,7 @@ impl Command { command.stderr(stderr_writer); command.stdin(stdin_reader); - let shared_child = SharedChild::spawn(&mut command)?; - let child = Arc::new(shared_child); - let child_ = child.clone(); let guard = Arc::new(RwLock::new(())); - let (tx, rx) = channel(1); spawn_pipe_reader( @@ -334,32 +345,34 @@ impl Command { raw, ); - spawn(move || { - let _ = match child_.wait() { - Ok(status) => { - let _l = guard.write().unwrap(); - block_on_task(async move { - tx.send(CommandEvent::Terminated(TerminatedPayload { - code: status.code(), - #[cfg(windows)] - signal: None, - #[cfg(unix)] - signal: status.signal(), - })) - .await - }) - } - Err(e) => { - let _l = guard.write().unwrap(); - block_on_task(async move { tx.send(CommandEvent::Error(e.to_string())).await }) - } - }; - }); + // Always go through process-wrap so the spawn path is uniform across + // platforms; the `process_group` switch is just an optional wrapper + // rather than a separate child type. + let mut cmd_wrap = process_wrap::std::StdCommandWrap::from(command); + + if process_group { + #[cfg(unix)] + cmd_wrap.wrap(process_wrap::std::ProcessGroup::leader()); + + #[cfg(windows)] + { + cmd_wrap.wrap(process_wrap::std::CreationFlags(CREATE_NO_WINDOW)); + cmd_wrap.wrap(process_wrap::std::JobObject); + } + } + + let wrapped_child = cmd_wrap.spawn()?; + let pid = wrapped_child.id(); + let inner = Arc::new(Mutex::new(wrapped_child)); + let inner_wait = inner.clone(); + + spawn_wait_thread(move || wait_on_child(&inner_wait), tx, guard); Ok(( rx, CommandChild { - inner: child, + inner, + pid, stdin_writer, }, )) @@ -508,6 +521,51 @@ fn spawn_pipe_reader) -> CommandEvent + Send + Copy + 'static>( }); } +/// Polls the child until it exits, returning its final exit status. +/// +/// process-wrap's child wrappers only expose `&mut self` wait methods, so a +/// background blocking wait would hold the lock and starve `kill()`. Polling +/// `try_wait` keeps the lock free between checks; process-wrap caches the exit +/// status, so reaping (including the rest of a process group) settles here. +fn wait_on_child( + inner: &Arc>>, +) -> std::io::Result { + loop { + if let Some(status) = inner.lock().unwrap().try_wait()? { + return Ok(status); + } + std::thread::sleep(std::time::Duration::from_millis(10)); + } +} + +fn spawn_wait_thread( + wait_fn: impl FnOnce() -> std::io::Result + Send + 'static, + tx: Sender, + guard: Arc>, +) { + spawn(move || { + let _ = match wait_fn() { + Ok(status) => { + let _l = guard.write().unwrap(); + block_on_task(async move { + tx.send(CommandEvent::Terminated(TerminatedPayload { + code: status.code(), + #[cfg(windows)] + signal: None, + #[cfg(unix)] + signal: status.signal(), + })) + .await + }) + } + Err(e) => { + let _l = guard.write().unwrap(); + block_on_task(async move { tx.send(CommandEvent::Error(e.to_string())).await }) + } + }; + }); +} + // tests for the commands functions. #[cfg(test)] mod tests { @@ -657,4 +715,166 @@ mod tests { "cat: test/: Is a directory\n\n" ); } + + #[cfg(not(windows))] + #[test] + fn test_cmd_spawn_process_group_output() { + let cmd = Command::new("cat") + .args(["test/test.txt"]) + .set_process_group(true); + let (mut rx, _) = cmd.spawn().unwrap(); + + tauri::async_runtime::block_on(async move { + while let Some(event) = rx.recv().await { + match event { + CommandEvent::Terminated(payload) => { + assert_eq!(payload.code, Some(0)); + } + CommandEvent::Stdout(line) => { + assert_eq!(String::from_utf8(line).unwrap(), "This is a test doc!"); + } + _ => {} + } + } + }); + } + + #[cfg(not(windows))] + #[test] + fn test_cmd_process_group_kill() { + // Spawn a shell that runs a sleep command as a child process. + // With process_group enabled, killing the parent should also kill the child. + let cmd = Command::new("sh") + .args(["-c", "sleep 60"]) + .set_process_group(true); + let (mut rx, child) = cmd.spawn().unwrap(); + let pid = child.pid(); + + // Verify the process is running + let ret = unsafe { libc::kill(pid as i32, 0) }; + assert_eq!(ret, 0, "process should be running"); + + // Kill the process group + child.kill().unwrap(); + + tauri::async_runtime::block_on(async move { + while let Some(event) = rx.recv().await { + if let CommandEvent::Terminated(payload) = event { + // Process was killed by signal, so code is None and signal is Some + assert!(payload.code.is_none() || payload.signal.is_some()); + break; + } + } + }); + + // Verify the process group is gone + let ret = unsafe { libc::killpg(pid as i32, 0) }; + assert_ne!(ret, 0, "process group should no longer exist"); + } + + #[cfg(not(windows))] + #[test] + fn test_cmd_process_group_output() { + let cmd = Command::new("cat") + .args(["test/test.txt"]) + .set_process_group(true); + let output = tauri::async_runtime::block_on(cmd.output()).unwrap(); + + assert_eq!(String::from_utf8(output.stderr).unwrap(), ""); + assert_eq!( + String::from_utf8(output.stdout).unwrap(), + "This is a test doc!\n" + ); + } + + /// End-to-end test simulating the PyInstaller scenario from issue #1332. + /// + /// PyInstaller wraps the real application in a thin bootloader process. + /// Without process groups, killing the bootloader orphans the real app. + /// This test verifies that with `process_group` enabled, killing the + /// wrapper also kills the grandchild process. + #[cfg(not(windows))] + #[test] + fn test_pyinstaller_simulation_without_process_group() { + // Without process_group: killing the wrapper does NOT kill the grandchild. + let cmd = Command::new("sh").args(["test/pyinstaller_sim.sh"]); + let (mut rx, child) = cmd.spawn().unwrap(); + + // Collect the child PID from stdout + let grandchild_pid = tauri::async_runtime::block_on(async { + let mut pid = None; + while let Some(event) = rx.recv().await { + if let CommandEvent::Stdout(line) = &event { + let line_str = String::from_utf8_lossy(line); + if let Some(rest) = line_str.strip_prefix("CHILD_PID=") { + pid = rest.trim().parse::().ok(); + } + } + if pid.is_some() { + break; + } + } + pid.expect("should have received CHILD_PID from script") + }); + + // Verify the grandchild is running + let ret = unsafe { libc::kill(grandchild_pid, 0) }; + assert_eq!(ret, 0, "grandchild should be running before kill"); + + // Kill just the direct child (no process group) + child.kill().unwrap(); + std::thread::sleep(std::time::Duration::from_millis(100)); + + // The grandchild is STILL alive — this is the bug + let ret = unsafe { libc::kill(grandchild_pid, 0) }; + assert_eq!( + ret, 0, + "grandchild should survive when process_group is off" + ); + + // Clean up the orphaned grandchild + unsafe { libc::kill(grandchild_pid, libc::SIGKILL) }; + } + + #[cfg(not(windows))] + #[test] + fn test_pyinstaller_simulation_with_process_group() { + // With process_group: killing the wrapper ALSO kills the grandchild. + let cmd = Command::new("sh") + .args(["test/pyinstaller_sim.sh"]) + .set_process_group(true); + let (mut rx, child) = cmd.spawn().unwrap(); + + // Collect the grandchild PID from stdout + let grandchild_pid = tauri::async_runtime::block_on(async { + let mut pid = None; + while let Some(event) = rx.recv().await { + if let CommandEvent::Stdout(line) = &event { + let line_str = String::from_utf8_lossy(line); + if let Some(rest) = line_str.strip_prefix("CHILD_PID=") { + pid = rest.trim().parse::().ok(); + } + } + if pid.is_some() { + break; + } + } + pid.expect("should have received CHILD_PID from script") + }); + + // Verify the grandchild is running + let ret = unsafe { libc::kill(grandchild_pid, 0) }; + assert_eq!(ret, 0, "grandchild should be running before kill"); + + // Kill the process group + child.kill().unwrap(); + std::thread::sleep(std::time::Duration::from_millis(100)); + + // The grandchild should now be DEAD + let ret = unsafe { libc::kill(grandchild_pid, 0) }; + assert_ne!( + ret, 0, + "grandchild should be killed when process_group is on" + ); + } } diff --git a/plugins/shell/test/pyinstaller_sim.sh b/plugins/shell/test/pyinstaller_sim.sh new file mode 100755 index 0000000000..56c44ddf0d --- /dev/null +++ b/plugins/shell/test/pyinstaller_sim.sh @@ -0,0 +1,21 @@ +#!/bin/bash +# Simulates a PyInstaller-wrapped application. +# +# PyInstaller bundles a thin "bootloader" that spawns the real Python app +# as a child process. When Tauri kills the bootloader, the real app is +# orphaned unless the entire process group is terminated. +# +# This script mimics that pattern: +# - It spawns a long-running child ("the real app") +# - Prints the child's PID so the test harness can verify it was killed +# - Waits on the child (like PyInstaller's bootloader does) + +# "The real application" — a grandchild from Tauri's perspective +sleep 3600 & +CHILD_PID=$! + +echo "WRAPPER_PID=$$" +echo "CHILD_PID=$CHILD_PID" + +# The bootloader waits for the real app to finish +wait $CHILD_PID