Joshua
Joshua

Reputation: 43317

Under what circumstances, if any, can ReadFile hang after WSAEventSelect said it was ready?

This is rather ugly. We're getting weird hangups caused (read: triggered) by some security scanning software. I suspect it has a nonstandard TCP/IP stack, and the customer is asking why hangups.

Static analysis suggests only two possible locations for the hangup. The hangup either has to be in ReadFile() or WriteFile() on a socket; and WriteFile() cannot hang here unless the scanner is designed to make WriteFile() hang by setting the window size to zero. If WriteFile() were to return at all even if it didn't make progress I'd be able to knock the thing out of its wedged state. I also don't think the log state is consistent with WriteFile() returning.

So onto ReadFile(): this is the calling sequence:

    SOCKET conn;
    HANDLE unwedgeEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
    HANDLE listenEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
    if (listenEvent == NULL) return;
    //...
    conn = accept(lstn);
    for (;;) {
        HANDLE wakeup[2];
        wakeup[0] = unwedgeEvent;
        wakeup[1] = listenEvent;
        if (WSAEventSelect(conn, socket, FD_READ | FD_CLOSE) == 0) {
            // Log error 
            break;
        }
        which = WaitForMultipleObjects(2, wakeup, FALSE, INFINITE);
        if (which < 0) {
           // Log error
           break;
        }
        if (which == 1) {
            DWORD read;
            r = ReadFile(conn, chunk, 4096, &read, NULL);
            if (r == 0) {
                // Handle error -- not stuck here
            } else {
                // Handle data -- not stuck here either
            }
        }
        if (which == 0) break;
    }

Where signalling unwedgeEvent doesn't manage to accomplish anything and the thread remains stuck forever.

So the real question is have I gone nuts or is this really a thing that can happen?

So this has gone somewhat off the deep end; I don't need non-blocking sockets at all. I need a select() that takes handle arguments to things that are sockets and things that are not sockets.

The following API sequences do not hang in ReadFile:

---------------------------------------------------------------
Sender                                 B Receiver
                                         WSAEventSelect
                                       * WaitForMultipeObjects
send(buffer size = 1)
                                         ReadFile(size = 1)
                                         WSAEventSelect
                                       * WaitForMultipeObjects
send(buffer size = 1)
                                         ReadFile(size = 1)
................................................................
                                         WSAEventSelect
                                       * WaitForMultipeObjects
send(buffer size = 2)
                                         ReadFile(size = 1)
                                         WaitForMultipeObjects
                                         ReadFile(size = 1)
................................................................
                                         WSAEventSelect
                                       * WaitForMultipeObjects
send(buffer size = 1)
send(buffer size = 1)
                                         ReadFile(size = 1)
                                         WaitForMultipeObjects
                                         ReadFile(size = 1)
................................................................
                                         WSAEventSelect
send(buffer size = 1)
                                         WaitForMultipeObjects
send(buffer size = 1)
                                         ReadFile(size = 1)
                                         WaitForMultipeObjects
                                         ReadFile(size = 1)
................................................................
                                         WSAEventSelect
send(buffer size = 1)
                                         WaitForMultipeObjects
send(buffer size = 1)
                                         ReadFile(size = 2)
                                       * WaitForMultipeObjects
send(buffer size = 1)
                                         ReadFile(size = 1)
................................................................
                                         WSAEventSelect
send(buffer size = 1)
                                         WaitForMultipeObjects
                                         ReadFile(size = 1)
send(buffer size = 1)
                                         WaitForMultipeObjects
                                         ReadFile(size = 1)

Upvotes: 0

Views: 226

Answers (1)

Remy Lebeau
Remy Lebeau

Reputation: 597051

I see a number of issues with your code:

  • You should use WSACreateEvent() and WSAWaitForMultipleEvents() instead of CreateEvent() and WaitForMultipleObjects(). Although the current implementation is that the former APIs simply map to the latter APIs, Microsoft is free to change that implementation at any time without breaking code that uses the former APIs properly.

  • In your call to WSAEventSelect(), socket should be listenEvent instead.

  • WSAEventSelect() returns SOCKET_ERROR (-1) on failure, not 0 like you have coded.

  • You are not calling WSAEnumNetworkEvents() at all, which you need to do in order to determine if FD_READ was the actual type of event triggered, and to clear the socket's event state and reset the event object. So, you may be acting on a stale read state, which could explain why you end up calling ReadFile() when there is actually nothing available to read.

  • WSAEventSelect() puts the socket into non-blocking mode (per its documentation), so it is actually not possible for ReadFile() (or any other reading function) to block on the socket. However, it could fail immediately with a WSAEWOULDBLOCK error, so make sure you are not treating that condition as a fatal error.

  • The WSAEventSelect() documentation does not list ReadFile() as a supported function for re-enabling events when calling WSAEnumNetworkEvents(). Although the Socket Handles documentation does say that a Winsock SOCKET can be used with non-Winsock I/O functions like ReadFile(), it recommends that a SOCKET should only be used with Winsock functions. So, you should use send()/recv() or WSASend()/WSARecv() instead of WriteFile()/ReadFile().

With that said, try something more like the following:

HANDLE unwedgeEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
if (unwedgeEvent == NULL) {
    // Log error 
    return;
}

...

SOCKET conn = accept(lstn, NULL, NULL);
if (conn == INVALID_SOCKET) {
    // Log error 
    return;
}

...

WSAEVENT listenEvent = WSACreateEvent();
if (listenEvent == NULL) {
    // Log error 
}
else if (WSAEventSelect(conn, listenEvent, FD_READ | FD_CLOSE) == SOCKET_ERROR) {
    // Log error 
}
else {
    WSAEVENT wakeup[2];
    wakeup[0] = (WSAEVENT) unwedgeEvent;
    wakeup[1] = listenEvent;

    char chunk[4096];               
    int read;

    do {
        DWORD which = WSAWaitForMultipleEvents(2, wakeup, FALSE, WSA_INFINITE, FALSE);
        if (which == WSA_WAIT_FAILED) {
            // Log error
            break;
        }

        if (which == WSA_WAIT_EVENT_0) {
            break;
        }

        if (which == (WSA_WAIT_EVENT_0+1)) {
            WSANETWORKEVENTS events = {};
            if (WSAEnumNetworkEvents(conn, listenEvent, &events) == SOCKET_ERROR) {
                // Log error
                break;
            }

            if (events.lNetworkEvents & FD_READ) {
                read = recv(conn, chunk, sizeof(chunk), 0);
                if (read == SOCKET_ERROR) {
                    if (WSAGetLastError() != WSAEWOULDBLOCK) {
                        // Log error
                        break;
                    }
                }
                else if (read == 0) {
                    break;
                }
                else {
                    // Handle chunk up to read number of bytes
                }
            }

            if (events.lNetworkEvents & FD_CLOSE) {
                break;
            }
        }
    }
    while (true);

    WSACloseEvent(listenEvent);
}

closesocket(conn);

Upvotes: 1

Related Questions