Skip to content

Commit 5af51bf

Browse files
committed
revert: remove PTY gate/permit code from terminal.rs
RUST_TEST_THREADS=1 is the actual fix — the SIGSEGV is caused by musl's fork() in multi-threaded processes, not just concurrent PTY operations. The gate code added complexity without addressing the root cause.
1 parent daa00ac commit 5af51bf

File tree

1 file changed

+13
-58
lines changed

1 file changed

+13
-58
lines changed

crates/pty_terminal/src/terminal.rs

Lines changed: 13 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -12,40 +12,13 @@ use crate::geo::ScreenSize;
1212

1313
type ChildWaitResult = Result<ExitStatus, Arc<std::io::Error>>;
1414

15-
// On musl libc (Alpine Linux), concurrent PTY operations trigger
16-
// SIGSEGV/SIGBUS in musl internals (sysconf, fcntl). This affects
17-
// openpty+fork, FD operations (read/write/close), and FD drops.
18-
// Ensure only one Terminal exists at a time by blocking spawn until
19-
// all PTY FDs from the previous Terminal are closed.
20-
#[cfg(target_env = "musl")]
21-
static PTY_GATE: (std::sync::Mutex<bool>, std::sync::Condvar) =
22-
(std::sync::Mutex::new(false), std::sync::Condvar::new());
23-
24-
/// RAII guard that releases the PTY gate when dropped.
25-
/// Shared via `Arc` so the gate stays held while any part of the
26-
/// Terminal (reader, writer, background thread) is still alive.
27-
#[cfg(target_env = "musl")]
28-
struct PtyPermit;
29-
30-
#[cfg(target_env = "musl")]
31-
impl Drop for PtyPermit {
32-
fn drop(&mut self) {
33-
let (lock, cvar) = &PTY_GATE;
34-
*lock.lock().unwrap_or_else(|e| e.into_inner()) = false;
35-
cvar.notify_one();
36-
}
37-
}
38-
3915
/// The read half of a PTY connection. Implements [`Read`].
4016
///
4117
/// Reading feeds data through an internal vt100 parser (shared with [`PtyWriter`]),
4218
/// keeping `screen_contents()` up-to-date with parsed terminal output.
4319
pub struct PtyReader {
4420
reader: Box<dyn Read + Send>,
4521
parser: Arc<Mutex<vt100::Parser<Vt100Callbacks>>>,
46-
/// Prevent concurrent PTY sessions on musl; released when all parts are dropped.
47-
#[cfg(target_env = "musl")]
48-
_permit: Arc<PtyPermit>,
4922
}
5023

5124
/// The write half of a PTY connection. Implements [`Write`].
@@ -56,9 +29,6 @@ pub struct PtyWriter {
5629
writer: Arc<Mutex<Option<Box<dyn Write + Send>>>>,
5730
parser: Arc<Mutex<vt100::Parser<Vt100Callbacks>>>,
5831
master: Box<dyn MasterPty + Send>,
59-
/// Prevent concurrent PTY sessions on musl; released when all parts are dropped.
60-
#[cfg(target_env = "musl")]
61-
_permit: Arc<PtyPermit>,
6232
}
6333

6434
/// A cloneable handle to a child process spawned in a PTY.
@@ -286,17 +256,14 @@ impl Terminal {
286256
///
287257
/// Panics if the writer lock is poisoned when the background thread closes it.
288258
pub fn spawn(size: ScreenSize, cmd: CommandBuilder) -> anyhow::Result<Self> {
289-
// On musl, block until no other Terminal is alive.
259+
// On musl libc (Alpine Linux), concurrent PTY operations trigger
260+
// SIGSEGV/SIGBUS in musl internals (sysconf, fcntl). This affects
261+
// both openpty+fork and FD cleanup (close) from background threads.
262+
// Serialize all PTY lifecycle operations that touch musl internals.
290263
#[cfg(target_env = "musl")]
291-
let permit = {
292-
let (lock, cvar) = &PTY_GATE;
293-
let mut busy = lock.lock().unwrap_or_else(|e| e.into_inner());
294-
while *busy {
295-
busy = cvar.wait(busy).unwrap_or_else(|e| e.into_inner());
296-
}
297-
*busy = true;
298-
Arc::new(PtyPermit)
299-
};
264+
static PTY_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
265+
#[cfg(target_env = "musl")]
266+
let _spawn_guard = PTY_LOCK.lock().unwrap_or_else(|e| e.into_inner());
300267

301268
let pty_pair = portable_pty::native_pty_system().openpty(portable_pty::PtySize {
302269
rows: size.rows,
@@ -326,16 +293,15 @@ impl Terminal {
326293
let writer = Arc::clone(&writer);
327294
let exit_status = Arc::clone(&exit_status);
328295
let slave = pty_pair.slave;
329-
// Hold a permit clone so the gate stays held while FDs are being cleaned up.
330-
#[cfg(target_env = "musl")]
331-
let _permit = Arc::clone(&permit);
332296
move || {
333297
let _ = exit_status.set(child.wait().map_err(Arc::new));
298+
// On musl, serialize FD cleanup (close) with PTY spawn to
299+
// prevent racing on musl-internal state.
300+
#[cfg(target_env = "musl")]
301+
let _cleanup_guard = PTY_LOCK.lock().unwrap_or_else(|e| e.into_inner());
334302
// Close writer first, then drop slave to trigger EOF on the reader.
335303
*writer.lock().unwrap() = None;
336304
drop(slave);
337-
// _permit is dropped here (after FD cleanup), releasing the gate
338-
// once all other permit clones are also dropped.
339305
}
340306
});
341307

@@ -350,19 +316,8 @@ impl Terminal {
350316
)));
351317

352318
Ok(Self {
353-
pty_reader: PtyReader {
354-
reader,
355-
parser: Arc::clone(&parser),
356-
#[cfg(target_env = "musl")]
357-
_permit: Arc::clone(&permit),
358-
},
359-
pty_writer: PtyWriter {
360-
writer,
361-
parser,
362-
master,
363-
#[cfg(target_env = "musl")]
364-
_permit: permit,
365-
},
319+
pty_reader: PtyReader { reader, parser: Arc::clone(&parser) },
320+
pty_writer: PtyWriter { writer, parser, master },
366321
child_handle: ChildHandle { child_killer, exit_status },
367322
})
368323
}

0 commit comments

Comments
 (0)