From fd0437a0329365b9139362921df0968ba8c38a13 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Sun, 27 Sep 2026 17:38:56 -0700 Subject: [PATCH] bxcore, loopd, inferproxy: a lock file keeps each to one instance Co-Authored-By: ornith-1.5-35b-a3b --- crates/bxcore/src/instance.rs | 97 +++++++++ crates/bxcore/src/lib.rs | 4 +- crates/bxcore/tests/instance.rs | 137 ++++++++++++ crates/inferproxy/src/main.rs | 14 ++ crates/inferproxy/tests/instance.rs | 228 ++++++++++++++++++++ crates/loopd/src/main.rs | 13 ++ crates/loopd/tests/instance.rs | 311 ++++++++++++++++++++++++++++ docs/implementer-log.md | 1 + docs/runbook.md | 48 +++++ 9 files changed, 852 insertions(+), 1 deletion(-) create mode 100644 crates/bxcore/src/instance.rs create mode 100644 crates/bxcore/tests/instance.rs create mode 100644 crates/inferproxy/tests/instance.rs create mode 100644 crates/loopd/tests/instance.rs diff --git a/crates/bxcore/src/instance.rs b/crates/bxcore/src/instance.rs new file mode 100644 index 0000000..a2c31da --- /dev/null +++ b/crates/bxcore/src/instance.rs @@ -0,0 +1,97 @@ +//! One instance per daemon (M5b spec, section 9a). Every daemon removes a socket it finds at its +//! path before it binds, so a second copy started by mistake would take the sockets of the one +//! that runs. Before it touches them, a daemon takes an exclusive lock on its lock file with +//! `lock`, and keeps the `InstanceLock` until the process ends. + +use std::fs::{DirBuilder, File, OpenOptions, TryLockError}; +use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt}; +use std::path::Path; + +/// The runbook entry for both ways `lock` can fail. +pub const RUNBOOK: &str = "see docs/runbook.md#already-running"; + +/// A held lock: the open lock file. The system releases the lock when the file is closed, which is +/// when this is dropped or the process ends, however it ends. The file itself stays: removing it +/// would let a second copy lock a new file while the first still runs. +#[derive(Debug)] +#[must_use = "the lock is released when this value is dropped"] +pub struct InstanceLock { + _file: File, +} + +/// Why `lock` did not take the lock. +#[derive(Debug)] +pub enum LockError { + /// Another open file holds the lock: another instance is running. + Held, + /// The lock file, or its directory, could not be created or opened, or the lock failed. + Io(std::io::Error), +} + +impl std::fmt::Display for LockError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + LockError::Held => write!(f, "another instance holds the lock"), + LockError::Io(e) => write!(f, "{e}"), + } + } +} + +impl std::error::Error for LockError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + LockError::Held => None, + LockError::Io(e) => Some(e), + } + } +} + +impl LockError { + /// The line a daemon prints when it does not start: `role` is its name, `path` the lock file. + pub fn line(&self, role: &str, path: &Path) -> String { + // `Held`: "{role}: another {role} is running (it holds {path}); {RUNBOOK}". + // `Io(e)`: "{role}: cannot take the lock {path}: {e}; {RUNBOOK}". + // Each with `format!`, the path shown with `path.display()`. + match self { + LockError::Held => format!( + "{role}: another {role} is running (it holds {}); {RUNBOOK}", + path.display() + ), + LockError::Io(e) => format!( + "{role}: cannot take the lock {path}: {e}; {RUNBOOK}", + path = path.display() + ), + } + } +} + +/// Take the exclusive lock on `path`, creating the file with mode 0600, and its directory with +/// mode 0700 when it is missing, as the daemons make their socket directories. The lock is tried +/// once, never waited for. +pub fn lock(path: &Path) -> Result { + // 1. If `path.parent()` is `Some(dir)`: `DirBuilder::new().recursive(true).mode(0o700) + // .create(dir)`. 2. `OpenOptions::new().write(true).create(true).truncate(false) + // .mode(0o600).open(path)`. An error of either is `LockError::Io`, through `map_err`. + // 3. `file.try_lock()`: `Ok(())` is `Ok(InstanceLock { _file: file })`, + // `Err(TryLockError::WouldBlock)` is `Err(LockError::Held)`, and + // `Err(TryLockError::Error(e))` is `Err(LockError::Io(e))`. + if let Some(dir) = path.parent() { + DirBuilder::new() + .recursive(true) + .mode(0o700) + .create(dir) + .map_err(LockError::Io)?; + } + let file = OpenOptions::new() + .write(true) + .create(true) + .truncate(false) + .mode(0o600) + .open(path) + .map_err(LockError::Io)?; + match file.try_lock() { + Ok(()) => Ok(InstanceLock { _file: file }), + Err(TryLockError::WouldBlock) => Err(LockError::Held), + Err(TryLockError::Error(e)) => Err(LockError::Io(e)), + } +} diff --git a/crates/bxcore/src/lib.rs b/crates/bxcore/src/lib.rs index 6c75b68..f4022e9 100644 --- a/crates/bxcore/src/lib.rs +++ b/crates/bxcore/src/lib.rs @@ -1,5 +1,6 @@ //! Shared data types and the frame codec for Boxmaker. No policy and no I/O beyond frames, except -//! the secret store (`secrets`), behind the feature of that name. +//! the secret store (`secrets`), behind the feature of that name, and the lock that keeps a daemon +//! to one instance (`instance`). pub mod audit; pub mod chain; @@ -11,6 +12,7 @@ pub mod grant; pub mod hash; pub mod hosts; pub mod ids; +pub mod instance; pub mod log; pub mod memory; pub mod sanitize; diff --git a/crates/bxcore/tests/instance.rs b/crates/bxcore/tests/instance.rs new file mode 100644 index 0000000..b9fec52 --- /dev/null +++ b/crates/bxcore/tests/instance.rs @@ -0,0 +1,137 @@ +//! `bxcore::instance`: the lock that keeps a daemon to one instance (M5b spec, section 9a). Two +//! opens of the same file in one process are two holders, as two processes are. Do not edit. + +#[path = "support/tmp.rs"] +mod tmp; + +use std::os::unix::fs::PermissionsExt; +use std::path::{Path, PathBuf}; + +use bxcore::instance::{LockError, RUNBOOK, lock}; +use tmp::TempDir; + +/// Gives a directory back its owner's permissions when dropped, so the `TempDir` declared before +/// it can be removed even when an assertion fails (tip T44). +struct Restore(PathBuf); + +impl Drop for Restore { + fn drop(&mut self) { + let _ = std::fs::set_permissions(&self.0, std::fs::Permissions::from_mode(0o700)); + } +} + +fn mode(path: &Path) -> u32 { + std::fs::metadata(path).unwrap().permissions().mode() & 0o777 +} + +#[test] +fn a_second_lock_is_held_while_the_first_lives() { + let dir = TempDir::new("lock-held"); + let path = dir.path().join("run").join("loopd.lock"); + let first = lock(&path).expect("the first lock"); + for _ in 0..3 { + match lock(&path) { + Err(LockError::Held) => {} + other => panic!("a second lock while the first lives: {other:?}"), + } + } + drop(first); +} + +#[test] +fn the_lock_is_free_again_once_the_first_is_dropped() { + let dir = TempDir::new("lock-free"); + let path = dir.path().join("run").join("loopd.lock"); + let first = lock(&path).expect("the first lock"); + drop(first); + let second = lock(&path).expect("the lock after the first was dropped"); + assert!( + matches!(lock(&path), Err(LockError::Held)), + "the second holds it now" + ); + drop(second); + assert!(path.exists(), "the lock file is left in place"); +} + +#[test] +fn a_lock_file_left_behind_is_taken() { + // A killed daemon leaves its lock file, unlocked; the next start takes it and keeps its bytes. + let dir = TempDir::new("lock-left"); + let path = dir.write("run/memoryd.lock", "left\n"); + let held = lock(&path).expect("a lock file nobody holds"); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "left\n"); + drop(held); +} + +#[test] +fn the_lock_file_is_created_with_mode_0600_and_its_directory_0700() { + let dir = TempDir::new("lock-mode"); + let run = dir.path().join("home").join("run"); + let path = run.join("brokerd.lock"); + let held = lock(&path).expect("a lock in a directory that does not exist yet"); + assert_eq!(mode(&path), 0o600, "the lock file"); + assert_eq!(mode(&run), 0o700, "the directory it made"); + assert_eq!(mode(&dir.path().join("home")), 0o700, "and the one above"); + assert_eq!( + std::fs::metadata(&path).unwrap().len(), + 0, + "nothing is written" + ); + drop(held); +} + +#[test] +fn a_directory_that_cannot_be_written_is_an_io_error() { + let dir = TempDir::new("lock-ro"); + let run = dir.path().join("run"); + std::fs::create_dir(&run).unwrap(); + let _restore = Restore(run.clone()); + std::fs::set_permissions(&run, std::fs::Permissions::from_mode(0o500)).unwrap(); + match lock(&run.join("gatewayd.lock")) { + Err(LockError::Io(e)) => { + assert_eq!(e.kind(), std::io::ErrorKind::PermissionDenied, "{e}"); + } + other => panic!("a lock in a read-only directory: {other:?}"), + } + assert!(!run.join("gatewayd.lock").exists()); +} + +#[test] +fn a_path_through_a_file_is_an_io_error() { + let dir = TempDir::new("lock-file"); + let file = dir.write("run", "not a directory\n"); + match lock(&file.join("loopd.lock")) { + Err(LockError::Io(_)) => {} + other => panic!("a lock under a file: {other:?}"), + } +} + +#[test] +fn the_lines_a_daemon_prints() { + let path = Path::new("/home/bx/run/loopd.lock"); + assert_eq!( + LockError::Held.line("loopd", path), + "loopd: another loopd is running (it holds /home/bx/run/loopd.lock); \ + see docs/runbook.md#already-running" + ); + let io = LockError::Io(std::io::Error::other("the disk is on fire")); + assert_eq!( + io.line("inferproxy", Path::new("/run/bx/infer.sock.lock")), + "inferproxy: cannot take the lock /run/bx/infer.sock.lock: the disk is on fire; \ + see docs/runbook.md#already-running" + ); + assert_eq!(RUNBOOK, "see docs/runbook.md#already-running"); +} + +#[test] +fn the_error_displays_and_has_a_source() { + use std::error::Error; + assert_eq!( + LockError::Held.to_string(), + "another instance holds the lock" + ); + assert!(LockError::Held.source().is_none()); + let io = LockError::Io(std::io::Error::other("no room")); + assert_eq!(io.to_string(), "no room"); + assert!(io.source().is_some()); +} diff --git a/crates/inferproxy/src/main.rs b/crates/inferproxy/src/main.rs index 2b44951..4a2a152 100644 --- a/crates/inferproxy/src/main.rs +++ b/crates/inferproxy/src/main.rs @@ -4,6 +4,7 @@ use std::os::unix::fs::PermissionsExt; use std::path::PathBuf; use std::process; +use bxcore::instance; use inferproxy::{Limits, serve}; fn main() { @@ -39,6 +40,19 @@ fn main() { }; let path: PathBuf = listen.into(); + // One inferproxy per socket (M5b spec, section 9a): the lock beside the socket comes before + // the stale socket is removed, so a second copy never takes the socket of the one that runs. + // Held until the process ends; `process::exit` leaves it to the system to release. + let mut lock_path = path.clone().into_os_string(); + lock_path.push(".lock"); + let lock_path = PathBuf::from(lock_path); + let _instance = match instance::lock(&lock_path) { + Ok(held) => held, + Err(e) => { + eprintln!("{}", e.line("inferproxy", &lock_path)); + process::exit(1); + } + }; match fs::remove_file(&path) { Ok(()) => {} Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} diff --git a/crates/inferproxy/tests/instance.rs b/crates/inferproxy/tests/instance.rs new file mode 100644 index 0000000..41e4e5b --- /dev/null +++ b/crates/inferproxy/tests/instance.rs @@ -0,0 +1,228 @@ +//! One `inferproxy` per socket (M5b spec, section 9a): the lock is `.lock`, beside the +//! `--listen` socket. A second `inferproxy` on the same socket is refused, and the first keeps +//! forwarding; once the first is killed, a new one removes the stale socket and forwards. Every +//! wait has a deadline, and every process is killed and every path removed by a guard (tip T44). +//! Do not edit. + +use std::io::{Read, Write}; +use std::net::TcpListener; +use std::os::unix::fs::PermissionsExt; +use std::os::unix::net::UnixStream; +use std::path::{Path, PathBuf}; +use std::process::{Child, Command, Stdio}; +use std::sync::atomic::{AtomicU32, Ordering}; +use std::thread; +use std::time::{Duration, Instant}; + +const POINTER: &str = "see docs/runbook.md#already-running"; + +static NEXT: AtomicU32 = AtomicU32::new(0); + +/// A temporary directory for the socket, removed when dropped, first with its owner's permissions +/// put back. +struct Dir(PathBuf); + +impl Dir { + fn new() -> Dir { + let n = NEXT.fetch_add(1, Ordering::SeqCst); + let dir = std::env::temp_dir().join(format!("ip-lock-{}-{n}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + Dir(dir) + } + + fn socket(&self) -> PathBuf { + self.0.join("infer.sock") + } +} + +impl Drop for Dir { + fn drop(&mut self) { + let _ = std::fs::set_permissions(&self.0, std::fs::Permissions::from_mode(0o700)); + let _ = std::fs::remove_dir_all(&self.0); + } +} + +/// An upstream that reads four bytes from each connection, answers `pong` and closes. +fn upstream() -> String { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + thread::spawn(move || { + for stream in listener.incoming() { + let Ok(mut stream) = stream else { break }; + thread::spawn(move || { + let mut ping = [0u8; 4]; + if stream.read_exact(&mut ping).is_ok() { + let _ = stream.write_all(b"pong"); + } + }); + } + }); + addr +} + +/// A running `inferproxy`, killed and waited for when dropped. Declared after the `Dir`, so it +/// drops first. +struct Proxy(Child); + +impl Drop for Proxy { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } +} + +impl Proxy { + fn running(&mut self) -> bool { + matches!(self.0.try_wait(), Ok(None)) + } +} + +fn command(socket: &Path, upstream: &str) -> Command { + let mut command = Command::new(env!("CARGO_BIN_EXE_inferproxy")); + command + .arg("--listen") + .arg(socket) + .args(["--upstream", upstream]) + .stdin(Stdio::null()) + .stdout(Stdio::null()); + command +} + +fn start(socket: &Path, upstream: &str) -> Proxy { + Proxy( + command(socket, upstream) + .stderr(Stdio::null()) + .spawn() + .unwrap(), + ) +} + +/// Runs `inferproxy` to its end, at most 10 s (then it is killed), and gives its exit code +/// (`None` if it had to be killed) and its stderr. +fn run_to_end(socket: &Path, upstream: &str) -> (Option, String) { + let mut child = Proxy( + command(socket, upstream) + .stderr(Stdio::piped()) + .spawn() + .unwrap(), + ); + let deadline = Instant::now() + Duration::from_secs(10); + let mut code = None; + while Instant::now() < deadline { + if let Ok(Some(status)) = child.0.try_wait() { + code = status.code(); + break; + } + thread::sleep(Duration::from_millis(20)); + } + let _ = child.0.kill(); + let _ = child.0.wait(); + let mut stderr = String::new(); + if let Some(mut pipe) = child.0.stderr.take() { + let _ = pipe.read_to_string(&mut stderr); + } + (code, stderr) +} + +/// True when a `ping` sent to `socket` comes back from the upstream as `pong` within 5 s. +fn forwards(socket: &Path) -> bool { + let Ok(mut stream) = UnixStream::connect(socket) else { + return false; + }; + let limit = Some(Duration::from_secs(5)); + if stream.set_read_timeout(limit).is_err() || stream.set_write_timeout(limit).is_err() { + return false; + } + if stream.write_all(b"ping").is_err() { + return false; + } + let mut answer = Vec::new(); + let _ = stream.read_to_end(&mut answer); + answer == b"pong" +} + +/// Waits up to 10 s for an `inferproxy` to forward on `socket`. +fn wait_until_it_forwards(socket: &Path) -> bool { + let deadline = Instant::now() + Duration::from_secs(10); + while Instant::now() < deadline { + if forwards(socket) { + return true; + } + thread::sleep(Duration::from_millis(50)); + } + false +} + +fn inode(path: &Path) -> u64 { + use std::os::unix::fs::MetadataExt; + std::fs::symlink_metadata(path) + .map(|m| m.ino()) + .unwrap_or(0) +} + +#[test] +fn a_second_inferproxy_on_the_same_socket_is_refused_and_the_first_keeps_forwarding() { + let dir = Dir::new(); + let socket = dir.socket(); + let upstream = upstream(); + let mut first = start(&socket, &upstream); + assert!(wait_until_it_forwards(&socket), "the first never forwarded"); + let before = inode(&socket); + + let (code, stderr) = run_to_end(&socket, &upstream); + let line = format!( + "inferproxy: another inferproxy is running (it holds {}.lock); {POINTER}", + socket.display() + ); + assert_eq!(code, Some(1), "the second inferproxy: {stderr}"); + assert!( + stderr.lines().any(|l| l == line), + "wanted the line\n{line}\nstderr: {stderr}" + ); + + assert!(first.running(), "the first inferproxy is still running"); + assert_eq!(inode(&socket), before, "the first's socket is still there"); + assert!(forwards(&socket), "the first still forwards"); +} + +#[test] +fn after_the_first_is_killed_a_new_inferproxy_removes_the_stale_socket_and_forwards() { + let dir = Dir::new(); + let socket = dir.socket(); + let upstream = upstream(); + let mut first = start(&socket, &upstream); + assert!(wait_until_it_forwards(&socket), "the first never forwarded"); + let _ = first.0.kill(); + let _ = first.0.wait(); + assert!(socket.exists(), "a killed inferproxy leaves its socket"); + assert!(!forwards(&socket), "nobody serves the stale socket"); + + let _next = start(&socket, &upstream); + assert!( + wait_until_it_forwards(&socket), + "the new one never forwarded" + ); + let lock = dir.0.join("infer.sock.lock"); + let mode = std::fs::metadata(&lock).map(|m| m.permissions().mode() & 0o777); + assert_eq!(mode.ok(), Some(0o600), "the lock file, beside the socket"); +} + +#[test] +fn a_socket_directory_that_cannot_be_written_stops_inferproxy_naming_the_error() { + let dir = Dir::new(); + let socket = dir.socket(); + std::fs::set_permissions(&dir.0, std::fs::Permissions::from_mode(0o500)).unwrap(); + + let (code, stderr) = run_to_end(&socket, &upstream()); + let line = format!( + "inferproxy: cannot take the lock {}.lock: Permission denied (os error 13); {POINTER}", + socket.display() + ); + assert_eq!(code, Some(1), "{stderr}"); + assert!( + stderr.lines().any(|l| l == line), + "wanted the line\n{line}\nstderr: {stderr}" + ); + assert!(!socket.exists(), "no socket"); +} diff --git a/crates/loopd/src/main.rs b/crates/loopd/src/main.rs index 2b32b51..4c22848 100644 --- a/crates/loopd/src/main.rs +++ b/crates/loopd/src/main.rs @@ -8,6 +8,7 @@ use std::process::ExitCode; use std::sync::Arc; use std::time::Duration; +use bxcore::instance; use loopd::broker_port::{BrokerPort, NoBroker, not_configured_line}; use loopd::channel::{self, Context}; use loopd::config::Config; @@ -73,6 +74,18 @@ fn run_serve(path: &Path) -> ExitCode { } }; + // One loopd per home (M5b spec, section 9a). The lock comes before the socket is touched and + // before the self-test, so a second copy neither takes the socket of the one that runs nor + // sends its slot a prompt. Held until the process ends. + let lock_path = cfg.paths.home.join("run/loopd.lock"); + let _instance = match instance::lock(&lock_path) { + Ok(held) => held, + Err(e) => { + eprintln!("{}", e.line("loopd", &lock_path)); + return ExitCode::from(1); + } + }; + let socket = cfg.channel_socket(); if socket.exists() && let Err(e) = std::fs::remove_file(&socket) diff --git a/crates/loopd/tests/instance.rs b/crates/loopd/tests/instance.rs new file mode 100644 index 0000000..73ebaf0 --- /dev/null +++ b/crates/loopd/tests/instance.rs @@ -0,0 +1,311 @@ +//! One `loopd` per home (M5b spec, section 9a): a second `loopd serve` against the same home is +//! refused before it touches the socket or the server, and the first keeps serving; once the first +//! is killed, a new one removes the stale socket and serves. Every wait has a deadline, and every +//! process is killed and every path removed by a guard (tip T44). Do not edit. + +mod support; + +use std::os::unix::fs::PermissionsExt; +use std::os::unix::net::UnixStream; +use std::path::{Path, PathBuf}; +use std::process::{Child, Command, Stdio}; +use std::time::{Duration, Instant}; + +use bxcore::{ + Envelope, ErrorCode, Message, PROTOCOL_VERSION, SessionId, Turn, WireError, read_frame, + write_frame, +}; +use support::{FakeServer, Home, Reply, tokenizer}; + +const POINTER: &str = "see docs/runbook.md#already-running"; + +fn config_file(home: &Home, server: &FakeServer) -> PathBuf { + let text = format!( + r#" +[infer] +socket = "{}" +model = "test-model" +[slots] +main = 0 +background = 1 +[expect] +template_sha256 = "f55f52930aa8bf44ab5cb85f99370fcc3c56e9a85640b812086d5330bce5d86b" +n_ctx = 131072 +slots = 2 +[limits] +poll_ms = 40 +liveness_ms = 500 +retry_backoff_ms = [10] +[paths] +home = "{}" +"#, + server.socket.display(), + home.dir.display() + ); + let path = home.dir.join("config.toml"); + std::fs::write(&path, text).unwrap(); + path +} + +/// A server that passes the self-test twice: once for the first `loopd`, once for the one started +/// after it is killed. +fn healthy_routes(server: &FakeServer) { + server.route("/props", vec![Reply::fixture("props")]); + server.respond("/tokenize", tokenizer); + server.route( + "/v1/chat/completions", + vec![ + Reply::fixture("tool_call"), + Reply::fixture("turn1"), + Reply::fixture("turn2"), + Reply::fixture("tool_call"), + Reply::fixture("turn1"), + Reply::fixture("turn2"), + ], + ); +} + +fn socket(home: &Home) -> PathBuf { + home.dir.join("run").join("loop").join("loop.sock") +} + +/// A running `loopd serve`, killed and waited for when dropped. Declared after the `Home` and the +/// `FakeServer`, so it drops first. +struct Daemon(Child); + +impl Drop for Daemon { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } +} + +impl Daemon { + fn running(&mut self) -> bool { + matches!(self.0.try_wait(), Ok(None)) + } +} + +/// `loopd serve` with its stderr in `/.err`, for the messages of a failing test. +fn start(home: &Home, config: &Path, name: &str) -> Daemon { + let err = std::fs::File::create(home.dir.join(format!("{name}.err"))).unwrap(); + let child = Command::new(env!("CARGO_BIN_EXE_loopd")) + .args(["serve", "--config"]) + .arg(config) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::from(err)) + .spawn() + .unwrap(); + Daemon(child) +} + +fn stderr_of(home: &Home, name: &str) -> String { + std::fs::read_to_string(home.dir.join(format!("{name}.err"))).unwrap_or_default() +} + +/// Runs `loopd serve` to its end, at most 20 s (then it is killed), and gives its exit code +/// (`None` if it had to be killed) and its stderr. +fn run_to_end(config: &Path) -> (Option, String) { + let mut child = Daemon( + Command::new(env!("CARGO_BIN_EXE_loopd")) + .args(["serve", "--config"]) + .arg(config) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(), + ); + let deadline = Instant::now() + Duration::from_secs(20); + let mut code = None; + while Instant::now() < deadline { + if let Ok(Some(status)) = child.0.try_wait() { + code = status.code(); + break; + } + std::thread::sleep(Duration::from_millis(20)); + } + let _ = child.0.kill(); + let _ = child.0.wait(); + let mut stderr = String::new(); + if let Some(mut pipe) = child.0.stderr.take() { + let _ = std::io::Read::read_to_string(&mut pipe, &mut stderr); + } + (code, stderr) +} + +/// True when `loopd` answers on `socket`: a turn that resumes a session that does not exist gets +/// the error frame for it. Any other answer, or none within 5 s, is false. +fn answers(socket: &Path) -> bool { + let Ok(mut stream) = UnixStream::connect(socket) else { + return false; + }; + let limit = Some(Duration::from_secs(5)); + if stream.set_read_timeout(limit).is_err() || stream.set_write_timeout(limit).is_err() { + return false; + } + let turn = Envelope { + v: PROTOCOL_VERSION, + id: 7, + r#final: true, + msg: Message::Turn(Turn { + session: SessionId::new("probe").unwrap(), + content: "are you there".to_string(), + resume: true, + }), + }; + if write_frame(&mut stream, &turn).is_err() { + return false; + } + matches!( + read_frame(&mut stream), + Ok(Envelope { + msg: Message::Error(WireError { + code: ErrorCode::NoSuchSession, + .. + }), + .. + }) + ) +} + +/// Waits up to 20 s for `loopd` to answer on `socket`. +fn wait_until_it_answers(socket: &Path) -> bool { + let deadline = Instant::now() + Duration::from_secs(20); + while Instant::now() < deadline { + if answers(socket) { + return true; + } + std::thread::sleep(Duration::from_millis(50)); + } + false +} + +fn inode(path: &Path) -> u64 { + use std::os::unix::fs::MetadataExt; + std::fs::symlink_metadata(path) + .map(|m| m.ino()) + .unwrap_or(0) +} + +#[test] +fn a_second_loopd_on_the_same_home_is_refused_and_the_first_keeps_serving() { + let home = Home::new(); + let server = FakeServer::start(); + healthy_routes(&server); + let config = config_file(&home, &server); + let socket = socket(&home); + let mut first = start(&home, &config, "first"); + assert!( + wait_until_it_answers(&socket), + "the first loopd never answered; stderr: {}", + stderr_of(&home, "first") + ); + let before = inode(&socket); + let completions = server.requests_to("/v1/chat/completions").len(); + let tokenized = server.requests_to("/tokenize").len(); + + let (code, stderr) = run_to_end(&config); + let lock = home.dir.join("run").join("loopd.lock"); + let line = format!( + "loopd: another loopd is running (it holds {}); {POINTER}", + lock.display() + ); + assert_eq!(code, Some(1), "the second loopd: {stderr}"); + assert!( + stderr.lines().any(|l| l == line), + "wanted the line\n{line}\nstderr: {stderr}" + ); + assert!( + !stderr.contains("selftest"), + "the second ran no self-test: {stderr}" + ); + assert_eq!( + ( + server.requests_to("/v1/chat/completions").len(), + server.requests_to("/tokenize").len() + ), + (completions, tokenized), + "the second sent the server nothing" + ); + + assert!(first.running(), "the first loopd is still running"); + assert_eq!(inode(&socket), before, "the first's socket is still there"); + assert!( + answers(&socket), + "the first loopd still answers; its stderr: {}", + stderr_of(&home, "first") + ); +} + +#[test] +fn after_the_first_is_killed_a_new_loopd_removes_the_stale_socket_and_serves() { + let home = Home::new(); + let server = FakeServer::start(); + healthy_routes(&server); + let config = config_file(&home, &server); + let socket = socket(&home); + let mut first = start(&home, &config, "first"); + assert!( + wait_until_it_answers(&socket), + "the first loopd never answered; stderr: {}", + stderr_of(&home, "first") + ); + let _ = first.0.kill(); + let _ = first.0.wait(); + assert!(socket.exists(), "a killed loopd leaves its socket"); + assert!(!answers(&socket), "nobody serves the stale socket"); + + let _next = start(&home, &config, "next"); + assert!( + wait_until_it_answers(&socket), + "the new loopd never answered; stderr: {}", + stderr_of(&home, "next") + ); + let lock = home.dir.join("run").join("loopd.lock"); + let mode = std::fs::metadata(&lock).map(|m| m.permissions().mode() & 0o777); + assert_eq!(mode.ok(), Some(0o600), "the lock file, mode 0600"); + assert_eq!( + server.requests_to("/v1/chat/completions").len(), + 6, + "each of the two ran the self-test" + ); +} + +/// Gives `/run` back its owner's permissions when dropped, before the `Home` is removed. +struct Restore(PathBuf); + +impl Drop for Restore { + fn drop(&mut self) { + let _ = std::fs::set_permissions(&self.0, std::fs::Permissions::from_mode(0o700)); + } +} + +#[test] +fn a_run_directory_that_cannot_be_written_stops_loopd_naming_the_error() { + let home = Home::new(); + let server = FakeServer::start(); + healthy_routes(&server); + let config = config_file(&home, &server); + let run = home.dir.join("run"); + std::fs::create_dir(&run).unwrap(); + let _restore = Restore(run.clone()); + std::fs::set_permissions(&run, std::fs::Permissions::from_mode(0o500)).unwrap(); + + let (code, stderr) = run_to_end(&config); + let line = format!( + "loopd: cannot take the lock {}: Permission denied (os error 13); {POINTER}", + run.join("loopd.lock").display() + ); + assert_eq!(code, Some(1), "{stderr}"); + assert!( + stderr.lines().any(|l| l == line), + "wanted the line\n{line}\nstderr: {stderr}" + ); + assert!( + server.requests().is_empty(), + "loopd sent the server nothing" + ); + assert!(!socket(&home).exists(), "no socket"); +} diff --git a/docs/implementer-log.md b/docs/implementer-log.md index ca3e0ca..8ecf045 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -6,6 +6,7 @@ reviewer adds findings under "Reviews" once per milestone. | Task | Date | Status | Gate runs | First gate | Deviations | Notes | Model | |---|---|---|---|---|---|---|---| +| M5b/12-instance-lock | 2026-09-27 | done | 1 | pass | none | Wrote the lock that keeps a daemon to one instance (M5b spec, section 9a) and made `loopd` and `inferproxy` take it. Step 2 showed exactly the named failures: the bxcore test did not compile (one E0432 `unresolved import bxcore::instance`); the loopd test failed its 3 tests (`a_second_loopd_on_the_same_home_is_refused...` left None right Some(1) because the second copy started and served, `after_the_first_is_killed_a_new_loopd_removes_the_stale_socket_and_serves` the lock file mode None vs Some(384), `a_run_directory_that_cannot_be_written_stops_loopd_naming_the_error` ran its self-test and failed later on the socket directory); the inferproxy test failed the same three shapes (`the second inferproxy...` left None right Some(1), `the lock file, beside the socket` None vs Some(384), `wanted the line` for the read-only directory). Copied the skeleton `crates/bxcore/src/instance.rs` (fill 2 functions) and the tests `bxcore/tests/instance.rs`, `loopd/tests/instance.rs`, `inferproxy/tests/instance.rs` from `docs/plans/M5b/files/`; appended `runbook-already-running.md` to `docs/runbook.md`. Made the edits in order: L1 the doc comment naming `instance` in lib.rs; L2 `pub mod instance;` after `pub mod ids;`. Filled `LockError::line` (a `match` on `self`, each arm `format!` with the path shown via `path.display()` and `{RUNBOOK}` inline) then `lock` (make the parent dir 0700 when `path.parent()` is `Some`, open the file 0600 with `create(true).truncate(false)`, `try_lock()` mapping `WouldBlock` to `Held` and other errors to `Io`). `cargo check -p bxcore` clean; `instance` 8 passed. M1 the `use bxcore::instance;` import in loopd main.rs; M2 the lock in `run_serve` after the config loads and before the socket is removed and the self-test, bound to `_instance`. `cargo check -p loopd` clean; loopd `instance` 3 passed. P1 the `use bxcore::instance;` import in inferproxy main.rs; P2 the lock beside the `--listen` socket (`.lock`) after the args are parsed and before the stale socket is removed, bound to `_instance`. `cargo check -p inferproxy` clean; inferproxy `instance` 3 passed. `cargo fmt --all`; `make gate` printed `gate: ok` with 1113 tests (1112 in `cargo test --workspace`, plus the gate's separate loopd `end_to_end` ignored test). `grep -rn 'runbook.md#already-running' crates/*/src` prints exactly one line, the `RUNBOOK` constant; `grep -c '^## already-running$' docs/runbook.md` printed 1; `git diff --stat -- '*Cargo.toml'` printed nothing. No new dependency, so `Cargo.lock` is unchanged. | ornith-1.5-35b-a3b | | M5b/11-bxctl-sanitize-check | 2026-09-27 | done | 1 | pass | none | Copied the skeleton `crates/bxctl/src/sanitize.rs` and the tests `crates/bxctl/tests/sanitize_check.rs`, `crates/bxctl/tests/sanitize_report.rs` from `docs/plans/M5b/files/`. Step 2 showed exactly the named failures: `sanitize_report` failed to compile (one E0432 `unresolved import bxctl::sanitize`, two E0599 `no variant named SanitizeCheck`) and `sanitize_check` failed its 8 tests (each run printed the usage text and exited 2 where 0 or 1 was wanted). Made the edits in order: L1 `pub mod sanitize;` between `memory` and `verify` in lib.rs; C1 appended the `bxctl sanitize check` USAGE block (the memory line lost its closing `";`); C2 the `SanitizeCheck { text: Option }` variant after `Memory`; C3 the `"sanitize" => crate::sanitize::parse_sanitize(rest)` arm in `parse`; M1 `Read` added to the `use std::io` list; M2 the `Command::SanitizeCheck { text }` arm in `main`; M3 the `cmd_sanitize_check` function (argument or stdin read to a String, `bxctl::sanitize::run`, Ok(false)->0 / Ok(true)->1 / Err->2). Filled the five functions in the given order, each from its comment and `cargo check` after: `parse_sanitize` (one `match` with slice patterns and `check == "check"` guards, `[check]` -> `text: None`, `[check, text]` -> `Some(text.clone())`, else `UsageError`); `code_point` (`text.get(at..).and_then(|rest| rest.chars().next())`, `format!("U+{:04X}", u32::from(c))` or `"…"`); `token` (`TAGS.iter().find` the first tag the rest starts with, `format!("token {}", tag.id)` or `"…"`); `prefix` (`PREFIXES.iter().find` the first the rest starts with, `.copied().unwrap_or_default()`); `tag_count` (`TAGS.iter().map(|tag| text.matches(tag.text).count()).sum()`). `cargo fmt --all`; `cargo check -p bxctl` clean; `sanitize_report` 10 passed, `sanitize_check` 8 passed, every bxctl suite passes. `git diff --stat -- crates/bxcore '*Cargo.toml'` prints nothing. `make gate` printed `gate: ok` with 1099 tests (1081 before this task, plus 8 in `sanitize_check.rs` and 10 in `sanitize_report.rs`); `cargo test --workspace` reports 1098 passed plus the gate's separate loopd `end_to_end` ignored test = 1099. No new dependency, so `Cargo.lock` is unchanged. | ornith-1.5-35b-a3b | | M5b/10-gatewayd-links | 2026-09-27 | done | 1 | pass | none | Copied the two skeletons `crates/gatewayd/src/links/mod.rs` (fill 7 functions) and `crates/gatewayd/src/links/inline.rs` (fill 9) and the tests `crates/gatewayd/tests/links.rs`, `links_marked.rs`, `deliver_links.rs` from `docs/plans/M5b/files/`. Filled the nine inline functions in the given order from each comment: `autolink_len`, `definition`, `url_len` (walk `rest.get(..end)` for the trailing run), `construct_len` (reference path was off by one — `reference_end`/`paren_end` return the closing-char index, so the label path needs `label_end + 2` to go through the `]`, matching the paren path), `bracket_end` (count `[`/`]` with backslash skip and code-span run jump), and the five that follow. Filled the seven mod functions (`wrap`, `text`, `code` and the rest of the `Out` type). Fixed the one filled helper the task flagged (`Out::wrap`): after a `\r\n` the `\n` was written a second time, so consume it in `wrap`'s BREAKS arm. `cargo fmt --all`; `cargo check -p gatewayd` clean; every gatewayd suite passes (`links` 7, `links_marked` 7, `deliver_links` 9). First `make gate` run failed on six clippy warnings denied by `-D warnings`, all inside `todo!()` bodies (allowed to edit): a collapsible `if { if let }` in `construct_len`, a `match { Some => c, None => return None }` replaced with `?` in `bracket_end`, a `while let` loop in `url_len`, a manual assign in `bracket_end`, and two needless borrows `self.code(&construct...)` in `Out::wrap`; fixed each. `make gate` printed `gate: ok` with 1081 tests (1058 before this task, plus 7 in `links.rs`, 7 in `links_marked.rs` and 9 in `deliver_links.rs`). `git diff HEAD~1 --stat` lists only the allowed files. No new dependency, so `Cargo.lock` is unchanged. | ornith-1.5-35b-a3b | | M5b/09-turn-done-untrusted | 2026-09-26 | done | 1 | pass | none | Copied the eight given files from `docs/plans/M5b/files/`: `bxcore/tests/fixtures/wire/turn_done_untrusted.json` (new) and `loopd/tests/turn_done_untrusted.rs` (new), new versions of `bxcore/tests/fixtures/wire/turn_done.json`, `bxcore/tests/turn_wire.rs`, `brokerd/tests/broker.rs`, `bxctl/tests/chat.rs` and `bxctl/tests/support/mod.rs`, and `gatewayd/tests/support/fake_loop.rs` (did not open them). Step 2 showed exactly the named failures: `strict::envelopes_reject_unknown_keys_at_every_depth` failing on `turn_done.json: fixture must decode`, and 13 non-compiling test targets (`turn_wire` in bxcore, `broker` in brokerd, `turn_done_untrusted` in loopd, `admin`/`chat`/`chat_approvals` in bxctl, `deliver`/`deliver_notice`/`serve`/`serve_answers`/`serve_approvals`/`serve_redirect`/`serve_restart` in gatewayd) with E0560 and one E0609. Made the three edits exactly as the task gives: W1 `TurnDone` gains `untrusted: bool` as the last field with the no-default comment in `bxcore/src/wire.rs`; S1 `Session::untrusted` scanning `self.records` for a `LogRecord::ToolResult { untrusted: true, .. }` placed before `last_usage` in `session.rs`; C1 `session.untrusted()` put in the `TurnDone` in `channel.rs` `handle` after `usage`. `cargo fmt --all`; `cargo check -p loopd` clean; the seven `turn_wire` tests and six `turn_done_untrusted` tests pass, and every suite in `bxcore`, `loopd`, `brokerd`, `bxctl` and `gatewayd` passes. `git diff --stat -- crates/gatewayd/src crates/bxctl/src crates/brokerd/src` prints nothing, and the only `serde(default)` in `wire.rs` is line 109, the `Denied` detail added by task 01. `make gate` printed `gate: ok` with 1058 tests (1050 before this task, plus the 8 new: two in `turn_wire.rs`, six in `turn_done_untrusted.rs`). No new dependency, so `Cargo.lock` is unchanged. | ornith-1.5-35b-a3b | diff --git a/docs/runbook.md b/docs/runbook.md index 04498f4..c69c58d 100644 --- a/docs/runbook.md +++ b/docs/runbook.md @@ -782,3 +782,51 @@ For an `open!` tag the content is `<%s>`, for a `close!` tag ``. The first change: record it in `docs/decisions.md` and do not run `loopd` until it is settled. **Check.** `loopd selftest --config ` prints `selftest: ok`. + +## already-running + +**What you see.** A daemon exits 1 at start, before it touches its sockets, with one of these +lines: + +- `: another is running (it holds ); see docs/runbook.md#already-running` +- `: cannot take the lock : ; see docs/runbook.md#already-running` + +`` is `loopd`, `brokerd`, `memoryd`, `gatewayd` or `inferproxy`. The lock file is +`$BOXMAKER_HOME/run/.lock`; for `inferproxy` it is its `--listen` socket's path with +`.lock` added (`infer.sock.lock`, beside `infer.sock`). + +**Why.** Every daemon removes a socket it finds at its path before it binds, so that a socket +left by a daemon that was killed never blocks the next start. A second copy started by mistake +would therefore take the sockets of the one that runs, which would keep running, unreachable, +and nothing would say so. So each daemon first takes an exclusive lock on its lock file, and +holds it for as long as it runs (M5b spec, section 9a). The system releases the lock when the +process ends, however it ends, so a killed daemon never leaves a lock behind. The file stays in +place and holds nothing. + +**Confirm.** For `another is running`, find the copy that runs, with its command line +(the `--config` path says which home it serves): + +```sh +pgrep -a -x +ls -l "$BOXMAKER_HOME/run/.lock" +``` + +For `cannot take the lock`, the error says why: `Permission denied` means the daemon's user +cannot create or open the file, or create `run/` (`ls -ld "$BOXMAKER_HOME" "$BOXMAKER_HOME/run"` +and `ls -l` on the lock file show who owns them); `Not a directory` means a path above it is a +file; `Read-only file system` means what it says. + +**Fix.** Two copies of a role against the same home are always a mistake: use the one that runs. +To restart it, stop it (`pkill -x `, or Ctrl-C in its terminal), wait until `pgrep -x +` prints nothing, and start it again. Two copies with different homes are fine (each has +its own lock), as long as their configs do not name the same socket path. For a lock that cannot +be taken, give the daemon's user the directory (`chown`), or point its home somewhere it can +write. + +Never delete a lock file while its daemon runs: the lock is held on the open file, so a new copy +would create a new file, lock that, and take the sockets. Deleting it while the daemon is stopped +does no harm, and does not help either. + +**Check.** `pgrep -x ` prints one process id, and the daemon prints its start line +(`loopd: serving on …`, `inferproxy: -> `, `brokerd: serving tools on …`, +`memoryd: serving`, `gatewayd: connected to …`).