diff --git a/AGENTS.md b/AGENTS.md index 6b0bf37..02be51b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,6 +8,7 @@ is the full contributor document and wins wherever the two disagree. ## Layout - `crates/termlens/` — the published library. `terminal.rs` is the runtime, + `spawn.rs` starts the child (on Unix by itself, see DESIGN §1), `screen.rs` the snapshot types, `emu/` the VT emulation behind a small internal trait, `graphics.rs` the inline-image capture and decoder. - `crates/termlens-cli/` — the `termlens` command (`inspect`, `diff`, @@ -17,7 +18,8 @@ is the full contributor document and wins wherever the two disagree. - `fixtures/` — deterministic terminal programs the integration suite spawns in real PTYs. Never published; `publish = false`. - `docs/DESIGN.md` — four layers, the wait-semantics contract, the snapshot - format. **Read it before touching `wait.rs`, `terminal.rs`, or `emu/`.** + format. **Read it before touching `wait.rs`, `terminal.rs`, `spawn.rs`, + or `emu/`.** ## Build and test diff --git a/CHANGELOG.md b/CHANGELOG.md index daed3ea..447b0eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,31 @@ reads that marker. ## [Unreleased] +### Fixed + +- On Unix, a program that exists but cannot be executed now fails + `TerminalBuilder::spawn` with an `Error::Spawn` that says why, and + `termlens inspect` exits 2 with that message (#563). Typical causes are a + missing `#!` interpreter, an interpreter that is not a program, a missing + dynamic loader, and an argument list over the kernel's limit. The message + names the interpreter, with any `\r` that CRLF line endings left on it + shown escaped. `spawn` used to return `Ok` with a child already killed by + SIGABRT (`fatal runtime error: assertion failed: + output.write(&bytes).is_ok()`), which inspect showed as `exited: killed + by signal: Aborted`. portable-pty closed the pipe std reports a failed + exec through, so on Unix termlens now starts the child itself. Program + resolution, the environment, the session and the signals are unchanged, + and the child still inherits no descriptor besides its terminal. A + working directory the child cannot enter is now named in the error. A + few rare setups are still left to portable-pty, as before, and + `docs/DESIGN.md` lists them. +- The second writer termlens opens on the PTY master is close-on-exec. A + process the test started while a terminal was alive inherited it before, + which kept the master open in a process unrelated to the terminal. +- With `TERM` set twice through `.env`, XTGETTCAP's `TN` reply reports the + later value, the one the child gets. It reported the first, so an + application could get two answers to "which terminal is this?". + ## [0.11.5] - 2026-10-07 ### Changed diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 596570d..3a767ad 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -134,10 +134,11 @@ cargo insta review # inspect and accept/reject each diff `wait_idle`) must pass the **stress workflow** (`stress.yml` — the suite in a 100-iteration loop on Ubuntu, macOS and Windows, sharded across five `--test-threads` values). Trigger it from the Actions tab on your branch, - or ask a maintainer to. So must a test that *adds* PTY spawns: three of - the tests added on one day in September 2026 passed locally and went red - only under stress, and stress on `main` is what told one author's flake - apart from an inherited one. + or ask a maintainer to. So must a change to how the child is started + (`spawn.rs`), which every test goes through, and a test that *adds* PTY + spawns: three of the tests added on one day in September 2026 passed + locally and went red only under stress, and stress on `main` is what told + one author's flake apart from an inherited one. - **No `sleep` as a wait.** A sleep is either too short and flaky under load, or too long and slow on every run, and it names nothing: when it fails, the log says only that time passed. Wait on what you mean — diff --git a/SECURITY.md b/SECURITY.md index 888f844..a8716f6 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -10,17 +10,24 @@ issues in escape-sequence handling) are treated as security issues. What the project does continuously, enforced by required CI on every change: -- **`unsafe` is confined to three audited single-line blocks** — `dup(2)`, - to open a second writer on the PTY master for the responder thread; - `File::from_raw_fd`, which takes sole ownership of that duplicate so it is - closed exactly once; and `kill(2)` behind `Terminal::signal`. They sit - under `SAFETY` comments (the first two share one) and each carries its own - `#[allow(unsafe_code)]`. The workspace sets `unsafe_code = "warn"` and CI - runs clippy with `-D warnings`, so a new block fails unless it carries - that `#[allow]` as well: the attribute is what review has to see, not a - count the lint keeps. None touches memory the child can influence. - Everything on the parsing path is pure Rust: escape sequences from the - child are parsed, never executed or evaluated. +- **`unsafe` is confined to five audited blocks, all Unix-only.** Three + are single lines. `BorrowedFd::borrow_raw` borrows the PTY master's + descriptor, which std then duplicates, close-on-exec, into the responder + thread's second writer. `access(2)` resolves the program the way the PTY + layer does. `kill(2)` sits behind `Terminal::signal`. The fourth asks + `sigaction(2)` whether the kernel reaps this process's children. The + fifth is the `pre_exec` hook in `src/spawn.rs`, which runs in the forked + child before exec. It repeats portable-pty's own hook step by step, with + two changes: inherited descriptors are marked close-on-exec rather than + closed, so that a failed exec can be reported (#563), and the umask step + is left out because termlens never sets a umask. Each block sits under + its own `SAFETY` comment and carries its own `#[allow(unsafe_code)]`. + The workspace sets `unsafe_code = "warn"` and CI runs clippy with + `-D warnings`, so a new block fails unless it carries that `#[allow]` as + well: the attribute is what review has to see, not a count the lint + keeps. None touches memory the child can influence. Everything on the + parsing path is pure Rust: escape sequences from the child are parsed, + never executed or evaluated. - **Dependency policy** (`cargo-deny` job): RUSTSEC advisories, yanked crates, license allowlist, and crates.io-only sources — on every PR, with weekly grouped Dependabot updates and Dependabot alerts enabled. diff --git a/crates/termlens/src/lib.rs b/crates/termlens/src/lib.rs index 79b8572..d2ae601 100644 --- a/crates/termlens/src/lib.rs +++ b/crates/termlens/src/lib.rs @@ -112,6 +112,7 @@ mod error; mod graphics; mod keys; mod screen; +mod spawn; mod terminal; mod utf8; mod wait; diff --git a/crates/termlens/src/spawn.rs b/crates/termlens/src/spawn.rs new file mode 100644 index 0000000..e349dea --- /dev/null +++ b/crates/termlens/src/spawn.rs @@ -0,0 +1,536 @@ +//! Starting the child inside the PTY. +//! +//! On Unix termlens forks and executes the child itself instead of handing +//! the command to the PTY layer's `spawn_command`. Between fork and exec, +//! portable-pty closes every descriptor above stdio, and one of them is the +//! pipe through which std reports a failed `exec`. A program that exists but +//! cannot be executed, such as a script whose `#!` interpreter is missing, +//! then died of a runtime abort, and `spawn` returned `Ok` with a child +//! already killed by SIGABRT (#563; upstream, wezterm/wezterm#7893). +//! +//! So the command is built the way portable-pty 0.9.0 builds it, with the +//! same program resolution, working directory, environment, session and +//! signals, and one difference: descriptors above stdio are marked +//! close-on-exec instead of closed. A successful exec closes the same set the +//! PTY layer closed, and a failed one can still report why. The test at the +//! foot of this file starts one probe both ways and requires the same +//! answers, so a portable-pty release that changes its own spawn fails there +//! rather than drifting apart from this one unseen. +//! +//! Whatever this path is not sure of goes to the PTY layer unchanged: a +//! process whose children the kernel reaps (std would panic reporting the +//! failure), a working directory that is not one with no `HOME` in the +//! child's environment to fall back to (the PTY layer reads the passwd +//! entry), a program that does not resolve (the PTY layer says why, in the +//! words termlens has always reported), a slave device it cannot name or +//! open. Windows always goes there, because ConPTY reports a failed +//! `CreateProcess` directly. + +use std::ffi::OsString; + +use portable_pty::{Child, CommandBuilder, PtyPair}; + +/// A child running in the PTY, as the PTY layer hands one back. +pub(crate) type PtyChild = Box; + +/// Start `cmd` on the slave side of `pair`, with `env` set on top of the +/// inherited environment, or on top of nothing when `env_clear` is set. +/// Later entries in `env` win. +pub(crate) fn spawn_child( + pair: &PtyPair, + mut cmd: CommandBuilder, + env_clear: bool, + env: &[(OsString, OsString)], +) -> Result { + if env_clear { + cmd.env_clear(); + } + for (key, value) in env { + cmd.env(key, value); + } + #[cfg(unix)] + if let Some(spawned) = unix::spawn(pair.master.as_ref(), &cmd, env_clear, env) { + return spawned; + } + pair.slave + .spawn_command(cmd) + .map_err(|error| error.to_string()) +} + +#[cfg(unix)] +mod unix { + use std::ffi::{CString, OsStr, OsString}; + use std::io::{self, Read}; + use std::os::unix::ffi::OsStrExt; + use std::os::unix::fs::OpenOptionsExt; + use std::os::unix::process::CommandExt; + use std::path::{Component, Path, PathBuf}; + use std::process::Command; + + use portable_pty::{CommandBuilder, MasterPty}; + + use super::PtyChild; + + /// Spawn `cmd` the way the PTY layer would, or `None` to leave it to the + /// PTY layer. + pub(super) fn spawn( + master: &dyn MasterPty, + cmd: &CommandBuilder, + env_clear: bool, + env: &[(OsString, OsString)], + ) -> Option> { + // std reports a failed exec only after reaping the child, and panics + // when the reap fails, which it does wherever the kernel reaps + // children itself. The PTY layer never reports the failure, so it + // never gets that far. + if children_reaped_by_the_kernel() { + return None; + } + let argv = cmd.get_argv(); + let program = argv.first()?; + // Where the PTY layer runs the child: the working directory, or the + // child's HOME when that is not a directory. Without a HOME it reads + // the passwd entry, which is left to it. + let dir = match cmd.get_cwd().map(Path::new).filter(|dir| dir.is_dir()) { + Some(dir) => dir.to_owned(), + None => PathBuf::from(cmd.get_env("HOME")?.to_str()?), + }; + let resolved = resolve(program, &dir, cmd.get_env("PATH"))?; + // A second handle on the slave, by name, because the PTY layer does + // not expose its own. O_NOCTTY keeps the device from becoming this + // process's controlling terminal; std adds O_CLOEXEC. + let slave = std::fs::OpenOptions::new() + .read(true) + .write(true) + .custom_flags(libc::O_NOCTTY) + .open(master.tty_name()?) + .ok()?; + let stdin = slave.try_clone().ok()?; + let stdout = slave.try_clone().ok()?; + + let mut command = Command::new(&resolved); + command + .arg0(program) + .args(&argv[1..]) + .current_dir(&dir) + .stdin(stdin) + .stdout(stdout) + .stderr(slave); + if env_clear { + command.env_clear(); + } + // When the test process has no SHELL, the PTY layer fills one in + // from the login shell, and the builder holds the value it chose. + if let Some(shell) = cmd.get_env("SHELL") { + command.env("SHELL", shell); + } + for (key, value) in env { + command.env(key, value); + } + let controlling_tty = cmd.get_controlling_tty(); + // SAFETY: the closure runs in the forked child before exec, where a + // lock held by another thread of this process stays held forever. It + // takes the PTY layer's own pre_exec step by step, but for two: the + // umask step, which acts only on a umask termlens never sets, is left + // out, and descriptors are marked close-on-exec rather than closed. + // signal, sigemptyset, sigprocmask, setsid, ioctl and fcntl are + // async-signal-safe and act on the child alone. Listing /dev/fd + // allocates, as the PTY layer's listing always did at this point. + #[allow(unsafe_code)] + unsafe { + command.pre_exec(move || { + // Defaults for the signals a test runner tends to change, and + // an empty mask, so the child does not inherit an ignored + // SIGINT from whatever started the test. + for signal in [ + libc::SIGCHLD, + libc::SIGHUP, + libc::SIGINT, + libc::SIGQUIT, + libc::SIGTERM, + libc::SIGALRM, + ] { + libc::signal(signal, libc::SIG_DFL); + } + let mut none = std::mem::MaybeUninit::::uninit(); + libc::sigemptyset(none.as_mut_ptr()); + libc::sigprocmask(libc::SIG_SETMASK, none.as_ptr(), std::ptr::null_mut()); + // A session of its own with the PTY as its controlling + // terminal, which is what turns ^C into SIGINT and a closed + // master into SIGHUP. + if libc::setsid() == -1 { + return Err(io::Error::last_os_error()); + } + if controlling_tty && libc::ioctl(0, libc::TIOCSCTTY as _, 0) == -1 { + return Err(io::Error::last_os_error()); + } + // The descriptors the PTY layer closes here, marked + // close-on-exec instead. A successful exec closes the same + // set; a failed one leaves std the pipe it reports through. + if let Ok(entries) = std::fs::read_dir("/dev/fd") { + let fds: Vec = entries + .filter_map(|entry| entry.ok()?.file_name().to_str()?.parse().ok()) + .filter(|&fd| fd > 2) + .collect(); + for fd in fds { + libc::fcntl(fd, libc::F_SETFD, libc::FD_CLOEXEC); + } + } + Ok(()) + }); + } + Some(match command.spawn() { + Ok(child) => Ok(Box::new(child)), + Err(error) => Err(explain(&error, &resolved, &dir)), + }) + } + + /// Whether this process's children are reaped by the kernel the moment + /// they exit: SIGCHLD ignored, or `SA_NOCLDWAIT` set on it, either + /// inherited from whatever started the test. + fn children_reaped_by_the_kernel() -> bool { + // SAFETY: with a null new action, sigaction only writes the current + // one into `current`, and a zeroed sigaction is a valid value for it + // to overwrite. + #[allow(unsafe_code)] + let current = unsafe { + let mut current: libc::sigaction = std::mem::zeroed(); + (libc::sigaction(libc::SIGCHLD, std::ptr::null(), &mut current) == 0).then_some(current) + }; + current.is_some_and(|action| { + action.sa_sigaction == libc::SIG_IGN || action.sa_flags & libc::SA_NOCLDWAIT != 0 + }) + } + + /// The file the PTY layer would execute for `program` run in `dir`, + /// chosen by its rules, or `None` for the PTY layer to say why there is + /// none. + fn resolve(program: &OsStr, dir: &Path, path: Option<&OsStr>) -> Option { + let program = Path::new(program); + let runnable = |file: &Path| !file.is_dir() && executable(file); + if program.is_absolute() { + return runnable(program).then(|| program.to_owned()); + } + // `./x` and `../x` are looked up in `dir` alone. + if matches!( + program.components().next(), + Some(Component::CurDir | Component::ParentDir) + ) { + let file = dir.join(program); + return runnable(&file).then_some(file); + } + // Anything else, `sub/x` included, along PATH, each entry taken + // relative to `dir`. + std::env::split_paths(path?) + .map(|entry| dir.join(entry).join(program)) + .find(|file| runnable(file)) + } + + /// access(2) with `X_OK`, as the PTY layer's resolution asks it: the + /// kernel's verdict for this process, ACLs and noexec mounts included, + /// where the mode bits would be a guess. For a directory, whether it can + /// be entered. + fn executable(file: &Path) -> bool { + let Ok(file) = CString::new(file.as_os_str().as_bytes()) else { + return false; + }; + // SAFETY: `file` is a NUL-terminated string that outlives the call, + // and access(2) only reads it. + #[allow(unsafe_code)] + let status = unsafe { libc::access(file.as_ptr(), libc::X_OK) }; + status == 0 + } + + /// Why the child could not be started. std hands back the errno of + /// whichever step failed (the fork, a NUL in an argument, entering the + /// working directory, the session setup, the exec), and the errno alone + /// does not say which. So the program is blamed only for what the exec + /// alone could raise, and where the kernel's `ENOENT` or `EACCES` points + /// at a file that plainly is there, the file it went looking for is + /// named: the `#!` interpreter of a script, or the loader of a binary. + /// Anything else reads as std gave it, as the PTY layer's spawn did. + fn explain(error: &io::Error, program: &Path, dir: &Path) -> String { + let shown = program.display(); + match error.raw_os_error() { + // Entering the directory fails with these too, and fails first. + Some( + libc::ENOENT | libc::EACCES | libc::ENOTDIR | libc::ELOOP | libc::ENAMETOOLONG, + ) if !dir.is_dir() || !executable(dir) => { + format!("cannot enter {}: {error}", dir.display()) + } + Some(libc::ENOENT) if program.is_file() => match interpreter(program) { + Ok(Some(interpreter)) if !present(dir, &interpreter) => format!( + "cannot execute {shown}: its #! interpreter {interpreter:?} does not exist" + ), + Ok(Some(interpreter)) => format!( + "cannot execute {shown}: {error}; its #! interpreter {interpreter:?} \ + exists, so what is missing is something the interpreter needs" + ), + Ok(None) => format!( + "cannot execute {shown}: {error}; the file exists, so what is missing \ + is its dynamic loader" + ), + Err(_) => format!( + "cannot execute {shown}: {error}; the file exists, so what is missing \ + is its #! interpreter or its dynamic loader" + ), + }, + Some(libc::EACCES) => match interpreter(program) { + Ok(Some(interpreter)) if !runnable(dir, &interpreter) => format!( + "cannot execute {shown}: its #! interpreter {interpreter:?} is not an \ + executable file" + ), + _ => format!("cannot execute {shown}: {error}"), + }, + Some( + libc::ENOENT + | libc::E2BIG + | libc::ETXTBSY + | libc::ENOEXEC + | libc::EISDIR + | libc::ENOTDIR + | libc::ELOOP + | libc::ENAMETOOLONG, + ) => format!("cannot execute {shown}: {error}"), + _ => error.to_string(), + } + } + + /// Whether an interpreter path names something, looked up as the kernel + /// looks it up: relative to the directory the child runs in. + fn present(dir: &Path, interpreter: &Path) -> bool { + !interpreter.as_os_str().is_empty() && dir.join(interpreter).exists() + } + + /// Whether an interpreter path names a file the kernel would execute. + fn runnable(dir: &Path, interpreter: &Path) -> bool { + let file = dir.join(interpreter); + present(dir, interpreter) && file.is_file() && executable(&file) + } + + /// What a script's `#!` line names, read the way the kernel reads it: + /// the first word after `#!` on the first line, ended by a space, a tab + /// or a NUL, and on macOS by a `#` as well. A `\r` stays part of the + /// name, which is why a script saved with CRLF line endings cannot run. + /// `Ok(None)` for a file that is not a script, and `Err` for one that + /// cannot be read, which may be either. + fn interpreter(file: &Path) -> io::Result> { + let mut head = Vec::with_capacity(512); + std::fs::File::open(file)? + .take(512) + .read_to_end(&mut head)?; + let Some(rest) = head.strip_prefix(b"#!") else { + return Ok(None); + }; + let line_end = + |b: &u8| matches!(b, b'\n' | b'\0') || (cfg!(target_os = "macos") && *b == b'#'); + let line = rest.split(line_end).next().unwrap_or_default(); + let name = line + .split(|&b| b == b' ' || b == b'\t') + .find(|word| !word.is_empty()) + .unwrap_or_default(); + Ok(Some(PathBuf::from(OsStr::from_bytes(name)))) + } +} + +#[cfg(all(test, unix))] +mod tests { + use std::ffi::OsString; + use std::io::{Read, Write}; + use std::sync::{mpsc, Arc, Mutex, PoisonError}; + use std::thread; + use std::time::Duration; + + use portable_pty::{CommandBuilder, PtySize}; + + /// What a child sees of how it was started: argv[0], the working + /// directory, the environment (its names, and a checksum of the whole, so + /// that a failure never prints a value), the descriptors it holds, the + /// umask, and whether it leads a process group that is the PTY's + /// foreground one with the PTY as its controlling terminal. It then waits + /// for a line, so that nothing it printed can be lost to an exit before + /// the read. + const PROBE: &str = r#"printf 'argv0=%s\n' "$0" +pwd +env | sed 's/=.*//' | LC_ALL=C sort | tr '\n' ' '; echo +env | LC_ALL=C sort | cksum +ls /dev/fd | tr '\n' ' '; echo +umask +[ "$(ps -o pgid= -p $$ | tr -d ' ')" = "$$" ] && echo group-leader +[ "$(ps -o tpgid= -p $$ | tr -d ' ')" = "$$" ] && echo foreground +exec 3 Result { + let progress = Arc::new(Mutex::new(Probe::default())); + let (done, outcome) = mpsc::channel(); + let shared = Arc::clone(&progress); + thread::spawn(move || { + let _ = done.send(run_probe(through_termlens, &shared)); + }); + match outcome.recv_timeout(Duration::from_secs(60)) { + Ok(printed) => Ok(printed), + Err(mpsc::RecvTimeoutError::Timeout) => { + let state = progress.lock().unwrap_or_else(PoisonError::into_inner); + let shell = state.pid.map_or_else(String::new, |pid| { + run( + "ps", + &[ + "-o", + "pid=,stat=,pgid=,tpgid=,wchan=", + "-p", + &pid.to_string(), + ], + ) + }); + let tty = state.tty.as_ref().map_or_else(String::new, |tty| { + let flag = if cfg!(target_os = "macos") { + "-f" + } else { + "-F" + }; + run("stty", &[flag, &tty.to_string_lossy(), "-a"]) + }); + Err(format!( + "hung while {}, having printed:\n{}\nthe shell (ps): {shell}\nthe terminal \ + (stty): {tty}", + state.step, + String::from_utf8_lossy(&state.seen) + )) + } + Err(mpsc::RecvTimeoutError::Disconnected) => Err("failed; its panic is above".into()), + } + } + + fn run(program: &str, args: &[&str]) -> String { + std::process::Command::new(program) + .args(args) + .output() + .map_or_else( + |error| error.to_string(), + |out| { + String::from_utf8_lossy(&out.stdout).into_owned() + + &String::from_utf8_lossy(&out.stderr) + }, + ) + } + + #[derive(Default)] + struct Probe { + step: &'static str, + seen: Vec, + pid: Option, + tty: Option, + } + + type Progress = Arc>; + + fn step(progress: &Progress, now: &'static str) { + progress.lock().unwrap_or_else(PoisonError::into_inner).step = now; + } + + fn run_probe(through_termlens: bool, progress: &Progress) -> String { + step(progress, "opening the PTY"); + let (pair, lifecycle) = crate::terminal::open_pty(PtySize { + rows: 80, + cols: 300, + pixel_width: 0, + pixel_height: 0, + }) + .expect("a PTY opens"); + progress.lock().unwrap_or_else(PoisonError::into_inner).tty = pair.master.tty_name(); + let mut cmd = CommandBuilder::new("sh"); + cmd.args(["-c", PROBE]); + cmd.cwd(std::env::current_dir().expect("a working directory")); + let env = [(OsString::from("TERM"), OsString::from("xterm-256color"))]; + for (key, value) in &env { + cmd.env(key, value); + } + let mut reader = pair.master.try_clone_reader().expect("a reader"); + let mut writer = pair.master.take_writer().expect("a writer"); + step(progress, "spawning sh"); + let mut child = if through_termlens { + super::unix::spawn(pair.master.as_ref(), &cmd, false, &env) + .expect("termlens's own spawn takes this command") + .expect("sh starts") + } else { + pair.slave.spawn_command(cmd).expect("sh starts") + }; + progress.lock().unwrap_or_else(PoisonError::into_inner).pid = child.process_id(); + drop(pair.slave); + drop(lifecycle); + + // Read up to the marker's line ending, not just the marker: a read + // that stops between the two leaves one side a line shorter. + step(progress, "reading what sh printed"); + let mut seen = Vec::new(); + let mut chunk = [0; 4096]; + while !String::from_utf8_lossy(&seen) + .replace("\r\n", "\n") + .contains("probe-finished\n") + { + let read = reader.read(&mut chunk).expect("the PTY reads"); + assert!( + read > 0, + "the probe ended early:\n{}", + String::from_utf8_lossy(&seen) + ); + seen.extend_from_slice(&chunk[..read]); + progress + .lock() + .unwrap_or_else(PoisonError::into_inner) + .seen + .extend_from_slice(&chunk[..read]); + } + step(progress, "answering sh's read"); + writer.write_all(b"\n").expect("the PTY writes"); + // Drained to the end, as a Terminal's reader drains: the terminal + // echoes the line, and on macOS a process exiting with output still + // queued waits for it to be read, so an unread echo is a shell that + // never finishes exiting. + step(progress, "draining the terminal"); + let mut rest = Vec::new(); + let _ = reader.read_to_end(&mut rest); + step(progress, "waiting for sh to exit"); + child.wait().expect("sh exits"); + // Every handle on the master closes under the lifecycle lock, as a + // Terminal's do: on macOS the last close revokes the device, and a + // revoke landing after the next probe has opened the same, recycled + // device would hang up that probe's session. + step(progress, "closing the PTY"); + let teardown = crate::terminal::pty_lifecycle_guard(); + drop(writer); + drop(reader); + drop(pair.master); + drop(teardown); + String::from_utf8_lossy(&seen).replace("\r\n", "\n") + } + + /// termlens's own spawn starts the child the PTY layer would have + /// started, as far as the child can tell, so a portable-pty release that + /// changes its spawn fails here instead of drifting apart from this one. + #[test] + fn the_child_cannot_tell_which_spawn_started_it() { + match (probe(true), probe(false)) { + (Ok(ours), Ok(theirs)) => { + assert!( + ours.contains("group-leader\nforeground\ncontrolling-tty\nprobe-finished"), + "the probe did not run to the end:\n{ours}" + ); + assert_eq!(ours, theirs); + } + (ours, theirs) => panic!( + "through termlens's spawn: {}\n\nthrough the PTY layer's: {}", + ours.unwrap_or_else(|problem| problem), + theirs.unwrap_or_else(|problem| problem) + ), + } + } +} diff --git a/crates/termlens/src/terminal.rs b/crates/termlens/src/terminal.rs index dc9683e..bdd73c2 100644 --- a/crates/termlens/src/terminal.rs +++ b/crates/termlens/src/terminal.rs @@ -28,6 +28,7 @@ use crate::error::{Error, Result}; use crate::keys::Input; use crate::keys::{mouse_legacy, mouse_sgr, mouse_utf8}; use crate::screen::{MouseMode, Screen}; +use crate::spawn::{spawn_child, PtyChild}; use crate::utf8::Utf8Sanitizer; use crate::wait::{next_backoff, Expired, Monitor, INITIAL_BACKOFF, POLL_CAP}; @@ -64,7 +65,7 @@ struct ExitWatch { impl ExitWatch { fn tick( &mut self, - child: &mut Box, + child: &mut PtyChild, exit_status: &mut Option, state: &mut EmuState, ) { @@ -254,15 +255,15 @@ static PTY_LIFECYCLE: Mutex<()> = Mutex::new(()); /// The lifecycle lock, held — or `None` on a platform with nothing to /// serialize. Callers hold and drop it the same way either way. -type LifecycleGuard = Option>; +pub(crate) type LifecycleGuard = Option>; #[cfg(unix)] -fn pty_lifecycle_guard() -> LifecycleGuard { +pub(crate) fn pty_lifecycle_guard() -> LifecycleGuard { Some(PTY_LIFECYCLE.lock().unwrap_or_else(PoisonError::into_inner)) } #[cfg(not(unix))] -fn pty_lifecycle_guard() -> LifecycleGuard { +pub(crate) fn pty_lifecycle_guard() -> LifecycleGuard { None } @@ -294,7 +295,7 @@ const PTY_OPEN_BACKOFF: Duration = Duration::from_millis(25); /// classifying an `anyhow` error by its text is guesswork, a permanent fault /// still reports its own message at the end, and the cost of being wrong is /// under two seconds on a spawn that was going to fail anyway. -fn open_pty(size: PtySize) -> Result<(PtyPair, LifecycleGuard)> { +pub(crate) fn open_pty(size: PtySize) -> Result<(PtyPair, LifecycleGuard)> { let pty = native_pty_system(); let mut last = String::new(); for attempt in 0..PTY_OPEN_ATTEMPTS { @@ -324,21 +325,19 @@ type SharedWriter = Arc>>>; /// thread. `take_writer` may only be called once, so duplicate the /// descriptor instead: both refer to the same open file description, /// which is what we want — same terminal, independent blocking. +/// +/// Close-on-exec, as std makes every descriptor it opens. A plain `dup` was +/// inherited by every child the test process started while the terminal was +/// alive, holding the master open in processes that have no business with +/// it. #[cfg(unix)] fn dup_writer(master: &dyn portable_pty::MasterPty) -> Option { - use std::os::unix::io::FromRawFd; - let fd = master.as_raw_fd()?; - // SAFETY: `fd` is the live master descriptor (the caller still owns - // the master). dup(2) returns a fresh descriptor we take sole - // ownership of; `File` closes it exactly once on drop. - #[allow(unsafe_code)] - let duped = unsafe { libc::dup(fd) }; - if duped < 0 { - return None; - } + // SAFETY: `fd` is the live master descriptor, owned by the master the + // caller holds for the whole of this borrow. #[allow(unsafe_code)] - Some(unsafe { std::fs::File::from_raw_fd(duped) }) + let master = unsafe { std::os::fd::BorrowedFd::borrow_raw(fd) }; + master.try_clone_to_owned().ok().map(std::fs::File::from) } #[cfg(not(unix))] @@ -1863,7 +1862,8 @@ impl TerminalBuilder { /// command (empty program name, missing /// [`current_dir`](Self::current_dir), a bare program name under /// [`env_clear`](Self::env_clear) with no `PATH` to resolve it) or the - /// program cannot be executed; [`Error::Size`] when the configured + /// program cannot be executed, with the reason the OS gave (on Unix a + /// missing `#!` interpreter is named); [`Error::Size`] when the configured /// [`size`](Self::size) has a dimension below 2 or past the /// 1000-per-axis limit; [`Error::Pty`] when the PTY cannot be opened. pub fn spawn(self, program: impl AsRef) -> Result { @@ -1904,10 +1904,9 @@ impl TerminalBuilder { // `std::process::Command`, so it is made absolute here. Handed // over relative, the PTY layer joins it onto a relative program // path and the child then enters it before exec: `dir/./prog` - // looked up from inside `dir`, which does not exist. The exec - // failure could not even be reported, because the PTY layer - // closes the pipe std reports it through, so the child died of - // a runtime abort instead. `absolute` fails only on an empty + // looked up from inside `dir`, which does not exist. Until + // #563 that exec failure was not even reported: the child died + // of a runtime abort instead. `absolute` fails only on an empty // path or an unreadable cwd; the given path stands then. Some(dir) => cmd.cwd(std::path::absolute(dir).unwrap_or_else(|_| dir.clone())), None => { @@ -1916,29 +1915,28 @@ impl TerminalBuilder { } } } - if self.env_clear { - cmd.env_clear(); - } // Whatever the child is told TERM is, XTGETTCAP's `TN` reports the // same string — an application must not be able to get two different - // answers to "which terminal is this?". - let term_name = self.envs.iter().find(|(k, _)| k == "TERM").map_or_else( + // answers to "which terminal is this?". Set twice, the later value is + // the one the child gets, so it is the one reported. + let term_name = self.envs.iter().rfind(|(k, _)| k == "TERM").map_or_else( || "xterm-256color".to_owned(), |(_, v)| v.to_string_lossy().into_owned(), ); + // What the child gets on top of the inherited environment, or on top + // of nothing under env_clear. Later entries win. + let mut env: Vec<(OsString, OsString)> = Vec::new(); if !self.envs.iter().any(|(k, _)| k == "TERM") { - cmd.env("TERM", "xterm-256color"); + env.push(("TERM".into(), "xterm-256color".into())); } // The PTY layer fills SHELL from the host's login shell when the // variable is absent, which put one machine-shaped value inside an // environment env_clear promises is hermetic (#221). Pinned the way // TERM is; an explicit `.env("SHELL", …)` still wins. if self.env_clear && !self.envs.iter().any(|(k, _)| k == "SHELL") { - cmd.env("SHELL", "/bin/sh"); - } - for (key, value) in &self.envs { - cmd.env(key, value); + env.push(("SHELL".into(), "/bin/sh".into())); } + env.extend(self.envs.iter().cloned()); // Attach the reader thread BEFORE spawning the child: a program that // writes and exits within its first millisecond must find a drain @@ -2064,10 +2062,11 @@ impl TerminalBuilder { }) .map_err(Error::Io)?; - let child = pair.slave.spawn_command(cmd).map_err(|e| Error::Spawn { - command: command_desc.clone(), - reason: e.to_string(), - })?; + let child = + spawn_child(&pair, cmd, self.env_clear, &env).map_err(|reason| Error::Spawn { + command: command_desc.clone(), + reason, + })?; // Close the parent's slave handle so the master sees EOF once the // child (and its descendants) release the terminal. drop(pair.slave); @@ -2556,7 +2555,7 @@ fn reader_loop( /// See the crate-level docs for a full example. Dropping a `Terminal` kills /// and reaps the child — tests never leak zombies, even on panic. pub struct Terminal { - child: Box, + child: PtyChild, // Option only so Drop can close it under the PTY lifecycle lock; // Some for the entire life of the value outside Drop. master: Option>, diff --git a/crates/termlens/tests/common/mod.rs b/crates/termlens/tests/common/mod.rs index 31a9acd..c065c70 100644 --- a/crates/termlens/tests/common/mod.rs +++ b/crates/termlens/tests/common/mod.rs @@ -26,6 +26,23 @@ pub(crate) fn emit(steps: &[&str]) -> termlens::Result { spawn_emit(Terminal::builder(), steps) } +/// Write an executable script at `path` through `sh`, so that this process +/// never holds the file open for writing. A test binary forks all the time, +/// and a child forked while the file was open keeps a copy of the +/// descriptor until it execs. Running the script in that window fails with +/// ETXTBSY, "Text file busy", which is never the failure under test. +#[cfg(unix)] +#[allow(dead_code)] +pub(crate) fn write_script(path: &std::path::Path, contents: &str) { + let status = Command::new("sh") + .args(["-c", r#"printf '%s' "$2" > "$1" && chmod 755 "$1""#, "sh"]) + .arg(path) + .arg(contents) + .status() + .expect("sh runs"); + assert!(status.success(), "writing {} failed", path.display()); +} + /// Fixture names already rebuilt by this test process. static BUILT: Mutex> = Mutex::new(Vec::new()); diff --git a/crates/termlens/tests/descriptors.rs b/crates/termlens/tests/descriptors.rs new file mode 100644 index 0000000..a8099c5 --- /dev/null +++ b/crates/termlens/tests/descriptors.rs @@ -0,0 +1,113 @@ +//! What a child inherits from the test process: its terminal, and nothing +//! termlens holds. A binary of its own, because the assertions are about the +//! whole process's descriptor table, which tests running beside them would +//! change. +#![cfg(unix)] + +use std::collections::BTreeSet; +use std::os::fd::AsRawFd; +use std::process::Command; +use std::sync::{Mutex, PoisonError}; +use std::time::Duration; + +use termlens::{Key, Terminal}; + +/// The tests below measure and change one descriptor table, so they take +/// turns: one leaves a descriptor inheritable that the other would count. +static TABLE: Mutex<()> = Mutex::new(()); + +/// PATH and nothing else, so that no colour or locale setting of the +/// machine's reaches what is parsed. +fn path() -> std::ffi::OsString { + std::env::var_os("PATH").unwrap_or_default() +} + +/// The descriptors an ordinary child of this process has open, as `ls` +/// lists them. The child's own listing descriptor is among them, the same +/// one each time. +fn in_an_ordinary_child() -> BTreeSet { + let out = Command::new("ls") + .arg("/dev/fd") + .env_clear() + .env("PATH", path()) + .output() + .expect("ls runs"); + assert!(out.status.success(), "{out:?}"); + String::from_utf8_lossy(&out.stdout) + .split_whitespace() + .map(|fd| fd.parse().expect("a descriptor number")) + .collect() +} + +/// A shell command that prints whether descriptor `fd` is open in it. +fn probe(fd: i32) -> String { + format!("if [ -e /dev/fd/{fd} ]; then echo inherited; else echo closed; fi") +} + +/// A terminal alive in the test process adds nothing to what the process's +/// other children inherit. The writer termlens duplicates from the master +/// was a plain `dup`, so every child started while a terminal lived held +/// that master open. +#[test] +fn a_live_terminal_leaks_no_descriptor_into_an_ordinary_child() -> termlens::Result<()> { + let _table = TABLE.lock().unwrap_or_else(PoisonError::into_inner); + let before = in_an_ordinary_child(); + let t = Terminal::builder() + .timeout(Duration::from_secs(10)) + .spawn("cat")?; + let during = in_an_ordinary_child(); + drop(t); + assert_eq!( + during, before, + "a descriptor of the live terminal reached an unrelated child" + ); + Ok(()) +} + +/// A child of termlens gets stdio and nothing else, even from a test process +/// holding a descriptor that any other child would inherit. The PTY layer +/// closed every such descriptor between fork and exec. termlens, which now +/// spawns the child itself, marks them close-on-exec instead, so that a +/// failed exec can still report through std's pipe (#563). +#[test] +fn a_termlens_child_inherits_nothing_but_its_terminal() -> termlens::Result<()> { + let _table = TABLE.lock().unwrap_or_else(PoisonError::into_inner); + let file = std::fs::File::open(concat!(env!("CARGO_MANIFEST_DIR"), "/Cargo.toml"))?; + let fd = file.as_raw_fd(); + // SAFETY: `fd` is open for as long as `file` lives, and F_SETFD changes + // only its descriptor flags. + #[allow(unsafe_code)] + let cleared = unsafe { libc::fcntl(fd, libc::F_SETFD, 0) }; + assert_eq!(cleared, 0, "clearing close-on-exec on {fd}"); + + let ordinary = Command::new("sh") + .args(["-c", &probe(fd)]) + .env_clear() + .env("PATH", path()) + .output() + .expect("sh runs"); + assert_eq!( + String::from_utf8_lossy(&ordinary.stdout).trim(), + "inherited", + "an ordinary child must inherit {fd}, or this test proves nothing" + ); + + // The child reads a line before it exits, so its answer cannot be lost + // to a teardown racing the read (DESIGN.md §2). + let mut t = Terminal::builder() + .timeout(Duration::from_secs(10)) + .env_clear() + .env("PATH", path()) + .args(["-c", &format!("{}; read _", probe(fd))]) + .spawn("sh")?; + t.wait_until(|s| s.contains("closed") || s.contains("inherited"))?; + let screen = t.screen().text(); + t.send(Key::Enter)?; + assert!(t.wait_exit()?.success()); + assert!( + screen.contains("closed") && !screen.contains("inherited"), + "descriptor {fd} reached the child:\n{screen}" + ); + drop(file); + Ok(()) +} diff --git a/crates/termlens/tests/inspect.rs b/crates/termlens/tests/inspect.rs index 649f828..f47b7c5 100644 --- a/crates/termlens/tests/inspect.rs +++ b/crates/termlens/tests/inspect.rs @@ -775,6 +775,34 @@ fn inspect_example_and_command_agree_on_exit_codes_when_inspect_cannot_run() { } } +/// A program that exists but cannot be executed is inspect failing to run +/// it, exit 2 with the missing interpreter named, in both. Both used to +/// paint a screen for a child killed by SIGABRT and exit 0 (#563). +#[cfg(unix)] +#[test] +fn inspect_example_and_command_agree_that_a_missing_interpreter_is_a_spawn_failure() { + let scratch = std::path::Path::new(env!("CARGO_TARGET_TMPDIR")) + .join(format!("inspect-exec-failure-{}", std::process::id())); + std::fs::create_dir_all(&scratch).expect("scratch directory"); + let script = scratch.join("missing-interpreter"); + common::write_script(&script, "#!/nonexistent/interpreter\n"); + let script = script.to_str().expect("utf-8 scratch directory"); + let (example, command) = run_both(&["--size", "40x3", script]); + for (who, out) in [("example", &example), ("command", &command)] { + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(2), "{who}: {stderr}"); + assert!( + out.stdout.is_empty(), + "{who} painted a screen for a child that never ran" + ); + assert!( + stderr.contains("failed to spawn") && stderr.contains("/nonexistent/interpreter"), + "{who}: {stderr}" + ); + } + let _ = std::fs::remove_dir_all(&scratch); +} + /// `--help` and `--version` succeed on stdout with nothing on stderr, and /// `--version` is the one line both print. The usage text itself is not /// compared: it names the tool, and the two legitimately differ (#453). diff --git a/crates/termlens/tests/process.rs b/crates/termlens/tests/process.rs index 65dc3ef..17a9ad8 100644 --- a/crates/termlens/tests/process.rs +++ b/crates/termlens/tests/process.rs @@ -91,6 +91,140 @@ fn a_relative_current_dir_is_the_test_process_s_and_a_relative_program_resolves_ Ok(()) } +/// A program that exists but cannot be executed is a spawn error naming what +/// is missing, not a child dead of SIGABRT (#563). The PTY layer closed the +/// pipe std reports a failed exec through, so the child aborted writing to +/// it and `spawn` returned `Ok`. The usual culprit is the interpreter a +/// script's `#!` line names. The sneakiest is the `\r` that CRLF line +/// endings leave on it, which the message shows escaped. The messages keep +/// the prefix the skill's failure table promises. +#[cfg(unix)] +#[test] +fn a_script_whose_interpreter_cannot_run_fails_to_spawn_and_names_it() -> termlens::Result<()> { + use std::os::unix::fs::PermissionsExt; + + let scratch = std::path::Path::new(env!("CARGO_TARGET_TMPDIR")) + .join(format!("exec-failure-{}", std::process::id())); + std::fs::create_dir_all(&scratch)?; + let refused = |name: &str, script: &str, mode: u32| -> termlens::Result { + let path = scratch.join(name); + common::write_script(&path, script); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(mode))?; + match Terminal::builder() + .timeout(Duration::from_secs(10)) + .spawn(&path) + { + Ok(_) => panic!("{name}: spawned, and its interpreter cannot run"), + Err(Error::Spawn { reason, .. }) => Ok(reason), + Err(other) => panic!("{name}: not a spawn error: {other}"), + } + }; + for (name, script, culprit) in [ + ( + "missing", + "#!/nonexistent/interpreter\necho unreachable\n", + r#"its #! interpreter "/nonexistent/interpreter" does not exist"#, + ), + ( + "crlf", + "#!/bin/sh\r\necho unreachable\r\n", + r#"its #! interpreter "/bin/sh\r" does not exist"#, + ), + ] { + let reason = refused(name, script, 0o755)?; + assert!( + reason.starts_with("cannot execute ") && reason.ends_with(culprit), + "{name}: the message must name the interpreter: {reason}" + ); + } + // An interpreter that is there but is not a program is named as well. + // The kernel's EACCES alone would blame the script, which can be run. + let plain = scratch.join("plain-text"); + std::fs::write(&plain, "not a program\n")?; + std::fs::set_permissions(&plain, std::fs::Permissions::from_mode(0o644))?; + let reason = refused( + "plain-interpreter", + &format!("#!{}\n", plain.display()), + 0o755, + )?; + assert!( + reason.ends_with(&format!( + "its #! interpreter {plain:?} is not an executable file" + )), + "{reason}" + ); + // A script that may be run but not read cannot show its `#!` line, so + // the message names both suspects rather than guessing the loader. Root + // reads it anyway, and the interpreter is named outright. + let reason = refused("execute-only", "#!/nonexistent/interpreter\n", 0o111)?; + assert!(reason.contains("#! interpreter"), "{reason}"); + let _ = std::fs::remove_dir_all(&scratch); + Ok(()) +} + +/// Any exec the kernel refuses is a spawn error carrying its reason, not +/// only a missing interpreter. Every Unix refuses an argument this long, +/// with E2BIG. +#[cfg(unix)] +#[test] +fn an_argument_the_kernel_refuses_is_a_spawn_error_with_its_reason() { + let spawned = Terminal::builder() + .timeout(Duration::from_secs(10)) + .args(["-c", "true", &"x".repeat(2 << 20)]) + .spawn("sh"); + // Matched, not unwrapped: the terminal and the error both carry the + // 2 MiB command line, which a failing assertion would print whole. + match spawned { + Ok(_) => panic!("a 2 MiB argument is past every Unix's limit, and it spawned"), + Err(Error::Spawn { reason, .. }) => { + assert!(reason.contains("Argument list too long"), "{reason}"); + } + Err(_) => panic!("not a spawn error"), + } +} + +/// A failure before the exec reads as std reports it, and does not blame a +/// program that is fine. A NUL in a variable is refused before anything is +/// forked. A working directory that cannot be entered fails in the child, +/// before the exec, and the directory is named. +#[cfg(unix)] +#[test] +fn a_failure_before_the_exec_does_not_blame_the_program() -> termlens::Result<()> { + use std::os::unix::fs::PermissionsExt; + + use termlens::TerminalBuilder; + + let reason = + |builder: TerminalBuilder| match builder.timeout(Duration::from_secs(10)).spawn("sh") { + Ok(_) => panic!("spawned"), + Err(Error::Spawn { reason, .. }) => reason, + Err(other) => panic!("not a spawn error: {other}"), + }; + let nul = reason(Terminal::builder().env("A", "a\0b")); + assert!( + nul.contains("nul byte") && !nul.contains("cannot execute"), + "{nul}" + ); + + let scratch = std::path::Path::new(env!("CARGO_TARGET_TMPDIR")) + .join(format!("unenterable-{}", std::process::id())); + std::fs::create_dir_all(&scratch)?; + std::fs::set_permissions(&scratch, std::fs::Permissions::from_mode(0o600))?; + // Root enters any directory, and then there is nothing to refuse. + let refused = std::fs::metadata(scratch.join("probe")) + .is_err_and(|e| e.kind() == std::io::ErrorKind::PermissionDenied); + let chdir = refused.then(|| reason(Terminal::builder().current_dir(&scratch))); + std::fs::set_permissions(&scratch, std::fs::Permissions::from_mode(0o700))?; + let _ = std::fs::remove_dir_all(&scratch); + if let Some(chdir) = chdir { + assert!( + chdir.starts_with("cannot enter ") && chdir.contains("unenterable-"), + "{chdir}" + ); + } + Ok(()) +} + #[test] fn current_dir_runs_the_child_where_asked() -> termlens::Result<()> { // Canonicalize: /tmp is a symlink on macOS and `--cwd` reports the real diff --git a/crates/termlens/tests/queries.rs b/crates/termlens/tests/queries.rs index 234228d..dce92a8 100644 --- a/crates/termlens/tests/queries.rs +++ b/crates/termlens/tests/queries.rs @@ -858,6 +858,8 @@ fn xtgettcap_answers_what_it_knows_and_declines_the_rest() -> termlens::Result<( /// `TN` reports whatever `TERM` the child was actually given, so an /// application cannot get two different answers to "which terminal is this?". +/// Set twice, the later value is the one the child gets, and so the one `TN` +/// reports: it used to report the first. #[test] #[cfg_attr( windows, @@ -867,6 +869,7 @@ fn xtgettcap_tn_follows_the_configured_term() -> termlens::Result<()> { let mut t = common::spawn_emit( Terminal::builder() .size(120, 6) + .env("TERM", "vt100") .env("TERM", "xterm") .timeout(Duration::from_secs(10)), &[ diff --git a/docs/DESIGN.md b/docs/DESIGN.md index a31c8db..b4c5272 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -201,6 +201,82 @@ answer" — a hang becomes a diagnosis. `answer_queries(false)` mutes the responder for tests that need a silent terminal; the diagnosis still works. +### On Unix termlens starts the child itself + +portable-pty opens the PTY, but on Unix it no longer starts the child. Its +`spawn_command` closes every descriptor above stdio between fork and exec, +and one of them is the close-on-exec pipe through which std reports a +failed `exec`. When the program exists but cannot be executed, the child's +write into that pipe failed and std aborted the child. `spawn` then +returned `Ok` with a process already dead of SIGABRT (#563; upstream, +wezterm/wezterm#7893). The usual causes are a missing `#!` interpreter (a +`\r` left by CRLF line endings counts), a missing dynamic loader, and an +argument list over the kernel's limit. + +`src/spawn.rs` builds the command portable-pty 0.9.0 would build and runs +it with `std::process::Command`: + +- **Resolution** finds the same file by the same rules. `./x` and `../x` + are looked up in the working directory alone. Any other relative name is + searched for along the child's `PATH`, each entry taken from the working + directory. A candidate must pass `access(X_OK)` and must not be a + directory. `argv[0]` is the name as given. A working directory that is + not a directory gives way to the child's `HOME`, as in portable-pty. +- **The environment** is the same: the inherited one unless `env_clear`, + plus the `SHELL` that portable-pty fills in from the login shell when the + process has none, then termlens's pins and the test's own variables. +- **The session** gets the same `pre_exec` steps. Six signals go back to + their defaults and the mask is emptied. `setsid` gives the child a + session of its own, and `TIOCSCTTY` makes the PTY its controlling + terminal. portable-pty's umask step is left out, because it acts only on + a umask termlens never sets. +- **Descriptors** above stdio are marked close-on-exec rather than closed. + A successful exec closes exactly what portable-pty closed, and a failed + one still has std's pipe to report through. + +A unit test in `spawn.rs` starts one probe both ways and requires the same +answers about argv[0], directory, environment, descriptors, umask, process +group and controlling terminal. A portable-pty release that changes its +own spawn fails that test instead of drifting apart from this one. + +The error names the culprit where the kernel's errno would point at the +wrong file. `ENOENT` for a script that is plainly there means a missing +`#!` interpreter, which is named. For a binary it means a missing loader. +`EACCES` from an interpreter that cannot be run names the interpreter. A +failure before the exec, such as a NUL in an argument or a directory that +cannot be entered, reads as std reports it, with the directory named in the +second case. + +Reporting the exec has a price. std's `spawn` now returns only once the +exec has happened, so a spawn holds the lifecycle lock about a quarter of a +millisecond longer on Linux. On a machine whose security software inspects +every exec, it is held for the length of that inspection. + +The slave is opened by name (`tty_name`, `O_NOCTTY`), because portable-pty +does not expose its own handle. Four cases still go to portable-pty +unchanged: + +- a process whose children the kernel reaps, because SIGCHLD is ignored + or carries `SA_NOCLDWAIT`. std panics when it cannot reap a failed exec, + and every wait fails there anyway; +- a working directory that is not one, with no UTF-8 `HOME` in the child's + environment; portable-pty then reads the passwd entry; +- a program that does not resolve. portable-pty's diagnosis is what + termlens has always reported, word for word; +- a slave that cannot be named or opened. + +These are the old behaviour, not new risk. On Windows ConPTY reports a +failed `CreateProcess` itself, and portable-pty spawns the child as before. + +termlens's own descriptors are all close-on-exec, the second writer it +duplicates from the master included. One window remains, and it belongs to +the PTY layer. `openpty` creates the pair without close-on-exec, and +portable-pty marks it a moment later. A child that another thread of the +test process forks inside that window inherits the PTY. If it holds the +slave, that terminal reports no EOF until the child exits. It is measured +at a few in a thousand concurrent spawns, and only alongside processes the +test starts itself. + ### Windows: ConPTY renders, and that decides what is claimed `portable-pty` gives the same four layers a ConPTY backend, and the crate diff --git a/skills/termlens/SKILL.md b/skills/termlens/SKILL.md index f29c11b..a832da3 100644 --- a/skills/termlens/SKILL.md +++ b/skills/termlens/SKILL.md @@ -424,6 +424,7 @@ included in `X`, and `terminal closed (EOF) while waiting for X` is | `input not receivable: the application has not enabled mouse tracking` | `click`/`drag`/`scroll` before the app enabled the mouse | `wait_until(|s| s.mouse_mode() != MouseMode::None)` first | | `input not receivable: mouse at (50, 2) is outside the 20x5 grid` | coordinates swapped or out of range | rule 6 | | `failed to spawn \`sh\`: \`sh\` is a bare program name and env_clear() removed PATH` | bare program name under `env_clear` | absolute path, `bin!`, or `.env("PATH", …)` | +| ``failed to spawn `./x`: cannot execute …: its #! interpreter "…" does not exist`` | the script's `#!` line names a file that is not there, or with `is not an executable file` one that cannot be run; a `\r` at the end of the quoted name means CRLF line endings | install the interpreter, point the `#!` line at one that exists, or save the script with LF endings | | `invalid terminal size: a terminal needs at least 2 columns and 2 rows` | geometry below 2x2 (past 1000 has its own message) | rule 5 | | `the terminal emulator failed and the screen stopped advancing` (`Error::Emulator`) | a bug in the emulation, not in your app | report it to termlens with the detail it names |