fix: use permit-based gate to serialize entire PTY lifetime on musl

The previous approach (locking only spawn and drop) was insufficient
because concurrent reads/writes on PTY FDs also trigger SIGSEGV in
musl internals. Replace the per-operation PTY_LOCK with a gate that
ensures only one Terminal can exist at a time on musl.

The gate uses a Condvar + Arc<PtyPermit> pattern: spawn blocks until
no other Terminal is active, then distributes Arc permits to reader,
writer, and the background cleanup thread. When all permits are dropped,
the gate reopens for the next Terminal.

Author: claude

crates/pty_terminal/src/terminal.rs

@@ -1,7 +1,6 @@
 use std::{
     collections::VecDeque,
     io::{Read, Write},
-    mem::ManuallyDrop,
     sync::{Arc, Mutex, OnceLock},
     thread,
 };
@@ -15,18 +14,38 @@ type ChildWaitResult = Result<ExitStatus, Arc<std::io::Error>>;
 
 // On musl libc (Alpine Linux), concurrent PTY operations trigger
 // SIGSEGV/SIGBUS in musl internals (sysconf, fcntl). This affects
-// openpty+fork, FD cleanup (close), and FD drops from any thread.
-// Serialize all PTY lifecycle operations that touch musl internals.
+// openpty+fork, FD operations (read/write/close), and FD drops.
+// Ensure only one Terminal exists at a time by blocking spawn until
+// all PTY FDs from the previous Terminal are closed.
 #[cfg(target_env = "musl")]
-static PTY_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
+static PTY_GATE: (std::sync::Mutex<bool>, std::sync::Condvar) =
+    (std::sync::Mutex::new(false), std::sync::Condvar::new());
+
+/// RAII guard that releases the PTY gate when dropped.
+/// Shared via `Arc` so the gate stays held while any part of the
+/// Terminal (reader, writer, background thread) is still alive.
+#[cfg(target_env = "musl")]
+struct PtyPermit;
+
+#[cfg(target_env = "musl")]
+impl Drop for PtyPermit {
+    fn drop(&mut self) {
+        let (lock, cvar) = &PTY_GATE;
+        *lock.lock().unwrap_or_else(|e| e.into_inner()) = false;
+        cvar.notify_one();
+    }
+}
 
 /// The read half of a PTY connection. Implements [`Read`].
 ///
 /// Reading feeds data through an internal vt100 parser (shared with [`PtyWriter`]),
 /// keeping `screen_contents()` up-to-date with parsed terminal output.
 pub struct PtyReader {
-    reader: ManuallyDrop<Box<dyn Read + Send>>,
+    reader: Box<dyn Read + Send>,
     parser: Arc<Mutex<vt100::Parser<Vt100Callbacks>>>,
+    /// Prevent concurrent PTY sessions on musl; released when all parts are dropped.
+    #[cfg(target_env = "musl")]
+    _permit: Arc<PtyPermit>,
 }
 
 /// The write half of a PTY connection. Implements [`Write`].
@@ -36,25 +55,10 @@ pub struct PtyReader {
 pub struct PtyWriter {
     writer: Arc<Mutex<Option<Box<dyn Write + Send>>>>,
     parser: Arc<Mutex<vt100::Parser<Vt100Callbacks>>>,
-    master: ManuallyDrop<Box<dyn MasterPty + Send>>,
-}
-
-impl Drop for PtyReader {
-    fn drop(&mut self) {
-        #[cfg(target_env = "musl")]
-        let _guard = PTY_LOCK.lock().unwrap_or_else(|e| e.into_inner());
-        // SAFETY: called exactly once, from drop.
-        unsafe { ManuallyDrop::drop(&mut self.reader) };
-    }
-}
-
-impl Drop for PtyWriter {
-    fn drop(&mut self) {
-        #[cfg(target_env = "musl")]
-        let _guard = PTY_LOCK.lock().unwrap_or_else(|e| e.into_inner());
-        // SAFETY: called exactly once, from drop.
-        unsafe { ManuallyDrop::drop(&mut self.master) };
-    }
+    master: Box<dyn MasterPty + Send>,
+    /// Prevent concurrent PTY sessions on musl; released when all parts are dropped.
+    #[cfg(target_env = "musl")]
+    _permit: Arc<PtyPermit>,
 }
 
 /// A cloneable handle to a child process spawned in a PTY.
@@ -282,8 +286,17 @@ impl Terminal {
     ///
     /// Panics if the writer lock is poisoned when the background thread closes it.
     pub fn spawn(size: ScreenSize, cmd: CommandBuilder) -> anyhow::Result<Self> {
+        // On musl, block until no other Terminal is alive.
         #[cfg(target_env = "musl")]
-        let _spawn_guard = PTY_LOCK.lock().unwrap_or_else(|e| e.into_inner());
+        let permit = {
+            let (lock, cvar) = &PTY_GATE;
+            let mut busy = lock.lock().unwrap_or_else(|e| e.into_inner());
+            while *busy {
+                busy = cvar.wait(busy).unwrap_or_else(|e| e.into_inner());
+            }
+            *busy = true;
+            Arc::new(PtyPermit)
+        };
 
         let pty_pair = portable_pty::native_pty_system().openpty(portable_pty::PtySize {
             rows: size.rows,
@@ -313,15 +326,16 @@ impl Terminal {
             let writer = Arc::clone(&writer);
             let exit_status = Arc::clone(&exit_status);
             let slave = pty_pair.slave;
+            // Hold a permit clone so the gate stays held while FDs are being cleaned up.
+            #[cfg(target_env = "musl")]
+            let _permit = Arc::clone(&permit);
             move || {
                 let _ = exit_status.set(child.wait().map_err(Arc::new));
-                // On musl, serialize FD cleanup (close) with PTY spawn to
-                // prevent racing on musl-internal state.
-                #[cfg(target_env = "musl")]
-                let _cleanup_guard = PTY_LOCK.lock().unwrap_or_else(|e| e.into_inner());
                 // Close writer first, then drop slave to trigger EOF on the reader.
                 *writer.lock().unwrap() = None;
                 drop(slave);
+                // _permit is dropped here (after FD cleanup), releasing the gate
+                // once all other permit clones are also dropped.
             }
         });
 
@@ -337,10 +351,18 @@ impl Terminal {
 
         Ok(Self {
             pty_reader: PtyReader {
-                reader: ManuallyDrop::new(reader),
+                reader,
                 parser: Arc::clone(&parser),
+                #[cfg(target_env = "musl")]
+                _permit: Arc::clone(&permit),
+            },
+            pty_writer: PtyWriter {
+                writer,
+                parser,
+                master,
+                #[cfg(target_env = "musl")]
+                _permit: permit,
             },
-            pty_writer: PtyWriter { writer, parser, master: ManuallyDrop::new(master) },
             child_handle: ChildHandle { child_killer, exit_status },
         })
     }

crates/pty_terminal_test/tests/milestone.rs

@@ -1,26 +1,14 @@
 use std::io::Write;
-#[cfg(target_env = "musl")]
-use std::sync::Mutex;
 
 use ntest::timeout;
 use portable_pty::CommandBuilder;
 use pty_terminal::geo::ScreenSize;
 use pty_terminal_test::TestTerminal;
 use subprocess_test::command_for_fn;
 
-// On musl, concurrent PTY spawns + FD operations in the same process cause
-// SIGSEGV in musl internals. The crate-level PTY_LOCK serializes spawn and
-// drop, but interleaved reads/writes between two live Terminals can still
-// trigger the race. Serialize entire test bodies as a workaround.
-#[cfg(target_env = "musl")]
-static TEST_MUTEX: Mutex<()> = Mutex::new(());
-
 #[test]
 #[timeout(5000)]
 fn milestone_raw_mode_keystrokes() {
-    #[cfg(target_env = "musl")]
-    let _guard = TEST_MUTEX.lock().unwrap_or_else(|e| e.into_inner());
-
     let cmd = CommandBuilder::from(command_for_fn!((), |(): ()| {
         use std::io::{Read, Write, stdout};
 
@@ -89,9 +77,6 @@ fn milestone_raw_mode_keystrokes() {
 #[test]
 #[timeout(5000)]
 fn milestone_does_not_pollute_screen() {
-    #[cfg(target_env = "musl")]
-    let _guard = TEST_MUTEX.lock().unwrap_or_else(|e| e.into_inner());
-
     let cmd = CommandBuilder::from(command_for_fn!((), |(): ()| {
         use std::io::{Read, Write, stdout};