-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Add kill-tree helper and runtime sidecar PID registry #14443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from 2 commits
047a684
6cef7c4
130dad2
5b6c549
74c437b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| Title: Add kill-tree helper and runtime sidecar PID registry (Fixes #14360) | ||
|
|
||
| Summary | ||
|
|
||
| This PR adds a best-effort process-tree killer and a lightweight runtime registry to help ensure that descendant processes spawned by sidecars are terminated when a sidecar is killed. | ||
|
|
||
| What I changed | ||
|
|
||
| - crates/tauri/src/process.rs | ||
| - Added `pub fn kill_process_tree(pid: u32) -> std::io::Result<()>` which invokes platform-specific shell/PowerShell snippets to terminate a process tree (best-effort, no new deps). | ||
|
|
||
| - crates/tauri/src/manager/mod.rs | ||
| - Added `sidecar_pids: Arc<Mutex<HashSet<u32>>>` and methods: `register_sidecar`, `unregister_sidecar`, `drain_sidecar_pids`. | ||
|
|
||
| - crates/tauri/src/app.rs | ||
| - Added `AppHandle::register_sidecar` and `AppHandle::unregister_sidecar` convenience methods. | ||
| - Wired `cleanup_before_exit()` to drain the sidecar registry and call `kill_process_tree` for each registered PID. | ||
|
|
||
| - crates/tauri-cli/src/interface/rust/desktop.rs | ||
| - Dev-run kill path now attempts a best-effort kill-tree invocation for dev child processes. | ||
|
|
||
| Testing done | ||
|
|
||
| - Ran `cargo test -p tauri` locally: unit tests and doc-tests passed. | ||
| - The changes are designed to be best-effort (no panics on failures). The runtime requires spawners to call `register_sidecar(pid)` after spawning a sidecar so it can be cleaned up at exit. | ||
|
|
||
| Notes & follow-ups | ||
|
|
||
| - This is a pragmatic, short-term fix using shell/PowerShell helpers. We can consider a Rust-native implementation later for better portability and finer control. | ||
| - We should update any sidecar spawners (plugins or examples that call `Command::spawn()`) to call `app.handle().register_sidecar(child.id() as u32)` after spawning and `unregister_sidecar` when stopping the sidecar. | ||
| - Add an integration test that spawns a parent process which itself spawns a child, registers the parent PID, and asserts both are gone after cleanup. | ||
|
|
||
| References | ||
|
|
||
| - Fixes #14360 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,3 +128,60 @@ fn restart_macos_app(current_binary: &std::path::Path, env: &Env) { | |
| } | ||
| } | ||
| } | ||
|
|
||
| pub fn kill_process_tree(pid: u32) -> std::io::Result<()> { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since this is a generic function and not bound to this app instance imo this should be moved into https://github.com/tauri-apps/plugins-workspace/tree/v2/plugins/process
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be okay if I leave a TODO for now and move it in a follow-up PR?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, we at least have to decide how to proceed here because once this is added in tauri we cannot remove it without a major release since removing apis is a breaking change.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I move kill_process_tree to the process plugin now as you suggested? I’ll update this PR to avoid a future breaking change.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think that would make sense, yes. we can always move it back if needed (that way around works) |
||
| #[cfg(windows)] | ||
| { | ||
| use std::process::Command; | ||
|
|
||
| let ps = format!( | ||
| "function Kill-Tree {{ Param([int]$ppid); Get-CimInstance Win32_Process | Where-Object {{ $_.ParentProcessId -eq $ppid }} | ForEach-Object {{ Kill-Tree $_.ProcessId }}; Stop-Process -Id $ppid -ErrorAction SilentlyContinue }}; Kill-Tree {}", | ||
| pid | ||
| ); | ||
|
|
||
| let status = Command::new("powershell") | ||
| .arg("-NoProfile") | ||
| .arg("-Command") | ||
| .arg(ps) | ||
| .status()?; | ||
|
|
||
| if status.success() { | ||
| Ok(()) | ||
| } else { | ||
| Err(std::io::Error::new( | ||
| std::io::ErrorKind::Other, | ||
| format!("powershell kill-tree failed with status: {}", status), | ||
| )) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(not(windows))] | ||
| { | ||
| use std::process::Command; | ||
|
|
||
| let sh = format!(r#" | ||
| getcpid() {{ | ||
| for cpid in $(pgrep -P "$1" 2>/dev/null || true); do | ||
| getcpid "$cpid" | ||
| echo "$cpid" | ||
| done | ||
| }} | ||
| for p in $(getcpid {pid}); do | ||
| kill -9 "$p" 2>/dev/null || true | ||
| done | ||
| kill -9 {pid} 2>/dev/null || true | ||
| "#, pid = pid); | ||
|
|
||
| let status = Command::new("sh").arg("-c").arg(sh).status()?; | ||
|
|
||
| if status.success() { | ||
| Ok(()) | ||
| } else { | ||
| Err(std::io::Error::new( | ||
| std::io::ErrorKind::Other, | ||
| format!("sh kill-tree failed with status: {}", status), | ||
| )) | ||
| } | ||
| } | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.