From dd72b2e4bddc8759da8fc50c9a7e1a1809a43eca Mon Sep 17 00:00:00 2001 From: Jake Smith Date: Mon, 8 Jul 2024 19:27:52 +0100 Subject: [PATCH] fix 2 issues 1) readtmsCheckGC was incorrectly asking readtms to ignore GC. The intent was to pass back as a result whether the call GC'd or not, so the caller could choose how to handle it. It led to dafilesrv not closing the socket after a GC in this situation, causing a spurious follow-on error on a notifySelect. 2) UDP reading should not have tried to be reading more data if it had already received something. This was not a new issue in itself, but the semantics changed to keep reading beyond min_size until blocked. It makes no sense to read if have already received >0 from recvfrom when dealing with UDP. Altered the logic and added comments re. semantics. --- fs/dafsserver/dafsserver.cpp | 2 +- system/jlib/jsocket.cpp | 14 ++++++++++++-- system/jlib/jsocket.hpp | 5 +++-- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/fs/dafsserver/dafsserver.cpp b/fs/dafsserver/dafsserver.cpp index fe34df008db..655325b90f2 100644 --- a/fs/dafsserver/dafsserver.cpp +++ b/fs/dafsserver/dafsserver.cpp @@ -2928,7 +2928,7 @@ class CRemoteFileServer : implements IRemoteFileServer, public CInterface { try { - sock->read(dst, minSize, maxSize, szRead, WAIT_FOREVER, true); + sock->read(dst, minSize, maxSize, szRead, WAIT_FOREVER, false); } catch (IJSOCK_Exception *e) { diff --git a/system/jlib/jsocket.cpp b/system/jlib/jsocket.cpp index 6263353978a..816f69d725e 100644 --- a/system/jlib/jsocket.cpp +++ b/system/jlib/jsocket.cpp @@ -170,6 +170,7 @@ class jlib_thrown_decl SocketException: public IJSOCK_Exception, public CInterfa case JSOCKERR_handle_too_large: return str.append("handle too large"); case JSOCKERR_bad_netaddr: return str.append("bad net addr"); case JSOCKERR_ipv6_not_implemented: return str.append("IPv6 not implemented"); + case JSOCKERR_small_udp_packet: return str.append("small UDP packet"); // OS errors #ifdef _WIN32 case WSAEINTR: return str.append("WSAEINTR(10004) - Interrupted system call."); @@ -1918,6 +1919,8 @@ void CSocket::readtms(void* buf, size32_t min_size, size32_t max_size, size32_t * NB: If min_size==0 then will return asap if no data is avail. * NB: If min_size==0, but notified of graceful close, throw graceful close exception. * NB: timeout is meaningless if min_size is 0 + * + * NB: for UDP, it will never try to read more. It will call recvfrom once, if it gets less than min_size, it will throw an exception. */ sizeRead = 0; @@ -1946,7 +1949,13 @@ void CSocket::readtms(void* buf, size32_t min_size, size32_t max_size, size32_t if (rc > 0) { sizeRead += rc; - if (sizeRead == max_size) + if (sockmode==sm_udp_server) + { + if (sizeRead >= min_size) + break; + throwJSockException(JSOCKERR_small_udp_packet, "readtms: UDP packet smaller than min_size", __FILE__, __LINE__); // else, it is an error in UDP if receive anything less than min_size + } + else if (sizeRead == max_size) break; // NB: will exit when blocked if sizeRead >= min_size } @@ -1973,9 +1982,10 @@ void CSocket::readtms(void* buf, size32_t min_size, size32_t max_size, size32_t } else { - // NB: can only be here if sizeRead < min_size OR min_size = 0 if (err == JSE_WOULDBLOCK || err == EAGAIN) // if EGAIN or EWOULDBLOCK - no more data to read { + //NB: in UDP can only reach here if have not read anything so far. + if (sizeRead >= min_size) break; // otherwise, continue waiting for min_size diff --git a/system/jlib/jsocket.hpp b/system/jlib/jsocket.hpp index bab8bb7866e..820ca668569 100644 --- a/system/jlib/jsocket.hpp +++ b/system/jlib/jsocket.hpp @@ -56,9 +56,10 @@ enum JSOCKET_ERROR_CODES { JSOCKERR_cancel_accept = -8, // accept JSOCKERR_connectionless_socket = -9, // accept, cancel_accept JSOCKERR_graceful_close = -10, // read,send - JSOCKERR_handle_too_large = -11, // select, connect etc (linux only) + JSOCKERR_handle_too_large = -11, // select, connect etc (linux only) JSOCKERR_bad_netaddr = -12, // get/set net address - JSOCKERR_ipv6_not_implemented = -13 // various + JSOCKERR_ipv6_not_implemented = -13, // various + JSOCKERR_small_udp_packet = -14 // small udp packet }; // Block operation flags