From c4d73eda7e6de59acca9b35291f2d16a27e8199a Mon Sep 17 00:00:00 2001 From: kevin9327 <5299031+kevin9327@users.noreply.github.com> Date: Wed, 9 Sep 2026 07:22:32 +0900 Subject: [PATCH] Refuse a start when the port is held on the other loopback The readiness check accepts an answer at 127.0.0.1 or at [::1], because a process binds whichever loopback its runtime resolved `localhost` to. The guard in front of it asked only 127.0.0.1, so a port held on ::1 alone was reported free and the start went ahead into it -- the one outcome that guard exists to prevent. Both addresses are asked now. --- CHANGELOG.md | 11 ++++++++ desktop/src-tauri/src/stack.rs | 46 +++++++++++++++++++++++++++++----- 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dba57a687..4eeb04e76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,17 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### The desktop app notices a busy port whichever loopback holds it + +The shell refuses to start when something already holds port 3001 or 3010, because otherwise the +readiness check that follows is answered by a server it never started: everything reads green and +none of it is yours. That readiness check asks both loopback addresses on purpose, since a process +binds whichever one its runtime resolved `localhost` to — Node picks `::1`, Bun picks `127.0.0.1` — +so an answer at either counts. The refusal in front of it asked only `127.0.0.1`, which meant a port +held on `::1` alone was reported free and the start went ahead into it. Both addresses are asked +now, so the two agree on what "in use" means and the person is told which port is taken and what +OpenBot wanted it for. + ### One command to stop what `start.sh` started Stopping the local stack meant four commands read off the end of a successful start, and the one diff --git a/desktop/src-tauri/src/stack.rs b/desktop/src-tauri/src/stack.rs index 066ea6714..b5a5041b2 100644 --- a/desktop/src-tauri/src/stack.rs +++ b/desktop/src-tauri/src/stack.rs @@ -366,12 +366,7 @@ pub fn services_that_exited(engine: &Address, root: &Path) -> Vec<(String, Strin /// outside. pub fn port_already_taken(ports: &[(&'static str, u16)]) -> Option { for (name, port) in ports { - if std::net::TcpStream::connect_timeout( - &std::net::SocketAddr::from(([127, 0, 0, 1], *port)), - std::time::Duration::from_millis(300), - ) - .is_ok() - { + if held_on_a_loopback(*port) { return Some(format!( "Something is already listening on port {port}, which OpenBot uses for the {name}. \ Stop it, or change the port, and start again." @@ -381,6 +376,28 @@ pub fn port_already_taken(ports: &[(&'static str, u16)]) -> Option { None } +/// Whether anything is listening on `port`, at either address a loopback service can be bound to. +/// +/// Both, for the reason `LOOPBACKS` below already records: a process binds whichever loopback its +/// runtime resolved `localhost` to, and binding one and not the other is normal rather than broken. +/// Asking only `127.0.0.1` therefore called a port free that the readiness check would then accept a +/// stranger's answer on, which is the exact outcome the check above exists to prevent. A refused +/// connection comes back at once on both addresses, so the second question costs nothing on a port +/// nobody holds. +fn held_on_a_loopback(port: u16) -> bool { + const LOOPBACK_ADDRESSES: [std::net::IpAddr; 2] = [ + std::net::IpAddr::V4(std::net::Ipv4Addr::LOCALHOST), + std::net::IpAddr::V6(std::net::Ipv6Addr::LOCALHOST), + ]; + LOOPBACK_ADDRESSES.iter().any(|address| { + std::net::TcpStream::connect_timeout( + &std::net::SocketAddr::new(*address, port), + std::time::Duration::from_millis(300), + ) + .is_ok() + }) +} + /// Wait until the API answers, or say why it never did. /// /// Spawning is not starting. Each of these three can exit in the first second for a reason that has @@ -654,6 +671,23 @@ mod tests { ); } + #[test] + fn a_port_held_on_the_other_loopback_is_still_held() { + // The readiness check accepts an answer at either loopback, for the reason `LOOPBACKS` + // gives: a process binds whichever one its runtime resolved `localhost` to. A guard that + // asks only 127.0.0.1 therefore calls a port free that `answering_at` would then accept a + // stranger's answer on, which is the one outcome this check exists to prevent. + let listener = std::net::TcpListener::bind("[::1]:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + + let problem = port_already_taken(&[("app", port)]).expect("a held port is a problem"); + assert!(problem.contains(&port.to_string()), "{problem}"); + assert!( + problem.contains("app"), + "must say what it is for: {problem}" + ); + } + #[test] fn migrate_is_not_raised_as_a_service() { // Raised alongside the others it exits immediately, and Compose reports a service that will