From 94cb2368cc152b2a7275cb51afb6a564c9d5f5a3 Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Wed, 22 Apr 2026 19:57:05 -0400 Subject: [PATCH 01/14] Apply hoists: run_future doesnt call tokio::spawn, client::new/with_loopback return type has a breaking change and server::start_announcing's loop signature changed --- src/client/inner.rs | 86 ++++++++++++++++++++++----------- src/client/mod.rs | 106 ++++++++++++++++++++++++++++++----------- src/lib.rs | 6 ++- src/server/mod.rs | 42 ++++++++++++---- tests/client_server.rs | 36 +++++++++----- 5 files changed, 197 insertions(+), 79 deletions(-) diff --git a/src/client/inner.rs b/src/client/inner.rs index 5db723a..f344a3b 100644 --- a/src/client/inner.rs +++ b/src/client/inner.rs @@ -344,13 +344,22 @@ impl Inner where PayloadDefinitions: PayloadWireFormat + Clone + std::fmt::Debug + 'static, { - pub fn spawn( + /// Construct an `Inner` and return the control/update channels plus + /// the run-loop future. The caller drives the future with whatever + /// executor it owns (typically `tokio::spawn`). + /// + /// The future is kept unbounded on `Send` / `'static` at this layer + /// so bare-metal executors can drive it without paying the thread- + /// safety tax; `tokio::spawn` callers bind `Send + 'static` at their + /// spawn site, where the compiler can see the concrete state types. + pub fn new( interface: Ipv4Addr, e2e_registry: Arc>, multicast_loopback: bool, ) -> ( Sender>, mpsc::UnboundedReceiver>, + impl core::future::Future, ) { info!("Initializing SOME/IP Client"); let (control_sender, control_receiver) = mpsc::channel(4); @@ -374,8 +383,7 @@ where multicast_loopback, phantom: std::marker::PhantomData, }; - inner.run(); - (control_sender, update_receiver) + (control_sender, update_receiver, inner.run_future()) } async fn bind_discovery(&mut self) -> Result<(), Error> { @@ -878,8 +886,8 @@ where } #[allow(clippy::too_many_lines)] - fn run(mut self) { - tokio::spawn(async move { + fn run_future(mut self) -> impl core::future::Future { + async move { info!("SOME/IP Client processing loop started"); loop { let Self { @@ -1028,7 +1036,7 @@ where } self.handle_control_message().await; } - }); + } } } @@ -1367,11 +1375,12 @@ mod tests { #[tokio::test] async fn test_inner_spawn_and_shutdown() { - let (control_sender, mut update_receiver) = Inner::::spawn( + let (control_sender, mut update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); // Drop control sender to trigger loop exit drop(control_sender); // The update receiver should eventually return None when the inner loop exits @@ -1401,11 +1410,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_bind_discovery_continues() { - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); let (rx, msg) = TestControl::bind_discovery(); drop(rx); @@ -1417,11 +1427,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_unbind_discovery_continues() { - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); let (rx, msg) = TestControl::unbind_discovery(); drop(rx); @@ -1433,11 +1444,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_set_interface_continues() { - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); // SetInterface(LOCALHOST) on a fresh inner goes straight to // bind_discovery + send response (interface already matches). @@ -1451,11 +1463,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_send_sd_continues() { - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); // Bind discovery first so the SendSD path has a socket to use let (rx, msg) = TestControl::bind_discovery(); @@ -1480,11 +1493,12 @@ mod tests { #[tokio::test] async fn test_queued_messages_all_complete() { - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); // Bind discovery so SetInterface will take the multi-step path: // iteration 1: unbind discovery, re-queue SetInterface @@ -1550,11 +1564,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_add_endpoint_continues() { - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); let (rx, msg) = TestControl::add_endpoint(0x1234, 0x0001, addr, 0); @@ -1567,11 +1582,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_remove_endpoint_continues() { - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); let (rx, msg) = TestControl::remove_endpoint(0x1234, 0x0001); drop(rx); @@ -1583,11 +1599,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_send_to_service_send_complete_continues() { - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); // Add an endpoint first so SendToService doesn't fail with ServiceNotFound let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); @@ -1609,11 +1626,12 @@ mod tests { async fn test_bind_discovery_with_loopback() { // Spawn inner with multicast_loopback=true so bind_discovery exercises // the loopback-enabled branch of SocketManager::bind_discovery. - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), true, ); + tokio::spawn(run_fut); let (rx, msg) = TestControl::bind_discovery(); control_sender.send(msg).await.unwrap(); @@ -1623,11 +1641,12 @@ mod tests { #[tokio::test] async fn test_bind_discovery_idempotent() { // Binding discovery twice should succeed (early return on already-bound) - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); let (rx, msg) = TestControl::bind_discovery(); control_sender.send(msg).await.unwrap(); @@ -1642,11 +1661,12 @@ mod tests { #[tokio::test] async fn test_send_sd_auto_binds_discovery() { // SendSD without a bound discovery socket should auto-bind and succeed - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); let target = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 30490); let sd_header = empty_sd_header(); @@ -1662,11 +1682,12 @@ mod tests { #[tokio::test] async fn test_send_to_service_auto_binds_unicast() { // SendToService with no unicast sockets should auto-bind ephemeral - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); let (rx, msg) = TestControl::add_endpoint(0x1234, 0x0001, addr, 0); @@ -1686,11 +1707,12 @@ mod tests { #[tokio::test] async fn test_subscribe_with_endpoint_sends_sd() { // Subscribe with a known endpoint and bound discovery should send the SD message - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); // Bind discovery first let (rx, msg) = TestControl::bind_discovery(); @@ -1716,11 +1738,12 @@ mod tests { #[tokio::test] async fn test_subscribe_auto_binds_discovery() { // Subscribe without discovery bound should auto-bind and succeed - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); // Add endpoint but do NOT bind discovery let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); @@ -1740,11 +1763,12 @@ mod tests { #[tokio::test] async fn test_subscribe_unknown_service_returns_error() { - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); let (rx, msg) = TestControl::subscribe(0xFFFF, 0xFFFF, 1, 3, 0x01, 0); control_sender.send(msg).await.unwrap(); @@ -1758,11 +1782,12 @@ mod tests { #[tokio::test] async fn test_send_to_service_reuses_existing_unicast_socket() { // When a unicast socket already exists, SendToService should reuse it - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); let (rx, msg) = TestControl::add_endpoint(0x1234, 0x0001, addr, 0); @@ -1792,11 +1817,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_subscribe_service_not_found_continues() { // Subscribe with no endpoint → ServiceNotFound response is dropped - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); let (rx, msg) = TestControl::subscribe(0x1234, 0x0001, 1, 3, 0x01, 0); drop(rx); @@ -1809,11 +1835,12 @@ mod tests { #[tokio::test] async fn test_set_interface_changes_interface() { // SetInterface to a different address exercises the interface!=current path - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); // Change to a different loopback-range address (127.0.0.2). // Binding discovery on 127.0.0.2 should succeed on most systems. @@ -1833,11 +1860,12 @@ mod tests { #[tokio::test] async fn test_set_interface_with_discovery_bound_changes_interface() { // SetInterface when discovery is already bound: unbind → change → rebind - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); // Bind discovery on LOCALHOST first let (rx, msg) = TestControl::bind_discovery(); @@ -1863,11 +1891,12 @@ mod tests { async fn test_subscribe_specific_port_reuse() { // Subscribe twice with the same specific client_port exercises the // bind_unicast port-reuse path (port != 0 && already bound). - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); // Add endpoint and bind discovery let (rx, msg) = TestControl::bind_discovery(); @@ -1909,11 +1938,12 @@ mod tests { use std::vec; use tokio::net::UdpSocket; - let (control_sender, _update_receiver) = Inner::::spawn( + let (control_sender, _update_receiver, run_fut) = Inner::::new( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); + tokio::spawn(run_fut); let raw = UdpSocket::bind("127.0.0.1:0").await.unwrap(); let target = SocketAddrV4::new(Ipv4Addr::LOCALHOST, raw.local_addr().unwrap().port()); diff --git a/src/client/mod.rs b/src/client/mod.rs index 223ab5b..8758ce8 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -185,11 +185,37 @@ where { /// Creates a new client bound to the given network interface and spawns its event loop. /// - /// Returns a `(Client, ClientUpdates)` pair. The `Client` handle is - /// [`Clone`]-able and can be shared across tasks. `ClientUpdates` receives - /// discovery, unicast, and error updates from the event loop. + /// Returns a `(Client, ClientUpdates, run_future)` triple. The `Client` + /// handle is [`Clone`]-able and can be shared across tasks. + /// `ClientUpdates` receives discovery, unicast, and error updates from + /// the event loop. `run_future` is the event loop itself — the caller + /// must drive it to completion (typically via `tokio::spawn`) for the + /// client to process any messages. + /// + /// The future is not bounded `Send + 'static` at this layer; the + /// concrete captured state is `Send + 'static` in practice, and + /// `tokio::spawn` will bind those where required at the call site. + /// Bare-metal callers driving the future on a single-task executor + /// pay no `Send` tax. + /// + /// ```no_run + /// # use simple_someip::{Client, RawPayload}; + /// # use std::net::Ipv4Addr; + /// # async fn demo() { + /// let (client, mut updates, run) = Client::::new(Ipv4Addr::LOCALHOST); + /// tokio::spawn(run); + /// // ...interact with `client` and `updates`... + /// # let _ = (client, updates); + /// # } + /// ``` #[must_use] - pub fn new(interface: Ipv4Addr) -> (Self, ClientUpdates) { + pub fn new( + interface: Ipv4Addr, + ) -> ( + Self, + ClientUpdates, + impl core::future::Future, + ) { Self::new_with_loopback(interface, false) } @@ -220,10 +246,14 @@ where pub fn new_with_loopback( interface: Ipv4Addr, multicast_loopback: bool, - ) -> (Self, ClientUpdates) { + ) -> ( + Self, + ClientUpdates, + impl core::future::Future, + ) { let e2e_registry = Arc::new(Mutex::new(E2ERegistry::new())); - let (control_sender, update_receiver) = - Inner::spawn(interface, Arc::clone(&e2e_registry), multicast_loopback); + let (control_sender, update_receiver, run_future) = + Inner::new(interface, Arc::clone(&e2e_registry), multicast_loopback); let client = Self { interface: Arc::new(RwLock::new(interface)), @@ -231,7 +261,7 @@ where e2e_registry, }; let updates = ClientUpdates { update_receiver }; - (client, updates) + (client, updates, run_future) } /// Returns the current network interface address. @@ -693,14 +723,16 @@ mod tests { #[tokio::test] async fn test_client_new_and_interface() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); assert_eq!(client.interface(), Ipv4Addr::LOCALHOST); client.shut_down(); } #[tokio::test] async fn test_client_debug() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let debug_str = format!("{client:?}"); assert!(debug_str.contains("Client")); assert!(debug_str.contains("127.0.0.1")); @@ -746,7 +778,8 @@ mod tests { #[tokio::test] async fn test_subscribe_unknown_service_returns_error() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let result = client.subscribe(0xFFFF, 0xFFFF, 1, 3, 0x01, 0).await; assert!( matches!(result, Err(Error::ServiceNotFound)), @@ -757,7 +790,8 @@ mod tests { #[tokio::test] async fn test_subscribe_no_wait_unknown_service_does_not_panic() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); // subscribe_no_wait is fire-and-forget — it should not panic even // when the service is unknown (the inner loop sends ServiceNotFound // on the dropped response channel, which is harmless). @@ -769,7 +803,8 @@ mod tests { #[tokio::test] async fn test_bind_discovery_and_unbind() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); client.unbind_discovery().await.unwrap(); client.shut_down(); @@ -777,7 +812,8 @@ mod tests { #[tokio::test] async fn test_set_interface() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let new_addr = Ipv4Addr::LOCALHOST; client.set_interface(new_addr).await.unwrap(); assert_eq!(client.interface(), new_addr); @@ -786,7 +822,8 @@ mod tests { #[tokio::test] async fn test_add_endpoint_succeeds() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::new(192, 168, 1, 1), 30000); client.add_endpoint(0x1234, 0x0001, addr, 0).await.unwrap(); client.shut_down(); @@ -794,7 +831,8 @@ mod tests { #[tokio::test] async fn test_send_to_service_unknown_returns_error() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let msg = crate::protocol::Message::new_sd(1, &empty_sd_header()); let result = client.send_to_service(0xFFFF, 0xFFFF, msg).await; assert!( @@ -806,7 +844,8 @@ mod tests { #[tokio::test] async fn test_remove_endpoint_succeeds() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::new(192, 168, 1, 1), 30000); client.add_endpoint(0x1234, 0x0001, addr, 0).await.unwrap(); client.remove_endpoint(0x1234, 0x0001).await.unwrap(); @@ -847,7 +886,8 @@ mod tests { #[tokio::test] async fn test_send_sd_message() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); // Bind discovery first so the send path uses the existing socket client.bind_discovery().await.unwrap(); let target = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 30490); @@ -858,7 +898,8 @@ mod tests { #[tokio::test] async fn test_send_to_service_success_returns_pending_response() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 30000); client.add_endpoint(0x1234, 0x0001, addr, 0).await.unwrap(); let msg = crate::protocol::Message::new_sd(1, &empty_sd_header()); @@ -870,7 +911,8 @@ mod tests { #[tokio::test] async fn test_recv_returns_none_after_shutdown() { - let (client, mut updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); client.shut_down(); // Now the inner loop should exit; recv() should return None let result = tokio::time::timeout(std::time::Duration::from_secs(2), updates.recv()).await; @@ -880,7 +922,8 @@ mod tests { #[tokio::test] async fn test_register_and_unregister_e2e() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let key = E2EKey { service_id: 0x1234, method_or_event_id: 0x0001, @@ -893,7 +936,8 @@ mod tests { #[tokio::test] async fn test_client_is_clone() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let client2 = client.clone(); assert_eq!(client.interface(), client2.interface()); client.shut_down(); @@ -901,14 +945,16 @@ mod tests { #[tokio::test] async fn test_client_updates_debug() { - let (_client, updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (_client, updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let debug_str = format!("{updates:?}"); assert!(debug_str.contains("ClientUpdates")); } #[tokio::test] async fn test_request_unknown_service_returns_error() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let msg = crate::protocol::Message::new_sd(1, &empty_sd_header()); let result = client.request(0xFFFF, 0xFFFF, msg).await; assert!( @@ -920,7 +966,8 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_does_not_panic() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); let sd_header = empty_sd_header(); @@ -943,7 +990,8 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_without_discovery_bound() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); // Don't bind discovery — the task should handle the error gracefully. let sd_header = empty_sd_header(); let handle = @@ -964,7 +1012,8 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_abort_stops_task() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); let sd_header = empty_sd_header(); @@ -1089,7 +1138,8 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_stops_on_shutdown() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); let sd_header = empty_sd_header(); diff --git a/src/lib.rs b/src/lib.rs index 7cc0529..c516f9d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -66,8 +66,10 @@ //! //! #[tokio::main] //! async fn main() { -//! // Client::new returns a Clone-able handle and an update stream. -//! let (client, mut updates) = Client::::new([192, 168, 1, 100].into()); +//! // Client::new returns a Clone-able handle, an update stream, and +//! // the run-loop future. Spawn the future on any executor. +//! let (client, mut updates, run) = Client::::new([192, 168, 1, 100].into()); +//! tokio::spawn(run); //! client.bind_discovery().await.unwrap(); //! //! while let Some(update) = updates.recv().await { diff --git a/src/server/mod.rs b/src/server/mod.rs index b129301..0c9e842 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -263,9 +263,21 @@ impl Server { }) } - /// Start announcing the service via Service Discovery + /// Build the periodic-SD-announcement future. /// - /// This sends periodic `OfferService` messages to the SD multicast group + /// Returns a future that sends an `OfferService` message to the SD + /// multicast group every second. The caller must drive the future + /// (typically via `tokio::spawn`) for announcements to fire; this + /// function does no work on its own. + /// + /// ```no_run + /// # use simple_someip::server::Server; + /// # async fn demo(server: Server) -> Result<(), simple_someip::server::Error> { + /// let announce_fut = server.announcement_loop()?; + /// tokio::spawn(announce_fut); + /// # Ok(()) + /// # } + /// ``` /// /// # Errors /// @@ -273,15 +285,14 @@ impl Server { /// called on a server constructed via [`Server::new_passive`] — passive /// servers have no real SD socket bound to port 30490, so any /// announcements would go out with an incorrect source port. - /// - /// Otherwise currently always returns `Ok(())`; SD send failures are - /// logged internally. - pub fn start_announcing(&self) -> Result<(), Error> { + pub fn announcement_loop( + &self, + ) -> Result + 'static, Error> { if self.is_passive { return Err(Error::Io(std::io::Error::new( std::io::ErrorKind::InvalidInput, format!( - "start_announcing called on passive Server for service 0x{:04X}; \ + "announcement_loop called on passive Server for service 0x{:04X}; \ announcements must be driven externally (e.g. via \ `simple_someip::Client::start_sd_announcements`)", self.config.service_id @@ -292,7 +303,7 @@ impl Server { let sd_socket = Arc::clone(&self.sd_socket); let sd_state = Arc::clone(&self.sd_state); - tokio::spawn(async move { + Ok(async move { let mut announcement_count = 0u32; loop { match sd_state.send_offer_service(&config, &sd_socket).await { @@ -319,8 +330,21 @@ impl Server { // Send announcements every 1 second tokio::time::sleep(tokio::time::Duration::from_secs(1)).await; } - }); + }) + } + /// Deprecated shim for [`Self::announcement_loop`] that spawns the + /// returned future on tokio internally. Kept so the old + /// `server.start_announcing()?;` idiom continues to compile; new code + /// should call `announcement_loop` and spawn on its own executor so + /// the server is portable to bare-metal. + /// + /// # Errors + /// + /// Same as [`Self::announcement_loop`]. + pub fn start_announcing(&self) -> Result<(), Error> { + let fut = self.announcement_loop()?; + tokio::spawn(fut); Ok(()) } diff --git a/tests/client_server.rs b/tests/client_server.rs index ffd6d34..d48fee1 100644 --- a/tests/client_server.rs +++ b/tests/client_server.rs @@ -56,7 +56,8 @@ async fn test_client_server_subscribe_and_receive_event() { let server_handle = tokio::spawn(async move { server.run().await }); // Create client and subscribe to the server's event group - let (client, mut updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); @@ -99,7 +100,8 @@ async fn test_client_send_sd_auto_binds_discovery() { let server_handle = tokio::spawn(async move { server.run().await }); // Create client — NO bind_discovery - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); // send_sd_message should auto-bind discovery and succeed let sd_header = VecSdHeader { @@ -130,7 +132,8 @@ async fn test_client_bind_unbind_lifecycle_with_server() { let (mut server, server_port) = create_server(0x5B, 1).await; let server_handle = tokio::spawn(async move { server.run().await }); - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); // Bind discovery, subscribe, then unbind and rebind client.bind_discovery().await.unwrap(); @@ -158,7 +161,8 @@ async fn test_add_endpoint_and_send_to_service() { let publisher = server.publisher(); let server_handle = tokio::spawn(async move { server.run().await }); - let (client, mut updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); // Register the server's endpoint manually (simulating non-broadcasting service) @@ -218,7 +222,8 @@ async fn test_subscribe_auto_binds_discovery() { let server_handle = tokio::spawn(async move { server.run().await }); // Create client — do NOT bind discovery manually - let (client, mut updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); // Subscribe should auto-bind discovery internally @@ -260,7 +265,8 @@ async fn test_client_request_resolves_via_unicast_reply() { let publisher = server.publisher(); let server_handle = tokio::spawn(async move { server.run().await }); - let (client, mut updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); @@ -321,7 +327,8 @@ async fn test_e2e_protect_on_publish_and_check_on_receive() { let server_handle = tokio::spawn(async move { server.run().await }); - let (client, mut updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); // Register matching E2E profile on client client.register_e2e(key, profile); @@ -385,12 +392,14 @@ async fn test_multiple_subscribers_receive_events() { let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); // Client 1 - let (client1, mut updates1) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client1, mut updates1, run_fut1) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut1); client1.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client1.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); // Client 2 - let (client2, mut updates2) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client2, mut updates2, run_fut2) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut2); client2.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client2.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); @@ -443,7 +452,8 @@ async fn test_multiple_subscribers_receive_events() { /// Verify ClientUpdates returns None after client shutdown. #[tokio::test] async fn test_updates_drain_after_shutdown() { - let (client, mut updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); client.shut_down(); let result = tokio::time::timeout(std::time::Duration::from_secs(2), updates.recv()) @@ -458,7 +468,8 @@ async fn test_cloned_client_works() { let (mut server, server_port) = create_server(0x5B, 1).await; let server_handle = tokio::spawn(async move { server.run().await }); - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let client2 = client.clone(); // Both clones can send commands @@ -478,7 +489,8 @@ async fn test_subscribe_specific_port_reuse() { let (mut server, server_port) = create_server(0x5B, 1).await; let server_handle = tokio::spawn(async move { server.run().await }); - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); From 07f7abea8cb4e14350a2f02cb94e58aad3a79d5b Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Wed, 22 Apr 2026 20:09:15 -0400 Subject: [PATCH 02/14] Add testing, add sensible panics, updated docs --- README.md | 2 +- examples/client_server/src/main.rs | 4 +- src/client/inner.rs | 14 ++-- src/client/mod.rs | 53 ++++++++++++-- src/server/README.md | 7 +- src/server/mod.rs | 107 ++++++++++++++++++++--------- 6 files changed, 134 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index 1f827a3..2d2f876 100644 --- a/README.md +++ b/README.md @@ -95,7 +95,7 @@ use std::net::Ipv4Addr; async fn main() -> Result<(), Box> { let config = ServerConfig::new(Ipv4Addr::new(192, 168, 1, 200), 30500, 0x1234, 1); let mut server = Server::new(config).await?; - server.start_announcing()?; + tokio::spawn(server.announcement_loop()?); let publisher = server.publisher(); tokio::spawn(async move { server.run().await }); diff --git a/examples/client_server/src/main.rs b/examples/client_server/src/main.rs index 82771d0..f78410d 100644 --- a/examples/client_server/src/main.rs +++ b/examples/client_server/src/main.rs @@ -10,7 +10,7 @@ //! This ensures remote nodes see a single coherent network identity for //! multicast announcements. //! -//! The server's built-in `start_announcing()` is NOT used — instead, the +//! The server's built-in `announcement_loop()` is NOT used — instead, the //! client's `start_sd_announcements()` handles periodic multicast //! announcements. The server's `run()` loop still handles unicast SD //! traffic (e.g. `SubscribeAck`/`SubscribeNack` responses) on its own @@ -125,7 +125,7 @@ async fn main() -> Result<(), Box> { let mut server = Server::new(config).await?; info!("Server bound on port {MY_SERVER_PORT}"); - // NOTE: We intentionally do NOT call server.start_announcing(). + // NOTE: We intentionally do NOT spawn server.announcement_loop(). // The client's start_sd_announcements handles all SD traffic. let _publisher = server.publisher(); diff --git a/src/client/inner.rs b/src/client/inner.rs index f344a3b..3b6ed72 100644 --- a/src/client/inner.rs +++ b/src/client/inner.rs @@ -348,10 +348,12 @@ where /// the run-loop future. The caller drives the future with whatever /// executor it owns (typically `tokio::spawn`). /// - /// The future is kept unbounded on `Send` / `'static` at this layer - /// so bare-metal executors can drive it without paying the thread- - /// safety tax; `tokio::spawn` callers bind `Send + 'static` at their - /// spawn site, where the compiler can see the concrete state types. + /// The future is bounded `Send + 'static` because every in-repo + /// consumer spawns it on a multithreaded executor and because the + /// concrete captured state (tokio mpsc, `TokioSocket`, E2E registry) + /// is already Send. A bare-metal consumer whose transport produces + /// `!Send` state needs a cfg-gated alternative constructor; none + /// exists yet — it's planned alongside the bare-metal port. pub fn new( interface: Ipv4Addr, e2e_registry: Arc>, @@ -359,7 +361,7 @@ where ) -> ( Sender>, mpsc::UnboundedReceiver>, - impl core::future::Future, + impl core::future::Future + Send + 'static, ) { info!("Initializing SOME/IP Client"); let (control_sender, control_receiver) = mpsc::channel(4); @@ -886,7 +888,7 @@ where } #[allow(clippy::too_many_lines)] - fn run_future(mut self) -> impl core::future::Future { + fn run_future(mut self) -> impl core::future::Future + Send + 'static { async move { info!("SOME/IP Client processing loop started"); loop { diff --git a/src/client/mod.rs b/src/client/mod.rs index 8758ce8..7292721 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -192,11 +192,10 @@ where /// must drive it to completion (typically via `tokio::spawn`) for the /// client to process any messages. /// - /// The future is not bounded `Send + 'static` at this layer; the - /// concrete captured state is `Send + 'static` in practice, and - /// `tokio::spawn` will bind those where required at the call site. - /// Bare-metal callers driving the future on a single-task executor - /// pay no `Send` tax. + /// The future is bounded `Send + 'static` because every in-repo + /// consumer spawns it on a multithreaded executor. Bare-metal + /// consumers whose transport produces `!Send` state will get a + /// cfg-gated alternative constructor alongside the bare-metal port. /// /// ```no_run /// # use simple_someip::{Client, RawPayload}; @@ -214,7 +213,7 @@ where ) -> ( Self, ClientUpdates, - impl core::future::Future, + impl core::future::Future + Send + 'static, ) { Self::new_with_loopback(interface, false) } @@ -249,7 +248,7 @@ where ) -> ( Self, ClientUpdates, - impl core::future::Future, + impl core::future::Future + Send + 'static, ) { let e2e_registry = Arc::new(Mutex::new(E2ERegistry::new())); let (control_sender, update_receiver, run_future) = @@ -1159,4 +1158,44 @@ mod tests { "task should have exited cleanly, not panicked" ); } + + /// Documents the footgun: if the caller drops `run_fut` without ever + /// polling it, the control channel's receiver goes with it and + /// subsequent `Client` method calls panic on `control_sender.send()`. + /// + /// This is intrinsic to the caller-driven lifecycle introduced in + /// phase 6 — the run loop is no longer owned by `Client::new`, so + /// failing to spawn it is the caller's responsibility. The test + /// pins the behavior deterministically so that any attempt to + /// silently "fix" this (e.g. internal spawn fallback) would break it + /// and force a review. + #[tokio::test] + #[should_panic(expected = "SendError")] + async fn dropping_run_future_without_spawn_panics_on_next_client_call() { + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + // Caller explicitly discards the run loop. + drop(run_fut); + // Any client method that enqueues a control message panics on + // `.unwrap()` of the send Result — document it instead of + // hiding it. + client.bind_discovery().await.unwrap(); + } + + /// If the run loop is cancelled mid-poll (caller-initiated timeout, + /// graceful shutdown), subsequent `Client` calls see the control + /// channel closed and surface a panic from `control_sender.send()`. + /// Same structural contract as dropping the run future. + #[tokio::test] + #[should_panic(expected = "SendError")] + async fn cancelling_run_future_closes_control_channel() { + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + let handle = tokio::spawn(run_fut); + // Let the loop start. + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + handle.abort(); + // Give the abort time to land. + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + + client.bind_discovery().await.unwrap(); + } } diff --git a/src/server/README.md b/src/server/README.md index cb78f05..989824f 100644 --- a/src/server/README.md +++ b/src/server/README.md @@ -55,8 +55,9 @@ async fn main() -> Result<(), Box> { // Create the server let mut server = Server::new(config).await?; - // Start announcing the service (sends OfferService every 1s) - server.start_announcing()?; + // Start announcing the service (sends OfferService every 1s). + // Spawn the announcement loop future on your executor. + tokio::spawn(server.announcement_loop()?); // Get event publisher for sending events let publisher = server.publisher(); @@ -153,7 +154,7 @@ Configuration for a SOME/IP service provider: Main server struct: - `new(config: ServerConfig) -> Result` - Create new server -- `start_announcing() -> Result<()>` - Start SD announcements +- `announcement_loop() -> Result + Send + 'static>` - Build the SD announcement future; caller spawns on their executor - `publisher() -> Arc` - Get event publisher - `run() -> Result<()>` - Run event loop (handles subscriptions) - `register_e2e(key, profile)` - Register E2E protection for a message key diff --git a/src/server/mod.rs b/src/server/mod.rs index 0c9e842..e8ddc52 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -82,7 +82,7 @@ pub struct Server { e2e_registry: Arc>, /// `true` if this server was constructed via [`Server::new_passive`]. /// Passive servers have no real SD socket bound to port 30490; their - /// SD handling is managed externally. Calling [`Self::start_announcing`] + /// SD handling is managed externally. Calling [`Self::announcement_loop`] /// or [`Self::run`] on a passive server is a programming error and /// returns an [`Error::Io`] with [`std::io::ErrorKind::InvalidInput`]. is_passive: bool, @@ -209,7 +209,7 @@ impl Server { /// incoming `SubscribeEventGroup` / `FindService` messages and routes /// them to the right `EventPublisher` via /// [`EventPublisher::register_subscriber`]). Do **not** call - /// [`Server::start_announcing`] or spawn [`Server::run`] on a passive + /// [`Server::announcement_loop`] or spawn [`Server::run`] on a passive /// server — the external dispatcher owns those responsibilities. /// /// # Errors @@ -229,7 +229,7 @@ impl Server { // Bind a placeholder SD socket on an ephemeral port. Nothing will // route to it (neither multicast nor unicast on 30490), and neither - // `start_announcing` nor `run` should be called for a passive + // `announcement_loop` nor `run` should be called for a passive // server. We still allocate it so the `Server` struct shape is // identical to the full-server path. let sd_placeholder_addr = std::net::SocketAddr::new(IpAddr::V4(config.interface), 0); @@ -287,7 +287,7 @@ impl Server { /// announcements would go out with an incorrect source port. pub fn announcement_loop( &self, - ) -> Result + 'static, Error> { + ) -> Result + Send + 'static, Error> { if self.is_passive { return Err(Error::Io(std::io::Error::new( std::io::ErrorKind::InvalidInput, @@ -333,21 +333,6 @@ impl Server { }) } - /// Deprecated shim for [`Self::announcement_loop`] that spawns the - /// returned future on tokio internally. Kept so the old - /// `server.start_announcing()?;` idiom continues to compile; new code - /// should call `announcement_loop` and spawn on its own executor so - /// the server is portable to bare-metal. - /// - /// # Errors - /// - /// Same as [`Self::announcement_loop`]. - pub fn start_announcing(&self) -> Result<(), Error> { - let fut = self.announcement_loop()?; - tokio::spawn(fut); - Ok(()) - } - /// Send a unicast `OfferService` to a specific address (in response to `FindService`) async fn send_unicast_offer(&self, target: std::net::SocketAddr) -> Result<(), Error> { use crate::protocol::Header as SomeIpHeader; @@ -1357,10 +1342,15 @@ mod tests { assert_eq!(entry.service_id(), 0x5B); assert_eq!(entry.instance_id(), 1); - // Also test that start_announcing doesn't error + // Also test that announcement_loop builds a future without error. drop(server); let (server2, _) = create_test_server(0x5B, 1).await; - assert!(server2.start_announcing().is_ok()); + let fut = server2 + .announcement_loop() + .expect("announcement_loop on a regular server must build"); + // Spawn and immediately drop the handle — we only care that the + // construction did not error here. + drop(tokio::spawn(fut)); } #[tokio::test] @@ -1948,11 +1938,12 @@ mod tests { } #[tokio::test] - async fn start_announcing_on_passive_returns_invalid_input() { + async fn announcement_loop_on_passive_returns_invalid_input() { let server = make_passive_server(0x005C, 0x0001).await; let err = server - .start_announcing() - .expect_err("start_announcing on a passive server must fail"); + .announcement_loop() + .err() + .expect("announcement_loop on a passive server must fail"); match err { Error::Io(io_err) => { assert_eq!(io_err.kind(), std::io::ErrorKind::InvalidInput); @@ -1995,18 +1986,66 @@ mod tests { } #[tokio::test] - async fn start_announcing_on_regular_server_still_succeeds() { - // Regression guard: the new is_passive check must not break the + async fn announcement_loop_on_regular_server_still_succeeds() { + // Regression guard: the is_passive check must not break the // standard non-passive path. let (server, _port) = create_test_server(0x005C, 0x0001).await; - server - .start_announcing() - .expect("start_announcing on a regular server must still succeed"); - // The announcer task runs forever; the test succeeds as soon as - // start_announcing returns Ok. The spawned task is cleaned up - // when the Tokio test runtime shuts down at the end of this - // test — `tokio::spawn` tasks are not aborted by dropping - // unrelated handles, they ride the runtime lifecycle. + let fut = server + .announcement_loop() + .expect("announcement_loop on a regular server must build"); + // The announcer loops forever; the test succeeds as soon as + // construction returns Ok. Spawn + drop the JoinHandle — the + // task rides the runtime lifecycle until the test's tokio + // runtime shuts down at end-of-test. + drop(tokio::spawn(fut)); + } + + /// Direct test that `announcement_loop` actually emits an SD + /// announcement when driven. Explicit coverage for the primary entry + /// point (avoids regressions where only the deleted shim was exercised). + #[tokio::test] + async fn announcement_loop_sends_offer_service_when_driven() { + use crate::protocol::MessageId; + + // Bind a receiver on the SD multicast port with loopback so we + // actually see the outgoing announcement. Use a dedicated + // receiver socket via socket2 to match the SD bind pattern. + let iface = std::net::Ipv4Addr::LOCALHOST; + let recv = { + let s = socket2::Socket::new( + socket2::Domain::IPV4, + socket2::Type::DGRAM, + Some(socket2::Protocol::UDP), + ) + .unwrap(); + s.set_reuse_address(true).unwrap(); + #[cfg(unix)] + s.set_reuse_port(true).unwrap(); + s.bind(&std::net::SocketAddr::new(IpAddr::V4(iface), sd::MULTICAST_PORT).into()) + .unwrap(); + s.set_nonblocking(true).unwrap(); + let std_s: std::net::UdpSocket = s.into(); + let rs = tokio::net::UdpSocket::from_std(std_s).unwrap(); + rs.join_multicast_v4(sd::MULTICAST_IP, iface).unwrap(); + rs + }; + + let config = ServerConfig::new(iface, 30501, 0x005C, 0x0001); + let server = Server::new_with_loopback(config, true).await.unwrap(); + let fut = server.announcement_loop().expect("build loop"); + let handle = tokio::spawn(fut); + + let mut buf = [0u8; 1500]; + let (n, _src) = + tokio::time::timeout(std::time::Duration::from_secs(3), recv.recv_from(&mut buf)) + .await + .expect("timed out waiting for announcement") + .expect("recv failed"); + + let view = crate::protocol::MessageView::parse(&buf[..n]).unwrap(); + assert_eq!(view.header().message_id(), MessageId::SD); + + handle.abort(); } #[tokio::test] From 84ee4e8544396609948b9b35075bfcbf895a8683 Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Thu, 23 Apr 2026 13:13:15 -0400 Subject: [PATCH 03/14] phase 6: respond to PR #80 feedback (typed Shutdown + test/doc fixes) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the six open comments on PR #80 that 76ee480 did not cover; the three structural decisions (Client::new breaking signature, start_announcing -> announcement_loop rename, panic -> typed error) were all confirmed as "break, no wrapper" by the maintainer. Typed Error::Shutdown (comment #80-6) ===================================== Added `Error::Shutdown` to `client::Error` — the variant the run-loop control channel surfaces when its receiver is gone (run future dropped, cancelled, or exited). Converted every public Client method that routes through `control_sender` to return `Err(Error::Shutdown)` on channel closure instead of panicking on `.unwrap()` of the send or recv result: - 9 call sites in client/mod.rs switched from self.control_sender.send(..).await.unwrap(); response.await.unwrap() to self.control_sender.send(..).await.map_err(|_| Error::Shutdown)?; response.await.map_err(|_| Error::Shutdown)? - `request()`'s `response_rx.await.expect(...)` follows the same pattern. - `PendingResponse::response()`'s `.expect(...)` does too. Rewrote the two `#[should_panic]` regression tests that were locking in the panic behavior: - `dropping_run_future_without_spawn_returns_shutdown_error` - `cancelling_run_future_closes_control_channel_returns_shutdown_error` Both now assert `matches!(err, Error::Shutdown)` after the expected drop/abort, so any regression that reintroduces the panic path fails the test on the panic itself and any regression that reverts the typed error fails on the matches! check. Removed the `# Panics` doc sections from affected methods; `set_interface` keeps a `# Panics` note scoped to the interface-lock-poisoning case, which is unrelated to the control channel. The new variant's doc comment names the lifecycle condition that produces it. Test-hygiene fixes (comments #80-4, #80-5, #80-10) ================================================== - `drop(tokio::spawn(fut))` at two test sites (server/mod.rs lines 1310 and 1956) replaced with `drop(fut)`. Spawning + detaching the JoinHandle left the announcer task alive for the rest of the test binary's lifetime, emitting multicast that could confuse sibling tests bound to the same group. The test only cares that construction returned Ok, so don't poll the future at all. - `announcement_loop_sends_offer_service_when_driven` strengthened from "message_id == SD" to parsing the SD header, filtering for an `OfferService` entry matching the server's configured service_id / instance_id, and asserting major_version plus a non-zero TTL. Matches the pattern used by the sd_state.rs multicast tests (dedicated service_id + recv loop filter) so parallel test traffic doesn't cause false matches. Docs (#80-3, #80-7, #80-8) ========================== - src/lib.rs: the phase-6 quickstart docs claimed the client run-loop future "can be spawned on any executor", but it depends on tokio (select!, time, sockets). Tightened to "spawn on the tokio runtime" plus a one-line rationale. - examples/client_server/src/main.rs and examples/discovery_client/ src/main.rs: both still destructured the old 2-tuple from `Client::new`. Updated to destructure the 3-tuple and spawn the run future. They were already failing `cargo check --workspace`. - README.md: same fix, with explicit wording that not spawning the run future produces `Error::Shutdown` on subsequent client calls. Verification: `cargo test --all-features --lib` 426/427 pass. The one failure (`announcement_loop_sends_offer_service_when_driven`) is a pre-existing multicast-loopback environmental issue on this machine — reproduces on the 76ee480 base without any of these changes. CI will validate it there. Co-Authored-By: Claude Opus 4.7 --- README.md | 10 +- examples/client_server/src/main.rs | 3 +- examples/discovery_client/src/main.rs | 3 +- src/client/error.rs | 10 ++ src/client/mod.rs | 192 ++++++++++++++++---------- src/lib.rs | 4 +- src/server/mod.rs | 75 ++++++++-- 7 files changed, 205 insertions(+), 92 deletions(-) diff --git a/README.md b/README.md index 2d2f876..5fd13a6 100644 --- a/README.md +++ b/README.md @@ -66,9 +66,13 @@ use std::net::Ipv4Addr; #[tokio::main] async fn main() { - // Client::new returns a (Client, ClientUpdates) pair. - // Client is Clone-able and can be shared across tasks. - let (client, mut updates) = Client::::new(Ipv4Addr::new(192, 168, 1, 100)); + // Client::new returns a Clone-able handle, an update stream, and + // the run-loop future. Spawn the future on the tokio runtime; + // without it the control channel has no driver and Client method + // calls will return `Error::Shutdown`. + let (client, mut updates, run) = + Client::::new(Ipv4Addr::new(192, 168, 1, 100)); + tokio::spawn(run); // Bind the SD multicast socket to discover services client.bind_discovery().await.unwrap(); diff --git a/examples/client_server/src/main.rs b/examples/client_server/src/main.rs index f78410d..0353dd6 100644 --- a/examples/client_server/src/main.rs +++ b/examples/client_server/src/main.rs @@ -106,7 +106,8 @@ async fn main() -> Result<(), Box> { // ── Create the client (handles discovery, subscriptions, SD socket) ── - let (client, mut updates) = simple_someip::Client::::new(interface); + let (client, mut updates, run) = simple_someip::Client::::new(interface); + tokio::spawn(run); client.bind_discovery().await?; info!("Client discovery bound"); diff --git a/examples/discovery_client/src/main.rs b/examples/discovery_client/src/main.rs index 41b90fc..d032702 100644 --- a/examples/discovery_client/src/main.rs +++ b/examples/discovery_client/src/main.rs @@ -287,7 +287,8 @@ async fn main() -> Result<(), Error> { info!("Starting discovery client on interface {interface}"); - let (client, mut updates) = simple_someip::Client::::new(interface); + let (client, mut updates, run) = simple_someip::Client::::new(interface); + tokio::spawn(run); client.bind_discovery().await.unwrap(); let mut state = DiscoveryState::new(); diff --git a/src/client/error.rs b/src/client/error.rs index 97063c0..8f84bc7 100644 --- a/src/client/error.rs +++ b/src/client/error.rs @@ -50,6 +50,16 @@ pub enum Error { /// [`crate::transport::TransportError`]). #[error(transparent)] Transport(#[from] crate::transport::TransportError), + /// The client's internal run-loop future has exited — either because + /// the caller dropped it before or during polling, the executor + /// cancelled its task, or it returned. All public `Client` methods + /// that enqueue a control message or await its response return + /// this variant when the control channel is closed, rather than + /// panicking on `.unwrap()` of the send / recv result. Treat it as + /// a caller-side lifecycle error: the `Client` handle has outlived + /// its driver and further calls on it cannot make progress. + #[error("client run loop is no longer running")] + Shutdown, } #[cfg(test)] diff --git a/src/client/mod.rs b/src/client/mod.rs index 7292721..acf5843 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -62,15 +62,12 @@ impl

PendingResponse

{ /// /// # Errors /// - /// Returns the same errors as the request itself (e.g. deserialization failure). - /// - /// # Panics - /// - /// Panics if the inner loop dropped the response channel. + /// Returns the same errors as the request itself (e.g. deserialization + /// failure). Returns [`Error::Shutdown`] if the client's run-loop + /// future exits before the response is delivered — the caller's + /// `PendingResponse` handle outlived its driver. pub async fn response(self) -> Result { - self.receiver - .await - .expect("inner loop dropped response channel") + self.receiver.await.map_err(|_| Error::Shutdown)? } } @@ -279,13 +276,21 @@ where /// /// Returns an error if rebinding sockets on the new interface fails. /// + /// Returns [`Error::Shutdown`] if the client's run-loop future has + /// exited before this call — the control-channel send cannot + /// complete without its receiver. + /// /// # Panics /// - /// Panics if the internal control channel or interface lock is poisoned/closed. + /// Panics if the interface lock is poisoned (indicates prior panic + /// while the lock was held). pub async fn set_interface(&self, interface: Ipv4Addr) -> Result<(), Error> { let (response, message) = ControlMessage::set_interface(interface); - self.control_sender.send(message).await.unwrap(); - response.await.unwrap()?; + self.control_sender + .send(message) + .await + .map_err(|_| Error::Shutdown)?; + response.await.map_err(|_| Error::Shutdown)??; *self.interface.write().expect("interface lock poisoned") = interface; Ok(()) } @@ -295,14 +300,17 @@ where /// # Errors /// /// Returns an error if binding the multicast socket fails. - /// - /// # Panics - /// - /// Panics if the internal control channel is closed. + /// Returns [`Error::Shutdown`] if the client's run-loop future has + /// exited before this call (dropped, cancelled, or otherwise gone) + /// — the `Client` handle has outlived its driver and further + /// control-channel sends cannot make progress. pub async fn bind_discovery(&self) -> Result<(), Error> { let (response, message) = ControlMessage::bind_discovery(); - self.control_sender.send(message).await.unwrap(); - response.await.unwrap() + self.control_sender + .send(message) + .await + .map_err(|_| Error::Shutdown)?; + response.await.map_err(|_| Error::Shutdown)? } /// Unbinds the SD multicast discovery socket. @@ -310,14 +318,17 @@ where /// # Errors /// /// Returns an error if unbinding the multicast socket fails. - /// - /// # Panics - /// - /// Panics if the internal control channel is closed. + /// Returns [`Error::Shutdown`] if the client's run-loop future has + /// exited before this call (dropped, cancelled, or otherwise gone) + /// — the `Client` handle has outlived its driver and further + /// control-channel sends cannot make progress. pub async fn unbind_discovery(&self) -> Result<(), Error> { let (response, message) = ControlMessage::unbind_discovery(); - self.control_sender.send(message).await.unwrap(); - response.await.unwrap() + self.control_sender + .send(message) + .await + .map_err(|_| Error::Shutdown)?; + response.await.map_err(|_| Error::Shutdown)? } /// Subscribes to an event group on a known service. @@ -325,10 +336,10 @@ where /// # Errors /// /// Returns an error if the service is not found or subscription fails. - /// - /// # Panics - /// - /// Panics if the internal control channel is closed. + /// Returns [`Error::Shutdown`] if the client's run-loop future has + /// exited before this call (dropped, cancelled, or otherwise gone) + /// — the `Client` handle has outlived its driver and further + /// control-channel sends cannot make progress. pub async fn subscribe( &self, service_id: u16, @@ -346,8 +357,11 @@ where event_group_id, client_port, ); - self.control_sender.send(message).await.unwrap(); - response.await.unwrap() + self.control_sender + .send(message) + .await + .map_err(|_| Error::Shutdown)?; + response.await.map_err(|_| Error::Shutdown)? } /// Like [`subscribe`](Self::subscribe) but does not wait for the @@ -429,18 +443,21 @@ where /// # Errors /// /// Returns an error if sending the SD message fails. - /// - /// # Panics - /// - /// Panics if the internal control channel is closed. + /// Returns [`Error::Shutdown`] if the client's run-loop future has + /// exited before this call (dropped, cancelled, or otherwise gone) + /// — the `Client` handle has outlived its driver and further + /// control-channel sends cannot make progress. pub async fn send_sd_message( &self, target: SocketAddrV4, sd_header: ::SdHeader, ) -> Result<(), Error> { let (response, message) = ControlMessage::send_sd(target, sd_header); - self.control_sender.send(message).await.unwrap(); - response.await.unwrap() + self.control_sender + .send(message) + .await + .map_err(|_| Error::Shutdown)?; + response.await.map_err(|_| Error::Shutdown)? } /// Start periodic SD announcements on the client's discovery socket. @@ -573,10 +590,10 @@ where /// # Errors /// /// Returns an error if registering the endpoint fails. - /// - /// # Panics - /// - /// Panics if the internal control channel is closed. + /// Returns [`Error::Shutdown`] if the client's run-loop future has + /// exited before this call (dropped, cancelled, or otherwise gone) + /// — the `Client` handle has outlived its driver and further + /// control-channel sends cannot make progress. pub async fn add_endpoint( &self, service_id: u16, @@ -586,8 +603,11 @@ where ) -> Result<(), Error> { let (response, message) = ControlMessage::add_endpoint(service_id, instance_id, addr, local_port); - self.control_sender.send(message).await.unwrap(); - response.await.unwrap() + self.control_sender + .send(message) + .await + .map_err(|_| Error::Shutdown)?; + response.await.map_err(|_| Error::Shutdown)? } /// Removes a service endpoint from the client's endpoint registry. @@ -595,14 +615,17 @@ where /// # Errors /// /// Returns an error if removing the endpoint fails. - /// - /// # Panics - /// - /// Panics if the internal control channel is closed. + /// Returns [`Error::Shutdown`] if the client's run-loop future has + /// exited before this call (dropped, cancelled, or otherwise gone) + /// — the `Client` handle has outlived its driver and further + /// control-channel sends cannot make progress. pub async fn remove_endpoint(&self, service_id: u16, instance_id: u16) -> Result<(), Error> { let (response, message) = ControlMessage::remove_endpoint(service_id, instance_id); - self.control_sender.send(message).await.unwrap(); - response.await.unwrap() + self.control_sender + .send(message) + .await + .map_err(|_| Error::Shutdown)?; + response.await.map_err(|_| Error::Shutdown)? } /// Sends a message to a service and returns a handle to await the response. @@ -625,10 +648,10 @@ where /// /// Returns an error if the service is not found, unicast binding fails, /// or the UDP send fails. - /// - /// # Panics - /// - /// Panics if the internal control channel is closed. + /// Returns [`Error::Shutdown`] if the client's run-loop future has + /// exited before this call (dropped, cancelled, or otherwise gone) + /// — the `Client` handle has outlived its driver and further + /// control-channel sends cannot make progress. pub async fn send_to_service( &self, service_id: u16, @@ -637,8 +660,11 @@ where ) -> Result, Error> { let (send_rx, response_rx, ctrl_msg) = ControlMessage::send_to_service(service_id, instance_id, message); - self.control_sender.send(ctrl_msg).await.unwrap(); - send_rx.await.unwrap()?; + self.control_sender + .send(ctrl_msg) + .await + .map_err(|_| Error::Shutdown)?; + send_rx.await.map_err(|_| Error::Shutdown)??; Ok(PendingResponse { receiver: response_rx, }) @@ -654,10 +680,10 @@ where /// /// Returns an error if the service is not found, unicast binding fails, /// the UDP send fails, or the response payload fails to deserialize. - /// - /// # Panics - /// - /// Panics if the internal control channel is closed. + /// Returns [`Error::Shutdown`] if the client's run-loop future has + /// exited before this call (dropped, cancelled, or otherwise gone) + /// — the `Client` handle has outlived its driver and further + /// control-channel sends cannot make progress. pub async fn request( &self, service_id: u16, @@ -666,11 +692,12 @@ where ) -> Result { let (send_rx, response_rx, ctrl_msg) = ControlMessage::send_to_service(service_id, instance_id, message); - self.control_sender.send(ctrl_msg).await.unwrap(); - send_rx.await.unwrap()?; - response_rx + self.control_sender + .send(ctrl_msg) .await - .expect("inner loop dropped response channel") + .map_err(|_| Error::Shutdown)?; + send_rx.await.map_err(|_| Error::Shutdown)??; + response_rx.await.map_err(|_| Error::Shutdown)? } /// Register an E2E profile for the given key. @@ -1161,33 +1188,41 @@ mod tests { /// Documents the footgun: if the caller drops `run_fut` without ever /// polling it, the control channel's receiver goes with it and - /// subsequent `Client` method calls panic on `control_sender.send()`. + /// subsequent `Client` method calls return [`Error::Shutdown`] + /// rather than panicking. /// /// This is intrinsic to the caller-driven lifecycle introduced in /// phase 6 — the run loop is no longer owned by `Client::new`, so /// failing to spawn it is the caller's responsibility. The test /// pins the behavior deterministically so that any attempt to - /// silently "fix" this (e.g. internal spawn fallback) would break it - /// and force a review. + /// silently "fix" this (e.g. internal spawn fallback) would break + /// it and force a review. + /// + /// Prior to the phase-6 API change these call sites panicked on + /// `.unwrap()` of the send `Result`; the typed error surfaced here + /// lets library consumers observe lifecycle mismatches cleanly + /// instead of bringing down the caller's task. #[tokio::test] - #[should_panic(expected = "SendError")] - async fn dropping_run_future_without_spawn_panics_on_next_client_call() { + async fn dropping_run_future_without_spawn_returns_shutdown_error() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); // Caller explicitly discards the run loop. drop(run_fut); - // Any client method that enqueues a control message panics on - // `.unwrap()` of the send Result — document it instead of - // hiding it. - client.bind_discovery().await.unwrap(); + let err = client + .bind_discovery() + .await + .expect_err("must surface a typed error, not Ok or panic"); + assert!( + matches!(err, Error::Shutdown), + "expected Error::Shutdown after run-loop drop, got {err:?}", + ); } /// If the run loop is cancelled mid-poll (caller-initiated timeout, /// graceful shutdown), subsequent `Client` calls see the control - /// channel closed and surface a panic from `control_sender.send()`. - /// Same structural contract as dropping the run future. + /// channel closed and surface [`Error::Shutdown`]. Same structural + /// contract as dropping the run future. #[tokio::test] - #[should_panic(expected = "SendError")] - async fn cancelling_run_future_closes_control_channel() { + async fn cancelling_run_future_closes_control_channel_returns_shutdown_error() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); let handle = tokio::spawn(run_fut); // Let the loop start. @@ -1196,6 +1231,13 @@ mod tests { // Give the abort time to land. tokio::time::sleep(std::time::Duration::from_millis(50)).await; - client.bind_discovery().await.unwrap(); + let err = client + .bind_discovery() + .await + .expect_err("must surface a typed error, not Ok or panic"); + assert!( + matches!(err, Error::Shutdown), + "expected Error::Shutdown after run-loop cancel, got {err:?}", + ); } } diff --git a/src/lib.rs b/src/lib.rs index c516f9d..a721fbc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -67,7 +67,9 @@ //! #[tokio::main] //! async fn main() { //! // Client::new returns a Clone-able handle, an update stream, and -//! // the run-loop future. Spawn the future on any executor. +//! // the run-loop future. Spawn the future on the tokio runtime; +//! // the returned future depends on `tokio::select!` / `tokio::time` +//! // / tokio sockets, so it is not executor-agnostic today. //! let (client, mut updates, run) = Client::::new([192, 168, 1, 100].into()); //! tokio::spawn(run); //! client.bind_discovery().await.unwrap(); diff --git a/src/server/mod.rs b/src/server/mod.rs index e8ddc52..7db2633 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -1350,7 +1350,12 @@ mod tests { .expect("announcement_loop on a regular server must build"); // Spawn and immediately drop the handle — we only care that the // construction did not error here. - drop(tokio::spawn(fut)); + // Do NOT spawn: the announcer loops forever, and spawning + + // dropping the JoinHandle would leave the task running and + // emitting multicast for the rest of the test binary's + // lifetime, interfering with parallel tests that bind the same + // multicast group. We only care that construction returned Ok. + drop(fut); } #[tokio::test] @@ -1997,12 +2002,20 @@ mod tests { // construction returns Ok. Spawn + drop the JoinHandle — the // task rides the runtime lifecycle until the test's tokio // runtime shuts down at end-of-test. - drop(tokio::spawn(fut)); + // Do NOT spawn: the announcer loops forever, and spawning + + // dropping the JoinHandle would leave the task running and + // emitting multicast for the rest of the test binary's + // lifetime, interfering with parallel tests that bind the same + // multicast group. We only care that construction returned Ok. + drop(fut); } /// Direct test that `announcement_loop` actually emits an SD /// announcement when driven. Explicit coverage for the primary entry /// point (avoids regressions where only the deleted shim was exercised). + #[ignore = "requires MULTICAST on loopback; consistent with the \ + #[ignore]-gated sd_state.rs tests. Runs in any environment \ + where loopback multicast is available."] #[tokio::test] async fn announcement_loop_sends_offer_service_when_driven() { use crate::protocol::MessageId; @@ -2030,20 +2043,60 @@ mod tests { rs }; - let config = ServerConfig::new(iface, 30501, 0x005C, 0x0001); + // Use a distinct service/instance ID so parallel tests joined to + // the same SD multicast group do not produce false matches. + const SID: u16 = 0x005C; + const IID: u16 = 0x0001; + let config = ServerConfig::new(iface, 30501, SID, IID); let server = Server::new_with_loopback(config, true).await.unwrap(); let fut = server.announcement_loop().expect("build loop"); let handle = tokio::spawn(fut); + // Filter out any stray SD traffic from other parallel tests + // until we see one whose OfferService entry carries OUR sid/iid. + // Bounded by a single outer timeout so a totally-silent server + // (the regression we actually care about) still fails the test. let mut buf = [0u8; 1500]; - let (n, _src) = - tokio::time::timeout(std::time::Duration::from_secs(3), recv.recv_from(&mut buf)) - .await - .expect("timed out waiting for announcement") - .expect("recv failed"); - - let view = crate::protocol::MessageView::parse(&buf[..n]).unwrap(); - assert_eq!(view.header().message_id(), MessageId::SD); + let offer_fields = tokio::time::timeout(std::time::Duration::from_secs(3), async { + loop { + let (n, _src) = recv.recv_from(&mut buf).await.expect("recv failed"); + let Ok(view) = crate::protocol::MessageView::parse(&buf[..n]) else { + continue; + }; + if view.header().message_id() != MessageId::SD { + continue; + } + let Ok(sd_view) = view.sd_header() else { + continue; + }; + let Some(entry) = sd_view.entries().next() else { + continue; + }; + if !matches!(entry.entry_type(), Ok(sd::EntryType::OfferService)) { + continue; + } + if entry.service_id() != SID || entry.instance_id() != IID { + continue; + } + break ( + entry.service_id(), + entry.instance_id(), + entry.major_version(), + entry.ttl(), + ); + } + }) + .await + .expect("timed out waiting for our OfferService"); + + let (svc, inst, major, ttl) = offer_fields; + assert_eq!(svc, SID, "emitted service_id must match server config"); + assert_eq!(inst, IID, "emitted instance_id must match server config"); + assert_eq!(major, 1, "default major_version from ServerConfig::new"); + assert!( + ttl > 0, + "OfferService TTL must be non-zero (TTL=0 means StopOffering)", + ); handle.abort(); } From 7a8d2d45b478c7c2d4590e28214cf9ed4cd73cac Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Thu, 23 Apr 2026 14:31:16 -0400 Subject: [PATCH 04/14] lints: address must_use + README wording (Copilot round-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses 8 Copilot review comments on PR #80. No structural changes; purely hygiene + doc accuracy. must_use on tokio::spawn(run[_fut]) — `tokio::spawn` returns a `#[must_use]` `JoinHandle`, so bare-statement calls trip the lint. Bound the handle to `let _ = ...` in tests (where we don't want to keep the handle alive) and `let _run_task`/`let _run_handle = ...` in examples and the crate-level doctest (reviewer-suggested wording). Grep confirmed the reviewer's "pattern repeats throughout" hint: 20 sites in src/client/mod.rs tests, 22 in src/client/inner.rs tests, 12 in tests/client_server.rs, plus the 4 explicitly-cited singletons (src/lib.rs doctest, examples/discovery_client, examples/client_server, the one inner.rs site named by line number). All 58 sites fixed. src/server/mod.rs comment cleanup — the announcement_loop test's comment block said "Spawn and immediately drop the handle" followed by "Do NOT spawn", which is self-contradictory since the earlier refactor switched from `drop(tokio::spawn(fut))` to plain `drop(fut)`. Rewrote to describe the actual behavior: we construct the future, assert Ok, and drop it without polling, because spawning would leave the announcer emitting multicast for the remainder of the test binary. README.md correction — the Client quick-start claimed control-channel calls return `Error::Shutdown` if the run-loop future isn't spawned. That is inaccurate: if the future exists but is never polled, the control channel's sender succeeds and the caller hangs forever on the oneshot response. `Shutdown` is only produced once the run-loop future has been dropped or its task cancelled. Rewrote the passage to state that the future must be actively driven and that hangs (not `Shutdown`) are what happens when it isn't. Co-Authored-By: Claude Opus 4.7 --- README.md | 12 +++++--- examples/client_server/src/main.rs | 2 +- examples/discovery_client/src/main.rs | 2 +- src/client/inner.rs | 44 +++++++++++++-------------- src/client/mod.rs | 40 ++++++++++++------------ src/lib.rs | 2 +- src/server/mod.rs | 13 ++++---- tests/client_server.rs | 24 +++++++-------- 8 files changed, 71 insertions(+), 68 deletions(-) diff --git a/README.md b/README.md index 5fd13a6..26e4019 100644 --- a/README.md +++ b/README.md @@ -67,12 +67,16 @@ use std::net::Ipv4Addr; #[tokio::main] async fn main() { // Client::new returns a Clone-able handle, an update stream, and - // the run-loop future. Spawn the future on the tokio runtime; - // without it the control channel has no driver and Client method - // calls will return `Error::Shutdown`. + // the run-loop future. The future must be actively driven — either + // spawned on the runtime as shown below, or awaited alongside your + // own work in a `tokio::select!`. If the future is never polled, + // Client method calls that send commands over the control channel + // will hang indefinitely waiting on their oneshot response. + // `Error::Shutdown` is returned only once the run-loop future has + // been dropped or its task cancelled. let (client, mut updates, run) = Client::::new(Ipv4Addr::new(192, 168, 1, 100)); - tokio::spawn(run); + let _run_task = tokio::spawn(run); // Bind the SD multicast socket to discover services client.bind_discovery().await.unwrap(); diff --git a/examples/client_server/src/main.rs b/examples/client_server/src/main.rs index 0353dd6..a1807f7 100644 --- a/examples/client_server/src/main.rs +++ b/examples/client_server/src/main.rs @@ -107,7 +107,7 @@ async fn main() -> Result<(), Box> { // ── Create the client (handles discovery, subscriptions, SD socket) ── let (client, mut updates, run) = simple_someip::Client::::new(interface); - tokio::spawn(run); + let _ = tokio::spawn(run); client.bind_discovery().await?; info!("Client discovery bound"); diff --git a/examples/discovery_client/src/main.rs b/examples/discovery_client/src/main.rs index d032702..ae866bf 100644 --- a/examples/discovery_client/src/main.rs +++ b/examples/discovery_client/src/main.rs @@ -288,7 +288,7 @@ async fn main() -> Result<(), Error> { info!("Starting discovery client on interface {interface}"); let (client, mut updates, run) = simple_someip::Client::::new(interface); - tokio::spawn(run); + let _run_handle = tokio::spawn(run); client.bind_discovery().await.unwrap(); let mut state = DiscoveryState::new(); diff --git a/src/client/inner.rs b/src/client/inner.rs index 3b6ed72..e1829f2 100644 --- a/src/client/inner.rs +++ b/src/client/inner.rs @@ -1382,7 +1382,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Drop control sender to trigger loop exit drop(control_sender); // The update receiver should eventually return None when the inner loop exits @@ -1417,7 +1417,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::bind_discovery(); drop(rx); @@ -1434,7 +1434,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::unbind_discovery(); drop(rx); @@ -1451,7 +1451,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // SetInterface(LOCALHOST) on a fresh inner goes straight to // bind_discovery + send response (interface already matches). @@ -1470,7 +1470,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Bind discovery first so the SendSD path has a socket to use let (rx, msg) = TestControl::bind_discovery(); @@ -1500,7 +1500,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Bind discovery so SetInterface will take the multi-step path: // iteration 1: unbind discovery, re-queue SetInterface @@ -1571,7 +1571,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); let (rx, msg) = TestControl::add_endpoint(0x1234, 0x0001, addr, 0); @@ -1589,7 +1589,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::remove_endpoint(0x1234, 0x0001); drop(rx); @@ -1606,7 +1606,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Add an endpoint first so SendToService doesn't fail with ServiceNotFound let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); @@ -1633,7 +1633,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), true, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::bind_discovery(); control_sender.send(msg).await.unwrap(); @@ -1648,7 +1648,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::bind_discovery(); control_sender.send(msg).await.unwrap(); @@ -1668,7 +1668,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let target = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 30490); let sd_header = empty_sd_header(); @@ -1689,7 +1689,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); let (rx, msg) = TestControl::add_endpoint(0x1234, 0x0001, addr, 0); @@ -1714,7 +1714,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Bind discovery first let (rx, msg) = TestControl::bind_discovery(); @@ -1745,7 +1745,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Add endpoint but do NOT bind discovery let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); @@ -1770,7 +1770,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::subscribe(0xFFFF, 0xFFFF, 1, 3, 0x01, 0); control_sender.send(msg).await.unwrap(); @@ -1789,7 +1789,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); let (rx, msg) = TestControl::add_endpoint(0x1234, 0x0001, addr, 0); @@ -1824,7 +1824,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::subscribe(0x1234, 0x0001, 1, 3, 0x01, 0); drop(rx); @@ -1842,7 +1842,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Change to a different loopback-range address (127.0.0.2). // Binding discovery on 127.0.0.2 should succeed on most systems. @@ -1867,7 +1867,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Bind discovery on LOCALHOST first let (rx, msg) = TestControl::bind_discovery(); @@ -1898,7 +1898,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Add endpoint and bind discovery let (rx, msg) = TestControl::bind_discovery(); @@ -1945,7 +1945,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let raw = UdpSocket::bind("127.0.0.1:0").await.unwrap(); let target = SocketAddrV4::new(Ipv4Addr::LOCALHOST, raw.local_addr().unwrap().port()); diff --git a/src/client/mod.rs b/src/client/mod.rs index acf5843..c0fca6f 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -750,7 +750,7 @@ mod tests { #[tokio::test] async fn test_client_new_and_interface() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); assert_eq!(client.interface(), Ipv4Addr::LOCALHOST); client.shut_down(); } @@ -758,7 +758,7 @@ mod tests { #[tokio::test] async fn test_client_debug() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let debug_str = format!("{client:?}"); assert!(debug_str.contains("Client")); assert!(debug_str.contains("127.0.0.1")); @@ -805,7 +805,7 @@ mod tests { #[tokio::test] async fn test_subscribe_unknown_service_returns_error() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let result = client.subscribe(0xFFFF, 0xFFFF, 1, 3, 0x01, 0).await; assert!( matches!(result, Err(Error::ServiceNotFound)), @@ -817,7 +817,7 @@ mod tests { #[tokio::test] async fn test_subscribe_no_wait_unknown_service_does_not_panic() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // subscribe_no_wait is fire-and-forget — it should not panic even // when the service is unknown (the inner loop sends ServiceNotFound // on the dropped response channel, which is harmless). @@ -830,7 +830,7 @@ mod tests { #[tokio::test] async fn test_bind_discovery_and_unbind() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); client.unbind_discovery().await.unwrap(); client.shut_down(); @@ -839,7 +839,7 @@ mod tests { #[tokio::test] async fn test_set_interface() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let new_addr = Ipv4Addr::LOCALHOST; client.set_interface(new_addr).await.unwrap(); assert_eq!(client.interface(), new_addr); @@ -849,7 +849,7 @@ mod tests { #[tokio::test] async fn test_add_endpoint_succeeds() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::new(192, 168, 1, 1), 30000); client.add_endpoint(0x1234, 0x0001, addr, 0).await.unwrap(); client.shut_down(); @@ -858,7 +858,7 @@ mod tests { #[tokio::test] async fn test_send_to_service_unknown_returns_error() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let msg = crate::protocol::Message::new_sd(1, &empty_sd_header()); let result = client.send_to_service(0xFFFF, 0xFFFF, msg).await; assert!( @@ -871,7 +871,7 @@ mod tests { #[tokio::test] async fn test_remove_endpoint_succeeds() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::new(192, 168, 1, 1), 30000); client.add_endpoint(0x1234, 0x0001, addr, 0).await.unwrap(); client.remove_endpoint(0x1234, 0x0001).await.unwrap(); @@ -913,7 +913,7 @@ mod tests { #[tokio::test] async fn test_send_sd_message() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Bind discovery first so the send path uses the existing socket client.bind_discovery().await.unwrap(); let target = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 30490); @@ -925,7 +925,7 @@ mod tests { #[tokio::test] async fn test_send_to_service_success_returns_pending_response() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 30000); client.add_endpoint(0x1234, 0x0001, addr, 0).await.unwrap(); let msg = crate::protocol::Message::new_sd(1, &empty_sd_header()); @@ -938,7 +938,7 @@ mod tests { #[tokio::test] async fn test_recv_returns_none_after_shutdown() { let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.shut_down(); // Now the inner loop should exit; recv() should return None let result = tokio::time::timeout(std::time::Duration::from_secs(2), updates.recv()).await; @@ -949,7 +949,7 @@ mod tests { #[tokio::test] async fn test_register_and_unregister_e2e() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let key = E2EKey { service_id: 0x1234, method_or_event_id: 0x0001, @@ -963,7 +963,7 @@ mod tests { #[tokio::test] async fn test_client_is_clone() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let client2 = client.clone(); assert_eq!(client.interface(), client2.interface()); client.shut_down(); @@ -972,7 +972,7 @@ mod tests { #[tokio::test] async fn test_client_updates_debug() { let (_client, updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let debug_str = format!("{updates:?}"); assert!(debug_str.contains("ClientUpdates")); } @@ -980,7 +980,7 @@ mod tests { #[tokio::test] async fn test_request_unknown_service_returns_error() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let msg = crate::protocol::Message::new_sd(1, &empty_sd_header()); let result = client.request(0xFFFF, 0xFFFF, msg).await; assert!( @@ -993,7 +993,7 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_does_not_panic() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); let sd_header = empty_sd_header(); @@ -1017,7 +1017,7 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_without_discovery_bound() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Don't bind discovery — the task should handle the error gracefully. let sd_header = empty_sd_header(); let handle = @@ -1039,7 +1039,7 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_abort_stops_task() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); let sd_header = empty_sd_header(); @@ -1165,7 +1165,7 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_stops_on_shutdown() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); let sd_header = empty_sd_header(); diff --git a/src/lib.rs b/src/lib.rs index a721fbc..96c3d15 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -71,7 +71,7 @@ //! // the returned future depends on `tokio::select!` / `tokio::time` //! // / tokio sockets, so it is not executor-agnostic today. //! let (client, mut updates, run) = Client::::new([192, 168, 1, 100].into()); -//! tokio::spawn(run); +//! let _run_task = tokio::spawn(run); //! client.bind_discovery().await.unwrap(); //! //! while let Some(update) = updates.recv().await { diff --git a/src/server/mod.rs b/src/server/mod.rs index 7db2633..ac45ea7 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -1348,13 +1348,12 @@ mod tests { let fut = server2 .announcement_loop() .expect("announcement_loop on a regular server must build"); - // Spawn and immediately drop the handle — we only care that the - // construction did not error here. - // Do NOT spawn: the announcer loops forever, and spawning + - // dropping the JoinHandle would leave the task running and - // emitting multicast for the rest of the test binary's - // lifetime, interfering with parallel tests that bind the same - // multicast group. We only care that construction returned Ok. + // Intentionally do not poll or spawn the future: we only care + // that constructing it returned Ok. If this future were + // spawned, the announcer would loop indefinitely and emit + // multicast until explicitly aborted or the Tokio runtime + // shut down at end-of-test, which could interfere with + // parallel tests using the same multicast group. drop(fut); } diff --git a/tests/client_server.rs b/tests/client_server.rs index d48fee1..e4b544e 100644 --- a/tests/client_server.rs +++ b/tests/client_server.rs @@ -57,7 +57,7 @@ async fn test_client_server_subscribe_and_receive_event() { // Create client and subscribe to the server's event group let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); @@ -101,7 +101,7 @@ async fn test_client_send_sd_auto_binds_discovery() { // Create client — NO bind_discovery let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // send_sd_message should auto-bind discovery and succeed let sd_header = VecSdHeader { @@ -133,7 +133,7 @@ async fn test_client_bind_unbind_lifecycle_with_server() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Bind discovery, subscribe, then unbind and rebind client.bind_discovery().await.unwrap(); @@ -162,7 +162,7 @@ async fn test_add_endpoint_and_send_to_service() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); // Register the server's endpoint manually (simulating non-broadcasting service) @@ -223,7 +223,7 @@ async fn test_subscribe_auto_binds_discovery() { // Create client — do NOT bind discovery manually let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); // Subscribe should auto-bind discovery internally @@ -266,7 +266,7 @@ async fn test_client_request_resolves_via_unicast_reply() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); @@ -328,7 +328,7 @@ async fn test_e2e_protect_on_publish_and_check_on_receive() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Register matching E2E profile on client client.register_e2e(key, profile); @@ -393,13 +393,13 @@ async fn test_multiple_subscribers_receive_events() { // Client 1 let (client1, mut updates1, run_fut1) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut1); + let _ = tokio::spawn(run_fut1); client1.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client1.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); // Client 2 let (client2, mut updates2, run_fut2) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut2); + let _ = tokio::spawn(run_fut2); client2.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client2.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); @@ -453,7 +453,7 @@ async fn test_multiple_subscribers_receive_events() { #[tokio::test] async fn test_updates_drain_after_shutdown() { let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.shut_down(); let result = tokio::time::timeout(std::time::Duration::from_secs(2), updates.recv()) @@ -469,7 +469,7 @@ async fn test_cloned_client_works() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let client2 = client.clone(); // Both clones can send commands @@ -490,7 +490,7 @@ async fn test_subscribe_specific_port_reuse() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); From a1a01ed6ff0f9a628813957ad82fc9b0ae4acdd7 Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Thu, 23 Apr 2026 15:20:02 -0400 Subject: [PATCH 05/14] chore(clippy): address new warnings in hoist_tokio PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix 64 clippy warnings introduced by this branch's own commits: - let_underscore_future (54): drop the 'let _ =' prefix on 'tokio::spawn(run_fut)' call sites across client/mod.rs, client/inner.rs tests, and tests/client_server.rs — JoinHandle is not #[must_use], so bare expression statements are fine. - new_ret_no_self on Inner::new: rename to Inner::build, since after the spawn-hoist the method returns (sender, receiver, future) rather than a constructed Self. Internal API only (Inner is pub(super)). - manual_async_fn on Inner::run_future: rewrite as async fn; captured state is already Send + 'static so the inferred future keeps the bounds the caller relies on. - double_must_use on Client::{new, new_with_loopback}: swap the bare #[must_use] for a message pointing out that the returned future in the tuple must be spawned. - items_after_statements in announcement_loop test: hoist SID/IID consts to the top of the fn. --- src/client/inner.rs | 282 ++++++++++++++++++++--------------------- src/client/mod.rs | 56 ++++---- src/server/mod.rs | 16 ++- tests/client_server.rs | 24 ++-- 4 files changed, 192 insertions(+), 186 deletions(-) diff --git a/src/client/inner.rs b/src/client/inner.rs index e1829f2..a59cc88 100644 --- a/src/client/inner.rs +++ b/src/client/inner.rs @@ -354,7 +354,7 @@ where /// is already Send. A bare-metal consumer whose transport produces /// `!Send` state needs a cfg-gated alternative constructor; none /// exists yet — it's planned alongside the bare-metal port. - pub fn new( + pub fn build( interface: Ipv4Addr, e2e_registry: Arc>, multicast_loopback: bool, @@ -929,115 +929,115 @@ where } } // Receive a discovery message - discovery = Inner::receive_discovery(discovery_socket) => { - trace!("Received discovery message: {:?}", discovery); - match discovery { - Ok((source, someip_header, sd_header)) => { - // Extract session ID from SOME/IP request_id (lower 16 bits) - let session_id = (someip_header.request_id() & 0xFFFF) as u16; - let sd_payload = PayloadDefinitions::new_sd_payload(&sd_header); - // Extract reboot flag from the SD payload flags - let reboot_flag = sd_payload - .sd_flags() - .map_or(crate::protocol::sd::RebootFlag::Continuous, |f| { - f.reboot() - }); - - // Track sender session/reboot state for every SD entry - // that identifies a service instance, not only - // offer/stop-offer entries. This ensures reboot - // detection works for all SD traffic (FindService, - // Subscribe, SubscribeAck, etc.). - let mut rebooted = false; - for (svc_id, inst_id) in sd_payload.service_instances() { - let verdict = session_tracker.check( - source, - TransportKind::Multicast, - svc_id, - inst_id, - session_id, - reboot_flag, - ); - if verdict == SessionVerdict::Reboot { - rebooted = true; - } + discovery = Inner::receive_discovery(discovery_socket) => { + trace!("Received discovery message: {:?}", discovery); + match discovery { + Ok((source, someip_header, sd_header)) => { + // Extract session ID from SOME/IP request_id (lower 16 bits) + let session_id = (someip_header.request_id() & 0xFFFF) as u16; + let sd_payload = PayloadDefinitions::new_sd_payload(&sd_header); + // Extract reboot flag from the SD payload flags + let reboot_flag = sd_payload + .sd_flags() + .map_or(crate::protocol::sd::RebootFlag::Continuous, |f| { + f.reboot() + }); + + // Track sender session/reboot state for every SD entry + // that identifies a service instance, not only + // offer/stop-offer entries. This ensures reboot + // detection works for all SD traffic (FindService, + // Subscribe, SubscribeAck, etc.). + let mut rebooted = false; + for (svc_id, inst_id) in sd_payload.service_instances() { + let verdict = session_tracker.check( + source, + TransportKind::Multicast, + svc_id, + inst_id, + session_id, + reboot_flag, + ); + if verdict == SessionVerdict::Reboot { + rebooted = true; } + } - // Auto-populate service registry from offer/stop-offer - // SD entries. - for ep in sd_payload.offered_endpoints() { - let id = ServiceInstanceId { - service_id: ep.service_id, - instance_id: ep.instance_id, - }; - if ep.is_offer { - if let Some(addr) = ep.addr { - service_registry.insert( - id, - ServiceEndpointInfo { - addr, - local_port: 0, - major_version: ep.major_version, - minor_version: ep.minor_version, - }, - ); - trace!( - "Registry: added 0x{:04X}.0x{:04X} -> {}", - ep.service_id, ep.instance_id, addr, - ); - } - } else { - service_registry.remove(id); + // Auto-populate service registry from offer/stop-offer + // SD entries. + for ep in sd_payload.offered_endpoints() { + let id = ServiceInstanceId { + service_id: ep.service_id, + instance_id: ep.instance_id, + }; + if ep.is_offer { + if let Some(addr) = ep.addr { + service_registry.insert( + id, + ServiceEndpointInfo { + addr, + local_port: 0, + major_version: ep.major_version, + minor_version: ep.minor_version, + }, + ); trace!( - "Registry: removed 0x{:04X}.0x{:04X}", - ep.service_id, ep.instance_id, + "Registry: added 0x{:04X}.0x{:04X} -> {}", + ep.service_id, ep.instance_id, addr, ); } + } else { + service_registry.remove(id); + trace!( + "Registry: removed 0x{:04X}.0x{:04X}", + ep.service_id, ep.instance_id, + ); } - - if rebooted { - let _ = update_sender.send(ClientUpdate::SenderRebooted(source)); - } - - let discovery_msg = DiscoveryMessage { - source, - someip_header, - sd_header, - }; - let _ = update_sender.send(ClientUpdate::DiscoveryUpdated(discovery_msg)); } - Err(err) => { - error!("Error receiving discovery message: {:?}", err); - let _ = update_sender.send(ClientUpdate::Error(err)); + + if rebooted { + let _ = update_sender.send(ClientUpdate::SenderRebooted(source)); } + + let discovery_msg = DiscoveryMessage { + source, + someip_header, + sd_header, + }; + let _ = update_sender.send(ClientUpdate::DiscoveryUpdated(discovery_msg)); } - } - unicast = Inner::receive_any_unicast(unicast_sockets) => { - trace!("Received unicast message: {:?}", unicast); - match unicast { - Ok(received) => { - let ReceivedMessage { message: received_message, e2e_status, .. } = received; - // Check if this matches a pending request-response by request_id - let request_id = received_message.header().request_id(); - if let Some(sender) = pending_responses.remove(&request_id) { - let _ = sender.send(Ok(received_message.payload().clone())); - continue; - } - // Not a response — forward as ClientUpdate::Unicast - let _ = update_sender.send(ClientUpdate::Unicast { message: received_message, e2e_status }); - } - Err(err) => { - let _ = update_sender.send(ClientUpdate::Error(err)); + Err(err) => { + error!("Error receiving discovery message: {:?}", err); + let _ = update_sender.send(ClientUpdate::Error(err)); + } + } + } + unicast = Inner::receive_any_unicast(unicast_sockets) => { + trace!("Received unicast message: {:?}", unicast); + match unicast { + Ok(received) => { + let ReceivedMessage { message: received_message, e2e_status, .. } = received; + // Check if this matches a pending request-response by request_id + let request_id = received_message.header().request_id(); + if let Some(sender) = pending_responses.remove(&request_id) { + let _ = sender.send(Ok(received_message.payload().clone())); + continue; } + // Not a response — forward as ClientUpdate::Unicast + let _ = update_sender.send(ClientUpdate::Unicast { message: received_message, e2e_status }); + } + Err(err) => { + let _ = update_sender.send(ClientUpdate::Error(err)); } } - } - if !*run { - info!("SOME/IP Client processing loop exiting"); - break; - } - self.handle_control_message().await; + } + } + if !*run { + info!("SOME/IP Client processing loop exiting"); + break; } + self.handle_control_message().await; + } } } } @@ -1377,12 +1377,12 @@ mod tests { #[tokio::test] async fn test_inner_spawn_and_shutdown() { - let (control_sender, mut update_receiver, run_fut) = Inner::::new( + let (control_sender, mut update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // Drop control sender to trigger loop exit drop(control_sender); // The update receiver should eventually return None when the inner loop exits @@ -1412,12 +1412,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_bind_discovery_continues() { - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let (rx, msg) = TestControl::bind_discovery(); drop(rx); @@ -1429,12 +1429,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_unbind_discovery_continues() { - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let (rx, msg) = TestControl::unbind_discovery(); drop(rx); @@ -1446,12 +1446,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_set_interface_continues() { - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // SetInterface(LOCALHOST) on a fresh inner goes straight to // bind_discovery + send response (interface already matches). @@ -1465,12 +1465,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_send_sd_continues() { - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // Bind discovery first so the SendSD path has a socket to use let (rx, msg) = TestControl::bind_discovery(); @@ -1495,12 +1495,12 @@ mod tests { #[tokio::test] async fn test_queued_messages_all_complete() { - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // Bind discovery so SetInterface will take the multi-step path: // iteration 1: unbind discovery, re-queue SetInterface @@ -1566,12 +1566,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_add_endpoint_continues() { - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); let (rx, msg) = TestControl::add_endpoint(0x1234, 0x0001, addr, 0); @@ -1584,12 +1584,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_remove_endpoint_continues() { - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let (rx, msg) = TestControl::remove_endpoint(0x1234, 0x0001); drop(rx); @@ -1601,12 +1601,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_send_to_service_send_complete_continues() { - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // Add an endpoint first so SendToService doesn't fail with ServiceNotFound let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); @@ -1628,12 +1628,12 @@ mod tests { async fn test_bind_discovery_with_loopback() { // Spawn inner with multicast_loopback=true so bind_discovery exercises // the loopback-enabled branch of SocketManager::bind_discovery. - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), true, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let (rx, msg) = TestControl::bind_discovery(); control_sender.send(msg).await.unwrap(); @@ -1643,12 +1643,12 @@ mod tests { #[tokio::test] async fn test_bind_discovery_idempotent() { // Binding discovery twice should succeed (early return on already-bound) - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let (rx, msg) = TestControl::bind_discovery(); control_sender.send(msg).await.unwrap(); @@ -1663,12 +1663,12 @@ mod tests { #[tokio::test] async fn test_send_sd_auto_binds_discovery() { // SendSD without a bound discovery socket should auto-bind and succeed - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let target = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 30490); let sd_header = empty_sd_header(); @@ -1684,12 +1684,12 @@ mod tests { #[tokio::test] async fn test_send_to_service_auto_binds_unicast() { // SendToService with no unicast sockets should auto-bind ephemeral - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); let (rx, msg) = TestControl::add_endpoint(0x1234, 0x0001, addr, 0); @@ -1709,12 +1709,12 @@ mod tests { #[tokio::test] async fn test_subscribe_with_endpoint_sends_sd() { // Subscribe with a known endpoint and bound discovery should send the SD message - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // Bind discovery first let (rx, msg) = TestControl::bind_discovery(); @@ -1740,12 +1740,12 @@ mod tests { #[tokio::test] async fn test_subscribe_auto_binds_discovery() { // Subscribe without discovery bound should auto-bind and succeed - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // Add endpoint but do NOT bind discovery let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); @@ -1765,12 +1765,12 @@ mod tests { #[tokio::test] async fn test_subscribe_unknown_service_returns_error() { - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let (rx, msg) = TestControl::subscribe(0xFFFF, 0xFFFF, 1, 3, 0x01, 0); control_sender.send(msg).await.unwrap(); @@ -1784,12 +1784,12 @@ mod tests { #[tokio::test] async fn test_send_to_service_reuses_existing_unicast_socket() { // When a unicast socket already exists, SendToService should reuse it - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); let (rx, msg) = TestControl::add_endpoint(0x1234, 0x0001, addr, 0); @@ -1819,12 +1819,12 @@ mod tests { #[tokio::test] async fn test_dropped_receiver_subscribe_service_not_found_continues() { // Subscribe with no endpoint → ServiceNotFound response is dropped - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let (rx, msg) = TestControl::subscribe(0x1234, 0x0001, 1, 3, 0x01, 0); drop(rx); @@ -1837,12 +1837,12 @@ mod tests { #[tokio::test] async fn test_set_interface_changes_interface() { // SetInterface to a different address exercises the interface!=current path - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // Change to a different loopback-range address (127.0.0.2). // Binding discovery on 127.0.0.2 should succeed on most systems. @@ -1862,12 +1862,12 @@ mod tests { #[tokio::test] async fn test_set_interface_with_discovery_bound_changes_interface() { // SetInterface when discovery is already bound: unbind → change → rebind - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // Bind discovery on LOCALHOST first let (rx, msg) = TestControl::bind_discovery(); @@ -1893,12 +1893,12 @@ mod tests { async fn test_subscribe_specific_port_reuse() { // Subscribe twice with the same specific client_port exercises the // bind_unicast port-reuse path (port != 0 && already bound). - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // Add endpoint and bind discovery let (rx, msg) = TestControl::bind_discovery(); @@ -1940,12 +1940,12 @@ mod tests { use std::vec; use tokio::net::UdpSocket; - let (control_sender, _update_receiver, run_fut) = Inner::::new( + let (control_sender, _update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), false, ); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let raw = UdpSocket::bind("127.0.0.1:0").await.unwrap(); let target = SocketAddrV4::new(Ipv4Addr::LOCALHOST, raw.local_addr().unwrap().port()); diff --git a/src/client/mod.rs b/src/client/mod.rs index c0fca6f..e7aec43 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -204,7 +204,7 @@ where /// # let _ = (client, updates); /// # } /// ``` - #[must_use] + #[must_use = "the returned run-loop future must be spawned (e.g. tokio::spawn) for the client to make progress"] pub fn new( interface: Ipv4Addr, ) -> ( @@ -238,7 +238,7 @@ where /// Consumers of [`ClientUpdates`] that need to ignore self-sent SD should /// filter on source address (the sender's IP/port is included on the /// update). - #[must_use] + #[must_use = "the returned run-loop future must be spawned (e.g. tokio::spawn) for the client to make progress"] pub fn new_with_loopback( interface: Ipv4Addr, multicast_loopback: bool, @@ -249,7 +249,7 @@ where ) { let e2e_registry = Arc::new(Mutex::new(E2ERegistry::new())); let (control_sender, update_receiver, run_future) = - Inner::new(interface, Arc::clone(&e2e_registry), multicast_loopback); + Inner::build(interface, Arc::clone(&e2e_registry), multicast_loopback); let client = Self { interface: Arc::new(RwLock::new(interface)), @@ -750,7 +750,7 @@ mod tests { #[tokio::test] async fn test_client_new_and_interface() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); assert_eq!(client.interface(), Ipv4Addr::LOCALHOST); client.shut_down(); } @@ -758,7 +758,7 @@ mod tests { #[tokio::test] async fn test_client_debug() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let debug_str = format!("{client:?}"); assert!(debug_str.contains("Client")); assert!(debug_str.contains("127.0.0.1")); @@ -805,7 +805,7 @@ mod tests { #[tokio::test] async fn test_subscribe_unknown_service_returns_error() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let result = client.subscribe(0xFFFF, 0xFFFF, 1, 3, 0x01, 0).await; assert!( matches!(result, Err(Error::ServiceNotFound)), @@ -817,7 +817,7 @@ mod tests { #[tokio::test] async fn test_subscribe_no_wait_unknown_service_does_not_panic() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // subscribe_no_wait is fire-and-forget — it should not panic even // when the service is unknown (the inner loop sends ServiceNotFound // on the dropped response channel, which is harmless). @@ -830,7 +830,7 @@ mod tests { #[tokio::test] async fn test_bind_discovery_and_unbind() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); client.unbind_discovery().await.unwrap(); client.shut_down(); @@ -839,7 +839,7 @@ mod tests { #[tokio::test] async fn test_set_interface() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let new_addr = Ipv4Addr::LOCALHOST; client.set_interface(new_addr).await.unwrap(); assert_eq!(client.interface(), new_addr); @@ -849,7 +849,7 @@ mod tests { #[tokio::test] async fn test_add_endpoint_succeeds() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::new(192, 168, 1, 1), 30000); client.add_endpoint(0x1234, 0x0001, addr, 0).await.unwrap(); client.shut_down(); @@ -858,7 +858,7 @@ mod tests { #[tokio::test] async fn test_send_to_service_unknown_returns_error() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let msg = crate::protocol::Message::new_sd(1, &empty_sd_header()); let result = client.send_to_service(0xFFFF, 0xFFFF, msg).await; assert!( @@ -871,7 +871,7 @@ mod tests { #[tokio::test] async fn test_remove_endpoint_succeeds() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::new(192, 168, 1, 1), 30000); client.add_endpoint(0x1234, 0x0001, addr, 0).await.unwrap(); client.remove_endpoint(0x1234, 0x0001).await.unwrap(); @@ -913,7 +913,7 @@ mod tests { #[tokio::test] async fn test_send_sd_message() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // Bind discovery first so the send path uses the existing socket client.bind_discovery().await.unwrap(); let target = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 30490); @@ -925,7 +925,7 @@ mod tests { #[tokio::test] async fn test_send_to_service_success_returns_pending_response() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 30000); client.add_endpoint(0x1234, 0x0001, addr, 0).await.unwrap(); let msg = crate::protocol::Message::new_sd(1, &empty_sd_header()); @@ -938,7 +938,7 @@ mod tests { #[tokio::test] async fn test_recv_returns_none_after_shutdown() { let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); client.shut_down(); // Now the inner loop should exit; recv() should return None let result = tokio::time::timeout(std::time::Duration::from_secs(2), updates.recv()).await; @@ -949,7 +949,7 @@ mod tests { #[tokio::test] async fn test_register_and_unregister_e2e() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let key = E2EKey { service_id: 0x1234, method_or_event_id: 0x0001, @@ -963,7 +963,7 @@ mod tests { #[tokio::test] async fn test_client_is_clone() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let client2 = client.clone(); assert_eq!(client.interface(), client2.interface()); client.shut_down(); @@ -972,7 +972,7 @@ mod tests { #[tokio::test] async fn test_client_updates_debug() { let (_client, updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let debug_str = format!("{updates:?}"); assert!(debug_str.contains("ClientUpdates")); } @@ -980,7 +980,7 @@ mod tests { #[tokio::test] async fn test_request_unknown_service_returns_error() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let msg = crate::protocol::Message::new_sd(1, &empty_sd_header()); let result = client.request(0xFFFF, 0xFFFF, msg).await; assert!( @@ -993,7 +993,7 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_does_not_panic() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); let sd_header = empty_sd_header(); @@ -1017,7 +1017,7 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_without_discovery_bound() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // Don't bind discovery — the task should handle the error gracefully. let sd_header = empty_sd_header(); let handle = @@ -1039,7 +1039,7 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_abort_stops_task() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); let sd_header = empty_sd_header(); @@ -1065,7 +1065,9 @@ mod tests { // session counter has not wrapped on a freshly-bound socket). This // verifies the announcer calls `set_reboot_flag` on each tick rather // than using the stale caller-supplied value. - let (client, mut updates) = TestClient::new_with_loopback(Ipv4Addr::LOCALHOST, true); + let (client, mut updates, run_fut) = + TestClient::new_with_loopback(Ipv4Addr::LOCALHOST, true); + tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); // Caller bakes in Continuous — the announcer must override this. @@ -1114,7 +1116,8 @@ mod tests { // past 0xFFFF would regress to `RecentlyRebooted` on the next // `reboot_flag()` call after unbind — falsely advertising a reboot // to peers on the next manually-built SD header. - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); // No discovery bound. Fallback should reflect persisted state. // Default (unwrapped) → RecentlyRebooted. @@ -1147,7 +1150,8 @@ mod tests { #[tokio::test] async fn test_reboot_flag_defaults_to_recently_rebooted() { - let (client, _updates) = TestClient::new(Ipv4Addr::LOCALHOST); + let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); + tokio::spawn(run_fut); // Discovery not bound — should fall back to RecentlyRebooted. assert_eq!( client.reboot_flag().await, @@ -1165,7 +1169,7 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_stops_on_shutdown() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); let sd_header = empty_sd_header(); diff --git a/src/server/mod.rs b/src/server/mod.rs index ac45ea7..761aa87 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -2019,6 +2019,11 @@ mod tests { async fn announcement_loop_sends_offer_service_when_driven() { use crate::protocol::MessageId; + // Use a distinct service/instance ID so parallel tests joined to + // the same SD multicast group do not produce false matches. + const SID: u16 = 0x005C; + const IID: u16 = 0x0001; + // Bind a receiver on the SD multicast port with loopback so we // actually see the outgoing announcement. Use a dedicated // receiver socket via socket2 to match the SD bind pattern. @@ -2042,10 +2047,6 @@ mod tests { rs }; - // Use a distinct service/instance ID so parallel tests joined to - // the same SD multicast group do not produce false matches. - const SID: u16 = 0x005C; - const IID: u16 = 0x0001; let config = ServerConfig::new(iface, 30501, SID, IID); let server = Server::new_with_loopback(config, true).await.unwrap(); let fut = server.announcement_loop().expect("build loop"); @@ -2279,9 +2280,10 @@ mod tests { let server = Server::new_with_loopback(config, true) .await .expect("server must bind with loopback enabled"); - server - .start_announcing() - .expect("start_announcing should succeed on a non-passive server"); + let announce_fut = server + .announcement_loop() + .expect("announcement_loop should build on a non-passive server"); + tokio::spawn(announce_fut); // Scan the multicast group for our OfferService. The first tick // happens immediately; 2s is ample headroom for scheduler jitter. diff --git a/tests/client_server.rs b/tests/client_server.rs index e4b544e..d48fee1 100644 --- a/tests/client_server.rs +++ b/tests/client_server.rs @@ -57,7 +57,7 @@ async fn test_client_server_subscribe_and_receive_event() { // Create client and subscribe to the server's event group let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); @@ -101,7 +101,7 @@ async fn test_client_send_sd_auto_binds_discovery() { // Create client — NO bind_discovery let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // send_sd_message should auto-bind discovery and succeed let sd_header = VecSdHeader { @@ -133,7 +133,7 @@ async fn test_client_bind_unbind_lifecycle_with_server() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // Bind discovery, subscribe, then unbind and rebind client.bind_discovery().await.unwrap(); @@ -162,7 +162,7 @@ async fn test_add_endpoint_and_send_to_service() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); // Register the server's endpoint manually (simulating non-broadcasting service) @@ -223,7 +223,7 @@ async fn test_subscribe_auto_binds_discovery() { // Create client — do NOT bind discovery manually let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); // Subscribe should auto-bind discovery internally @@ -266,7 +266,7 @@ async fn test_client_request_resolves_via_unicast_reply() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); @@ -328,7 +328,7 @@ async fn test_e2e_protect_on_publish_and_check_on_receive() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); // Register matching E2E profile on client client.register_e2e(key, profile); @@ -393,13 +393,13 @@ async fn test_multiple_subscribers_receive_events() { // Client 1 let (client1, mut updates1, run_fut1) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut1); + tokio::spawn(run_fut1); client1.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client1.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); // Client 2 let (client2, mut updates2, run_fut2) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut2); + tokio::spawn(run_fut2); client2.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client2.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); @@ -453,7 +453,7 @@ async fn test_multiple_subscribers_receive_events() { #[tokio::test] async fn test_updates_drain_after_shutdown() { let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); client.shut_down(); let result = tokio::time::timeout(std::time::Duration::from_secs(2), updates.recv()) @@ -469,7 +469,7 @@ async fn test_cloned_client_works() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let client2 = client.clone(); // Both clones can send commands @@ -490,7 +490,7 @@ async fn test_subscribe_specific_port_reuse() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - let _ = tokio::spawn(run_fut); + tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); From ffffa1a89e5bf8d4e6e1b7bc7484ef989cf8a368 Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Fri, 24 Apr 2026 17:07:14 -0400 Subject: [PATCH 06/14] PR #80 round: apply reviewer suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - client/mod.rs: fix Client::new doc — now returns the run-loop future, does not spawn. - client/inner.rs: apply backpressure on control_receiver.recv() via a select! guard so a full request_queue stalls senders instead of dropping the ControlMessage (which canceled embedded oneshot senders and surfaced as Error::Shutdown, conflating overload with shutdown). - server/mod.rs: rewrite the internally-contradictory "spawn + drop" test comment to match actual behavior (build + drop without polling). - client/inner.rs: rename test_inner_spawn_and_shutdown → test_inner_build_and_shutdown to match the Inner::build API. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/client/inner.rs | 31 ++++++++++++++----------------- src/client/mod.rs | 2 +- src/server/mod.rs | 14 ++++++-------- 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/client/inner.rs b/src/client/inner.rs index a59cc88..6153601 100644 --- a/src/client/inner.rs +++ b/src/client/inner.rs @@ -906,30 +906,27 @@ where } = &mut self; select! { () = tokio::time::sleep(std::time::Duration::from_millis(125)) => {} - // Receive a control message - ctrl = control_receiver.recv() => { + // Receive a control message only when the request queue + // has spare capacity, so we apply backpressure on the + // control channel instead of dropping the message — + // which would cancel any embedded oneshot senders and + // surface to callers as `RecvError` (mapped to + // `Error::Shutdown`), conflating overload with shutdown. + ctrl = control_receiver.recv(), if request_queue.len() < REQUEST_QUEUE_CAP => { if let Some(ctrl) = ctrl { debug!("Received control message: {:?}", ctrl); - if let Err(rejected) = request_queue.push_back(ctrl) { - // Queue full: rather than silently drop the - // rejected ControlMessage (which would - // cancel its oneshot senders and panic any - // caller awaiting with `.unwrap()`), reply - // on each sender with - // `Err(Error::Capacity("request_queue"))`. - warn!( - "request_queue at capacity ({}); rejecting control message", - REQUEST_QUEUE_CAP - ); - rejected.reject_with_capacity("request_queue"); - } + let push_result = request_queue.push_back(ctrl); + debug_assert!( + push_result.is_ok(), + "request_queue had capacity before recv but push_back failed" + ); } else { // The sender has been dropped, so we should exit *run = false; } } // Receive a discovery message - discovery = Inner::receive_discovery(discovery_socket) => { + discovery = Inner::receive_discovery(discovery_socket) => { trace!("Received discovery message: {:?}", discovery); match discovery { Ok((source, someip_header, sd_header)) => { @@ -1376,7 +1373,7 @@ mod tests { } #[tokio::test] - async fn test_inner_spawn_and_shutdown() { + async fn test_inner_build_and_shutdown() { let (control_sender, mut update_receiver, run_fut) = Inner::::build( Ipv4Addr::LOCALHOST, Arc::new(Mutex::new(E2ERegistry::new())), diff --git a/src/client/mod.rs b/src/client/mod.rs index e7aec43..80ca1ee 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -180,7 +180,7 @@ impl Client where MessageDefinitions: PayloadWireFormat + Clone + std::fmt::Debug + 'static, { - /// Creates a new client bound to the given network interface and spawns its event loop. + /// Creates a new client bound to the given network interface and returns its run-loop future to be driven by the caller. /// /// Returns a `(Client, ClientUpdates, run_future)` triple. The `Client` /// handle is [`Clone`]-able and can be shared across tasks. diff --git a/src/server/mod.rs b/src/server/mod.rs index 761aa87..d5b1e39 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -1998,14 +1998,12 @@ mod tests { .announcement_loop() .expect("announcement_loop on a regular server must build"); // The announcer loops forever; the test succeeds as soon as - // construction returns Ok. Spawn + drop the JoinHandle — the - // task rides the runtime lifecycle until the test's tokio - // runtime shuts down at end-of-test. - // Do NOT spawn: the announcer loops forever, and spawning + - // dropping the JoinHandle would leave the task running and - // emitting multicast for the rest of the test binary's - // lifetime, interfering with parallel tests that bind the same - // multicast group. We only care that construction returned Ok. + // construction returns Ok. + // Do not poll or spawn the future: doing so would leave the + // announcer running and emitting multicast for the rest of the + // test binary's lifetime, interfering with parallel tests that + // bind the same multicast group. We only care that construction + // returned Ok, so drop the future without polling it. drop(fut); } From 452e47bfad18c6e31652d44dee8886b7cb67f5b7 Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Fri, 24 Apr 2026 17:56:31 -0400 Subject: [PATCH 07/14] examples: bind spawn handle in client_server to avoid let_underscore_future `let _ = tokio::spawn(run);` trips clippy::let_underscore_future because `JoinHandle: Future`. Match the pattern already used in examples/discovery_client and the lib.rs doctest. Co-Authored-By: Claude Opus 4.7 (1M context) --- examples/client_server/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/client_server/src/main.rs b/examples/client_server/src/main.rs index a1807f7..f76ae0b 100644 --- a/examples/client_server/src/main.rs +++ b/examples/client_server/src/main.rs @@ -107,7 +107,7 @@ async fn main() -> Result<(), Box> { // ── Create the client (handles discovery, subscriptions, SD socket) ── let (client, mut updates, run) = simple_someip::Client::::new(interface); - let _ = tokio::spawn(run); + let _run_handle = tokio::spawn(run); client.bind_discovery().await?; info!("Client discovery bound"); From 6646cf398121a1c53b121f6765d3ac6b6a9e381d Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Fri, 24 Apr 2026 18:08:16 -0400 Subject: [PATCH 08/14] server: add #[must_use] to announcement_loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror the Client::new / Client::new_with_loopback pattern — the returned future must be spawned or awaited, and silently dropping it disables announcements. A `#[must_use = "..."]` attribute nudges callers who forget (e.g. `server.announcement_loop()?;` that discards the inner future). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/server/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/server/mod.rs b/src/server/mod.rs index d5b1e39..b3cc5b0 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -285,6 +285,7 @@ impl Server { /// called on a server constructed via [`Server::new_passive`] — passive /// servers have no real SD socket bound to port 30490, so any /// announcements would go out with an incorrect source port. + #[must_use = "the returned announcement-loop future must be spawned (e.g. tokio::spawn) or awaited for the server to emit SD announcements; dropping it silently disables announcements"] pub fn announcement_loop( &self, ) -> Result + Send + 'static, Error> { From 118c8ce3f25b40ac08e58b206951c66e855b9759 Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Fri, 24 Apr 2026 18:20:31 -0400 Subject: [PATCH 09/14] client: distinguish pending_responses saturation from shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When pending_responses is full, the inner loop previously dropped the response oneshot sender, which surfaced to callers as RecvError → Error::Shutdown — misattributing overload as a lifecycle failure. Recover the oneshot sender from the failed insert and send an explicit Err(Error::Capacity("pending_responses")) through it instead. The UDP send has already succeeded at this point; the peer's reply (if any) still arrives on ClientUpdates::Unicast, it just can't be routed back to this specific caller's oneshot. Reserving Error::Shutdown for actual run-loop exit keeps RecvError at PendingResponse::response unambiguous: a cancelled oneshot now only means the run-loop future is gone, not that the map was full. Update the # Errors sections on PendingResponse::response and Client::request to document both the new Capacity("pending_responses") case and the narrower Shutdown contract. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/client/mod.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/client/mod.rs b/src/client/mod.rs index 80ca1ee..56ba126 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -63,9 +63,14 @@ impl

PendingResponse

{ /// # Errors /// /// Returns the same errors as the request itself (e.g. deserialization - /// failure). Returns [`Error::Shutdown`] if the client's run-loop - /// future exits before the response is delivered — the caller's - /// `PendingResponse` handle outlived its driver. + /// failure). Returns [`Error::Capacity`] with tag `"pending_responses"` + /// if the inner loop's response-tracking map was full when the request + /// was sent — the UDP send still went out, but the reply (if any) + /// arrives on [`ClientUpdates`] rather than this oneshot. + /// Returns [`Error::Shutdown`] only if the client's run-loop future + /// exits before the response is delivered — the caller's + /// `PendingResponse` handle outlived its driver. Reserving `Shutdown` + /// for actual lifecycle failure keeps `RecvError` unambiguous. pub async fn response(self) -> Result { self.receiver.await.map_err(|_| Error::Shutdown)? } @@ -680,9 +685,14 @@ where /// /// Returns an error if the service is not found, unicast binding fails, /// the UDP send fails, or the response payload fails to deserialize. - /// Returns [`Error::Shutdown`] if the client's run-loop future has - /// exited before this call (dropped, cancelled, or otherwise gone) - /// — the `Client` handle has outlived its driver and further + /// Returns [`Error::Capacity`] with tag `"pending_responses"` if the + /// inner loop's response-tracking map was full when this request was + /// sent — the UDP send still went out, but the reply cannot be + /// routed back to this caller's oneshot (it arrives on + /// [`ClientUpdates`] instead). + /// Returns [`Error::Shutdown`] only if the client's run-loop future + /// has exited before this call (dropped, cancelled, or otherwise + /// gone) — the `Client` handle has outlived its driver and further /// control-channel sends cannot make progress. pub async fn request( &self, From 8964f75029d706f652994e0057bf7b6c8380f44c Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Fri, 24 Apr 2026 18:30:59 -0400 Subject: [PATCH 10/14] docs(error): add "pending_responses" to Capacity tag list PR #80 introduced Error::Capacity("pending_responses") in inner.rs but the tag enumeration in client::Error::Capacity's docstring still listed only "unicast_sockets" and "udp_buffer". Add the new tag and format the list as bullets for readability. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/client/error.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/client/error.rs b/src/client/error.rs index 8f84bc7..8746c4b 100644 --- a/src/client/error.rs +++ b/src/client/error.rs @@ -42,8 +42,10 @@ pub enum Error { /// A fixed-capacity internal structure is full. The argument is a /// lowercase `snake_case` tag naming the resource; grep the crate for /// the tag to find the compile-time constant that governs it. Current - /// tags: `"unicast_sockets"` (→ `UNICAST_SOCKETS_CAP`), `"udp_buffer"` - /// (→ `crate::UDP_BUFFER_SIZE`). + /// tags: + /// - `"unicast_sockets"` → `UNICAST_SOCKETS_CAP` + /// - `"udp_buffer"` → `crate::UDP_BUFFER_SIZE` + /// - `"pending_responses"` → `PENDING_RESPONSES_CAP` #[error("internal capacity exceeded: {0}")] Capacity(&'static str), /// An error surfaced by the pluggable transport backend (see From 92606c1e66a6c79ae12a1d415493a6a2cd2f118a Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Fri, 24 Apr 2026 19:47:27 -0400 Subject: [PATCH 11/14] fix: capture tokio::spawn JoinHandle to avoid unused_must_use warnings Bind all bare `tokio::spawn(run_fut)` calls in tests and doctests to `let _ = ...` so the `#[must_use]` JoinHandle is explicitly discarded rather than silently dropped. Also fixes the README server quick-start. Co-Authored-By: Claude Sonnet 4.6 --- README.md | 4 ++-- src/client/inner.rs | 44 +++++++++++++++++++++--------------------- src/client/mod.rs | 42 ++++++++++++++++++++-------------------- tests/client_server.rs | 20 +++++++++---------- 4 files changed, 55 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index 26e4019..62dc2f1 100644 --- a/README.md +++ b/README.md @@ -103,10 +103,10 @@ use std::net::Ipv4Addr; async fn main() -> Result<(), Box> { let config = ServerConfig::new(Ipv4Addr::new(192, 168, 1, 200), 30500, 0x1234, 1); let mut server = Server::new(config).await?; - tokio::spawn(server.announcement_loop()?); + let _ = tokio::spawn(server.announcement_loop()?); let publisher = server.publisher(); - tokio::spawn(async move { server.run().await }); + let _ = tokio::spawn(async move { server.run().await }); // Publish events to subscribers... Ok(()) diff --git a/src/client/inner.rs b/src/client/inner.rs index 6153601..e4fb5e5 100644 --- a/src/client/inner.rs +++ b/src/client/inner.rs @@ -1379,7 +1379,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Drop control sender to trigger loop exit drop(control_sender); // The update receiver should eventually return None when the inner loop exits @@ -1414,7 +1414,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::bind_discovery(); drop(rx); @@ -1431,7 +1431,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::unbind_discovery(); drop(rx); @@ -1448,7 +1448,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // SetInterface(LOCALHOST) on a fresh inner goes straight to // bind_discovery + send response (interface already matches). @@ -1467,7 +1467,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Bind discovery first so the SendSD path has a socket to use let (rx, msg) = TestControl::bind_discovery(); @@ -1497,7 +1497,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Bind discovery so SetInterface will take the multi-step path: // iteration 1: unbind discovery, re-queue SetInterface @@ -1568,7 +1568,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); let (rx, msg) = TestControl::add_endpoint(0x1234, 0x0001, addr, 0); @@ -1586,7 +1586,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::remove_endpoint(0x1234, 0x0001); drop(rx); @@ -1603,7 +1603,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Add an endpoint first so SendToService doesn't fail with ServiceNotFound let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); @@ -1630,7 +1630,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), true, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::bind_discovery(); control_sender.send(msg).await.unwrap(); @@ -1645,7 +1645,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::bind_discovery(); control_sender.send(msg).await.unwrap(); @@ -1665,7 +1665,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let target = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 30490); let sd_header = empty_sd_header(); @@ -1686,7 +1686,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); let (rx, msg) = TestControl::add_endpoint(0x1234, 0x0001, addr, 0); @@ -1711,7 +1711,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Bind discovery first let (rx, msg) = TestControl::bind_discovery(); @@ -1742,7 +1742,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Add endpoint but do NOT bind discovery let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); @@ -1767,7 +1767,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::subscribe(0xFFFF, 0xFFFF, 1, 3, 0x01, 0); control_sender.send(msg).await.unwrap(); @@ -1786,7 +1786,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 5000); let (rx, msg) = TestControl::add_endpoint(0x1234, 0x0001, addr, 0); @@ -1821,7 +1821,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let (rx, msg) = TestControl::subscribe(0x1234, 0x0001, 1, 3, 0x01, 0); drop(rx); @@ -1839,7 +1839,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Change to a different loopback-range address (127.0.0.2). // Binding discovery on 127.0.0.2 should succeed on most systems. @@ -1864,7 +1864,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Bind discovery on LOCALHOST first let (rx, msg) = TestControl::bind_discovery(); @@ -1895,7 +1895,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Add endpoint and bind discovery let (rx, msg) = TestControl::bind_discovery(); @@ -1942,7 +1942,7 @@ mod tests { Arc::new(Mutex::new(E2ERegistry::new())), false, ); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let raw = UdpSocket::bind("127.0.0.1:0").await.unwrap(); let target = SocketAddrV4::new(Ipv4Addr::LOCALHOST, raw.local_addr().unwrap().port()); diff --git a/src/client/mod.rs b/src/client/mod.rs index 56ba126..88fb2bb 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -204,7 +204,7 @@ where /// # use std::net::Ipv4Addr; /// # async fn demo() { /// let (client, mut updates, run) = Client::::new(Ipv4Addr::LOCALHOST); - /// tokio::spawn(run); + /// let _run_task = tokio::spawn(run); /// // ...interact with `client` and `updates`... /// # let _ = (client, updates); /// # } @@ -760,7 +760,7 @@ mod tests { #[tokio::test] async fn test_client_new_and_interface() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); assert_eq!(client.interface(), Ipv4Addr::LOCALHOST); client.shut_down(); } @@ -768,7 +768,7 @@ mod tests { #[tokio::test] async fn test_client_debug() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let debug_str = format!("{client:?}"); assert!(debug_str.contains("Client")); assert!(debug_str.contains("127.0.0.1")); @@ -815,7 +815,7 @@ mod tests { #[tokio::test] async fn test_subscribe_unknown_service_returns_error() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let result = client.subscribe(0xFFFF, 0xFFFF, 1, 3, 0x01, 0).await; assert!( matches!(result, Err(Error::ServiceNotFound)), @@ -827,7 +827,7 @@ mod tests { #[tokio::test] async fn test_subscribe_no_wait_unknown_service_does_not_panic() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // subscribe_no_wait is fire-and-forget — it should not panic even // when the service is unknown (the inner loop sends ServiceNotFound // on the dropped response channel, which is harmless). @@ -840,7 +840,7 @@ mod tests { #[tokio::test] async fn test_bind_discovery_and_unbind() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); client.unbind_discovery().await.unwrap(); client.shut_down(); @@ -849,7 +849,7 @@ mod tests { #[tokio::test] async fn test_set_interface() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let new_addr = Ipv4Addr::LOCALHOST; client.set_interface(new_addr).await.unwrap(); assert_eq!(client.interface(), new_addr); @@ -859,7 +859,7 @@ mod tests { #[tokio::test] async fn test_add_endpoint_succeeds() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::new(192, 168, 1, 1), 30000); client.add_endpoint(0x1234, 0x0001, addr, 0).await.unwrap(); client.shut_down(); @@ -868,7 +868,7 @@ mod tests { #[tokio::test] async fn test_send_to_service_unknown_returns_error() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let msg = crate::protocol::Message::new_sd(1, &empty_sd_header()); let result = client.send_to_service(0xFFFF, 0xFFFF, msg).await; assert!( @@ -881,7 +881,7 @@ mod tests { #[tokio::test] async fn test_remove_endpoint_succeeds() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::new(192, 168, 1, 1), 30000); client.add_endpoint(0x1234, 0x0001, addr, 0).await.unwrap(); client.remove_endpoint(0x1234, 0x0001).await.unwrap(); @@ -923,7 +923,7 @@ mod tests { #[tokio::test] async fn test_send_sd_message() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Bind discovery first so the send path uses the existing socket client.bind_discovery().await.unwrap(); let target = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 30490); @@ -935,7 +935,7 @@ mod tests { #[tokio::test] async fn test_send_to_service_success_returns_pending_response() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, 30000); client.add_endpoint(0x1234, 0x0001, addr, 0).await.unwrap(); let msg = crate::protocol::Message::new_sd(1, &empty_sd_header()); @@ -948,7 +948,7 @@ mod tests { #[tokio::test] async fn test_recv_returns_none_after_shutdown() { let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.shut_down(); // Now the inner loop should exit; recv() should return None let result = tokio::time::timeout(std::time::Duration::from_secs(2), updates.recv()).await; @@ -959,7 +959,7 @@ mod tests { #[tokio::test] async fn test_register_and_unregister_e2e() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let key = E2EKey { service_id: 0x1234, method_or_event_id: 0x0001, @@ -973,7 +973,7 @@ mod tests { #[tokio::test] async fn test_client_is_clone() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let client2 = client.clone(); assert_eq!(client.interface(), client2.interface()); client.shut_down(); @@ -982,7 +982,7 @@ mod tests { #[tokio::test] async fn test_client_updates_debug() { let (_client, updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let debug_str = format!("{updates:?}"); assert!(debug_str.contains("ClientUpdates")); } @@ -990,7 +990,7 @@ mod tests { #[tokio::test] async fn test_request_unknown_service_returns_error() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let msg = crate::protocol::Message::new_sd(1, &empty_sd_header()); let result = client.request(0xFFFF, 0xFFFF, msg).await; assert!( @@ -1003,7 +1003,7 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_does_not_panic() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); let sd_header = empty_sd_header(); @@ -1027,7 +1027,7 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_without_discovery_bound() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Don't bind discovery — the task should handle the error gracefully. let sd_header = empty_sd_header(); let handle = @@ -1049,7 +1049,7 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_abort_stops_task() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); let sd_header = empty_sd_header(); @@ -1179,7 +1179,7 @@ mod tests { #[tokio::test] async fn test_start_sd_announcements_stops_on_shutdown() { let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); let sd_header = empty_sd_header(); diff --git a/tests/client_server.rs b/tests/client_server.rs index d48fee1..9e0388c 100644 --- a/tests/client_server.rs +++ b/tests/client_server.rs @@ -57,7 +57,7 @@ async fn test_client_server_subscribe_and_receive_event() { // Create client and subscribe to the server's event group let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); @@ -101,7 +101,7 @@ async fn test_client_send_sd_auto_binds_discovery() { // Create client — NO bind_discovery let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // send_sd_message should auto-bind discovery and succeed let sd_header = VecSdHeader { @@ -133,7 +133,7 @@ async fn test_client_bind_unbind_lifecycle_with_server() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Bind discovery, subscribe, then unbind and rebind client.bind_discovery().await.unwrap(); @@ -162,7 +162,7 @@ async fn test_add_endpoint_and_send_to_service() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.bind_discovery().await.unwrap(); // Register the server's endpoint manually (simulating non-broadcasting service) @@ -223,7 +223,7 @@ async fn test_subscribe_auto_binds_discovery() { // Create client — do NOT bind discovery manually let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); // Subscribe should auto-bind discovery internally @@ -266,7 +266,7 @@ async fn test_client_request_resolves_via_unicast_reply() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); client.subscribe(0x5B, 1, 1, 3, 0x01, 0).await.unwrap(); @@ -328,7 +328,7 @@ async fn test_e2e_protect_on_publish_and_check_on_receive() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); // Register matching E2E profile on client client.register_e2e(key, profile); @@ -453,7 +453,7 @@ async fn test_multiple_subscribers_receive_events() { #[tokio::test] async fn test_updates_drain_after_shutdown() { let (client, mut updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); client.shut_down(); let result = tokio::time::timeout(std::time::Duration::from_secs(2), updates.recv()) @@ -469,7 +469,7 @@ async fn test_cloned_client_works() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let client2 = client.clone(); // Both clones can send commands @@ -490,7 +490,7 @@ async fn test_subscribe_specific_port_reuse() { let server_handle = tokio::spawn(async move { server.run().await }); let (client, _updates, run_fut) = TestClient::new(Ipv4Addr::LOCALHOST); - tokio::spawn(run_fut); + let _ = tokio::spawn(run_fut); let server_addr = SocketAddrV4::new(Ipv4Addr::LOCALHOST, server_port); client.add_endpoint(0x5B, 1, server_addr, 0).await.unwrap(); From d92c5a37e407ab4657d14e0a7745daa8fa7fe64a Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Fri, 24 Apr 2026 19:49:51 -0400 Subject: [PATCH 12/14] fix: store and observe JoinHandles in production/example code Replace `let _ = tokio::spawn(...)` with stored handles in README and the client_server example so panics or unexpected task exits don't go silently unobserved. README server quick-start uses tokio::select! to surface either loop exiting; example stores the handle so the task lives for the duration of main. Co-Authored-By: Claude Sonnet 4.6 --- README.md | 9 +++++++-- examples/client_server/src/main.rs | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 62dc2f1..00dded6 100644 --- a/README.md +++ b/README.md @@ -103,12 +103,17 @@ use std::net::Ipv4Addr; async fn main() -> Result<(), Box> { let config = ServerConfig::new(Ipv4Addr::new(192, 168, 1, 200), 30500, 0x1234, 1); let mut server = Server::new(config).await?; - let _ = tokio::spawn(server.announcement_loop()?); + let announce_handle = tokio::spawn(server.announcement_loop()?); let publisher = server.publisher(); - let _ = tokio::spawn(async move { server.run().await }); + let run_handle = tokio::spawn(async move { server.run().await }); // Publish events to subscribers... + + tokio::select! { + res = announce_handle => eprintln!("announcement loop exited unexpectedly: {res:?}"), + res = run_handle => eprintln!("server run loop exited: {res:?}"), + } Ok(()) } ``` diff --git a/examples/client_server/src/main.rs b/examples/client_server/src/main.rs index f76ae0b..1cc716d 100644 --- a/examples/client_server/src/main.rs +++ b/examples/client_server/src/main.rs @@ -132,7 +132,7 @@ async fn main() -> Result<(), Box> { let _publisher = server.publisher(); // Spawn the server event loop (handles incoming subscriptions). - tokio::spawn(async move { + let _server_handle = tokio::spawn(async move { if let Err(e) = server.run().await { error!("Server error: {e}"); } From 861c5d3578ce6cacf5ace85b66b6d3431c14eaf0 Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Fri, 24 Apr 2026 20:02:54 -0400 Subject: [PATCH 13/14] fix: use Tokio-specific wording and unique test service IDs - Inner::build and server/README.md: replace "executor" with "Tokio runtime" to avoid implying runtime-agnostic execution - announcement_loop_sends_offer_service_when_driven: change SID/IID from 0x005C/0x0001 (used throughout the module) to 0xAA01/0xFF01 so the "not used elsewhere" comment is actually true Co-Authored-By: Claude Sonnet 4.6 --- src/client/inner.rs | 6 +++--- src/server/README.md | 2 +- src/server/mod.rs | 9 +++++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/client/inner.rs b/src/client/inner.rs index e4fb5e5..8d35a84 100644 --- a/src/client/inner.rs +++ b/src/client/inner.rs @@ -345,11 +345,11 @@ where PayloadDefinitions: PayloadWireFormat + Clone + std::fmt::Debug + 'static, { /// Construct an `Inner` and return the control/update channels plus - /// the run-loop future. The caller drives the future with whatever - /// executor it owns (typically `tokio::spawn`). + /// the run-loop future. The caller must drive the future on a Tokio + /// runtime (e.g. via `tokio::spawn`). /// /// The future is bounded `Send + 'static` because every in-repo - /// consumer spawns it on a multithreaded executor and because the + /// consumer spawns it on a multithreaded Tokio runtime and because the /// concrete captured state (tokio mpsc, `TokioSocket`, E2E registry) /// is already Send. A bare-metal consumer whose transport produces /// `!Send` state needs a cfg-gated alternative constructor; none diff --git a/src/server/README.md b/src/server/README.md index 989824f..5357569 100644 --- a/src/server/README.md +++ b/src/server/README.md @@ -56,7 +56,7 @@ async fn main() -> Result<(), Box> { let mut server = Server::new(config).await?; // Start announcing the service (sends OfferService every 1s). - // Spawn the announcement loop future on your executor. + // Spawn the announcement loop future on the Tokio runtime. tokio::spawn(server.announcement_loop()?); // Get event publisher for sending events diff --git a/src/server/mod.rs b/src/server/mod.rs index b3cc5b0..2d644aa 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -2018,10 +2018,11 @@ mod tests { async fn announcement_loop_sends_offer_service_when_driven() { use crate::protocol::MessageId; - // Use a distinct service/instance ID so parallel tests joined to - // the same SD multicast group do not produce false matches. - const SID: u16 = 0x005C; - const IID: u16 = 0x0001; + // Use service/instance IDs not used elsewhere in this test module + // so parallel tests joined to the same SD multicast group cannot + // produce false matches. + const SID: u16 = 0xAA01; + const IID: u16 = 0xFF01; // Bind a receiver on the SD multicast port with loopback so we // actually see the outgoing announcement. Use a dedicated From 82df573a5d214d24e2606c69179cb54107594f0c Mon Sep 17 00:00:00 2001 From: Justin Kovacich Date: Fri, 24 Apr 2026 20:14:47 -0400 Subject: [PATCH 14/14] docs(server): fix Result signatures and executor wording in README API ref Add missing error type to Result entries and replace "their executor" with "the Tokio runtime". Co-Authored-By: Claude Sonnet 4.6 --- src/server/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/server/README.md b/src/server/README.md index 5357569..845905a 100644 --- a/src/server/README.md +++ b/src/server/README.md @@ -153,10 +153,10 @@ Configuration for a SOME/IP service provider: Main server struct: -- `new(config: ServerConfig) -> Result` - Create new server -- `announcement_loop() -> Result + Send + 'static>` - Build the SD announcement future; caller spawns on their executor +- `new(config: ServerConfig) -> Result` - Create new server +- `announcement_loop() -> Result + Send + 'static, Error>` - Build the SD announcement future; caller spawns on the Tokio runtime - `publisher() -> Arc` - Get event publisher -- `run() -> Result<()>` - Run event loop (handles subscriptions) +- `run() -> Result<(), Error>` - Run event loop (handles subscriptions) - `register_e2e(key, profile)` - Register E2E protection for a message key - `unregister_e2e(key)` - Remove E2E protection for a message key