From 171fc0f6f445936af64271ab99abe32049094a0e Mon Sep 17 00:00:00 2001 From: Ramon Bartl Date: Mon, 14 Sep 2026 16:51:35 +0200 Subject: [PATCH] Give the backend calls a deadline http::get and http::post connected with TcpStream::connect and read with read_to_string, neither of which has a timeout. A backend that accepts the connection and then never answers blocks the caller forever. The caller that matters is the tray ticker. self_heal_from_backend runs inside spawn_ticker, which is tauri::async_runtime::spawn, so a hung read pins a Tokio worker and the menu-bar pill stops updating until the app is restarted. That is the exact outcome the surrounding code already works to prevent. Its catch_unwind comment says a single bad frame would otherwise freeze the menu-bar pill indefinitely until the user restarts the app. The guard covers panics; a hang arrived through the other door, and for a local HTTP call it is the more likely of the two. toggle_timer makes the same call on its own thread, so there it leaks a thread rather than freezing the tray. Five seconds on connect, read and write. The crate had no tests; it has two. connect_to takes the address so the test drives the real function rather than a copy of it: with the timeouts removed, the silent-backend test fails after 30 seconds instead of passing in 5. --- desktop/src-tauri/src/http.rs | 115 ++++++++++++++++++++++++++++++++-- 1 file changed, 111 insertions(+), 4 deletions(-) diff --git a/desktop/src-tauri/src/http.rs b/desktop/src-tauri/src/http.rs index db2156c7..5b4b11d2 100644 --- a/desktop/src-tauri/src/http.rs +++ b/desktop/src-tauri/src/http.rs @@ -10,6 +10,43 @@ use std::time::Duration; use crate::BACKEND_ADDR; +/// How long to wait on a connect, a write, or a read. +/// +/// Every one of these is needed. Without a read timeout a +/// backend that accepts the connection and then never +/// answers blocks the caller forever, and the caller that +/// matters is the tray ticker: it runs on a Tokio worker, +/// so a hung read pins that worker and the menu-bar pill +/// stops updating until the app is restarted. The ticker +/// is already wrapped in catch_unwind against exactly that +/// outcome from a panic; a hang got there by the other +/// door. +/// +/// Five seconds is generous for a local FastAPI answering +/// /api/clocks/active and short enough that a stuck one +/// costs a single tick. +const IO_TIMEOUT: Duration = Duration::from_secs(5); + +/// Connect to ``addr`` with every timeout applied. +/// +/// Takes the address rather than reading BACKEND_ADDR so a +/// test can point the real function at a stub listener. +/// Every caller passes BACKEND_ADDR. +fn connect_to(addr: &str) -> Result { + let parsed = addr.parse().map_err(|e| format!("{e}"))?; + let s = TcpStream::connect_timeout(&parsed, IO_TIMEOUT) + .map_err(|e| format!("{e}"))?; + s.set_read_timeout(Some(IO_TIMEOUT)) + .map_err(|e| format!("{e}"))?; + s.set_write_timeout(Some(IO_TIMEOUT)) + .map_err(|e| format!("{e}"))?; + Ok(s) +} + +fn connect() -> Result { + connect_to(BACKEND_ADDR) +} + /// Check whether the backend port is accepting /// connections. pub fn is_port_open() -> bool { @@ -22,8 +59,7 @@ pub fn is_port_open() -> bool { /// HTTP GET to the local backend. pub fn get(path: &str) -> Result { - let mut s = TcpStream::connect(BACKEND_ADDR) - .map_err(|e| format!("{e}"))?; + let mut s = connect()?; let req = format!( "GET {path} HTTP/1.1\r\n\ Host: 127.0.0.1\r\n\ @@ -38,8 +74,7 @@ pub fn get(path: &str) -> Result { pub fn post( path: &str, body: &str, ) -> Result { - let mut s = TcpStream::connect(BACKEND_ADDR) - .map_err(|e| format!("{e}"))?; + let mut s = connect()?; let req = format!( "POST {path} HTTP/1.1\r\n\ Host: 127.0.0.1\r\n\ @@ -65,3 +100,75 @@ fn read_body( None => Ok(buf), } } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Read as _; + use std::net::TcpListener; + use std::time::Instant; + + /// A listener that accepts and then says nothing, + /// which is what a wedged backend looks like from + /// here: the connect succeeds, the write succeeds, + /// and the read never finishes. + fn silent_listener() -> (TcpListener, String) { + let l = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = l.local_addr().unwrap().to_string(); + (l, addr) + } + + /// Goes through connect_to, so removing a timeout + /// there breaks these rather than leaving them green. + fn request(addr: &str) -> Result { + let mut s = connect_to(addr)?; + s.write_all(b"GET / HTTP/1.1\r\n\r\n") + .map_err(|e| format!("{e}"))?; + read_body(&mut s) + } + + #[test] + fn a_silent_backend_does_not_block_forever() { + let (listener, addr) = silent_listener(); + let accepted = std::thread::spawn(move || { + // Hold the connection open without replying. + if let Ok((mut sock, _)) = listener.accept() { + let mut sink = [0u8; 64]; + let _ = sock.read(&mut sink); + std::thread::sleep( + Duration::from_secs(30), + ); + } + }); + + let started = Instant::now(); + let result = request(&addr); + let waited = started.elapsed(); + + assert!( + result.is_err(), + "a silent backend should time out, got {result:?}", + ); + assert!( + waited < IO_TIMEOUT + Duration::from_secs(2), + "waited {waited:?}, expected about {IO_TIMEOUT:?}", + ); + drop(accepted); + } + + #[test] + fn a_normal_response_is_parsed() { + let (listener, addr) = silent_listener(); + std::thread::spawn(move || { + if let Ok((mut sock, _)) = listener.accept() { + let mut sink = [0u8; 256]; + let _ = sock.read(&mut sink); + let _ = sock.write_all( + b"HTTP/1.1 200 OK\r\n\r\n{\"ok\":true}", + ); + } + }); + let body = request(&addr).unwrap(); + assert_eq!(body, "{\"ok\":true}"); + } +}