Skip to content

Commit

Permalink
fix 2 issues
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jakesmith committed Jul 9, 2024
1 parent 2b0e9a9 commit dd72b2e
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 5 deletions.
2 changes: 1 addition & 1 deletion fs/dafsserver/dafsserver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
14 changes: 12 additions & 2 deletions system/jlib/jsocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down
5 changes: 3 additions & 2 deletions system/jlib/jsocket.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit dd72b2e

Please sign in to comment.