On 08/28/2014 03:47 PM, Kyotaro HORIGUCHI wrote:
To make the code mentioned above (Patch 0002) tidy, rewrite the
socket emulation code for win32 backends so that each socket
can have its own non-blocking state. (patch 0001)
The first patch that makes non-blocking sockets behave more sanely on
Windows seems like a good idea, independently of the second patch. I'm
looking at the first patch now, I'll make a separate post about the
second patch.
Some concern about this patch,
- This patch allows the number of non-blocking socket to be below
64 (FD_SETSIZE) on win32 backend but it seems to be sufficient.
Yeah, that's plenty.
- This patch introduced redundant socket emulation for win32
backend but win32 bare socket for Port is already nonblocking
as described so it donsn't seem to be a serious problem on
performance. Addition to it, since I don't know the reason why
win32/socket.c provides the blocking-mode socket emulation, I
decided to preserve win32/socket.c to have blocking socket
emulation. Possibly it can be removed.
On Windows, the backend has an emulation layer for POSIX signals, which
uses threads and Windows events. The reason win32/socket.c always uses
non-blocking mode internally is that it needs to wait for the socket to
become readable/writeable, and for the signal-emulation event, at the
same time. So no, we can't remove it.
The approach taken in the first patch seems sensible. I changed it to
not use FD_SET, though. A custom array seems better, that way we don't
need the pgwin32_nonblockset_init() call, we can just use initialize the
variable. It's a little bit more code, but it's well-contained in
win32/socket.c. Please take a look, to double-check that I didn't screw up.
- Heikki
commit aaaec23b08677baaed900f72db3f9628c0070922
Author: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Tue Sep 2 20:05:47 2014 +0300
Improve non-blocking sockets emulation on Windows.
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 605d891..cba79a7 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -795,10 +795,6 @@ pq_set_nonblocking(bool nonblocking)
if (MyProcPort->noblock == nonblocking)
return;
-#ifdef WIN32
- pgwin32_noblock = nonblocking ? 1 : 0;
-#else
-
/*
* Use COMMERROR on failure, because ERROR would try to send the error to
* the client, which might require changing the mode again, leading to
@@ -816,7 +812,6 @@ pq_set_nonblocking(bool nonblocking)
ereport(COMMERROR,
(errmsg("could not set socket to blocking mode: %m")));
}
-#endif
MyProcPort->noblock = nonblocking;
}
diff --git a/src/backend/port/win32/socket.c b/src/backend/port/win32/socket.c
index c981169..51982f0 100644
--- a/src/backend/port/win32/socket.c
+++ b/src/backend/port/win32/socket.c
@@ -3,6 +3,9 @@
* socket.c
* Microsoft Windows Win32 Socket Functions
*
+ * Blocking socket functions implemented so they listen on both the socket
+ * and the signal event, required for signal handling.
+ *
* Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
*
* IDENTIFICATION
@@ -14,18 +17,19 @@
#include "postgres.h"
/*
- * Indicate if pgwin32_recv() and pgwin32_send() should operate
- * in non-blocking mode.
- *
- * Since the socket emulation layer always sets the actual socket to
- * non-blocking mode in order to be able to deliver signals, we must
- * specify this in a separate flag if we actually need non-blocking
- * operation.
- *
- * This flag changes the behaviour *globally* for all socket operations,
- * so it should only be set for very short periods of time.
+ * Keep track which sockets are in non-blocking mode. Since the socket
+ * emulation layer always sets the actual socket to non-blocking mode in
+ * order to be able to deliver signals, we must track ourselves whether to
+ * present blocking or non-blocking behavior to the callers of pgwin32_recv()
+ * and pgwin32_send(). We expect there to be only a few non-blocking sockets,
+ * so a small array will do.
*/
-int pgwin32_noblock = 0;
+#define MAX_NONBLOCKING_SOCKETS 10
+
+static SOCKET nonblocking_sockets[MAX_NONBLOCKING_SOCKETS];
+static int num_nonblocking_sockets = 0;
+
+static bool pgwin32_socket_is_nonblocking(SOCKET s);
#undef socket
#undef accept
@@ -33,11 +37,57 @@ int pgwin32_noblock = 0;
#undef select
#undef recv
#undef send
+#undef closesocket
/*
- * Blocking socket functions implemented so they listen on both
- * the socket and the signal event, required for signal handling.
+ * Add a socket to the list of non-blocking sockets.
*/
+void
+pgwin32_set_socket_noblock(SOCKET s)
+{
+ if (pgwin32_socket_is_nonblocking(s))
+ return; /* already non-nlocking */
+
+ if (num_nonblocking_sockets >= MAX_NONBLOCKING_SOCKETS)
+ elog(ERROR, "too many non-blocking sockets");
+
+ nonblocking_sockets[num_nonblocking_sockets++] = s;
+}
+
+/*
+ * Remove a socket from the list of non-blocking sockets.
+ */
+void
+pgwin32_set_socket_block(SOCKET s)
+{
+ int i;
+
+ for (i = 0; i < num_nonblocking_sockets; i++)
+ {
+ if (nonblocking_sockets[i] == s)
+ {
+ /* Found. Move the last entry to this slot. */
+ if (i != num_nonblocking_sockets - 1)
+ nonblocking_sockets[i] =
+ nonblocking_sockets[num_nonblocking_sockets - 1];
+ num_nonblocking_sockets--;
+ break;
+ }
+ }
+}
+
+static bool
+pgwin32_socket_is_nonblocking(SOCKET s)
+{
+ int i;
+
+ for (i = 0; i < num_nonblocking_sockets; i++)
+ {
+ if (nonblocking_sockets[i] == s)
+ return true;
+ }
+ return false;
+}
/*
* Convert the last socket error code into errno
@@ -334,7 +384,7 @@ pgwin32_recv(SOCKET s, char *buf, int len, int f)
return -1;
}
- if (pgwin32_noblock)
+ if (pgwin32_socket_is_nonblocking(s))
{
/*
* No data received, and we are in "emulated non-blocking mode", so
@@ -420,7 +470,7 @@ pgwin32_send(SOCKET s, const void *buf, int len, int flags)
return -1;
}
- if (pgwin32_noblock)
+ if (pgwin32_socket_is_nonblocking(s))
{
/*
* No data sent, and we are in "emulated non-blocking mode", so
@@ -645,6 +695,16 @@ pgwin32_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, c
/*
+ * If the socket is in non-blocking mode, remove the entry from the array.
+ */
+int
+pgwin32_closesocket(SOCKET s)
+{
+ pgwin32_set_socket_block(s);
+ return closesocket(s);
+}
+
+/*
* Return win32 error string, since strerror can't
* handle winsock codes
*/
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index c7f41a5..32be003 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3255,22 +3255,9 @@ PgstatCollectorMain(int argc, char *argv[])
/*
* Try to receive and process a message. This will not block,
* since the socket is set to non-blocking mode.
- *
- * XXX On Windows, we have to force pgwin32_recv to cooperate,
- * despite the previous use of pg_set_noblock() on the socket.
- * This is extremely broken and should be fixed someday.
*/
-#ifdef WIN32
- pgwin32_noblock = 1;
-#endif
-
len = recv(pgStatSock, (char *) &msg,
sizeof(PgStat_Msg), 0);
-
-#ifdef WIN32
- pgwin32_noblock = 0;
-#endif
-
if (len < 0)
{
if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR)
diff --git a/src/include/port/win32.h b/src/include/port/win32.h
index 550c3ec..e3c0473 100644
--- a/src/include/port/win32.h
+++ b/src/include/port/win32.h
@@ -368,6 +368,7 @@ void pg_queue_signal(int signum);
#define select(n, r, w, e, timeout) pgwin32_select(n, r, w, e, timeout)
#define recv(s, buf, len, flags) pgwin32_recv(s, buf, len, flags)
#define send(s, buf, len, flags) pgwin32_send(s, buf, len, flags)
+#define closesocket(s) pgwin32_closesocket(s)
SOCKET pgwin32_socket(int af, int type, int protocol);
SOCKET pgwin32_accept(SOCKET s, struct sockaddr * addr, int *addrlen);
@@ -375,11 +376,12 @@ int pgwin32_connect(SOCKET s, const struct sockaddr * name, int namelen);
int pgwin32_select(int nfds, fd_set *readfs, fd_set *writefds, fd_set *exceptfds, const struct timeval * timeout);
int pgwin32_recv(SOCKET s, char *buf, int len, int flags);
int pgwin32_send(SOCKET s, const void *buf, int len, int flags);
+int pgwin32_closesocket(SOCKET s);
const char *pgwin32_socket_strerror(int err);
int pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout);
-
-extern int pgwin32_noblock;
+void pgwin32_set_socket_block(SOCKET s);
+void pgwin32_set_socket_noblock(SOCKET s);
/* in backend/port/win32/security.c */
extern int pgwin32_is_admin(void);
diff --git a/src/port/noblock.c b/src/port/noblock.c
index 1da0339..c8b35e0 100644
--- a/src/port/noblock.c
+++ b/src/port/noblock.c
@@ -22,11 +22,21 @@ pg_set_noblock(pgsocket sock)
{
#if !defined(WIN32)
return (fcntl(sock, F_SETFL, O_NONBLOCK) != -1);
-#else
+
+#elif defined(FRONTEND)
unsigned long ioctlsocket_ret = 1;
/* Returns non-0 on failure, while fcntl() returns -1 on failure */
return (ioctlsocket(sock, FIONBIO, &ioctlsocket_ret) == 0);
+
+#else
+ /*
+ * Sockets in Windows backend processes are wrapped, and blocking mode is
+ * handled by the emulation layer. See src/backend/port/win32/socket.c for
+ * details.
+ */
+ pgwin32_set_socket_noblock(sock);
+ return 1;
#endif
}
@@ -41,10 +51,16 @@ pg_set_block(pgsocket sock)
if (flags < 0 || fcntl(sock, F_SETFL, (long) (flags & ~O_NONBLOCK)))
return false;
return true;
-#else
+
+#elif FRONTEND
unsigned long ioctlsocket_ret = 0;
/* Returns non-0 on failure, while fcntl() returns -1 on failure */
return (ioctlsocket(sock, FIONBIO, &ioctlsocket_ret) == 0);
+
+#else
+ /* See pg_set_noblock */
+ pgwin32_set_socket_block(sock);
+ return 1;
#endif
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
