diff --git a/apps/desktop/src-tauri/src/pty.rs b/apps/desktop/src-tauri/src/pty.rs index a80e4beb..db0e009c 100644 --- a/apps/desktop/src-tauri/src/pty.rs +++ b/apps/desktop/src-tauri/src/pty.rs @@ -2,14 +2,55 @@ use log::error; use portable_pty::{native_pty_system, CommandBuilder, PtySize}; use std::{ io::{Read, Write}, - sync::{Arc, Mutex}, - thread, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, Mutex, + }, + thread::{self, JoinHandle}, }; use tauri::{AppHandle, Emitter, Runtime}; pub struct PtyState { pub writer: Arc>>, pub master: Box, + /// Set by `kill_pty` (or the `Drop` impl) to tell the reader thread it + /// must stop emitting events. The thread checks this flag between + /// reads so bytes still in flight from a previous shell cannot land in + /// the tab of the next one. + shutdown: Arc, + /// Owned handle to the reader thread. Held in an `Option` so + /// `shutdown_and_join` can take it out (joining consumes the handle) + /// without needing `&mut self`. + thread_handle: Option>, +} + +impl PtyState { + /// Flags the reader for shutdown and waits for it to finish. Called + /// explicitly by `spawn_pty` (when replacing an old session) and + /// `kill_pty` (when the user closes the terminal) so the next session + /// cannot receive stale `pty-output` events from the old reader. + /// + /// Both callers always go through this path; there is no implicit + /// `Drop`-based teardown, because `Drop` would have to run on + /// `&mut self` and could not drop `master` before `handle.join()`, + /// which would deadlock (the reader is blocked inside `read`, and + /// only dropping the master wakes it up). + fn shutdown_and_join(self) { + self.shutdown.store(true, Ordering::Release); + // Destructure so we can drop `master` *before* joining. Dropping + // the master closes the native PTY handle and unblocks the + // reader's pending `read` call with `Ok(0)` / `Err` on all + // supported platforms; the join then returns promptly. + let PtyState { + master, + thread_handle, + .. + } = self; + drop(master); + if let Some(handle) = thread_handle { + let _ = handle.join(); + } + } } #[tauri::command] @@ -20,7 +61,18 @@ pub fn spawn_pty( rows: Option, cols: Option, ) -> Result<(), String> { - // If session exists, it will be replaced (dropping old resources kills the previous PTY) + // Take any previous session out of the mutex *before* tearing it down. + // Holding the lock across `shutdown_and_join` would serialize the join + // with every other PTY command and could deadlock if the reader was + // trying to re-enter (e.g. via `emit`). Dropping `old` here flushes + // the old thread to completion with the lock released. + let old = { + let mut guard = state.lock().map_err(|e| e.to_string())?; + guard.take() + }; + if let Some(old_state) = old { + old_state.shutdown_and_join(); + } let pty_system = native_pty_system(); @@ -85,24 +137,25 @@ pub fn spawn_pty( err })?; - let pty_state = PtyState { - writer: Arc::new(Mutex::new(writer)), - master: pair.master, - }; + let shutdown = Arc::new(AtomicBool::new(false)); + let shutdown_for_thread = shutdown.clone(); - *state.lock().map_err(|e| { - let err = e.to_string(); - error!("PTY state lock failed: {}", err); - err - })? = Some(pty_state); - - // Spawn a thread to read from the PTY and emit to the frontend - thread::spawn(move || { + // Spawn a thread to read from the PTY and emit to the frontend. The + // thread owns its cloned reader; when the master is dropped on + // shutdown the native handle closes and the next `read` returns + // `Ok(0)` / `Err`, breaking the loop. + let handle = thread::spawn(move || { let mut buffer = [0u8; 4096]; loop { match reader.read(&mut buffer) { Ok(0) => break, Ok(n) => { + // Observe the shutdown flag *before* emitting so bytes + // read from a shell that the user already killed + // cannot surface inside a new session's tab. + if shutdown_for_thread.load(Ordering::Acquire) { + break; + } let data = String::from_utf8_lossy(&buffer[..n]).to_string(); let _ = app.emit("pty-output", data); } @@ -114,6 +167,19 @@ pub fn spawn_pty( } }); + let pty_state = PtyState { + writer: Arc::new(Mutex::new(writer)), + master: pair.master, + shutdown, + thread_handle: Some(handle), + }; + + *state.lock().map_err(|e| { + let err = e.to_string(); + error!("PTY state lock failed: {}", err); + err + })? = Some(pty_state); + Ok(()) } @@ -153,11 +219,20 @@ pub fn resize_pty( Ok(()) } -/// Kills the currently active PTY session (if any). -/// Dropping the PtyState closes the master PTY handle which signals the shell to exit. +/// Kills the currently active PTY session (if any). Flags the reader +/// thread for shutdown, drops the master to unblock it, and joins the +/// thread before returning so `spawn_pty` can safely reuse the slot. #[tauri::command] pub fn kill_pty(state: tauri::State<'_, Arc>>>) -> Result<(), String> { - let mut guard = state.lock().map_err(|e| e.to_string())?; - *guard = None; // Drops PtyState — closes master PTY and terminates child + // Take the state out of the mutex *before* tearing it down. Joining + // inside the lock would block every other PTY command and could + // deadlock on a reader that re-enters via `emit`. + let taken = { + let mut guard = state.lock().map_err(|e| e.to_string())?; + guard.take() + }; + if let Some(old_state) = taken { + old_state.shutdown_and_join(); + } Ok(()) }