From adb1b790eb67473d30ae29589658c05b3bd5bfbf Mon Sep 17 00:00:00 2001 From: Azer Sadykhzadeh Date: Fri, 16 Oct 2026 22:55:50 +0000 Subject: [PATCH 1/3] End and reap a local shell when its tab closes, and drop the unused os and process plugins Removing a local terminal dropped its portable-pty child without waiting on it, so every closed tab left a zombie for as long as the app ran, and a shell that ignored the PTY hangup kept running. LocalShell now sends the hangup itself and reaps the shell on a thread of its own. A shell whose exit try_wait already collected is never signalled, because its pid may belong to another process by then. tauri-plugin-os and tauri-plugin-process were registered and os:default was granted, but nothing in the frontend imports either plugin, and restarts go through AppHandle::request_restart. They only added code and let the page read OS details it has no use for. --- Cargo.lock | 65 --------------- desktop/src-tauri/Cargo.toml | 2 - desktop/src-tauri/capabilities/default.json | 3 +- desktop/src-tauri/src/commands.rs | 2 +- desktop/src-tauri/src/main.rs | 2 - desktop/src-tauri/src/state.rs | 90 +++++++++++++++++---- 6 files changed, 77 insertions(+), 87 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 33c749f3..a9812e3a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -703,8 +703,6 @@ dependencies = [ "tauri", "tauri-build", "tauri-plugin-dialog", - "tauri-plugin-os", - "tauri-plugin-process", "tauri-plugin-shell", "tauri-plugin-single-instance", "tauri-plugin-updater", @@ -1843,16 +1841,6 @@ dependencies = [ "typenum", ] -[[package]] -name = "gethostname" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1bd49230192a3797a9a4d6abe9b3eed6f7fa4c8a8a4947977c6f80025f92cbd8" -dependencies = [ - "rustix", - "windows-link", -] - [[package]] name = "getrandom" version = "0.2.17" @@ -3414,22 +3402,6 @@ dependencies = [ "pin-project-lite", ] -[[package]] -name = "os_info" -version = "3.15.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9cf20a545b305cf1da722b236b5155c9bb35f1d5ceb28c048bd96ca842f41b5b" -dependencies = [ - "android_system_properties", - "log", - "nix 0.31.3", - "objc2", - "objc2-foundation", - "objc2-ui-kit", - "serde", - "windows-sys 0.61.2", -] - [[package]] name = "os_pipe" version = "1.2.3" @@ -5310,15 +5282,6 @@ dependencies = [ "syn 2.0.119", ] -[[package]] -name = "sys-locale" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8eab9a99a024a169fe8a903cf9d4a3b3601109bcc13bd9e3c6fff259138626c4" -dependencies = [ - "libc", -] - [[package]] name = "system-configuration" version = "0.7.0" @@ -5592,34 +5555,6 @@ dependencies = [ "url", ] -[[package]] -name = "tauri-plugin-os" -version = "2.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4bd69d272ab772b30462779de12a40f837616831c794c71531bea91474e1061f" -dependencies = [ - "gethostname", - "log", - "os_info", - "serde", - "serde_json", - "serialize-to-javascript", - "sys-locale", - "tauri", - "tauri-plugin", - "thiserror 2.0.21", -] - -[[package]] -name = "tauri-plugin-process" -version = "2.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "74ae3d8549ae564c0397377373e262f340908eb23a5b9fbe03f2a00007dc0038" -dependencies = [ - "tauri", - "tauri-plugin", -] - [[package]] name = "tauri-plugin-shell" version = "2.4.0" diff --git a/desktop/src-tauri/Cargo.toml b/desktop/src-tauri/Cargo.toml index be8044fc..df10a732 100644 --- a/desktop/src-tauri/Cargo.toml +++ b/desktop/src-tauri/Cargo.toml @@ -13,9 +13,7 @@ clavyn-core = { path = "../../core" } tauri = { version = "2", features = [] } tauri-plugin-shell = "2" tauri-plugin-dialog = "2" -tauri-plugin-os = "2" tauri-plugin-updater = "2" -tauri-plugin-process = "2" tauri-plugin-single-instance = "2" serde = { workspace = true } serde_json = { workspace = true } diff --git a/desktop/src-tauri/capabilities/default.json b/desktop/src-tauri/capabilities/default.json index 962531c1..f269fef5 100644 --- a/desktop/src-tauri/capabilities/default.json +++ b/desktop/src-tauri/capabilities/default.json @@ -5,7 +5,6 @@ "windows": ["main"], "permissions": [ "core:default", - "shell:allow-open", - "os:default" + "shell:allow-open" ] } diff --git a/desktop/src-tauri/src/commands.rs b/desktop/src-tauri/src/commands.rs index 38c153bd..42c23612 100644 --- a/desktop/src-tauri/src/commands.rs +++ b/desktop/src-tauri/src/commands.rs @@ -766,7 +766,7 @@ pub async fn create_local_terminal( let terminal = LocalTerminal { writer, master, - child, + child: crate::state::LocalShell::new(child), owns_session_id: owns_session_id.clone(), }; let refused = { diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index 9024747e..988255e6 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -121,8 +121,6 @@ fn main() { builder .plugin(tauri_plugin_shell::init()) .plugin(tauri_plugin_dialog::init()) - .plugin(tauri_plugin_os::init()) - .plugin(tauri_plugin_process::init()) .plugin(tauri_plugin_updater::Builder::new().build()) .setup(|app| { // State lives in the non-roaming directory, so a roaming profile diff --git a/desktop/src-tauri/src/state.rs b/desktop/src-tauri/src/state.rs index 76c7d1df..6c559a71 100644 --- a/desktop/src-tauri/src/state.rs +++ b/desktop/src-tauri/src/state.rs @@ -356,9 +356,9 @@ pub struct LocalTerminal { /// which take the same lock to admit a session id. pub writer: Arc>>, pub master: Box, - /// Kept alive so the shell is not reaped, and polled with `try_wait` to - /// tell a live terminal from one whose shell has already exited. - pub child: Box, + /// Polled with `try_wait` to tell a live terminal from one whose shell + /// has already exited. Dropping the terminal ends and reaps the shell. + pub child: LocalShell, /// Cleared when this terminal stops holding its session id. /// /// The thread draining the PTY outlives the map entry: it is detached, and @@ -371,6 +371,56 @@ pub struct LocalTerminal { pub owns_session_id: Arc, } +/// The shell process behind a local terminal. +/// +/// Dropping it ends the shell — SIGHUP on Unix, as closing a terminal window +/// does, and process termination on Windows — and reaps it on a thread of its +/// own, so a closed tab leaves neither a running shell nor a zombie behind. +/// Background jobs the user detached from the shell are not touched. +pub struct LocalShell { + child: Option>, + /// Set once `try_wait` has collected the exit status. The process id may + /// belong to an unrelated process from then on, so it is never signalled. + reaped: bool, +} + +impl LocalShell { + pub fn new(child: Box) -> Self { + Self { + child: Some(child), + reaped: false, + } + } + + pub fn try_wait(&mut self) -> std::io::Result> { + let Some(child) = self.child.as_mut() else { + return Ok(None); + }; + let status = child.try_wait()?; + self.reaped |= status.is_some(); + Ok(status) + } +} + +impl Drop for LocalShell { + fn drop(&mut self) { + let Some(mut child) = self.child.take() else { + return; + }; + if self.reaped { + return; + } + let _ = child.kill(); + // `wait` blocks until the shell is gone, which a shell that ignores + // SIGHUP may never be; a thread of its own keeps that off the caller. + let _ = std::thread::Builder::new() + .name("local-shell-reaper".into()) + .spawn(move || { + let _ = child.wait(); + }); + } +} + impl LocalTerminal { /// Marks the terminal as no longer holding its session id. pub fn disown(&self) { @@ -527,7 +577,7 @@ mod local_terminal_ownership_tests { LocalTerminal { writer, master: pair.master, - child, + child: LocalShell::new(child), owns_session_id: owns_session_id.clone(), }, owns_session_id, @@ -549,9 +599,25 @@ mod local_terminal_ownership_tests { // The id is free for reuse from here, so the reader still draining the // old shell must stop addressing it. - let mut closed = locals.remove("local-0").expect("the terminal was live"); + let _closed = locals.remove("local-0").expect("the terminal was live"); assert!(!owns.load(Ordering::Acquire)); - let _ = closed.child.kill(); + } + + /// Closing a tab ends its shell and collects it, rather than leaving a + /// running shell or a zombie for as long as the app runs. + #[cfg(target_os = "linux")] + #[test] + fn dropping_a_terminal_ends_and_reaps_its_shell() { + let (terminal, _) = terminal(); + let pid = terminal.child.child.as_ref().and_then(|c| c.process_id()).expect("pid"); + let proc_entry = std::path::PathBuf::from(format!("/proc/{pid}")); + assert!(proc_entry.exists()); + drop(terminal); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while proc_entry.exists() { + assert!(std::time::Instant::now() < deadline, "the shell is still around"); + std::thread::sleep(std::time::Duration::from_millis(20)); + } } #[test] @@ -562,9 +628,6 @@ mod local_terminal_ownership_tests { let reaped = locals.reap_exited(|_| true); assert_eq!(reaped.len(), 1); assert!(!owns.load(Ordering::Acquire)); - for mut terminal in reaped { - let _ = terminal.child.kill(); - } } #[test] @@ -576,8 +639,7 @@ mod local_terminal_ownership_tests { assert!(locals.remove("other").is_none()); assert!(owns.load(Ordering::Acquire)); - let mut open = locals.remove("local-0").expect("the terminal was live"); - let _ = open.child.kill(); + let _open = locals.remove("local-0").expect("the terminal was live"); } /// The window the token closes: a close drops the first open's @@ -597,10 +659,9 @@ mod local_terminal_ownership_tests { // And its fulfil installs nothing — the caller tears the shell down. let (stale_terminal, _) = terminal(); - let mut refused = locals + let _refused = locals .fulfil("local-0", stale, stale_terminal) .expect_err("the reservation is no longer the first request's"); - let _ = refused.child.kill(); assert!(locals.contains("local-0")); assert!(locals.get_live("local-0").is_none()); @@ -608,8 +669,7 @@ mod local_terminal_ownership_tests { assert!(locals.fulfil("local-0", current, live_terminal).is_ok()); assert!(owns.load(Ordering::Acquire)); - let mut open = locals.remove("local-0").expect("the terminal is live"); - let _ = open.child.kill(); + let _open = locals.remove("local-0").expect("the terminal is live"); } } From 524778f761b036ac8f7d79f2d28d49094263bdc9 Mon Sep 17 00:00:00 2001 From: Azer Sadykhzadeh Date: Fri, 16 Oct 2026 23:13:44 +0000 Subject: [PATCH 2/3] Give a closed tab's shell time to exit, and end it off the map lock portable-pty's kill() sends SIGHUP, polls for up to 200 ms and then kills outright, and the terminal is dropped under the local terminal map's lock. Closing a tab whose shell took its time therefore held up every other local terminal operation, and cut short whatever the shell does on hangup. The whole shutdown now runs on the reaper thread: the shell gets two seconds to exit after the PTY hangs up, and only then is it signalled. If no thread can be started, the shell is ended inline rather than left as a zombie. --- desktop/src-tauri/src/state.rs | 96 ++++++++++++++++++++++++++++++---- 1 file changed, 86 insertions(+), 10 deletions(-) diff --git a/desktop/src-tauri/src/state.rs b/desktop/src-tauri/src/state.rs index 6c559a71..d4294fa0 100644 --- a/desktop/src-tauri/src/state.rs +++ b/desktop/src-tauri/src/state.rs @@ -371,12 +371,21 @@ pub struct LocalTerminal { pub owns_session_id: Arc, } +/// How long a closed tab's shell gets to exit on its own after the terminal +/// hangs up, before it is signalled. +const SHELL_EXIT_GRACE: std::time::Duration = std::time::Duration::from_secs(2); + /// The shell process behind a local terminal. /// -/// Dropping it ends the shell — SIGHUP on Unix, as closing a terminal window -/// does, and process termination on Windows — and reaps it on a thread of its -/// own, so a closed tab leaves neither a running shell nor a zombie behind. -/// Background jobs the user detached from the shell are not touched. +/// Dropping it ends the shell and reaps it, so a closed tab leaves neither a +/// running shell nor a zombie behind. All of that happens on a thread of its +/// own, because a terminal is dropped under the terminal map's lock and the +/// shutdown can take seconds. Dropping the PTY hangs the terminal up, which +/// ends an ordinary shell, so the shell first gets [`SHELL_EXIT_GRACE`] to +/// finish whatever it does on hangup. Only then is it signalled: SIGHUP with +/// portable-pty's short grace and a forced kill after it on Unix, process +/// termination on Windows. Background jobs the user detached from the shell +/// are not touched. pub struct LocalShell { child: Option>, /// Set once `try_wait` has collected the exit status. The process id may @@ -402,22 +411,48 @@ impl LocalShell { } } +/// End `child` and collect its exit status, blocking until it is gone. +fn end_shell(mut child: Box) { + let deadline = std::time::Instant::now() + SHELL_EXIT_GRACE; + loop { + match child.try_wait() { + Ok(Some(_)) => return, + Ok(None) if std::time::Instant::now() < deadline => { + std::thread::sleep(std::time::Duration::from_millis(50)); + } + _ => break, + } + } + let _ = child.kill(); + let _ = child.wait(); +} + impl Drop for LocalShell { fn drop(&mut self) { - let Some(mut child) = self.child.take() else { + let Some(child) = self.child.take() else { return; }; if self.reaped { return; } - let _ = child.kill(); - // `wait` blocks until the shell is gone, which a shell that ignores - // SIGHUP may never be; a thread of its own keeps that off the caller. - let _ = std::thread::Builder::new() + // The slot lets this thread take the child back if no reaper thread + // can be started, and end it here rather than leave a zombie. + let slot = Arc::new(std::sync::Mutex::new(Some(child))); + let reaper_slot = slot.clone(); + let spawned = std::thread::Builder::new() .name("local-shell-reaper".into()) .spawn(move || { - let _ = child.wait(); + let child = reaper_slot.lock().unwrap_or_else(|e| e.into_inner()).take(); + if let Some(child) = child { + end_shell(child); + } }); + if spawned.is_err() { + let child = slot.lock().unwrap_or_else(|e| e.into_inner()).take(); + if let Some(child) = child { + end_shell(child); + } + } } } @@ -620,6 +655,47 @@ mod local_terminal_ownership_tests { } } + /// A shell that ignores the hangup is still ended, after its grace period, + /// and the drop that starts all this returns at once: it runs under the + /// terminal map's lock. + #[cfg(target_os = "linux")] + #[test] + fn a_shell_that_ignores_the_hangup_is_ended_without_blocking_the_drop() { + let pair = native_pty_system() + .openpty(PtySize { + rows: 24, + cols: 80, + pixel_width: 0, + pixel_height: 0, + }) + .expect("a pty can be opened"); + let mut command = CommandBuilder::new("/bin/sh"); + command.args(["-c", "trap '' HUP; sleep 60"]); + let child = pair.slave.spawn_command(command).expect("a shell can be spawned"); + let pid = child.process_id().expect("pid"); + let terminal = LocalTerminal { + writer: Arc::new(std::sync::Mutex::new( + pair.master.take_writer().expect("the master has a writer"), + )), + master: pair.master, + child: LocalShell::new(child), + owns_session_id: Arc::new(AtomicBool::new(true)), + }; + // Let the trap take effect before the hangup arrives. + std::thread::sleep(std::time::Duration::from_millis(200)); + + let started = std::time::Instant::now(); + drop(terminal); + assert!(started.elapsed() < std::time::Duration::from_millis(100)); + + let proc_entry = std::path::PathBuf::from(format!("/proc/{pid}")); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while proc_entry.exists() { + assert!(std::time::Instant::now() < deadline, "the shell is still around"); + std::thread::sleep(std::time::Duration::from_millis(50)); + } + } + #[test] fn reaping_an_exited_terminal_hands_its_session_id_back() { let mut locals = LocalTerminals::default(); From 2fa9d19e829ca30328cdfb42b864627fbefeacaa Mon Sep 17 00:00:00 2001 From: Azer Sadykhzadeh Date: Sat, 17 Oct 2026 07:04:32 +0000 Subject: [PATCH 3/3] Send a closed tab's shell SIGHUP directly The PTY hangup only reaches the shell once every copy of the master is closed, and the output reader keeps one open while it is blocked reading. A shell busy with a foreground command therefore heard nothing until the grace period ran out and portable-pty's kill took over. It now gets SIGHUP as soon as the tab closes, as with a terminal window, and the grace period starts from there. --- Cargo.lock | 1 + desktop/src-tauri/Cargo.toml | 5 +++ desktop/src-tauri/src/state.rs | 64 ++++++++++++++++++++++++++++++---- 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a9812e3a..49fbaf5a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -691,6 +691,7 @@ dependencies = [ "core-foundation-sys", "keyring", "keyring-core", + "libc", "objc2-local-authentication", "portable-pty", "reqwest 0.12.28", diff --git a/desktop/src-tauri/Cargo.toml b/desktop/src-tauri/Cargo.toml index df10a732..096f5eb8 100644 --- a/desktop/src-tauri/Cargo.toml +++ b/desktop/src-tauri/Cargo.toml @@ -39,6 +39,11 @@ tauri = { version = "2", features = ["test"] } # the known hosts store takes; the workspace pin keeps it the same russh. russh = { workspace = true } +[target.'cfg(unix)'.dependencies] +# Sends a closed tab's shell SIGHUP directly; already in the tree through +# portable-pty and tokio. +libc = "0.2" + [target.'cfg(target_os = "linux")'.dependencies] # Resolves the D-Bus session address before the single-instance plugin does, # because the plugin resolves it with `unwrap` and `panic = "abort"` turns that diff --git a/desktop/src-tauri/src/state.rs b/desktop/src-tauri/src/state.rs index d4294fa0..4d6d9df1 100644 --- a/desktop/src-tauri/src/state.rs +++ b/desktop/src-tauri/src/state.rs @@ -380,12 +380,11 @@ const SHELL_EXIT_GRACE: std::time::Duration = std::time::Duration::from_secs(2); /// Dropping it ends the shell and reaps it, so a closed tab leaves neither a /// running shell nor a zombie behind. All of that happens on a thread of its /// own, because a terminal is dropped under the terminal map's lock and the -/// shutdown can take seconds. Dropping the PTY hangs the terminal up, which -/// ends an ordinary shell, so the shell first gets [`SHELL_EXIT_GRACE`] to -/// finish whatever it does on hangup. Only then is it signalled: SIGHUP with -/// portable-pty's short grace and a forced kill after it on Unix, process -/// termination on Windows. Background jobs the user detached from the shell -/// are not touched. +/// shutdown can take seconds. On Unix the shell is sent SIGHUP, as closing a +/// terminal window does, and gets [`SHELL_EXIT_GRACE`] to finish whatever it +/// does on hangup. Only then is it forced: portable-pty's kill, which ends in +/// SIGKILL on Unix and process termination on Windows. Background jobs the +/// user detached from the shell are not touched. pub struct LocalShell { child: Option>, /// Set once `try_wait` has collected the exit status. The process id may @@ -413,6 +412,17 @@ impl LocalShell { /// End `child` and collect its exit status, blocking until it is gone. fn end_shell(mut child: Box) { + // The PTY hangup only reaches the shell once every copy of the master is + // closed, and the output reader holds one until it next wakes up, so the + // shell is sent SIGHUP directly. It has not been waited on yet, so its + // pid still names it. + #[cfg(unix)] + if let Some(pid) = child.process_id().and_then(|pid| libc::pid_t::try_from(pid).ok()) { + // SAFETY: kill(2) takes plain integers and has no memory effects. + unsafe { + libc::kill(pid, libc::SIGHUP); + } + } let deadline = std::time::Instant::now() + SHELL_EXIT_GRACE; loop { match child.try_wait() { @@ -696,6 +706,48 @@ mod local_terminal_ownership_tests { } } + /// The output reader keeps its own copy of the PTY master, so closing the + /// terminal does not hang the tty up while the reader is blocked. The shell + /// still hears about the close at once rather than after the grace period. + #[cfg(target_os = "linux")] + #[test] + fn a_busy_shell_gets_the_hangup_without_waiting_for_the_reader() { + let pair = native_pty_system() + .openpty(PtySize { + rows: 24, + cols: 80, + pixel_width: 0, + pixel_height: 0, + }) + .expect("a pty can be opened"); + let mut command = CommandBuilder::new("/bin/sh"); + command.args(["-c", "trap 'exit 0' HUP; while :; do sleep 0.05; done"]); + let child = pair.slave.spawn_command(command).expect("a shell can be spawned"); + let pid = child.process_id().expect("pid"); + let reader = pair.master.try_clone_reader().expect("a reader"); + let terminal = LocalTerminal { + writer: Arc::new(std::sync::Mutex::new( + pair.master.take_writer().expect("the master has a writer"), + )), + master: pair.master, + child: LocalShell::new(child), + owns_session_id: Arc::new(AtomicBool::new(true)), + }; + std::thread::sleep(std::time::Duration::from_millis(200)); + + drop(terminal); + let proc_entry = std::path::PathBuf::from(format!("/proc/{pid}")); + let deadline = std::time::Instant::now() + std::time::Duration::from_millis(1500); + while proc_entry.exists() { + assert!( + std::time::Instant::now() < deadline, + "the shell did not get the hangup while the reader held the master" + ); + std::thread::sleep(std::time::Duration::from_millis(20)); + } + drop(reader); + } + #[test] fn reaping_an_exited_terminal_hands_its_session_id_back() { let mut locals = LocalTerminals::default();