diff --git a/crates/engine/src/tools/standin_tools.rs b/crates/engine/src/tools/standin_tools.rs index 08635f28..da9d44a8 100644 --- a/crates/engine/src/tools/standin_tools.rs +++ b/crates/engine/src/tools/standin_tools.rs @@ -410,6 +410,23 @@ fn standin_path(p: &str) -> Result { std::fs::canonicalize(&path).map_err(|e| ToolError::io(path.display(), e)) } +/// How long a rebuilt stand-in waits for its predecessor's port. +const TAKEOVER_WAIT: std::time::Duration = std::time::Duration::from_secs(2); + +/// Listen on `port`, trying a busy one again for up to `wait`: the port a replaced stand-in just +/// gave up can stay busy a moment, or be lent out briefly as another connection's local port. +fn serve_on(port: u16, answerer: Arc, wait: std::time::Duration) -> std::io::Result { + let until = std::time::Instant::now() + wait; + loop { + match standin_server::serve(port, Limits::default(), answerer.clone()) { + Err(e) if e.kind() == std::io::ErrorKind::AddrInUse && std::time::Instant::now() < until => { + std::thread::sleep(std::time::Duration::from_millis(50)); + } + r => return r, + } + } +} + fn web_standin_start(ctx: &mut Ctx<'_>, params: Value) -> Result { let p: StartParams = parse_params(params)?; let file = standin_path(&p.path)?; @@ -434,15 +451,15 @@ fn web_standin_start(ctx: &mut Ctx<'_>, params: Value) -> Result = replay.clone(); // The file was rebuilt since it was loaded: answer from the new one, at the same address. - let port = match stale.as_ref().and_then(|id| all.running.lock().unwrap_or_else(PoisonError::into_inner).remove(id)) { + let (port, taken_over) = match stale.as_ref().and_then(|id| all.running.lock().unwrap_or_else(PoisonError::into_inner).remove(id)) { Some(old) => { let port = old.server.port; drop(old); - port + (port, true) } - None => p.port, + None => (p.port, false), }; - let server = standin_server::serve(port, Limits::default(), answerer) + let server = serve_on(port, answerer, if taken_over { TAKEOVER_WAIT } else { std::time::Duration::ZERO }) .map_err(|e| ToolError::io(format!("listening on port {port}"), e).with_remediation("Pick another port, or leave port out to use a free one."))?; let id = { let mut next = all.next.lock().unwrap_or_else(PoisonError::into_inner); diff --git a/crates/engine/src/tools/standin_tools/tests.rs b/crates/engine/src/tools/standin_tools/tests.rs index 89a1f8d0..c56292b5 100644 --- a/crates/engine/src/tools/standin_tools/tests.rs +++ b/crates/engine/src/tools/standin_tools/tests.rs @@ -380,3 +380,21 @@ fn effects_are_honest() { assert!(build.effects.contains(Effects::WRITES_FILESYSTEM) && build.effects.contains(Effects::CARRIES_TARGET_CONTENT)); assert!(r.get("web_standin_show").unwrap().effects.contains(Effects::CARRIES_TARGET_CONTENT)); } + +/// A rebuilt stand-in takes over its predecessor's port even while the system holds it a moment +/// longer; a new one doesn't wait (seen in CI as "Address already in use"). +#[test] +fn a_takeover_waits_for_its_port_and_a_new_start_does_not() { + let held = std::net::TcpListener::bind(("127.0.0.1", 0)).unwrap(); + let port = held.local_addr().unwrap().port(); + let answerer: Arc = Arc::new(|_: &Request| Response::json(200, &json!({}))); + let err = serve_on(port, answerer.clone(), Duration::ZERO).map(|_| ()).unwrap_err(); + assert_eq!(err.kind(), std::io::ErrorKind::AddrInUse); + let release = std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(300)); + drop(held); + }); + let server = serve_on(port, answerer, TAKEOVER_WAIT).unwrap(); + assert_eq!(server.port, port); + release.join().unwrap(); +}