From 2cb33b1a5d48bb185a4f27a56b6e4bc697ac2d10 Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Tue, 25 Aug 2026 09:24:35 +0200 Subject: [PATCH] service: the daemon needs more than launchd's default descriptors The Mac daemon died with `Error: Too many open files (os error 24)`. It is in `service.err.log` on Silber. WHY. `render_launch_agent_plist` writes `SoftResourceLimits` ONLY when the operator passes `--memory-max-mb`, and even then it writes only `ResidentSetSize`. `NumberOfFiles` appears nowhere. So a fabric agent inherits launchd's own default, which is small, and nobody chose it. That default is not a ceiling anybody picked for this process. The daemon holds a QUIC endpoint, a connection per peer, a control socket, a dial socket per tunnel, and the files it is syncing. A DESCRIPTOR CEILING IS NOT OPTIONAL, so it is now always written. The resident-set ceiling stays opt-in and unchanged: launchd treats ResidentSetSize as a reclaim preference rather than a kill, but it is still a fixed number, and shipping one by default declares a healthy working set nobody has measured. 8192 IS NOT A MEASUREMENT AND I AM SAYING SO. It is far above any working set this daemon has shown; the entry syncing 17,600 files sat at a few dozen descriptors. The point is to remove an arbitrary small number, not to install a different arbitrary number close enough to matter. I ALSO ADDED A CHECK THAT THE PLIST IS A PLIST. A malformed one does not fail a string assertion, it fails at install time on somebody's machine. `rendered_plists_are_valid_property_lists` renders both branches and hands them to `plutil -lint`. I proved it can fail by injecting an unclosed tag: plutil rejected the plist for memory_max_mb=None: (Encountered unknown tag unclosed on line 31) Off a Mac there is no `plutil`, so it says the plist was not linted rather than passing silently as though it had been. NOT APPLIED TO THE RUNNING SERVICE YET. The plist on disk is the one the old build wrote; it takes a `fabric service install` to replace it, and machine runtime belongs to root, so I will tell `Silber.root` before touching the launchd job. Agent: Silber.fabric --- src/service.rs | 103 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 85 insertions(+), 18 deletions(-) diff --git a/src/service.rs b/src/service.rs index d39277f..c169c7a 100644 --- a/src/service.rs +++ b/src/service.rs @@ -602,27 +602,53 @@ pub fn render_launch_agent_plist(home: &FabricHome, spec: &ServiceSpec) -> Resul // launchd treats ResidentSetSize as a reclaim preference rather than a kill, // but it is still a fixed number, and shipping one by default declares a // healthy working set nobody has measured yet. - let resource_limits = match spec.memory_max_mb { - None => String::new(), - Some(mb) => { - let rss_bytes = mb - .checked_mul(1024) - .and_then(|value| value.checked_mul(1024)) - .context("--memory-max-mb is too large")?; - format!( - " SoftResourceLimits\n\ - \n\ - ResidentSetSize\n\ - {rss_bytes}\n\ - \n\ - HardResourceLimits\n\ + // A DESCRIPTOR CEILING IS NOT OPTIONAL, unlike the resident-set one. + // + // A launchd agent inherits launchd's own default, and that default is small. + // This daemon holds a QUIC endpoint, a connection per peer, a control + // socket, a dial socket per tunnel and the files it is syncing, so the + // default is not a ceiling anybody chose for it. + // + // It ran out: `service.err.log` on the Mac carries + // `Error: Too many open files (os error 24)` and the daemon died there. + // + // 8192 is not a measurement, and saying so matters. It is simply far above + // any working set this daemon has shown, which for the entry that syncs + // 17,600 files sat at a few dozen descriptors. The point is to remove an + // arbitrary small number, not to install a different arbitrary number close + // enough to matter. + let mut soft_limits = String::from( + " NumberOfFiles\n\ + 8192\n", + ); + let mut hard_limits = String::new(); + + // The resident-set ceiling stays opt-in. launchd treats ResidentSetSize as a + // reclaim preference rather than a kill, but it is still a fixed number, and + // shipping one by default declares a healthy working set nobody has measured. + if let Some(mb) = spec.memory_max_mb { + let rss_bytes = mb + .checked_mul(1024) + .and_then(|value| value.checked_mul(1024)) + .context("--memory-max-mb is too large")?; + soft_limits.push_str(&format!( + " ResidentSetSize\n\ + {rss_bytes}\n" + )); + hard_limits = format!( + " HardResourceLimits\n\ \n\ ResidentSetSize\n\ {rss_bytes}\n\ \n" - ) - } - }; + ); + } + let resource_limits = format!( + " SoftResourceLimits\n\ + \n\ +{soft_limits} \n\ +{hard_limits}" + ); let stdout_path = home.root().join("logs/service.out.log"); let stderr_path = home.root().join("logs/service.err.log"); let args = spec @@ -873,9 +899,15 @@ mod tests { assert!(unit.contains("Restart=on-failure")); let plist = render_launch_agent_plist(&home, &spec)?; + // The resident-set ceiling stays opt-in, so none of it appears here. assert!(!plist.contains("ResidentSetSize")); - assert!(!plist.contains("SoftResourceLimits")); assert!(!plist.contains("HardResourceLimits")); + // The DESCRIPTOR ceiling is not opt-in. Without it the daemon inherits + // launchd's own small default, and it has already died of that: + // `Error: Too many open files (os error 24)`. + assert!(plist.contains("SoftResourceLimits")); + assert!(plist.contains("NumberOfFiles")); + assert!(plist.contains("8192")); assert!(plist.contains("KeepAlive")); Ok(()) } @@ -974,3 +1006,38 @@ mod tests { Ok(()) } } + +#[cfg(test)] +mod plist_validity { + use super::*; + + /// A malformed plist does not fail a string assertion, it fails at install + /// time on somebody's machine. Render both branches and hand them to the + /// system parser. + #[test] + fn rendered_plists_are_valid_property_lists() -> Result<()> { + let home = FabricHome::new(std::path::Path::new("/home/nathan/.local/share/fabric")); + for memory in [None, Some(512u64)] { + let spec = ServiceSpec::new("/usr/local/bin/fabric", home.root(), true, true, memory)?; + let plist = render_launch_agent_plist(&home, &spec)?; + let path = std::env::temp_dir().join(format!("fabric-plist-{:?}.plist", memory)); + std::fs::write(&path, &plist)?; + let out = std::process::Command::new("plutil") + .arg("-lint") + .arg(&path) + .output(); + let _ = std::fs::remove_file(&path); + match out { + Ok(out) => assert!( + out.status.success(), + "plutil rejected the plist for memory_max_mb={memory:?}: {}\n{plist}", + String::from_utf8_lossy(&out.stderr) + ), + // Not a Mac, so there is nothing to lint with. Say so rather + // than passing silently as if it had been checked. + Err(_) => eprintln!("plutil unavailable, plist not linted here"), + } + } + Ok(()) + } +}