From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: Daniel Gustafsson <daniel@yesql.se>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: byavuz81@gmail.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Cc: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Cc: Michael Paquier <michael@paquier.xyz>
Subject: Re: BUG #17391: While using --with-ssl=openssl and PG_TEST_EXTRA='ssl' options, SSL tests fail on OpenBSD 7.0
Date: Fri, 11 Feb 2022 16:54:54 -0500
Message-ID: <2187263.1644616494@sss.pgh.pa.us> (raw)
In-Reply-To: <2077573.1644599439@sss.pgh.pa.us>
References: <C44CE9FB-B37E-47D3-924B-4AF808AE98B1@yesql.se>
<887CBA28-0942-4084-BE09-FBE0D6A1DB53@yesql.se>
<621771.1644098845@sss.pgh.pa.us>
<635760.1644108785@sss.pgh.pa.us>
<1113966.1644280235@sss.pgh.pa.us>
<C112C400-2CB5-4DCA-A699-00DC0333B665@yesql.se>
<1537890.1644462214@sss.pgh.pa.us>
<1952254.1644540297@sss.pgh.pa.us>
<20220211011025.ek7exh6owpzjyudn@alap3.anarazel.de>
<1955929.1644542728@sss.pgh.pa.us>
<20220211015455.mkl6cvstttp5zfco@alap3.anarazel.de>
<1959196.1644544971@sss.pgh.pa.us>
<2077573.1644599439@sss.pgh.pa.us>
I wrote:
>> Hmm, maybe. It sure *looks* like we need to do
>> - return conn->asyncStatus == PGASYNC_BUSY || conn->write_failed;
>> + return conn->asyncStatus == PGASYNC_BUSY && !conn->write_failed;
> Dunno anything about the Windows angle, but after hacking together
> a reproducer for this, I'm fairly convinced that this is indeed a bug.
After further study, I now think that checking write_failed here is
the wrong thing entirely, because it's irrelevant to what we want to
do, which is keep reading until we've collected a server message or
seen read EOF.
What we do need to do though is check to see if we've closed the
socket. That's because places like libpqwalreceiver.c assume
that if PQisBusy is true, then PQsocket() must be valid:
while (PQisBusy(streamConn))
{
int rc;
rc = WaitLatchOrSocket(MyLatch,
WL_EXIT_ON_PM_DEATH | WL_SOCKET_READABLE |
WL_LATCH_SET,
PQsocket(streamConn),
0,
WAIT_EVENT_LIBPQWALRECEIVER_RECEIVE);
It seems like the existing coding in PQisBusy could allow that to fail.
I scraped the buildfarm for instances of the "cannot wait on socket
event without a socket" error that latch.c would emit if that happened,
but found none going back three months. That's perhaps because of the
other behavior I noted that if the walsender crashes, we'll probably
report write failure immediately in PQsendQuery. So maybe this is
mostly hypothetical in practice, but I think what we actually want
here is as attached.
In any case, I still don't see a way for this error to result in
an infinite loop, so it doesn't seem like an explanation for the
Windows problems.
regards, tom lane
Attachments:
[text/x-diff] PQisBusy-fix.patch (932B, ../2187263.1644616494@sss.pgh.pa.us/2-PQisBusy-fix.patch)
download | inline diff:diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.cindex 59121873d2..9afd4d88b4 100644--- a/src/interfaces/libpq/fe-exec.c+++ b/src/interfaces/libpq/fe-exec.c@@ -1957,10 +1957,14 @@ PQisBusy(PGconn *conn)
parseInput(conn);
/*
- * PQgetResult will return immediately in all states except BUSY, or if we- * had a write failure.+ * PQgetResult will return immediately in all states except BUSY. Also,+ * if we've detected read EOF and dropped the connection, we can expect+ * that PQgetResult will fail immediately. Note that we do *not* check+ * conn->write_failed here --- once that's become set, we know we have+ * trouble, but we need to keep trying to read until we have a complete+ * server message or detect read EOF.
*/
- return conn->asyncStatus == PGASYNC_BUSY || conn->write_failed;+ return conn->asyncStatus == PGASYNC_BUSY && conn->status != CONNECTION_BAD;
}
/*
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-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, daniel@yesql.se, thomas.munro@gmail.com, pgsql-bugs@lists.postgresql.org, heikki.linnakangas@iki.fi, michael@paquier.xyz
Subject: Re: BUG #17391: While using --with-ssl=openssl and PG_TEST_EXTRA='ssl' options, SSL tests fail on OpenBSD 7.0
In-Reply-To: <2187263.1644616494@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