On Wed Nov 22, 2023 at 3:00 PM CST, Heikki Linnakangas wrote:
On 22/11/2023 19:29, Tristan Partin wrote:
> On Thu Nov 16, 2023 at 8:33 AM CST, Heikki Linnakangas wrote:
>> On 06/11/2023 19:16, Tristan Partin wrote:
>>>>> That sounds like a much better solution. Attached you will find a v4
>>>>> that implements your suggestion. Please let me know if there is
>>>>> something that I missed. I can confirm that the patch works.
>>
>> This patch is missing a select(). It will busy loop until the connection
>> is established or cancelled.
>
> If I add a wait (select, poll, etc.), then I can't control-C during the
> blocking call, so it doesn't really solve the problem.
Hmm, they should return with EINTR on signal. At least on Linux; I'm not
sure how portable that is. See signal(7) man page, section "Interruption
of system calls and library functions by signal handlers". You could
also use a timeout like 5 s to ensure that you wake up and notice that
the signal was received eventually, even if it doesn't interrupt the
blocking call.
Ha, you're right. I had this working yesterday, but convinced myself it
didn't. I had a do while loop wrapping the blocking call. Here is a v4,
which seems to pass the tests that were pointed out to be failing
earlier.
Noticed that I copy-pasted pqSocketPoll() into the psql code. I think
this may be controversial. Not sure what the best solution to the issue
is. I will pay attention to the buildfarm animals when they pick this
up.
--
Tristan Partin
Neon (https://neon.tech)
From 1b4303df1200c561cf478f9c7037ff940f6cd741 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@neon.tech>
Date: Mon, 24 Jul 2023 11:12:59 -0500
Subject: [PATCH v4] Allow SIGINT to cancel psql database reconnections
After installing the SIGINT handler in psql, SIGINT can no longer cancel
database reconnections. For instance, if the user starts a reconnection
and then needs to do some form of interaction (ie psql is polling),
there is no way to cancel the reconnection process currently.
Use PQconnectStartParams() in order to insert a CancelRequested check
into the polling loop.
---
src/bin/psql/command.c | 129 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 128 insertions(+), 1 deletion(-)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 82cc091568..588eed9351 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -11,6 +11,11 @@
#include <time.h>
#include <pwd.h>
#include <utime.h>
+#if HAVE_POLL
+#include <poll.h>
+#else
+#include <sys/select.h>
+#endif
#ifndef WIN32
#include <sys/stat.h> /* for stat() */
#include <sys/time.h> /* for setitimer() */
@@ -40,6 +45,7 @@
#include "large_obj.h"
#include "libpq-fe.h"
#include "libpq/pqcomm.h"
+#include "libpq/pqsignal.h"
#include "mainloop.h"
#include "portability/instr_time.h"
#include "pqexpbuffer.h"
@@ -3251,6 +3257,90 @@ copy_previous_query(PQExpBuffer query_buf, PQExpBuffer previous_buf)
return false;
}
+/*
+ * Check a file descriptor for read and/or write data, possibly waiting.
+ * If neither forRead nor forWrite are set, immediately return a timeout
+ * condition (without waiting). Return >0 if condition is met, 0
+ * if a timeout occurred, -1 if an error or interrupt occurred.
+ *
+ * Timeout is infinite if end_time is -1. Timeout is immediate (no blocking)
+ * if end_time is 0 (or indeed, any time before now).
+ */
+static int
+pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time)
+{
+ /* We use poll(2) if available, otherwise select(2) */
+#ifdef HAVE_POLL
+ struct pollfd input_fd;
+ int timeout_ms;
+
+ if (!forRead && !forWrite)
+ return 0;
+
+ input_fd.fd = sock;
+ input_fd.events = POLLERR;
+ input_fd.revents = 0;
+
+ if (forRead)
+ input_fd.events |= POLLIN;
+ if (forWrite)
+ input_fd.events |= POLLOUT;
+
+ /* Compute appropriate timeout interval */
+ if (end_time == ((time_t) -1))
+ timeout_ms = -1;
+ else
+ {
+ time_t now = time(NULL);
+
+ if (end_time > now)
+ timeout_ms = (end_time - now) * 1000;
+ else
+ timeout_ms = 0;
+ }
+
+ return poll(&input_fd, 1, timeout_ms);
+#else /* !HAVE_POLL */
+
+ fd_set input_mask;
+ fd_set output_mask;
+ fd_set except_mask;
+ struct timeval timeout;
+ struct timeval *ptr_timeout;
+
+ if (!forRead && !forWrite)
+ return 0;
+
+ FD_ZERO(&input_mask);
+ FD_ZERO(&output_mask);
+ FD_ZERO(&except_mask);
+ if (forRead)
+ FD_SET(sock, &input_mask);
+
+ if (forWrite)
+ FD_SET(sock, &output_mask);
+ FD_SET(sock, &except_mask);
+
+ /* Compute appropriate timeout interval */
+ if (end_time == ((time_t) -1))
+ ptr_timeout = NULL;
+ else
+ {
+ time_t now = time(NULL);
+
+ if (end_time > now)
+ timeout.tv_sec = end_time - now;
+ else
+ timeout.tv_sec = 0;
+ timeout.tv_usec = 0;
+ ptr_timeout = &timeout;
+ }
+
+ return select(sock + 1, &input_mask, &output_mask,
+ &except_mask, ptr_timeout);
+#endif /* HAVE_POLL */
+}
+
/*
* Ask the user for a password; 'username' is the username the
* password is for, if one has been explicitly specified.
@@ -3324,6 +3414,7 @@ do_connect(enum trivalue reuse_previous_specification,
bool keep_password = true;
bool has_connection_string;
bool reuse_previous;
+ bool for_read;
has_connection_string = dbname ?
recognized_connection_string(dbname) : false;
@@ -3614,11 +3705,47 @@ do_connect(enum trivalue reuse_previous_specification,
values[paramnum] = NULL;
/* Note we do not want libpq to re-expand the dbname parameter */
- n_conn = PQconnectdbParams(keywords, values, false);
+ n_conn = PQconnectStartParams(keywords, values, false);
pg_free(keywords);
pg_free(values);
+ for_read = false;
+ while (true) {
+ int rc;
+ int sock;
+
+ if (cancel_pressed)
+ break;
+
+ sock = PQsocket(n_conn);
+ if (sock == -1)
+ break;
+
+ rc = pqSocketPoll(sock, for_read, !for_read, (time_t)-1);
+ Assert(rc != 0); /* Timeout is impossible. */
+ if (rc == -1)
+ {
+ success = false;
+ break;
+ }
+
+ switch (PQconnectPoll(n_conn)) {
+ case PGRES_POLLING_OK:
+ case PGRES_POLLING_FAILED:
+ goto finish;
+ case PGRES_POLLING_READING:
+ for_read = true;
+ continue;
+ case PGRES_POLLING_WRITING:
+ for_read = false;
+ continue;
+ case PGRES_POLLING_ACTIVE:
+ pg_unreachable();
+ }
+ }
+
+ finish:
if (PQstatus(n_conn) == CONNECTION_OK)
break;
--
Tristan Partin
Neon (https://neon.tech)
