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 show that calling WSACleanup
during PQfinish is 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.

Back-patch of HEAD commit 7d00a6b2d.

Discussion: https://postgr.es/m/ac976d8c-03df-d6b8-025c-15a2de8d9af1@postgrespro.ru

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index a615dc0411c0a54dce4de3b9c1f05be811dba293..1513484e7d0dbebe483d7663dee2829ece69e5dd 100644 (file)
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -91,21 +91,6 @@
     
    
 
-   
-    
-     On Windows, there is a way to improve performance if a single
-     database connection is repeatedly started and shutdown.  Internally,
-     libpq calls WSAStartup() and WSACleanup() for connection startup
-     and shutdown, respectively.  WSAStartup() increments an internal
-     Windows library reference count which is decremented by WSACleanup().
-     When the reference count is just one, calling WSACleanup() frees
-     all resources and all DLLs are unloaded.  This is an expensive
-     operation.  To avoid this, an application can manually call
-     WSAStartup() so resources will not be freed when the last database
-     connection is closed.
-    
-   
-
    
     
      PQconnectdbParamsPQconnectdbParams

diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c
index c25e3f7a888f02aa55d12df25c60b8a572f09efa..c7d848a13e7423c4c9d7e618eb10c19d92a7d479 100644 (file)
--- a/src/bin/pg_dump/parallel.c
+++ b/src/bin/pg_dump/parallel.c
@@ -229,19 +229,6 @@ static char *readMessageFromPipe(int fd);
    (strncmp(msg, prefix, strlen(prefix)) == 0)
 
 
-/*
- * 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
@@ -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 b4d24071713efece84f72d8f35416dc9108c9484..4c45c126e4cac2874398390cdb70e19acbfc35ad 100644 (file)
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3866,23 +3866,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));
@@ -4075,10 +4082,6 @@ freePGconn(PGconn *conn)
    termPQExpBuffer(&conn->workBuffer);
 
    free(conn);
-
-#ifdef WIN32
-   WSACleanup();
-#endif
 }
 
 /*