From 77f36b1e98ee663172f271214b1b5123eb03a2bc Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Thu, 13 Aug 2026 12:21:14 +0200 Subject: [PATCH] fix(actor): leave Disconnected state behind after every disconnect A failed outbound dial left PeerActor in Connecting, so check_connection_status re-fired the 20-second timeout error on every 100ms tick until the actor was stopped. ServerActor had the same bug: its disconnect paths only cleared state when already Connected. Reset connection_state to Disconnected in all four disconnect paths. Fixes #31 --- soulseek-rs-lib/src/actor/peer_actor.rs | 39 ++++++++++++++++++++--- soulseek-rs-lib/src/actor/server_actor.rs | 30 ++++++++++++++--- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/soulseek-rs-lib/src/actor/peer_actor.rs b/soulseek-rs-lib/src/actor/peer_actor.rs index ad8710f..17330f4 100644 --- a/soulseek-rs-lib/src/actor/peer_actor.rs +++ b/soulseek-rs-lib/src/actor/peer_actor.rs @@ -658,6 +658,7 @@ impl PeerActor { debug!("[peer:{}] disconnect: {}", username, error); self.stream.take(); + self.connection_state = ConnectionState::Disconnected; if self.disconnect_reported { return; @@ -687,6 +688,7 @@ impl PeerActor { debug!("[peer:{}] disconnect", username); self.stream.take(); + self.connection_state = ConnectionState::Disconnected; if self.disconnect_reported { return; @@ -927,21 +929,48 @@ mod tests { stream.set_nonblocking(true).unwrap(); let (far_end, _) = listener.accept().unwrap(); + let (mut actor, rx) = make_actor(Some(stream)); + actor.on_start(); + (actor, rx, far_end) + } + + fn make_actor( + stream: Option, + ) -> (PeerActor, Receiver) { let (tx, rx) = std::sync::mpsc::channel(); let peer = Peer::new( "bob".to_string(), ConnectionType::P, "127.0.0.1".to_string(), - u32::from(addr.port()), + 0, None, 0, 0, 0, ); - let mut actor = - PeerActor::new(peer, Some(stream), None, tx, "me".to_string(), 7); - actor.on_start(); - (actor, rx, far_end) + let actor = PeerActor::new(peer, stream, None, tx, "me".to_string(), 7); + (actor, rx) + } + + #[test] + fn a_timed_out_connect_parks_the_actor_in_disconnected() { + let (mut actor, rx) = make_actor(None); + actor.connection_state = ConnectionState::Connecting { + since: Instant::now().checked_sub(Duration::from_secs(21)).unwrap(), + }; + + actor.tick(); + + assert!( + matches!(actor.connection_state, ConnectionState::Disconnected), + "a timed-out connect must leave Connecting" + ); + match rx.try_recv() { + Ok(ClientOperation::PeerConnectFailed(7, username)) => { + assert_eq!(username, "bob"); + } + other => panic!("expected PeerConnectFailed, got {other:?}"), + } } #[test] diff --git a/soulseek-rs-lib/src/actor/server_actor.rs b/soulseek-rs-lib/src/actor/server_actor.rs index a2e796c..57c1778 100644 --- a/soulseek-rs-lib/src/actor/server_actor.rs +++ b/soulseek-rs-lib/src/actor/server_actor.rs @@ -834,7 +834,6 @@ impl ServerActor { closed by the server" ); self.session.record(SessionLoss::Displaced); - self.connection_state = ConnectionState::Disconnected; self.disconnect(); } @@ -845,8 +844,8 @@ impl ServerActor { // it, so anything still waiting on the network is waiting for good. if matches!(self.connection_state, ConnectionState::Connected) { self.session.record(SessionLoss::Disconnected); - self.connection_state = ConnectionState::Disconnected; } + self.connection_state = ConnectionState::Disconnected; self.stream.take(); } @@ -854,6 +853,7 @@ impl ServerActor { debug!("[server] disconnected"); self.stream.take(); + self.connection_state = ConnectionState::Disconnected; } fn check_connection_status(&mut self) { @@ -939,13 +939,35 @@ impl Actor for ServerActor { #[cfg(test)] mod tests { - use super::post_login_messages; - use crate::message::Message; + use super::*; fn code_of(message: &Message) -> u32 { u32::from_le_bytes(message.get_data()[0..4].try_into().unwrap()) } + #[test] + fn a_timed_out_connect_parks_the_actor_in_disconnected() { + let (tx, _rx) = std::sync::mpsc::channel(); + let mut actor = ServerActor::new( + PeerAddress::new("127.0.0.1".to_string(), 1), + tx, + 0, + false, + 0, + 0, + ); + actor.connection_state = ConnectionState::Connecting { + since: Instant::now().checked_sub(Duration::from_secs(21)).unwrap(), + }; + + actor.tick(); + + assert!( + matches!(actor.connection_state, ConnectionState::Disconnected), + "a timed-out connect must leave Connecting" + ); + } + #[test] fn post_login_messages_carry_counts_and_conditional_wait_port() { let messages = post_login_messages(true, 4321, 3, 7);