On 2016-01-02 16:20:58 +0100, Andres Freund wrote:
> I really right now can see only two somewhat surgical fixes:
> 
> 1) We do a nonblocking or select() *after* registering our events. Both
>    in WaitLatchOrSocket() and waitforsinglesocket. Since select/poll are
>    explicitly level triggered, that should make us notice any events we
>    might have missed. select() appears to have been available for a fair
>    while.
> 
> 2) We explicitly shutdown(SD_BOTH) the socket whenever we get a FD_CLOSE
>    object. I *think* this should trigger errors in WSArecv, WSAEventSelect
>    et al. Doesn't solve the problem that we might miss important events
>    though.
> 
> 
> Given 2) isn't a complete fix and I can't find reliable documentation
> since when shutdown() is supported I'm inclined to go with 1).


Attached is an attempt to blindly implement 1). It needs some further
polishing and thought, but I'd like to verify it actually fixes things
before investing more time in this.

Greetings,

Andres Freund
diff --git a/src/backend/port/win32_latch.c b/src/backend/port/win32_latch.c
index 0e3aaee..b4441d8 100644
--- a/src/backend/port/win32_latch.c
+++ b/src/backend/port/win32_latch.c
@@ -177,6 +177,65 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
 	/* Ensure that signals are serviced even if latch is already set */
 	pgwin32_dispatch_queued_signals();
 
+	/*
+	 * FD_WRITE and FD_CLOSE are edge and not level triggered
+	 * (c.f. WSAEventSelect's documentation), and at least FD_CLOSE is only
+	 * signalled when the state changes while the event is attached to the
+	 * socket. To avoid waiting, potentially indefinitely, do a non-blocking
+	 * select() on the socket diagnosing the current state. Check this after
+	 * the signal dispatching above, to avoid starving signal dispatch.
+	 *
+	 * Unfortunately, according to the documentation, windows doesn't signal
+	 * writeability when a socket is closed. Only WSAPoll() does so - but it's
+	 * only available in newer versions of windows. XXX: Is this an active
+	 * race condition?
+	 */
+	if (wakeEvents & (WL_SOCKET_READABLE | WL_SOCKET_WRITEABLE))
+	{
+		fd_set		input_mask;
+		fd_set	   *input_maskp = NULL;
+		fd_set		output_mask;
+		fd_set	   *output_maskp = NULL;
+		struct timeval tv;
+
+		FD_ZERO(&input_mask);
+		FD_ZERO(&output_mask);
+		tv.tv_sec = 0;
+		tv.tv_usec = 0;
+
+		if (wakeEvents & WL_SOCKET_READABLE)
+		{
+			FD_SET(sock, &input_mask);
+			input_maskp = &input_mask;
+		}
+		if (wakeEvents & WL_SOCKET_WRITEABLE)
+		{
+			FD_SET(sock, &output_mask);
+			output_maskp = &output_mask;
+		}
+
+		rc = select(1, input_maskp, output_maskp, NULL, &tv);
+
+		if (rc < 0)
+		{
+			elog(ERROR, "select() failed: %lu", GetLastError());
+		}
+		else if (rc > 0)
+		{
+			if ((wakeEvents & WL_SOCKET_READABLE) && FD_ISSET(sock, &input_mask))
+			{
+				/* data available in socket, or EOF */
+				result |= WL_SOCKET_READABLE;
+			}
+			if ((wakeEvents & WL_SOCKET_WRITEABLE) && FD_ISSET(sock, &output_mask))
+			{
+				/* socket is writable */
+				result |= WL_SOCKET_WRITEABLE;
+			}
+			return result;
+		}
+	}
+
 	do
 	{
 		/*
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to