From cba5b994c990bba8df9b8bb75f25ca40aef6b68b Mon Sep 17 00:00:00 2001
From: Thomas Munro <tmunro@postgresql.org>
Date: Mon, 14 Feb 2022 16:29:44 +1300
Subject: [PATCH] Use WL_SOCKET_CLOSED for client_connection_check_interval.
Previously we used poll() directly to check for a POLLRDHUP event.
Instead, use the WaitEventSet API to poll the socket for
WL_SOCKET_CLOSED, which knows how to detect this condition on many more
operating systems.
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Maksim Milyutin <milyutinma@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/77def86b27e41f0efcba411460e929ae%40postgrespro.ru
---
doc/src/sgml/config.sgml | 6 ++---
src/backend/libpq/pqcomm.c | 46 ++++++++++++++++++------------------
src/backend/utils/misc/guc.c | 7 ++----
src/include/libpq/libpq.h | 1 +
4 files changed, 29 insertions(+), 31 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index fc63172efd..53b361e7a9 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1012,9 +1012,9 @@ include_dir 'conf.d'
the kernel reports that the connection is closed.
</para>
<para>
- This option is currently available only on systems that support the
- non-standard <symbol>POLLRDHUP</symbol> extension to the
- <symbol>poll</symbol> system call, including Linux.
+ This option relies on kernel events exposed by Linux, macOS, illumos
+ and the BSD family of operating systems, and is not currently available
+ on other systems.
</para>
<para>
If the value is specified without units, it is taken as milliseconds.
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 22eb04948e..9e34f616de 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -204,7 +204,7 @@ pq_init(void)
(errmsg("could not set socket to nonblocking mode: %m")));
#endif
- FeBeWaitSet = CreateWaitEventSet(TopMemoryContext, 3);
+ FeBeWaitSet = CreateWaitEventSet(TopMemoryContext, FeBeWaitSetNEvents);
socket_pos = AddWaitEventToSet(FeBeWaitSet, WL_SOCKET_WRITEABLE,
MyProcPort->sock, NULL, NULL);
latch_pos = AddWaitEventToSet(FeBeWaitSet, WL_LATCH_SET, PGINVALID_SOCKET,
@@ -1960,33 +1960,33 @@ pq_settcpusertimeout(int timeout, Port *port)
bool
pq_check_connection(void)
{
-#if defined(POLLRDHUP)
- /*
- * POLLRDHUP is a Linux extension to poll(2) to detect sockets closed by
- * the other end. We don't have a portable way to do that without
- * actually trying to read or write data on other systems. We don't want
- * to read because that would be confused by pipelined queries and COPY
- * data. Perhaps in future we'll try to write a heartbeat message instead.
- */
- struct pollfd pollfd;
+ WaitEvent events[FeBeWaitSetNEvents];
int rc;
- pollfd.fd = MyProcPort->sock;
- pollfd.events = POLLOUT | POLLIN | POLLRDHUP;
- pollfd.revents = 0;
+ /*
+ * It's OK to modify the socket event filter without restoring, because
+ * all FeBeWaitSet socket wait sites do the same.
+ */
+ ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetSocketPos, WL_SOCKET_CLOSED, NULL);
- rc = poll(&pollfd, 1, 0);
-
- if (rc < 0)
+retry:
+ rc = WaitEventSetWait(FeBeWaitSet, 0, events, lengthof(events), 0);
+ for (int i = 0; i < rc; ++i)
{
- ereport(COMMERROR,
- (errcode_for_socket_access(),
- errmsg("could not poll socket: %m")));
- return false;
+ if (events[i].events & WL_SOCKET_CLOSED)
+ return false;
+ if (events[i].events & WL_LATCH_SET)
+ {
+ /*
+ * A latch event might be preventing other events from being
+ * reported. Reset it and poll again. No need to restore it
+ * because no code should expect latches to survive across
+ * CHECK_FOR_INTERRUPTS().
+ */
+ ResetLatch(MyLatch);
+ goto retry;
+ }
}
- else if (rc == 1 && (pollfd.revents & (POLLHUP | POLLRDHUP)))
- return false;
-#endif
return true;
}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index f505413a7f..e2fe219aa8 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -12192,14 +12192,11 @@ check_huge_page_size(int *newval, void **extra, GucSource source)
static bool
check_client_connection_check_interval(int *newval, void **extra, GucSource source)
{
-#ifndef POLLRDHUP
- /* Linux only, for now. See pq_check_connection(). */
- if (*newval != 0)
+ if (!WaitEventSetCanReportClosed() && *newval != 0)
{
- GUC_check_errdetail("client_connection_check_interval must be set to 0 on platforms that lack POLLRDHUP.");
+ GUC_check_errdetail("client_connection_check_interval must be set to 0 on this platform");
return false;
}
-#endif
return true;
}
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index f0786e08b4..d348a55812 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -62,6 +62,7 @@ extern WaitEventSet *FeBeWaitSet;
#define FeBeWaitSetSocketPos 0
#define FeBeWaitSetLatchPos 1
+#define FeBeWaitSetNEvents 3
extern int StreamServerPort(int family, const char *hostName,
unsigned short portNumber, const char *unixSocketDir,