From c37dc98cea2983734decc37cadc2f3a19edd1e85 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Fri, 30 Apr 2021 10:48:32 +1200 Subject: [PATCH v2 2/2] Use WL_SOCKET_CLOSED for client_connection_check_interval. Previously we used poll() directly to check for a POLLRDHUP event. Instead, use WaitEventSet to poll the socket for WL_SOCKET_CLOSED, which knows how to detect equivalent events on many more operating systems. XXX Manually tested by killing psql, sending an async query with libpq and then sending 'X' and hanging up, and by yanking an ethernet cable (but the last requires TCP keepalives to be enabled). Need to find a clever way to do at least the first two in a TAP (?) test without races. Discussion: https://postgr.es/m/77def86b27e41f0efcba411460e929ae%40postgrespro.ru --- doc/src/sgml/config.sgml | 5 ++--- src/backend/libpq/pqcomm.c | 35 +++++++++++++---------------------- src/backend/utils/misc/guc.c | 7 ++----- 3 files changed, 17 insertions(+), 30 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index aa3e178240..c94e0ed9ef 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1012,9 +1012,8 @@ include_dir 'conf.d' the kernel reports that the connection is closed. - This option is currently available only on systems that support the - non-standard POLLRDHUP extension to the - poll system call, including Linux. + This option relies on kernel events exposed by Linux, BSD-family, + macOS and illumos, and is not available on other operating systems. 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 89a5f901aa..5660ece4e3 100644 --- a/src/backend/libpq/pqcomm.c +++ b/src/backend/libpq/pqcomm.c @@ -1932,33 +1932,24 @@ pq_settcpusertimeout(int timeout, Port *port) bool pq_check_connection(void) { -#if defined(POLLRDHUP) + WaitEvent events[3]; + int rc; + /* - * 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. + * Temporarily ignore the latch, while we check if the socket has been + * closed by the other end (if that is possible on this OS). */ - struct pollfd pollfd; - int rc; - - pollfd.fd = MyProcPort->sock; - pollfd.events = POLLOUT | POLLIN | POLLRDHUP; - pollfd.revents = 0; - - rc = poll(&pollfd, 1, 0); + ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetLatchPos, WL_LATCH_SET, NULL); + ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetSocketPos, WL_SOCKET_CLOSED, NULL); + rc = WaitEventSetWait(FeBeWaitSet, 0, events, lengthof(events), 0); + ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetLatchPos, WL_LATCH_SET, MyLatch); - if (rc < 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].pos == FeBeWaitSetSocketPos && + events[i].events & WL_SOCKET_CLOSED) + return false; } - 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 68b62d523d..465b5ec1b6 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -12063,14 +12063,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; } -- 2.30.2