From 9d0d9ef700cc1ae538a4e87abee1bd2ddbb4d40f Mon Sep 17 00:00:00 2001 From: Robbie Clark Date: Thu, 19 Mar 2026 16:53:40 +0000 Subject: [PATCH 1/5] feat(shell): add process group spawn option (fix #1332) Add a `processGroup` boolean option to the shell plugin's spawn command. When enabled, the child process is spawned in its own process group (POSIX) or job object (Windows) using the `process-wrap` crate, so that killing the child also kills the entire process tree. This fixes the issue where programs like PyInstaller wrappers spawn a child process that gets orphaned when Tauri kills the parent. --- .changes/shell-process-group.md | 6 + Cargo.lock | 14 ++ plugins/shell/Cargo.toml | 4 + plugins/shell/guest-js/index.ts | 10 ++ plugins/shell/src/commands.rs | 7 + plugins/shell/src/process/mod.rs | 280 +++++++++++++++++++++++++++---- 6 files changed, 292 insertions(+), 29 deletions(-) create mode 100644 .changes/shell-process-group.md diff --git a/.changes/shell-process-group.md b/.changes/shell-process-group.md new file mode 100644 index 0000000000..0209a534f1 --- /dev/null +++ b/.changes/shell-process-group.md @@ -0,0 +1,6 @@ +--- +"tauri-plugin-shell": minor:feat +"@tauri-apps/plugin-shell": minor:feat +--- + +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 5b08920e7d..a0cde10fc0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4718,6 +4718,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.9.0", + "nix 0.30.1", + "tracing", + "windows 0.61.1", +] + [[package]] name = "psl-types" version = "2.0.11" @@ -6944,9 +6956,11 @@ name = "tauri-plugin-shell" version = "2.3.5" dependencies = [ "encoding_rs", + "libc", "log", "open", "os_pipe", + "process-wrap", "regex", "schemars", "serde", diff --git a/plugins/shell/Cargo.toml b/plugins/shell/Cargo.toml index fbbd51f42c..b312455b65 100644 --- a/plugins/shell/Cargo.toml +++ b/plugins/shell/Cargo.toml @@ -33,6 +33,10 @@ regex = "1" open = { version = "5", features = ["shellexecute-on-windows"] } encoding_rs = "0.8" os_pipe = "1" +process-wrap = { version = "8.2", features = ["std"] } + +[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..fcf2789df4 100644 --- a/plugins/shell/src/process/mod.rs +++ b/plugins/shell/src/process/mod.rs @@ -58,15 +58,79 @@ 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: ChildKind, stdin_writer: PipeWriter, } +enum ChildKind { + Direct(Arc), + #[cfg(any(unix, windows))] + ProcessGroup(GroupChild), +} + +#[cfg(unix)] +struct GroupChild { + shared: Arc, + pgid: i32, +} + +#[cfg(unix)] +impl GroupChild { + fn id(&self) -> u32 { + self.shared.id() + } + + fn kill(&self) -> std::io::Result<()> { + // SAFETY: killpg is a standard POSIX syscall. The pgid was obtained from + // the child's PID, which equals its PGID since it was spawned as a group leader. + let ret = unsafe { libc::killpg(self.pgid, libc::SIGKILL) }; + if ret == 0 { + Ok(()) + } else { + Err(std::io::Error::last_os_error()) + } + } +} + +#[cfg(windows)] +struct GroupChild { + inner: Arc>>, + id: u32, +} + +#[cfg(windows)] +impl GroupChild { + fn id(&self) -> u32 { + self.id + } + + fn kill(&self) -> std::io::Result<()> { + self.inner.lock().unwrap().kill() + } + + fn wait(&self) -> std::io::Result { + loop { + match self.inner.lock().unwrap().try_wait()? { + Some(status) => return Ok(status), + None => {} + } + std::thread::sleep(std::time::Duration::from_millis(10)); + } + } + + fn clone_for_wait(&self) -> Self { + Self { + inner: self.inner.clone(), + id: self.id, + } + } +} + impl CommandChild { /// Writes to process stdin. pub fn write(&mut self, buf: &[u8]) -> crate::Result<()> { @@ -75,14 +139,24 @@ impl CommandChild { } /// Sends a kill signal to the child. + /// When the child was spawned with `process_group` enabled, + /// this kills the entire process group (POSIX) or job object (Windows). pub fn kill(self) -> crate::Result<()> { - self.inner.kill()?; + match self.inner { + ChildKind::Direct(child) => child.kill()?, + #[cfg(any(unix, windows))] + ChildKind::ProcessGroup(group) => group.kill()?, + } Ok(()) } /// Returns the process pid. pub fn pid(&self) -> u32 { - self.inner.id() + match &self.inner { + ChildKind::Direct(child) => child.id(), + #[cfg(any(unix, windows))] + ChildKind::ProcessGroup(group) => group.id(), + } } } @@ -175,6 +249,7 @@ impl Command { Self { cmd: command, raw_out: false, + process_group: false, } } @@ -243,6 +318,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 +389,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 +398,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 +416,73 @@ 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 }) + let child_kind = if process_group { + #[cfg(any(unix, windows))] + { + let mut cmd_wrap = process_wrap::std::StdCommandWrap::from(command); + + #[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 child_id = wrapped_child.id(); + + #[cfg(unix)] + let group = { + let inner_child = wrapped_child.into_inner(); + let shared = Arc::new(SharedChild::new(inner_child)?); + let shared_clone = shared.clone(); + let pgid = child_id as i32; + + spawn_wait_thread(move || shared_clone.wait(), tx, guard); + + GroupChild { shared, pgid } + }; + + #[cfg(windows)] + let group = { + let group = GroupChild { + inner: Arc::new(std::sync::Mutex::new(wrapped_child)), + id: child_id, + }; + let group_wait = group.clone_for_wait(); + + spawn_wait_thread(move || group_wait.wait(), tx, guard); + + group + }; + + ChildKind::ProcessGroup(group) + } + + #[cfg(not(any(unix, windows)))] + { + return Err(std::io::Error::new( + std::io::ErrorKind::Unsupported, + "process groups are not supported on this platform", + ) + .into()); + } + } else { + let shared_child = SharedChild::spawn(&mut command)?; + let child = Arc::new(shared_child); + let child_ = child.clone(); + + spawn_wait_thread(move || child_.wait(), tx, guard); + + ChildKind::Direct(child) + }; Ok(( rx, CommandChild { - inner: child, + inner: child_kind, stdin_writer, }, )) @@ -508,6 +631,34 @@ fn spawn_pipe_reader) -> CommandEvent + Send + Copy + 'static>( }); } +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 +808,75 @@ 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" + ); + } } From 883b0455ececea3f12df042807ecbcb538cc68b7 Mon Sep 17 00:00:00 2001 From: Robbie Clark Date: Thu, 19 Mar 2026 17:01:22 +0000 Subject: [PATCH 2/5] test(shell): add PyInstaller simulation tests for process group kill Add end-to-end tests that reproduce the exact scenario from issue #1332: a wrapper process (like PyInstaller's bootloader) spawns a grandchild. - Without process_group: killing the wrapper orphans the grandchild - With process_group: killing the wrapper also kills the grandchild --- plugins/shell/src/process/mod.rs | 85 +++++++++++++++++++++++++++ plugins/shell/test/pyinstaller_sim.sh | 21 +++++++ 2 files changed, 106 insertions(+) create mode 100755 plugins/shell/test/pyinstaller_sim.sh diff --git a/plugins/shell/src/process/mod.rs b/plugins/shell/src/process/mod.rs index fcf2789df4..843e818170 100644 --- a/plugins/shell/src/process/mod.rs +++ b/plugins/shell/src/process/mod.rs @@ -879,4 +879,89 @@ mod tests { "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 From 2b7c74ab677fa589a32717cdb8922666d6483cce Mon Sep 17 00:00:00 2001 From: Robbie Clark Date: Wed, 24 Jun 2026 18:00:22 +0100 Subject: [PATCH 3/5] refactor(shell): drop redundant cfg gates, trim process-wrap features Address review feedback on #3351: - Remove `#[cfg(any(unix, windows))]` gates on `ChildKind::ProcessGroup`, the kill/pid match arms, and the spawn block, plus the now-dead `#[cfg(not(any(unix, windows)))]` fallback. That cfg only excluded wasm, which this plugin never builds for. - Set `default-features = false` on process-wrap and enable only the features actually used: std, process-group (unix), creation-flags and job-object (windows). Drops kill-on-drop, process-session, and tracing. Co-Authored-By: Claude Opus 4.8 (1M context) --- Cargo.lock | 3 +- plugins/shell/Cargo.toml | 7 ++- plugins/shell/src/process/mod.rs | 75 +++++++++++++------------------- 3 files changed, 37 insertions(+), 48 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1e0a1c7e79..39bfad2ced 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4614,9 +4614,8 @@ version = "8.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a3ef4f2f0422f23a82ec9f628ea2acd12871c81a9362b02c43c1aa86acfc3ba1" dependencies = [ - "indexmap 2.9.0", + "indexmap 2.11.4", "nix 0.30.1", - "tracing", "windows 0.61.1", ] diff --git a/plugins/shell/Cargo.toml b/plugins/shell/Cargo.toml index b312455b65..d7c951a7cf 100644 --- a/plugins/shell/Cargo.toml +++ b/plugins/shell/Cargo.toml @@ -33,7 +33,12 @@ regex = "1" open = { version = "5", features = ["shellexecute-on-windows"] } encoding_rs = "0.8" os_pipe = "1" -process-wrap = { version = "8.2", features = ["std"] } +process-wrap = { version = "8.2", default-features = false, features = [ + "std", + "process-group", + "creation-flags", + "job-object", +] } [target.'cfg(unix)'.dependencies] libc = "0.2" diff --git a/plugins/shell/src/process/mod.rs b/plugins/shell/src/process/mod.rs index 843e818170..a4fb50b388 100644 --- a/plugins/shell/src/process/mod.rs +++ b/plugins/shell/src/process/mod.rs @@ -69,7 +69,6 @@ pub struct CommandChild { enum ChildKind { Direct(Arc), - #[cfg(any(unix, windows))] ProcessGroup(GroupChild), } @@ -144,7 +143,6 @@ impl CommandChild { pub fn kill(self) -> crate::Result<()> { match self.inner { ChildKind::Direct(child) => child.kill()?, - #[cfg(any(unix, windows))] ChildKind::ProcessGroup(group) => group.kill()?, } Ok(()) @@ -154,7 +152,6 @@ impl CommandChild { pub fn pid(&self) -> u32 { match &self.inner { ChildKind::Direct(child) => child.id(), - #[cfg(any(unix, windows))] ChildKind::ProcessGroup(group) => group.id(), } } @@ -417,58 +414,46 @@ impl Command { ); let child_kind = if process_group { - #[cfg(any(unix, windows))] - { - let mut cmd_wrap = process_wrap::std::StdCommandWrap::from(command); - - #[cfg(unix)] - cmd_wrap.wrap(process_wrap::std::ProcessGroup::leader()); + let mut cmd_wrap = process_wrap::std::StdCommandWrap::from(command); - #[cfg(windows)] - { - cmd_wrap.wrap(process_wrap::std::CreationFlags(CREATE_NO_WINDOW)); - cmd_wrap.wrap(process_wrap::std::JobObject); - } + #[cfg(unix)] + cmd_wrap.wrap(process_wrap::std::ProcessGroup::leader()); - let wrapped_child = cmd_wrap.spawn()?; - let child_id = wrapped_child.id(); + #[cfg(windows)] + { + cmd_wrap.wrap(process_wrap::std::CreationFlags(CREATE_NO_WINDOW)); + cmd_wrap.wrap(process_wrap::std::JobObject); + } - #[cfg(unix)] - let group = { - let inner_child = wrapped_child.into_inner(); - let shared = Arc::new(SharedChild::new(inner_child)?); - let shared_clone = shared.clone(); - let pgid = child_id as i32; + let wrapped_child = cmd_wrap.spawn()?; + let child_id = wrapped_child.id(); - spawn_wait_thread(move || shared_clone.wait(), tx, guard); + #[cfg(unix)] + let group = { + let inner_child = wrapped_child.into_inner(); + let shared = Arc::new(SharedChild::new(inner_child)?); + let shared_clone = shared.clone(); + let pgid = child_id as i32; - GroupChild { shared, pgid } - }; + spawn_wait_thread(move || shared_clone.wait(), tx, guard); - #[cfg(windows)] - let group = { - let group = GroupChild { - inner: Arc::new(std::sync::Mutex::new(wrapped_child)), - id: child_id, - }; - let group_wait = group.clone_for_wait(); + GroupChild { shared, pgid } + }; - spawn_wait_thread(move || group_wait.wait(), tx, guard); - - group + #[cfg(windows)] + let group = { + let group = GroupChild { + inner: Arc::new(std::sync::Mutex::new(wrapped_child)), + id: child_id, }; + let group_wait = group.clone_for_wait(); - ChildKind::ProcessGroup(group) - } + spawn_wait_thread(move || group_wait.wait(), tx, guard); - #[cfg(not(any(unix, windows)))] - { - return Err(std::io::Error::new( - std::io::ErrorKind::Unsupported, - "process groups are not supported on this platform", - ) - .into()); - } + group + }; + + ChildKind::ProcessGroup(group) } else { let shared_child = SharedChild::spawn(&mut command)?; let child = Arc::new(shared_child); From 67e04729e562403b9d33d0ed5c97bde7e616a691 Mon Sep 17 00:00:00 2001 From: Robbie Clark Date: Wed, 24 Jun 2026 20:26:35 +0100 Subject: [PATCH 4/5] fix(shell): enable process-wrap tracing feature, fix CI formatting and changefile - Add `tracing` feature to process-wrap: with default-features = false its Windows job_object.rs calls `debug\!` unconditionally while the import is gated behind `#[cfg(feature = "tracing")]`, failing the Windows build with "cannot find macro `debug`". - Reformat process/mod.rs assertions to satisfy `cargo fmt --check`. - Reindent the process-wrap features array to 2 spaces for taplo. - Use covector package keys (`shell` / `shell-js`) in the change file so the check-change-files and covector status jobs pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- .changes/shell-process-group.md | 4 ++-- Cargo.lock | 1 + plugins/shell/Cargo.toml | 9 +++++---- plugins/shell/src/process/mod.rs | 10 ++++++++-- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/.changes/shell-process-group.md b/.changes/shell-process-group.md index 0209a534f1..3a9bc68323 100644 --- a/.changes/shell-process-group.md +++ b/.changes/shell-process-group.md @@ -1,6 +1,6 @@ --- -"tauri-plugin-shell": minor:feat -"@tauri-apps/plugin-shell": minor:feat +"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 39bfad2ced..2e55e2b4ba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4616,6 +4616,7 @@ checksum = "a3ef4f2f0422f23a82ec9f628ea2acd12871c81a9362b02c43c1aa86acfc3ba1" dependencies = [ "indexmap 2.11.4", "nix 0.30.1", + "tracing", "windows 0.61.1", ] diff --git a/plugins/shell/Cargo.toml b/plugins/shell/Cargo.toml index d7c951a7cf..2c83e0f6e7 100644 --- a/plugins/shell/Cargo.toml +++ b/plugins/shell/Cargo.toml @@ -34,10 +34,11 @@ 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", + "std", + "process-group", + "creation-flags", + "job-object", + "tracing", ] } [target.'cfg(unix)'.dependencies] diff --git a/plugins/shell/src/process/mod.rs b/plugins/shell/src/process/mod.rs index a4fb50b388..272582ab41 100644 --- a/plugins/shell/src/process/mod.rs +++ b/plugins/shell/src/process/mod.rs @@ -905,7 +905,10 @@ mod tests { // 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"); + assert_eq!( + ret, 0, + "grandchild should survive when process_group is off" + ); // Clean up the orphaned grandchild unsafe { libc::kill(grandchild_pid, libc::SIGKILL) }; @@ -947,6 +950,9 @@ mod tests { // 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"); + assert_ne!( + ret, 0, + "grandchild should be killed when process_group is on" + ); } } From e8d08018994eec33da5cae46fc464d74b8d71a82 Mon Sep 17 00:00:00 2001 From: Robbie Clark Date: Wed, 24 Jun 2026 21:21:41 +0100 Subject: [PATCH 5/5] refactor(shell): unify child handling on process-wrap, drop SharedChild Collapse the ChildKind enum and the per-platform GroupChild types into a single process-wrap-backed child. The command always spawns through StdCommandWrap; the process_group flag is now just an optional wrapper rather than a switch into a separate child type. This drops the shared_child dependency and resolves two review notes: the Unix path no longer hand-rolls killpg, and kill() now goes through process-wrap's kill (start_kill + wait), which reaps the entire process group instead of only signalling it. A background thread polls try_wait to surface the exit status, since process-wrap's wrappers only expose &mut self wait methods (the reason SharedChild was used on Unix in the first place). Co-Authored-By: Claude Opus 4.8 (1M context) --- Cargo.lock | 11 --- plugins/shell/Cargo.toml | 1 - plugins/shell/src/process/mod.rs | 156 ++++++++----------------------- 3 files changed, 39 insertions(+), 129 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2e55e2b4ba..1dd84d7852 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5645,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" @@ -6790,7 +6780,6 @@ dependencies = [ "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 2c83e0f6e7..0775b53c4e 100644 --- a/plugins/shell/Cargo.toml +++ b/plugins/shell/Cargo.toml @@ -28,7 +28,6 @@ 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" diff --git a/plugins/shell/src/process/mod.rs b/plugins/shell/src/process/mod.rs index 272582ab41..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. @@ -63,73 +62,11 @@ pub struct Command { /// Spawned child process. pub struct CommandChild { - inner: ChildKind, + inner: Arc>>, + pid: u32, stdin_writer: PipeWriter, } -enum ChildKind { - Direct(Arc), - ProcessGroup(GroupChild), -} - -#[cfg(unix)] -struct GroupChild { - shared: Arc, - pgid: i32, -} - -#[cfg(unix)] -impl GroupChild { - fn id(&self) -> u32 { - self.shared.id() - } - - fn kill(&self) -> std::io::Result<()> { - // SAFETY: killpg is a standard POSIX syscall. The pgid was obtained from - // the child's PID, which equals its PGID since it was spawned as a group leader. - let ret = unsafe { libc::killpg(self.pgid, libc::SIGKILL) }; - if ret == 0 { - Ok(()) - } else { - Err(std::io::Error::last_os_error()) - } - } -} - -#[cfg(windows)] -struct GroupChild { - inner: Arc>>, - id: u32, -} - -#[cfg(windows)] -impl GroupChild { - fn id(&self) -> u32 { - self.id - } - - fn kill(&self) -> std::io::Result<()> { - self.inner.lock().unwrap().kill() - } - - fn wait(&self) -> std::io::Result { - loop { - match self.inner.lock().unwrap().try_wait()? { - Some(status) => return Ok(status), - None => {} - } - std::thread::sleep(std::time::Duration::from_millis(10)); - } - } - - fn clone_for_wait(&self) -> Self { - Self { - inner: self.inner.clone(), - id: self.id, - } - } -} - impl CommandChild { /// Writes to process stdin. pub fn write(&mut self, buf: &[u8]) -> crate::Result<()> { @@ -137,23 +74,18 @@ impl CommandChild { Ok(()) } - /// Sends a kill signal to the child. - /// When the child was spawned with `process_group` enabled, - /// this kills the entire process group (POSIX) or job object (Windows). + /// 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<()> { - match self.inner { - ChildKind::Direct(child) => child.kill()?, - ChildKind::ProcessGroup(group) => group.kill()?, - } + self.inner.lock().unwrap().kill()?; Ok(()) } /// Returns the process pid. pub fn pid(&self) -> u32 { - match &self.inner { - ChildKind::Direct(child) => child.id(), - ChildKind::ProcessGroup(group) => group.id(), - } + self.pid } } @@ -413,9 +345,12 @@ impl Command { raw, ); - let child_kind = if process_group { - let mut cmd_wrap = process_wrap::std::StdCommandWrap::from(command); + // 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()); @@ -424,50 +359,20 @@ impl Command { cmd_wrap.wrap(process_wrap::std::CreationFlags(CREATE_NO_WINDOW)); cmd_wrap.wrap(process_wrap::std::JobObject); } + } - let wrapped_child = cmd_wrap.spawn()?; - let child_id = wrapped_child.id(); - - #[cfg(unix)] - let group = { - let inner_child = wrapped_child.into_inner(); - let shared = Arc::new(SharedChild::new(inner_child)?); - let shared_clone = shared.clone(); - let pgid = child_id as i32; - - spawn_wait_thread(move || shared_clone.wait(), tx, guard); - - GroupChild { shared, pgid } - }; - - #[cfg(windows)] - let group = { - let group = GroupChild { - inner: Arc::new(std::sync::Mutex::new(wrapped_child)), - id: child_id, - }; - let group_wait = group.clone_for_wait(); - - spawn_wait_thread(move || group_wait.wait(), tx, guard); - - group - }; - - ChildKind::ProcessGroup(group) - } else { - let shared_child = SharedChild::spawn(&mut command)?; - let child = Arc::new(shared_child); - let child_ = child.clone(); - - spawn_wait_thread(move || child_.wait(), tx, guard); + let wrapped_child = cmd_wrap.spawn()?; + let pid = wrapped_child.id(); + let inner = Arc::new(Mutex::new(wrapped_child)); + let inner_wait = inner.clone(); - ChildKind::Direct(child) - }; + spawn_wait_thread(move || wait_on_child(&inner_wait), tx, guard); Ok(( rx, CommandChild { - inner: child_kind, + inner, + pid, stdin_writer, }, )) @@ -616,6 +521,23 @@ 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,