diff --git a/CHANGELOG.md b/CHANGELOG.md index d18bd88a..0f14ddfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -153,6 +153,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **The daemon raises its own open-file limit, and says so when it still runs + out.** A pane costs about three descriptors, and the daemon inherited its + soft limit from whatever launched it — with the historic macOS default of + 256 that is a ceiling of roughly eighty panes, against a documented maximum + of 16,384. The eighty-first failed with `dup of fd 93 failed`, which names + neither the limit nor the way past it. The soft limit is now raised to the + hard one at startup, which needs no privilege and leaves an administrator + who lowered the hard limit in charge; measured under an inherited soft limit + of 96, the ceiling went from 26 extra tabs to more than 45. When the hard + limit really is the ceiling, the refusal now says "out of file descriptors — + this account's open-file limit is the ceiling on how many panes can run at + once; raise it (`ulimit -n`, …)", keeping the original error in parentheses. + - **`doctor` now checks the configured shell.** A `shell` naming something that is not there — missing, a directory, not executable — makes every new tab and every `tty7 new` fail, while `doctor` reported `config ok`, because diff --git a/crates/tty7-core/src/daemon/pane.rs b/crates/tty7-core/src/daemon/pane.rs index 8e4dd328..3e4d580d 100644 --- a/crates/tty7-core/src/daemon/pane.rs +++ b/crates/tty7-core/src/daemon/pane.rs @@ -128,6 +128,28 @@ struct SpawnConfig { /// "daemon refused Spawn: spawn failed: Unable to spawn … (ENOENT: No such /// file or directory)" — and the one fact that matters is buried in it. /// +/// Re-word a spawn failure that is really the open-file limit. +/// +/// The text is matched rather than an errno read, because portable-pty hands +/// back an `anyhow::Error` built from a message — the code is not on it to +/// consult. Both spellings are covered: the libc phrase, and the one +/// portable-pty writes itself when the `dup` behind the pty fails. +/// +/// A miss costs nothing: the original error is used unchanged. +pub(crate) fn out_of_descriptors(error: &anyhow::Error) -> Option { + let text = format!("{error:#}").to_ascii_lowercase(); + let looks_like = text.contains("too many open files") + || text.contains("emfile") + || text.contains("dup of fd"); + looks_like.then(|| { + anyhow::anyhow!( + "out of file descriptors — this account's open-file limit is the ceiling \ + on how many panes can run at once; raise it (`ulimit -n`, or the \ + launchd/systemd limit for the session) and start the server again ({error:#})" + ) + }) +} + fn build_spawn_config( pane: u64, cwd: Option, @@ -4619,6 +4641,50 @@ mod tests { assert!(matches!(rx.try_recv(), Ok(DaemonMsg::Agent(None)))); } + /// Running out of descriptors says so, in words that name the way out. + /// + /// This is the ceiling on how many panes can run at once, and neither of + /// the two sentences it arrives as says that. libc gives "Too many open + /// files (os error 24)"; portable-pty, when the `dup` behind a pty is what + /// failed, gives "dup of fd 93 failed" — which does not even contain the + /// word "file". A reader meeting either one has no reason to think about + /// `ulimit`. + /// + /// Matched on the text rather than an errno, because portable-pty hands + /// back an `anyhow::Error` built from a message and the code is not on it + /// to consult. Anything else passes through untouched: a wrong guess here + /// would replace a true sentence with a false one. + #[test] + fn descriptor_exhaustion_is_re_worded_and_nothing_else_is() { + for text in [ + "Too many open files (os error 24)", + "dup of fd 93 failed", + "EMFILE: too many open files", + ] { + let re = out_of_descriptors(&anyhow::anyhow!("{text}")) + .unwrap_or_else(|| panic!("{text:?} is descriptor exhaustion and was not named")); + let said = format!("{re:#}"); + assert!( + said.contains("out of file descriptors") && said.contains("ulimit"), + "the re-wording has to name the limit and the way past it: {said}" + ); + assert!( + said.contains(text), + "and keep the original, for whoever needs it: {said}" + ); + } + for text in [ + "no such program on this machine: /nope", + "Permission denied (os error 13)", + "spawn failed: ENOENT", + ] { + assert!( + out_of_descriptors(&anyhow::anyhow!("{text}")).is_none(), + "{text:?} is not descriptor exhaustion and must be left alone" + ); + } + } + /// A second agent in the same pane does not inherit the first one's turn. /// /// The probe reads the foreground process twice a second, and `claude; diff --git a/crates/tty7-core/src/daemon/server.rs b/crates/tty7-core/src/daemon/server.rs index a0d94109..da895dde 100644 --- a/crates/tty7-core/src/daemon/server.rs +++ b/crates/tty7-core/src/daemon/server.rs @@ -365,6 +365,8 @@ pub fn run_daemon() -> anyhow::Result<()> { // first that does. Keep it that way, or move this up again. #[cfg(unix)] serve_sigterm(registry.clone()); + #[cfg(unix)] + raise_open_file_limit(); #[cfg(any(unix, windows))] { @@ -654,6 +656,53 @@ fn run_with(registry: Arc) -> anyhow::Result<()> { } #[cfg(unix)] +/// Take the open-file limit up to whatever this user is allowed. +/// +/// A pane costs about three descriptors — the pty master, and the pipes behind +/// it — and the daemon inherits its soft limit from whatever started it. With +/// the historic macOS default of 256 that is a ceiling of roughly eighty +/// panes, against a `MAX_PANES` of 16,384, and the eighty-first fails with +/// `dup of fd 93 failed`: a sentence that names neither the limit nor the way +/// past it. +/// +/// Raising the *soft* limit to the *hard* one needs no privilege — it is the +/// ordinary thing a long-lived server does at startup, and it leaves an +/// administrator who lowered the hard limit in charge, since that is the one +/// this cannot touch. +/// +/// Capped at `OPEN_MAX` because macOS refuses `RLIM_INFINITY` here outright: +/// asking for it fails and leaves the low limit in place, which is the one +/// outcome worth avoiding. +/// +/// Best effort throughout. A daemon that could not raise its limit still runs; +/// it simply runs into the ceiling sooner, and `spawn` now says so when it +/// does. +#[cfg(unix)] +fn raise_open_file_limit() { + const OPEN_MAX: libc::rlim_t = 10_240; + let mut limit = libc::rlimit { + rlim_cur: 0, + rlim_max: 0, + }; + if unsafe { libc::getrlimit(libc::RLIMIT_NOFILE, &mut limit) } != 0 { + return; + } + let want = limit.rlim_max.min(OPEN_MAX); + if want <= limit.rlim_cur { + return; + } + let raised = libc::rlimit { + rlim_cur: want, + rlim_max: limit.rlim_max, + }; + if unsafe { libc::setrlimit(libc::RLIMIT_NOFILE, &raised) } == 0 { + log::debug!( + "open-file limit raised from {} to {want}", + limit.rlim_cur + ); + } +} + fn serve_sigterm(registry: Arc) { let set = unsafe { let mut set: libc::sigset_t = std::mem::zeroed(); @@ -762,6 +811,13 @@ fn handle_conn(stream: Stream, registry: Arc) -> anyhow::Result<()> { match DaemonPane::spawn(id, cwd, size, shell, owner, workspace, restore, on_dead) { Ok(p) => p, Err(e) => { + // Descriptor exhaustion is the one failure along this + // path whose own words say nothing useful — portable-pty + // reports `dup of fd 93 failed`, and libc reports `Too + // many open files`, neither of which names the limit or + // the way past it. Several calls in `spawn` can hit it, + // so it is re-worded here, where all of them arrive. + let e = crate::daemon::pane::out_of_descriptors(&e).unwrap_or(e); let mut w = write_stream; // The daemon's own error is already a sentence; a second // "spawn failed:" in front of it only pads the one the