defragmentTCP() does not handle WSAEWOULDBLOCK error on Windows when no data ready from non-blocking socket for read()

All other questions regarding DCMTK

Moderator: Moderator Team

Post Reply
Message
Author
wrenashe
Posts: 20
Joined: Tue, 2013-10-08, 11:24

defragmentTCP() does not handle WSAEWOULDBLOCK error on Windows when no data ready from non-blocking socket for read()

#1 Post by wrenashe »

We (Bonnie found the issue) have a DICOM message relay on Windows to test,
storescu --> our DICOM router(SCP --> SCU, using DCMTK3.6.6) --> external SCP,

We got the error shown below on Windows, We do not have the same problem on Linux with our DICOM router built from the same DCMTK.
C:\testbin>storescu -aec xxxPACS4 -aet CL_SCP LOCALHOST 11110 C:\testbin\data\*
E: Store Failed, file: C:\testbin\data\CT.1.3.6.1.4.1.-----------------------.1-------.12.0:
E: 0006:020e DIMSE Failed to send message
E: 0006:031d TCP I/O Error (An established connection was aborted by the software in your host machine.) occurred in routine: writeDataPDU
E: Store SCU Failed: 0006:020e DIMSE Failed to send message
E: 0006:031d TCP I/O Error (An established connection was aborted by the software in your host machine.) occurred in routine: writeDataPDU
E: Association Abort Failed: 0006:031d TCP I/O Error (An established connection was aborted by the software in your host machine.) occurred in routine: sendAbortTCP

The investigation shows the DICOM router SCP side got an issue about "DIMSE Read PDV failed". In defragmentTCP(), read() in non-blocking mode fails when the socket is not ready to be read. The code is not aware of the situation like bytesRead = -1 and WSAEWOULDBLOCK as the error, just returns DUL_NETWORKCLOSED and results in the failure.

The modified code diff (also from Bonnie here) fixes this problem on Windows against our testing scenario, just for your reference here, hope it helps.

Code: Select all

3699a3700,3704
> #ifdef HAVE_WINSOCK_H
> 			if (OFStandard::getLastNetworkErrorCode().value() == WSAEWOULDBLOCK) {
> 				Sleep(1);
> 			}
> #endif
3713c3718,3722
<         } while (bytesRead == -1 && OFStandard::getLastNetworkErrorCode().value() == DCMNET_EINTR);
---
> 		} while ((bytesRead == -1 && OFStandard::getLastNetworkErrorCode().value() == DCMNET_EINTR)
> #ifdef HAVE_WINSOCK_H
> 			     || (bytesRead == -1 && (OFStandard::getLastNetworkErrorCode().value() == WSAEWOULDBLOCK))
> #endif
> 			     );
3723a3733,3743
> #ifdef HAVE_WINSOCK_H
> 			DWORD errorCode = GetLastError();
> 			LPVOID lpMsgBuf;
> 			FormatMessage(
> 				FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM,
> 				NULL, errorCode,
> 				MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
> 				(LPTSTR)&lpMsgBuf, 0, NULL);
> 			printf("defragmentTCP(): bytesRead=%d errorCode=%d %s\n", bytesRead, errorCode, (LPTSTR)lpMsgBuf);
> 			LocalFree(lpMsgBuf);
> #endif

J. Riesmeier
DCMTK Developer
Posts: 2501
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: defragmentTCP() does not handle WSAEWOULDBLOCK error on Windows when no data ready from non-blocking socket for read

#2 Post by J. Riesmeier »

Thank you for the report. I've added this issue to DCMTK's issue tracker, so it doesn't get lost: https://support.dcmtk.org/redmine/issues/1006


Marco Eichelberg
OFFIS DICOM Team
OFFIS DICOM Team
Posts: 1437
Joined: Tue, 2004-11-02, 17:22
Location: Oldenburg, Germany
Contact:

Re: defragmentTCP() does not handle WSAEWOULDBLOCK error on Windows when no data ready from non-blocking socket for read

#4 Post by Marco Eichelberg »

Can you explain what the "Sleep(1)" is intended for? I cannot see how this would be neccessary (or useful). Can you perhaps test what happens if you remove that statement in your code?

wrenashe
Posts: 20
Joined: Tue, 2013-10-08, 11:24

Re: defragmentTCP() does not handle WSAEWOULDBLOCK error on Windows when no data ready from non-blocking socket for read

#5 Post by wrenashe »

It is to wait for a while to recheck socket availability.

The code here resolves the problem we met here, In defragmentTCP(), read() in non-blocking mode fails when the socket is not ready to be read, if we do not wait for a while, defragmentTCP() will return then we have no data received.

Marco Eichelberg
OFFIS DICOM Team
OFFIS DICOM Team
Posts: 1437
Joined: Tue, 2004-11-02, 17:22
Location: Oldenburg, Germany
Contact:

Re: defragmentTCP() does not handle WSAEWOULDBLOCK error on Windows when no data ready from non-blocking socket for read

#6 Post by Marco Eichelberg »

I understand that. However, the block directly following your proposed Sleep() statement should more or less do the same: wait until data is available on the socket or the timeout expires:

Code: Select all

            /* if DUL_NOBLOCK is specified as a blocking option, we only want to wait a certain
             * time for receiving data over the network. If no data was received during that time,
             * DUL_READTIMEOUT shall be returned. Note that if DUL_BLOCK is specified the application
             * will not stop waiting until data is actually received over the network.
             */
            if (block == DUL_NOBLOCK)
            {
                /* determine remaining time to wait */
                timeToWait = timeout - (int) (time(NULL) - timerStart);

                /* go ahead and see if within timeout seconds data will be received over the network. */
                /* if not, return DUL_READTIMEOUT, if yes, stay in this function. */
                if (!connection->networkDataAvailable(timeToWait)) return DUL_READTIMEOUT;
            }
It seems that the Sleep() somehow only adds additional waiting time and interferes with the timeout.

wrenashe
Posts: 20
Joined: Tue, 2013-10-08, 11:24

Re: defragmentTCP() does not handle WSAEWOULDBLOCK error on Windows when no data ready from non-blocking socket for read

#7 Post by wrenashe »

In our Windows user context, that timeToWait is not long enough to wait for the data availability from the socket and meanwhile, that do-while condition does not tell WSAEWOULDBLOCK error, thus DUL_NETWORKCLOSED is returned instead of
DUL_READTIMEOUT expected. By the way the code works fine on our Linux runtime context.

Code: Select all

        do
        {
            /* if DUL_NOBLOCK is specified as a blocking option, we only want to wait a certain
             * time for receiving data over the network. If no data was received during that time,
             * DUL_READTIMEOUT shall be returned. Note that if DUL_BLOCK is specified the application
             * will not stop waiting until data is actually received over the network.
             */
#ifdef HAVE_WINSOCK_H
             if (OFStandard::getLastNetworkErrorCode().value() == WSAEWOULDBLOCK) {
                Sleep(1);
             }
#endif
            if (block == DUL_NOBLOCK)
            {
                /* determine remaining time to wait */
                timeToWait = timeout - (int) (time(NULL) - timerStart);

                /* go ahead and see if within timeout seconds data will be received over the network. */
                /* if not, return DUL_READTIMEOUT, if yes, stay in this function. */
                if (!connection->networkDataAvailable(timeToWait)) return DUL_READTIMEOUT;
            }

            /* data has become available, now call read(). */
            bytesRead = connection->read((char*)b, size_t(l));

        } while ((bytesRead == -1 && OFStandard::getLastNetworkErrorCode().value() == DCMNET_EINTR)
#ifdef HAVE_WINSOCK_H
                || (bytesRead == -1 && (OFStandard::getLastNetworkErrorCode().value() == WSAEWOULDBLOCK))
#endif
                 );

Marco Eichelberg
OFFIS DICOM Team
OFFIS DICOM Team
Posts: 1437
Joined: Tue, 2004-11-02, 17:22
Location: Oldenburg, Germany
Contact:

Re: defragmentTCP() does not handle WSAEWOULDBLOCK error on Windows when no data ready from non-blocking socket for read

#8 Post by Marco Eichelberg »

I fully understand that the modification to the do/while loop is necessary. I am still not convinced, however, that the Sleep() statement is necessary or useful.
Basically you pass a timeout value to ASC_initializeNetwork() that is too small for your use case, and then add a hard-coded timeout of one second. This does not seem to me to be a good solution, it is rather an ugly hack.
Is it possible for you to test what happens if you just remove the Sleep() statement? In that case the code should - if everything is OK - just continue to work.
Unfortunately I have no network code that runs the network in non-blocking mode, so I cannot test this myself.

Post Reply

Who is online

Users browsing this forum: Ahrefs [Bot], Google [Bot] and 1 guest