On TCP, if there is no response to a request, the connection hangs forever

[JosuGZ]

If for some reason a device fails to respond, or the connection is lost without being properly closed, the TCP client hangs forever.

This could be solved with a timeout, but I can't find a way to set one.

[JosuGZ]

Something like this could be a solution, making the timeout configurable somewhere and maybe allowing to close the connection (this "fix" keeps it open).

I don't know the library enough to make further changes at this moment.

diff --git a/src/service/tcp.rs b/src/service/tcp.rs
index 6dcdbf2..d359e8a 100644
--- a/src/service/tcp.rs
+++ b/src/service/tcp.rs
@@ -82,15 +82,19 @@ impl Context {
         let req_hdr = req_adu.hdr;
 
         self.service.send(req_adu).await?;
-        let res_adu = self
-            .service
-            .next()
-            .await
-            .ok_or_else(Error::last_os_error)??;
-
-        match res_adu.pdu {
-            ResponsePdu(Ok(res)) => verify_response_header(req_hdr, res_adu.hdr).and(Ok(res)),
-            ResponsePdu(Err(err)) => Err(Error::new(ErrorKind::Other, err)),
+        let next_response_future = self.service.next();
+        let timeout = tokio::time::Duration::from_secs(4);
+        let next_response_or_timed_out = tokio::time::timeout(timeout, next_response_future).await;
+
+        if let Ok(next_response) = next_response_or_timed_out {
+            let res_adu = next_response.ok_or_else(Error::last_os_error)??;
+
+            match res_adu.pdu {
+                ResponsePdu(Ok(res)) => verify_response_header(req_hdr, res_adu.hdr).and(Ok(res)),
+                ResponsePdu(Err(err)) => Err(Error::new(ErrorKind::Other, err)),
+            }
+        } else {
+            Err(Error::new(ErrorKind::TimedOut, "Timeout"))
         }
     }
 }

[ColinFinck]

@JosuGZ Repeating what I've written in #60 (comment):
This is not a problem specific to tokio-modbus, but to any crate that communicates with external devices. I have yet to find a crate, which provides timeouts for every step of the communication in order to reliably prevent hangs.

I have found it way more scalable to shift this responsibility to the crate user: My code only calls two functions of tokio-modbus, connect and read_holding_registers. I have reasonable expectations about the duration of these function calls. Which is why I can simply put tokio::time::timeout around both calls and be done with it. This guarantees that the call returns after the given timeout, no matter what happens inside the crate.

It's ultimately up to the tokio-modbus team to decide what to do here. But I have even put tokio::time::timeout around crates that claimed to provide timeouts for their communication. Better be safe than sorry!