From 8a797764c1532dbeed8f665cc15b56246ff2beff Mon Sep 17 00:00:00 2001 From: bunnie Date: Thu, 4 Aug 2022 21:10:17 +0800 Subject: [PATCH] (possibly dubious) fix on 'unattended' Stream close If a Stream is opened and a process isn't constantly polling it, it'll never close if the remote side goes away -- you'll just end up hanging in a FIN-WAIT-2 condition. Add some logic to detect this and close out streams automatically where the remote has hung up. I'm not actually sure this is what you're *supposed* to do, but, lacking a formal specification, all I can do is try to get this thing to pass the test bench. And pass it does. --- services/net/src/main.rs | 41 ++++++++++++++++++++++++++--- services/net/src/std_tcplistener.rs | 4 ++- services/net/src/std_tcpstream.rs | 1 + 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/services/net/src/main.rs b/services/net/src/main.rs index 6f1367062..7deb704f4 100644 --- a/services/net/src/main.rs +++ b/services/net/src/main.rs @@ -246,6 +246,9 @@ fn main() -> ! { // socket handles waiting to enter the closed state let mut tcp_tx_wait_fin: Vec<(SocketHandle, xous::MessageSender)> = Vec::new(); + // socket handles corresponding to servers that could be closed by clients + let mut tcp_server_remote_close_poll: Vec = Vec::new(); + // When a client issues a Connect request, it will get placed here while the connection is // being established. let mut tcp_connect_waiting: Vec< @@ -819,6 +822,7 @@ fn main() -> ! { msg, &mut iface, &mut tcp_accept_waiting, + &mut tcp_server_remote_close_poll, process_sockets.entry(pid).or_default(), ); xous::try_send_message( @@ -1317,7 +1321,9 @@ fn main() -> ! { } else { continue; } - } else if socket.state() == smoltcp::socket::TcpState::CloseWait { + } else if socket.state() == smoltcp::socket::TcpState::CloseWait + // this state added to handle the auto-close edge case on a remote hang-up + || socket.state() == smoltcp::socket::TcpState::Closed { // stop waiting if we're in CloseWait, as we don't plan to transmit } else { continue; @@ -1330,7 +1336,9 @@ fn main() -> ! { // If it can't receive, then the only explanation was that it timed out if !socket.can_recv() { - if socket.state() == smoltcp::socket::TcpState::CloseWait { + if socket.state() == smoltcp::socket::TcpState::CloseWait + // this state added to handle the auto-close edge case on a remote hang-up + || socket.state() == smoltcp::socket::TcpState::Closed { log::debug!("rxrcv connection closed"); let body = env.body.memory_message_mut().unwrap(); log::debug!("rxrcv of {}", 0); @@ -1372,7 +1380,10 @@ fn main() -> ! { &mut None => continue, Some(s) => { socket = iface.get_socket::(s.handle); - if !socket.can_send() { + log::debug!("tx_state: {:?} {:?}", socket.state(), socket.local_endpoint()); + if socket.state() == smoltcp::socket::TcpState::Closed { + // stop waiting if the stocket just closed on us outright + } else if !socket.can_send() { if let Some(trigger) = s.expiry { if trigger.get() < now { // timer expired @@ -1388,7 +1399,7 @@ fn main() -> ! { connection.take().unwrap() }; - if !socket.can_send() { + if !socket.can_send() || socket.state() == smoltcp::socket::TcpState::Closed { respond_with_error(env, NetError::TimedOut); continue; } @@ -1437,6 +1448,7 @@ fn main() -> ! { Some(s) => { let socket = iface.get_socket::(s.handle); if socket.is_active() { + tcp_server_remote_close_poll.push(s.handle); ep = socket.remote_endpoint(); connection.take().unwrap() } else { @@ -1546,6 +1558,9 @@ fn main() -> ! { if !socket.is_open() { log::debug!("socket closed {:?}", socket.local_endpoint()); iface.remove_socket(*handle); + tcp_server_remote_close_poll.retain(|x| { + *x != *handle + }); xous::return_scalar(*sender, 0).ok(); false } else { @@ -1554,6 +1569,24 @@ fn main() -> ! { } }); + tcp_server_remote_close_poll.retain(|handle| { + let socket = iface.get_socket::(*handle); + log::debug!("remote close poll: state {:?} local {:?}", socket.state(), socket.local_endpoint()); + if socket.state() == smoltcp::socket::TcpState::CloseWait { + // initiate the closing ack, but allow the explicit drop to remove the socket once the handle is finished + // this handles the special case that a stream was accepted, but then the client hangs up + // and the server isn't actively polling the loop. By pushing the socket to the "close" + // state, we allow the three-way close handshake to actually complete instead of hanging + // in a FIN-WAIT-2 state. + socket.close(); + } + if !socket.is_open() { + false + } else { + true + } + }); + // this block contains the ICMP Rx handler. Tx is initiated by an incoming message to the Net crate. log::trace!("pump: icmp"); { diff --git a/services/net/src/std_tcplistener.rs b/services/net/src/std_tcplistener.rs index a9e5723b6..5ed377a6d 100644 --- a/services/net/src/std_tcplistener.rs +++ b/services/net/src/std_tcplistener.rs @@ -67,6 +67,7 @@ pub(crate) fn std_tcp_accept( mut msg: xous::MessageEnvelope, iface: &mut Interface::, tcp_accept_waiting: &mut Vec>, + tcp_server_remote_close_poll: &mut Vec, our_sockets: &Vec>, ) { let fd = (msg.body.id() >> 16) & 0xffff; @@ -97,6 +98,7 @@ pub(crate) fn std_tcp_accept( if socket.is_active() { log::debug!("accept did not block; immediately returning TcpSocket"); let buf = body.buf.as_slice_mut::(); + tcp_server_remote_close_poll.push(*handle); tcp_accept_success(buf, fd as u16, socket.remote_endpoint()); return; } @@ -121,7 +123,7 @@ pub(crate) fn std_tcp_accept( } pub(crate) fn tcp_accept_success(buf: &mut [u8], fd: u16, ep: IpEndpoint) { - log::debug!("tcp accept: {:?}", ep); + log::debug!("tcp accept: remote {:?}", ep); buf[0] = 0; let fd_arr = fd.to_le_bytes(); buf[1] = fd_arr[0]; diff --git a/services/net/src/std_tcpstream.rs b/services/net/src/std_tcpstream.rs index 9396472df..f95fb9d33 100644 --- a/services/net/src/std_tcpstream.rs +++ b/services/net/src/std_tcpstream.rs @@ -99,6 +99,7 @@ pub(crate) fn std_tcp_tx( }; let socket = iface.get_socket::(*handle); + // handle the case that the connection closed due to the receiver quitting if !socket.can_send() { log::trace!("tx can't send, will retry"); let expiry = body