The issue we have is that the function (networkDataAwailable) treats errors like regular timeouts, making it hard to debug the issue in the first place as it looks like the request timed out.
I think it should at least log an error in such a failing case or ideally retry the select call. Thus the code:
Code: Select all
OFBool DcmTCPConnection::networkDataAvailable(int timeout)
{
struct timeval t;
fd_set fdset;
int nfound;
FD_ZERO(&fdset);
#ifdef __MINGW32__
/* on MinGW, FD_SET expects an unsigned first argument */
FD_SET((unsigned int) getSocket(), &fdset);
#else
FD_SET(getSocket(), &fdset);
#endif
t.tv_sec = timeout;
t.tv_usec = 0;
#ifdef HAVE_INTP_SELECT
nfound = select(getSocket() + 1, (int *)(&fdset), NULL, NULL, &t);
#else
nfound = select(getSocket() + 1, &fdset, NULL, NULL, &t);
#endif
if (nfound <= 0) return OFFalse;
else
{
if (FD_ISSET(getSocket(), &fdset)) return OFTrue;
else return OFFalse; /* This should not really happen */
}
}
Code: Select all
OFBool DcmTCPConnection::networkDataAvailable(int timeout)
{
OFTimer timer;
int lTimeout = timeout;
fd_set fdset;
int nfound;
FD_ZERO(&fdset);
#ifdef __MINGW32__
/* on MinGW, FD_SET expects an unsigned first argument */
FD_SET((unsigned int) getSocket(), &fdset);
#else
FD_SET(getSocket(), &fdset);
#endif
while(1) {
// timeval can be undefined after the call to select http://man7.org/linux/man-pages/man2/select.2.html
struct timeval t;
t.tv_sec = lTimeout;
t.tv_usec = 0;
#ifdef HAVE_INTP_SELECT
nfound = select(getSocket() + 1, (int *)(&fdset), NULL, NULL, &t);
#else
nfound = select(getSocket() + 1, &fdset, NULL, NULL, &t);
#endif
if (nfound < 0) {
// check for interrupt call
if (errno == EINTR) {
int diff = (int)timer.getDiff();
if (diff < timeout) {
lTimeout = timeout - diff;
continue; // retry
}
}
else {
char buf[256];
DCMNET_ERROR("socket select returned with error: "
<< OFStandard::strerror(errno, buf, sizeof(buf)) );
return OFFalse;
}
}
if (nfound == 0) {
return OFFalse; // a regular timeout
}
else {
if (FD_ISSET(getSocket(), &fdset)) return OFTrue;
else return OFFalse; /* This should not really happen */
}
}
return OFFalse;
}