From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: Victor Spirin <v.spirin@postgrespro.ru>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Sometimes the output to the stdout in Windows disappears
Date: Sat, 17 Oct 2020 14:44:37 -0400
Message-ID: <132799.1602960277@sss.pgh.pa.us> (raw)
In-Reply-To: <117919.1602949857@sss.pgh.pa.us>
References: <ac976d8c-03df-d6b8-025c-15a2de8d9af1@postgrespro.ru>
<4124040.1598898684@sss.pgh.pa.us>
<b92a7e09-9a60-d76b-e226-7e191fdc6f34@postgrespro.ru>
<855416.1600022228@sss.pgh.pa.us>
<ee02eaa2-03f7-74ea-bbdf-3196e506bae3@gmail.com>
<4156857.1602865112@sss.pgh.pa.us>
<b32688da-fafc-bbf6-b786-2dd7d0a4d578@gmail.com>
<117919.1602949857@sss.pgh.pa.us>
I wrote:
> Alexander Lakhin <exclusion@gmail.com> writes:
>> What bothers me is:
>> There must be a call to *WSACleanup* for each successful call to
>> WSAStartup
>> <https://docs.microsoft.com/en-us/windows/desktop/api/winsock/nf-winsock-wsastartup;.
> Yeah, that is a very odd statement. Surely, the Windows kernel manages
> to cope if a program crashes without having done that. So what exactly
> is the downside of intentionally not doing it?
A bit of grepping showed me that the backend, initdb, and pg_regress
all call WSAStartup without ever doing WSACleanup, and we've seen no
ill effects from that. So it seems clear that this documentation can
safely be ignored.
I propose the attached patch. If this doesn't cause buildfarm problems,
perhaps we should back-patch it.
BTW, I notice that libpq is asking WSAStartup for Winsock version 1.1,
which is remarkably ancient. Almost everyplace else is asking for
version 2.2, which has been current for a decade or two. Shouldn't
we update that? (It occurs to me to wonder if this in itself is
some kind of problem; I wonder how well Winsock works when there are
requests for different API versions in the same program.)
regards, tom lane
Attachments:remove-WSACleanup-calls.patch (text/x-diff, 3.5K) downloaddiff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgmlindex 3315f1dd05..de60281fcb 100644--- a/doc/src/sgml/libpq.sgml+++ b/doc/src/sgml/libpq.sgml@@ -91,21 +91,6 @@
</para>
</warning>
- <note>- <para>- On Windows, there is a way to improve performance if a single- database connection is repeatedly started and shutdown. Internally,- libpq calls <function>WSAStartup()</function> and <function>WSACleanup()</function> for connection startup- and shutdown, respectively. <function>WSAStartup()</function> increments an internal- Windows library reference count which is decremented by <function>WSACleanup()</function>.- When the reference count is just one, calling <function>WSACleanup()</function> frees- all resources and all DLLs are unloaded. This is an expensive- operation. To avoid this, an application can manually call- <function>WSAStartup()</function> so resources will not be freed when the last database- connection is closed.- </para>- </note>-
<variablelist>
<varlistentry id="libpq-PQconnectdbParams">
<term><function>PQconnectdbParams</function><indexterm><primary>PQconnectdbParams</primary></indexterm></term>
diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.cindex a967e11378..b51cc76c7d 100644--- 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.cindex af27fee6b5..704c9e2f79 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
}
/*
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, exclusion@gmail.com, v.spirin@postgrespro.ru, pgsql-hackers@lists.postgresql.org
Subject: Re: Sometimes the output to the stdout in Windows disappears
In-Reply-To: <132799.1602960277@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox