diff --git a/src/engine/hot_loop/hmds.rs b/src/engine/hot_loop/hmds.rs index e5a24e39..6f823a59 100644 --- a/src/engine/hot_loop/hmds.rs +++ b/src/engine/hot_loop/hmds.rs @@ -75,6 +75,18 @@ impl HmdsState { } } + /// Give up on the transport. + /// + /// The flag and the connection move together. Setting the flag alone left + /// the dead socket in place, and the reconnect scheduler returns early + /// while a connection is present — so the transport was stuck for the life + /// of the process, on both the liveness timeout and the ordinary + /// receive-error path (ibx#367). + pub(crate) fn disconnect(&mut self, hmds_conn: &mut Option) { + self.disconnected = true; + *hmds_conn = None; + } + pub(crate) fn poll( &mut self, hmds_conn: &mut Option, @@ -91,7 +103,7 @@ impl HmdsState { Ok(0) => {} Err(e) => { log::error!("HMDS connection lost: {}", e); - self.disconnected = true; + self.disconnect(hmds_conn); return; } Ok(n) => { diff --git a/src/engine/hot_loop/mod.rs b/src/engine/hot_loop/mod.rs index 2b33d09c..f6ce4d7d 100644 --- a/src/engine/hot_loop/mod.rs +++ b/src/engine/hot_loop/mod.rs @@ -740,7 +740,7 @@ impl HotLoop { if warmed_up && since_recv > LIVENESS_TEST_SECS { if since_recv > LIVENESS_DEAD_SECS { log::error!("HMDS liveness timeout ({}s silent) — connection lost", since_recv); - self.hmds.disconnected = true; + self.hmds.disconnect(&mut self.hmds_conn); } else if self.hb.pending_hmds_test.is_none() { let test_id = self.hb.next_test_id(); let _ = conn.send_fix(&[ @@ -1039,6 +1039,14 @@ impl HotLoop { self.hmds.disconnected = false; self.hb.last_hmds_recv = Instant::now(); self.hb.last_hmds_sent = Instant::now(); + // The probe that went unanswered belonged to the dead session. + // Leaving it pending suppressed the next one — the liveness + // check only sends a TestRequest when none is outstanding — so + // a fresh connection went silent-to-dead without ever being + // probed. The warm-up starts here too, or the new session is + // already past it (ibx#367). + self.hb.pending_hmds_test = None; + self.hb.hmds_up_since = Instant::now(); self.hmds_reconnect_attempt = 0; self.hmds_next_attempt_at = None; self.pending_hmds_reconnect = None; @@ -1463,6 +1471,87 @@ fn extract_text_tag(msg: &[u8], tag: u32) -> Option { #[cfg(test)] mod tests { + + /// A reconnected transport must not inherit the dead session's outstanding + /// probe. The liveness check only sends a TestRequest when none is pending, + /// so a stale one suppressed every probe on the new connection and it went + /// from silent to declared dead without being asked anything. The warm-up + /// has to restart with it, or the new session is already past it. + #[test] + fn an_hmds_reconnect_does_not_inherit_the_dead_session_probe() { + let mut hl = HotLoop::new(Arc::new(SharedState::new()), None, None); + hl.hb.pending_hmds_test = Some(("t-1".to_string(), Instant::now())); + hl.hb.hmds_up_since = Instant::now() - Duration::from_secs(600); + hl.hmds.disconnected = true; + + // A reconnect that has just succeeded. + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let sock = std::net::TcpStream::connect(listener.local_addr().unwrap()).unwrap(); + let (_peer, _) = listener.accept().unwrap(); + let (tx, rx) = crossbeam_channel::bounded(1); + tx.send(Ok(crate::protocol::connection::Connection::new_raw(sock).unwrap())).unwrap(); + hl.pending_hmds_reconnect = Some(rx); + + hl.poll_hmds_reconnect(); + + assert!(hl.hmds_conn.is_some(), "the connection is installed"); + assert!(!hl.hmds.disconnected, "and the transport is live again"); + assert!( + hl.hb.pending_hmds_test.is_none(), + "the dead session's probe is dropped, or it suppresses every probe on the new one", + ); + assert!( + hl.hb.hmds_up_since.elapsed() < Duration::from_secs(LIVENESS_WARMUP_SECS), + "and the new session starts inside its warm-up", + ); + } + + /// ibx#367: the liveness timeout set the disconnected flag and left the + /// dead socket in place, and the reconnect scheduler returns early while a + /// connection is present — so the first HMDS liveness timeout was permanent + /// for the life of the process. Historical requests issued afterwards + /// targeted the abandoned socket and never completed. + #[test] + fn an_hmds_disconnect_lets_its_reconnect_run() { + let mut hl = HotLoop::new(Arc::new(SharedState::new()), None, None); + hl.set_reconnect_auth(crate::gateway::ReconnectAuth { + host: "gw.example".into(), + username: "u".into(), + password: zeroize::Zeroizing::new(String::new()), + paper: true, + session_key: Default::default(), + session_token: Default::default(), + server_session_id: String::new(), + hw_info: String::new(), + encoded: String::new(), + hmds_host: "hmds.example".into(), + hmds_farm: "hfarm".into(), + }); + + // A live transport, so releasing it is observable. + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let sock = std::net::TcpStream::connect(listener.local_addr().unwrap()).unwrap(); + let (_peer, _) = listener.accept().unwrap(); + hl.hmds_conn = Some(crate::protocol::connection::Connection::new_raw(sock).unwrap()); + hl.hmds.disconnected = false; + assert!(hl.hmds_next_attempt_at.is_none(), "nothing scheduled while it is up"); + + // Driven through the receive path rather than by calling the helper: + // the peer is gone, so `try_recv` errors and the poll gives up on the + // transport. This is the ordinary way HMDS dies. + drop(_peer); + hl.hmds.poll(&mut hl.hmds_conn, &SharedState::new(), &None, &mut hl.hb); + + assert!(hl.hmds.disconnected, "the transport is marked dead"); + assert!(hl.hmds_conn.is_none(), "and the socket is released with it"); + + // The scheduler now gets past its early return and arms the first attempt. + hl.maybe_spawn_hmds_reconnect(); + assert!( + hl.hmds_next_attempt_at.is_some(), + "an HMDS reconnect is scheduled rather than skipped forever", + ); + } use super::*; use std::sync::Arc; use crate::bridge::{Event, SharedState};