[Fix]: Make PTY reader thread joinable so old-session output cannot bleed into the next tab

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<Mutex<Box<dyn Write + Send>>>,
     pub master: Box<dyn portable_pty::MasterPty + Send>,
+    /// 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<AtomicBool>,
+    /// 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<JoinHandle<()>>,
+}
+
+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<R: Runtime>(
     rows: Option<u16>,
     cols: Option<u16>,
 ) -> 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<R: Runtime>(
         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<R: Runtime>(
         }
     });
 
+    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<Mutex<Option<PtyState>>>>) -> 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(())
 }