summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorTom Lane2020-10-17 20:53:48 +0000
committerTom Lane2020-10-17 20:53:48 +0000
commit7d00a6b2de8ab1e95e052663064defb0bc3022f6 (patch)
tree77dd4239a2ce182dd4fb9bada70065b4e9e6ff6a /src
parent540849814cdc22ea025777d374ff6705b4d64a0f (diff)
In libpq for Windows, call WSAStartup once and WSACleanup not at all.
The Windows documentation insists that every WSAStartup call should have a matching WSACleanup call. However, if that ever had actual relevance, it wasn't in this century. Every remotely-modern Windows kernel is capable of cleaning up when a process exits without doing that, and must be so to avoid resource leaks in case of a process crash. Moreover, Postgres backends have done WSAStartup without WSACleanup since commit 4cdf51e64 in 2004, and we've never seen any indication of a problem with that. libpq's habit of doing WSAStartup during connection start and WSACleanup during shutdown is also rather inefficient, since a series of non-overlapping connection requests leads to repeated, quite expensive DLL unload/reload cycles. We document a workaround for that (having the application call WSAStartup for itself), but that's just a kluge. It's also worth noting that it's far from uncommon for applications to exit without doing PQfinish, and we've not heard reports of trouble from that either. However, the real reason for acting on this is that recent experiments by Alexander Lakhin suggest that calling WSACleanup during PQfinish might be triggering the symptom we occasionally see that a process using libpq fails to emit expected stdio output. Therefore, let's change libpq so that it calls WSAStartup only once per process, during the first connection attempt, and never calls WSACleanup at all. While at it, get rid of the only other WSACleanup call in our code tree, in pg_dump/parallel.c; that presumably is equally useless. If this proves to suppress the fairly-common ecpg test failures we see on Windows, I'll back-patch, but for now let's just do it in HEAD and see what happens. Discussion: https://postgr.es/m/ac976d8c-03df-d6b8-025c-15a2de8d9af1@postgrespro.ru
Diffstat (limited to 'src')
-rw-r--r--src/bin/pg_dump/parallel.c16
-rw-r--r--src/interfaces/libpq/fe-connect.c31
2 files changed, 18 insertions, 29 deletions
diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c
index a967e113782..b51cc76c7dc 100644
--- a/src/bin/pg_dump/parallel.c
+++ b/src/bin/pg_dump/parallel.c
@@ -230,19 +230,6 @@ static char *readMessageFromPipe(int fd);
/*
- * Shutdown callback to clean up socket access
- */
-#ifdef WIN32
-static void
-shutdown_parallel_dump_utils(int code, void *unused)
-{
- /* Call the cleanup function only from the main thread */
- if (mainThreadId == GetCurrentThreadId())
- WSACleanup();
-}
-#endif
-
-/*
* Initialize parallel dump support --- should be called early in process
* startup. (Currently, this is called whether or not we intend parallel
* activity.)
@@ -267,8 +254,7 @@ init_parallel_dump_utils(void)
pg_log_error("WSAStartup failed: %d", err);
exit_nicely(1);
}
- /* ... and arrange to shut it down at exit */
- on_exit_nicely(shutdown_parallel_dump_utils, NULL);
+
parallel_init_done = true;
}
#endif
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index af27fee6b51..704c9e2f79f 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3871,23 +3871,30 @@ makeEmptyPGconn(void)
#ifdef WIN32
/*
- * Make sure socket support is up and running.
+ * Make sure socket support is up and running in this process.
+ *
+ * Note: the Windows documentation says that we should eventually do a
+ * matching WSACleanup() call, but experience suggests that that is at
+ * least as likely to cause problems as fix them. So we don't.
*/
- WSADATA wsaData;
+ static bool wsastartup_done = false;
- if (WSAStartup(MAKEWORD(1, 1), &wsaData))
- return NULL;
+ if (!wsastartup_done)
+ {
+ WSADATA wsaData;
+
+ if (WSAStartup(MAKEWORD(1, 1), &wsaData) != 0)
+ return NULL;
+ wsastartup_done = true;
+ }
+
+ /* Forget any earlier error */
WSASetLastError(0);
-#endif
+#endif /* WIN32 */
conn = (PGconn *) malloc(sizeof(PGconn));
if (conn == NULL)
- {
-#ifdef WIN32
- WSACleanup();
-#endif
return conn;
- }
/* Zero all pointers and booleans */
MemSet(conn, 0, sizeof(PGconn));
@@ -4080,10 +4087,6 @@ freePGconn(PGconn *conn)
termPQExpBuffer(&conn->workBuffer);
free(conn);
-
-#ifdef WIN32
- WSACleanup();
-#endif
}
/*