5

I have the following simplified IO Completion Port server C++ code:

int main(..)
{
    startCompletionPortThreadProc();

    // Await client connection

    sockaddr_in clientAddress;
    int clientAddressSize = sizeof( clientAddress );
    SOCKET acceptSocket = WSAAccept( serverSocket, (SOCKADDR*)&clientAddress, &clientAddressSize, NULL, NULL);  

    // Connected

    CreateIoCompletionPort( (HANDLE)acceptSocket, completionPort, 0, 0 );

    // Issue initial read
    read( acceptSocket );
}


DWORD WINAPI completionPortThreadProc( LPVOID param )
{
    DWORD bytesTransferred = 0;
    ULONG_PTR completionKey = NULL;
    LPPER_IO_DATA perIoData = NULL;

    while( GetQueuedCompletionStatus( completionPort, &bytesTransferred, &completionKey, (LPOVERLAPPED*)&perIoData, INFINITE ) )
    {
        if( WaitForSingleObject( exitEvent, 0 ) == WAIT_OBJECT_0 )
        {
            break;
        }

        if( !perIoData )
            continue;

        if( bytesTransferred == 0 )
        {
            //TODO
        }

        switch( perIoData->operation )
        {
            case OPERATION_READ:
            {
                // Bytes have been received

                if( bytesTransferred < perIoData->WSABuf.len )
                {
                    // Terminate string
                    perIoData->WSABuf.buf[bytesTransferred]   = '\0';
                    perIoData->WSABuf.buf[bytesTransferred+1] = '\0';
                }

                // Add data to message build
                message += std::tstring( (TCHAR*)perIoData->WSABuf.buf );

                // Perform next read
                    perIoData->WSABuf.len = sizeof( perIoData->inOutBuffer );
                    perIoData->flags = 0; 

                    if( WSARecv( perIoData->socket, &( perIoData->WSABuf ), 1, &bytesTransferred, &( perIoData->flags ), &( perIoData->overlapped ), NULL ) == 0 )
                    {
                        // Part message
                        continue;
                    }

                    if( WSAGetLastError() == WSA_IO_PENDING )
                    {
                        // End of message
//TODO: Process message here
                        continue;
                    }
                }
            }
            break;

            case OPERATION_WRITE:
            {
                perIoData->bytesSent += bytesTransferred;

                if( perIoData->bytesSent < perIoData->bytesToSend )
                {
                    perIoData->WSABuf.buf = (char*)&( perIoData->inOutBuffer[perIoData->bytesSent] );
                    perIoData->WSABuf.len = ( perIoData->bytesToSend - perIoData->bytesSent);
                }
                else
                {
                    perIoData->WSABuf.buf  = (char*)perIoData->inOutBuffer;
                    perIoData->WSABuf.len  = _tcslen( perIoData->inOutBuffer ) * sizeof( TCHAR );
                    perIoData->bytesSent   = 0;
                    perIoData->bytesToSend = perIoData->WSABuf.len;
                }

                if( perIoData->bytesToSend )
                {
                    if( WSASend( perIoData->socket, &( perIoData->WSABuf ), 1, &bytesTransferred, 0, &( perIoData->overlapped ), NULL ) == 0 )
                        continue;

                    if( WSAGetLastError() == WSA_IO_PENDING )
                        continue;
                }
            }
            break;
        }
    }

    return 0;
}

bool SocketServer::read( SOCKET socket, HANDLE completionPort )
{
    PER_IO_DATA* perIoData = new PER_IO_DATA;
    ZeroMemory( perIoData, sizeof( PER_IO_DATA ) );

    perIoData->socket            = socket;
    perIoData->operation         = OPERATION_READ;
    perIoData->WSABuf.buf        = (char*)perIoData->inOutBuffer; 
    perIoData->WSABuf.len        = sizeof( perIoData->inOutBuffer );
    perIoData->overlapped.hEvent = WSACreateEvent();

    DWORD bytesReceived = 0;
    if( WSARecv( perIoData->socket, &( perIoData->WSABuf ), 1, &bytesReceived, &( perIoData->flags ), &( perIoData->overlapped ), NULL ) == SOCKET_ERROR )
    {
        int gle = WSAGetLastError();
        if( WSAGetLastError() != WSA_IO_PENDING )
        {
            delete perIoData;
            return false;
        }
    }

    return true;
}

bool SocketServer::write( SOCKET socket, std::tstring& data )
{
    PER_IO_DATA* perIoData = new PER_IO_DATA;
    ZeroMemory( perIoData, sizeof( PER_IO_DATA ) );

    perIoData->socket            = socket; 
    perIoData->operation         = OPERATION_WRITE;
    perIoData->WSABuf.buf        = (char*)data.c_str();
    perIoData->WSABuf.len        = _tcslen( data.c_str() ) * sizeof( TCHAR );
    perIoData->bytesToSend       = perIoData->WSABuf.len;  
    perIoData->overlapped.hEvent = WSACreateEvent();

    DWORD bytesSent = 0;
    if( WSASend( perIoData->socket, &( perIoData->WSABuf ), 1, &bytesSent, 0, &( perIoData->overlapped ), NULL ) == SOCKET_ERROR )
    {
        if( WSAGetLastError() != WSA_IO_PENDING )
        {
            delete perIoData;
            return false;
        }
    }

    return true;
}

1) The first issue I have is with the initial read.

On client connection (accept), I issue a read. As the client hasn't sent any data yet, WSAGetLastError() is WSA_IO_PENDING and the read method returns.

When the client then sends data, the thread remains stuck in the GetQueuedCompletionStatus call (as I assume I need another WSARecv call?).

Am I supposed to keep looping the read method until data arrives? That doesn't seem logical, I thought by issuing the initial read GetQueuedCompletionStatus would complete when data arrived.

2) I need to read and write data bi-directional without acknowledgements. Therefore I've also created a client with the IOCP thread. Is it actually possible to do this with completion ports or does a read have to be followed by a write?

Sorry for what feels like basic questions, but after trawling the internet and building IOCP examples I'm still unable to answer the questions.

Many thanks in advance.

Brent Campbell
  • 395
  • 1
  • 4
  • 12

1 Answers1

3

On client connection (accept), I issue a read. As the client hasn't sent any data yet, WSAGetLastError() is WSA_IO_PENDING and the read method returns.

That is normal behavior.

When the client then sends data, the thread remains stuck in the GetQueuedCompletionStatus call (as I assume I need another WSARecv call?).

No, you do not need another call. And if it is getting stuck, then you are not associating the read with the I/O Completion Port correctly.

Am I supposed to keep looping the read method until data arrives?

No. You need to call WSARecv() one time for the initial read. The WSA_IO_PENDING error means the read is waiting for data and will signal the I/O Completion Port when data actually arrives. DO NOT call WSARecv() (or any other read function) until that signal actually arrives. Then you can call WSARecv() again to wait for more data. Repeat until the socket is disconnected.

I thought by issuing the initial read GetQueuedCompletionStatus would complete when data arrived.

That is exactly what is supposed to happen.

2) I need to read and write data bi-directional without acknowledgements. Therefore I've also created a client with the IOCP thread. Is it actually possible to do this with completion ports

Yes. Reading and writing are separate operations, they are not dependent on each other.

does a read have to be followed by a write?

Not if your protocol does not require it, no.

Now, with that said, there are some issues with your code.

On a minor note, WSAAccept() is synchronous, you should consider using AcceptEx() instead so it can use the same I/O Completion Port for reporting new connections.

But more importantly, when a pending I/O operation fails, GetQueuedCompletionStatus() returns FALSE, the returned LPOVERLAPPED pointer will be non-NULL, and GetLastError() will report why the I/O operation failed. However, if GetQueuedCompletionStatus() itself fails, the returned LPOVERLAPPED pointer will be NULL, and GetLastError() will report why GetQueuedCompletionStatus() failed. This difference is clearly explained in the documentation, but your while loop is not accounting for it. Use a do..while loop instead and act according to the LPOVERLAPPED pointer:

DWORD WINAPI completionPortThreadProc( LPVOID param )
{
    DWORD bytesTransferred = 0;
    ULONG_PTR completionKey = NULL;
    LPPER_IO_DATA perIoData = NULL;

    do
    {
        if( GetQueuedCompletionStatus( completionPort, &bytesTransferred, &completionKey, (LPOVERLAPPED*)&perIoData, INFINITE ) )
        {
            // I/O success, handle perIoData based on completionKey as needed...
        }
        else if( perIoData )
        {
            // I/O failed, handle perIoData based on completionKey as needed...
        }
        else
        {
            // GetQueuedCompletionStatus() failure...
            break;
        }    
    }
    while( WaitForSingleObject( exitEvent, 0 ) == WAIT_TIMEOUT );

    return 0;
}

On a side note, instead of using an event object to signal when completionPortThreadProc() should exit, consider using PostQueuedCompletionionStatus() instead to post a termination completionKey to the I/O Completion Port, then your loop can look for that value:

DWORD WINAPI completionPortThreadProc( LPVOID param )
{
    DWORD bytesTransferred = 0;
    ULONG_PTR completionKey = NULL;
    LPPER_IO_DATA perIoData = NULL;

    do
    {
        if( GetQueuedCompletionStatus( completionPort, &bytesTransferred, &completionKey, (LPOVERLAPPED*)&perIoData, INFINITE ) )
        {
            if( completionKey == MyTerminateKey )
                break;

            if( completionKey == MySocketIOKey )
            {
                // I/O success, handle perIoData as needed...
            }
        }
        else if( perIoData )
        {
            // I/O failed, handle perIoData based on completionKey as needed...
        }
        else
        {
            // GetQueuedCompletionStatus() failure...
            break;
        }    
    }
    while( true );

    return 0;
}

CreateIoCompletionPort( (HANDLE)acceptSocket, completionPort, MySocketIOKey, 0 );

PostQueuedCompletionStatus( completionPort, 0, MyTerminateKey, NULL );
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Hi Remy, Many thanks for your long explanation, it's very much appreciated, I was going insane here. I will work through your comments and report back! I took a fair amount of the code from other examples which I assumed worked. – Brent Campbell Jun 07 '16 at 21:42
  • Hi Remy, ok, the new loop was key and specifically checking GetLastError in 'if( perIoData )'. I/O had failed with WSA_OPERATION_ABORTED hence why it never completed. I had an accept thread that issued the read and then ended. I would have thought it would have still worked but obviously my design is wrong. Many thanks for your help, I will use your thoughts in my new design. Hopefully your comments will also help others as a lot of examples seemed to use code like mine. – Brent Campbell Jun 07 '16 at 22:38
  • When a thread terminates, any I/O operations it has started that are still pending are automatically aborted. You should be calling `WSAAccept()` in a loop within a thread that lives for at least the lifetime of the listening socket (or move the client acceptance to the IO Completion Port and issue the initial accept from such a thread) so I/Os don't get aborted.. – Remy Lebeau Jun 07 '16 at 22:49
  • 1
    Remy, the thread termination and outstanding I/O issue ceased to be an issue from Windows Vista onwards... See Windows via C++ (Richter) (http://www.lenholgate.com/blog/2008/02/major-vista-overlapped-io-change.html) – Len Holgate Jun 08 '16 at 11:16