From 93856b58ef453bfbf760875a5c4c4d029f974a4b Mon Sep 17 00:00:00 2001 From: lemunozm Date: Sat, 1 May 2021 18:17:46 +0200 Subject: [PATCH 01/25] Architecture changes --- src/network.rs | 7 +++-- src/network/driver.rs | 64 ++++++++++++++++++++++++++++++++----------- src/network/loader.rs | 4 +-- src/network/poll.rs | 35 +++++++++++++---------- src/node.rs | 11 +++++--- 5 files changed, 82 insertions(+), 39 deletions(-) diff --git a/src/network.rs b/src/network.rs index 85651cd7..fc58a74c 100644 --- a/src/network.rs +++ b/src/network.rs @@ -150,13 +150,14 @@ impl NetworkProcessor { let processors = &mut self.processors; self.poll.process_event(timeout, |poll_event| { match poll_event { - PollEvent::Network(resource_id) => { - let adapter_id = resource_id.adapter_id() as usize; - processors[adapter_id].process(resource_id, &mut |net_event| { + PollEvent::Network(resource_id, interest) => { + let processor = &processors[resource_id.adapter_id() as usize]; + processor.process(resource_id, interest, &mut |net_event| { log::trace!("Processed {:?}", net_event); event_callback(net_event); }); } + #[allow(dead_code)] //TODO: remove it with native event support PollEvent::Waker => todo!(), } diff --git a/src/network/driver.rs b/src/network/driver.rs index 9fd250a1..f4255ac5 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -1,6 +1,6 @@ use super::endpoint::{Endpoint}; use super::resource_id::{ResourceId, ResourceType}; -use super::poll::{Poll}; +use super::poll::{Poll, Interest}; use super::registry::{ResourceRegistry}; use super::remote_addr::{RemoteAddr}; use super::adapter::{Adapter, Remote, Local, SendStatus, AcceptedType, ReadStatus}; @@ -12,14 +12,30 @@ use std::io::{self}; #[cfg(doctest)] use super::transport::{Transport}; -/// Enum used to describe and event that an adapter network has produced. +/// Enum used to describe a network event that an internal transport adapter has produced. pub enum NetEvent<'a> { - /// New endpoint has been connected to a listener. - /// This event will be sent only in connection oriented protocols as *TCP*. - /// It also contains the resource id of the listener that accepted this connection. - Connected(Endpoint, ResourceId), + /// New endpoint has been connected. + /// This event is only generated after a [`crate::network::NetworkController::connect_async()`] + /// call. + /// The event contains the endpoint of the connection attempt + /// (same endpoint returned by the `connect_async()` method). + /// The `bool` indicates if the connection could be established or not. + /// + /// Note that this event will only be generated by connection-oriented transports as *TCP*. + Connected(Endpoint, bool), + + /// New endpoint has been accepted by a listener. + /// The event contains the resource id of the listener that accepted this connection. + /// + /// Note that this event will only be generated by connection-oriented transports as *TCP*. + Accepted(Endpoint, ResourceId), /// Input message received by the network. + /// In packet-based transports, the data of a message sent corresponds with the data of this + /// event. This one-to-one relation is not conserved in stream-based transports as *TCP*. + /// + /// If you want a packet-based protocol over *TCP* use + /// [`crate::transport::Transport::FramedTcp`]. Message(Endpoint, &'a [u8]), /// This event is only dispatched when a connection is lost. @@ -27,15 +43,17 @@ pub enum NetEvent<'a> { /// When this event is received, the resource is considered already removed, /// the user do not need to remove it after this event. /// A [`NetEvent::Message`] event will never be generated after this event from this endpoint. - /// This event will be sent only in connection oriented protocols as *Tcp*. - /// *UDP*, for example, is NOT connection oriented, and the event can no be detected. + + /// Note that this event will only be generated by connection-oriented transports as *TCP*. + /// *UDP*, for example, is NOT connection-oriented, and the event can no be detected. Disconnected(Endpoint), } impl std::fmt::Debug for NetEvent<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let string = match self { - Self::Connected(endpoint, id) => format!("Connected({}, {})", endpoint, id), + Self::Connected(endpoint, ok) => format!("Connected({}, {})", endpoint, ok), + Self::Accepted(endpoint, id) => format!("Accepted({}, {})", endpoint, id), Self::Message(endpoint, data) => format!("Message({}, {})", endpoint, data.len()), Self::Disconnected(endpoint) => format!("Disconnected({})", endpoint), }; @@ -51,7 +69,7 @@ pub trait ActionController: Send + Sync { } pub trait EventProcessor: Send + Sync { - fn process(&self, resource_id: ResourceId, event_callback: &mut dyn FnMut(NetEvent<'_>)); + fn process(&self, id: ResourceId, interest: Interest, callback: &mut dyn FnMut(NetEvent<'_>)); } pub struct Driver { @@ -124,16 +142,30 @@ impl ActionController for Driver { } impl> EventProcessor for Driver { - fn process(&self, id: ResourceId, event_callback: &mut dyn FnMut(NetEvent<'_>)) { + fn process(&self, id: ResourceId, interest: Interest, callback: &mut dyn FnMut(NetEvent<'_>)) { match id.resource_type() { - ResourceType::Remote => self.process_remote(id, event_callback), - ResourceType::Local => self.process_local(id, event_callback), + ResourceType::Remote => match interest { + Interest::Readable => self.process_remote_read(id, callback), + Interest::Writable => self.process_remote_write(id, callback), + } + ResourceType::Local => match interest { + Interest::Readable => self.process_local_read(id, callback), + Interest::Writable => (), + } } } } impl> Driver { - fn process_remote(&self, id: ResourceId, mut event_callback: impl FnMut(NetEvent<'_>)) { + fn process_remote_write(&self, id: ResourceId, mut event_callback: impl FnMut(NetEvent<'_>)) { + if let Some(remote) = self.remote_registry.get(id) { + let endpoint = Endpoint::new(id, remote.addr); + event_callback(NetEvent::Connected(endpoint, true)); //TODO + self.remote_registry.remove(id); + } + } + + fn process_remote_read(&self, id: ResourceId, mut event_callback: impl FnMut(NetEvent<'_>)) { if let Some(remote) = self.remote_registry.get(id) { let endpoint = Endpoint::new(id, remote.addr); log::trace!("Processed remote for {}", endpoint); @@ -151,7 +183,7 @@ impl> Driver { } } - fn process_local(&self, id: ResourceId, mut event_callback: impl FnMut(NetEvent<'_>)) { + fn process_local_read(&self, id: ResourceId, mut event_callback: impl FnMut(NetEvent<'_>)) { if let Some(local) = self.local_registry.get(id) { log::trace!("Processed local for {}", id); local.resource.accept(|accepted| { @@ -160,7 +192,7 @@ impl> Driver { AcceptedType::Remote(addr, remote) => { let remote_id = self.remote_registry.add(remote, addr); let endpoint = Endpoint::new(remote_id, addr); - event_callback(NetEvent::Connected(endpoint, id)); + event_callback(NetEvent::Accepted(endpoint, id)); } AcceptedType::Data(addr, data) => { let endpoint = Endpoint::new(id, addr); diff --git a/src/network/loader.rs b/src/network/loader.rs index c7d458c7..0f01041f 100644 --- a/src/network/loader.rs +++ b/src/network/loader.rs @@ -1,6 +1,6 @@ use super::endpoint::{Endpoint}; use super::resource_id::{ResourceId}; -use super::poll::{Poll}; +use super::poll::{Poll, Interest}; use super::remote_addr::{RemoteAddr}; use super::driver::{NetEvent, Driver, ActionController, EventProcessor}; use super::adapter::{Adapter, SendStatus}; @@ -80,7 +80,7 @@ impl ActionController for UnimplementedDriver { } impl EventProcessor for UnimplementedDriver { - fn process(&self, _: ResourceId, _: &mut dyn FnMut(NetEvent<'_>)) { + fn process(&self, _: ResourceId, _: Interest, _: &mut dyn FnMut(NetEvent<'_>)) { panic!("{}", UNIMPLEMENTED_DRIVER_ERR); } } diff --git a/src/network/poll.rs b/src/network/poll.rs index 110c7504..95fbb846 100644 --- a/src/network/poll.rs +++ b/src/network/poll.rs @@ -1,14 +1,19 @@ use super::resource_id::{ResourceId, ResourceType, ResourceIdGenerator}; -use mio::{Poll as MioPoll, Interest, Token, Events, Registry, Waker}; +use mio::{Poll as MioPoll, Interest as MioInterest, Token, Events, Registry, Waker}; use mio::event::{Source}; use std::time::{Duration}; use std::sync::{Arc}; use std::io::{ErrorKind}; +pub enum Interest { + Readable, + Writable, +} + pub enum PollEvent { - Network(ResourceId), + Network(ResourceId, Interest), Waker, } @@ -53,19 +58,21 @@ impl Poll { match self.mio_poll.poll(&mut self.events, timeout) { Ok(_) => { for mio_event in &self.events { - let poll_event = match mio_event.token() { - Self::WAKER_TOKEN => { - log::trace!("POLL EVENT: waker"); - PollEvent::Waker + if Self::WAKER_TOKEN == mio_event.token() { + log::trace!("POLL WAKER EVENT"); + event_callback(PollEvent::Waker); + } + else { + let resource_id = ResourceId::from(mio_event.token()); + if mio_event.is_readable() { + log::trace!("POLL EVENT (R): {}", resource_id); + event_callback(PollEvent::Network(resource_id, Interest::Readable)); } - token => { - let resource_id = ResourceId::from(token); - log::trace!("POLL EVENT: {}", resource_id); - PollEvent::Network(resource_id) + if mio_event.is_writable() { + log::trace!("POLL EVENT (W): {}", resource_id); + event_callback(PollEvent::Network(resource_id, Interest::Writable)); } - }; - - event_callback(poll_event); + } } break } @@ -100,7 +107,7 @@ impl PollRegistry { pub fn add(&self, source: &mut dyn Source) -> ResourceId { let id = self.id_generator.generate(); - self.registry.register(source, id.into(), Interest::READABLE).unwrap(); + self.registry.register(source, id.into(), MioInterest::READABLE | MioInterest::WRITABLE).unwrap(); log::trace!("Register to poll: {}", id); id } diff --git a/src/node.rs b/src/node.rs index 5b89d073..0129a1fd 100644 --- a/src/node.rs +++ b/src/node.rs @@ -110,7 +110,8 @@ impl From> for StoredNodeEvent { /// This kind of event is dispatched by `NodeListener::to_event_queue()`. #[derive(Debug, Clone)] pub enum StoredNetEvent { - Connected(Endpoint, ResourceId), + Connected(Endpoint, bool), + Accepted(Endpoint, ResourceId), Message(Endpoint, Vec), Disconnected(Endpoint), } @@ -118,7 +119,8 @@ pub enum StoredNetEvent { impl From> for StoredNetEvent { fn from(net_event: NetEvent<'_>) -> Self { match net_event { - NetEvent::Connected(endpoint, id) => Self::Connected(endpoint, id), + NetEvent::Connected(endpoint, ok) => Self::Connected(endpoint, ok), + NetEvent::Accepted(endpoint, id) => Self::Accepted(endpoint, id), NetEvent::Message(endpoint, data) => Self::Message(endpoint, Vec::from(data)), NetEvent::Disconnected(endpoint) => Self::Disconnected(endpoint), } @@ -129,8 +131,9 @@ impl StoredNetEvent { /// Use this `StoredNetEvent` as a `NetEvent` referencing its data. fn borrow(&self) -> NetEvent<'_> { match self { - Self::Connected(endpoint, id) => NetEvent::Connected(*endpoint, *id), - Self::Message(endpoint, data) => NetEvent::Message(*endpoint, &data), + Self::Connected(endpoint, ok) => NetEvent::Connected(*endpoint, *ok), + Self::Accepted(endpoint, id) => NetEvent::Accepted(*endpoint, *id), + Self::Message(endpoint, data) => NetEvent::Message(*endpoint, data), Self::Disconnected(endpoint) => NetEvent::Disconnected(*endpoint), } } From 18fd64b4e11574b21ff7f95436ad103ac150d070 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Mon, 3 May 2021 18:15:21 +0200 Subject: [PATCH 02/25] Tcp async connection working. --- src/adapters/tcp.rs | 7 ++-- src/network.rs | 86 ++++++++++++++++++++++++++++++++--------- src/network/driver.rs | 30 +++++++------- src/network/poll.rs | 4 +- src/network/registry.rs | 27 ++++++++----- src/node.rs | 6 +-- tests/integration.rs | 46 ++++++++++++---------- 7 files changed, 136 insertions(+), 70 deletions(-) diff --git a/src/adapters/tcp.rs b/src/adapters/tcp.rs index 0567dc6c..e94054a7 100644 --- a/src/adapters/tcp.rs +++ b/src/adapters/tcp.rs @@ -7,7 +7,7 @@ use crate::network::{RemoteAddr}; use mio::net::{TcpListener, TcpStream}; use mio::event::{Source}; -use std::net::{SocketAddr, TcpStream as StdTcpStream}; +use std::net::{SocketAddr}; use std::io::{self, ErrorKind, Read, Write}; use std::ops::{Deref}; use std::mem::{MaybeUninit}; @@ -42,10 +42,9 @@ impl Resource for RemoteResource { impl Remote for RemoteResource { fn connect(remote_addr: RemoteAddr) -> io::Result> { let peer_addr = *remote_addr.socket_addr(); - let stream = StdTcpStream::connect(peer_addr)?; + let stream = TcpStream::connect(peer_addr)?; let local_addr = stream.local_addr()?; - stream.set_nonblocking(true)?; - Ok(ConnectionInfo { remote: TcpStream::from_std(stream).into(), local_addr, peer_addr }) + Ok(ConnectionInfo { remote: stream.into(), local_addr, peer_addr }) } fn receive(&self, mut process_data: impl FnMut(&[u8])) -> ReadStatus { diff --git a/src/network.rs b/src/network.rs index fc58a74c..0a18e236 100644 --- a/src/network.rs +++ b/src/network.rs @@ -174,37 +174,87 @@ mod tests { static ref TIMEOUT: Duration = Duration::from_millis(1000); } + fn no_more_events(mut processor: NetworkProcessor) { + let mut was_event = false; + processor.process_poll_event(Some(*TIMEOUT), |_| was_event = true); + assert!(!was_event); + } + #[test] - fn create_remove_listener() { + fn successful_connection() { + let (controller, mut processor) = self::split(); + let (listener_id, addr) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); + let (endpoint, _) = controller.connect(Transport::Tcp, RemoteAddr::Socket(addr)).unwrap(); + + dbg!(addr); + + let mut was_connected = 0; + let mut was_accepted = 0; + for _ in 0..2 { + processor.process_poll_event(Some(*TIMEOUT), |net_event| match net_event { + NetEvent::Connected(net_endpoint) => { + assert_eq!(endpoint, net_endpoint); + was_connected += 1; + } + NetEvent::Accepted(_, net_listener_id) => { + assert_eq!(listener_id, net_listener_id); + was_accepted += 1; + } + _ => unreachable!() + }); + } + assert_eq!(was_connected, 1); + assert_eq!(was_accepted, 1); + + no_more_events(processor); + } + + #[test] + fn unreachable_connection() { let (controller, mut processor) = self::split(); + let (endpoint, _) = controller.connect(Transport::Tcp, "127.0.0.1:5555").unwrap(); + + let mut was_disconnected = false; + processor.process_poll_event(Some(*TIMEOUT), |net_event| match net_event { + NetEvent::Disconnected(net_endpoint) => { + assert_eq!(endpoint, net_endpoint); + was_disconnected = true; + } + _ => unreachable!() + }); + assert!(was_disconnected); + + no_more_events(processor); + } + + #[test] + fn create_remove_listener() { + let (controller, processor) = self::split(); let (listener_id, _) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); assert!(controller.remove(listener_id)); // Do not generate an event assert!(!controller.remove(listener_id)); - let mut was_event = false; - processor.process_poll_event(Some(*TIMEOUT), |_| was_event = true); - assert!(!was_event); + no_more_events(processor); } #[test] fn create_remove_listener_with_connection() { let (controller, mut processor) = self::split(); let (listener_id, addr) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); - controller.connect(Transport::Tcp, addr).unwrap(); + controller.connect(Transport::Tcp, RemoteAddr::Socket(addr)).unwrap(); - let mut was_event = false; - processor.process_poll_event(Some(*TIMEOUT), |net_event| match net_event { - NetEvent::Connected(_, _) => { - assert!(controller.remove(listener_id)); - assert!(!controller.remove(listener_id)); - was_event = true; - } - _ => unreachable!(), - }); - assert!(was_event); + for _ in 0..2 { + // We expect two events + processor.process_poll_event(Some(*TIMEOUT), |net_event| match net_event { + NetEvent::Connected(_) => (), + NetEvent::Accepted(_, _) => { + assert!(controller.remove(listener_id)); + assert!(!controller.remove(listener_id)); + } + _ => unreachable!(), + }); + } - let mut was_event = false; - processor.process_poll_event(Some(*TIMEOUT), |_| was_event = true); - assert!(!was_event); + no_more_events(processor); } } diff --git a/src/network/driver.rs b/src/network/driver.rs index f4255ac5..a10f3734 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -15,14 +15,13 @@ use super::transport::{Transport}; /// Enum used to describe a network event that an internal transport adapter has produced. pub enum NetEvent<'a> { /// New endpoint has been connected. - /// This event is only generated after a [`crate::network::NetworkController::connect_async()`] + /// This event is only generated after a [`crate::network::NetworkController::connect()`] /// call. - /// The event contains the endpoint of the connection attempt - /// (same endpoint returned by the `connect_async()` method). - /// The `bool` indicates if the connection could be established or not. + /// The event contains the endpoint of the connection + /// (same endpoint returned by the `connect()` method). /// /// Note that this event will only be generated by connection-oriented transports as *TCP*. - Connected(Endpoint, bool), + Connected(Endpoint), /// New endpoint has been accepted by a listener. /// The event contains the resource id of the listener that accepted this connection. @@ -38,7 +37,7 @@ pub enum NetEvent<'a> { /// [`crate::transport::Transport::FramedTcp`]. Message(Endpoint, &'a [u8]), - /// This event is only dispatched when a connection is lost. + /// This event is only dispatched when a connection is lost of could not be established. /// Remove explicitely a resource will NOT generate the event. /// When this event is received, the resource is considered already removed, /// the user do not need to remove it after this event. @@ -52,7 +51,7 @@ pub enum NetEvent<'a> { impl std::fmt::Debug for NetEvent<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let string = match self { - Self::Connected(endpoint, ok) => format!("Connected({}, {})", endpoint, ok), + Self::Connected(endpoint) => format!("Connected({})", endpoint), Self::Accepted(endpoint, id) => format!("Accepted({}, {})", endpoint, id), Self::Message(endpoint, data) => format!("Message({}, {})", endpoint, data.len()), Self::Disconnected(endpoint) => format!("Disconnected({})", endpoint), @@ -107,7 +106,7 @@ impl ActionController for Driver { R::connect(addr).map(|info| { ( Endpoint::new( - self.remote_registry.add(info.remote, info.peer_addr), + self.remote_registry.add(info.remote, info.peer_addr, false), info.peer_addr, ), info.local_addr, @@ -117,7 +116,7 @@ impl ActionController for Driver { fn listen(&self, addr: SocketAddr) -> io::Result<(ResourceId, SocketAddr)> { L::listen(addr) - .map(|info| (self.local_registry.add(info.local, info.local_addr), info.local_addr)) + .map(|info| (self.local_registry.add(info.local, info.local_addr, true), info.local_addr)) } fn send(&self, endpoint: Endpoint, data: &[u8]) -> SendStatus { @@ -147,11 +146,11 @@ impl> EventProcessor for Driver { ResourceType::Remote => match interest { Interest::Readable => self.process_remote_read(id, callback), Interest::Writable => self.process_remote_write(id, callback), - } + }, ResourceType::Local => match interest { Interest::Readable => self.process_local_read(id, callback), Interest::Writable => (), - } + }, } } } @@ -160,8 +159,11 @@ impl> Driver { fn process_remote_write(&self, id: ResourceId, mut event_callback: impl FnMut(NetEvent<'_>)) { if let Some(remote) = self.remote_registry.get(id) { let endpoint = Endpoint::new(id, remote.addr); - event_callback(NetEvent::Connected(endpoint, true)); //TODO - self.remote_registry.remove(id); + log::trace!("Processed remote for {}", endpoint); + if !remote.is_ready() { + remote.mark_as_ready(); + event_callback(NetEvent::Connected(endpoint)); + } } } @@ -190,7 +192,7 @@ impl> Driver { log::trace!("Processed local accepted type {}", accepted); match accepted { AcceptedType::Remote(addr, remote) => { - let remote_id = self.remote_registry.add(remote, addr); + let remote_id = self.remote_registry.add(remote, addr, true); let endpoint = Endpoint::new(remote_id, addr); event_callback(NetEvent::Accepted(endpoint, id)); } diff --git a/src/network/poll.rs b/src/network/poll.rs index 95fbb846..fcd7f6a4 100644 --- a/src/network/poll.rs +++ b/src/network/poll.rs @@ -107,7 +107,9 @@ impl PollRegistry { pub fn add(&self, source: &mut dyn Source) -> ResourceId { let id = self.id_generator.generate(); - self.registry.register(source, id.into(), MioInterest::READABLE | MioInterest::WRITABLE).unwrap(); + self.registry + .register(source, id.into(), MioInterest::READABLE | MioInterest::WRITABLE) + .unwrap(); log::trace!("Register to poll: {}", id); id } diff --git a/src/network/registry.rs b/src/network/registry.rs index dbd431ec..47c98984 100644 --- a/src/network/registry.rs +++ b/src/network/registry.rs @@ -6,17 +6,30 @@ use crate::util::thread::{OTHER_THREAD_ERR}; use std::collections::{HashMap}; use std::net::{SocketAddr}; -use std::sync::{Arc, RwLock}; +use std::sync::{Arc, RwLock, atomic::{AtomicBool, Ordering}}; pub struct Register { pub resource: S, + + // Most significant addr of the resource + // If the resource is a remote resource, the addr will be the peer addr. + // If the resource is a local resource, the addr will be the local addr. pub addr: SocketAddr, + ready: AtomicBool, poll_registry: Arc, } impl Register { - fn new(resource: S, addr: SocketAddr, poll_registry: Arc) -> Self { - Self { resource, addr, poll_registry } + fn new(resource: S, addr: SocketAddr, ready: bool, poll_registry: Arc) -> Self { + Self { resource, addr, ready: AtomicBool::new(ready), poll_registry } + } + + pub fn is_ready(&self) -> bool { + self.ready.load(Ordering::Relaxed) + } + + pub fn mark_as_ready(&self) { + self.ready.store(true, Ordering::Relaxed); } } @@ -27,10 +40,6 @@ impl Drop for Register { } pub struct ResourceRegistry { - // We store the most significant addr of the resource because if the resource disconnects, - // it can not be retrieved. - // If the resource is a remote resource, the addr will be the peer addr. - // If the resource is a local resource, the addr will be the local addr. resources: RwLock>>>, poll_registry: Arc, } @@ -44,9 +53,9 @@ impl ResourceRegistry { } /// Add a resource into the registry. - pub fn add(&self, mut resource: S, addr: SocketAddr) -> ResourceId { + pub fn add(&self, mut resource: S, addr: SocketAddr, ready: bool) -> ResourceId { let id = self.poll_registry.add(resource.source()); - let register = Register::new(resource, addr, self.poll_registry.clone()); + let register = Register::new(resource, addr, ready, self.poll_registry.clone()); self.resources.write().expect(OTHER_THREAD_ERR).insert(id, Arc::new(register)); id } diff --git a/src/node.rs b/src/node.rs index 0129a1fd..74e04776 100644 --- a/src/node.rs +++ b/src/node.rs @@ -110,7 +110,7 @@ impl From> for StoredNodeEvent { /// This kind of event is dispatched by `NodeListener::to_event_queue()`. #[derive(Debug, Clone)] pub enum StoredNetEvent { - Connected(Endpoint, bool), + Connected(Endpoint), Accepted(Endpoint, ResourceId), Message(Endpoint, Vec), Disconnected(Endpoint), @@ -119,7 +119,7 @@ pub enum StoredNetEvent { impl From> for StoredNetEvent { fn from(net_event: NetEvent<'_>) -> Self { match net_event { - NetEvent::Connected(endpoint, ok) => Self::Connected(endpoint, ok), + NetEvent::Connected(endpoint) => Self::Connected(endpoint), NetEvent::Accepted(endpoint, id) => Self::Accepted(endpoint, id), NetEvent::Message(endpoint, data) => Self::Message(endpoint, Vec::from(data)), NetEvent::Disconnected(endpoint) => Self::Disconnected(endpoint), @@ -131,7 +131,7 @@ impl StoredNetEvent { /// Use this `StoredNetEvent` as a `NetEvent` referencing its data. fn borrow(&self) -> NetEvent<'_> { match self { - Self::Connected(endpoint, ok) => NetEvent::Connected(*endpoint, *ok), + Self::Connected(endpoint) => NetEvent::Connected(*endpoint), Self::Accepted(endpoint, id) => NetEvent::Accepted(*endpoint, *id), Self::Message(endpoint, data) => NetEvent::Message(*endpoint, data), Self::Disconnected(endpoint) => NetEvent::Disconnected(*endpoint), diff --git a/tests/integration.rs b/tests/integration.rs index ecb81f98..dd4daf9e 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -100,7 +100,8 @@ fn start_echo_server( } } } - NetEvent::Connected(endpoint, id) => { + NetEvent::Connected(..) => unreachable!(), + NetEvent::Accepted(endpoint, id) => { assert_eq!(listener_id, id); match transport.is_connection_oriented() { true => assert!(clients.insert(endpoint)), @@ -141,10 +142,7 @@ fn start_echo_client_manager( let mut clients = HashSet::new(); for _ in 0..clients_number { - let (server, _) = node.network().connect(transport, server_addr).unwrap(); - let status = node.network().send(server, MIN_MESSAGE); - assert_eq!(SendStatus::Sent, status); - assert!(clients.insert(server)); + node.network().connect(transport, server_addr).unwrap(); } listener.for_each(move |event| match event { @@ -158,7 +156,12 @@ fn start_echo_client_manager( node.stop(); //Exit from thread. } } - NetEvent::Connected(..) => unreachable!(), + NetEvent::Connected(server) => { + let status = node.network().send(server, MIN_MESSAGE); + assert_eq!(SendStatus::Sent, status); + assert!(clients.insert(server)); + } + NetEvent::Accepted(..) => unreachable!(), NetEvent::Disconnected(_) => unreachable!(), }, }); @@ -189,7 +192,8 @@ fn start_burst_receiver( node.stop(); } } - NetEvent::Connected(..) => (), + NetEvent::Connected(..) => unreachable!(), + NetEvent::Accepted(..) => (), NetEvent::Disconnected(_) => (), }, }); @@ -271,7 +275,6 @@ fn message_size(transport: Transport, message_size: usize) { assert_eq!(status, SendStatus::Sent); } - // Protocols as TCP blocks the sender if the receiver is not reading data and its buffer is fill. let mut _async_sender: Option> = None; let mut received_message = Vec::new(); @@ -288,20 +291,21 @@ fn message_size(transport: Transport, message_size: usize) { received_message.extend_from_slice(&data); } } - NetEvent::Connected(..) => { - if transport.is_connection_oriented() { - let node = node.clone(); - let sent_message = sent_message.clone(); - _async_sender = Some(NamespacedThread::spawn("test-sender", move || { - let status = node.network().send(receiver, &sent_message); - assert_eq!(status, SendStatus::Sent); - assert!(node.network().remove(receiver.resource_id())); - })); - } - else { - unreachable!(); - } + NetEvent::Connected(endpoint) => { + assert_eq!(receiver, endpoint); + + let node = node.clone(); + let sent_message = sent_message.clone(); + + // Protocols as TCP blocks the sender if the receiver is not reading data + // and its buffer is fill. + _async_sender = Some(NamespacedThread::spawn("test-sender", move || { + let status = node.network().send(receiver, &sent_message); + assert_eq!(status, SendStatus::Sent); + assert!(node.network().remove(receiver.resource_id())); + })); } + NetEvent::Accepted(..) => (), NetEvent::Disconnected(_) => { assert_eq!(sent_message.len(), received_message.len()); assert_eq!(sent_message, received_message); From 09f3198763c3c8cba709cc7a640988d43c295c74 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Mon, 3 May 2021 09:13:11 +0200 Subject: [PATCH 03/25] All integration tests working except WS --- src/adapters/framed_tcp.rs | 11 ++++---- src/adapters/ws.rs | 9 ++----- src/network.rs | 2 -- src/network/driver.rs | 2 +- src/node.rs | 5 ++-- tests/integration.rs | 55 ++++++++++++++++++++------------------ 6 files changed, 40 insertions(+), 44 deletions(-) diff --git a/src/adapters/framed_tcp.rs b/src/adapters/framed_tcp.rs index 9218ea76..5cc84cbc 100644 --- a/src/adapters/framed_tcp.rs +++ b/src/adapters/framed_tcp.rs @@ -8,7 +8,7 @@ use crate::util::encoding::{self, Decoder, MAX_ENCODED_SIZE}; use mio::net::{TcpListener, TcpStream}; use mio::event::{Source}; -use std::net::{SocketAddr, TcpStream as StdTcpStream}; +use std::net::{SocketAddr}; use std::io::{self, ErrorKind, Read, Write}; use std::ops::{Deref}; use std::cell::{RefCell}; @@ -28,8 +28,8 @@ pub(crate) struct RemoteResource { } // SAFETY: -// That RefCell can be used with Sync because the decoder is only used in the read_event. -// This way, we save the cost of a Mutex. +// That RefCell can be used with Sync because the decoder is only used in the read_event, +// that will be called always from the same thread. This way, we save the cost of a Mutex. unsafe impl Sync for RemoteResource {} impl From for RemoteResource { @@ -47,10 +47,9 @@ impl Resource for RemoteResource { impl Remote for RemoteResource { fn connect(remote_addr: RemoteAddr) -> io::Result> { let peer_addr = *remote_addr.socket_addr(); - let stream = StdTcpStream::connect(peer_addr)?; + let stream = TcpStream::connect(peer_addr)?; let local_addr = stream.local_addr()?; - stream.set_nonblocking(true)?; - Ok(ConnectionInfo { remote: TcpStream::from_std(stream).into(), local_addr, peer_addr }) + Ok(ConnectionInfo { remote: stream.into(), local_addr, peer_addr }) } fn receive(&self, mut process_data: impl FnMut(&[u8])) -> ReadStatus { diff --git a/src/adapters/ws.rs b/src/adapters/ws.rs index 0506ec71..0523e708 100644 --- a/src/adapters/ws.rs +++ b/src/adapters/ws.rs @@ -20,7 +20,7 @@ use tungstenite::error::{Error}; use url::Url; use std::sync::{Mutex}; -use std::net::{SocketAddr, TcpStream as StdTcpStream}; +use std::net::{SocketAddr}; use std::io::{self, ErrorKind}; use std::ops::{DerefMut}; @@ -77,14 +77,9 @@ impl Remote for RemoteResource { } }; - // Synchronous tcp handshake - let stream = StdTcpStream::connect(peer_addr)?; + let stream = TcpStream::connect(peer_addr)?; let local_addr = stream.local_addr()?; - // Make it an asynchronous mio TcpStream - stream.set_nonblocking(true)?; - let stream = TcpStream::from_std(stream); - // Synchronous waiting for web socket handshake let mut handshake_result = ws_connect(url, stream); let remote = loop { diff --git a/src/network.rs b/src/network.rs index 0a18e236..88ed5e2d 100644 --- a/src/network.rs +++ b/src/network.rs @@ -186,8 +186,6 @@ mod tests { let (listener_id, addr) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); let (endpoint, _) = controller.connect(Transport::Tcp, RemoteAddr::Socket(addr)).unwrap(); - dbg!(addr); - let mut was_connected = 0; let mut was_accepted = 0; for _ in 0..2 { diff --git a/src/network/driver.rs b/src/network/driver.rs index a10f3734..4ce696d0 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -23,7 +23,7 @@ pub enum NetEvent<'a> { /// Note that this event will only be generated by connection-oriented transports as *TCP*. Connected(Endpoint), - /// New endpoint has been accepted by a listener. + /// New endpoint has been accepted by a listener and is ready to use. /// The event contains the resource id of the listener that accepted this connection. /// /// Note that this event will only be generated by connection-oriented transports as *TCP*. diff --git a/src/node.rs b/src/node.rs index 74e04776..53b800c9 100644 --- a/src/node.rs +++ b/src/node.rs @@ -106,8 +106,9 @@ impl From> for StoredNodeEvent { } } -/// Analogous to [`NetEvent`] but without reference the data. -/// This kind of event is dispatched by `NodeListener::to_event_queue()`. +/// Analogous to [`NetEvent`] but with static lifetime (without reference the data). +/// This kind of event is dispatched by `NodeListener::to_event_queue()` +/// and can be easily stored in any container. #[derive(Debug, Clone)] pub enum StoredNetEvent { Connected(Endpoint), diff --git a/tests/integration.rs b/tests/integration.rs index dd4daf9e..59bdf38f 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -82,6 +82,14 @@ fn start_echo_server( listener.for_each(move |event| match event { NodeEvent::Signal(_) => panic!("{}", TIMEOUT_EVENT_RECV_ERR), NodeEvent::Network(net_event) => match net_event { + NetEvent::Connected(..) => unreachable!(), + NetEvent::Accepted(endpoint, id) => { + assert_eq!(listener_id, id); + match transport.is_connection_oriented() { + true => assert!(clients.insert(endpoint)), + false => unreachable!(), + } + } NetEvent::Message(endpoint, data) => { assert_eq!(MIN_MESSAGE, data); @@ -100,14 +108,6 @@ fn start_echo_server( } } } - NetEvent::Connected(..) => unreachable!(), - NetEvent::Accepted(endpoint, id) => { - assert_eq!(listener_id, id); - match transport.is_connection_oriented() { - true => assert!(clients.insert(endpoint)), - false => unreachable!(), - } - } NetEvent::Disconnected(endpoint) => { match transport.is_connection_oriented() { true => { @@ -140,6 +140,7 @@ fn start_echo_client_manager( node.signals().send_with_timer((), *TIMEOUT); let mut clients = HashSet::new(); + let mut received = 0; for _ in 0..clients_number { node.network().connect(transport, server_addr).unwrap(); @@ -148,19 +149,21 @@ fn start_echo_client_manager( listener.for_each(move |event| match event { NodeEvent::Signal(_) => panic!("{}", TIMEOUT_EVENT_RECV_ERR), NodeEvent::Network(net_event) => match net_event { + NetEvent::Connected(server) => { + let status = node.network().send(server, MIN_MESSAGE); + assert_eq!(SendStatus::Sent, status); + assert!(clients.insert(server)); + } NetEvent::Message(endpoint, data) => { assert!(clients.remove(&endpoint)); assert_eq!(MIN_MESSAGE, data); node.network().remove(endpoint.resource_id()); - if clients.len() == 0 { + + received += 1; + if received == clients_number { node.stop(); //Exit from thread. } } - NetEvent::Connected(server) => { - let status = node.network().send(server, MIN_MESSAGE); - assert_eq!(SendStatus::Sent, status); - assert!(clients.insert(server)); - } NetEvent::Accepted(..) => unreachable!(), NetEvent::Disconnected(_) => unreachable!(), }, @@ -184,6 +187,8 @@ fn start_burst_receiver( listener.for_each(move |event| match event { NodeEvent::Signal(_) => panic!("{}", TIMEOUT_EVENT_RECV_ERR), NodeEvent::Network(net_event) => match net_event { + NetEvent::Connected(..) => unreachable!(), + NetEvent::Accepted(..) => (), NetEvent::Message(_, data) => { let expected_message = format!("{}: {}", SMALL_MESSAGE, count); assert_eq!(expected_message, String::from_utf8_lossy(&data)); @@ -192,8 +197,6 @@ fn start_burst_receiver( node.stop(); } } - NetEvent::Connected(..) => unreachable!(), - NetEvent::Accepted(..) => (), NetEvent::Disconnected(_) => (), }, }); @@ -281,16 +284,6 @@ fn message_size(transport: Transport, message_size: usize) { listener.for_each(move |event| match event { NodeEvent::Signal(_) => panic!("{}", TIMEOUT_EVENT_RECV_ERR), NodeEvent::Network(net_event) => match net_event { - NetEvent::Message(_, data) => { - if transport.is_packet_based() { - received_message = data.to_vec(); - assert_eq!(sent_message, received_message); - node.stop(); - } - else { - received_message.extend_from_slice(&data); - } - } NetEvent::Connected(endpoint) => { assert_eq!(receiver, endpoint); @@ -306,6 +299,16 @@ fn message_size(transport: Transport, message_size: usize) { })); } NetEvent::Accepted(..) => (), + NetEvent::Message(_, data) => { + if transport.is_packet_based() { + received_message = data.to_vec(); + assert_eq!(sent_message, received_message); + node.stop(); + } + else { + received_message.extend_from_slice(&data); + } + } NetEvent::Disconnected(_) => { assert_eq!(sent_message.len(), received_message.len()); assert_eq!(sent_message, received_message); From d3013d207397b67a19bfd6b5daf025719983958b Mon Sep 17 00:00:00 2001 From: lemunozm Date: Mon, 3 May 2021 14:44:43 +0200 Subject: [PATCH 04/25] Fixed registry concurrence problem --- src/network.rs | 8 ++++--- src/network/driver.rs | 27 +++++++++++++++-------- src/network/poll.rs | 1 - src/network/registry.rs | 5 ++++- src/node.rs | 6 ++--- tests/integration.rs | 49 ++++++++++++++++++++++++++++++----------- 6 files changed, 66 insertions(+), 30 deletions(-) diff --git a/src/network.rs b/src/network.rs index 88ed5e2d..9ec3278f 100644 --- a/src/network.rs +++ b/src/network.rs @@ -190,7 +190,8 @@ mod tests { let mut was_accepted = 0; for _ in 0..2 { processor.process_poll_event(Some(*TIMEOUT), |net_event| match net_event { - NetEvent::Connected(net_endpoint) => { + NetEvent::Ready(net_endpoint, status) => { + assert!(status); assert_eq!(endpoint, net_endpoint); was_connected += 1; } @@ -214,7 +215,8 @@ mod tests { let mut was_disconnected = false; processor.process_poll_event(Some(*TIMEOUT), |net_event| match net_event { - NetEvent::Disconnected(net_endpoint) => { + NetEvent::Ready(net_endpoint, status) => { + assert!(!status); assert_eq!(endpoint, net_endpoint); was_disconnected = true; } @@ -244,7 +246,7 @@ mod tests { for _ in 0..2 { // We expect two events processor.process_poll_event(Some(*TIMEOUT), |net_event| match net_event { - NetEvent::Connected(_) => (), + NetEvent::Ready(..) => (), NetEvent::Accepted(_, _) => { assert!(controller.remove(listener_id)); assert!(!controller.remove(listener_id)); diff --git a/src/network/driver.rs b/src/network/driver.rs index 4ce696d0..a0f5ab7a 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -14,14 +14,18 @@ use super::transport::{Transport}; /// Enum used to describe a network event that an internal transport adapter has produced. pub enum NetEvent<'a> { - /// New endpoint has been connected. + /// New endpoint is ready to use. /// This event is only generated after a [`crate::network::NetworkController::connect()`] /// call. /// The event contains the endpoint of the connection - /// (same endpoint returned by the `connect()` method). - /// - /// Note that this event will only be generated by connection-oriented transports as *TCP*. - Connected(Endpoint), + /// (same endpoint returned by the `connect()` method), + /// and a boolean indication if the *status*. + /// In *non connection-oriented transports* as *UDP* it simply means that the resource + /// is ready to use, and the status will be always `true`. + /// In connection-oriented transports it means that the handshake has been performed, and the + /// connection is established and ready for its usage. + /// Since this handshake could fail, the status could be `false`. + Ready(Endpoint, bool), /// New endpoint has been accepted by a listener and is ready to use. /// The event contains the resource id of the listener that accepted this connection. @@ -37,7 +41,7 @@ pub enum NetEvent<'a> { /// [`crate::transport::Transport::FramedTcp`]. Message(Endpoint, &'a [u8]), - /// This event is only dispatched when a connection is lost of could not be established. + /// This event is only dispatched when a connection is lost. /// Remove explicitely a resource will NOT generate the event. /// When this event is received, the resource is considered already removed, /// the user do not need to remove it after this event. @@ -51,7 +55,7 @@ pub enum NetEvent<'a> { impl std::fmt::Debug for NetEvent<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let string = match self { - Self::Connected(endpoint) => format!("Connected({})", endpoint), + Self::Ready(endpoint, status) => format!("Ready({}, {})", endpoint, status), Self::Accepted(endpoint, id) => format!("Accepted({}, {})", endpoint, id), Self::Message(endpoint, data) => format!("Message({}, {})", endpoint, data.len()), Self::Disconnected(endpoint) => format!("Disconnected({})", endpoint), @@ -162,7 +166,7 @@ impl> Driver { log::trace!("Processed remote for {}", endpoint); if !remote.is_ready() { remote.mark_as_ready(); - event_callback(NetEvent::Connected(endpoint)); + event_callback(NetEvent::Ready(endpoint, true)); } } } @@ -179,7 +183,12 @@ impl> Driver { if let ReadStatus::Disconnected = status { // Checked becasue, the user in the callback could have removed the same resource. if self.remote_registry.remove(id) { - event_callback(NetEvent::Disconnected(endpoint)); + if remote.is_ready() { + event_callback(NetEvent::Disconnected(endpoint)); + } + else { + event_callback(NetEvent::Ready(endpoint, false)); + } } } } diff --git a/src/network/poll.rs b/src/network/poll.rs index fcd7f6a4..8e1d5232 100644 --- a/src/network/poll.rs +++ b/src/network/poll.rs @@ -110,7 +110,6 @@ impl PollRegistry { self.registry .register(source, id.into(), MioInterest::READABLE | MioInterest::WRITABLE) .unwrap(); - log::trace!("Register to poll: {}", id); id } diff --git a/src/network/registry.rs b/src/network/registry.rs index 47c98984..3a8f9a8a 100644 --- a/src/network/registry.rs +++ b/src/network/registry.rs @@ -54,9 +54,12 @@ impl ResourceRegistry { /// Add a resource into the registry. pub fn add(&self, mut resource: S, addr: SocketAddr, ready: bool) -> ResourceId { + // The registry must be locked for the entire implementation to avoid + // poll to generate events over false unregistered resources. + let mut registry = self.resources.write().expect(OTHER_THREAD_ERR); let id = self.poll_registry.add(resource.source()); let register = Register::new(resource, addr, ready, self.poll_registry.clone()); - self.resources.write().expect(OTHER_THREAD_ERR).insert(id, Arc::new(register)); + registry.insert(id, Arc::new(register)); id } diff --git a/src/node.rs b/src/node.rs index 53b800c9..f18b3797 100644 --- a/src/node.rs +++ b/src/node.rs @@ -111,7 +111,7 @@ impl From> for StoredNodeEvent { /// and can be easily stored in any container. #[derive(Debug, Clone)] pub enum StoredNetEvent { - Connected(Endpoint), + Ready(Endpoint, bool), Accepted(Endpoint, ResourceId), Message(Endpoint, Vec), Disconnected(Endpoint), @@ -120,7 +120,7 @@ pub enum StoredNetEvent { impl From> for StoredNetEvent { fn from(net_event: NetEvent<'_>) -> Self { match net_event { - NetEvent::Connected(endpoint) => Self::Connected(endpoint), + NetEvent::Ready(endpoint, status) => Self::Ready(endpoint, status), NetEvent::Accepted(endpoint, id) => Self::Accepted(endpoint, id), NetEvent::Message(endpoint, data) => Self::Message(endpoint, Vec::from(data)), NetEvent::Disconnected(endpoint) => Self::Disconnected(endpoint), @@ -132,7 +132,7 @@ impl StoredNetEvent { /// Use this `StoredNetEvent` as a `NetEvent` referencing its data. fn borrow(&self) -> NetEvent<'_> { match self { - Self::Connected(endpoint) => NetEvent::Connected(*endpoint), + Self::Ready(endpoint, status) => NetEvent::Ready(*endpoint, *status), Self::Accepted(endpoint, id) => NetEvent::Accepted(*endpoint, *id), Self::Message(endpoint, data) => NetEvent::Message(*endpoint, data), Self::Disconnected(endpoint) => NetEvent::Disconnected(*endpoint), diff --git a/tests/integration.rs b/tests/integration.rs index 59bdf38f..893118c3 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -82,7 +82,7 @@ fn start_echo_server( listener.for_each(move |event| match event { NodeEvent::Signal(_) => panic!("{}", TIMEOUT_EVENT_RECV_ERR), NodeEvent::Network(net_event) => match net_event { - NetEvent::Connected(..) => unreachable!(), + NetEvent::Ready(..) => unreachable!(), NetEvent::Accepted(endpoint, id) => { assert_eq!(listener_id, id); match transport.is_connection_oriented() { @@ -149,7 +149,8 @@ fn start_echo_client_manager( listener.for_each(move |event| match event { NodeEvent::Signal(_) => panic!("{}", TIMEOUT_EVENT_RECV_ERR), NodeEvent::Network(net_event) => match net_event { - NetEvent::Connected(server) => { + NetEvent::Ready(server, status) => { + assert!(status); let status = node.network().send(server, MIN_MESSAGE); assert_eq!(SendStatus::Sent, status); assert!(clients.insert(server)); @@ -187,7 +188,7 @@ fn start_burst_receiver( listener.for_each(move |event| match event { NodeEvent::Signal(_) => panic!("{}", TIMEOUT_EVENT_RECV_ERR), NodeEvent::Network(net_event) => match net_event { - NetEvent::Connected(..) => unreachable!(), + NetEvent::Ready(..) => unreachable!(), NetEvent::Accepted(..) => (), NetEvent::Message(_, data) => { let expected_message = format!("{}: {}", SMALL_MESSAGE, count); @@ -210,19 +211,40 @@ fn start_burst_sender( expected_count: usize, ) -> NamespacedThread<()> { NamespacedThread::spawn("test-sender", move || { - let (node, _) = node::split::<()>(); + let (node, listener) = node::split::<()>(); let (receiver, _) = node.network().connect(transport, receiver_addr).unwrap(); - for count in 0..expected_count { - let message = format!("{}: {}", SMALL_MESSAGE, count); - let status = node.network().send(receiver, message.as_bytes()); - assert_eq!(SendStatus::Sent, status); - if !transport.is_connection_oriented() { - // We need a rate to not lose packet. - std::thread::sleep(Duration::from_micros(20)); + let mut count = 0; + listener.for_each(move |event| match event { + NodeEvent::Signal(_) => { + if count < expected_count { + let message = format!("{}: {}", SMALL_MESSAGE, count); + let status = node.network().send(receiver, message.as_bytes()); + assert_eq!(SendStatus::Sent, status); + + count += 1; + if !transport.is_connection_oriented() { + // We need a rate to not lose packet. + node.signals().send_with_timer((), Duration::from_micros(20)); + } + else { + node.signals().send(()); + } + } + else { + node.stop(); + } + }, + NodeEvent::Network(net_event) => match net_event { + NetEvent::Ready(_, status) => { + assert!(status); + node.signals().send(()); + }, + NetEvent::Disconnected(_) => (), + _ => unreachable!(), } - } + }); }) } @@ -284,7 +306,8 @@ fn message_size(transport: Transport, message_size: usize) { listener.for_each(move |event| match event { NodeEvent::Signal(_) => panic!("{}", TIMEOUT_EVENT_RECV_ERR), NodeEvent::Network(net_event) => match net_event { - NetEvent::Connected(endpoint) => { + NetEvent::Ready(endpoint, status) => { + assert!(status); assert_eq!(receiver, endpoint); let node = node.clone(); From 72ecbe0f08cf039f026d126bf269e619396d5f78 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Tue, 4 May 2021 21:39:17 +0200 Subject: [PATCH 05/25] Ws with client handshake schema. New adapter schema --- src/adapters/ws.rs | 81 ++++++++++++++++++++++-------------------- src/network/adapter.rs | 30 ++++++++++++---- src/network/driver.rs | 2 +- 3 files changed, 68 insertions(+), 45 deletions(-) diff --git a/src/adapters/ws.rs b/src/adapters/ws.rs index 0523e708..39ccca67 100644 --- a/src/adapters/ws.rs +++ b/src/adapters/ws.rs @@ -14,6 +14,7 @@ use tungstenite::client::{client as ws_connect}; use tungstenite::handshake::{ HandshakeError, MidHandshake, server::{ServerHandshake, NoCallback}, + client::{ClientHandshake}, }; use tungstenite::error::{Error}; @@ -34,9 +35,9 @@ impl Adapter for WsAdapter { type Local = LocalResource; } -struct PendingHandshake { - mid_handshake: MidHandshake>, - pending_messages: Vec>, +enum PendingHandshake { + Client(MidHandshake>), + Server(MidHandshake>), } enum RemoteState { @@ -52,7 +53,10 @@ impl Resource for RemoteResource { fn source(&mut self) -> &mut dyn Source { match self.state.get_mut().unwrap() { RemoteState::WebSocket(web_socket) => web_socket.get_mut(), - RemoteState::Handshake(Some(handshake)) => handshake.mid_handshake.get_mut().get_mut(), + RemoteState::Handshake(Some(handshake)) => match handshake { + PendingHandshake::Client(mid_handshake) => mid_handshake.get_mut().get_mut(), + PendingHandshake::Server(mid_handshake) => mid_handshake.get_mut().get_mut(), + } RemoteState::Handshake(None) => unreachable!(), } } @@ -80,24 +84,26 @@ impl Remote for RemoteResource { let stream = TcpStream::connect(peer_addr)?; let local_addr = stream.local_addr()?; - // Synchronous waiting for web socket handshake - let mut handshake_result = ws_connect(url, stream); - let remote = loop { - match handshake_result { - Ok((web_socket, _)) => { - break RemoteResource { state: Mutex::new(RemoteState::WebSocket(web_socket)) } - } - Err(HandshakeError::Interrupted(mid_handshake)) => { - handshake_result = mid_handshake.handshake(); - } - Err(HandshakeError::Failure(err)) => { - //CHECK: give to the user an io::Error? - panic!("WS connect handshake error: {}", err) - } + let remote = match ws_connect(url, stream) { + Ok((web_socket, _)) => { + RemoteState::WebSocket(web_socket) + } + Err(HandshakeError::Interrupted(mid_handshake)) => { + RemoteState::Handshake(Some(PendingHandshake::Client(mid_handshake))) + } + Err(HandshakeError::Failure(Error::Io(err))) => return Err(err), + Err(HandshakeError::Failure(err)) => { + panic!("WS connect handshake error: {}", err) } }; - Ok(ConnectionInfo { remote, local_addr, peer_addr }) + Ok(ConnectionInfo { + remote: RemoteResource { + state: Mutex::new(remote) + }, + local_addr, + peer_addr + }) } fn receive(&self, mut process_data: impl FnMut(&[u8])) -> ReadStatus { @@ -136,20 +142,25 @@ impl Remote for RemoteResource { break ReadStatus::Disconnected // should not happen } }, - RemoteState::Handshake(handshake) => { - let current_handshake = handshake.take().unwrap(); - match current_handshake.mid_handshake.handshake() { - Ok(mut web_socket) => { - for pending_data in current_handshake.pending_messages { - Self::send_by_socket(&mut web_socket, &pending_data); - } + RemoteState::Handshake(pending) => match pending.take().unwrap() { + PendingHandshake::Client(mid_handshake) => match mid_handshake.handshake() { + Ok((web_socket, _)) => { *state = RemoteState::WebSocket(web_socket); } Err(HandshakeError::Interrupted(mid_handshake)) => { - *handshake = Some(PendingHandshake { - mid_handshake, - pending_messages: current_handshake.pending_messages, - }); + *state = RemoteState::Handshake(Some(PendingHandshake::Client(mid_handshake))) + } + Err(HandshakeError::Failure(err)) => { + //CHECK: give to the user an io::Error? + panic!("WS connect handshake error: {}", err) + } + } + PendingHandshake::Server(mid_handshake) => match mid_handshake.handshake() { + Ok(web_socket) => { + *state = RemoteState::WebSocket(web_socket); + } + Err(HandshakeError::Interrupted(mid_handshake)) => { + *pending = Some(PendingHandshake::Server(mid_handshake)); break ReadStatus::WaitNextEvent } Err(HandshakeError::Failure(ref err)) => { @@ -165,10 +176,7 @@ impl Remote for RemoteResource { fn send(&self, data: &[u8]) -> SendStatus { match self.state.lock().expect(OTHER_THREAD_ERR).deref_mut() { RemoteState::WebSocket(web_socket) => Self::send_by_socket(web_socket, data), - RemoteState::Handshake(handshake) => { - handshake.as_mut().unwrap().pending_messages.push(data.to_vec()); - SendStatus::Sent //Future versions: SendStatus::Enqueued - } + RemoteState::Handshake(handshake) => SendStatus::Sent, } } } @@ -234,10 +242,7 @@ impl Local for LocalResource { let remote_state = match ws_accept(stream) { Ok(web_socket) => Some(RemoteState::WebSocket(web_socket)), Err(HandshakeError::Interrupted(mid_handshake)) => { - Some(RemoteState::Handshake(Some(PendingHandshake { - mid_handshake, - pending_messages: Vec::new(), - }))) + Some(RemoteState::Handshake(Some(PendingHandshake::Server(mid_handshake)))) } Err(HandshakeError::Failure(ref err)) => { log::error!("WS accept handshake error: {}", err); diff --git a/src/network/adapter.rs b/src/network/adapter.rs index fb6ad493..c3d0ece4 100644 --- a/src/network/adapter.rs +++ b/src/network/adapter.rs @@ -40,6 +40,9 @@ pub struct ConnectionInfo { /// Peer address of the interal resource used. pub peer_addr: SocketAddr, + + /// Determines if the resource is ready. + pub ready: bool, } /// Plain struct used as a returned value of [`Local::listen()`] @@ -102,17 +105,28 @@ pub trait Remote: Resource + Sized { /// It means that the resource has available data to read, /// or there is some connection related issue, as a disconnection. /// The **implementator** is in charge of processing that action and returns a [`ReadStatus`]. + /// /// The `process_data` function must be called for each data chunk that represents a message. - /// This call will produce a `Message` API event. + /// This call will produce a [`crate::network::NetEvent::Message`] API event. /// Note that `receive()` could imply more than one call to `read`. /// The implementator must be read all data from the resource. /// For most of the cases it means read until the network resource returns `WouldBlock`. - fn receive(&self, process_data: impl FnMut(&[u8])) -> ReadStatus; - - /// Sends a raw data from a resource. + /// + /// The `ready` function must be called if some data makes this resource available for use. + /// For example: some data received as part as a handshake establishes successful the connection. + /// It implies that call to `ready` will generates a [`crate::network::NetEvent::Accepted`] or + /// [`crate::network::NetEvent::Ready`] depending if the resource was generated by a listener + /// or by the user itself as part of a connect. + /// Note that only the first call to ready will the expected effect. + /// Successive calls will be ignored. + fn receive(&self, process_data: impl FnMut(&[u8]), ready: impl FnMut()) -> ReadStatus; + + /// Sends raw data from a resource. /// The **implementator** is in charge to send the entire `data`. /// The [`SendStatus`] will contain the status of this attempt. - fn send(&self, data: &[u8]) -> SendStatus; + /// + /// The `ready` function parameter has the same functionality as [`Remote::receive()`] parameter. + fn send(&self, data: &[u8], ready: impl FnMut()) -> SendStatus; } /// Used as a parameter callback in [`Local::accept()`] @@ -120,7 +134,11 @@ pub enum AcceptedType<'a, R> { /// The listener has accepted a remote (`R`) with the specified addr. /// The remote will be registered in order to generate read events. (calls to /// [`Remote::receive()`]). - Remote(SocketAddr, R), + /// This remote will be visible to the user as an `Accepted` event if it is set as ready + /// (boolean value as true). + /// Otherwise, it will be registered, but the user will not be notified until it's mark as ready. + /// See `ready` parameter of [`Remote::receive()`] and [`Remote::send()`]. + Remote(SocketAddr, R, bool), /// The listener has accepted data that can be packed into a message from a specified addr. /// Despite of `Remote`, accept as a `Data` will not register any Remote. diff --git a/src/network/driver.rs b/src/network/driver.rs index a0f5ab7a..b20df1c9 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -27,7 +27,7 @@ pub enum NetEvent<'a> { /// Since this handshake could fail, the status could be `false`. Ready(Endpoint, bool), - /// New endpoint has been accepted by a listener and is ready to use. + /// New endpoint has been accepted by a listener and considered ready to use. /// The event contains the resource id of the listener that accepted this connection. /// /// Note that this event will only be generated by connection-oriented transports as *TCP*. From 18c8e13e414a9ce7f59d18ec4f28c22d3beafdce Mon Sep 17 00:00:00 2001 From: lemunozm Date: Tue, 4 May 2021 20:11:32 +0200 Subject: [PATCH 06/25] pending() adapter concept --- src/adapters/ws.rs | 30 +++-- src/network.rs | 10 +- src/network/adapter.rs | 74 ++++++++---- src/network/driver.rs | 250 ++++++++++++++++++++++++++-------------- src/network/loader.rs | 4 +- src/network/poll.rs | 31 ++--- src/network/registry.rs | 45 +++----- src/node.rs | 7 +- tests/integration.rs | 24 ++-- 9 files changed, 283 insertions(+), 192 deletions(-) diff --git a/src/adapters/ws.rs b/src/adapters/ws.rs index 39ccca67..795652e7 100644 --- a/src/adapters/ws.rs +++ b/src/adapters/ws.rs @@ -56,7 +56,7 @@ impl Resource for RemoteResource { RemoteState::Handshake(Some(handshake)) => match handshake { PendingHandshake::Client(mid_handshake) => mid_handshake.get_mut().get_mut(), PendingHandshake::Server(mid_handshake) => mid_handshake.get_mut().get_mut(), - } + }, RemoteState::Handshake(None) => unreachable!(), } } @@ -85,9 +85,7 @@ impl Remote for RemoteResource { let local_addr = stream.local_addr()?; let remote = match ws_connect(url, stream) { - Ok((web_socket, _)) => { - RemoteState::WebSocket(web_socket) - } + Ok((web_socket, _)) => RemoteState::WebSocket(web_socket), Err(HandshakeError::Interrupted(mid_handshake)) => { RemoteState::Handshake(Some(PendingHandshake::Client(mid_handshake))) } @@ -98,11 +96,9 @@ impl Remote for RemoteResource { }; Ok(ConnectionInfo { - remote: RemoteResource { - state: Mutex::new(remote) - }, + remote: RemoteResource { state: Mutex::new(remote) }, local_addr, - peer_addr + peer_addr, }) } @@ -148,13 +144,15 @@ impl Remote for RemoteResource { *state = RemoteState::WebSocket(web_socket); } Err(HandshakeError::Interrupted(mid_handshake)) => { - *state = RemoteState::Handshake(Some(PendingHandshake::Client(mid_handshake))) + *state = RemoteState::Handshake(Some(PendingHandshake::Client( + mid_handshake, + ))) } Err(HandshakeError::Failure(err)) => { //CHECK: give to the user an io::Error? panic!("WS connect handshake error: {}", err) } - } + }, PendingHandshake::Server(mid_handshake) => match mid_handshake.handshake() { Ok(web_socket) => { *state = RemoteState::WebSocket(web_socket); @@ -167,8 +165,8 @@ impl Remote for RemoteResource { log::error!("WS accept handshake error: {}", err); break ReadStatus::Disconnected // should not happen } - } - } + }, + }, } } } @@ -176,7 +174,7 @@ impl Remote for RemoteResource { fn send(&self, data: &[u8]) -> SendStatus { match self.state.lock().expect(OTHER_THREAD_ERR).deref_mut() { RemoteState::WebSocket(web_socket) => Self::send_by_socket(web_socket, data), - RemoteState::Handshake(handshake) => SendStatus::Sent, + RemoteState::Handshake(_) => SendStatus::Sent, } } } @@ -241,9 +239,9 @@ impl Local for LocalResource { Ok((stream, addr)) => { let remote_state = match ws_accept(stream) { Ok(web_socket) => Some(RemoteState::WebSocket(web_socket)), - Err(HandshakeError::Interrupted(mid_handshake)) => { - Some(RemoteState::Handshake(Some(PendingHandshake::Server(mid_handshake)))) - } + Err(HandshakeError::Interrupted(mid_handshake)) => Some( + RemoteState::Handshake(Some(PendingHandshake::Server(mid_handshake))), + ), Err(HandshakeError::Failure(ref err)) => { log::error!("WS accept handshake error: {}", err); None diff --git a/src/network.rs b/src/network.rs index 9ec3278f..8d733bfa 100644 --- a/src/network.rs +++ b/src/network.rs @@ -190,7 +190,7 @@ mod tests { let mut was_accepted = 0; for _ in 0..2 { processor.process_poll_event(Some(*TIMEOUT), |net_event| match net_event { - NetEvent::Ready(net_endpoint, status) => { + NetEvent::Connected(net_endpoint, status) => { assert!(status); assert_eq!(endpoint, net_endpoint); was_connected += 1; @@ -199,7 +199,7 @@ mod tests { assert_eq!(listener_id, net_listener_id); was_accepted += 1; } - _ => unreachable!() + _ => unreachable!(), }); } assert_eq!(was_connected, 1); @@ -215,12 +215,12 @@ mod tests { let mut was_disconnected = false; processor.process_poll_event(Some(*TIMEOUT), |net_event| match net_event { - NetEvent::Ready(net_endpoint, status) => { + NetEvent::Connected(net_endpoint, status) => { assert!(!status); assert_eq!(endpoint, net_endpoint); was_disconnected = true; } - _ => unreachable!() + _ => unreachable!(), }); assert!(was_disconnected); @@ -246,7 +246,7 @@ mod tests { for _ in 0..2 { // We expect two events processor.process_poll_event(Some(*TIMEOUT), |net_event| match net_event { - NetEvent::Ready(..) => (), + NetEvent::Connected(..) => (), NetEvent::Accepted(_, _) => { assert!(controller.remove(listener_id)); assert!(!controller.remove(listener_id)); diff --git a/src/network/adapter.rs b/src/network/adapter.rs index c3d0ece4..4a7aac76 100644 --- a/src/network/adapter.rs +++ b/src/network/adapter.rs @@ -1,4 +1,5 @@ -use crate::network::{RemoteAddr}; +use super::remote_addr::{RemoteAddr}; +use super::poll::{Readiness}; use mio::event::{Source}; @@ -40,9 +41,6 @@ pub struct ConnectionInfo { /// Peer address of the interal resource used. pub peer_addr: SocketAddr, - - /// Determines if the resource is ready. - pub ready: bool, } /// Plain struct used as a returned value of [`Local::listen()`] @@ -78,6 +76,7 @@ pub enum SendStatus { } /// Returned as a result of [`Remote::receive()`] +#[derive(Debug)] pub enum ReadStatus { /// This status must be returned if the resource has been disconnected or there was an error. /// The resource will be removed after this call and @@ -91,6 +90,26 @@ pub enum ReadStatus { WaitNextEvent, } +#[derive(Debug)] +pub enum PendingStatus { + /// The resource is no longer considered as a pending resource. + /// It it came from a listener, a [`crate::network::NetEvent::Accepted`] event will be generated. + /// It it came from a explicit connection, a [`crate::network::NetEvent::Connected`] + /// with its flag to `true` will be generated. + /// No more calls to [`Remote::pending()`] will be performed. + Ready, + + /// The resource needs more data to be considered as established. + Incomplete, + + /// The resource has not be able to perform the connection. + /// It it came from a listener, no event will be generated. + /// It it came from a explicit connection, a [`crate::network::NetEvent::Connected`] + /// with its flag to `false` will be generated. + /// No more calls to [`Remote::pending()`] will be performed and the resource will be removed. + Disconnected, +} + /// The resource used to represent a remote. /// It usually is a wrapper over a socket/stream. pub trait Remote: Resource + Sized { @@ -111,22 +130,30 @@ pub trait Remote: Resource + Sized { /// Note that `receive()` could imply more than one call to `read`. /// The implementator must be read all data from the resource. /// For most of the cases it means read until the network resource returns `WouldBlock`. - /// - /// The `ready` function must be called if some data makes this resource available for use. - /// For example: some data received as part as a handshake establishes successful the connection. - /// It implies that call to `ready` will generates a [`crate::network::NetEvent::Accepted`] or - /// [`crate::network::NetEvent::Ready`] depending if the resource was generated by a listener - /// or by the user itself as part of a connect. - /// Note that only the first call to ready will the expected effect. - /// Successive calls will be ignored. - fn receive(&self, process_data: impl FnMut(&[u8]), ready: impl FnMut()) -> ReadStatus; + fn receive(&self, process_data: impl FnMut(&[u8])) -> ReadStatus; /// Sends raw data from a resource. /// The **implementator** is in charge to send the entire `data`. /// The [`SendStatus`] will contain the status of this attempt. + fn send(&self, data: &[u8]) -> SendStatus; + + /// Called when a `Remote` is created (explicity of by a listener) or when it is not considered + /// read but it has Read/Write readiness. + /// No `Connected` or `Accepted` events will be generated until this function return + /// `PendingStatus::Ready`. /// - /// The `ready` function parameter has the same functionality as [`Remote::receive()`] parameter. - fn send(&self, data: &[u8], ready: impl FnMut()) -> SendStatus; + /// Implementing this function is optional. + /// The default implementation will consider ready the socket when it was ready to write into it. + /// Yo need to implement it if youtr protocol needs some necessary handshake for consider the + /// resource ready to use. + /// The **implementator** is in charge to write or read from the resource + /// (depending of the readiness) until it obtains an internal [`std::io::ErrorKind::WouldBlock`]. + fn pending(&self, readiness: Readiness) -> PendingStatus { + match readiness { + Readiness::Write => PendingStatus::Ready, + Readiness::Read => PendingStatus::Disconnected, + } + } } /// Used as a parameter callback in [`Local::accept()`] @@ -134,19 +161,15 @@ pub enum AcceptedType<'a, R> { /// The listener has accepted a remote (`R`) with the specified addr. /// The remote will be registered in order to generate read events. (calls to /// [`Remote::receive()`]). - /// This remote will be visible to the user as an `Accepted` event if it is set as ready - /// (boolean value as true). - /// Otherwise, it will be registered, but the user will not be notified until it's mark as ready. - /// See `ready` parameter of [`Remote::receive()`] and [`Remote::send()`]. - Remote(SocketAddr, R, bool), + /// A [`crate::network::NetEvent::Accepted`] will be generated once this remote resource + /// is considered *ready*. + Remote(SocketAddr, R), /// The listener has accepted data that can be packed into a message from a specified addr. /// Despite of `Remote`, accept as a `Data` will not register any Remote. - /// This will produce a `Message` API event. - /// The endpoint along this event will be unique if base of the specified addr and the listener + /// This will produce a [`crate::network::NetEvent::Message`] event. + /// The endpoint of this event will be unique containing the specified addr and the listener /// whom generates it. - /// This means that the user can treat the [`crate::network::Endpoint`] as if - /// it was an internal resource. Data(SocketAddr, &'a [u8]), } @@ -176,9 +199,10 @@ pub trait Local: Resource + Sized { /// Sends a raw data from a resource. /// Similar to [`Remote::send()`] but the resource that sends the data is a `Local`. + /// This behaviour usually happens when the transport to implement is not connection oriented. + /// /// The **implementator** must **only** implement this function if the local resource can /// also send data. - /// This behaviour usually happens when the transport to implement is not connection oriented. fn send_to(&self, _addr: SocketAddr, _data: &[u8]) -> SendStatus { panic!("Adapter not configured to send messages directly from the local resource") } diff --git a/src/network/driver.rs b/src/network/driver.rs index b20df1c9..18435cb4 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -1,12 +1,15 @@ use super::endpoint::{Endpoint}; use super::resource_id::{ResourceId, ResourceType}; -use super::poll::{Poll, Interest}; -use super::registry::{ResourceRegistry}; +use super::poll::{Poll, Readiness}; +use super::registry::{ResourceRegistry, Register}; use super::remote_addr::{RemoteAddr}; -use super::adapter::{Adapter, Remote, Local, SendStatus, AcceptedType, ReadStatus}; +use super::adapter::{Adapter, Remote, Local, SendStatus, AcceptedType, ReadStatus, PendingStatus}; use std::net::{SocketAddr}; -use std::sync::{Arc}; +use std::sync::{ + Arc, + atomic::{AtomicBool, Ordering}, +}; use std::io::{self}; #[cfg(doctest)] @@ -14,18 +17,18 @@ use super::transport::{Transport}; /// Enum used to describe a network event that an internal transport adapter has produced. pub enum NetEvent<'a> { - /// New endpoint is ready to use. + /// Connection result. /// This event is only generated after a [`crate::network::NetworkController::connect()`] /// call. /// The event contains the endpoint of the connection /// (same endpoint returned by the `connect()` method), - /// and a boolean indication if the *status*. + /// and a boolean indicating the *status* of that connection. /// In *non connection-oriented transports* as *UDP* it simply means that the resource /// is ready to use, and the status will be always `true`. /// In connection-oriented transports it means that the handshake has been performed, and the - /// connection is established and ready for its usage. + /// connection is established and ready to use. /// Since this handshake could fail, the status could be `false`. - Ready(Endpoint, bool), + Connected(Endpoint, bool), /// New endpoint has been accepted by a listener and considered ready to use. /// The event contains the resource id of the listener that accepted this connection. @@ -55,7 +58,7 @@ pub enum NetEvent<'a> { impl std::fmt::Debug for NetEvent<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let string = match self { - Self::Ready(endpoint, status) => format!("Ready({}, {})", endpoint, status), + Self::Connected(endpoint, status) => format!("Connected({}, {})", endpoint, status), Self::Accepted(endpoint, id) => format!("Accepted({}, {})", endpoint, id), Self::Message(endpoint, data) => format!("Message({}, {})", endpoint, data.len()), Self::Disconnected(endpoint) => format!("Disconnected({})", endpoint), @@ -72,12 +75,34 @@ pub trait ActionController: Send + Sync { } pub trait EventProcessor: Send + Sync { - fn process(&self, id: ResourceId, interest: Interest, callback: &mut dyn FnMut(NetEvent<'_>)); + fn process(&self, id: ResourceId, readiness: Readiness, callback: &mut dyn FnMut(NetEvent<'_>)); } +struct RemoteProperties { + peer_addr: SocketAddr, + local: Option, + ready: AtomicBool, +} + +impl RemoteProperties { + fn new(peer_addr: SocketAddr, local: Option) -> Self { + Self { peer_addr, local, ready: AtomicBool::new(false) } + } + + pub fn is_ready(&self) -> bool { + self.ready.load(Ordering::Relaxed) + } + + pub fn mark_as_ready(&self) { + self.ready.store(true, Ordering::Relaxed); + } +} + +struct LocalProperties; + pub struct Driver { - remote_registry: Arc>, - local_registry: Arc>, + remote_registry: Arc>, + local_registry: Arc>, } impl Driver { @@ -90,8 +115,12 @@ impl Driver { let local_poll_registry = poll.create_registry(adapter_id, ResourceType::Local); Driver { - remote_registry: Arc::new(ResourceRegistry::::new(remote_poll_registry)), - local_registry: Arc::new(ResourceRegistry::::new(local_poll_registry)), + remote_registry: Arc::new(ResourceRegistry::::new( + remote_poll_registry, + )), + local_registry: Arc::new(ResourceRegistry::::new( + local_poll_registry, + )), } } } @@ -108,19 +137,20 @@ impl Clone for Driver { impl ActionController for Driver { fn connect(&self, addr: RemoteAddr) -> io::Result<(Endpoint, SocketAddr)> { R::connect(addr).map(|info| { - ( - Endpoint::new( - self.remote_registry.add(info.remote, info.peer_addr, false), - info.peer_addr, - ), - info.local_addr, - ) + let endpoint = Endpoint::new( + self.remote_registry + .register(info.remote, RemoteProperties::new(info.peer_addr, None)), + info.peer_addr, + ); + (endpoint, info.local_addr) }) } fn listen(&self, addr: SocketAddr) -> io::Result<(ResourceId, SocketAddr)> { - L::listen(addr) - .map(|info| (self.local_registry.add(info.local, info.local_addr, true), info.local_addr)) + L::listen(addr).map(|info| { + let id = self.local_registry.register(info.local, LocalProperties); + (id, info.local_addr) + }) } fn send(&self, endpoint: Endpoint, data: &[u8]) -> SendStatus { @@ -138,90 +168,136 @@ impl ActionController for Driver { fn remove(&self, id: ResourceId) -> bool { match id.resource_type() { - ResourceType::Remote => self.remote_registry.remove(id), - ResourceType::Local => self.local_registry.remove(id), + ResourceType::Remote => self.remote_registry.deregister(id), + ResourceType::Local => self.local_registry.deregister(id), } } } impl> EventProcessor for Driver { - fn process(&self, id: ResourceId, interest: Interest, callback: &mut dyn FnMut(NetEvent<'_>)) { + fn process( + &self, + id: ResourceId, + readiness: Readiness, + event_callback: &mut dyn FnMut(NetEvent<'_>), + ) { match id.resource_type() { - ResourceType::Remote => match interest { - Interest::Readable => self.process_remote_read(id, callback), - Interest::Writable => self.process_remote_write(id, callback), - }, - ResourceType::Local => match interest { - Interest::Readable => self.process_local_read(id, callback), - Interest::Writable => (), - }, + ResourceType::Remote => { + if let Some(remote) = self.remote_registry.get(id) { + let endpoint = Endpoint::new(id, remote.properties.peer_addr); + log::trace!("Processed remote for {}", endpoint); + + if !remote.properties.is_ready() { + self.process_pending_remote(remote, endpoint, readiness, event_callback); + } + else { + match readiness { + Readiness::Write => { + () //TODO: non-blocking send + //self.write_to_remote(remote, endpoint, event_callback); + } + Readiness::Read => { + self.read_from_remote(remote, endpoint, event_callback); + } + } + } + } + } + ResourceType::Local => { + if let Some(local) = self.local_registry.get(id) { + log::trace!("Processed local for {}", id); + match readiness { + Readiness::Write => (), // TODO: non-blocking send + Readiness::Read => self.read_from_local(local, id, event_callback), + } + } + } } } } impl> Driver { - fn process_remote_write(&self, id: ResourceId, mut event_callback: impl FnMut(NetEvent<'_>)) { - if let Some(remote) = self.remote_registry.get(id) { - let endpoint = Endpoint::new(id, remote.addr); - log::trace!("Processed remote for {}", endpoint); - if !remote.is_ready() { - remote.mark_as_ready(); - event_callback(NetEvent::Ready(endpoint, true)); + fn process_pending_remote( + &self, + remote: Arc>, + endpoint: Endpoint, + readiness: Readiness, + mut event_callback: impl FnMut(NetEvent<'_>), + ) { + match remote.resource.pending(readiness) { + PendingStatus::Ready => { + remote.properties.mark_as_ready(); + match remote.properties.local { + Some(listener_id) => event_callback(NetEvent::Accepted(endpoint, listener_id)), + None => event_callback(NetEvent::Connected(endpoint, true)), + } } - } - } - - fn process_remote_read(&self, id: ResourceId, mut event_callback: impl FnMut(NetEvent<'_>)) { - if let Some(remote) = self.remote_registry.get(id) { - let endpoint = Endpoint::new(id, remote.addr); - log::trace!("Processed remote for {}", endpoint); - let status = remote.resource.receive(|data| { - event_callback(NetEvent::Message(endpoint, data)); - }); - log::trace!("Processed remote receive status {}", status); - - if let ReadStatus::Disconnected = status { - // Checked becasue, the user in the callback could have removed the same resource. - if self.remote_registry.remove(id) { - if remote.is_ready() { - event_callback(NetEvent::Disconnected(endpoint)); - } - else { - event_callback(NetEvent::Ready(endpoint, false)); - } + PendingStatus::Incomplete => (), + PendingStatus::Disconnected => { + self.remote_registry.deregister(endpoint.resource_id()); + if remote.properties.local.is_none() { + event_callback(NetEvent::Connected(endpoint, false)); } } } } - fn process_local_read(&self, id: ResourceId, mut event_callback: impl FnMut(NetEvent<'_>)) { - if let Some(local) = self.local_registry.get(id) { - log::trace!("Processed local for {}", id); - local.resource.accept(|accepted| { - log::trace!("Processed local accepted type {}", accepted); - match accepted { - AcceptedType::Remote(addr, remote) => { - let remote_id = self.remote_registry.add(remote, addr, true); - let endpoint = Endpoint::new(remote_id, addr); - event_callback(NetEvent::Accepted(endpoint, id)); - } - AcceptedType::Data(addr, data) => { - let endpoint = Endpoint::new(id, addr); - event_callback(NetEvent::Message(endpoint, data)); - } - } - }); + /* + fn write_to_remote( + &self, + remote: Arc>, + endpoint: Endpoint, + mut event_callback: impl FnMut(NetEvent<'_>) + ) { + todo!() + } + */ + + fn read_from_remote( + &self, + remote: Arc>, + endpoint: Endpoint, + mut event_callback: impl FnMut(NetEvent<'_>), + ) { + let status = + remote.resource.receive(|data| event_callback(NetEvent::Message(endpoint, data))); + log::trace!("Receive status: {:?}", status); + if let ReadStatus::Disconnected = status { + // Checked because, the user in the callback could have removed the same resource. + if self.remote_registry.deregister(endpoint.resource_id()) { + event_callback(NetEvent::Disconnected(endpoint)); + } } } -} -impl std::fmt::Display for ReadStatus { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let string = match self { - ReadStatus::Disconnected => "Disconnected", - ReadStatus::WaitNextEvent => "WaitNextEvent", - }; - write!(f, "ReadStatus::{}", string) + fn read_from_local( + &self, + local: Arc>, + id: ResourceId, + mut event_callback: impl FnMut(NetEvent<'_>), + ) { + local.resource.accept(|accepted| { + log::trace!("Accepted type: {}", accepted); + match accepted { + AcceptedType::Remote(addr, remote) => { + let remote_id = self + .remote_registry + .register(remote, RemoteProperties::new(addr, Some(id))); + + // We Manually generate a Poll Write event, because an accepted connection + // is ready to write. + let remote = self.remote_registry.get(remote_id).unwrap(); + let endpoint = Endpoint::new(remote_id, addr); + self.process_pending_remote(remote, endpoint, Readiness::Write, |net_event| { + event_callback(net_event) + }); + } + AcceptedType::Data(addr, data) => { + let endpoint = Endpoint::new(id, addr); + event_callback(NetEvent::Message(endpoint, data)); + } + } + }); } } diff --git a/src/network/loader.rs b/src/network/loader.rs index 0f01041f..3e6b1058 100644 --- a/src/network/loader.rs +++ b/src/network/loader.rs @@ -1,6 +1,6 @@ use super::endpoint::{Endpoint}; use super::resource_id::{ResourceId}; -use super::poll::{Poll, Interest}; +use super::poll::{Poll, Readiness}; use super::remote_addr::{RemoteAddr}; use super::driver::{NetEvent, Driver, ActionController, EventProcessor}; use super::adapter::{Adapter, SendStatus}; @@ -80,7 +80,7 @@ impl ActionController for UnimplementedDriver { } impl EventProcessor for UnimplementedDriver { - fn process(&self, _: ResourceId, _: Interest, _: &mut dyn FnMut(NetEvent<'_>)) { + fn process(&self, _: ResourceId, _: Readiness, _: &mut dyn FnMut(NetEvent<'_>)) { panic!("{}", UNIMPLEMENTED_DRIVER_ERR); } } diff --git a/src/network/poll.rs b/src/network/poll.rs index 8e1d5232..8f776f73 100644 --- a/src/network/poll.rs +++ b/src/network/poll.rs @@ -1,19 +1,20 @@ use super::resource_id::{ResourceId, ResourceType, ResourceIdGenerator}; -use mio::{Poll as MioPoll, Interest as MioInterest, Token, Events, Registry, Waker}; +use mio::{Poll as MioPoll, Interest, Token, Events, Registry, Waker}; use mio::event::{Source}; use std::time::{Duration}; use std::sync::{Arc}; use std::io::{ErrorKind}; -pub enum Interest { - Readable, - Writable, +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum Readiness { + Write, + Read, } pub enum PollEvent { - Network(ResourceId, Interest), + Network(ResourceId, Readiness), Waker, } @@ -63,14 +64,14 @@ impl Poll { event_callback(PollEvent::Waker); } else { - let resource_id = ResourceId::from(mio_event.token()); + let id = ResourceId::from(mio_event.token()); if mio_event.is_readable() { - log::trace!("POLL EVENT (R): {}", resource_id); - event_callback(PollEvent::Network(resource_id, Interest::Readable)); + log::trace!("POLL EVENT (R): {}", id); + event_callback(PollEvent::Network(id, Readiness::Read)); } if mio_event.is_writable() { - log::trace!("POLL EVENT (W): {}", resource_id); - event_callback(PollEvent::Network(resource_id, Interest::Writable)); + log::trace!("POLL EVENT (W): {}", id); + event_callback(PollEvent::Network(id, Readiness::Write)); } } } @@ -107,9 +108,7 @@ impl PollRegistry { pub fn add(&self, source: &mut dyn Source) -> ResourceId { let id = self.id_generator.generate(); - self.registry - .register(source, id.into(), MioInterest::READABLE | MioInterest::WRITABLE) - .unwrap(); + self.registry.register(source, id.into(), Interest::READABLE | Interest::WRITABLE).unwrap(); id } @@ -144,3 +143,9 @@ impl PollWaker { log::trace!("Wake poll..."); } } + +impl Clone for PollWaker { + fn clone(&self) -> Self { + Self { waker: self.waker.clone() } + } +} diff --git a/src/network/registry.rs b/src/network/registry.rs index 3a8f9a8a..e7ca9349 100644 --- a/src/network/registry.rs +++ b/src/network/registry.rs @@ -5,46 +5,33 @@ use super::adapter::{Resource}; use crate::util::thread::{OTHER_THREAD_ERR}; use std::collections::{HashMap}; -use std::net::{SocketAddr}; -use std::sync::{Arc, RwLock, atomic::{AtomicBool, Ordering}}; +use std::sync::{Arc, RwLock}; -pub struct Register { +pub struct Register { pub resource: S, + pub properties: P, - // Most significant addr of the resource - // If the resource is a remote resource, the addr will be the peer addr. - // If the resource is a local resource, the addr will be the local addr. - pub addr: SocketAddr, - ready: AtomicBool, poll_registry: Arc, } -impl Register { - fn new(resource: S, addr: SocketAddr, ready: bool, poll_registry: Arc) -> Self { - Self { resource, addr, ready: AtomicBool::new(ready), poll_registry } - } - - pub fn is_ready(&self) -> bool { - self.ready.load(Ordering::Relaxed) - } - - pub fn mark_as_ready(&self) { - self.ready.store(true, Ordering::Relaxed); +impl Register { + fn new(resource: S, properties: P, poll_registry: Arc) -> Self { + Self { resource, properties, poll_registry } } } -impl Drop for Register { +impl Drop for Register { fn drop(&mut self) { self.poll_registry.remove(self.resource.source()); } } -pub struct ResourceRegistry { - resources: RwLock>>>, +pub struct ResourceRegistry { + resources: RwLock>>>, poll_registry: Arc, } -impl ResourceRegistry { +impl ResourceRegistry { pub fn new(poll_registry: PollRegistry) -> Self { ResourceRegistry { resources: RwLock::new(HashMap::new()), @@ -53,12 +40,12 @@ impl ResourceRegistry { } /// Add a resource into the registry. - pub fn add(&self, mut resource: S, addr: SocketAddr, ready: bool) -> ResourceId { - // The registry must be locked for the entire implementation to avoid - // poll to generate events over false unregistered resources. + pub fn register(&self, mut resource: S, properties: P) -> ResourceId { + // The registry must be locked for the entire implementation to avoid the poll + // to generate events over not yet registered resources. let mut registry = self.resources.write().expect(OTHER_THREAD_ERR); let id = self.poll_registry.add(resource.source()); - let register = Register::new(resource, addr, ready, self.poll_registry.clone()); + let register = Register::new(resource, properties, self.poll_registry.clone()); registry.insert(id, Arc::new(register)); id } @@ -67,12 +54,12 @@ impl ResourceRegistry { /// This function ensure that the register is removed from the registry, /// but not the destruction of the resource itself. /// Because the resource is shared, the destruction will be delayed until the last reference. - pub fn remove(&self, id: ResourceId) -> bool { + pub fn deregister(&self, id: ResourceId) -> bool { self.resources.write().expect(OTHER_THREAD_ERR).remove(&id).is_some() } /// Returned a shared reference of the register. - pub fn get(&self, id: ResourceId) -> Option>> { + pub fn get(&self, id: ResourceId) -> Option>> { self.resources.read().expect(OTHER_THREAD_ERR).get(&id).cloned() } } diff --git a/src/node.rs b/src/node.rs index f18b3797..47ab9137 100644 --- a/src/node.rs +++ b/src/node.rs @@ -111,7 +111,7 @@ impl From> for StoredNodeEvent { /// and can be easily stored in any container. #[derive(Debug, Clone)] pub enum StoredNetEvent { - Ready(Endpoint, bool), + Connected(Endpoint, bool), Accepted(Endpoint, ResourceId), Message(Endpoint, Vec), Disconnected(Endpoint), @@ -120,7 +120,7 @@ pub enum StoredNetEvent { impl From> for StoredNetEvent { fn from(net_event: NetEvent<'_>) -> Self { match net_event { - NetEvent::Ready(endpoint, status) => Self::Ready(endpoint, status), + NetEvent::Connected(endpoint, status) => Self::Connected(endpoint, status), NetEvent::Accepted(endpoint, id) => Self::Accepted(endpoint, id), NetEvent::Message(endpoint, data) => Self::Message(endpoint, Vec::from(data)), NetEvent::Disconnected(endpoint) => Self::Disconnected(endpoint), @@ -132,7 +132,7 @@ impl StoredNetEvent { /// Use this `StoredNetEvent` as a `NetEvent` referencing its data. fn borrow(&self) -> NetEvent<'_> { match self { - Self::Ready(endpoint, status) => NetEvent::Ready(*endpoint, *status), + Self::Connected(endpoint, status) => NetEvent::Connected(*endpoint, *status), Self::Accepted(endpoint, id) => NetEvent::Accepted(*endpoint, *id), Self::Message(endpoint, data) => NetEvent::Message(*endpoint, data), Self::Disconnected(endpoint) => NetEvent::Disconnected(*endpoint), @@ -330,6 +330,7 @@ impl NodeListener { scope .builder() + .name(String::from("node-network-thread")) .spawn(move |_| { while handler.is_running() { if let Some(signal) = signal_receiver.receive_timeout(*SAMPLING_TIMEOUT) diff --git a/tests/integration.rs b/tests/integration.rs index 893118c3..139443c5 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -82,7 +82,7 @@ fn start_echo_server( listener.for_each(move |event| match event { NodeEvent::Signal(_) => panic!("{}", TIMEOUT_EVENT_RECV_ERR), NodeEvent::Network(net_event) => match net_event { - NetEvent::Ready(..) => unreachable!(), + NetEvent::Connected(..) => unreachable!(), NetEvent::Accepted(endpoint, id) => { assert_eq!(listener_id, id); match transport.is_connection_oriented() { @@ -149,7 +149,7 @@ fn start_echo_client_manager( listener.for_each(move |event| match event { NodeEvent::Signal(_) => panic!("{}", TIMEOUT_EVENT_RECV_ERR), NodeEvent::Network(net_event) => match net_event { - NetEvent::Ready(server, status) => { + NetEvent::Connected(server, status) => { assert!(status); let status = node.network().send(server, MIN_MESSAGE); assert_eq!(SendStatus::Sent, status); @@ -188,7 +188,7 @@ fn start_burst_receiver( listener.for_each(move |event| match event { NodeEvent::Signal(_) => panic!("{}", TIMEOUT_EVENT_RECV_ERR), NodeEvent::Network(net_event) => match net_event { - NetEvent::Ready(..) => unreachable!(), + NetEvent::Connected(..) => unreachable!(), NetEvent::Accepted(..) => (), NetEvent::Message(_, data) => { let expected_message = format!("{}: {}", SMALL_MESSAGE, count); @@ -235,15 +235,15 @@ fn start_burst_sender( else { node.stop(); } - }, + } NodeEvent::Network(net_event) => match net_event { - NetEvent::Ready(_, status) => { + NetEvent::Connected(_, status) => { assert!(status); node.signals().send(()); - }, + } NetEvent::Disconnected(_) => (), _ => unreachable!(), - } + }, }); }) } @@ -254,8 +254,8 @@ fn start_burst_sender( #[cfg_attr(feature = "tcp", test_case(Transport::FramedTcp, 100))] #[cfg_attr(feature = "udp", test_case(Transport::Udp, 1))] #[cfg_attr(feature = "udp", test_case(Transport::Udp, 100))] -#[cfg_attr(feature = "websocket", test_case(Transport::Ws, 1))] -#[cfg_attr(feature = "websocket", test_case(Transport::Ws, 100))] +//#[cfg_attr(feature = "websocket", test_case(Transport::Ws, 1))] +//#[cfg_attr(feature = "websocket", test_case(Transport::Ws, 100))] // NOTE: A medium-high `clients` value can exceeds the "open file" limits of an OS in CI // with an obfuscated error message. fn echo(transport: Transport, clients: usize) { @@ -268,7 +268,7 @@ fn echo(transport: Transport, clients: usize) { // Tcp: Does not apply: it's stream based #[cfg_attr(feature = "udp", test_case(Transport::Udp, 2000))] #[cfg_attr(feature = "tcp", test_case(Transport::FramedTcp, 200000))] -#[cfg_attr(feature = "websocket", test_case(Transport::Ws, 200000))] +//#[cfg_attr(feature = "websocket", test_case(Transport::Ws, 200000))] fn burst(transport: Transport, messages_count: usize) { //util::init_logger(LogThread::Enabled); // Enable it for better debugging @@ -279,7 +279,7 @@ fn burst(transport: Transport, messages_count: usize) { #[cfg_attr(feature = "tcp", test_case(Transport::Tcp, BIG_MESSAGE_SIZE))] #[cfg_attr(feature = "tcp", test_case(Transport::FramedTcp, BIG_MESSAGE_SIZE))] #[cfg_attr(feature = "udp", test_case(Transport::Udp, udp::MAX_COMPATIBLE_PAYLOAD_LEN))] -#[cfg_attr(feature = "websocket", test_case(Transport::Ws, BIG_MESSAGE_SIZE))] +//#[cfg_attr(feature = "websocket", test_case(Transport::Ws, BIG_MESSAGE_SIZE))] fn message_size(transport: Transport, message_size: usize) { //util::init_logger(LogThread::Enabled); // Enable it for better debugging @@ -306,7 +306,7 @@ fn message_size(transport: Transport, message_size: usize) { listener.for_each(move |event| match event { NodeEvent::Signal(_) => panic!("{}", TIMEOUT_EVENT_RECV_ERR), NodeEvent::Network(net_event) => match net_event { - NetEvent::Ready(endpoint, status) => { + NetEvent::Connected(endpoint, status) => { assert!(status); assert_eq!(receiver, endpoint); From 7e98a81a702a63ac9e948a6b09010d383c01f2ca Mon Sep 17 00:00:00 2001 From: lemunozm Date: Tue, 4 May 2021 21:14:54 +0200 Subject: [PATCH 07/25] added ready_to_write --- src/network/adapter.rs | 14 ++++++++++++-- src/network/driver.rs | 25 ++++++------------------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/network/adapter.rs b/src/network/adapter.rs index 4a7aac76..d98d6551 100644 --- a/src/network/adapter.rs +++ b/src/network/adapter.rs @@ -120,8 +120,9 @@ pub trait Remote: Resource + Sized { /// It also must return the extracted address as `SocketAddr`. fn connect(remote_addr: RemoteAddr) -> io::Result>; - /// Called when a remote endpoint received an event. - /// It means that the resource has available data to read, + /// Called when a remote resource received an event. + /// The resource must be *ready* to receive this call. + /// It means that it has available data to read, /// or there is some connection related issue, as a disconnection. /// The **implementator** is in charge of processing that action and returns a [`ReadStatus`]. /// @@ -133,10 +134,16 @@ pub trait Remote: Resource + Sized { fn receive(&self, process_data: impl FnMut(&[u8])) -> ReadStatus; /// Sends raw data from a resource. + /// The resource must be *ready* to receive this call. /// The **implementator** is in charge to send the entire `data`. /// The [`SendStatus`] will contain the status of this attempt. fn send(&self, data: &[u8]) -> SendStatus; + /// The resource is available to write. + /// It must be *ready* to receive this call. + /// Here the **implementator** optionally can try to write any pending data. + fn ready_to_write(&self) {} + /// Called when a `Remote` is created (explicity of by a listener) or when it is not considered /// read but it has Read/Write readiness. /// No `Connected` or `Accepted` events will be generated until this function return @@ -151,6 +158,9 @@ pub trait Remote: Resource + Sized { fn pending(&self, readiness: Readiness) -> PendingStatus { match readiness { Readiness::Write => PendingStatus::Ready, + + // If before getting a Write readiness it obtains a Read readiness + // means that a disconnection or a connection error has been produced. Readiness::Read => PendingStatus::Disconnected, } } diff --git a/src/network/driver.rs b/src/network/driver.rs index 18435cb4..d82e9fc1 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -22,12 +22,12 @@ pub enum NetEvent<'a> { /// call. /// The event contains the endpoint of the connection /// (same endpoint returned by the `connect()` method), - /// and a boolean indicating the *status* of that connection. + /// and a boolean indicating the *result* of that connection. /// In *non connection-oriented transports* as *UDP* it simply means that the resource - /// is ready to use, and the status will be always `true`. + /// is ready to use, and the boolean will be always `true`. /// In connection-oriented transports it means that the handshake has been performed, and the /// connection is established and ready to use. - /// Since this handshake could fail, the status could be `false`. + /// Since this handshake could fail, the boolean could be `false`. Connected(Endpoint, bool), /// New endpoint has been accepted by a listener and considered ready to use. @@ -192,10 +192,7 @@ impl> EventProcessor for Driver { } else { match readiness { - Readiness::Write => { - () //TODO: non-blocking send - //self.write_to_remote(remote, endpoint, event_callback); - } + Readiness::Write => remote.resource.ready_to_write(), Readiness::Read => { self.read_from_remote(remote, endpoint, event_callback); } @@ -207,7 +204,7 @@ impl> EventProcessor for Driver { if let Some(local) = self.local_registry.get(id) { log::trace!("Processed local for {}", id); match readiness { - Readiness::Write => (), // TODO: non-blocking send + Readiness::Write => (), Readiness::Read => self.read_from_local(local, id, event_callback), } } @@ -231,6 +228,7 @@ impl> Driver { Some(listener_id) => event_callback(NetEvent::Accepted(endpoint, listener_id)), None => event_callback(NetEvent::Connected(endpoint, true)), } + remote.resource.ready_to_write(); } PendingStatus::Incomplete => (), PendingStatus::Disconnected => { @@ -242,17 +240,6 @@ impl> Driver { } } - /* - fn write_to_remote( - &self, - remote: Arc>, - endpoint: Endpoint, - mut event_callback: impl FnMut(NetEvent<'_>) - ) { - todo!() - } - */ - fn read_from_remote( &self, remote: Arc>, From 771b8e851d4506cd5228a1240d0e4c9399772764 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Tue, 4 May 2021 22:55:56 +0200 Subject: [PATCH 08/25] Websocket working with async connections --- src/adapters/ws.rs | 115 ++++++++++++++++++++++++------------------- src/network.rs | 1 + src/network/poll.rs | 5 ++ tests/integration.rs | 8 +-- 4 files changed, 74 insertions(+), 55 deletions(-) diff --git a/src/adapters/ws.rs b/src/adapters/ws.rs index 795652e7..fbc16ec8 100644 --- a/src/adapters/ws.rs +++ b/src/adapters/ws.rs @@ -1,8 +1,8 @@ use crate::network::adapter::{ Resource, Remote, Local, Adapter, SendStatus, AcceptedType, ReadStatus, ConnectionInfo, - ListeningInfo, + ListeningInfo, PendingStatus, }; -use crate::network::{RemoteAddr}; +use crate::network::{RemoteAddr, Readiness}; use crate::util::thread::{OTHER_THREAD_ERR}; use mio::event::{Source}; @@ -138,68 +138,81 @@ impl Remote for RemoteResource { break ReadStatus::Disconnected // should not happen } }, - RemoteState::Handshake(pending) => match pending.take().unwrap() { - PendingHandshake::Client(mid_handshake) => match mid_handshake.handshake() { - Ok((web_socket, _)) => { - *state = RemoteState::WebSocket(web_socket); - } - Err(HandshakeError::Interrupted(mid_handshake)) => { - *state = RemoteState::Handshake(Some(PendingHandshake::Client( - mid_handshake, - ))) - } - Err(HandshakeError::Failure(err)) => { - //CHECK: give to the user an io::Error? - panic!("WS connect handshake error: {}", err) - } + RemoteState::Handshake(_) => unreachable!(), + } + } + } + + fn pending(&self, _readiness: Readiness) -> PendingStatus { + let mut state = self.state.lock().expect(OTHER_THREAD_ERR); + match state.deref_mut() { + RemoteState::WebSocket(_) => PendingStatus::Ready, + RemoteState::Handshake(pending) => match pending.take().unwrap() { + PendingHandshake::Client(mid_handshake) => match mid_handshake.handshake() { + Ok((web_socket, _)) => { + *state = RemoteState::WebSocket(web_socket); + PendingStatus::Ready + } + Err(HandshakeError::Interrupted(mid_handshake)) => { + *pending = Some(PendingHandshake::Client(mid_handshake)); + PendingStatus::Incomplete + } + Err(HandshakeError::Failure(Error::Io(_))) => { + PendingStatus::Disconnected }, - PendingHandshake::Server(mid_handshake) => match mid_handshake.handshake() { - Ok(web_socket) => { - *state = RemoteState::WebSocket(web_socket); - } - Err(HandshakeError::Interrupted(mid_handshake)) => { - *pending = Some(PendingHandshake::Server(mid_handshake)); - break ReadStatus::WaitNextEvent - } - Err(HandshakeError::Failure(ref err)) => { - log::error!("WS accept handshake error: {}", err); - break ReadStatus::Disconnected // should not happen - } + Err(HandshakeError::Failure(err)) => { + log::error!("WS connect handshake error: {}", err); + PendingStatus::Disconnected // should not happen + } + }, + PendingHandshake::Server(mid_handshake) => match mid_handshake.handshake() { + Ok(web_socket) => { + *state = RemoteState::WebSocket(web_socket); + PendingStatus::Ready + } + Err(HandshakeError::Interrupted(mid_handshake)) => { + *pending = Some(PendingHandshake::Server(mid_handshake)); + PendingStatus::Incomplete + } + Err(HandshakeError::Failure(Error::Io(_))) => { + PendingStatus::Disconnected }, + Err(HandshakeError::Failure(err)) => { + log::error!("WS accept handshake error: {}", err); + PendingStatus::Disconnected // should not happen + } }, - } + }, } } fn send(&self, data: &[u8]) -> SendStatus { match self.state.lock().expect(OTHER_THREAD_ERR).deref_mut() { - RemoteState::WebSocket(web_socket) => Self::send_by_socket(web_socket, data), - RemoteState::Handshake(_) => SendStatus::Sent, + RemoteState::WebSocket(web_socket) => { + let message = Message::Binary(data.to_vec()); + let mut result = web_socket.write_message(message); + loop { + match result { + Ok(_) => break SendStatus::Sent, + Err(Error::Io(ref err)) if err.kind() == ErrorKind::WouldBlock => { + result = web_socket.write_pending(); + } + Err(Error::Capacity(_)) => { + break SendStatus::MaxPacketSizeExceeded(data.len(), MAX_PAYLOAD_LEN) + } + Err(err) => { + log::error!("WS send error: {}", err); + break SendStatus::ResourceNotFound // should not happen + } + } + } + }, + RemoteState::Handshake(_) => unreachable!(), } } } impl RemoteResource { - fn send_by_socket(web_socket: &mut WebSocket, data: &[u8]) -> SendStatus { - let message = Message::Binary(data.to_vec()); - let mut result = web_socket.write_message(message); - loop { - match result { - Ok(_) => break SendStatus::Sent, - Err(Error::Io(ref err)) if err.kind() == ErrorKind::WouldBlock => { - result = web_socket.write_pending(); - } - Err(Error::Capacity(_)) => { - break SendStatus::MaxPacketSizeExceeded(data.len(), MAX_PAYLOAD_LEN) - } - Err(err) => { - log::error!("WS send error: {}", err); - break SendStatus::ResourceNotFound // should not happen - } - } - } - } - fn io_error_to_read_status(err: &io::Error) -> ReadStatus { if err.kind() == io::ErrorKind::WouldBlock { ReadStatus::WaitNextEvent diff --git a/src/network.rs b/src/network.rs index 8d733bfa..e7addc34 100644 --- a/src/network.rs +++ b/src/network.rs @@ -19,6 +19,7 @@ pub use endpoint::{Endpoint}; pub use remote_addr::{RemoteAddr, ToRemoteAddr}; pub use transport::{Transport}; pub use driver::{NetEvent}; +pub use poll::{Readiness}; use loader::{DriverLoader, ActionControllerList, EventProcessorList}; use poll::{Poll, PollEvent}; diff --git a/src/network/poll.rs b/src/network/poll.rs index 8f776f73..c26ddbcf 100644 --- a/src/network/poll.rs +++ b/src/network/poll.rs @@ -8,8 +8,13 @@ use std::sync::{Arc}; use std::io::{ErrorKind}; #[derive(Clone, Copy, Debug, PartialEq)] +/// Used for the adapter implementation. +/// Specify the kind of event that is available for a resource. pub enum Readiness { + /// The resource is available to write Write, + + /// The resource is available to read (has any content to read). Read, } diff --git a/tests/integration.rs b/tests/integration.rs index 139443c5..737011aa 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -254,8 +254,8 @@ fn start_burst_sender( #[cfg_attr(feature = "tcp", test_case(Transport::FramedTcp, 100))] #[cfg_attr(feature = "udp", test_case(Transport::Udp, 1))] #[cfg_attr(feature = "udp", test_case(Transport::Udp, 100))] -//#[cfg_attr(feature = "websocket", test_case(Transport::Ws, 1))] -//#[cfg_attr(feature = "websocket", test_case(Transport::Ws, 100))] +#[cfg_attr(feature = "websocket", test_case(Transport::Ws, 1))] +#[cfg_attr(feature = "websocket", test_case(Transport::Ws, 100))] // NOTE: A medium-high `clients` value can exceeds the "open file" limits of an OS in CI // with an obfuscated error message. fn echo(transport: Transport, clients: usize) { @@ -268,7 +268,7 @@ fn echo(transport: Transport, clients: usize) { // Tcp: Does not apply: it's stream based #[cfg_attr(feature = "udp", test_case(Transport::Udp, 2000))] #[cfg_attr(feature = "tcp", test_case(Transport::FramedTcp, 200000))] -//#[cfg_attr(feature = "websocket", test_case(Transport::Ws, 200000))] +#[cfg_attr(feature = "websocket", test_case(Transport::Ws, 200000))] fn burst(transport: Transport, messages_count: usize) { //util::init_logger(LogThread::Enabled); // Enable it for better debugging @@ -279,7 +279,7 @@ fn burst(transport: Transport, messages_count: usize) { #[cfg_attr(feature = "tcp", test_case(Transport::Tcp, BIG_MESSAGE_SIZE))] #[cfg_attr(feature = "tcp", test_case(Transport::FramedTcp, BIG_MESSAGE_SIZE))] #[cfg_attr(feature = "udp", test_case(Transport::Udp, udp::MAX_COMPATIBLE_PAYLOAD_LEN))] -//#[cfg_attr(feature = "websocket", test_case(Transport::Ws, BIG_MESSAGE_SIZE))] +#[cfg_attr(feature = "websocket", test_case(Transport::Ws, BIG_MESSAGE_SIZE))] fn message_size(transport: Transport, message_size: usize) { //util::init_logger(LogThread::Enabled); // Enable it for better debugging From 0cd4e69e553cb3f4c474a22cf78eae4a0b8b0def Mon Sep 17 00:00:00 2001 From: lemunozm Date: Tue, 4 May 2021 23:22:02 +0200 Subject: [PATCH 09/25] Removed write readiness for local resources --- src/network/driver.rs | 6 +++--- src/network/poll.rs | 8 ++++++-- src/network/registry.rs | 4 ++-- tests/integration.rs | 2 +- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/network/driver.rs b/src/network/driver.rs index d82e9fc1..b9518383 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -139,7 +139,7 @@ impl ActionController for Driver { R::connect(addr).map(|info| { let endpoint = Endpoint::new( self.remote_registry - .register(info.remote, RemoteProperties::new(info.peer_addr, None)), + .register(info.remote, RemoteProperties::new(info.peer_addr, None), true), info.peer_addr, ); (endpoint, info.local_addr) @@ -148,7 +148,7 @@ impl ActionController for Driver { fn listen(&self, addr: SocketAddr) -> io::Result<(ResourceId, SocketAddr)> { L::listen(addr).map(|info| { - let id = self.local_registry.register(info.local, LocalProperties); + let id = self.local_registry.register(info.local, LocalProperties, false); (id, info.local_addr) }) } @@ -269,7 +269,7 @@ impl> Driver { AcceptedType::Remote(addr, remote) => { let remote_id = self .remote_registry - .register(remote, RemoteProperties::new(addr, Some(id))); + .register(remote, RemoteProperties::new(addr, Some(id)), true); // We Manually generate a Poll Write event, because an accepted connection // is ready to write. diff --git a/src/network/poll.rs b/src/network/poll.rs index c26ddbcf..8a13cee1 100644 --- a/src/network/poll.rs +++ b/src/network/poll.rs @@ -111,9 +111,13 @@ impl PollRegistry { } } - pub fn add(&self, source: &mut dyn Source) -> ResourceId { + pub fn add(&self, source: &mut dyn Source, write_readiness: bool) -> ResourceId { let id = self.id_generator.generate(); - self.registry.register(source, id.into(), Interest::READABLE | Interest::WRITABLE).unwrap(); + let interest = match write_readiness { + true => Interest::READABLE | Interest::WRITABLE, + false => Interest::READABLE, + }; + self.registry.register(source, id.into(), interest).unwrap(); id } diff --git a/src/network/registry.rs b/src/network/registry.rs index e7ca9349..1e3e03ba 100644 --- a/src/network/registry.rs +++ b/src/network/registry.rs @@ -40,11 +40,11 @@ impl ResourceRegistry { } /// Add a resource into the registry. - pub fn register(&self, mut resource: S, properties: P) -> ResourceId { + pub fn register(&self, mut resource: S, properties: P, write_readiness: bool) -> ResourceId { // The registry must be locked for the entire implementation to avoid the poll // to generate events over not yet registered resources. let mut registry = self.resources.write().expect(OTHER_THREAD_ERR); - let id = self.poll_registry.add(resource.source()); + let id = self.poll_registry.add(resource.source(), write_readiness); let register = Register::new(resource, properties, self.poll_registry.clone()); registry.insert(id, Arc::new(register)); id diff --git a/tests/integration.rs b/tests/integration.rs index 737011aa..cae6c6ee 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -226,7 +226,7 @@ fn start_burst_sender( count += 1; if !transport.is_connection_oriented() { // We need a rate to not lose packet. - node.signals().send_with_timer((), Duration::from_micros(20)); + node.signals().send_with_timer((), Duration::from_micros(50)); } else { node.signals().send(()); From 646f5a1f53774e8fd899d9b1c236773d5e50fc6d Mon Sep 17 00:00:00 2001 From: lemunozm Date: Wed, 5 May 2021 18:26:01 +0200 Subject: [PATCH 10/25] Examples and tests adapted --- docs/performance_benchmarks.md | 6 +- examples/distributed/discovery_server.rs | 50 +++++------ examples/distributed/main.rs | 8 +- examples/distributed/participant.rs | 104 +++++++++++------------ examples/file-transfer/receiver.rs | 9 +- examples/file-transfer/sender.rs | 25 +++--- examples/multicast/main.rs | 22 +++-- examples/ping-pong/client.rs | 47 +++++----- examples/ping-pong/server.rs | 15 ++-- examples/throughput/main.rs | 9 +- src/adapters/ws.rs | 10 +-- src/network.rs | 11 ++- src/network/driver.rs | 23 +++-- src/network/poll.rs | 2 +- tests/integration.rs | 8 +- 15 files changed, 169 insertions(+), 180 deletions(-) diff --git a/docs/performance_benchmarks.md b/docs/performance_benchmarks.md index e827286f..5da83807 100644 --- a/docs/performance_benchmarks.md +++ b/docs/performance_benchmarks.md @@ -40,7 +40,7 @@ The following results are measured for the transmision of 1GB of data by localho | Transport | native | message-io | efficiency | |:----------:|:--------:|:----------:|:----------:| | UDP | 7.1 GB/s | 5.9 GB/s | ~83% | -| TCP | 6.4 GB/s | 5.4 GB/s | ~84% | +| TCP | 6.4 GB/s | 5.2 GB/s | ~81% | | Framed TCP | 5.5 GB/s | 5.0 GB/s | ~91% | | Web Socket | 590 MB/s | 560 MB/s | ~95% | @@ -71,9 +71,9 @@ The following results are measured by transferring 1-byte by localhost: | UDP | 1.2 us | 2.1 us | + ~0.9 us | | TCP | 2.6 us | 3.5 us | + ~0.9 us | | Framed TCP | 5.2 us | 6.6 us | + ~1.4 us | -| Web Socket | 9.1 us | 11.2 us | + ~2.1 us | +| Web Socket | 9.1 us | 10.1 us | + ~1.0 us | -Depending on the transport used, `message-io` adds around `1-2us` of overhead per chunk of data transsmision. +Depending on the transport used, `message-io` adds around `1us` of overhead per chunk of data transsmision. Because it is zero-copy at reading/writing messages, this overhead is constant and independently of the size of that chunk of data. The library only copies the pointer to the data. diff --git a/examples/distributed/discovery_server.rs b/examples/distributed/discovery_server.rs index 243383e9..b9042baa 100644 --- a/examples/distributed/discovery_server.rs +++ b/examples/distributed/discovery_server.rs @@ -5,6 +5,7 @@ use message_io::node::{self, NodeHandler, NodeListener}; use std::net::{SocketAddr}; use std::collections::{HashMap}; +use std::io::{self}; struct ParticipantInfo { addr: SocketAddr, @@ -13,34 +14,31 @@ struct ParticipantInfo { pub struct DiscoveryServer { handler: NodeHandler<()>, - listener: Option>, + node_listener: Option>, participants: HashMap, } impl DiscoveryServer { - pub fn new() -> Option { - let (handler, listener) = node::split::<()>(); + pub fn new() -> io::Result { + let (handler, node_listener) = node::split::<()>(); let listen_addr = "127.0.0.1:5000"; - match handler.network().listen(Transport::FramedTcp, listen_addr) { - Ok(_) => { - println!("Discovery server running at {}", listen_addr); - Some(DiscoveryServer { - handler, - listener: Some(listener), - participants: HashMap::new(), - }) - } - Err(_) => { - println!("Can not listen on {}", listen_addr); - None - } - } + handler.network().listen(Transport::FramedTcp, listen_addr)?; + + println!("Discovery server running at {}", listen_addr); + + Ok(DiscoveryServer { + handler, + node_listener: Some(node_listener), + participants: HashMap::new(), + }) } pub fn run(mut self) { - let listener = self.listener.take().unwrap(); - listener.for_each(move |event| match event.network() { + let node_listener = self.node_listener.take().unwrap(); + node_listener.for_each(move |event| match event.network() { + NetEvent::Connected(_, _) => unreachable!(), // There is no connect() calls. + NetEvent::Accepted(_, _) => (), // All endpoint accepted NetEvent::Message(endpoint, input_data) => { let message: Message = bincode::deserialize(&input_data).unwrap(); match message { @@ -53,19 +51,15 @@ impl DiscoveryServer { _ => unreachable!(), } } - NetEvent::Connected(_, _) => (), NetEvent::Disconnected(endpoint) => { // Participant disconection without explict unregistration. // We must remove from the registry too. - let participant_name = self.participants.iter().find_map(|(name, info)| { - match info.endpoint == endpoint { - true => Some(name.clone()), - false => None, - } - }); + let participant = + self.participants.iter().find(|(_, info)| info.endpoint == endpoint); - if let Some(name) = participant_name { - self.unregister(&name) + if let Some(participant) = participant { + let name = participant.0.to_string(); + self.unregister(&name); } } }); diff --git a/examples/distributed/main.rs b/examples/distributed/main.rs index 2e54b725..74e225a6 100644 --- a/examples/distributed/main.rs +++ b/examples/distributed/main.rs @@ -7,13 +7,13 @@ pub fn main() { match args.get(1).unwrap_or(&String::new()).as_ref() { "discovery-server" => match discovery_server::DiscoveryServer::new() { - Some(discovery_server) => discovery_server.run(), - None => println!("Can not run the discovery server"), + Ok(discovery_server) => discovery_server.run(), + Err(err) => println!("Can not run the discovery server: {}", err), }, "participant" => match args.get(2) { Some(name) => match participant::Participant::new(name) { - Some(participant) => participant.run(), - None => println!("Can not run the participant"), + Ok(participant) => participant.run(), + Err(err) => println!("Can not run the participant: {}", err), }, None => println!("The participant needs a 'name'"), }, diff --git a/examples/distributed/participant.rs b/examples/distributed/participant.rs index 76b445b0..02645e57 100644 --- a/examples/distributed/participant.rs +++ b/examples/distributed/participant.rs @@ -5,67 +5,78 @@ use message_io::node::{self, NodeHandler, NodeListener}; use std::net::{SocketAddr}; use std::collections::{HashMap}; +use std::io::{self}; pub struct Participant { handler: NodeHandler<()>, - listener: Option>, + node_listener: Option>, name: String, discovery_endpoint: Endpoint, public_addr: SocketAddr, known_participants: HashMap, // Used only for free resources later + grettings: HashMap, } impl Participant { - pub fn new(name: &str) -> Option { - let (handler, listener) = node::split(); + pub fn new(name: &str) -> io::Result { + let (handler, node_listener) = node::split(); - // A listener for any other participant that want to establish connection. - // 'addr' contains the port that the OS gives for us when we put a 0. + // A node_listener for any other participant that want to establish connection. + // Returned 'listen_addr' contains the port that the OS gives for us when we put a 0. let listen_addr = "127.0.0.1:0"; - let listen_addr = match handler.network().listen(Transport::Udp, listen_addr) { - Ok((_, addr)) => addr, - Err(_) => { - println!("Can not listen on {}", listen_addr); - return None - } - }; + let (_, listen_addr) = handler.network().listen(Transport::FramedTcp, listen_addr)?; let discovery_addr = "127.0.0.1:5000"; // Connection to the discovery server. - match handler.network().connect(Transport::FramedTcp, discovery_addr) { - Ok((endpoint, _)) => Some(Participant { - handler, - listener: Some(listener), - name: name.to_string(), - discovery_endpoint: endpoint, - public_addr: listen_addr, - known_participants: HashMap::new(), - }), - Err(_) => { - println!("Can not connect to the discovery server at {}", discovery_addr); - return None - } - } + let (endpoint, _) = handler.network().connect(Transport::FramedTcp, discovery_addr)?; + + Ok(Participant { + handler, + node_listener: Some(node_listener), + name: name.to_string(), + discovery_endpoint: endpoint, + public_addr: listen_addr, + known_participants: HashMap::new(), + grettings: HashMap::new(), + }) } pub fn run(mut self) { // Register this participant into the discovery server - let message = Message::RegisterParticipant(self.name.clone(), self.public_addr); - let output_data = bincode::serialize(&message).unwrap(); - self.handler.network().send(self.discovery_endpoint, &output_data); - - let listener = self.listener.take().unwrap(); - listener.for_each(move |event| match event.network() { + let node_listener = self.node_listener.take().unwrap(); + node_listener.for_each(move |event| match event.network() { + NetEvent::Connected(endpoint, established) => { + if endpoint == self.discovery_endpoint { + if established { + let message = + Message::RegisterParticipant(self.name.clone(), self.public_addr); + let output_data = bincode::serialize(&message).unwrap(); + self.handler.network().send(self.discovery_endpoint, &output_data); + } + else { + println!("Can not connect to the discovery server"); + } + } + else { + // Participant endpoint + let (name, message) = self.grettings.remove(&endpoint).unwrap(); + if established { + let gretings = format!("Hi '{}', {}", name, message); + let message = Message::Gretings(self.name.clone(), gretings); + let output_data = bincode::serialize(&message).unwrap(); + self.handler.network().send(endpoint, &output_data); + self.known_participants.insert(name.clone(), endpoint); + } + } + } + NetEvent::Accepted(_, _) => (), NetEvent::Message(_, input_data) => { let message: Message = bincode::deserialize(&input_data).unwrap(); match message { Message::ParticipantList(participants) => { println!("Participant list received ({} participants)", participants.len()); for (name, addr) in participants { - self.discovered_participant( - &name, - addr, - "I see you in the participant list", - ); + let text = "I see you in the participant list"; + self.discovered_participant(&name, addr, text); } } Message::ParticipantNotificationAdded(name, addr) => { @@ -74,12 +85,6 @@ impl Participant { } Message::ParticipantNotificationRemoved(name) => { println!("Removed participant '{}' from the network", name); - - // Free network resource. - // It is only necessary because the connections among participants - // are done by UDP, - // UDP is not connection-oriented protocol, and the - // Connected/Disconnected events are not generated by UDP. if let Some(endpoint) = self.known_participants.remove(&name) { self.handler.network().remove(endpoint.resource_id()); } @@ -90,7 +95,6 @@ impl Participant { _ => unreachable!(), } } - NetEvent::Connected(_, _) => (), NetEvent::Disconnected(endpoint) => { if endpoint == self.discovery_endpoint { println!("Discovery server disconnected, closing"); @@ -100,13 +104,9 @@ impl Participant { }); } - fn discovered_participant(&mut self, name: &str, addr: SocketAddr, message: &str) { - if let Ok((endpoint, _)) = self.handler.network().connect(Transport::Udp, addr) { - let gretings = format!("Hi '{}', {}", name, message); - let message = Message::Gretings(self.name.clone(), gretings); - let output_data = bincode::serialize(&message).unwrap(); - self.handler.network().send(endpoint, &output_data); - self.known_participants.insert(name.to_string(), endpoint); - } + fn discovered_participant(&mut self, name: &str, addr: SocketAddr, text: &str) { + let (endpoint, _) = self.handler.network().connect(Transport::FramedTcp, addr).unwrap(); + // Save the necessary info to send the message when the connection is established. + self.grettings.insert(endpoint, (name.into(), text.into())); } } diff --git a/examples/file-transfer/receiver.rs b/examples/file-transfer/receiver.rs index 2c1506fb..1440e6a7 100644 --- a/examples/file-transfer/receiver.rs +++ b/examples/file-transfer/receiver.rs @@ -18,14 +18,14 @@ pub fn run() { let (handler, listener) = node::split::<()>(); let listen_addr = "127.0.0.1:3005"; - match handler.network().listen(Transport::FramedTcp, listen_addr) { - Ok(_) => println!("Receiver running by TCP at {}", listen_addr), - Err(_) => return println!("Can not listening by TCP at {}", listen_addr), - } + handler.network().listen(Transport::FramedTcp, listen_addr).unwrap(); + println!("Receiver running by TCP at {}", listen_addr); let mut transfers: HashMap = HashMap::new(); listener.for_each(move |event| match event.network() { + NetEvent::Connected(_, _) => unreachable!(), + NetEvent::Accepted(_, _) => (), NetEvent::Message(endpoint, input_data) => { let message: SenderMsg = bincode::deserialize(&input_data).unwrap(); match message { @@ -64,7 +64,6 @@ pub fn run() { } } } - NetEvent::Connected(_, _) => {} NetEvent::Disconnected(endpoint) => { // Unexpected sender disconnection. Cleaninig. if transfers.contains_key(&endpoint) { diff --git a/examples/file-transfer/sender.rs b/examples/file-transfer/sender.rs index ee59fce5..a644f7a3 100644 --- a/examples/file-transfer/sender.rs +++ b/examples/file-transfer/sender.rs @@ -18,25 +18,27 @@ pub fn run(file_path: String) { let (handler, listener) = node::split(); let server_addr = "127.0.0.1:3005"; - let (server_id, _) = match handler.network().connect(Transport::FramedTcp, server_addr) { - Ok(server_id) => { - println!("Sender connected by TCP at {}", server_addr); - server_id - } - Err(_) => return println!("Can not connect to the receiver by TCP to {}", server_addr), - }; + let (server_id, _) = handler.network().connect(Transport::FramedTcp, server_addr).unwrap(); let file_size = fs::metadata(&file_path).unwrap().len() as usize; let mut file = File::open(&file_path).unwrap(); let file_name: String = file_path.rsplit('/').into_iter().next().unwrap_or(&file_path).into(); - let request = SenderMsg::FileRequest(file_name.clone(), file_size); - let output_data = bincode::serialize(&request).unwrap(); - handler.network().send(server_id, &output_data); - let mut file_bytes_sent = 0; listener.for_each(move |event| match event { NodeEvent::Network(net_event) => match net_event { + NetEvent::Connected(_, established) => { + if established { + println!("Sender connected by TCP at {}", server_addr); + let request = SenderMsg::FileRequest(file_name.clone(), file_size); + let output_data = bincode::serialize(&request).unwrap(); + handler.network().send(server_id, &output_data); + } + else { + println!("Can not connect to the receiver by TCP to {}", server_addr) + } + }, + NetEvent::Accepted(_, _) => unreachable!(), NetEvent::Message(_, input_data) => { let message: ReceiverMsg = bincode::deserialize(&input_data).unwrap(); match message { @@ -49,7 +51,6 @@ pub fn run(file_path: String) { }, } } - NetEvent::Connected(_, _) => unreachable!(), NetEvent::Disconnected(_) => { handler.stop(); println!("\nReceiver disconnected"); diff --git a/examples/multicast/main.rs b/examples/multicast/main.rs index ab4cf6c7..df162808 100644 --- a/examples/multicast/main.rs +++ b/examples/multicast/main.rs @@ -10,24 +10,22 @@ fn main() { let (handler, listener) = node::split::<()>(); - let addr = "239.255.0.1:3010"; - match handler.network().connect(Transport::Udp, addr) { - Ok((endpoint, _)) => { + let multicast_addr = "239.255.0.1:3010"; + let (endpoint, _) = handler.network().connect(Transport::Udp, multicast_addr).unwrap(); + + listener.for_each(move |event| match event.network() { + NetEvent::Connected(_, _always_true_for_udp) => { println!("Notifying on the network"); handler.network().send(endpoint, my_name.as_bytes()); - } - Err(_) => return eprintln!("Could not connect to {}", addr), - } - // Since the addrers belongs to the multicast range (from 224.0.0.0 to 239.255.255.255) - // the internal resource will be configured to receive multicast messages. - handler.network().listen(Transport::Udp, addr).unwrap(); - - listener.for_each(move |event| match event.network() { + // Since the address belongs to the multicast range (from 224.0.0.0 to 239.255.255.255) + // the internal resource will be configured to receive multicast messages. + handler.network().listen(Transport::Udp, multicast_addr).unwrap(); + } + NetEvent::Accepted(_, _) => unreachable!(), // UDP is not connection-oriented NetEvent::Message(_, data) => { println!("{} greets to the network!", String::from_utf8_lossy(&data)); } - NetEvent::Connected(_, _) => (), NetEvent::Disconnected(_) => (), }); } diff --git a/examples/ping-pong/client.rs b/examples/ping-pong/client.rs index b2f9ae8f..0af9abf0 100644 --- a/examples/ping-pong/client.rs +++ b/examples/ping-pong/client.rs @@ -6,37 +6,29 @@ use message_io::node::{self, NodeEvent}; use std::time::{Duration}; enum Signal { - // This is a self event called every second. - Greet, - // Other signals here, + Greet, // This is a self event called every second. + // Other signals here, } pub fn run(transport: Transport, remote_addr: RemoteAddr) { let (handler, listener) = node::split(); - let server_id = match handler.network().connect(transport, remote_addr.clone()) { - Ok((server_id, local_addr)) => { - println!("Connected to server by {} at {}", transport, server_id.addr()); - println!("Client identified by local port: {}", local_addr.port()); - server_id - } - Err(_) => { - return println!("Can not connect to the server by {} to {}", transport, remote_addr) - } - }; - - handler.signals().send(Signal::Greet); + let (server_id, local_addr) = + handler.network().connect(transport, remote_addr.clone()).unwrap(); listener.for_each(move |event| match event { - NodeEvent::Signal(signal) => match signal { - Signal::Greet => { - let message = FromClientMessage::Ping; - let output_data = bincode::serialize(&message).unwrap(); - handler.network().send(server_id, &output_data); - handler.signals().send_with_timer(Signal::Greet, Duration::from_secs(1)); - } - }, NodeEvent::Network(net_event) => match net_event { + NetEvent::Connected(_, established) => { + if established { + println!("Connected to server at {} by {}", server_id.addr(), transport); + println!("Client identified by local port: {}", local_addr.port()); + handler.signals().send(Signal::Greet); + } + else { + println!("Can not connect to server at {} by {}", remote_addr, transport) + } + } + NetEvent::Accepted(_, _) => unreachable!(), // Only generated when a listener accepts NetEvent::Message(_, input_data) => { let message: FromServerMessage = bincode::deserialize(&input_data).unwrap(); match message { @@ -46,11 +38,18 @@ pub fn run(transport: Transport, remote_addr: RemoteAddr) { FromServerMessage::UnknownPong => println!("Pong from server"), } } - NetEvent::Connected(_, _) => unreachable!(), // Only generated when a listener accepts NetEvent::Disconnected(_) => { println!("Server is disconnected"); handler.stop(); } }, + NodeEvent::Signal(signal) => match signal { + Signal::Greet => { + let message = FromClientMessage::Ping; + let output_data = bincode::serialize(&message).unwrap(); + handler.network().send(server_id, &output_data); + handler.signals().send_with_timer(Signal::Greet, Duration::from_secs(1)); + } + }, }); } diff --git a/examples/ping-pong/server.rs b/examples/ping-pong/server.rs index 3537e037..e5f9e015 100644 --- a/examples/ping-pong/server.rs +++ b/examples/ping-pong/server.rs @@ -16,13 +16,17 @@ pub fn run(transport: Transport, addr: SocketAddr) { let mut clients: HashMap = HashMap::new(); match handler.network().listen(transport, addr) { - Ok((_resource_id, real_addr)) => { - println!("Server running at {} by {}", real_addr, transport) - } + Ok((_id, real_addr)) => println!("Server running at {} by {}", real_addr, transport), Err(_) => return println!("Can not listening at {} by {}", addr, transport), } listener.for_each(move |event| match event.network() { + NetEvent::Connected(_, _) => (), // Only generated at connect() calls. + NetEvent::Accepted(endpoint, _listener_id) => { + // Only connection oriented protocols will generate this event + clients.insert(endpoint, ClientInfo { count: 0 }); + println!("Client ({}) connected (total clients: {})", endpoint.addr(), clients.len()); + } NetEvent::Message(endpoint, input_data) => { let message: FromClientMessage = bincode::deserialize(&input_data).unwrap(); match message { @@ -45,11 +49,6 @@ pub fn run(transport: Transport, addr: SocketAddr) { } } } - NetEvent::Connected(endpoint, _) => { - // Only connection oriented protocols will generate this event - clients.insert(endpoint, ClientInfo { count: 0 }); - println!("Client ({}) connected (total clients: {})", endpoint.addr(), clients.len()); - } NetEvent::Disconnected(endpoint) => { // Only connection oriented protocols will generate this event clients.remove(&endpoint).unwrap(); diff --git a/examples/throughput/main.rs b/examples/throughput/main.rs index 971fd82e..5f4048fc 100644 --- a/examples/throughput/main.rs +++ b/examples/throughput/main.rs @@ -56,9 +56,8 @@ fn throughput_message_io(transport: Transport, packet_size: usize) { let handler = handler.clone(); listener.for_each_async(move |event| match event.network() { - NetEvent::Connected(_, _) => { - t_ready.send(()).unwrap(); - } + NetEvent::Connected(_, _) => (), + NetEvent::Accepted(_, _) => t_ready.send(()).unwrap(), NetEvent::Message(_, data) => { received_bytes += data.len(); if received_bytes >= EXPECTED_BYTES { @@ -74,8 +73,8 @@ fn throughput_message_io(transport: Transport, packet_size: usize) { r_ready.recv().unwrap(); } - // To improve accuracy, - // ensure that internal thread is initialized for not oriented connection protocols + // Ensure that the connection is performed, + // the internal thread is initialized for not oriented connection protocols // and we are waiting in the internal poll for data. std::thread::sleep(Duration::from_millis(100)); diff --git a/src/adapters/ws.rs b/src/adapters/ws.rs index fbc16ec8..ab8a09e7 100644 --- a/src/adapters/ws.rs +++ b/src/adapters/ws.rs @@ -157,9 +157,7 @@ impl Remote for RemoteResource { *pending = Some(PendingHandshake::Client(mid_handshake)); PendingStatus::Incomplete } - Err(HandshakeError::Failure(Error::Io(_))) => { - PendingStatus::Disconnected - }, + Err(HandshakeError::Failure(Error::Io(_))) => PendingStatus::Disconnected, Err(HandshakeError::Failure(err)) => { log::error!("WS connect handshake error: {}", err); PendingStatus::Disconnected // should not happen @@ -174,9 +172,7 @@ impl Remote for RemoteResource { *pending = Some(PendingHandshake::Server(mid_handshake)); PendingStatus::Incomplete } - Err(HandshakeError::Failure(Error::Io(_))) => { - PendingStatus::Disconnected - }, + Err(HandshakeError::Failure(Error::Io(_))) => PendingStatus::Disconnected, Err(HandshakeError::Failure(err)) => { log::error!("WS accept handshake error: {}", err); PendingStatus::Disconnected // should not happen @@ -206,7 +202,7 @@ impl Remote for RemoteResource { } } } - }, + } RemoteState::Handshake(_) => unreachable!(), } } diff --git a/src/network.rs b/src/network.rs index e7addc34..f33cdfbe 100644 --- a/src/network.rs +++ b/src/network.rs @@ -55,9 +55,14 @@ impl NetworkController { /// Creates a connection to the specific address. /// The endpoint, an identifier of the new connection, will be returned. - /// If the connection can not be performed (e.g. the address is not reached) - /// the corresponding IO error is returned. - /// This function blocks until the resource has been connected and is ready to use. + /// This function will generate a [`NetEvent::Connected`] event with the result of the connection. + /// This call will **NOT** block to perform the connection. + /// + /// Note that this function can return an error in the case the internal socket + /// could not be binded or open in the OS, but never will return an error an regarding + /// the connection itself. + /// If you want to check if the connection has been established or not you have to read the + /// boolean indicator in the [`NetEvent::Connected`] event. pub fn connect( &self, transport: Transport, diff --git a/src/network/driver.rs b/src/network/driver.rs index b9518383..b39fb2fd 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -137,12 +137,12 @@ impl Clone for Driver { impl ActionController for Driver { fn connect(&self, addr: RemoteAddr) -> io::Result<(Endpoint, SocketAddr)> { R::connect(addr).map(|info| { - let endpoint = Endpoint::new( - self.remote_registry - .register(info.remote, RemoteProperties::new(info.peer_addr, None), true), - info.peer_addr, + let id = self.remote_registry.register( + info.remote, + RemoteProperties::new(info.peer_addr, None), + true, ); - (endpoint, info.local_addr) + (Endpoint::new(id, info.peer_addr), info.local_addr) }) } @@ -156,7 +156,10 @@ impl ActionController for Driver { fn send(&self, endpoint: Endpoint, data: &[u8]) -> SendStatus { match endpoint.resource_id().resource_type() { ResourceType::Remote => match self.remote_registry.get(endpoint.resource_id()) { - Some(remote) => remote.resource.send(data), + Some(remote) => match remote.properties.is_ready() { + true => remote.resource.send(data), + false => SendStatus::ResourceNotFound, + }, None => SendStatus::ResourceNotFound, }, ResourceType::Local => match self.local_registry.get(endpoint.resource_id()) { @@ -267,9 +270,11 @@ impl> Driver { log::trace!("Accepted type: {}", accepted); match accepted { AcceptedType::Remote(addr, remote) => { - let remote_id = self - .remote_registry - .register(remote, RemoteProperties::new(addr, Some(id)), true); + let remote_id = self.remote_registry.register( + remote, + RemoteProperties::new(addr, Some(id)), + true, + ); // We Manually generate a Poll Write event, because an accepted connection // is ready to write. diff --git a/src/network/poll.rs b/src/network/poll.rs index 8a13cee1..79e2d744 100644 --- a/src/network/poll.rs +++ b/src/network/poll.rs @@ -113,7 +113,7 @@ impl PollRegistry { pub fn add(&self, source: &mut dyn Source, write_readiness: bool) -> ResourceId { let id = self.id_generator.generate(); - let interest = match write_readiness { + let interest = match write_readiness { true => Interest::READABLE | Interest::WRITABLE, false => Interest::READABLE, }; diff --git a/tests/integration.rs b/tests/integration.rs index cae6c6ee..bc565397 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -292,17 +292,11 @@ fn message_size(transport: Transport, message_size: usize) { node.signals().send_with_timer((), *TIMEOUT); let (_, receiver_addr) = node.network().listen(transport, LOCAL_ADDR).unwrap(); - let (receiver, _) = node.network().connect(transport, receiver_addr).unwrap(); - if !transport.is_connection_oriented() { - let status = node.network().send(receiver, &sent_message); - assert_eq!(status, SendStatus::Sent); - } - let mut _async_sender: Option> = None; - let mut received_message = Vec::new(); + listener.for_each(move |event| match event { NodeEvent::Signal(_) => panic!("{}", TIMEOUT_EVENT_RECV_ERR), NodeEvent::Network(net_event) => match net_event { From 07e521f314e74c68acc1eda598e74637f4525172 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Wed, 5 May 2021 20:38:37 +0200 Subject: [PATCH 11/25] Added is_ready. Added connect_sync. Prepare version 0.14 --- CHANGELOG.md | 8 +++ Cargo.lock | 2 +- Cargo.toml | 2 +- README.md | 22 +++--- examples/file-transfer/sender.rs | 2 +- src/network.rs | 115 +++++++++++++++++++++++++------ src/network/driver.rs | 8 +++ src/network/loader.rs | 4 ++ 8 files changed, 129 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9b03c66..b7c2eff8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## Release 0.14.0 +- Asynchronous connections: `NetworkController::connect()` behaviour modified. +Now it performs a non-blocking connection. +- Reduced slightly the websocket latency. +- Adapter API modified to handle easily handshakes. +- Fixed websocket issue that could offer an accepted websocket that was not valid (the websocket handshake could have failed). +- Added `NetworkController::is_ready()` + ## Release 0.13.3 - Fixed a bad internal assert. diff --git a/Cargo.lock b/Cargo.lock index cf3ca9a4..aaaf7488 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -482,7 +482,7 @@ dependencies = [ [[package]] name = "message-io" -version = "0.13.3" +version = "0.14.0" dependencies = [ "bincode", "chrono", diff --git a/Cargo.toml b/Cargo.toml index 30c380cc..a8fe5d49 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "message-io" -version = "0.13.3" +version = "0.14.0" authors = ["lemunozm "] edition = "2018" readme = "README.md" diff --git a/README.md b/README.md index 9b6b0875..fc12e389 100644 --- a/README.md +++ b/README.md @@ -80,21 +80,20 @@ You could change the transport of your application in literally one line. Add to your `Cargo.toml` (all transports included by default): ```toml [dependencies] -message-io = "0.13" +message-io = "0.14" ``` If you **only** want to use a subset of the available transport battery, you can select them by their associated features `tcp`, `udp`, and `websocket`. For example, in order to include only *TCP* and *UDP*, add to your `Cargo.toml`: ```toml [dependencies] -message-io = { version = "0.13", default-features = false, features = ["tcp", "udp"] } +message-io = { version = "0.14", default-features = false, features = ["tcp", "udp"] } ``` -**Warning**: Version **0.12** comes with important API changes ([changelog](CHANGELOG.md)) -in order to reach [zero-copy write/read](https://github.com/lemunozm/message-io/issues/61) goal. -If you find problems porting your application to this version, -check the examples folder, API docs, and don't hesitate to open an issue. - +***Warning**: Version **0.14** modifies the [`connect()`]() behaviour to perform +[**non**-blocking connections](https://github.com/lemunozm/message-io/issues/61). +It is recommended to use this non-blocking mode in order to get the +best scalability and performance in your application, but if you need to perform a similar blocking connection as before, you can use [`connect_sync()`]().* ### All in one: TCP, UDP and WebSocket echo server The following example is the simplest server that reads messages from the clients and responds @@ -118,7 +117,8 @@ fn main() { // Read incoming network events. listener.for_each(move |event| match event.network() { - NetEvent::Connected(_endpoint, _) => println!("Client connected"), // Tcp or Ws + NetEvent::Connected(_, _) => unreachable!(), // Used for explicit connections. + NetEvent::Accepted(_endpoint, _listener) => println!("Client connected"), // Tcp or Ws NetEvent::Message(endpoint, data) => { println!("Received: {}", String::from_utf8_lossy(data)); handler.network().send(endpoint, data); @@ -149,8 +149,6 @@ fn main() { // You can change the transport to Udp or Ws (WebSocket). let (server, _) = handler.network().connect(Transport::FramedTcp, "127.0.0.1:3042").unwrap(); - handler.signals().send(Signal::Greet); // Start sending - listener.for_each(move |event| match event { NodeEvent::Signal(signal) => match signal { Signal::Greet => { // computed every second @@ -159,10 +157,12 @@ fn main() { } } NodeEvent::Network(net_event) => match net_event { + NetEvent::Connected(_endpoint, _ok) => handler.signals().send(Signal::Greet), + NetEvent::Accepted(_, _) => unreachable!(), // Only generated by listening NetEvent::Message(_endpoint, data) => { println!("Received: {}", String::from_utf8_lossy(data)); }, - _ => unreachable!(), // Connected and Disconnected are only generated by listening + NetEvent::Disconnected(_endpoint) => (), } }); } diff --git a/examples/file-transfer/sender.rs b/examples/file-transfer/sender.rs index a644f7a3..86715c2e 100644 --- a/examples/file-transfer/sender.rs +++ b/examples/file-transfer/sender.rs @@ -37,7 +37,7 @@ pub fn run(file_path: String) { else { println!("Can not connect to the receiver by TCP to {}", server_addr) } - }, + } NetEvent::Accepted(_, _) => unreachable!(), NetEvent::Message(_, input_data) => { let message: ReceiverMsg = bincode::deserialize(&input_data).unwrap(); diff --git a/src/network.rs b/src/network.rs index f33cdfbe..d1849700 100644 --- a/src/network.rs +++ b/src/network.rs @@ -53,7 +53,7 @@ impl NetworkController { Self { controllers } } - /// Creates a connection to the specific address. + /// Creates a connection to the specified address. /// The endpoint, an identifier of the new connection, will be returned. /// This function will generate a [`NetEvent::Connected`] event with the result of the connection. /// This call will **NOT** block to perform the connection. @@ -69,13 +69,47 @@ impl NetworkController { addr: impl ToRemoteAddr, ) -> io::Result<(Endpoint, SocketAddr)> { let addr = addr.to_remote_addr().unwrap(); - log::trace!("Connect to {} by adapter: {}", addr, transport.id()); self.controllers[transport.id() as usize].connect(addr).map(|(endpoint, addr)| { - log::trace!("Connected to {}", endpoint); + log::trace!("Connect to {}", endpoint); (endpoint, addr) }) } + /// Creates a connection to the specified address. + /// This function is similar to [`NetworkController::connect()`] but will block + /// to perform the connection, waiting until for the connection is ready. + /// If the connection can not be established, a `ConnectionRefused` error will be returned. + /// + /// Note that the `Connect` event will be also generated. + /// + /// Since this function blocks the current thread, it must not be used inside + /// the network callback. + /// In order to get the best scalability and performance, use the non-blocking + /// [`NetworkController::connect()`] version. + pub fn connect_sync( + &self, + transport: Transport, + addr: impl ToRemoteAddr, + ) -> io::Result<(Endpoint, SocketAddr)> { + let (endpoint, addr) = self.connect(transport, addr)?; + loop { + std::thread::sleep(Duration::from_millis(1)); + match self.is_ready(endpoint.resource_id()) { + Some(status) => { + if status { + return Ok((endpoint, addr)) + } + } + None => { + return Err(io::Error::new( + io::ErrorKind::ConnectionRefused, + "Connection refused", + )) + } + } + } + } + /// Listen messages from specified transport. /// The giver address will be used as interface and listening port. /// If the port can be opened, a [ResourceId] identifying the listener is returned @@ -88,13 +122,27 @@ impl NetworkController { addr: impl ToSocketAddrs, ) -> io::Result<(ResourceId, SocketAddr)> { let addr = addr.to_socket_addrs().unwrap().next().unwrap(); - log::trace!("Listen by {} by adapter: {}", addr, transport.id()); self.controllers[transport.id() as usize].listen(addr).map(|(resource_id, addr)| { - log::trace!("Listening by {}", resource_id); + log::trace!("Listening at {} by {}", addr, resource_id); (resource_id, addr) }) } + /// Send the data message thought the connection represented by the given endpoint. + /// This function returns a [`SendStatus`] indicating the status of this send. + /// There is no guarantee that send over a correct connection generates a [`SendStatus::Sent`] + /// because any time a connection can be disconnected (even while you are sending). + /// Except cases where you need to be sure that the message has been sent, + /// you will want to process a [`NetEvent::Disconnected`] to determine if the connection + + /// is *alive* instead of check if `send()` returned [`SendStatus::ResourceNotFound`]. + pub fn send(&self, endpoint: Endpoint, data: &[u8]) -> SendStatus { + log::trace!("Send {} bytes to {}", data.len(), endpoint); + let status = + self.controllers[endpoint.resource_id().adapter_id() as usize].send(endpoint, data); + log::trace!("Send status: {:?}", status); + status + } + /// Remove a network resource. /// Returns `false` if the resource id doesn't exists. /// This is used to remove resources as connection or listeners. @@ -114,19 +162,14 @@ impl NetworkController { value } - /// Send the data message thought the connection represented by the given endpoint. - /// This function returns a [`SendStatus`] indicating the status of this send. - /// There is no guarantee that send over a correct connection generates a [`SendStatus::Sent`] - /// because any time a connection can be disconnected (even while you are sending). - /// Except cases where you need to be sure that the message has been sent, - /// you will want to process a [`NetEvent::Disconnected`] to determine if the connection + - /// is *alive* instead of check if `send()` returned [`SendStatus::ResourceNotFound`]. - pub fn send(&self, endpoint: Endpoint, data: &[u8]) -> SendStatus { - log::trace!("Send {} bytes to {}", data.len(), endpoint); - let status = - self.controllers[endpoint.resource_id().adapter_id() as usize].send(endpoint, data); - log::trace!("Send status: {:?}", status); - status + /// Check a resource specified by `resource_id` is ready. + /// If the status is `true` means that the resource is ready to use. + /// In connection oriented transports, it implies the resource is connected. + /// If the status is `false` it means that the resource is not yet ready to use. + /// If the resource has been removed, disconnected, or does not exists in the network, + /// a `None` is returned. + pub fn is_ready(&self, resource_id: ResourceId) -> Option { + self.controllers[resource_id.adapter_id() as usize].is_ready(resource_id) } } @@ -175,6 +218,7 @@ impl NetworkProcessor { mod tests { use super::*; use std::time::{Duration}; + use crate::util::thread::{NamespacedThread}; lazy_static::lazy_static! { static ref TIMEOUT: Duration = Duration::from_millis(1000); @@ -190,7 +234,7 @@ mod tests { fn successful_connection() { let (controller, mut processor) = self::split(); let (listener_id, addr) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); - let (endpoint, _) = controller.connect(Transport::Tcp, RemoteAddr::Socket(addr)).unwrap(); + let (endpoint, _) = controller.connect(Transport::Tcp, addr).unwrap(); let mut was_connected = 0; let mut was_accepted = 0; @@ -214,6 +258,23 @@ mod tests { no_more_events(processor); } + #[test] + fn successful_connection_sync() { + let (controller, mut processor) = self::split(); + let (_, addr) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); + + let mut thread = NamespacedThread::spawn("test", move || { + let (endpoint, _) = controller.connect_sync(Transport::Tcp, addr).unwrap(); + assert!(controller.is_ready(endpoint.resource_id()).unwrap()); + }); + + for _ in 0..2 { + processor.process_poll_event(Some(*TIMEOUT), |_| ()); + } + + thread.join(); + } + #[test] fn unreachable_connection() { let (controller, mut processor) = self::split(); @@ -233,6 +294,20 @@ mod tests { no_more_events(processor); } + #[test] + fn unreachable_connection_sync() { + let (controller, mut processor) = self::split(); + + let mut thread = NamespacedThread::spawn("test", move || { + let err = controller.connect_sync(Transport::Tcp, "127.0.0.1:5555").unwrap_err(); + assert!(err.kind() == io::ErrorKind::ConnectionRefused); + }); + + processor.process_poll_event(Some(*TIMEOUT), |_| ()); + + thread.join(); + } + #[test] fn create_remove_listener() { let (controller, processor) = self::split(); @@ -247,7 +322,7 @@ mod tests { fn create_remove_listener_with_connection() { let (controller, mut processor) = self::split(); let (listener_id, addr) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); - controller.connect(Transport::Tcp, RemoteAddr::Socket(addr)).unwrap(); + controller.connect(Transport::Tcp, addr).unwrap(); for _ in 0..2 { // We expect two events diff --git a/src/network/driver.rs b/src/network/driver.rs index b39fb2fd..917e2f41 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -72,6 +72,7 @@ pub trait ActionController: Send + Sync { fn listen(&self, addr: SocketAddr) -> io::Result<(ResourceId, SocketAddr)>; fn send(&self, endpoint: Endpoint, data: &[u8]) -> SendStatus; fn remove(&self, id: ResourceId) -> bool; + fn is_ready(&self, id: ResourceId) -> Option; } pub trait EventProcessor: Send + Sync { @@ -175,6 +176,13 @@ impl ActionController for Driver { ResourceType::Local => self.local_registry.deregister(id), } } + + fn is_ready(&self, id: ResourceId) -> Option { + match id.resource_type() { + ResourceType::Remote => self.remote_registry.get(id).map(|r| r.properties.is_ready()), + ResourceType::Local => self.local_registry.get(id).map(|_| true), + } + } } impl> EventProcessor for Driver { diff --git a/src/network/loader.rs b/src/network/loader.rs index 3e6b1058..b775d2ba 100644 --- a/src/network/loader.rs +++ b/src/network/loader.rs @@ -77,6 +77,10 @@ impl ActionController for UnimplementedDriver { fn remove(&self, _: ResourceId) -> bool { panic!("{}", UNIMPLEMENTED_DRIVER_ERR); } + + fn is_ready(&self, _: ResourceId) -> Option { + panic!("{}", UNIMPLEMENTED_DRIVER_ERR); + } } impl EventProcessor for UnimplementedDriver { From ed16ccaff7b57eca1a48c9e9441429bcca4de791 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Wed, 5 May 2021 21:06:23 +0200 Subject: [PATCH 12/25] Added ResourceNotAvailable to SendStatus --- CHANGELOG.md | 1 + README.md | 10 +++++++--- src/network/adapter.rs | 4 ++++ src/network/driver.rs | 4 ++-- src/network/endpoint.rs | 2 +- 5 files changed, 15 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7c2eff8..335ab845 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Now it performs a non-blocking connection. - Adapter API modified to handle easily handshakes. - Fixed websocket issue that could offer an accepted websocket that was not valid (the websocket handshake could have failed). - Added `NetworkController::is_ready()` +- Added `SendStatus::ResourceNotAvailable` ## Release 0.13.3 - Fixed a bad internal assert. diff --git a/README.md b/README.md index fc12e389..3f60f678 100644 --- a/README.md +++ b/README.md @@ -90,10 +90,14 @@ For example, in order to include only *TCP* and *UDP*, add to your `Cargo.toml`: message-io = { version = "0.14", default-features = false, features = ["tcp", "udp"] } ``` -***Warning**: Version **0.14** modifies the [`connect()`]() behaviour to perform -[**non**-blocking connections](https://github.com/lemunozm/message-io/issues/61). +***Read before update to 0.14**: Version **0.14** modifies the [`connect()`]() behaviour to perform a +[**non**-blocking connections](https://github.com/lemunozm/message-io/issues/61) instead. It is recommended to use this non-blocking mode in order to get the -best scalability and performance in your application, but if you need to perform a similar blocking connection as before, you can use [`connect_sync()`]().* +best scalability and performance in your application. If you need to perform +a similar blocking connection as before (version 0.13), you can call to [`connect_sync()`](). +Note also that the previous `NetEvent::Connect` has been renamed to `NetEvent::Accepted`. +The current `NetEvent::Connect` is a new event to deal with the new non-blocking connections. +See [`NetEvent`]() docs for more info.* ### All in one: TCP, UDP and WebSocket echo server The following example is the simplest server that reads messages from the clients and responds diff --git a/src/network/adapter.rs b/src/network/adapter.rs index d98d6551..ea1815ec 100644 --- a/src/network/adapter.rs +++ b/src/network/adapter.rs @@ -73,6 +73,10 @@ pub enum SendStatus { /// This implies that a [`crate::network::NetEvent::Disconnected`] has happened or that /// the resource never existed. ResourceNotFound, + + /// The resource can not perform the required send operation. + /// Usually this is due because it is performing the handshake. + ResourceNotAvailable, } /// Returned as a result of [`Remote::receive()`] diff --git a/src/network/driver.rs b/src/network/driver.rs index 917e2f41..578ccfb4 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -41,7 +41,7 @@ pub enum NetEvent<'a> { /// event. This one-to-one relation is not conserved in stream-based transports as *TCP*. /// /// If you want a packet-based protocol over *TCP* use - /// [`crate::transport::Transport::FramedTcp`]. + /// [`crate::network::Transport::FramedTcp`]. Message(Endpoint, &'a [u8]), /// This event is only dispatched when a connection is lost. @@ -159,7 +159,7 @@ impl ActionController for Driver { ResourceType::Remote => match self.remote_registry.get(endpoint.resource_id()) { Some(remote) => match remote.properties.is_ready() { true => remote.resource.send(data), - false => SendStatus::ResourceNotFound, + false => SendStatus::ResourceNotAvailable, }, None => SendStatus::ResourceNotFound, }, diff --git a/src/network/endpoint.rs b/src/network/endpoint.rs index c87ac8f2..e7dfb287 100644 --- a/src/network/endpoint.rs +++ b/src/network/endpoint.rs @@ -55,7 +55,7 @@ impl Endpoint { // Only local resources allowed assert_eq!(id.resource_type(), super::resource_id::ResourceType::Local); - // Only packet based transport protocols allowed + // Only non connection-oriented transport protocols allowed assert!(!super::transport::Transport::from(id.adapter_id()).is_connection_oriented()); Endpoint::new(id, addr) From 8a2981523291bce938cd462949d7fdb51457ffc1 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Thu, 6 May 2021 00:20:42 +0200 Subject: [PATCH 13/25] Added pending as mandatory --- src/adapters/framed_tcp.rs | 8 +++- src/adapters/tcp.rs | 20 +++++++- src/adapters/template.rs | 22 +++++---- src/adapters/udp.rs | 8 +++- src/adapters/ws.rs | 96 +++++++++++++++++++++++--------------- src/network.rs | 51 ++++++++++++-------- src/network/adapter.rs | 16 ++----- 7 files changed, 137 insertions(+), 84 deletions(-) diff --git a/src/adapters/framed_tcp.rs b/src/adapters/framed_tcp.rs index 5cc84cbc..ea411c70 100644 --- a/src/adapters/framed_tcp.rs +++ b/src/adapters/framed_tcp.rs @@ -1,8 +1,8 @@ use crate::network::adapter::{ Resource, Remote, Local, Adapter, SendStatus, AcceptedType, ReadStatus, ConnectionInfo, - ListeningInfo, + ListeningInfo, PendingStatus, }; -use crate::network::{RemoteAddr}; +use crate::network::{RemoteAddr, Readiness}; use crate::util::encoding::{self, Decoder, MAX_ENCODED_SIZE}; use mio::net::{TcpListener, TcpStream}; @@ -111,6 +111,10 @@ impl Remote for RemoteResource { } } } + + fn pending(&self, _readiness: Readiness) -> PendingStatus { + super::tcp::check_stream_ready(&self.stream) + } } pub(crate) struct LocalResource { diff --git a/src/adapters/tcp.rs b/src/adapters/tcp.rs index e94054a7..95e3f28a 100644 --- a/src/adapters/tcp.rs +++ b/src/adapters/tcp.rs @@ -1,8 +1,8 @@ use crate::network::adapter::{ Resource, Remote, Local, Adapter, SendStatus, AcceptedType, ReadStatus, ConnectionInfo, - ListeningInfo, + ListeningInfo, PendingStatus, }; -use crate::network::{RemoteAddr}; +use crate::network::{RemoteAddr, Readiness}; use mio::net::{TcpListener, TcpStream}; use mio::event::{Source}; @@ -97,6 +97,22 @@ impl Remote for RemoteResource { } } } + + fn pending(&self, _readiness: Readiness) -> PendingStatus { + check_stream_ready(&self.stream) + } +} + +pub fn check_stream_ready(stream: &TcpStream) -> PendingStatus { + if let Ok(Some(_)) = stream.take_error() { + return PendingStatus::Disconnected + } + match stream.peer_addr() { + Ok(_) => PendingStatus::Ready, + Err(err) if err.kind() == io::ErrorKind::NotConnected => PendingStatus::Incomplete, + Err(err) if err.kind() == io::ErrorKind::InvalidInput => PendingStatus::Incomplete, + Err(_) => PendingStatus::Disconnected, + } } pub(crate) struct LocalResource { diff --git a/src/adapters/template.rs b/src/adapters/template.rs index 23696956..46815c4a 100644 --- a/src/adapters/template.rs +++ b/src/adapters/template.rs @@ -2,9 +2,9 @@ use crate::network::adapter::{ Resource, Remote, Local, Adapter, SendStatus, AcceptedType, ReadStatus, ConnectionInfo, - ListeningInfo, + ListeningInfo, PendingStatus, }; -use crate::network::{RemoteAddr}; +use crate::network::{RemoteAddr, Readiness}; use mio::event::{Source}; @@ -20,28 +20,32 @@ impl Adapter for MyAdapter { pub(crate) struct RemoteResource; impl Resource for RemoteResource { fn source(&mut self) -> &mut dyn Source { - todo!(); + todo!() } } impl Remote for RemoteResource { fn connect(remote_addr: RemoteAddr) -> io::Result> { - todo!(); + todo!() } fn receive(&self, process_data: impl FnMut(&[u8])) -> ReadStatus { - todo!(); + todo!() } fn send(&self, data: &[u8]) -> SendStatus { - todo!(); + todo!() + } + + fn pending(&self, _readiness: Readiness) -> PendingStatus { + todo!() } } pub(crate) struct LocalResource; impl Resource for LocalResource { fn source(&mut self) -> &mut dyn Source { - todo!(); + todo!() } } @@ -49,10 +53,10 @@ impl Local for LocalResource { type Remote = RemoteResource; fn listen(addr: SocketAddr) -> io::Result> { - todo!(); + todo!() } fn accept(&self, accept_remote: impl FnMut(AcceptedType<'_, Self::Remote>)) { - todo!(); + todo!() } } diff --git a/src/adapters/udp.rs b/src/adapters/udp.rs index 6ed06322..2618a61a 100644 --- a/src/adapters/udp.rs +++ b/src/adapters/udp.rs @@ -1,8 +1,8 @@ use crate::network::adapter::{ Resource, Remote, Local, Adapter, SendStatus, AcceptedType, ReadStatus, ConnectionInfo, - ListeningInfo, + ListeningInfo, PendingStatus, }; -use crate::network::{RemoteAddr}; +use crate::network::{RemoteAddr, Readiness}; use mio::net::{UdpSocket}; use mio::event::{Source}; @@ -74,6 +74,10 @@ impl Remote for RemoteResource { fn send(&self, data: &[u8]) -> SendStatus { send_packet(data, |data| self.socket.send(data)) } + + fn pending(&self, _readiness: Readiness) -> PendingStatus { + PendingStatus::Ready + } } pub(crate) struct LocalResource { diff --git a/src/adapters/ws.rs b/src/adapters/ws.rs index ab8a09e7..9c8d285e 100644 --- a/src/adapters/ws.rs +++ b/src/adapters/ws.rs @@ -36,6 +36,7 @@ impl Adapter for WsAdapter { } enum PendingHandshake { + Connect(Url, TcpStream), Client(MidHandshake>), Server(MidHandshake>), } @@ -54,6 +55,7 @@ impl Resource for RemoteResource { match self.state.get_mut().unwrap() { RemoteState::WebSocket(web_socket) => web_socket.get_mut(), RemoteState::Handshake(Some(handshake)) => match handshake { + PendingHandshake::Connect(_, stream) => stream, PendingHandshake::Client(mid_handshake) => mid_handshake.get_mut().get_mut(), PendingHandshake::Server(mid_handshake) => mid_handshake.get_mut().get_mut(), }, @@ -84,19 +86,15 @@ impl Remote for RemoteResource { let stream = TcpStream::connect(peer_addr)?; let local_addr = stream.local_addr()?; - let remote = match ws_connect(url, stream) { - Ok((web_socket, _)) => RemoteState::WebSocket(web_socket), - Err(HandshakeError::Interrupted(mid_handshake)) => { - RemoteState::Handshake(Some(PendingHandshake::Client(mid_handshake))) - } - Err(HandshakeError::Failure(Error::Io(err))) => return Err(err), - Err(HandshakeError::Failure(err)) => { - panic!("WS connect handshake error: {}", err) - } - }; + /* + */ Ok(ConnectionInfo { - remote: RemoteResource { state: Mutex::new(remote) }, + remote: RemoteResource { + state: Mutex::new(RemoteState::Handshake(Some(PendingHandshake::Connect( + url, stream, + )))), + }, local_addr, peer_addr, }) @@ -143,11 +141,58 @@ impl Remote for RemoteResource { } } + fn send(&self, data: &[u8]) -> SendStatus { + match self.state.lock().expect(OTHER_THREAD_ERR).deref_mut() { + RemoteState::WebSocket(web_socket) => { + let message = Message::Binary(data.to_vec()); + let mut result = web_socket.write_message(message); + loop { + match result { + Ok(_) => break SendStatus::Sent, + Err(Error::Io(ref err)) if err.kind() == ErrorKind::WouldBlock => { + result = web_socket.write_pending(); + } + Err(Error::Capacity(_)) => { + break SendStatus::MaxPacketSizeExceeded(data.len(), MAX_PAYLOAD_LEN) + } + Err(err) => { + log::error!("WS send error: {}", err); + break SendStatus::ResourceNotFound // should not happen + } + } + } + } + RemoteState::Handshake(_) => unreachable!(), + } + } + fn pending(&self, _readiness: Readiness) -> PendingStatus { let mut state = self.state.lock().expect(OTHER_THREAD_ERR); match state.deref_mut() { RemoteState::WebSocket(_) => PendingStatus::Ready, RemoteState::Handshake(pending) => match pending.take().unwrap() { + PendingHandshake::Connect(url, stream) => { + match super::tcp::check_stream_ready(&stream) { + PendingStatus::Ready => match ws_connect(url, stream) { + Ok((web_socket, _)) => { + *state = RemoteState::WebSocket(web_socket); + PendingStatus::Ready + } + Err(HandshakeError::Interrupted(mid_handshake)) => { + *pending = Some(PendingHandshake::Client(mid_handshake)); + PendingStatus::Incomplete + } + Err(HandshakeError::Failure(Error::Io(_))) => { + PendingStatus::Disconnected + } + Err(HandshakeError::Failure(err)) => { + log::error!("WS connect handshake error: {}", err); + PendingStatus::Disconnected // should not happen + } + }, + other => other, + } + } PendingHandshake::Client(mid_handshake) => match mid_handshake.handshake() { Ok((web_socket, _)) => { *state = RemoteState::WebSocket(web_socket); @@ -159,7 +204,7 @@ impl Remote for RemoteResource { } Err(HandshakeError::Failure(Error::Io(_))) => PendingStatus::Disconnected, Err(HandshakeError::Failure(err)) => { - log::error!("WS connect handshake error: {}", err); + log::error!("WS client handshake error: {}", err); PendingStatus::Disconnected // should not happen } }, @@ -174,38 +219,13 @@ impl Remote for RemoteResource { } Err(HandshakeError::Failure(Error::Io(_))) => PendingStatus::Disconnected, Err(HandshakeError::Failure(err)) => { - log::error!("WS accept handshake error: {}", err); + log::error!("WS server handshake error: {}", err); PendingStatus::Disconnected // should not happen } }, }, } } - - fn send(&self, data: &[u8]) -> SendStatus { - match self.state.lock().expect(OTHER_THREAD_ERR).deref_mut() { - RemoteState::WebSocket(web_socket) => { - let message = Message::Binary(data.to_vec()); - let mut result = web_socket.write_message(message); - loop { - match result { - Ok(_) => break SendStatus::Sent, - Err(Error::Io(ref err)) if err.kind() == ErrorKind::WouldBlock => { - result = web_socket.write_pending(); - } - Err(Error::Capacity(_)) => { - break SendStatus::MaxPacketSizeExceeded(data.len(), MAX_PAYLOAD_LEN) - } - Err(err) => { - log::error!("WS send error: {}", err); - break SendStatus::ResourceNotFound // should not happen - } - } - } - } - RemoteState::Handshake(_) => unreachable!(), - } - } } impl RemoteResource { diff --git a/src/network.rs b/src/network.rs index d1849700..8b4be744 100644 --- a/src/network.rs +++ b/src/network.rs @@ -222,6 +222,7 @@ mod tests { lazy_static::lazy_static! { static ref TIMEOUT: Duration = Duration::from_millis(1000); + static ref LOCALHOST_CONN_TIMEOUT: Duration = Duration::from_millis(5000); } fn no_more_events(mut processor: NetworkProcessor) { @@ -238,19 +239,22 @@ mod tests { let mut was_connected = 0; let mut was_accepted = 0; - for _ in 0..2 { - processor.process_poll_event(Some(*TIMEOUT), |net_event| match net_event { - NetEvent::Connected(net_endpoint, status) => { - assert!(status); - assert_eq!(endpoint, net_endpoint); - was_connected += 1; - } - NetEvent::Accepted(_, net_listener_id) => { - assert_eq!(listener_id, net_listener_id); - was_accepted += 1; - } - _ => unreachable!(), - }); + for _ in 0..5 { + processor.process_poll_event( + Some(*TIMEOUT), + |net_event| match net_event { + NetEvent::Connected(net_endpoint, status) => { + assert!(status); + assert_eq!(endpoint, net_endpoint); + was_connected += 1; + } + NetEvent::Accepted(_, net_listener_id) => { + assert_eq!(listener_id, net_listener_id); + was_accepted += 1; + } + _ => unreachable!(), + }, + ); } assert_eq!(was_connected, 1); assert_eq!(was_accepted, 1); @@ -268,8 +272,8 @@ mod tests { assert!(controller.is_ready(endpoint.resource_id()).unwrap()); }); - for _ in 0..2 { - processor.process_poll_event(Some(*TIMEOUT), |_| ()); + for _ in 0..5 { + processor.process_poll_event(Some(*LOCALHOST_CONN_TIMEOUT), |_| ()); } thread.join(); @@ -278,10 +282,15 @@ mod tests { #[test] fn unreachable_connection() { let (controller, mut processor) = self::split(); - let (endpoint, _) = controller.connect(Transport::Tcp, "127.0.0.1:5555").unwrap(); + + // Ensure that addr is not using by other process because it takes some secs to be reusable. + let (listener_id, addr) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); + controller.remove(listener_id); + + let (endpoint, _) = controller.connect(Transport::Tcp, addr).unwrap(); let mut was_disconnected = false; - processor.process_poll_event(Some(*TIMEOUT), |net_event| match net_event { + processor.process_poll_event(Some(*LOCALHOST_CONN_TIMEOUT), |net_event| match net_event { NetEvent::Connected(net_endpoint, status) => { assert!(!status); assert_eq!(endpoint, net_endpoint); @@ -298,12 +307,16 @@ mod tests { fn unreachable_connection_sync() { let (controller, mut processor) = self::split(); + // Ensure that addr is not using by other process because it takes some secs to be reusable. + let (listener_id, addr) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); + controller.remove(listener_id); + let mut thread = NamespacedThread::spawn("test", move || { - let err = controller.connect_sync(Transport::Tcp, "127.0.0.1:5555").unwrap_err(); + let err = controller.connect_sync(Transport::Tcp, addr).unwrap_err(); assert!(err.kind() == io::ErrorKind::ConnectionRefused); }); - processor.process_poll_event(Some(*TIMEOUT), |_| ()); + processor.process_poll_event(Some(*LOCALHOST_CONN_TIMEOUT), |_| ()); thread.join(); } diff --git a/src/network/adapter.rs b/src/network/adapter.rs index ea1815ec..10f57bff 100644 --- a/src/network/adapter.rs +++ b/src/network/adapter.rs @@ -24,8 +24,8 @@ pub trait Adapter: Send + Sync { /// asynchronously from events. /// Your [`Remote`] and [`Local`] entities must implement `Resource`. pub trait Resource: Send + Sync { - /// This is the only method required to make your element a resource. - /// Note: Any `mio` network element implements [`Source`], you probably wants to use + /// Returns a mutable reference to the internal `Source`. + /// Note: All `mio` network element implements [`Source`], you probably wants to use /// one of them as a base for your non-blocking transport. /// See [`Source`]. fn source(&mut self) -> &mut dyn Source; @@ -94,7 +94,7 @@ pub enum ReadStatus { WaitNextEvent, } -#[derive(Debug)] +#[derive(Debug, Clone, PartialEq)] pub enum PendingStatus { /// The resource is no longer considered as a pending resource. /// It it came from a listener, a [`crate::network::NetEvent::Accepted`] event will be generated. @@ -159,15 +159,7 @@ pub trait Remote: Resource + Sized { /// resource ready to use. /// The **implementator** is in charge to write or read from the resource /// (depending of the readiness) until it obtains an internal [`std::io::ErrorKind::WouldBlock`]. - fn pending(&self, readiness: Readiness) -> PendingStatus { - match readiness { - Readiness::Write => PendingStatus::Ready, - - // If before getting a Write readiness it obtains a Read readiness - // means that a disconnection or a connection error has been produced. - Readiness::Read => PendingStatus::Disconnected, - } - } + fn pending(&self, readiness: Readiness) -> PendingStatus; } /// Used as a parameter callback in [`Local::accept()`] From f62f28f324221e75363673869762b54217f788e0 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Sun, 9 May 2021 16:44:18 +0200 Subject: [PATCH 14/25] Added process_poll_events_until_timeout. Splitted conenctio tests for different transports. --- src/adapters/ws.rs | 3 - src/network.rs | 157 +++++++++++++++++++++------------------- src/network/endpoint.rs | 16 ---- src/network/poll.rs | 2 +- 4 files changed, 85 insertions(+), 93 deletions(-) diff --git a/src/adapters/ws.rs b/src/adapters/ws.rs index 9c8d285e..a01d09e1 100644 --- a/src/adapters/ws.rs +++ b/src/adapters/ws.rs @@ -86,9 +86,6 @@ impl Remote for RemoteResource { let stream = TcpStream::connect(peer_addr)?; let local_addr = stream.local_addr()?; - /* - */ - Ok(ConnectionInfo { remote: RemoteResource { state: Mutex::new(RemoteState::Handshake(Some(PendingHandshake::Connect( diff --git a/src/network.rs b/src/network.rs index 8b4be744..b78f9bf9 100644 --- a/src/network.rs +++ b/src/network.rs @@ -27,7 +27,7 @@ use poll::{Poll, PollEvent}; use strum::{IntoEnumIterator}; use std::net::{SocketAddr, ToSocketAddrs}; -use std::time::{Duration}; +use std::time::{Duration, Instant}; use std::io::{self}; /// Create a network instance giving its controller and processor. @@ -186,7 +186,7 @@ impl NetworkProcessor { } /// Process the next poll event. - /// This functions waits the timeout specified until the poll event is generated. + /// This method waits the timeout specified until the poll event is generated. /// If `None` is passed as timeout, it will wait indefinitely. /// Note that there is no 1-1 relation between an internal poll event and a [`NetEvent`]. /// You need to assume that process an internal poll event could call 0 or N times to @@ -212,6 +212,22 @@ impl NetworkProcessor { } }); } + + /// Process poll events until there is no more events during a `timeout` duration. + /// This method makes succesive calls to [`NetworkProcessor::process_poll_event()`]. + pub fn process_poll_events_until_timeout( + &mut self, + timeout: Duration, + mut event_callback: impl FnMut(NetEvent<'_>), + ) { + loop { + let now = Instant::now(); + self.process_poll_event(Some(timeout), |e| event_callback(e)); + if now.elapsed() > timeout { + break + } + } + } } #[cfg(test)] @@ -220,115 +236,112 @@ mod tests { use std::time::{Duration}; use crate::util::thread::{NamespacedThread}; + use test_case::test_case; + lazy_static::lazy_static! { static ref TIMEOUT: Duration = Duration::from_millis(1000); static ref LOCALHOST_CONN_TIMEOUT: Duration = Duration::from_millis(5000); } - fn no_more_events(mut processor: NetworkProcessor) { - let mut was_event = false; - processor.process_poll_event(Some(*TIMEOUT), |_| was_event = true); - assert!(!was_event); - } - - #[test] - fn successful_connection() { + #[cfg_attr(feature = "tcp", test_case(Transport::Tcp))] + #[cfg_attr(feature = "tcp", test_case(Transport::FramedTcp))] + #[cfg_attr(feature = "websocket", test_case(Transport::Ws))] + fn successful_connection(transport: Transport) { let (controller, mut processor) = self::split(); - let (listener_id, addr) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); - let (endpoint, _) = controller.connect(Transport::Tcp, addr).unwrap(); + let (listener_id, addr) = controller.listen(transport, "127.0.0.1:0").unwrap(); + let (endpoint, _) = controller.connect(transport, addr).unwrap(); let mut was_connected = 0; let mut was_accepted = 0; - for _ in 0..5 { - processor.process_poll_event( - Some(*TIMEOUT), - |net_event| match net_event { - NetEvent::Connected(net_endpoint, status) => { - assert!(status); - assert_eq!(endpoint, net_endpoint); - was_connected += 1; - } - NetEvent::Accepted(_, net_listener_id) => { - assert_eq!(listener_id, net_listener_id); - was_accepted += 1; - } - _ => unreachable!(), - }, - ); - } - assert_eq!(was_connected, 1); + processor.process_poll_events_until_timeout(*TIMEOUT, |net_event| match net_event { + NetEvent::Connected(net_endpoint, status) => { + assert!(status); + assert_eq!(endpoint, net_endpoint); + was_connected += 1; + } + NetEvent::Accepted(_, net_listener_id) => { + assert_eq!(listener_id, net_listener_id); + was_accepted += 1; + } + _ => unreachable!(), + }); assert_eq!(was_accepted, 1); - - no_more_events(processor); + assert_eq!(was_connected, 1); } - #[test] - fn successful_connection_sync() { + #[cfg_attr(feature = "tcp", test_case(Transport::Tcp))] + #[cfg_attr(feature = "tcp", test_case(Transport::FramedTcp))] + #[cfg_attr(feature = "websocket", test_case(Transport::Ws))] + fn successful_connection_sync(transport: Transport) { let (controller, mut processor) = self::split(); - let (_, addr) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); + let (_, addr) = controller.listen(transport, "127.0.0.1:0").unwrap(); let mut thread = NamespacedThread::spawn("test", move || { - let (endpoint, _) = controller.connect_sync(Transport::Tcp, addr).unwrap(); + let (endpoint, _) = controller.connect_sync(transport, addr).unwrap(); assert!(controller.is_ready(endpoint.resource_id()).unwrap()); }); - for _ in 0..5 { - processor.process_poll_event(Some(*LOCALHOST_CONN_TIMEOUT), |_| ()); - } + processor.process_poll_events_until_timeout(*TIMEOUT, |_| ()); thread.join(); } - #[test] - fn unreachable_connection() { + #[cfg_attr(feature = "tcp", test_case(Transport::Tcp))] + #[cfg_attr(feature = "tcp", test_case(Transport::FramedTcp))] + #[cfg_attr(feature = "websocket", test_case(Transport::Ws))] + fn unreachable_connection(transport: Transport) { let (controller, mut processor) = self::split(); - // Ensure that addr is not using by other process because it takes some secs to be reusable. - let (listener_id, addr) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); + // Ensure that addr is not using by other process + // because it takes some secs to be reusable. + let (listener_id, addr) = controller.listen(transport, "127.0.0.1:0").unwrap(); controller.remove(listener_id); - let (endpoint, _) = controller.connect(Transport::Tcp, addr).unwrap(); + let (endpoint, _) = controller.connect(transport, addr).unwrap(); let mut was_disconnected = false; - processor.process_poll_event(Some(*LOCALHOST_CONN_TIMEOUT), |net_event| match net_event { - NetEvent::Connected(net_endpoint, status) => { - assert!(!status); - assert_eq!(endpoint, net_endpoint); - was_disconnected = true; + processor.process_poll_events_until_timeout(*LOCALHOST_CONN_TIMEOUT, |net_event| { + match net_event { + NetEvent::Connected(net_endpoint, status) => { + assert!(!status); + assert_eq!(endpoint, net_endpoint); + was_disconnected = true; + } + _ => unreachable!(), } - _ => unreachable!(), }); assert!(was_disconnected); - - no_more_events(processor); } - #[test] - fn unreachable_connection_sync() { + #[cfg_attr(feature = "tcp", test_case(Transport::Tcp))] + #[cfg_attr(feature = "tcp", test_case(Transport::FramedTcp))] + #[cfg_attr(feature = "websocket", test_case(Transport::Ws))] + fn unreachable_connection_sync(transport: Transport) { let (controller, mut processor) = self::split(); - // Ensure that addr is not using by other process because it takes some secs to be reusable. - let (listener_id, addr) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); + // Ensure that addr is not using by other process + // because it takes some secs to be reusable. + let (listener_id, addr) = controller.listen(transport, "127.0.0.1:0").unwrap(); controller.remove(listener_id); let mut thread = NamespacedThread::spawn("test", move || { - let err = controller.connect_sync(Transport::Tcp, addr).unwrap_err(); + let err = controller.connect_sync(transport, addr).unwrap_err(); assert!(err.kind() == io::ErrorKind::ConnectionRefused); }); - processor.process_poll_event(Some(*LOCALHOST_CONN_TIMEOUT), |_| ()); + processor.process_poll_events_until_timeout(*LOCALHOST_CONN_TIMEOUT, |_| ()); thread.join(); } #[test] fn create_remove_listener() { - let (controller, processor) = self::split(); + let (controller, mut processor) = self::split(); let (listener_id, _) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); assert!(controller.remove(listener_id)); // Do not generate an event assert!(!controller.remove(listener_id)); - no_more_events(processor); + processor.process_poll_events_until_timeout(*TIMEOUT, |_| unreachable!()); } #[test] @@ -337,18 +350,16 @@ mod tests { let (listener_id, addr) = controller.listen(Transport::Tcp, "127.0.0.1:0").unwrap(); controller.connect(Transport::Tcp, addr).unwrap(); - for _ in 0..2 { - // We expect two events - processor.process_poll_event(Some(*TIMEOUT), |net_event| match net_event { - NetEvent::Connected(..) => (), - NetEvent::Accepted(_, _) => { - assert!(controller.remove(listener_id)); - assert!(!controller.remove(listener_id)); - } - _ => unreachable!(), - }); - } - - no_more_events(processor); + let mut was_accepted = false; + processor.process_poll_events_until_timeout(*TIMEOUT, |net_event| match net_event { + NetEvent::Connected(..) => (), + NetEvent::Accepted(_, _) => { + assert!(controller.remove(listener_id)); + assert!(!controller.remove(listener_id)); + was_accepted = true; + } + _ => unreachable!(), + }); + assert!(was_accepted); } } diff --git a/src/network/endpoint.rs b/src/network/endpoint.rs index e7dfb287..62fe37c1 100644 --- a/src/network/endpoint.rs +++ b/src/network/endpoint.rs @@ -90,22 +90,6 @@ mod tests { use crate::network::resource_id::{ResourceType, ResourceIdGenerator}; use crate::network::transport::{Transport}; - #[test] - #[should_panic] - fn from_remote_non_connection_oriented() { - let addr = "0.0.0.0:0".parse().unwrap(); - let generator = ResourceIdGenerator::new(Transport::Udp.id(), ResourceType::Remote); - Endpoint::from_listener(generator.generate(), addr); - } - - #[test] - #[should_panic] - fn from_local_connection_oriented() { - let addr = "0.0.0.0:0".parse().unwrap(); - let generator = ResourceIdGenerator::new(Transport::Tcp.id(), ResourceType::Local); - Endpoint::from_listener(generator.generate(), addr); - } - #[test] fn from_local_non_connection_oriented() { let addr = "0.0.0.0:0".parse().unwrap(); diff --git a/src/network/poll.rs b/src/network/poll.rs index 79e2d744..7c2b37c1 100644 --- a/src/network/poll.rs +++ b/src/network/poll.rs @@ -62,7 +62,7 @@ impl Poll { where C: FnMut(PollEvent) { loop { match self.mio_poll.poll(&mut self.events, timeout) { - Ok(_) => { + Ok(()) => { for mio_event in &self.events { if Self::WAKER_TOKEN == mio_event.token() { log::trace!("POLL WAKER EVENT"); From 34397495ef7ff611dbb79764027ee5049966467c Mon Sep 17 00:00:00 2001 From: lemunozm Date: Sun, 9 May 2021 22:10:58 +0200 Subject: [PATCH 15/25] Added Resource::source with Option. Forced because tungstenite takes the ownership of the stream and drop it in Failure cases. --- src/adapters/framed_tcp.rs | 8 ++++---- src/adapters/tcp.rs | 8 ++++---- src/adapters/template.rs | 4 ++-- src/adapters/udp.rs | 8 ++++---- src/adapters/ws.rs | 16 ++++++++-------- src/network/adapter.rs | 2 +- src/network/registry.rs | 6 ++++-- 7 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/adapters/framed_tcp.rs b/src/adapters/framed_tcp.rs index ea411c70..bbacf5bc 100644 --- a/src/adapters/framed_tcp.rs +++ b/src/adapters/framed_tcp.rs @@ -39,8 +39,8 @@ impl From for RemoteResource { } impl Resource for RemoteResource { - fn source(&mut self) -> &mut dyn Source { - &mut self.stream + fn source(&mut self) -> Option<&mut dyn Source> { + Some(&mut self.stream) } } @@ -122,8 +122,8 @@ pub(crate) struct LocalResource { } impl Resource for LocalResource { - fn source(&mut self) -> &mut dyn Source { - &mut self.listener + fn source(&mut self) -> Option<&mut dyn Source> { + Some(&mut self.listener) } } diff --git a/src/adapters/tcp.rs b/src/adapters/tcp.rs index 95e3f28a..b4e02325 100644 --- a/src/adapters/tcp.rs +++ b/src/adapters/tcp.rs @@ -34,8 +34,8 @@ impl From for RemoteResource { } impl Resource for RemoteResource { - fn source(&mut self) -> &mut dyn Source { - &mut self.stream + fn source(&mut self) -> Option<&mut dyn Source> { + Some(&mut self.stream) } } @@ -120,8 +120,8 @@ pub(crate) struct LocalResource { } impl Resource for LocalResource { - fn source(&mut self) -> &mut dyn Source { - &mut self.listener + fn source(&mut self) -> Option<&mut dyn Source> { + Some(&mut self.listener) } } diff --git a/src/adapters/template.rs b/src/adapters/template.rs index 46815c4a..28684210 100644 --- a/src/adapters/template.rs +++ b/src/adapters/template.rs @@ -19,7 +19,7 @@ impl Adapter for MyAdapter { pub(crate) struct RemoteResource; impl Resource for RemoteResource { - fn source(&mut self) -> &mut dyn Source { + fn source(&mut self) -> Option<&mut dyn Source> { todo!() } } @@ -44,7 +44,7 @@ impl Remote for RemoteResource { pub(crate) struct LocalResource; impl Resource for LocalResource { - fn source(&mut self) -> &mut dyn Source { + fn source(&mut self) -> Option<&mut dyn Source> { todo!() } } diff --git a/src/adapters/udp.rs b/src/adapters/udp.rs index 2618a61a..be552d17 100644 --- a/src/adapters/udp.rs +++ b/src/adapters/udp.rs @@ -35,8 +35,8 @@ pub(crate) struct RemoteResource { } impl Resource for RemoteResource { - fn source(&mut self) -> &mut dyn Source { - &mut self.socket + fn source(&mut self) -> Option<&mut dyn Source> { + Some(&mut self.socket) } } @@ -85,8 +85,8 @@ pub(crate) struct LocalResource { } impl Resource for LocalResource { - fn source(&mut self) -> &mut dyn Source { - &mut self.socket + fn source(&mut self) -> Option<&mut dyn Source> { + Some(&mut self.socket) } } diff --git a/src/adapters/ws.rs b/src/adapters/ws.rs index a01d09e1..e6b8b5fd 100644 --- a/src/adapters/ws.rs +++ b/src/adapters/ws.rs @@ -51,15 +51,15 @@ pub(crate) struct RemoteResource { } impl Resource for RemoteResource { - fn source(&mut self) -> &mut dyn Source { + fn source(&mut self) -> Option<&mut dyn Source> { match self.state.get_mut().unwrap() { - RemoteState::WebSocket(web_socket) => web_socket.get_mut(), + RemoteState::WebSocket(web_socket) => Some(web_socket.get_mut()), RemoteState::Handshake(Some(handshake)) => match handshake { - PendingHandshake::Connect(_, stream) => stream, - PendingHandshake::Client(mid_handshake) => mid_handshake.get_mut().get_mut(), - PendingHandshake::Server(mid_handshake) => mid_handshake.get_mut().get_mut(), + PendingHandshake::Connect(_, stream) => Some(stream), + PendingHandshake::Client(mid_handshake) => Some(mid_handshake.get_mut().get_mut()), + PendingHandshake::Server(mid_handshake) => Some(mid_handshake.get_mut().get_mut()), }, - RemoteState::Handshake(None) => unreachable!(), + RemoteState::Handshake(None) => None, } } } @@ -245,8 +245,8 @@ pub(crate) struct LocalResource { } impl Resource for LocalResource { - fn source(&mut self) -> &mut dyn Source { - &mut self.listener + fn source(&mut self) -> Option<&mut dyn Source> { + Some(&mut self.listener) } } diff --git a/src/network/adapter.rs b/src/network/adapter.rs index 10f57bff..1c4329b9 100644 --- a/src/network/adapter.rs +++ b/src/network/adapter.rs @@ -28,7 +28,7 @@ pub trait Resource: Send + Sync { /// Note: All `mio` network element implements [`Source`], you probably wants to use /// one of them as a base for your non-blocking transport. /// See [`Source`]. - fn source(&mut self) -> &mut dyn Source; + fn source(&mut self) -> Option<&mut dyn Source>; } /// Plain struct used as a returned value of [`Remote::connect()`] diff --git a/src/network/registry.rs b/src/network/registry.rs index 1e3e03ba..eb940f3c 100644 --- a/src/network/registry.rs +++ b/src/network/registry.rs @@ -22,7 +22,9 @@ impl Register { impl Drop for Register { fn drop(&mut self) { - self.poll_registry.remove(self.resource.source()); + if let Some(source) = self.resource.source() { + self.poll_registry.remove(source); + } } } @@ -44,7 +46,7 @@ impl ResourceRegistry { // The registry must be locked for the entire implementation to avoid the poll // to generate events over not yet registered resources. let mut registry = self.resources.write().expect(OTHER_THREAD_ERR); - let id = self.poll_registry.add(resource.source(), write_readiness); + let id = self.poll_registry.add(resource.source().unwrap(), write_readiness); let register = Register::new(resource, properties, self.poll_registry.clone()); registry.insert(id, Arc::new(register)); id From d0452cc7686d03fac76dbfa7d0cd5f6f5862375a Mon Sep 17 00:00:00 2001 From: lemunozm Date: Sun, 9 May 2021 08:42:39 +0200 Subject: [PATCH 16/25] Fixed windows poll event issue --- src/network/driver.rs | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/network/driver.rs b/src/network/driver.rs index 578ccfb4..c5b34378 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -199,13 +199,15 @@ impl> EventProcessor for Driver { log::trace!("Processed remote for {}", endpoint); if !remote.properties.is_ready() { - self.process_pending_remote(remote, endpoint, readiness, event_callback); + self.process_pending_remote(&remote, endpoint, readiness, |e| { + event_callback(e) + }); } - else { + if remote.properties.is_ready() { match readiness { Readiness::Write => remote.resource.ready_to_write(), Readiness::Read => { - self.read_from_remote(remote, endpoint, event_callback); + self.read_from_remote(&remote, endpoint, event_callback); } } } @@ -216,7 +218,7 @@ impl> EventProcessor for Driver { log::trace!("Processed local for {}", id); match readiness { Readiness::Write => (), - Readiness::Read => self.read_from_local(local, id, event_callback), + Readiness::Read => self.read_from_local(&local, id, event_callback), } } } @@ -227,7 +229,7 @@ impl> EventProcessor for Driver { impl> Driver { fn process_pending_remote( &self, - remote: Arc>, + remote: &Arc>, endpoint: Endpoint, readiness: Readiness, mut event_callback: impl FnMut(NetEvent<'_>), @@ -253,7 +255,7 @@ impl> Driver { fn read_from_remote( &self, - remote: Arc>, + remote: &Arc>, endpoint: Endpoint, mut event_callback: impl FnMut(NetEvent<'_>), ) { @@ -270,7 +272,7 @@ impl> Driver { fn read_from_local( &self, - local: Arc>, + local: &Arc>, id: ResourceId, mut event_callback: impl FnMut(NetEvent<'_>), ) { @@ -278,19 +280,11 @@ impl> Driver { log::trace!("Accepted type: {}", accepted); match accepted { AcceptedType::Remote(addr, remote) => { - let remote_id = self.remote_registry.register( + self.remote_registry.register( remote, RemoteProperties::new(addr, Some(id)), true, ); - - // We Manually generate a Poll Write event, because an accepted connection - // is ready to write. - let remote = self.remote_registry.get(remote_id).unwrap(); - let endpoint = Endpoint::new(remote_id, addr); - self.process_pending_remote(remote, endpoint, Readiness::Write, |net_event| { - event_callback(net_event) - }); } AcceptedType::Data(addr, data) => { let endpoint = Endpoint::new(id, addr); From 8d04a5d385b3d421a61da22a334cf073e1c4f2c6 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Sun, 9 May 2021 09:37:13 +0200 Subject: [PATCH 17/25] Added connection examples. Updated readme --- README.md | 10 ++++----- src/network.rs | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/node.rs | 6 +++--- 3 files changed, 63 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 3f60f678..c5cf2a15 100644 --- a/README.md +++ b/README.md @@ -90,14 +90,14 @@ For example, in order to include only *TCP* and *UDP*, add to your `Cargo.toml`: message-io = { version = "0.14", default-features = false, features = ["tcp", "udp"] } ``` -***Read before update to 0.14**: Version **0.14** modifies the [`connect()`]() behaviour to perform a +***Read before update to 0.14**: Version **0.14** modifies the [`connect()`](https://docs.rs/message-io/latest/message_io/network/struct.NetworkController.html#method.connect) behaviour to perform a [**non**-blocking connections](https://github.com/lemunozm/message-io/issues/61) instead. It is recommended to use this non-blocking mode in order to get the best scalability and performance in your application. If you need to perform -a similar blocking connection as before (version 0.13), you can call to [`connect_sync()`](). +a similar blocking connection as before (version 0.13), you can call to [`connect_sync()`](https://docs.rs/message-io/latest/message_io/network/struct.NetworkController.html#method.connect_sync). Note also that the previous `NetEvent::Connect` has been renamed to `NetEvent::Accepted`. The current `NetEvent::Connect` is a new event to deal with the new non-blocking connections. -See [`NetEvent`]() docs for more info.* +See [`NetEvent`](https://docs.rs/message-io/latest/message_io/network/enum.NetEvent.html) docs for more info.* ### All in one: TCP, UDP and WebSocket echo server The following example is the simplest server that reads messages from the clients and responds @@ -203,8 +203,8 @@ If a transport protocol can be built in top of [`mio`](https://github.com/tokio- 1. Add your *adapter* file in `src/adapters/.rs` that implements the traits that you find [here](https://docs.rs/message-io/latest/message_io/network/adapter/index.html). - It contains only 7 mandatory functions to implement (see the [template](src/adapters/template.rs)), - and it takes little more than 150 lines to implement an adapter. + It contains only 8 mandatory functions to implement (see the [template](src/adapters/template.rs)), + and it takes arround 150 lines to implement an adapter. 1. Add a new field in the `Transport` enum found in [src/network/transport.rs](src/network/transport.rs) to register your new adapter. diff --git a/src/network.rs b/src/network.rs index b78f9bf9..295c096d 100644 --- a/src/network.rs +++ b/src/network.rs @@ -63,6 +63,40 @@ impl NetworkController { /// the connection itself. /// If you want to check if the connection has been established or not you have to read the /// boolean indicator in the [`NetEvent::Connected`] event. + /// + /// Example + /// ``` + /// use message_io::node::{self, NodeEvent}; + /// use message_io::network::{Transport, NetEvent}; + /// + /// let (handler, listener) = node::split(); + /// handler.signals().send_with_timer((), std::time::Duration::from_secs(1)); + /// + /// let (id, addr) = handler.network().listen(Transport::FramedTcp, "127.0.0.1:0").unwrap(); + /// let (conn_endpoint, _) = handler.network().connect(Transport::FramedTcp, addr).unwrap(); + /// // The socket could not be able to send yet. + /// + /// listener.for_each(move |event| match event { + /// NodeEvent::Network(net_event) => match net_event { + /// NetEvent::Connected(endpoint, established) => { + /// assert_eq!(conn_endpoint, endpoint); + /// if established { + /// println!("Connected!"); + /// handler.network().send(endpoint, &[42]); + /// } + /// else { + /// println!("Could not connect"); + /// } + /// }, + /// NetEvent::Accepted(endpoint, listening_id) => { + /// assert_eq!(id, listening_id); + /// println!("New connected endpoint: {}", endpoint.addr()); + /// }, + /// _ => (), + /// } + /// NodeEvent::Signal(_) => handler.stop(), + /// }); + /// ``` pub fn connect( &self, transport: Transport, @@ -86,6 +120,27 @@ impl NetworkController { /// the network callback. /// In order to get the best scalability and performance, use the non-blocking /// [`NetworkController::connect()`] version. + /// + /// Example + /// ``` + /// use message_io::node::{self, NodeEvent}; + /// use message_io::network::{Transport, NetEvent}; + /// + /// let (handler, listener) = node::split(); + /// handler.signals().send_with_timer((), std::time::Duration::from_secs(1)); + /// + /// let (id, addr) = handler.network().listen(Transport::FramedTcp, "127.0.0.1:0").unwrap(); + /// match handler.network().connect_sync(Transport::FramedTcp, addr) { + /// Ok((endpoint, _)) => { + /// println!("Connected!"); + /// handler.network().send(endpoint, &[42]); + /// } + /// Err(err) if err.kind() == std::io::ErrorKind::ConnectionRefused => { + /// println!("Could not connect"); + /// } + /// Err(err) => println!("An OS error creating the socket"), + /// } + /// ``` pub fn connect_sync( &self, transport: Transport, diff --git a/src/node.rs b/src/node.rs index 47ab9137..c0780c64 100644 --- a/src/node.rs +++ b/src/node.rs @@ -286,7 +286,7 @@ impl NodeListener { /// /// let (handler, listener) = node::split(); /// handler.signals().send_with_timer((), std::time::Duration::from_secs(1)); - /// handler.network().listen(Transport::FramedTcp, "0.0.0.0:1234"); + /// let (id, addr) = handler.network().listen(Transport::FramedTcp, "127.0.0.1:0").unwrap(); /// /// listener.for_each(move |event| match event { /// NodeEvent::Network(net_event) => { /* Your logic here */ }, @@ -374,7 +374,7 @@ impl NodeListener { /// /// let (handler, listener) = node::split(); /// handler.signals().send_with_timer((), std::time::Duration::from_secs(1)); - /// handler.network().listen(Transport::FramedTcp, "0.0.0.0:1234"); + /// let (id, addr) = handler.network().listen(Transport::FramedTcp, "127.0.0.1:0").unwrap(); /// /// let task = listener.for_each(move |event| match event { /// NodeEvent::Network(net_event) => { /* Your logic here */ }, @@ -467,7 +467,7 @@ impl NodeListener { /// /// let (handler, listener) = node::split(); /// handler.signals().send_with_timer((), std::time::Duration::from_secs(1)); - /// handler.network().listen(Transport::FramedTcp, "0.0.0.0:1234"); + /// let (id, addr) = handler.network().listen(Transport::FramedTcp, "127.0.0.1:0").unwrap(); /// /// let (task, mut receiver) = listener.enqueue(); /// From 0eda95aeac6835cf68dc4b0e3613c71a628622c5 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Sun, 9 May 2021 10:10:43 +0200 Subject: [PATCH 18/25] Websocket making use of ready_to_write --- src/adapters/ws.rs | 12 ++++++++++++ src/network/adapter.rs | 33 +++++++++++++++++++-------------- src/network/driver.rs | 19 ++++++++++++++++--- 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/adapters/ws.rs b/src/adapters/ws.rs index e6b8b5fd..756783c6 100644 --- a/src/adapters/ws.rs +++ b/src/adapters/ws.rs @@ -223,6 +223,18 @@ impl Remote for RemoteResource { }, } } + + fn ready_to_write(&self) -> bool { + match self.state.lock().expect(OTHER_THREAD_ERR).deref_mut() { + RemoteState::WebSocket(web_socket) => match web_socket.write_pending() { + Ok(_) => true, + Err(Error::Io(ref err)) if err.kind() == ErrorKind::WouldBlock => true, + Err(_) => false, // Will be disconnected, + } + // This function is only call on ready resources. + RemoteState::Handshake(_) => unreachable!(), + } + } } impl RemoteResource { diff --git a/src/network/adapter.rs b/src/network/adapter.rs index 1c4329b9..1bd1f209 100644 --- a/src/network/adapter.rs +++ b/src/network/adapter.rs @@ -143,23 +143,28 @@ pub trait Remote: Resource + Sized { /// The [`SendStatus`] will contain the status of this attempt. fn send(&self, data: &[u8]) -> SendStatus; - /// The resource is available to write. - /// It must be *ready* to receive this call. - /// Here the **implementator** optionally can try to write any pending data. - fn ready_to_write(&self) {} - - /// Called when a `Remote` is created (explicity of by a listener) or when it is not considered - /// read but it has Read/Write readiness. + /// Called when a `Remote` is created (explicity of by a listener) + /// and it is not consider ready yet. + /// A remote resource **is considered ready** when it is totally connected + /// and can be used for writing data. + /// It implies that the user has received the `Connected` or `Accepted` method for that resource. + /// + /// This method is in charge to determine if a resource is ready or not. /// No `Connected` or `Accepted` events will be generated until this function return /// `PendingStatus::Ready`. - /// - /// Implementing this function is optional. - /// The default implementation will consider ready the socket when it was ready to write into it. - /// Yo need to implement it if youtr protocol needs some necessary handshake for consider the - /// resource ready to use. - /// The **implementator** is in charge to write or read from the resource - /// (depending of the readiness) until it obtains an internal [`std::io::ErrorKind::WouldBlock`]. + /// The method wil be called several times with different `Readiness` until the **implementator** + /// returns a `PendingStatus::Ready` or `PendingStatus::Disconnected`. fn pending(&self, readiness: Readiness) -> PendingStatus; + + /// The resource is available to write. + /// It must be *ready* to receive this call. + /// Here the **implementator** optionally can try to write any pending data. + /// The return value is an identification of the operation result. + /// If the method returns `true`, the operation was successful, otherwise, the resource will + /// be disconnected and removed. + fn ready_to_write(&self) -> bool { + true + } } /// Used as a parameter callback in [`Local::accept()`] diff --git a/src/network/driver.rs b/src/network/driver.rs index c5b34378..ff4ee8d1 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -199,13 +199,15 @@ impl> EventProcessor for Driver { log::trace!("Processed remote for {}", endpoint); if !remote.properties.is_ready() { - self.process_pending_remote(&remote, endpoint, readiness, |e| { + self.resolve_pending_remote(&remote, endpoint, readiness, |e| { event_callback(e) }); } if remote.properties.is_ready() { match readiness { - Readiness::Write => remote.resource.ready_to_write(), + Readiness::Write => { + self.write_to_remote(&remote, endpoint, event_callback); + } Readiness::Read => { self.read_from_remote(&remote, endpoint, event_callback); } @@ -227,7 +229,7 @@ impl> EventProcessor for Driver { } impl> Driver { - fn process_pending_remote( + fn resolve_pending_remote( &self, remote: &Arc>, endpoint: Endpoint, @@ -253,6 +255,17 @@ impl> Driver { } } + fn write_to_remote( + &self, + remote: &Arc>, + endpoint: Endpoint, + mut event_callback: impl FnMut(NetEvent<'_>), + ) { + if !remote.resource.ready_to_write() { + event_callback(NetEvent::Disconnected(endpoint)); + } + } + fn read_from_remote( &self, remote: &Arc>, From 288fa8abed7d206b962d0f76f4415c08902b3797 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Mon, 10 May 2021 23:22:58 +0200 Subject: [PATCH 19/25] Revert "Added Resource::source with Option. Forced because tungstenite takes the ownership of the stream and drop it in Failure cases." This reverts commit 34397495ef7ff611dbb79764027ee5049966467c. --- src/adapters/framed_tcp.rs | 8 ++++---- src/adapters/tcp.rs | 8 ++++---- src/adapters/template.rs | 4 ++-- src/adapters/udp.rs | 8 ++++---- src/adapters/ws.rs | 16 ++++++++-------- src/network/adapter.rs | 2 +- src/network/registry.rs | 6 ++---- 7 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/adapters/framed_tcp.rs b/src/adapters/framed_tcp.rs index bbacf5bc..ea411c70 100644 --- a/src/adapters/framed_tcp.rs +++ b/src/adapters/framed_tcp.rs @@ -39,8 +39,8 @@ impl From for RemoteResource { } impl Resource for RemoteResource { - fn source(&mut self) -> Option<&mut dyn Source> { - Some(&mut self.stream) + fn source(&mut self) -> &mut dyn Source { + &mut self.stream } } @@ -122,8 +122,8 @@ pub(crate) struct LocalResource { } impl Resource for LocalResource { - fn source(&mut self) -> Option<&mut dyn Source> { - Some(&mut self.listener) + fn source(&mut self) -> &mut dyn Source { + &mut self.listener } } diff --git a/src/adapters/tcp.rs b/src/adapters/tcp.rs index b4e02325..95e3f28a 100644 --- a/src/adapters/tcp.rs +++ b/src/adapters/tcp.rs @@ -34,8 +34,8 @@ impl From for RemoteResource { } impl Resource for RemoteResource { - fn source(&mut self) -> Option<&mut dyn Source> { - Some(&mut self.stream) + fn source(&mut self) -> &mut dyn Source { + &mut self.stream } } @@ -120,8 +120,8 @@ pub(crate) struct LocalResource { } impl Resource for LocalResource { - fn source(&mut self) -> Option<&mut dyn Source> { - Some(&mut self.listener) + fn source(&mut self) -> &mut dyn Source { + &mut self.listener } } diff --git a/src/adapters/template.rs b/src/adapters/template.rs index 28684210..46815c4a 100644 --- a/src/adapters/template.rs +++ b/src/adapters/template.rs @@ -19,7 +19,7 @@ impl Adapter for MyAdapter { pub(crate) struct RemoteResource; impl Resource for RemoteResource { - fn source(&mut self) -> Option<&mut dyn Source> { + fn source(&mut self) -> &mut dyn Source { todo!() } } @@ -44,7 +44,7 @@ impl Remote for RemoteResource { pub(crate) struct LocalResource; impl Resource for LocalResource { - fn source(&mut self) -> Option<&mut dyn Source> { + fn source(&mut self) -> &mut dyn Source { todo!() } } diff --git a/src/adapters/udp.rs b/src/adapters/udp.rs index be552d17..2618a61a 100644 --- a/src/adapters/udp.rs +++ b/src/adapters/udp.rs @@ -35,8 +35,8 @@ pub(crate) struct RemoteResource { } impl Resource for RemoteResource { - fn source(&mut self) -> Option<&mut dyn Source> { - Some(&mut self.socket) + fn source(&mut self) -> &mut dyn Source { + &mut self.socket } } @@ -85,8 +85,8 @@ pub(crate) struct LocalResource { } impl Resource for LocalResource { - fn source(&mut self) -> Option<&mut dyn Source> { - Some(&mut self.socket) + fn source(&mut self) -> &mut dyn Source { + &mut self.socket } } diff --git a/src/adapters/ws.rs b/src/adapters/ws.rs index 756783c6..e09c9616 100644 --- a/src/adapters/ws.rs +++ b/src/adapters/ws.rs @@ -51,15 +51,15 @@ pub(crate) struct RemoteResource { } impl Resource for RemoteResource { - fn source(&mut self) -> Option<&mut dyn Source> { + fn source(&mut self) -> &mut dyn Source { match self.state.get_mut().unwrap() { - RemoteState::WebSocket(web_socket) => Some(web_socket.get_mut()), + RemoteState::WebSocket(web_socket) => web_socket.get_mut(), RemoteState::Handshake(Some(handshake)) => match handshake { - PendingHandshake::Connect(_, stream) => Some(stream), - PendingHandshake::Client(mid_handshake) => Some(mid_handshake.get_mut().get_mut()), - PendingHandshake::Server(mid_handshake) => Some(mid_handshake.get_mut().get_mut()), + PendingHandshake::Connect(_, stream) => stream, + PendingHandshake::Client(mid_handshake) => mid_handshake.get_mut().get_mut(), + PendingHandshake::Server(mid_handshake) => mid_handshake.get_mut().get_mut(), }, - RemoteState::Handshake(None) => None, + RemoteState::Handshake(None) => unreachable!(), } } } @@ -257,8 +257,8 @@ pub(crate) struct LocalResource { } impl Resource for LocalResource { - fn source(&mut self) -> Option<&mut dyn Source> { - Some(&mut self.listener) + fn source(&mut self) -> &mut dyn Source { + &mut self.listener } } diff --git a/src/network/adapter.rs b/src/network/adapter.rs index 1bd1f209..6f1df409 100644 --- a/src/network/adapter.rs +++ b/src/network/adapter.rs @@ -28,7 +28,7 @@ pub trait Resource: Send + Sync { /// Note: All `mio` network element implements [`Source`], you probably wants to use /// one of them as a base for your non-blocking transport. /// See [`Source`]. - fn source(&mut self) -> Option<&mut dyn Source>; + fn source(&mut self) -> &mut dyn Source; } /// Plain struct used as a returned value of [`Remote::connect()`] diff --git a/src/network/registry.rs b/src/network/registry.rs index eb940f3c..1e3e03ba 100644 --- a/src/network/registry.rs +++ b/src/network/registry.rs @@ -22,9 +22,7 @@ impl Register { impl Drop for Register { fn drop(&mut self) { - if let Some(source) = self.resource.source() { - self.poll_registry.remove(source); - } + self.poll_registry.remove(self.resource.source()); } } @@ -46,7 +44,7 @@ impl ResourceRegistry { // The registry must be locked for the entire implementation to avoid the poll // to generate events over not yet registered resources. let mut registry = self.resources.write().expect(OTHER_THREAD_ERR); - let id = self.poll_registry.add(resource.source().unwrap(), write_readiness); + let id = self.poll_registry.add(resource.source(), write_readiness); let register = Register::new(resource, properties, self.poll_registry.clone()); registry.insert(id, Arc::new(register)); id From d996968ad8b8b5901992556f96bc6c39057b0e0c Mon Sep 17 00:00:00 2001 From: lemunozm Date: Mon, 10 May 2021 18:54:17 +0200 Subject: [PATCH 20/25] Temporal fix for poll leak in tungstenite --- src/adapters/ws.rs | 187 ++++++++++++++++++++++++++++++--------------- 1 file changed, 127 insertions(+), 60 deletions(-) diff --git a/src/adapters/ws.rs b/src/adapters/ws.rs index e09c9616..6397fd1e 100644 --- a/src/adapters/ws.rs +++ b/src/adapters/ws.rs @@ -20,7 +20,7 @@ use tungstenite::error::{Error}; use url::Url; -use std::sync::{Mutex}; +use std::sync::{Mutex, Arc}; use std::net::{SocketAddr}; use std::io::{self, ErrorKind}; use std::ops::{DerefMut}; @@ -36,14 +36,15 @@ impl Adapter for WsAdapter { } enum PendingHandshake { - Connect(Url, TcpStream), - Client(MidHandshake>), - Server(MidHandshake>), + Connect(Url, ArcTcpStream), + Client(MidHandshake>), + Server(MidHandshake>), } enum RemoteState { - WebSocket(WebSocket), + WebSocket(WebSocket), Handshake(Option), + Error(ArcTcpStream), } pub(crate) struct RemoteResource { @@ -53,13 +54,20 @@ pub(crate) struct RemoteResource { impl Resource for RemoteResource { fn source(&mut self) -> &mut dyn Source { match self.state.get_mut().unwrap() { - RemoteState::WebSocket(web_socket) => web_socket.get_mut(), + RemoteState::WebSocket(web_socket) => { + Arc::get_mut(&mut web_socket.get_mut().0).unwrap() + } RemoteState::Handshake(Some(handshake)) => match handshake { - PendingHandshake::Connect(_, stream) => stream, - PendingHandshake::Client(mid_handshake) => mid_handshake.get_mut().get_mut(), - PendingHandshake::Server(mid_handshake) => mid_handshake.get_mut().get_mut(), + PendingHandshake::Connect(_, stream) => Arc::get_mut(&mut stream.0).unwrap(), + PendingHandshake::Client(handshake) => { + Arc::get_mut(&mut handshake.get_mut().get_mut().0).unwrap() + } + PendingHandshake::Server(handshake) => { + Arc::get_mut(&mut handshake.get_mut().get_mut().0).unwrap() + } }, RemoteState::Handshake(None) => unreachable!(), + RemoteState::Error(stream) => Arc::get_mut(&mut stream.0).unwrap(), } } } @@ -89,7 +97,8 @@ impl Remote for RemoteResource { Ok(ConnectionInfo { remote: RemoteResource { state: Mutex::new(RemoteState::Handshake(Some(PendingHandshake::Connect( - url, stream, + url, + stream.into(), )))), }, local_addr, @@ -112,7 +121,7 @@ impl Remote for RemoteResource { // Seems like windows consume the `WouldBlock` notification // at peek() when it happens, and the poll never wakes it again. #[cfg(not(target_os = "windows"))] - let _peek_result = web_socket.get_ref().peek(&mut [0; 0]); + let _peek_result = web_socket.get_ref().0.peek(&mut [0; 0]); // We can not call process_data while the socket is blocked. // The user could lock it again if sends from the callback. @@ -134,6 +143,7 @@ impl Remote for RemoteResource { } }, RemoteState::Handshake(_) => unreachable!(), + RemoteState::Error(_) => unreachable!(), } } } @@ -160,6 +170,7 @@ impl Remote for RemoteResource { } } RemoteState::Handshake(_) => unreachable!(), + RemoteState::Error(_) => unreachable!(), } } @@ -169,58 +180,79 @@ impl Remote for RemoteResource { RemoteState::WebSocket(_) => PendingStatus::Ready, RemoteState::Handshake(pending) => match pending.take().unwrap() { PendingHandshake::Connect(url, stream) => { - match super::tcp::check_stream_ready(&stream) { - PendingStatus::Ready => match ws_connect(url, stream) { - Ok((web_socket, _)) => { - *state = RemoteState::WebSocket(web_socket); - PendingStatus::Ready - } - Err(HandshakeError::Interrupted(mid_handshake)) => { - *pending = Some(PendingHandshake::Client(mid_handshake)); - PendingStatus::Incomplete - } - Err(HandshakeError::Failure(Error::Io(_))) => { - PendingStatus::Disconnected - } - Err(HandshakeError::Failure(err)) => { - log::error!("WS connect handshake error: {}", err); - PendingStatus::Disconnected // should not happen - } - }, - other => other, - } - } - PendingHandshake::Client(mid_handshake) => match mid_handshake.handshake() { - Ok((web_socket, _)) => { - *state = RemoteState::WebSocket(web_socket); - PendingStatus::Ready - } - Err(HandshakeError::Interrupted(mid_handshake)) => { - *pending = Some(PendingHandshake::Client(mid_handshake)); - PendingStatus::Incomplete - } - Err(HandshakeError::Failure(Error::Io(_))) => PendingStatus::Disconnected, - Err(HandshakeError::Failure(err)) => { - log::error!("WS client handshake error: {}", err); - PendingStatus::Disconnected // should not happen + let stream_backup = stream.clone(); + let tcp_status = super::tcp::check_stream_ready(&stream.0); + if tcp_status != PendingStatus::Ready { + // TCP handshake not ready yet. + *pending = Some(PendingHandshake::Connect(url, stream)); + return tcp_status } - }, - PendingHandshake::Server(mid_handshake) => match mid_handshake.handshake() { - Ok(web_socket) => { - *state = RemoteState::WebSocket(web_socket); - PendingStatus::Ready + match ws_connect(url, stream) { + Ok((web_socket, _)) => { + *state = RemoteState::WebSocket(web_socket); + PendingStatus::Ready + } + Err(HandshakeError::Interrupted(mid_handshake)) => { + *pending = Some(PendingHandshake::Client(mid_handshake)); + PendingStatus::Incomplete + } + Err(HandshakeError::Failure(Error::Io(_))) => { + *state = RemoteState::Error(stream_backup); + PendingStatus::Disconnected + } + Err(HandshakeError::Failure(err)) => { + *state = RemoteState::Error(stream_backup); + log::error!("WS connect handshake error: {}", err); + PendingStatus::Disconnected // should not happen + } } - Err(HandshakeError::Interrupted(mid_handshake)) => { - *pending = Some(PendingHandshake::Server(mid_handshake)); - PendingStatus::Incomplete + } + PendingHandshake::Client(mid_handshake) => { + let stream_backup = mid_handshake.get_ref().get_ref().clone(); + match mid_handshake.handshake() { + Ok((web_socket, _)) => { + *state = RemoteState::WebSocket(web_socket); + PendingStatus::Ready + } + Err(HandshakeError::Interrupted(mid_handshake)) => { + *pending = Some(PendingHandshake::Client(mid_handshake)); + PendingStatus::Incomplete + } + Err(HandshakeError::Failure(Error::Io(_))) => { + *state = RemoteState::Error(stream_backup); + PendingStatus::Disconnected + } + Err(HandshakeError::Failure(err)) => { + *state = RemoteState::Error(stream_backup); + log::error!("WS client handshake error: {}", err); + PendingStatus::Disconnected // should not happen + } } - Err(HandshakeError::Failure(Error::Io(_))) => PendingStatus::Disconnected, - Err(HandshakeError::Failure(err)) => { - log::error!("WS server handshake error: {}", err); - PendingStatus::Disconnected // should not happen + } + PendingHandshake::Server(mid_handshake) => { + let stream_backup = mid_handshake.get_ref().get_ref().clone(); + match mid_handshake.handshake() { + Ok(web_socket) => { + *state = RemoteState::WebSocket(web_socket); + PendingStatus::Ready + } + Err(HandshakeError::Interrupted(mid_handshake)) => { + *pending = Some(PendingHandshake::Server(mid_handshake)); + PendingStatus::Incomplete + } + Err(HandshakeError::Failure(Error::Io(_))) => { + *state = RemoteState::Error(stream_backup); + PendingStatus::Disconnected + } + Err(HandshakeError::Failure(err)) => { + *state = RemoteState::Error(stream_backup); + log::error!("WS server handshake error: {}", err); + PendingStatus::Disconnected // should not happen + } } - }, + } }, + RemoteState::Error(_) => unreachable!(), } } @@ -230,9 +262,10 @@ impl Remote for RemoteResource { Ok(_) => true, Err(Error::Io(ref err)) if err.kind() == ErrorKind::WouldBlock => true, Err(_) => false, // Will be disconnected, - } + }, // This function is only call on ready resources. RemoteState::Handshake(_) => unreachable!(), + RemoteState::Error(_) => unreachable!(), } } } @@ -275,7 +308,7 @@ impl Local for LocalResource { loop { match self.listener.accept() { Ok((stream, addr)) => { - let remote_state = match ws_accept(stream) { + let remote_state = match ws_accept(stream.into()) { Ok(web_socket) => Some(RemoteState::WebSocket(web_socket)), Err(HandshakeError::Interrupted(mid_handshake)) => Some( RemoteState::Handshake(Some(PendingHandshake::Server(mid_handshake))), @@ -298,3 +331,37 @@ impl Local for LocalResource { } } } + +/// This struct is used to avoid the tungstenite handshake to take the ownership of the stream +/// an drop it without allow to the driver to deregister from the poll. +/// It can be removed when this issue is resolved: +/// https://github.com/snapview/tungstenite-rs/issues/51 +struct ArcTcpStream(Arc); + +impl From for ArcTcpStream { + fn from(stream: TcpStream) -> Self { + Self(Arc::new(stream)) + } +} + +impl io::Read for ArcTcpStream { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + (&*self.0).read(buf) + } +} + +impl io::Write for ArcTcpStream { + fn write(&mut self, buf: &[u8]) -> io::Result { + (&*self.0).write(buf) + } + + fn flush(&mut self) -> io::Result<()> { + (&*self.0).flush() + } +} + +impl Clone for ArcTcpStream { + fn clone(&self) -> Self { + Self(self.0.clone()) + } +} From cf528f28f3d7a3a89a1ba7eb7deea91ed02b8ceb Mon Sep 17 00:00:00 2001 From: lemunozm Date: Mon, 10 May 2021 20:06:48 +0200 Subject: [PATCH 21/25] Fixed ws accepted issue --- src/adapters/ws.rs | 48 +++++++++++++++++++++++++++++-------------- src/network.rs | 2 +- src/network/driver.rs | 4 +++- tests/integration.rs | 2 +- 4 files changed, 38 insertions(+), 18 deletions(-) diff --git a/src/adapters/ws.rs b/src/adapters/ws.rs index 6397fd1e..30441ce3 100644 --- a/src/adapters/ws.rs +++ b/src/adapters/ws.rs @@ -37,6 +37,7 @@ impl Adapter for WsAdapter { enum PendingHandshake { Connect(Url, ArcTcpStream), + Accept(ArcTcpStream), Client(MidHandshake>), Server(MidHandshake>), } @@ -59,6 +60,7 @@ impl Resource for RemoteResource { } RemoteState::Handshake(Some(handshake)) => match handshake { PendingHandshake::Connect(_, stream) => Arc::get_mut(&mut stream.0).unwrap(), + PendingHandshake::Accept(stream) => Arc::get_mut(&mut stream.0).unwrap(), PendingHandshake::Client(handshake) => { Arc::get_mut(&mut handshake.get_mut().get_mut().0).unwrap() } @@ -180,13 +182,13 @@ impl Remote for RemoteResource { RemoteState::WebSocket(_) => PendingStatus::Ready, RemoteState::Handshake(pending) => match pending.take().unwrap() { PendingHandshake::Connect(url, stream) => { - let stream_backup = stream.clone(); let tcp_status = super::tcp::check_stream_ready(&stream.0); if tcp_status != PendingStatus::Ready { // TCP handshake not ready yet. *pending = Some(PendingHandshake::Connect(url, stream)); return tcp_status } + let stream_backup = stream.clone(); match ws_connect(url, stream) { Ok((web_socket, _)) => { *state = RemoteState::WebSocket(web_socket); @@ -207,6 +209,28 @@ impl Remote for RemoteResource { } } } + PendingHandshake::Accept(stream) => { + let stream_backup = stream.clone(); + match ws_accept(stream.into()) { + Ok(web_socket) => { + *state = RemoteState::WebSocket(web_socket); + PendingStatus::Ready + } + Err(HandshakeError::Interrupted(mid_handshake)) => { + *pending = Some(PendingHandshake::Server(mid_handshake)); + PendingStatus::Incomplete + } + Err(HandshakeError::Failure(Error::Io(_))) => { + *state = RemoteState::Error(stream_backup); + PendingStatus::Disconnected + } + Err(HandshakeError::Failure(err)) => { + *state = RemoteState::Error(stream_backup); + log::error!("WS accept handshake error: {}", err); + PendingStatus::Disconnected + } + } + } PendingHandshake::Client(mid_handshake) => { let stream_backup = mid_handshake.get_ref().get_ref().clone(); match mid_handshake.handshake() { @@ -257,6 +281,8 @@ impl Remote for RemoteResource { } fn ready_to_write(&self) -> bool { + true + /* Is this needed? match self.state.lock().expect(OTHER_THREAD_ERR).deref_mut() { RemoteState::WebSocket(web_socket) => match web_socket.write_pending() { Ok(_) => true, @@ -267,6 +293,7 @@ impl Remote for RemoteResource { RemoteState::Handshake(_) => unreachable!(), RemoteState::Error(_) => unreachable!(), } + */ } } @@ -308,21 +335,12 @@ impl Local for LocalResource { loop { match self.listener.accept() { Ok((stream, addr)) => { - let remote_state = match ws_accept(stream.into()) { - Ok(web_socket) => Some(RemoteState::WebSocket(web_socket)), - Err(HandshakeError::Interrupted(mid_handshake)) => Some( - RemoteState::Handshake(Some(PendingHandshake::Server(mid_handshake))), - ), - Err(HandshakeError::Failure(ref err)) => { - log::error!("WS accept handshake error: {}", err); - None - } + let remote = RemoteResource { + state: Mutex::new(RemoteState::Handshake(Some(PendingHandshake::Accept( + stream.into(), + )))), }; - - if let Some(remote_state) = remote_state { - let remote = RemoteResource { state: Mutex::new(remote_state) }; - accept_remote(AcceptedType::Remote(addr, remote)); - } + accept_remote(AcceptedType::Remote(addr, remote)); } Err(ref err) if err.kind() == ErrorKind::WouldBlock => break, Err(ref err) if err.kind() == ErrorKind::Interrupted => continue, diff --git a/src/network.rs b/src/network.rs index 295c096d..8315d6cb 100644 --- a/src/network.rs +++ b/src/network.rs @@ -191,7 +191,7 @@ impl NetworkController { /// you will want to process a [`NetEvent::Disconnected`] to determine if the connection + /// is *alive* instead of check if `send()` returned [`SendStatus::ResourceNotFound`]. pub fn send(&self, endpoint: Endpoint, data: &[u8]) -> SendStatus { - log::trace!("Send {} bytes to {}", data.len(), endpoint); + log::trace!("Sending {} bytes to {}...", data.len(), endpoint); let status = self.controllers[endpoint.resource_id().adapter_id() as usize].send(endpoint, data); log::trace!("Send status: {:?}", status); diff --git a/src/network/driver.rs b/src/network/driver.rs index ff4ee8d1..4362108c 100644 --- a/src/network/driver.rs +++ b/src/network/driver.rs @@ -236,7 +236,9 @@ impl> Driver { readiness: Readiness, mut event_callback: impl FnMut(NetEvent<'_>), ) { - match remote.resource.pending(readiness) { + let status = remote.resource.pending(readiness); + log::trace!("Resolve pending for {}: {:?}", endpoint, status); + match status { PendingStatus::Ready => { remote.properties.mark_as_ready(); match remote.properties.local { diff --git a/tests/integration.rs b/tests/integration.rs index bc565397..f4ec9b12 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -281,7 +281,7 @@ fn burst(transport: Transport, messages_count: usize) { #[cfg_attr(feature = "udp", test_case(Transport::Udp, udp::MAX_COMPATIBLE_PAYLOAD_LEN))] #[cfg_attr(feature = "websocket", test_case(Transport::Ws, BIG_MESSAGE_SIZE))] fn message_size(transport: Transport, message_size: usize) { - //util::init_logger(LogThread::Enabled); // Enable it for better debugging + //util::init_logger(LogThread::Disabled); // Enable it for better debugging assert!(message_size <= transport.max_message_size()); From 1ab895f2595bb0687661ddd980a71a43d9d03480 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Mon, 10 May 2021 20:13:01 +0200 Subject: [PATCH 22/25] Minor changelog change --- CHANGELOG.md | 4 ++-- src/adapters/ws.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 335ab845..1b17cd8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,10 @@ ## Release 0.14.0 - Asynchronous connections: `NetworkController::connect()` behaviour modified. -Now it performs a non-blocking connection. +Now it performs a non-blocking connection. Previous behaviour with `connect_sync` version. - Reduced slightly the websocket latency. - Adapter API modified to handle easily handshakes. -- Fixed websocket issue that could offer an accepted websocket that was not valid (the websocket handshake could have failed). +- Fixed websocket issue that could offer an accepted connection that was yet not valid. - Added `NetworkController::is_ready()` - Added `SendStatus::ResourceNotAvailable` diff --git a/src/adapters/ws.rs b/src/adapters/ws.rs index 30441ce3..5296a587 100644 --- a/src/adapters/ws.rs +++ b/src/adapters/ws.rs @@ -211,7 +211,7 @@ impl Remote for RemoteResource { } PendingHandshake::Accept(stream) => { let stream_backup = stream.clone(); - match ws_accept(stream.into()) { + match ws_accept(stream) { Ok(web_socket) => { *state = RemoteState::WebSocket(web_socket); PendingStatus::Ready From a577f38fe180052f4c8a13bdce747c18b2772f1b Mon Sep 17 00:00:00 2001 From: lemunozm Date: Tue, 11 May 2021 20:22:15 +0200 Subject: [PATCH 23/25] Added StoredNetEvent::borrow() --- CHANGELOG.md | 1 + src/node.rs | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b17cd8e..4f07e815 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Now it performs a non-blocking connection. Previous behaviour with `connect_sync - Fixed websocket issue that could offer an accepted connection that was yet not valid. - Added `NetworkController::is_ready()` - Added `SendStatus::ResourceNotAvailable` +- Added `borrow()` method for `StoredNetEvent` to transform in into `NetEvent`. ## Release 0.13.3 - Fixed a bad internal assert. diff --git a/src/node.rs b/src/node.rs index c0780c64..dc0c277c 100644 --- a/src/node.rs +++ b/src/node.rs @@ -130,7 +130,7 @@ impl From> for StoredNetEvent { impl StoredNetEvent { /// Use this `StoredNetEvent` as a `NetEvent` referencing its data. - fn borrow(&self) -> NetEvent<'_> { + pub fn borrow(&self) -> NetEvent<'_> { match self { Self::Connected(endpoint, status) => NetEvent::Connected(*endpoint, *status), Self::Accepted(endpoint, id) => NetEvent::Accepted(*endpoint, *id), @@ -533,8 +533,6 @@ mod tests { let inner_handler = handler.clone(); listener.for_each(move |_| inner_handler.stop()); - // Since here `NodeTask` is already dropped just after listener call, - // the node is considered not running. assert!(!handler.is_running()); } From c6bd25e803211f8a0e1299b9a3edd9d8c7853804 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Wed, 12 May 2021 21:19:43 +0200 Subject: [PATCH 24/25] Added is_local() and is_remote() --- CHANGELOG.md | 1 + src/network/resource_id.rs | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f07e815..cd1b79f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Now it performs a non-blocking connection. Previous behaviour with `connect_sync - Added `NetworkController::is_ready()` - Added `SendStatus::ResourceNotAvailable` - Added `borrow()` method for `StoredNetEvent` to transform in into `NetEvent`. +- Added `is_local()` and `is_remote()` helpers to `ResourceId`. ## Release 0.13.3 - Fixed a bad internal assert. diff --git a/src/network/resource_id.rs b/src/network/resource_id.rs index 6e25ba73..c2859b63 100644 --- a/src/network/resource_id.rs +++ b/src/network/resource_id.rs @@ -71,6 +71,16 @@ impl ResourceId { } } + /// Tells if the id preresents a local resource. + pub fn is_local(&self) -> bool { + self.resource_type() == ResourceType::Local + } + + /// Tells if the id preresents a remote resource. + pub fn is_remote(&self) -> bool { + self.resource_type() == ResourceType::Remote + } + /// Returns the associated adapter id. /// Note that this returned value is the same as the value of [`crate::network::Transport::id()`] /// if that transport uses the same adapter. From f8be160a98e6cc40863cb1ad6a858c0584b836fc Mon Sep 17 00:00:00 2001 From: lemunozm Date: Sun, 16 May 2021 11:22:52 +0200 Subject: [PATCH 25/25] Added minor comment --- README.md | 4 ++-- src/adapters/tcp.rs | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index c5cf2a15..c807df78 100644 --- a/README.md +++ b/README.md @@ -90,14 +90,14 @@ For example, in order to include only *TCP* and *UDP*, add to your `Cargo.toml`: message-io = { version = "0.14", default-features = false, features = ["tcp", "udp"] } ``` -***Read before update to 0.14**: Version **0.14** modifies the [`connect()`](https://docs.rs/message-io/latest/message_io/network/struct.NetworkController.html#method.connect) behaviour to perform a +_**Read before update to 0.14**: Version **0.14** modifies the [`connect()`](https://docs.rs/message-io/latest/message_io/network/struct.NetworkController.html#method.connect) behaviour to perform a [**non**-blocking connections](https://github.com/lemunozm/message-io/issues/61) instead. It is recommended to use this non-blocking mode in order to get the best scalability and performance in your application. If you need to perform a similar blocking connection as before (version 0.13), you can call to [`connect_sync()`](https://docs.rs/message-io/latest/message_io/network/struct.NetworkController.html#method.connect_sync). Note also that the previous `NetEvent::Connect` has been renamed to `NetEvent::Accepted`. The current `NetEvent::Connect` is a new event to deal with the new non-blocking connections. -See [`NetEvent`](https://docs.rs/message-io/latest/message_io/network/enum.NetEvent.html) docs for more info.* +See [`NetEvent`](https://docs.rs/message-io/latest/message_io/network/enum.NetEvent.html) docs for more info._ ### All in one: TCP, UDP and WebSocket echo server The following example is the simplest server that reads messages from the clients and responds diff --git a/src/adapters/tcp.rs b/src/adapters/tcp.rs index 95e3f28a..d77e22e6 100644 --- a/src/adapters/tcp.rs +++ b/src/adapters/tcp.rs @@ -103,7 +103,10 @@ impl Remote for RemoteResource { } } +/// Check if a TcpStream can be considered connected. pub fn check_stream_ready(stream: &TcpStream) -> PendingStatus { + // A multiplatform non-blocking way to determine if the TCP stream is connected: + // Extracted from: https://github.com/tokio-rs/mio/issues/1486 if let Ok(Some(_)) = stream.take_error() { return PendingStatus::Disconnected }