diff --git a/Cargo.lock b/Cargo.lock index bf03565..4f96861 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1465,9 +1465,9 @@ checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" [[package]] name = "libc" -version = "0.2.186" +version = "0.2.189" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" +checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" [[package]] name = "libdbus-sys" @@ -2556,6 +2556,7 @@ dependencies = [ "getrandom 0.4.3", "igd-next", "keyring", + "libc", "ratatui", "schemars", "serde", diff --git a/soulseek-rs-lib/src/peer/listen.rs b/soulseek-rs-lib/src/peer/listen.rs index 72d5e1b..1407d93 100644 --- a/soulseek-rs-lib/src/peer/listen.rs +++ b/soulseek-rs-lib/src/peer/listen.rs @@ -13,6 +13,13 @@ use crate::types::Download; use crate::utils::lock::RwLockExt; use crate::{DownloadStatus, debug, error, info, trace}; +/// How long to wait before accepting again after a failure. +/// +/// The failure worth pausing for is running out of file descriptors, which a +/// busy search can reach: it does not clear by the next instruction, and +/// retrying flat out spins a core and floods the log. +const ACCEPT_BACKOFF: Duration = Duration::from_millis(100); + const PEER_INIT_MESSAGE_CODE: u8 = 1; /// How long an accepted peer gets to send its peer-init handshake. @@ -406,6 +413,11 @@ impl Listen { "[listener] Failed to accept connection: {}", stream.unwrap_err() ); + // Running out of file descriptors does not clear by the next + // instruction, and retrying flat out turns one exhausted + // moment into a spinning core and thousands of identical log + // lines. Pause long enough for something to be released. + std::thread::sleep(ACCEPT_BACKOFF); continue; }; diff --git a/soulseek-rs/Cargo.toml b/soulseek-rs/Cargo.toml index 73856c5..5c92ef6 100644 --- a/soulseek-rs/Cargo.toml +++ b/soulseek-rs/Cargo.toml @@ -47,3 +47,6 @@ getrandom = "0.4.3" [dev-dependencies] schemars = "1.2.1" tempfile = "3" + +[target."cfg(unix)".dependencies] +libc = "0.2.189" diff --git a/soulseek-rs/src/daemon/mod.rs b/soulseek-rs/src/daemon/mod.rs index 676c393..6937f4c 100644 --- a/soulseek-rs/src/daemon/mod.rs +++ b/soulseek-rs/src/daemon/mod.rs @@ -44,6 +44,15 @@ pub const TOKEN_NAME: &str = "daemon.token"; /// loses little, rare enough not to churn the disk on a busy queue. const SAVE_INTERVAL: Duration = Duration::from_secs(5); +/// Open files the daemon asks the OS for at startup. +/// +/// The client holds a socket per peer and will talk to a thousand of them +/// during a busy search, plus transfers, the share index, and control +/// connections. A shell hands its children a soft limit of 256 on macOS, which +/// a single search blows through — and a one-shot command never noticed +/// because it exited before it mattered. A resident process has to ask. +const WANTED_OPEN_FILES: u64 = 8192; + #[must_use] pub fn socket_path() -> Option { crate::persist::paths::state_dir().map(|dir| dir.join(SOCKET_NAME)) @@ -152,6 +161,21 @@ pub fn run( let token = load_or_create_token(&state_dir.join(TOKEN_NAME)) .map_err(|e| CliError::usage(format!("cannot read the token: {e}")))?; + // Before anything opens a socket: running out of file descriptors mid + // search leaves the client unable to accept peers *or* control + // connections, and the only symptom is everything quietly failing. + match raise_open_file_limit() { + Ok(limit) if limit < WANTED_OPEN_FILES => ctx.out.warn(&format!( + "only {limit} open files allowed; a busy search wants \ + {WANTED_OPEN_FILES}. Raise it with `ulimit -n {WANTED_OPEN_FILES}` \ + before starting the daemon." + )), + Ok(_) => {} + Err(e) => ctx + .out + .warn(&format!("could not check the open-file limit: {e}")), + } + // Bind before logging in: a second daemon should fail in a millisecond, // not after a full login handshake against the server. let mut listeners = Vec::new(); @@ -247,6 +271,59 @@ pub fn run( Ok(()) } +/// Ask the OS for enough open files to run a busy session, and report what we +/// actually got. +/// +/// Raising our own soft limit is what a service does: the limit a shell +/// inherits is sized for a shell. The hard limit is the ceiling, and asking +/// past it fails outright rather than clamping, so this asks for the smaller +/// of what we want and what we are allowed. +#[cfg(unix)] +fn raise_open_file_limit() -> std::io::Result { + // SAFETY: every call takes a pointer to a `rlimit` this function owns, and + // none of them retains it. + unsafe { + let mut limit = std::mem::zeroed::(); + if libc::getrlimit(libc::RLIMIT_NOFILE, &raw mut limit) != 0 { + return Err(std::io::Error::last_os_error()); + } + if limit.rlim_cur >= WANTED_OPEN_FILES { + return Ok(limit.rlim_cur); + } + + let previous = limit.rlim_cur; + let ceiling = if limit.rlim_max == libc::RLIM_INFINITY { + WANTED_OPEN_FILES + } else { + limit.rlim_max.min(WANTED_OPEN_FILES) + }; + + // macOS rejects an unlimited *hard* limit for open files, so raising + // the soft one while leaving `rlim_max` as it came back fails with + // EINVAL. Naming both is what it will accept. + for candidate in [ + libc::rlimit { + rlim_cur: ceiling, + rlim_max: limit.rlim_max, + }, + libc::rlimit { + rlim_cur: ceiling, + rlim_max: ceiling, + }, + ] { + if libc::setrlimit(libc::RLIMIT_NOFILE, &raw const candidate) == 0 { + return Ok(ceiling); + } + } + Ok(previous) + } +} + +#[cfg(not(unix))] +const fn raise_open_file_limit() -> std::io::Result { + Ok(WANTED_OPEN_FILES) +} + /// Say what is now possible, once, on stderr. /// /// The token goes here rather than only into a file because the moment you diff --git a/soulseek-rs/src/daemon/server/transport.rs b/soulseek-rs/src/daemon/server/transport.rs index 5c2e98b..b9ecd00 100644 --- a/soulseek-rs/src/daemon/server/transport.rs +++ b/soulseek-rs/src/daemon/server/transport.rs @@ -38,6 +38,9 @@ const MAX_LINE: u64 = 1 << 20; /// connection open for events without sending anything. const AUTH_TIMEOUT: Duration = Duration::from_secs(30); +/// How long to wait before accepting again after a failure. +const ACCEPT_BACKOFF: Duration = Duration::from_millis(100); + /// How many control connections may be open at once. Far above any real use /// (a TUI, a few scripts, an agent), and low enough that an unauthenticated /// peer cannot exhaust the daemon's threads. @@ -132,7 +135,12 @@ impl Listener { loop { match self.accept() { Ok(stream) => spawn_connection(stream, Arc::clone(&daemon)), - Err(e) => soulseek_rs::warn!("control accept failed: {e}"), + Err(e) => { + soulseek_rs::warn!("control accept failed: {e}"); + // Out of file descriptors does not clear by the next + // instruction; retrying flat out spins a core. + std::thread::sleep(ACCEPT_BACKOFF); + } } } }