Attempt to fix some issues in our Windows socket code.

Make sure WaitLatchOrSocket regards FD_CLOSE as a read-ready condition.
We might want to tweak this further, but it was surely wrong as-is.

Make pgwin32_waitforsinglesocket detach its private event object from the
passed socket before returning.  I suspect that failure to do so leads
to race conditions when other code (such as WaitLatchOrSocket) attaches
a different event object to the same socket.  Moreover, the existing
coding meant that repeated calls to pgwin32_waitforsinglesocket would
perform ResetEvent on an event actively connected to a socket, which
is rumored to be an unsafe practice; the WSAEventSelect documentation
appears to recommend against this, though it does not say not to do it
in so many words.

Also, uniformly use the coding pattern "WSAEventSelect(s, NULL, 0)" to
detach events from sockets, rather than passing the event in the second
parameter.  The WSAEventSelect documentation says that the second parameter
is ignored if the third is 0, so theoretically this should make no
difference.  However, elsewhere on the same reference page the use of NULL
in this context is recommended, and I have found suggestions on the net
that some versions of Windows have bugs with a non-NULL second parameter
in this usage.

Some other mostly-cosmetic cleanup, such as using the right one of
WSAGetLastError and GetLastError for reporting errors from these functions.

diff --git a/src/backend/port/win32/socket.c b/src/backend/port/win32/socket.c
index b2c632532834dd2424ebc8edc8efce3230e173c8..a7215cad6ec108ece1bd6f8947d29d572f77a00c 100644 (file)
--- a/src/backend/port/win32/socket.c
+++ b/src/backend/port/win32/socket.c
@@ -137,6 +137,7 @@ pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout)
        HANDLE          events[2];
        int                     r;
 
+       /* Create an event object just once and use it on all future calls */
        if (waitevent == INVALID_HANDLE_VALUE)
        {
                waitevent = CreateEvent(NULL, TRUE, FALSE, NULL);
@@ -150,20 +151,19 @@ pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout)
                                (errmsg_internal("could not reset socket waiting event: error code %lu", GetLastError())));
 
        /*
-        * make sure we don't multiplex this kernel event object with a different
-        * socket from a previous call
+        * Track whether socket is UDP or not.  (NB: most likely, this is both
+        * useless and wrong; there is no reason to think that the behavior of
+        * WSAEventSelect is different for TCP and UDP.)
         */
-
        if (current_socket != s)
-       {
-               if (current_socket != -1)
-                       WSAEventSelect(current_socket, waitevent, 0);
                isUDP = isDataGram(s);
-       }
-
        current_socket = s;
 
-       if (WSAEventSelect(s, waitevent, what) == SOCKET_ERROR)
+       /*
+        * Attach event to socket.  NOTE: we must detach it again before returning,
+        * since other bits of code may try to attach other events to the socket.
+        */
+       if (WSAEventSelect(s, waitevent, what) != 0)
        {
                TranslateSocketError();
                return 0;
@@ -196,10 +196,14 @@ pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout)
 
                                r = WSASend(s, &buf, 1, &sent, 0, NULL, NULL);
                                if (r == 0)             /* Completed - means things are fine! */
+                               {
+                                       WSAEventSelect(s, NULL, 0);
                                        return 1;
+                               }
                                else if (WSAGetLastError() != WSAEWOULDBLOCK)
                                {
                                        TranslateSocketError();
+                                       WSAEventSelect(s, NULL, 0);
                                        return 0;
                                }
                        }
@@ -210,6 +214,8 @@ pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout)
        else
                r = WaitForMultipleObjectsEx(2, events, FALSE, timeout, TRUE);
 
+       WSAEventSelect(s, NULL, 0);
+
        if (r == WAIT_OBJECT_0 || r == WAIT_IO_COMPLETION)
        {
                pgwin32_dispatch_queued_signals();
@@ -219,9 +225,12 @@ pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout)
        if (r == WAIT_OBJECT_0 + 1)
                return 1;
        if (r == WAIT_TIMEOUT)
+       {
+               errno = EWOULDBLOCK;
                return 0;
+       }
        ereport(ERROR,
-                       (errmsg_internal("unrecognized return value from WaitForMultipleObjects: %d (%lu)", r, GetLastError())));
+                       (errmsg_internal("unrecognized return value from WaitForMultipleObjects: %d (error code %lu)", r, GetLastError())));
        return 0;
 }
 
@@ -543,9 +552,12 @@ pgwin32_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, c
                if (writefds && FD_ISSET(sockets[i], writefds))
                        flags |= FD_WRITE | FD_CLOSE;
 
-               if (WSAEventSelect(sockets[i], events[i], flags) == SOCKET_ERROR)
+               if (WSAEventSelect(sockets[i], events[i], flags) != 0)
                {
                        TranslateSocketError();
+                       /* release already-assigned event objects */
+                       while (--i >= 0)
+                               WSAEventSelect(sockets[i], NULL, 0);
                        for (i = 0; i < numevents; i++)
                                WSACloseEvent(events[i]);
                        return -1;
@@ -565,9 +577,9 @@ pgwin32_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, c
                for (i = 0; i < numevents; i++)
                {
                        ZeroMemory(&resEvents, sizeof(resEvents));
-                       if (WSAEnumNetworkEvents(sockets[i], events[i], &resEvents) == SOCKET_ERROR)
-                               ereport(FATAL,
-                                               (errmsg_internal("failed to enumerate network events: error code %lu", GetLastError())));
+                       if (WSAEnumNetworkEvents(sockets[i], events[i], &resEvents) != 0)
+                               elog(ERROR, "failed to enumerate network events: error code %u",
+                                        WSAGetLastError());
                        /* Read activity? */
                        if (readfds && FD_ISSET(sockets[i], readfds))
                        {
@@ -594,10 +606,10 @@ pgwin32_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, c
                }
        }
 
-       /* Clean up all handles */
+       /* Clean up all the event objects */
        for (i = 0; i < numevents; i++)
        {
-               WSAEventSelect(sockets[i], events[i], 0);
+               WSAEventSelect(sockets[i], NULL, 0);
                WSACloseEvent(events[i]);
        }
 

diff --git a/src/backend/port/win32_latch.c b/src/backend/port/win32_latch.c
index 622798924dcde413693575396ae7e585a7b36099..62800ad66f2e507c837c0aafa0afa1c54130c02e 100644 (file)
--- a/src/backend/port/win32_latch.c
+++ b/src/backend/port/win32_latch.c
@@ -130,10 +130,11 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
        numevents = 2;
        if (wakeEvents & (WL_SOCKET_READABLE | WL_SOCKET_WRITEABLE))
        {
+               /* Need an event object to represent events on the socket */
                int                     flags = 0;
 
                if (wakeEvents & WL_SOCKET_READABLE)
-                       flags |= FD_READ;
+                       flags |= (FD_READ | FD_CLOSE);
                if (wakeEvents & WL_SOCKET_WRITEABLE)
                        flags |= FD_WRITE;
 
@@ -201,11 +202,11 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
                        WSANETWORKEVENTS resEvents;
 
                        ZeroMemory(&resEvents, sizeof(resEvents));
-                       if (WSAEnumNetworkEvents(sock, sockevent, &resEvents) == SOCKET_ERROR)
-                               elog(ERROR, "failed to enumerate network events: error code %lu",
-                                        GetLastError());
+                       if (WSAEnumNetworkEvents(sock, sockevent, &resEvents) != 0)
+                               elog(ERROR, "failed to enumerate network events: error code %u",
+                                        WSAGetLastError());
                        if ((wakeEvents & WL_SOCKET_READABLE) &&
-                               (resEvents.lNetworkEvents & FD_READ))
+                               (resEvents.lNetworkEvents & (FD_READ | FD_CLOSE)))
                        {
                                result |= WL_SOCKET_READABLE;
                        }
@@ -233,10 +234,10 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
        }
        while (result == 0);
 
-       /* Clean up the handle we created for the socket */
+       /* Clean up the event object we created for the socket */
        if (sockevent != WSA_INVALID_EVENT)
        {
-               WSAEventSelect(sock, sockevent, 0);
+               WSAEventSelect(sock, NULL, 0);
                WSACloseEvent(sockevent);
        }