-
-
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 4 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,54 @@ | ||
| Title: Use plugin cleanup hooks for sidecar shutdown; add kill-tree helper (Fixes #14360) | ||
Legend-Master marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| Summary | ||
|
|
||
| This PR centralizes process-kill helpers in the runtime and moves responsibility for sidecar shutdown to plugins by adding a plugin-level cleanup hook. The core runtime no longer contains hardcoded sidecar draining logic; instead it calls `Plugin::cleanup_before_exit` so plugins (for example, the shell plugin) can take care of stopping sidecars and terminating process trees. | ||
|
|
||
| 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/plugin.rs | ||
| - Added a plugin lifecycle hook `fn cleanup_before_exit(&mut self, app: &AppHandle<R>) {}` with a default no-op. | ||
| - Added `PluginStore::cleanup_before_exit(&mut self, app: &AppHandle<R>)` which invokes the hook for every registered plugin. | ||
|
|
||
| - crates/tauri/src/app.rs | ||
| - Replaced the hardcoded sidecar PID draining and kill logic in the app shutdown path with a call to `plugins.lock().unwrap().cleanup_before_exit(self.app_handle())` so plugins perform shutdown work. | ||
| - Removed the previous `AppHandle::register_sidecar` / `unregister_sidecar` convenience methods from the public API (spawners should migrate to plugin-managed registries). | ||
|
|
||
| - Note on migration / shell plugin responsibilities | ||
| - The shell plugin (which lives in the plugins workspace) should implement `cleanup_before_exit` and perform any sidecar shutdown it needs. A minimal implementation is to drain the manager's sidecar registry and call `kill_process_tree` for each PID, e.g.: | ||
|
|
||
| ```text | ||
| fn cleanup_before_exit(&mut self, app: &AppHandle<R>) { | ||
| let pids = app.manager.drain_sidecar_pids(); | ||
| for pid in pids { | ||
| let _ = tauri::process::kill_process_tree(pid); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| This keeps process-management details inside the shell plugin (owner of sidecar lifecycle) and makes the runtime more extensible. | ||
|
|
||
| Testing done | ||
|
|
||
| - Ran `cargo check -p tauri` locally to verify compilation after wiring the plugin hook (no compile errors; one doc warning for `kill_process_tree`). | ||
|
|
||
| Notes & follow-ups | ||
|
|
||
| - Implement `cleanup_before_exit` in the shell plugin (plugins-workspace repo). The shell plugin should be updated to drain any sidecar PID registries it manages and use `kill_process_tree` to ensure descendant processes are terminated. | ||
| - Update examples and any existing sidecar spawners to use the shell plugin or to call plugin-provided APIs instead of the removed `AppHandle::register_sidecar`/`unregister_sidecar`. | ||
| - Consider adding an integration test that validates sidecar shutdown via the shell plugin's cleanup hook. | ||
|
|
||
| - Windows implementation note | ||
|
|
||
| - On Windows `kill_process_tree` now prefers the built-in `taskkill /T /PID <pid> /F` utility | ||
| which will terminate a process tree. If `taskkill` isn't available or returns a non-zero | ||
| exit status (for example due to permissions), the runtime falls back to a PowerShell-based | ||
| recursive traversal that mirrors the prior behavior. This makes termination more robust on | ||
| typical Windows environments while still preserving the previous fallback. | ||
|
|
||
| References | ||
|
|
||
| - Fixes #14360 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,6 +104,15 @@ pub trait Plugin<R: Runtime>: Send { | |
| #[allow(unused_variables)] | ||
| fn on_event(&mut self, app: &AppHandle<R>, event: &RunEvent) {} | ||
|
|
||
| /// Callback invoked when the application is performing cleanup before exit. | ||
| /// | ||
| /// Plugins can use this hook to perform any process shutdown/cleanup they need | ||
| /// to do before the runtime exits (for example, killing sidecars or stopping | ||
| /// background tasks). This is guaranteed to run from the thread performing | ||
Legend-Master marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| /// the app cleanup/exit sequence. | ||
| #[allow(unused_variables)] | ||
| fn cleanup_before_exit(&mut self, app: &AppHandle<R>) {} | ||
|
Contributor
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. We would also want to implement this for the
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. for sure! I’ll add this to the TauriPlugin trait as well so it’s accessible via tauri::plugin::Builder. |
||
|
|
||
| /// Extend commands to [`crate::Builder::invoke_handler`]. | ||
| #[allow(unused_variables)] | ||
| fn extend_api(&mut self, invoke: Invoke<R>) -> bool { | ||
|
|
@@ -979,6 +988,15 @@ impl<R: Runtime> PluginStore<R> { | |
| .for_each(|plugin| plugin.on_event(app, event)) | ||
| } | ||
|
|
||
| /// Runs the cleanup_before_exit hook for all plugins in the store. | ||
| pub(crate) fn cleanup_before_exit(&mut self, app: &AppHandle<R>) { | ||
| self.store.iter_mut().for_each(|plugin| { | ||
| #[cfg(feature = "tracing")] | ||
| let _span = tracing::trace_span!("plugin::hooks::cleanup_before_exit", name = plugin.name()).entered(); | ||
| plugin.cleanup_before_exit(app) | ||
| }) | ||
| } | ||
|
|
||
| /// Runs the plugin `extend_api` hook if it exists. Returns whether the invoke message was handled or not. | ||
| /// | ||
| /// The message is not handled when the plugin exists **and** the command does not. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,3 +128,96 @@ fn restart_macos_app(current_binary: &std::path::Path, env: &Env) { | |
| } | ||
| } | ||
| } | ||
|
|
||
| /// Kill a process and its descendant process tree (best-effort). | ||
| /// | ||
| /// On Windows this function prefers the built-in `taskkill /T /PID <pid> /F` | ||
| /// utility which can terminate a process tree. If `taskkill` is unavailable | ||
| /// or returns a non-zero exit status (for example due to permissions), the | ||
| /// function falls back to a PowerShell-based recursive traversal which mirrors | ||
| /// the previous implementation. | ||
| /// | ||
| /// On Unix-like systems a small shell function using `pgrep -P` is used to | ||
| /// collect child PIDs and send `SIGKILL` to descendants and the root PID. | ||
| /// | ||
| /// Note: terminating processes is inherently best-effort and may fail for | ||
| /// protected or system processes, or when the caller lacks sufficient | ||
| /// privileges. Callers should handle and log any errors returned by this | ||
| /// function. | ||
| 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; | ||
|
|
||
| // Prefer the built-in `taskkill` utility on Windows which can terminate a process | ||
| // tree with `/T`. If that fails (permissions, not found, or non-zero exit), fall | ||
| // back to a PowerShell-based recursive stop that mirrors the previous behavior. | ||
| let pid_s = pid.to_string(); | ||
|
|
||
| if let Ok(status) = Command::new("taskkill") | ||
| .args(&["/T", "/PID", &pid_s, "/F"]) // /F to force termination | ||
| .status() | ||
| { | ||
| if status.success() { | ||
| return Ok(()); | ||
| } | ||
| // If taskkill returned non-zero, fall through to try PowerShell. | ||
| } | ||
|
|
||
| // Fallback: Use PowerShell to recursively find and stop child processes, then stop the root. | ||
| // This mirrors the approach used elsewhere in the project (tauri-cli) and preserves | ||
| // behavior on systems where taskkill isn't available or failed due to permissions. | ||
| 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!("kill-tree failed: powershell exited with status: {}", status), | ||
| )) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(not(windows))] | ||
| { | ||
| use std::process::Command; | ||
|
|
||
| // On Unix, recursively collect children via pgrep -P and kill them. We use a small | ||
| // shell function to traverse descendants and then kill them. Use SIGKILL to ensure | ||
| // termination (best effort). | ||
| 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.