Strange IOCP behaviour when communicating with browsers

Question

I'm writing IOCP server for video streaming from desktop client to browser. Both sides uses WebSocket protocol to unify server's achitecture (and because there is no other way for browsers to perform a full-duplex exchange).

The working thread starts like this:

unsigned int __stdcall WorkerThread(void * param){
    int ThreadId = (int)param;
    OVERLAPPED *overlapped = nullptr;
    IO_Context *ctx = nullptr;
    Client *client = nullptr;
    DWORD transfered = 0;
    BOOL QCS = 0;

    while(WAIT_OBJECT_0 != WaitForSingleObject(EventShutdown, 0)){
        QCS = GetQueuedCompletionStatus(hIOCP, &transfered, (PULONG_PTR)&client, &overlapped, INFINITE);

        if(!client){
            if( Debug ) printf("No client\n");
            break;
        }
        ctx = (IO_Context *)overlapped;
        if(!QCS || (QCS && !transfered)){
            printf("Error %d\n", WSAGetLastError());
            DeleteClient(client);
            continue;
        }

        switch(auto opcode = client->ProcessCurrentEvent(ctx, transfered)){
            // Client owed to receive some data
            case OPCODE_RECV_DEBT:{ 
                if((SOCKET_ERROR == client->Recv()) && (WSA_IO_PENDING != WSAGetLastError())) DeleteClient(client);
                break;
            }
            // Client received all data or the beginning of new message
            case OPCODE_RECV_DONE:{ 
                std::string message;
                client->GetInput(message);
                // Analizing the first byte of WebSocket frame
                switch( opcode = message[0] & 0xFF ){ 
                    // HTTP_HANDSHAKE is 'G' - from GET HTTP...
                    case HTTP_HANDSHAKE:{
                        message = websocket::handshake(message);
                        while(!client->SetSend(message)) Sleep(1); // Set outgoing data
                        if((SOCKET_ERROR == client->Send()) && (WSA_IO_PENDING != WSAGetLastError())) DeleteClient(client);
                        break;
                    }
                    // Browser sent a closing frame (0x88) - performing clean WebSocket closure
                    case FIN_CLOSE:{
                        websocket::frame frame;
                        frame.parse(message);
                        frame.masked = false;
                        if( frame.pl_len == 0 ){
                            unsigned short reason = 1000;
                            frame.payload.resize(sizeof(reason));
                            frame.payload[0] = (reason >> 8) & 0xFF;
                            frame.payload[1] =  reason       & 0xFF;
                        }
                        frame.pack(message);
                        while(!client->SetSend(message)) Sleep(1);
                        if((SOCKET_ERROR == client->Send()) && (WSA_IO_PENDING != WSAGetLastError())) DeleteClient(client);
                        shutdown(client->Socket(), SD_SEND);
                        break;
                    }

IO context struct:

struct IO_Context{
    OVERLAPPED overlapped;
    WSABUF data;
    char buffer[IO_BUFFER_LENGTH];
    unsigned char opcode;
    unsigned long long debt;
    std::string message;
    IO_Context(){
        debt = 0;
        opcode = 0;
        data.buf = buffer;
        data.len = IO_BUFFER_LENGTH;
        overlapped.Offset = overlapped.OffsetHigh = 0;
        overlapped.Internal = overlapped.InternalHigh = 0;
        overlapped.Pointer = nullptr;
        overlapped.hEvent = nullptr;
    }
    ~IO_Context(){ while(!HasOverlappedIoCompleted(&overlapped)) Sleep(1); }
};

Client Send function:

int Client::Send(){
    int var_buf = O.message.size();
    // "O" is IO_Context for Output
    O.data.len = (var_buf>IO_BUFFER_LENGTH)?IO_BUFFER_LENGTH:var_buf;
    var_buf = O.data.len;
    while(var_buf > 0) O.data.buf[var_buf] = O.message[--var_buf];
    O.message.erase(0, O.data.len);
    return WSASend(connection, &O.data, 1, nullptr, 0, &O.overlapped, nullptr);
}

When the desktop client disconnects (it uses just closesocket() to do it, no shutdown()) the GetQueuedCompletionStatus returns TRUE and sets transfered to 0 - in this case WSAGetLastError() returns 64 (The specified network name is no longer available), and it has sense - client disconnected (line with if(!QCS || (QCS && !transfered))). But when the browser disconnects, the error codes confuse me... It can be 0, 997 (pending operation), 87 (invalid parameter)... and no codes related to end of connection.

Why do IOCP select this events? How can it select a pending operation? Why the error is 0 when 0 bytes transferred? Also it leads to endless trying to delete an object associated with the overlapped structure, because the destructor calls ~IO_Context(){ while(!HasOverlappedIoCompleted(&overlapped)) Sleep(1); } for secure deleting. In DeleteClient call the socket is closing with closesocket(), but, as you can see, I'm posting a shutdown(client->Socket(), SD_SEND); call before it (in FIN_CLOSE section).

I understand that there are two sides of a connection and closing it on a server side does not mean that an other side will close it too. But I need to create a stabile server, immune to bad and half opened connections. For example, the user of web application can rapidly press F5 to reload page few times (yeah, some dudes do so :) ) - the connection will reopen few times, and the server must not lag or crash due to this actions.

How to handle this "bad" events in IOCP?

Answer 1

you have many wrong code here.

while(WAIT_OBJECT_0 != WaitForSingleObject(EventShutdown, 0)){
    QCS = GetQueuedCompletionStatus(hIOCP, &transfered, (PULONG_PTR)&client, &overlapped, INFINITE);

this is not efficient and wrong code for stop WorkerThread. at first you do excess call WaitForSingleObject, use excess EventShutdown and main this anyway fail todo shutdown. if your code wait for packet inside GetQueuedCompletionStatus that you say EventShutdown - not break GetQueuedCompletionStatus call - you continue infinite wait here. correct way for shutdown - PostQueuedCompletionStatus(hIOCP, 0, 0, 0) instead call SetEvent(EventShutdown) and if worked thread view client == 0 - he break loop. and usually you need have multiple WorkerThread (not single). and multiple calls PostQueuedCompletionStatus(hIOCP, 0, 0, 0) - exactly count of working threads. also you need synchronize this calls with io - do this only after all io already complete and no new io packets will be queued to iocp. so "null packets" must be the last queued to port

if(!QCS || (QCS && !transfered)){
            printf("Error %d\n", WSAGetLastError());
            DeleteClient(client);
            continue;
        }

if !QCS - the value in client not initialized, you simply can not use it and call DeleteClient(client); is wrong under this condition

when object (client) used from several thread - who must delete it ? what be if one thread delete object, when another still use it ? correct solution will be if you use reference counting on such object (client). and based on your code - you have single client per hIOCP ? because you retriever pointer for client as completion key for hIOCP which is single for all I/O operation on sockets bind to the hIOCP. all this is wrong design.

you need store pointer to client in IO_Context. and add reference to client in IO_Context and release client in IO_Context destructor.

class IO_Context : public OVERLAPPED {
    Client *client;
    ULONG opcode;
    // ...

public:
    IO_Context(Client *client, ULONG opcode) : client(client), opcode(opcode) {
        client->AddRef();
    }

    ~IO_Context() {
        client->Release();
    }

    void OnIoComplete(ULONG transfered) {
        OnIoComplete(RtlNtStatusToDosError(Internal), transfered);
    }

    void OnIoComplete(ULONG error, ULONG transfered) {
        client->OnIoComplete(opcode, error, transfered);
        delete this;
    }

    void CheckIoError(ULONG error) {
        switch(error) {
            case NOERROR:
            case ERROR_IO_PENDING:
                break;
            default:
                OnIoComplete(error, 0);
        }
    }
};

then are you have single IO_Context ? if yes, this is fatal error. the IO_Context must be unique for every I/O operation.

if (IO_Context* ctx = new IO_Context(client, op))
{
    ctx->CheckIoError(WSAxxx(ctx) == 0 ? NOERROR : WSAGetLastError());
}

and from worked threads

ULONG WINAPI WorkerThread(void * param)
{
    ULONG_PTR key;
    OVERLAPPED *overlapped;
    ULONG transfered;
    while(GetQueuedCompletionStatus(hIOCP, &transfered, &key, &overlapped, INFINITE)) {
        switch (key){
        case '_io_':
            static_cast<IO_Context*>(overlapped)->OnIoComplete(transfered);
            continue;
        case 'stop':
            // ...
            return 0;
        default: __debugbreak();
        }
    }

    __debugbreak();
    return GetLastError();
}

the code like while(!HasOverlappedIoCompleted(&overlapped)) Sleep(1); is always wrong. absolute and always. never write such code.

ctx = (IO_Context *)overlapped; despite in your concrete case this give correct result, not nice and can be break if you change definition of IO_Context. you can use CONTAINING_RECORD(overlapped, IO_Context, overlapped) if you use struct IO_Context{ OVERLAPPED overlapped; } but better use class IO_Context : public OVERLAPPED and static_cast<IO_Context*>(overlapped)

now about Why do IOCP select this events? How to handle this "bad" events in IOCP?

the IOCP nothing select. he simply signaling when I/O complete. all. which specific wsa errors you got on different network operation absolute independent from use IOCP or any other completion mechanism.

on graceful disconnect is normal when error code is 0 and 0 bytes transferred in recv operation. you need permanent have recv request active after connection done, and if recv complete with 0 bytes transferred this mean that disconnect happens