From 496ce76b74d29f079e82996a3f0494360077c80e Mon Sep 17 00:00:00 2001
From: branchseer <dk4rest@gmail.com>
Date: Mon, 16 Feb 2026 10:05:56 +0800
Subject: [PATCH] fix: resolve flaky PTY tests on macOS and Windows

Defer PTY slave and master cleanup to background thread after child.wait()
instead of dropping them eagerly during spawn.

macOS (is_terminal flaky empty output): dropping the slave fd immediately
after spawn meant that when the child exited quickly, all slave references
closed before the reader's first read(). macOS returns EIO without draining
the output buffer, causing portable-pty to report Ok(0) and read_to_end to
return empty data.

Windows (read_to_end_returns_exit_status_nonzero timeout): the reader pipe
only gets EOF when ClosePseudoConsole is called, which requires all
Arc<Mutex<Inner>> references (held by master and slave) to be dropped.
The master was held in PtyWriter and never explicitly dropped, so EOF
depended on undocumented ConPTY auto-close behavior that is unreliable
under CI load.

The fix wraps master in Arc<Mutex<Option<...>>> (like writer) and drops
writer, master, and slave sequentially in the background thread after
child.wait() returns. This deterministically triggers EOF on both
platforms regardless of timing.
---
 crates/pty_terminal/src/terminal.rs | 37 ++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/crates/pty_terminal/src/terminal.rs b/crates/pty_terminal/src/terminal.rs
index 9e44a10c..55784c18 100644
--- a/crates/pty_terminal/src/terminal.rs
+++ b/crates/pty_terminal/src/terminal.rs
@@ -26,7 +26,7 @@ pub struct PtyReader {
 pub struct PtyWriter {
     writer: Arc<Mutex<Option<Box<dyn Write + Send>>>>,
     parser: Arc<Mutex<vt100::Parser<Vt100Callbacks>>>,
-    master: Box<dyn MasterPty + Send>,
+    master: Arc<Mutex<Option<Box<dyn MasterPty + Send>>>>,
 }
 
 /// A cloneable handle to a child process spawned in a PTY.
@@ -200,18 +200,22 @@ impl PtyWriter {
     ///
     /// # Errors
     ///
-    /// Returns an error if the PTY cannot be resized.
+    /// Returns an error if the PTY cannot be resized or the child has already exited.
     ///
     /// # Panics
     ///
     /// Panics if the parser lock is poisoned.
     pub fn resize(&self, size: ScreenSize) -> anyhow::Result<()> {
-        self.master.resize(portable_pty::PtySize {
+        let guard = self.master.lock().unwrap();
+        let master =
+            guard.as_ref().ok_or_else(|| anyhow::anyhow!("cannot resize: child has exited"))?;
+        master.resize(portable_pty::PtySize {
             rows: size.rows,
             cols: size.cols,
             pixel_width: 0,
             pixel_height: 0,
         })?;
+        drop(guard);
 
         self.parser.lock().unwrap().screen_mut().set_size(size.rows, size.cols);
 
@@ -258,24 +262,41 @@ impl Terminal {
         let reader = pty_pair.master.try_clone_reader()?;
         let writer: Arc<Mutex<Option<Box<dyn Write + Send>>>> =
             Arc::new(Mutex::new(Some(pty_pair.master.take_writer()?)));
-        // Spawn child and immediately drop slave to ensure EOF is signaled when child exits
         let mut child = pty_pair.slave.spawn_command(cmd)?;
         let child_killer = child.clone_killer();
-        drop(pty_pair.slave); // Critical: drop slave so EOF is signaled when child exits
-        let master = pty_pair.master;
+        let master: Arc<Mutex<Option<Box<dyn MasterPty + Send>>>> =
+            Arc::new(Mutex::new(Some(pty_pair.master)));
         let exit_status: Arc<OnceLock<ExitStatus>> = Arc::new(OnceLock::new());
 
-        // Background thread: wait for child to exit, set exit status, then close writer to trigger EOF
+        // Background thread: wait for child to exit, then clean up.
+        //
+        // The slave and master are kept alive until after `child.wait()` returns
+        // rather than being dropped immediately after spawn.
+        //
+        // macOS: if the parent's slave fd is closed early and the child exits
+        // quickly, all slave references close before the reader issues its first
+        // `read()`. macOS then returns EIO on the master without draining the
+        // output buffer, causing data loss.
+        //
+        // Windows (ConPTY): the reader pipe only gets EOF when the ConPTY handle
+        // is destroyed via `ClosePseudoConsole`, which requires all
+        // `Arc<Mutex<Inner>>` references (held by both master and slave) to be
+        // dropped. Dropping the master here brings that refcount to zero, ensuring
+        // the reader sees EOF promptly after the child exits.
         thread::spawn({
             let writer = Arc::clone(&writer);
+            let master = Arc::clone(&master);
             let exit_status = Arc::clone(&exit_status);
+            let slave = pty_pair.slave;
             move || {
                 // Wait for child and set exit status
                 if let Ok(status) = child.wait() {
                     let _ = exit_status.set(status);
                 }
-                // Close writer to signal EOF to the reader
+                // Close writer, master, and slave to trigger EOF on the reader.
                 *writer.lock().unwrap() = None;
+                *master.lock().unwrap() = None;
+                drop(slave);
             }
         });