public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jelte Fennema <[email protected]>
To: Denis Laxalde <[email protected]>
Cc: Greg Stark <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Gregory Stark (as CFM) <[email protected]>
Cc: Jelte Fennema <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Jacob Champion <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Justin Pryzby <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: [EXTERNAL] Re: Add non-blocking version of PQcancel
Date: Tue, 28 Mar 2023 17:54:06 +0200
Message-ID: <CAGECzQQUw3UFP8ivMYg0tt3-vDscSZMRuwpABjkUO2m9=JUSTA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAM-w4HP4yN2krwqhXUBQYCjzSjCAWyXMb1xAOo4cv_-_hLXUWA@mail.gmail.com>
<CAGECzQRZTyPVGLANavQ7uokDywx-bodpK=NSWqgxM-mnJ_foQg@mail.gmail.com>
<CAM-w4HNCHEuCBcUxE8H6t15SJKzZd6hKtyHNUTa30VhJxqmuOw@mail.gmail.com>
<CAGECzQSvTjte1-QmzcjnJJkr7BYvuOK4FkjXpXVnRA2AJHuAHw@mail.gmail.com>
<CAGECzQR57o1wdo9fG7grmmabrt1jotHR-T=nQZGXiZRHpvgrGg@mail.gmail.com>
<CAM-w4HOf4+em6TivqZXtbVvpahHRYExxosK5eAyje5C+dSxWSg@mail.gmail.com>
<[email protected]>
<CAM-w4HPyCqVuf=fR-VBB1xcCtUF0+cefpVOv29O-JYbAG4dh2w@mail.gmail.com>
<CAGECzQR7kRY3WKKwx0FdmAA3P2vD+SGALOcXV-dcqMwM4LHV8A@mail.gmail.com>
<CAGECzQTaGtAWmM98q9uup2pjODR8WSOsF_UV3YG7THoqatJApA@mail.gmail.com>
<[email protected]>
On Tue, 28 Mar 2023 at 16:54, Denis Laxalde <[email protected]> wrote:
> I had a look into your patchset (v16), did a quick review and played a
> bit with the feature.
>
> Patch 2 is missing the documentation about PQcancelSocket() and contains
> a few typos; please find attached a (fixup) patch to correct these.
Thanks applied that patch and attached a new patchset
> Namely, I wonder why it returns a PGcancelConn and what's the
> point of requiring the user to call PQcancelStatus() to see if something
> got wrong. Maybe it could be defined as:
>
> int PQcancelSend(PGcancelConn *cancelConn);
>
> where the return value would be status? And the user would only need to
> call PQcancelErrorMessage() in case of error. This would leave only one
> single way to create a PGcancelConn value (i.e. PQcancelConn()), which
> seems less confusing to me.
To clarify what you mean, the API would then be like this:
PGcancelConn cancelConn = PQcancelConn(conn);
if (PQcancelSend(cancelConn) == CONNECTION_BAD) {
printf("ERROR %s\n", PQcancelErrorMessage(cancelConn))
exit(1)
}
Instead of:
PGcancelConn cancelConn = PQcancelSend(conn);
if (PQcancelStatus(cancelConn) == CONNECTION_BAD) {
printf("ERROR %s\n", PQcancelErrorMessage(cancelConn))
exit(1)
}
Those are so similar, that I have no preference either way. If more
people prefer one over the other I'm happy to change it, but for now
I'll keep it as is.
> As part of my testing, I've implemented non-blocking cancellation in
> Psycopg, based on v16 on this patchset. Overall this worked fine and
> seems useful; if you want to try it:
>
> https://github.com/dlax/psycopg3/tree/pg16/non-blocking-pqcancel
That's great to hear! I'll try to take a closer look at that change tomorrow.
> (The only thing I found slightly inconvenient is the need to convey the
> connection encoding (from PGconn) when handling error message from the
> PGcancelConn.)
Could you expand a bit more on this? And if you have any idea on how
to improve the API with regards to this?
Attachments:
[application/octet-stream] v17-0002-Copy-and-store-addrinfo-in-libpq-owned-private-m.patch (11.6K, ../CAGECzQQUw3UFP8ivMYg0tt3-vDscSZMRuwpABjkUO2m9=JUSTA@mail.gmail.com/2-v17-0002-Copy-and-store-addrinfo-in-libpq-owned-private-m.patch)
download | inline diff:
From fa487867fa7f14b165dc79bc4d0fc9b71c12b5d3 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <[email protected]>
Date: Mon, 27 Mar 2023 11:17:56 +0200
Subject: [PATCH v17 2/5] Copy and store addrinfo in libpq-owned private memory
This refactors libpq to copy addrinfos returned by getaddrinfo to
memory owned by libpq such that future improvements can alter for
example the order of entries.
As a nice side effect of this refactor the mechanism for iteration
over addresses in PQconnectPoll is now identical to its iteration
over hosts.
Author: Jelte Fennema <[email protected]>
Reviewed-by: Aleksander Alekseev <[email protected]>
Reviewed-by: Michael Banck <[email protected]>
Reviewed-by: Andrey Borodin <[email protected]>
Discussion: https://postgr.es/m/PR3PR83MB04768E2FF04818EEB2179949F7A69@PR3PR83MB0476.EURPRD83.prod.outlook.com
---
src/include/libpq/pqcomm.h | 6 ++
src/interfaces/libpq/fe-connect.c | 112 +++++++++++++++++++++---------
src/interfaces/libpq/libpq-int.h | 7 +-
src/tools/pgindent/typedefs.list | 1 +
4 files changed, 92 insertions(+), 34 deletions(-)
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index bff7dd18a23..c85090259d9 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -27,6 +27,12 @@ typedef struct
socklen_t salen;
} SockAddr;
+typedef struct
+{
+ int family;
+ SockAddr addr;
+} AddrInfo;
+
/* Configure the UNIX socket location for the well known port. */
#define UNIXSOCK_PATH(path, port, sockdir) \
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index b71378d94c5..4e798e1672c 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -389,6 +389,7 @@ static bool fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
static void freePGconn(PGconn *conn);
static void closePGconn(PGconn *conn);
static void release_conn_addrinfo(PGconn *conn);
+static int store_conn_addrinfo(PGconn *conn, struct addrinfo *addrlist);
static void sendTerminateConn(PGconn *conn);
static PQconninfoOption *conninfo_init(PQExpBuffer errorMessage);
static PQconninfoOption *parse_connection_string(const char *connstr,
@@ -2295,7 +2296,7 @@ connectDBComplete(PGconn *conn)
time_t finish_time = ((time_t) -1);
int timeout = 0;
int last_whichhost = -2; /* certainly different from whichhost */
- struct addrinfo *last_addr_cur = NULL;
+ int last_whichaddr = -2; /* certainly different from whichaddr */
if (conn == NULL || conn->status == CONNECTION_BAD)
return 0;
@@ -2339,11 +2340,11 @@ connectDBComplete(PGconn *conn)
if (flag != PGRES_POLLING_OK &&
timeout > 0 &&
(conn->whichhost != last_whichhost ||
- conn->addr_cur != last_addr_cur))
+ conn->whichaddr != last_whichaddr))
{
finish_time = time(NULL) + timeout;
last_whichhost = conn->whichhost;
- last_addr_cur = conn->addr_cur;
+ last_whichaddr = conn->whichaddr;
}
/*
@@ -2490,9 +2491,9 @@ keep_going: /* We will come back to here until there is
/* Time to advance to next address, or next host if no more addresses? */
if (conn->try_next_addr)
{
- if (conn->addr_cur && conn->addr_cur->ai_next)
+ if (conn->whichaddr < conn->naddr)
{
- conn->addr_cur = conn->addr_cur->ai_next;
+ conn->whichaddr++;
reset_connection_state_machine = true;
}
else
@@ -2505,6 +2506,7 @@ keep_going: /* We will come back to here until there is
{
pg_conn_host *ch;
struct addrinfo hint;
+ struct addrinfo *addrlist;
int thisport;
int ret;
char portstr[MAXPGPATH];
@@ -2545,7 +2547,7 @@ keep_going: /* We will come back to here until there is
/* Initialize hint structure */
MemSet(&hint, 0, sizeof(hint));
hint.ai_socktype = SOCK_STREAM;
- conn->addrlist_family = hint.ai_family = AF_UNSPEC;
+ hint.ai_family = AF_UNSPEC;
/* Figure out the port number we're going to use. */
if (ch->port == NULL || ch->port[0] == '\0')
@@ -2568,8 +2570,8 @@ keep_going: /* We will come back to here until there is
{
case CHT_HOST_NAME:
ret = pg_getaddrinfo_all(ch->host, portstr, &hint,
- &conn->addrlist);
- if (ret || !conn->addrlist)
+ &addrlist);
+ if (ret || !addrlist)
{
libpq_append_conn_error(conn, "could not translate host name \"%s\" to address: %s",
ch->host, gai_strerror(ret));
@@ -2580,8 +2582,8 @@ keep_going: /* We will come back to here until there is
case CHT_HOST_ADDRESS:
hint.ai_flags = AI_NUMERICHOST;
ret = pg_getaddrinfo_all(ch->hostaddr, portstr, &hint,
- &conn->addrlist);
- if (ret || !conn->addrlist)
+ &addrlist);
+ if (ret || !addrlist)
{
libpq_append_conn_error(conn, "could not parse network address \"%s\": %s",
ch->hostaddr, gai_strerror(ret));
@@ -2590,7 +2592,7 @@ keep_going: /* We will come back to here until there is
break;
case CHT_UNIX_SOCKET:
- conn->addrlist_family = hint.ai_family = AF_UNIX;
+ hint.ai_family = AF_UNIX;
UNIXSOCK_PATH(portstr, thisport, ch->host);
if (strlen(portstr) >= UNIXSOCK_PATH_BUFLEN)
{
@@ -2605,8 +2607,8 @@ keep_going: /* We will come back to here until there is
* name as a Unix-domain socket path.
*/
ret = pg_getaddrinfo_all(NULL, portstr, &hint,
- &conn->addrlist);
- if (ret || !conn->addrlist)
+ &addrlist);
+ if (ret || !addrlist)
{
libpq_append_conn_error(conn, "could not translate Unix-domain socket path \"%s\" to address: %s",
portstr, gai_strerror(ret));
@@ -2615,8 +2617,15 @@ keep_going: /* We will come back to here until there is
break;
}
- /* OK, scan this addrlist for a working server address */
- conn->addr_cur = conn->addrlist;
+ /*
+ * Store a copy of the addrlist in private memory so we can perform
+ * randomization for load balancing.
+ */
+ ret = store_conn_addrinfo(conn, addrlist);
+ pg_freeaddrinfo_all(hint.ai_family, addrlist);
+ if (ret)
+ goto error_return; /* message already logged */
+
reset_connection_state_machine = true;
conn->try_next_host = false;
}
@@ -2673,31 +2682,30 @@ keep_going: /* We will come back to here until there is
{
/*
* Try to initiate a connection to one of the addresses
- * returned by pg_getaddrinfo_all(). conn->addr_cur is the
+ * returned by pg_getaddrinfo_all(). conn->whichaddr is the
* next one to try.
*
* The extra level of braces here is historical. It's not
* worth reindenting this whole switch case to remove 'em.
*/
{
- struct addrinfo *addr_cur = conn->addr_cur;
char host_addr[NI_MAXHOST];
int sock_type;
+ AddrInfo *addr_cur;
/*
* Advance to next possible host, if we've tried all of
* the addresses for the current host.
*/
- if (addr_cur == NULL)
+ if (conn->whichaddr == conn->naddr)
{
conn->try_next_host = true;
goto keep_going;
}
+ addr_cur = &conn->addr[conn->whichaddr];
/* Remember current address for possible use later */
- memcpy(&conn->raddr.addr, addr_cur->ai_addr,
- addr_cur->ai_addrlen);
- conn->raddr.salen = addr_cur->ai_addrlen;
+ memcpy(&conn->raddr, &addr_cur->addr, sizeof(SockAddr));
/*
* Set connip, too. Note we purposely ignore strdup
@@ -2732,7 +2740,7 @@ keep_going: /* We will come back to here until there is
*/
sock_type |= SOCK_NONBLOCK;
#endif
- conn->sock = socket(addr_cur->ai_family, sock_type, 0);
+ conn->sock = socket(addr_cur->family, sock_type, 0);
if (conn->sock == PGINVALID_SOCKET)
{
int errorno = SOCK_ERRNO;
@@ -2743,7 +2751,7 @@ keep_going: /* We will come back to here until there is
* cases where the address list includes both IPv4 and
* IPv6 but kernel only accepts one family.
*/
- if (addr_cur->ai_next != NULL ||
+ if (conn->whichaddr < conn->naddr ||
conn->whichhost + 1 < conn->nconnhost)
{
conn->try_next_addr = true;
@@ -2769,7 +2777,7 @@ keep_going: /* We will come back to here until there is
* TCP sockets, nonblock mode, close-on-exec. Try the
* next address if any of this fails.
*/
- if (addr_cur->ai_family != AF_UNIX)
+ if (addr_cur->family != AF_UNIX)
{
if (!connectNoDelay(conn))
{
@@ -2800,7 +2808,7 @@ keep_going: /* We will come back to here until there is
#endif /* F_SETFD */
#endif
- if (addr_cur->ai_family != AF_UNIX)
+ if (addr_cur->family != AF_UNIX)
{
#ifndef WIN32
int on = 1;
@@ -2892,8 +2900,8 @@ keep_going: /* We will come back to here until there is
* Start/make connection. This should not block, since we
* are in nonblock mode. If it does, well, too bad.
*/
- if (connect(conn->sock, addr_cur->ai_addr,
- addr_cur->ai_addrlen) < 0)
+ if (connect(conn->sock, (struct sockaddr *) &addr_cur->addr.addr,
+ addr_cur->addr.salen) < 0)
{
if (SOCK_ERRNO == EINPROGRESS ||
#ifdef WIN32
@@ -4318,6 +4326,49 @@ freePGconn(PGconn *conn)
free(conn);
}
+/*
+ * store_conn_addrinfo
+ * - copy addrinfo to PGconn object
+ *
+ * Copies the addrinfos from addrlist to the PGconn object such that the
+ * addrinfos can be manipulated by libpq. Returns a positive integer on
+ * failure, otherwise zero.
+ */
+static int
+store_conn_addrinfo(PGconn *conn, struct addrinfo *addrlist)
+{
+ struct addrinfo *ai = addrlist;
+
+ conn->whichaddr = 0;
+
+ conn->naddr = 0;
+ while (ai)
+ {
+ ai = ai->ai_next;
+ conn->naddr++;
+ }
+
+ conn->addr = calloc(conn->naddr, sizeof(AddrInfo));
+ if (conn->addr == NULL)
+ {
+ libpq_append_conn_error(conn, "out of memory");
+ return 1;
+ }
+
+ ai = addrlist;
+ for (int i = 0; i < conn->naddr; i++)
+ {
+ conn->addr[i].family = ai->ai_family;
+
+ memcpy(&conn->addr[i].addr.addr, ai->ai_addr,
+ ai->ai_addrlen);
+ conn->addr[i].addr.salen = ai->ai_addrlen;
+ ai = ai->ai_next;
+ }
+
+ return 0;
+}
+
/*
* release_conn_addrinfo
* - Free any addrinfo list in the PGconn.
@@ -4325,11 +4376,10 @@ freePGconn(PGconn *conn)
static void
release_conn_addrinfo(PGconn *conn)
{
- if (conn->addrlist)
+ if (conn->addr)
{
- pg_freeaddrinfo_all(conn->addrlist_family, conn->addrlist);
- conn->addrlist = NULL;
- conn->addr_cur = NULL; /* for safety */
+ free(conn->addr);
+ conn->addr = NULL;
}
}
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 1ff57044508..760ee3f6912 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -471,9 +471,10 @@ struct pg_conn
PGTargetServerType target_server_type; /* desired session properties */
bool try_next_addr; /* time to advance to next address/host? */
bool try_next_host; /* time to advance to next connhost[]? */
- struct addrinfo *addrlist; /* list of addresses for current connhost */
- struct addrinfo *addr_cur; /* the one currently being tried */
- int addrlist_family; /* needed to know how to free addrlist */
+ int naddr; /* number of addresses returned by getaddrinfo */
+ int whichaddr; /* the address currently being tried */
+ AddrInfo *addr; /* the array of addresses for the currently
+ * tried host */
bool send_appname; /* okay to send application_name? */
/* Miscellaneous stuff */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 0b7bc457671..dfa1e309ee3 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -26,6 +26,7 @@ AcquireSampleRowsFunc
ActionList
ActiveSnapshotElt
AddForeignUpdateTargets_function
+AddrInfo
AffixNode
AffixNodeData
AfterTriggerEvent
--
2.34.1
[application/octet-stream] v17-0005-Start-using-new-libpq-cancel-APIs.patch (9.2K, ../CAGECzQQUw3UFP8ivMYg0tt3-vDscSZMRuwpABjkUO2m9=JUSTA@mail.gmail.com/3-v17-0005-Start-using-new-libpq-cancel-APIs.patch)
download | inline diff:
From 4157ff7eba87420258ac750eb5654ad3447b2576 Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Wed, 25 Jan 2023 13:32:15 +0100
Subject: [PATCH v17 5/5] Start using new libpq cancel APIs
A previous commit introduced new APIs to libpq for cancelling queries.
This replaces the usage of the old APIs in the codebase with these newer
ones.
---
contrib/dblink/dblink.c | 30 ++++--
contrib/postgres_fdw/connection.c | 99 ++++++++++++++++---
.../postgres_fdw/expected/postgres_fdw.out | 15 +++
contrib/postgres_fdw/sql/postgres_fdw.sql | 7 ++
src/fe_utils/connect_utils.c | 10 +-
src/test/isolation/isolationtester.c | 29 +++---
6 files changed, 139 insertions(+), 51 deletions(-)
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 78a8bcee6e3..e139f66e116 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -1326,22 +1326,32 @@ PG_FUNCTION_INFO_V1(dblink_cancel_query);
Datum
dblink_cancel_query(PG_FUNCTION_ARGS)
{
- int res;
PGconn *conn;
- PGcancel *cancel;
- char errbuf[256];
+ PGcancelConn *cancelConn;
+ char *msg;
dblink_init();
conn = dblink_get_named_conn(text_to_cstring(PG_GETARG_TEXT_PP(0)));
- cancel = PQgetCancel(conn);
+ cancelConn = PQcancelSend(conn);
- res = PQcancel(cancel, errbuf, 256);
- PQfreeCancel(cancel);
+ PG_TRY();
+ {
+ if (PQcancelStatus(cancelConn) == CONNECTION_BAD)
+ {
+ msg = pchomp(PQcancelErrorMessage(cancelConn));
+ }
+ else
+ {
+ msg = "OK";
+ }
+ }
+ PG_FINALLY();
+ {
+ PQcancelFinish(cancelConn);
+ }
+ PG_END_TRY();
- if (res == 1)
- PG_RETURN_TEXT_P(cstring_to_text("OK"));
- else
- PG_RETURN_TEXT_P(cstring_to_text(errbuf));
+ PG_RETURN_TEXT_P(cstring_to_text(msg));
}
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 8eb9194506c..3f9a408a6af 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -1233,35 +1233,104 @@ pgfdw_reset_xact_state(ConnCacheEntry *entry, bool toplevel)
static bool
pgfdw_cancel_query(PGconn *conn)
{
- PGcancel *cancel;
- char errbuf[256];
PGresult *result = NULL;
- TimestampTz endtime;
- bool timed_out;
/*
* If it takes too long to cancel the query and discard the result, assume
* the connection is dead.
*/
- endtime = TimestampTzPlusMilliseconds(GetCurrentTimestamp(), 30000);
+ TimestampTz endtime = TimestampTzPlusMilliseconds(GetCurrentTimestamp(), 30000);
+ bool timed_out = false;
+ bool failed = false;
+ PGcancelConn *cancel_conn = PQcancelConn(conn);
- /*
- * Issue cancel request. Unfortunately, there's no good way to limit the
- * amount of time that we might block inside PQgetCancel().
- */
- if ((cancel = PQgetCancel(conn)))
+
+ if (PQcancelStatus(cancel_conn) == CONNECTION_BAD)
{
- if (!PQcancel(cancel, errbuf, sizeof(errbuf)))
+ PG_TRY();
{
ereport(WARNING,
(errcode(ERRCODE_CONNECTION_FAILURE),
errmsg("could not send cancel request: %s",
- errbuf)));
- PQfreeCancel(cancel);
- return false;
+ pchomp(PQcancelErrorMessage(cancel_conn)))));
+ }
+ PG_FINALLY();
+ {
+ PQcancelFinish(cancel_conn);
+ }
+ PG_END_TRY();
+ return false;
+ }
+
+ /* In what follows, do not leak any PGcancelConn on an error. */
+ PG_TRY();
+ {
+ while (true)
+ {
+ TimestampTz now = GetCurrentTimestamp();
+ long cur_timeout;
+ PostgresPollingStatusType pollres = PQcancelPoll(cancel_conn);
+ int waitEvents = WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH;
+
+ if (pollres == PGRES_POLLING_OK)
+ {
+ break;
+ }
+
+ /* If timeout has expired, give up, else get sleep time. */
+ cur_timeout = TimestampDifferenceMilliseconds(now, endtime);
+ if (cur_timeout <= 0)
+ {
+ timed_out = true;
+ failed = true;
+ goto exit;
+ }
+
+ switch (pollres)
+ {
+ case PGRES_POLLING_READING:
+ waitEvents |= WL_SOCKET_READABLE;
+ break;
+ case PGRES_POLLING_WRITING:
+ waitEvents |= WL_SOCKET_WRITEABLE;
+ break;
+ default:
+ failed = true;
+ goto exit;
+ }
+
+ /* Sleep until there's something to do */
+ WaitLatchOrSocket(MyLatch, waitEvents, PQcancelSocket(cancel_conn),
+ cur_timeout, PG_WAIT_EXTENSION);
+ ResetLatch(MyLatch);
+
+ CHECK_FOR_INTERRUPTS();
+ }
+exit: ;
+ if (failed)
+ {
+ if (timed_out)
+ {
+ ereport(WARNING,
+ (errmsg("could not cancel request due to timeout")));
+ }
+ else
+ {
+ ereport(WARNING,
+ (errcode(ERRCODE_CONNECTION_FAILURE),
+ errmsg("could not send cancel request: %s",
+ pchomp(PQcancelErrorMessage(cancel_conn)))));
+ }
}
- PQfreeCancel(cancel);
}
+ PG_FINALLY();
+ {
+ PQcancelFinish(cancel_conn);
+ }
+ PG_END_TRY();
+
+ if (failed)
+ return false;
/* Get and discard the result of the query. */
if (pgfdw_get_cleanup_result(conn, endtime, &result, &timed_out))
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 04a3ef450cf..064c3103a5e 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -2688,6 +2688,21 @@ SELECT t1.c1, t2.c2 FROM v4 t1 LEFT JOIN ft5 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c
(10 rows)
ALTER VIEW v4 OWNER TO regress_view_owner;
+-- Make sure this big CROSS JOIN query is pushed down
+EXPLAIN (VERBOSE, COSTS OFF) SELECT count(*) FROM ft1 CROSS JOIN ft2 CROSS JOIN ft4 CROSS JOIN ft5;
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan
+ Output: (count(*))
+ Relations: Aggregate on ((((public.ft1) INNER JOIN (public.ft2)) INNER JOIN (public.ft4)) INNER JOIN (public.ft5))
+ Remote SQL: SELECT count(*) FROM ((("S 1"."T 1" r1 INNER JOIN "S 1"."T 1" r2 ON (TRUE)) INNER JOIN "S 1"."T 3" r4 ON (TRUE)) INNER JOIN "S 1"."T 4" r6 ON (TRUE))
+(4 rows)
+
+-- Make sure query cancellation works
+SET statement_timeout = '10ms';
+select count(*) from ft1 CROSS JOIN ft2 CROSS JOIN ft4 CROSS JOIN ft5; -- this takes very long
+ERROR: canceling statement due to statement timeout
+RESET statement_timeout;
-- cleanup
DROP OWNED BY regress_view_owner;
DROP ROLE regress_view_owner;
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 4f3088c03ea..640958df136 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -713,6 +713,13 @@ SELECT t1.c1, t2.c2 FROM v4 t1 LEFT JOIN ft5 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c
SELECT t1.c1, t2.c2 FROM v4 t1 LEFT JOIN ft5 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c1, t2.c1 OFFSET 10 LIMIT 10;
ALTER VIEW v4 OWNER TO regress_view_owner;
+-- Make sure this big CROSS JOIN query is pushed down
+EXPLAIN (VERBOSE, COSTS OFF) SELECT count(*) FROM ft1 CROSS JOIN ft2 CROSS JOIN ft4 CROSS JOIN ft5;
+-- Make sure query cancellation works
+SET statement_timeout = '10ms';
+select count(*) from ft1 CROSS JOIN ft2 CROSS JOIN ft4 CROSS JOIN ft5; -- this takes very long
+RESET statement_timeout;
+
-- cleanup
DROP OWNED BY regress_view_owner;
DROP ROLE regress_view_owner;
diff --git a/src/fe_utils/connect_utils.c b/src/fe_utils/connect_utils.c
index 7a1edea7c8c..b32448c0103 100644
--- a/src/fe_utils/connect_utils.c
+++ b/src/fe_utils/connect_utils.c
@@ -157,19 +157,11 @@ connectMaintenanceDatabase(ConnParams *cparams,
void
disconnectDatabase(PGconn *conn)
{
- char errbuf[256];
-
Assert(conn != NULL);
if (PQtransactionStatus(conn) == PQTRANS_ACTIVE)
{
- PGcancel *cancel;
-
- if ((cancel = PQgetCancel(conn)))
- {
- (void) PQcancel(cancel, errbuf, sizeof(errbuf));
- PQfreeCancel(cancel);
- }
+ PQcancelFinish(PQcancelSend(conn));
}
PQfinish(conn);
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index 0a66235153a..3781f7982b2 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -946,26 +946,21 @@ try_complete_step(TestSpec *testspec, PermutationStep *pstep, int flags)
*/
if (td > max_step_wait && !canceled)
{
- PGcancel *cancel = PQgetCancel(conn);
+ PGcancelConn *cancel_conn = PQcancelSend(conn);
- if (cancel != NULL)
+ if (PQcancelStatus(cancel_conn) == CONNECTION_OK)
{
- char buf[256];
-
- if (PQcancel(cancel, buf, sizeof(buf)))
- {
- /*
- * print to stdout not stderr, as this should appear
- * in the test case's results
- */
- printf("isolationtester: canceling step %s after %d seconds\n",
- step->name, (int) (td / USECS_PER_SEC));
- canceled = true;
- }
- else
- fprintf(stderr, "PQcancel failed: %s\n", buf);
- PQfreeCancel(cancel);
+ /*
+ * print to stdout not stderr, as this should appear in
+ * the test case's results
+ */
+ printf("isolationtester: canceling step %s after %d seconds\n",
+ step->name, (int) (td / USECS_PER_SEC));
+ canceled = true;
}
+ else
+ fprintf(stderr, "PQcancel failed: %s\n", PQcancelErrorMessage(cancel_conn));
+ PQcancelFinish(cancel_conn);
}
/*
--
2.34.1
[application/octet-stream] v17-0003-Return-2-from-pqReadData-on-EOF.patch (4.3K, ../CAGECzQQUw3UFP8ivMYg0tt3-vDscSZMRuwpABjkUO2m9=JUSTA@mail.gmail.com/4-v17-0003-Return-2-from-pqReadData-on-EOF.patch)
download | inline diff:
From 55513ba5245ce92afba84fc1a0951e34f45928c8 Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Thu, 26 Jan 2023 12:24:38 +0100
Subject: [PATCH v17 3/5] Return -2 from pqReadData on EOF
This patch changes pqReadData to return -2 when a connection is cleanly
closed by the other side. For most of the Postgres protocol this is
considered an error, because the client will close the connection
instead of the server. But for Postgres its cancellation protocol
the distinction between errors and clean connection closure is
important, because clean connection closure is the way for the server to
signal that the cancellation was handled.
This patch is in preparation for a follow-up patch where pqReadData is
used for the cancellation protocol implementation.
No existing callsites of pqReadData or any of its internal functions
need to be updated as all of them check if the result is less than 0
instead a strict comparison against -1.
---
src/interfaces/libpq/fe-misc.c | 15 +++++++++++----
src/interfaces/libpq/fe-secure-openssl.c | 2 +-
src/interfaces/libpq/fe-secure.c | 6 ++++++
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 660cdec93c9..2d49188d910 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -556,8 +556,11 @@ pqPutMsgEnd(PGconn *conn)
* Possible return values:
* 1: successfully loaded at least one more byte
* 0: no data is presently available, but no error detected
- * -1: error detected (including EOF = connection closure);
+ * -1: error detected (excluding EOF = clean connection closure);
* conn->errorMessage set
+ * -2: EOF detected, connection is closed cleanly by other side;
+ * conn->errorMessage set
+ *
* NOTE: callers must not assume that pointers or indexes into conn->inBuffer
* remain valid across this call!
* ----------
@@ -639,7 +642,7 @@ retry3:
default:
/* pqsecure_read set the error message for us */
- return -1;
+ return nread;
}
}
if (nread > 0)
@@ -734,7 +737,7 @@ retry4:
default:
/* pqsecure_read set the error message for us */
- return -1;
+ return nread;
}
}
if (nread > 0)
@@ -751,13 +754,17 @@ definitelyEOF:
libpq_append_conn_error(conn, "server closed the connection unexpectedly\n"
"\tThis probably means the server terminated abnormally\n"
"\tbefore or while processing the request.");
+ /* Do *not* drop any already-read data; caller still wants it */
+ pqDropConnection(conn, false);
+ conn->status = CONNECTION_BAD; /* No more connection to backend */
+ return -2;
/* Come here if lower-level code already set a suitable errorMessage */
definitelyFailed:
/* Do *not* drop any already-read data; caller still wants it */
pqDropConnection(conn, false);
conn->status = CONNECTION_BAD; /* No more connection to backend */
- return -1;
+ return nread < 0 ? nread : -1;
}
/*
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 61f8a5c9c6c..351161bd0f9 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -248,7 +248,7 @@ rloop:
*/
libpq_append_conn_error(conn, "SSL connection has been closed unexpectedly");
result_errno = ECONNRESET;
- n = -1;
+ n = -2;
break;
default:
libpq_append_conn_error(conn, "unrecognized SSL error code: %d", err);
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 8069e381424..20265dcb317 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -199,6 +199,12 @@ pqsecure_close(PGconn *conn)
* On failure, this function is responsible for appending a suitable message
* to conn->errorMessage. The caller must still inspect errno, but only
* to determine whether to continue/retry after error.
+ *
+ * Returns -1 in case of failures, except in the case of where a failure means
+ * that there was a clean connection closure, in those cases -2 is returned.
+ * Currently only the TLS implementation of pqsecure_read ever returns -2. For
+ * the other implementations a clean connection closure is detected in
+ * pqReadData instead.
*/
ssize_t
pqsecure_read(PGconn *conn, void *ptr, size_t len)
--
2.34.1
[application/octet-stream] v17-0004-Add-non-blocking-version-of-PQcancel.patch (44.6K, ../CAGECzQQUw3UFP8ivMYg0tt3-vDscSZMRuwpABjkUO2m9=JUSTA@mail.gmail.com/5-v17-0004-Add-non-blocking-version-of-PQcancel.patch)
download | inline diff:
From 3d9f70f5847f184696dbd4380b79c4f853285621 Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Wed, 12 Jan 2022 09:52:05 +0100
Subject: [PATCH v17 4/5] Add non-blocking version of PQcancel
This patch makes the following changes in libpq:
1. Add a new PQcancelSend function, which sends cancellation requests
using the regular connection establishment code. This makes sure
that cancel requests support and use all connection options
including encryption.
2. Add a new PQcancelConn function which allows sending cancellation in
a non-blocking way by using it together with the newly added
PQcancelPoll and PQcancelSocket.
3. Use these two new cancellation APIs everywhere in the codebase where
signal-safety is not a necessity.
The existing PQcancel API is using blocking IO. This makes PQcancel
impossible to use in an event loop based codebase, without blocking the
event loop until the call returns. PQcancelConn can now be used instead,
to have a non-blocking way of sending cancel requests. The postgres_fdw
cancellation code has been modified to make use of this.
This patch also includes a test for all of libpq cancellation APIs. The
test can be easily run like this:
cd src/test/modules/libpq_pipeline
make && ./libpq_pipeline cancel
---
doc/src/sgml/libpq.sgml | 289 ++++++++++-
src/interfaces/libpq/exports.txt | 8 +
src/interfaces/libpq/fe-connect.c | 452 +++++++++++++++++-
src/interfaces/libpq/libpq-fe.h | 25 +-
src/interfaces/libpq/libpq-int.h | 9 +
.../modules/libpq_pipeline/libpq_pipeline.c | 265 +++++++++-
6 files changed, 996 insertions(+), 52 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 8579dcac952..aa404c4d155 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -265,7 +265,7 @@ PGconn *PQsetdb(char *pghost,
<varlistentry id="libpq-PQconnectStartParams">
<term><function>PQconnectStartParams</function><indexterm><primary>PQconnectStartParams</primary></indexterm></term>
<term><function>PQconnectStart</function><indexterm><primary>PQconnectStart</primary></indexterm></term>
- <term><function>PQconnectPoll</function><indexterm><primary>PQconnectPoll</primary></indexterm></term>
+ <term id="libpq-PQconnectPoll"><function>PQconnectPoll</function><indexterm><primary>PQconnectPoll</primary></indexterm></term>
<listitem>
<para>
<indexterm><primary>nonblocking connection</primary></indexterm>
@@ -5060,7 +5060,7 @@ int PQisBusy(PGconn *conn);
<xref linkend="libpq-PQsendQuery"/>/<xref linkend="libpq-PQgetResult"/>
can also attempt to cancel a command that is still being processed
by the server; see <xref linkend="libpq-cancel"/>. But regardless of
- the return value of <xref linkend="libpq-PQcancel"/>, the application
+ the return value of <xref linkend="libpq-PQcancelSend"/>, the application
must continue with the normal result-reading sequence using
<xref linkend="libpq-PQgetResult"/>. A successful cancellation will
simply cause the command to terminate sooner than it would have
@@ -5779,13 +5779,232 @@ int PQsetSingleRowMode(PGconn *conn);
this section.
<variablelist>
+ <varlistentry id="libpq-PQcancelSend">
+ <term><function>PQcancelSend</function><indexterm><primary>PQcancelSend</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ Requests that the server abandons processing of the current command.
+<synopsis>
+PGcancelConn *PQcancelSend(PGconn *conn);
+</synopsis>
+ </para>
+
+ <para>
+ This request is made over a connection that uses the same connection
+ options as the the original <structname>PGconn</structname>. So when the
+ original connection is encrypted (using TLS or GSS), the connection for
+ the cancel request is encrypted in the same way. Any connection options
+ that are only used during authentication or after authentication of
+ the client are ignored though, because cancellation requests do not
+ require authentication and the connection is closed right after the
+ cancellation request is submitted.
+ </para>
+
+ <para>
+ This function returns a <structname>PGcancelConn</structname>
+ object. <xref linkend="libpq-PQcancelStatus"/> can be used to check
+ if any error occured while sending the cancellation request. If
+ <xref linkend="libpq-PQcancelStatus"/> returns <symbol>CONNECTION_OK</symbol>
+ the request was sent successfully, but if it returns <symbol>CONNECTION_BAD</symbol>
+ an error occured. If an error occured the error message can be retrieved using
+ <xref linkend="libpq-PQcancelErrorMessage"/>.
+ </para>
+
+ <para>
+ Successful dispatch of the cancellation is no guarantee that the request
+ will have any effect, however. If the cancellation is effective, the
+ command being cancelled will terminate early and return an error result.
+ If the cancellation fails (say, because the server was already done
+ processing the command), then there will be no visible result at all.
+ </para>
+
+ <para>
+ Note that when <function>PQcancelSend</function> returns a non-null
+ pointer, you must call <xref linkend="libpq-PQcancelFinish"/> when you
+ are finished with it, in order to dispose of the structure and any
+ associated memory blocks. This must be done even if the cancel request
+ failed.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQcancelConn">
+ <term><function>PQcancelConn</function><indexterm><primary>PQcancelConn</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ A version of <xref linkend="libpq-PQcancelSend"/> that can be used to
+ send cancellation requests in a non-blocking manner.
+<synopsis>
+PGcancelConn *PQcancelConn(PGconn *conn);
+</synopsis>
+ </para>
+
+ <para>
+ <xref linkend="libpq-PQcancelConn"/> creates a
+ <structname>PGcancelConn</structname><indexterm><primary>PGcancelConn</primary></indexterm>
+ object, but it won't instantly start sending a cancel request over this
+ connection, unlike <xref linkend="libpq-PQcancelSend"/>.
+ The return value should still be passed to <xref linkend="libpq-PQcancelStatus"/>
+ though, to check if the <structname>PGcancelConn</structname> object was
+ created successfully. The <structname>PGcancelConn</structname> object
+ is an opaque structure that is not meant to be accessed directly by the
+ application. This <structname>PGcancelConn</structname> object can be
+ used to cancel the query that's running on the original connection in a
+ thread-safe and non-blocking way.
+ </para>
+
+ <para>
+ Note that when <function>PQcancelConn</function> returns a non-null
+ pointer, you must call <xref linkend="libpq-PQcancelFinish"/> when you
+ are finished with it, in order to dispose of the structure and any
+ associated memory blocks. This must be done even if the cancel request
+ failed or was abandoned.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQcancelStatus">
+ <term><function>PQcancelStatus</function><indexterm><primary>PQcancelStatus</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ A version of <xref linkend="libpq-PQstatus"/> that can be used for
+ cancellation connections.
+<synopsis>
+ConnStatusType PQcancelStatus(const PGcancelConn *conn);
+</synopsis>
+ </para>
+ <para>
+ In addition to all the statuses that a <structname>PGconn</structname>
+ can have, this connection can have one additional status:
+
+ <variablelist>
+ <varlistentry id="libpq-connection-starting">
+ <term><symbol>CONNECTION_STARTING</symbol></term>
+ <listitem>
+ <para>
+ Waiting for the first call to <xref linkend="libpq-PQcancelPoll"/>,
+ to actually open the socket. This is the connection state right after
+ calling <xref linkend="libpq-PQcancelConn"/>. No connection to the
+ server has been initiated yet at this point. To actually start
+ sending the cancel request use <xref linkend="libpq-PQcancelPoll"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </para>
+
+ <para>
+ One final note about the returned statuses is that
+ <symbol>CONNECTION_OK</symbol> has a slightly different meaning for a
+ <structname>PGcancelConn</structname> than what it has for a
+ <structname>PGconn</structname>. When <xref linkend="libpq-PQcancelStatus"/>
+ returns <symbol>CONNECTION_OK</symbol> for a <structname>PGcancelConn</structname>
+ it means that that the dispatch of the cancel request has completed (although
+ this is no promise that the query was actually cancelled) and that the
+ connection is now closed. While a <symbol>CONNECTION_OK</symbol> result
+ for <structname>PGconn</structname> means that queries can be sent over
+ the connection.
+ </para>
+
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQcancelSocket">
+ <term><function>PQcancelSocket</function><indexterm><primary>PQcancelSocket</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ A version of <xref linkend="libpq-PQsocket"/> that can be used for
+ cancellation connections.
+<synopsis>
+int PQcancelSocket(PGcancelConn *conn);
+</synopsis>
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQcancelPoll">
+ <term><function>PQcancelPoll</function><indexterm><primary>PQcancelPoll</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ A version of <xref linkend="libpq-PQconnectPoll"/> that can be used for
+ cancellation connections.
+<synopsis>
+PostgresPollingStatusType PQcancelPoll(PGcancelConn *conn);
+</synopsis>
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQcancelErrorMessage">
+ <term><function>PQcancelErrorMessage</function><indexterm><primary>PQcancelErrorMessage</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ A version of <xref linkend="libpq-PQerrorMessage"/> that can be used for
+ cancellation connections.
+<synopsis>
+char *PQcancelErrorMessage(const PGcancelConn *conn);
+</synopsis>
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQcancelFinish">
+ <term><function>PQcancelFinish</function><indexterm><primary>PQcancelFinish</primary></indexterm></term>
+ <listitem>
+ <para>
+ Closes the cancel connection (if it did not finish sending the cancel
+ request yet). Also frees memory used by the <structname>PGcancelConn</structname>
+ object.
+<synopsis>
+void PQcancelFinish(PGcancelConn *conn);
+</synopsis>
+ </para>
+
+ <para>
+ Note that even if the cancel attempt fails (as
+ indicated by <xref linkend="libpq-PQcancelStatus"/>), the application should call <xref linkend="libpq-PQcancelFinish"/>
+ to free the memory used by the <structname>PGcancelConn</structname> object.
+ The <structname>PGcancelConn</structname> pointer must not be used again after
+ <xref linkend="libpq-PQcancelFinish"/> has been called.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="libpq-PQcancelReset">
+ <term><function>PQcancelReset</function><indexterm><primary>PQcancelReset</primary></indexterm></term>
+ <listitem>
+ <para>
+ Resets the <symbol>PGcancelConn</symbol> so it can be reused for a new
+ cancel connection.
+<synopsis>
+void PQcancelReset(PGcancelConn *conn);
+</synopsis>
+ </para>
+
+ <para>
+ If the <symbol>PGcancelConn</symbol> is currently used to send a cancel
+ request, then this connection is closed. It will then prepare the
+ <symbol>PGcancelConn</symbol> object such that it can be used to send a
+ new cancel request. This can be used to create one <symbol>PGcancelConn</symbol>
+ for a <symbol>PGconn</symbol> and reuse that multiple times throughout
+ the lifetime of the original <symbol>PGconn</symbol>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-PQgetCancel">
<term><function>PQgetCancel</function><indexterm><primary>PQgetCancel</primary></indexterm></term>
<listitem>
<para>
Creates a data structure containing the information needed to cancel
- a command issued through a particular database connection.
+ a command using <xref linkend="libpq-PQcancel"/>.
<synopsis>
PGcancel *PQgetCancel(PGconn *conn);
</synopsis>
@@ -5827,14 +6046,28 @@ void PQfreeCancel(PGcancel *cancel);
<listitem>
<para>
- Requests that the server abandon processing of the current command.
+ An insecure version of <xref linkend="libpq-PQcancelSend"/>, but one
+ that can be used safely from within a signal handler.
<synopsis>
int PQcancel(PGcancel *cancel, char *errbuf, int errbufsize);
</synopsis>
</para>
<para>
- The return value is 1 if the cancel request was successfully
+ <xref linkend="libpq-PQcancel"/> should only be used if it's necessary
+ to cancel a query from a signal-handler. If signal-safety is not needed,
+ <xref linkend="libpq-PQcancelSend"/> should be used to cancel the query
+ instead. <xref linkend="libpq-PQcancel"/> can be safely invoked from a
+ signal handler, if the <parameter>errbuf</parameter> is a local variable
+ in the signal handler. The <structname>PGcancel</structname> object is
+ read-only as far as <xref linkend="libpq-PQcancel"/> is concerned, so it
+ can also be invoked from a thread that is separate from the one
+ manipulating the <structname>PGconn</structname> object.
+ </para>
+
+ <para>
+ The return value of <xref linkend="libpq-PQcancel"/>
+ is 1 if the cancel request was successfully
dispatched and 0 if not. If not, <parameter>errbuf</parameter> is filled
with an explanatory error message. <parameter>errbuf</parameter>
must be a char array of size <parameter>errbufsize</parameter> (the
@@ -5842,21 +6075,22 @@ int PQcancel(PGcancel *cancel, char *errbuf, int errbufsize);
</para>
<para>
- Successful dispatch is no guarantee that the request will have
- any effect, however. If the cancellation is effective, the current
- command will terminate early and return an error result. If the
- cancellation fails (say, because the server was already done
- processing the command), then there will be no visible result at
- all.
- </para>
-
- <para>
- <xref linkend="libpq-PQcancel"/> can safely be invoked from a signal
- handler, if the <parameter>errbuf</parameter> is a local variable in the
- signal handler. The <structname>PGcancel</structname> object is read-only
- as far as <xref linkend="libpq-PQcancel"/> is concerned, so it can
- also be invoked from a thread that is separate from the one
- manipulating the <structname>PGconn</structname> object.
+ To achieve signal-safety, some concessions needed to be made in the
+ implementation of <xref linkend="libpq-PQcancel"/>. Not all connection
+ options of the original connection are used when establishing a
+ connection for the cancellation request. This function connects to
+ postgres on the same address and port as the original connection. The
+ only connection options that are honored during this connection are
+ <varname>keepalives</varname>,
+ <varname>keepalives_idle</varname>,
+ <varname>keepalives_interval</varname>,
+ <varname>keepalives_count</varname>, and
+ <varname>tcp_user_timeout</varname>.
+ So, for example
+ <varname>connect_timeout</varname>,
+ <varname>gssencmode</varname>, and
+ <varname>sslmode</varname> are ignored. <emphasis>This means the connection
+ for the cancel request is never encrypted using TLS or GSS</emphasis>.
</para>
</listitem>
</varlistentry>
@@ -5868,13 +6102,22 @@ int PQcancel(PGcancel *cancel, char *errbuf, int errbufsize);
<listitem>
<para>
- <xref linkend="libpq-PQrequestCancel"/> is a deprecated variant of
- <xref linkend="libpq-PQcancel"/>.
+ <xref linkend="libpq-PQrequestCancel"/> is a deprecated and insecure
+ variant of <xref linkend="libpq-PQcancelSend"/>.
<synopsis>
int PQrequestCancel(PGconn *conn);
</synopsis>
</para>
+ <para>
+ <xref linkend="libpq-PQrequestCancel"/> only exists because of backwards
+ compatibility reasons. <xref linkend="libpq-PQcancelSend"/> should be
+ used instead, to avoid the security and thread-safety issues that this
+ function has. This function has the same security issues as
+ <xref linkend="libpq-PQcancel"/>, but without the benefit of being
+ signal-safe.
+ </para>
+
<para>
Requests that the server abandon processing of the current
command. It operates directly on the
@@ -9043,7 +9286,7 @@ int PQisthreadsafe();
The deprecated functions <xref linkend="libpq-PQrequestCancel"/> and
<xref linkend="libpq-PQoidStatus"/> are not thread-safe and should not be
used in multithread programs. <xref linkend="libpq-PQrequestCancel"/>
- can be replaced by <xref linkend="libpq-PQcancel"/>.
+ can be replaced by <xref linkend="libpq-PQcancelSend"/>.
<xref linkend="libpq-PQoidStatus"/> can be replaced by
<xref linkend="libpq-PQoidValue"/>.
</para>
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index e8bcc883709..f56e8c185c4 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -186,3 +186,11 @@ PQpipelineStatus 183
PQsetTraceFlags 184
PQmblenBounded 185
PQsendFlushRequest 186
+PQcancelSend 187
+PQcancelConn 188
+PQcancelPoll 189
+PQcancelStatus 190
+PQcancelSocket 191
+PQcancelErrorMessage 192
+PQcancelReset 193
+PQcancelFinish 194
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 4e798e1672c..8c89a892723 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -386,8 +386,10 @@ static PGPing internal_ping(PGconn *conn);
static PGconn *makeEmptyPGconn(void);
static void pqFreeCommandQueue(PGcmdQueueEntry *queue);
static bool fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
+static bool copyPGconn(PGconn *srcConn, PGconn *dstConn);
static void freePGconn(PGconn *conn);
static void closePGconn(PGconn *conn);
+static void release_conn_hosts(PGconn *conn);
static void release_conn_addrinfo(PGconn *conn);
static int store_conn_addrinfo(PGconn *conn, struct addrinfo *addrlist);
static void sendTerminateConn(PGconn *conn);
@@ -612,8 +614,17 @@ pqDropServerData(PGconn *conn)
conn->write_failed = false;
free(conn->write_err_msg);
conn->write_err_msg = NULL;
- conn->be_pid = 0;
- conn->be_key = 0;
+
+ /*
+ * Cancel connections should save their be_pid and be_key across
+ * PQcancelReset invocations. Otherwise they would not have access to the
+ * secret token of the connection they are supposed to cancel anymore.
+ */
+ if (!conn->cancelRequest)
+ {
+ conn->be_pid = 0;
+ conn->be_key = 0;
+ }
}
@@ -744,6 +755,113 @@ PQping(const char *conninfo)
return ret;
}
+/*
+ * PQcancelConn
+ *
+ * Asynchronously cancel a query on the given connection. This requires polling
+ * the returned PGcancelConn to actually complete the cancellation of the
+ * query.
+ */
+PGcancelConn *
+PQcancelConn(PGconn *conn)
+{
+ PGconn *cancelConn = makeEmptyPGconn();
+ pg_conn_host originalHost;
+
+ if (cancelConn == NULL)
+ return NULL;
+
+ /* Check we have an open connection */
+ if (!conn)
+ {
+ libpq_append_conn_error(cancelConn, "passed connection was NULL");
+ return (PGcancelConn *) cancelConn;
+ }
+
+ if (conn->sock == PGINVALID_SOCKET)
+ {
+ libpq_append_conn_error(cancelConn, "passed connection is not open");
+ return (PGcancelConn *) cancelConn;
+ }
+
+
+ /*
+ * Indicate that this connection is used to send a cancellation
+ */
+ cancelConn->cancelRequest = true;
+
+ if (!copyPGconn(conn, cancelConn))
+ return (PGcancelConn *) cancelConn;
+
+ /*
+ * Compute derived options
+ */
+ if (!connectOptions2(cancelConn))
+ return (PGcancelConn *) cancelConn;
+
+ /*
+ * Copy cancelation token data from the original connnection
+ */
+ cancelConn->be_pid = conn->be_pid;
+ cancelConn->be_key = conn->be_key;
+
+ /*
+ * Cancel requests should not iterate over all possible hosts. The request
+ * needs to be sent to the exact host and address that the original
+ * connection used. So we manually create the host and address arrays
+ * with a single element after freeing the host array that we generated
+ * from the connection options.
+ */
+ release_conn_hosts(cancelConn);
+ cancelConn->nconnhost = 1;
+ cancelConn->naddr = 1;
+
+ cancelConn->connhost = calloc(cancelConn->nconnhost, sizeof(pg_conn_host));
+ if (!cancelConn->connhost)
+ goto oom_error;
+
+ originalHost = conn->connhost[conn->whichhost];
+ if (originalHost.host)
+ {
+ cancelConn->connhost[0].host = strdup(originalHost.host);
+ if (!cancelConn->connhost[0].host)
+ goto oom_error;
+ }
+ if (originalHost.hostaddr)
+ {
+ cancelConn->connhost[0].hostaddr = strdup(originalHost.hostaddr);
+ if (!cancelConn->connhost[0].hostaddr)
+ goto oom_error;
+ }
+ if (originalHost.port)
+ {
+ cancelConn->connhost[0].port = strdup(originalHost.port);
+ if (!cancelConn->connhost[0].port)
+ goto oom_error;
+ }
+ if (originalHost.password)
+ {
+ cancelConn->connhost[0].password = strdup(originalHost.password);
+ if (!cancelConn->connhost[0].password)
+ goto oom_error;
+ }
+
+ cancelConn->addr = calloc(cancelConn->naddr, sizeof(AddrInfo));
+ if (!cancelConn->connhost)
+ goto oom_error;
+
+ cancelConn->addr[0].addr = conn->raddr;
+ cancelConn->addr[0].family = conn->raddr.addr.ss_family;
+
+ cancelConn->status = CONNECTION_STARTING;
+ return (PGcancelConn *) cancelConn;
+
+oom_error:
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(cancelConn, "out of memory");
+ return (PGcancelConn *) cancelConn;
+}
+
/*
* PQconnectStartParams
*
@@ -919,6 +1037,45 @@ fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
return true;
}
+/*
+ * Copy over option values from srcConn to dstConn
+ *
+ * Don't put anything cute here --- intelligence should be in
+ * connectOptions2 ...
+ *
+ * Returns true on success. On failure, returns false and sets error message of
+ * dstConn.
+ */
+static bool
+copyPGconn(PGconn *srcConn, PGconn *dstConn)
+{
+ const internalPQconninfoOption *option;
+
+ /* copy over connection options */
+ for (option = PQconninfoOptions; option->keyword; option++)
+ {
+ if (option->connofs >= 0)
+ {
+ const char **tmp = (const char **) ((char *) srcConn + option->connofs);
+
+ if (*tmp)
+ {
+ char **dstConnmember = (char **) ((char *) dstConn + option->connofs);
+
+ if (*dstConnmember)
+ free(*dstConnmember);
+ *dstConnmember = strdup(*tmp);
+ if (*dstConnmember == NULL)
+ {
+ libpq_append_conn_error(dstConn, "out of memory");
+ return false;
+ }
+ }
+ }
+ }
+ return true;
+}
+
/*
* connectOptions1
*
@@ -2249,10 +2406,18 @@ connectDBStart(PGconn *conn)
* Set up to try to connect to the first host. (Setting whichhost = -1 is
* a bit of a cheat, but PQconnectPoll will advance it to 0 before
* anything else looks at it.)
+ *
+ * Cancel requests are special though, they should only try one host and
+ * address. These fields have already set up in PQcancelConn. So leave
+ * these fields alone for cancel requests.
*/
- conn->whichhost = -1;
- conn->try_next_addr = false;
- conn->try_next_host = true;
+ if (!conn->cancelRequest)
+ {
+ conn->whichhost = -1;
+ conn->try_next_host = true;
+ conn->try_next_addr = false;
+ }
+
conn->status = CONNECTION_NEEDED;
/* Also reset the target_server_type state if needed */
@@ -2394,7 +2559,10 @@ connectDBComplete(PGconn *conn)
/*
* Now try to advance the state machine.
*/
- flag = PQconnectPoll(conn);
+ if (conn->cancelRequest)
+ flag = PQcancelPoll((PGcancelConn *) conn);
+ else
+ flag = PQconnectPoll(conn);
}
}
@@ -2519,13 +2687,17 @@ keep_going: /* We will come back to here until there is
* Oops, no more hosts.
*
* If we are trying to connect in "prefer-standby" mode, then drop
- * the standby requirement and start over.
+ * the standby requirement and start over. Don't do this for
+ * cancel requests though, since we are certain the list of
+ * servers won't change as the target_server_type option is not
+ * applicable to those connections.
*
* Otherwise, an appropriate error message is already set up, so
* we just need to set the right status.
*/
if (conn->target_server_type == SERVER_TYPE_PREFER_STANDBY &&
- conn->nconnhost > 0)
+ conn->nconnhost > 0 &&
+ !conn->cancelRequest)
{
conn->target_server_type = SERVER_TYPE_PREFER_STANDBY_PASS2;
conn->whichhost = 0;
@@ -3141,6 +3313,29 @@ keep_going: /* We will come back to here until there is
}
#endif /* USE_SSL */
+ /*
+ * For cancel requests this is as far as we need to go in the
+ * connection establishment. Now we can actually send our
+ * cancelation request.
+ */
+ if (conn->cancelRequest)
+ {
+ CancelRequestPacket cancelpacket;
+
+ packetlen = sizeof(cancelpacket);
+ cancelpacket.cancelRequestCode = (MsgType) pg_hton32(CANCEL_REQUEST_CODE);
+ cancelpacket.backendPID = pg_hton32(conn->be_pid);
+ cancelpacket.cancelAuthCode = pg_hton32(conn->be_key);
+ if (pqPacketSend(conn, 0, &cancelpacket, packetlen) != STATUS_OK)
+ {
+ libpq_append_conn_error(conn, "could not send cancel packet: %s",
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ goto error_return;
+ }
+ conn->status = CONNECTION_AWAITING_RESPONSE;
+ return PGRES_POLLING_READING;
+ }
+
/*
* Build the startup packet.
*/
@@ -3885,8 +4080,14 @@ keep_going: /* We will come back to here until there is
}
}
- /* We can release the address list now. */
- release_conn_addrinfo(conn);
+ /*
+ * For non cancel requests we can release the address list
+ * now. For cancel requests we never actually resolve
+ * addresses and instead the addrinfo exists for the lifetime
+ * of the connection.
+ */
+ if (!conn->cancelRequest)
+ release_conn_addrinfo(conn);
/*
* Contents of conn->errorMessage are no longer interesting
@@ -4254,19 +4455,8 @@ freePGconn(PGconn *conn)
free(conn->events[i].name);
}
- /* clean up pg_conn_host structures */
- for (int i = 0; i < conn->nconnhost; ++i)
- {
- free(conn->connhost[i].host);
- free(conn->connhost[i].hostaddr);
- free(conn->connhost[i].port);
- if (conn->connhost[i].password != NULL)
- {
- explicit_bzero(conn->connhost[i].password, strlen(conn->connhost[i].password));
- free(conn->connhost[i].password);
- }
- }
- free(conn->connhost);
+ release_conn_addrinfo(conn);
+ release_conn_hosts(conn);
free(conn->client_encoding_initial);
free(conn->events);
@@ -4383,6 +4573,31 @@ release_conn_addrinfo(PGconn *conn)
}
}
+/*
+ * release_conn_hosts
+ * - Free the host list in the PGconn.
+ */
+static void
+release_conn_hosts(PGconn *conn)
+{
+ if (conn->connhost)
+ {
+ for (int i = 0; i < conn->nconnhost; ++i)
+ {
+ free(conn->connhost[i].host);
+ free(conn->connhost[i].hostaddr);
+ free(conn->connhost[i].port);
+ if (conn->connhost[i].password != NULL)
+ {
+ explicit_bzero(conn->connhost[i].password, strlen(conn->connhost[i].password));
+ free(conn->connhost[i].password);
+ }
+ }
+ free(conn->connhost);
+ }
+}
+
+
/*
* sendTerminateConn
* - Send a terminate message to backend.
@@ -4390,6 +4605,15 @@ release_conn_addrinfo(PGconn *conn)
static void
sendTerminateConn(PGconn *conn)
{
+ /*
+ * The Postgres cancellation protocol does not have a notion of a
+ * Terminate message, so don't send one.
+ */
+ if (conn->cancelRequest)
+ {
+ return;
+ }
+
/*
* Note that the protocol doesn't allow us to send Terminate messages
* during the startup phase.
@@ -4443,7 +4667,13 @@ closePGconn(PGconn *conn)
conn->pipelineStatus = PQ_PIPELINE_OFF;
pqClearAsyncResult(conn); /* deallocate result */
pqClearConnErrorState(conn);
- release_conn_addrinfo(conn);
+
+ /*
+ * Since cancel requests never change their addrinfo we don't free it
+ * here. Otherwise we would have to rebuild it during a PQcancelReset.
+ */
+ if (!conn->cancelRequest)
+ release_conn_addrinfo(conn);
/* Reset all state obtained from server, too */
pqDropServerData(conn);
@@ -4858,6 +5088,180 @@ cancel_errReturn:
return false;
}
+/*
+ * PQcancelSend
+ *
+ * Send a cancellation request in a blocking fashion.
+ */
+PGcancelConn *
+PQcancelSend(PGconn *conn)
+{
+ PGcancelConn *cancelConn = PQcancelConn(conn);
+
+ if (!cancelConn || cancelConn->conn.status == CONNECTION_BAD)
+ return cancelConn;
+
+ if (!connectDBStart(&cancelConn->conn))
+ {
+ cancelConn->conn.status = CONNECTION_BAD;
+ return cancelConn;
+ }
+
+ (void) connectDBComplete(&cancelConn->conn);
+
+ return cancelConn;
+}
+
+/*
+ * PQcancelPoll
+ *
+ * Poll a cancel connection. For usage details see PQconnectPoll.
+ */
+PostgresPollingStatusType
+PQcancelPoll(PGcancelConn * cancelConn)
+{
+ PGconn *conn = (PGconn *) cancelConn;
+ int n;
+
+ /*
+ * Before we can call PQconnectPoll we first need to start the connection
+ * using connectDBstart. Non-cancel connections already do this whenever
+ * the connection is initialized. But cancel connections wait until the
+ * caller starts polling, because there might be a large delay between
+ * creating a cancel connection and actually wanting to use it.
+ */
+ if (conn->status == CONNECTION_STARTING)
+ {
+ if (!connectDBStart(&cancelConn->conn))
+ {
+ cancelConn->conn.status = CONNECTION_STARTED;
+ return PGRES_POLLING_WRITING;
+ }
+ }
+
+ /*
+ * The rest of the connection establishement we leave to PQconnectPoll,
+ * since it's very similar to normal connection establishment. But once we
+ * get to the CONNECTION_AWAITING_RESPONSE we need to do our own thing.
+ */
+ if (conn->status != CONNECTION_AWAITING_RESPONSE)
+ {
+ return PQconnectPoll(conn);
+ }
+
+ /*
+ * At this point we are waiting on the server to close the connection,
+ * which is its way of communicating that the cancel has been handled.
+ */
+
+ n = pqReadData(conn);
+
+ if (n == 0)
+ return PGRES_POLLING_READING;
+
+#ifndef WIN32
+
+ /*
+ * Windows is a bit special in its EOF behaviour for TCP. Sometimes it
+ * will error with an ECONNRESET when there is a clean connection closure.
+ * See these threads for details:
+ * https://www.postgresql.org/message-id/flat/90b34057-4176-7bb0-0dbb-9822a5f6425b%40greiz-reinsdorf.de
+ *
+ * https://www.postgresql.org/message-id/flat/CA%2BhUKG%2BOeoETZQ%3DQw5Ub5h3tmwQhBmDA%3DnuNO3KG%3DzWfUypFAw%40mail.gmail.com
+ *
+ * PQcancel ignores such errors and reports success for the cancellation
+ * anyway, so even if this is not always correct we do the same here. For
+ * all other OSes we consider any other error than EOF and report it as
+ * such.
+ */
+ if (n < 0 && n != -2)
+ {
+ conn->status = CONNECTION_BAD;
+ return PGRES_POLLING_FAILED;
+ }
+#endif
+
+ /*
+ * We don't expect any data, only connection closure. So if we strangly do
+ * receive some data we consider that an error.
+ */
+ if (n > 0)
+ {
+
+ libpq_append_conn_error(conn, "received unexpected response from server");
+ conn->status = CONNECTION_BAD;
+ return PGRES_POLLING_FAILED;
+ }
+
+ /*
+ * Getting here means that we received an EOF. Which is what we were
+ * expecting. The cancel request has completed.
+ */
+ cancelConn->conn.status = CONNECTION_OK;
+ resetPQExpBuffer(&conn->errorMessage);
+ return PGRES_POLLING_OK;
+}
+
+/*
+ * PQcancelStatus
+ *
+ * Get the status of a cancel connection.
+ */
+ConnStatusType
+PQcancelStatus(const PGcancelConn * cancelConn)
+{
+ return PQstatus((const PGconn *) cancelConn);
+}
+
+/*
+ * PQcancelSocket
+ *
+ * Get the socket of the cancel connection.
+ */
+int
+PQcancelSocket(const PGcancelConn * cancelConn)
+{
+ return PQsocket((const PGconn *) cancelConn);
+}
+
+/*
+ * PQcancelErrorMessage
+ *
+ * Get the socket of the cancel connection.
+ */
+char *
+PQcancelErrorMessage(const PGcancelConn * cancelConn)
+{
+ return PQerrorMessage((const PGconn *) cancelConn);
+}
+
+/*
+ * PQcancelReset
+ *
+ * Resets the cancel connection, so it can be reused to send a new cancel
+ * request.
+ */
+void
+PQcancelReset(PGcancelConn * cancelConn)
+{
+ closePGconn((PGconn *) cancelConn);
+ cancelConn->conn.status = CONNECTION_STARTING;
+ cancelConn->conn.whichhost = 0;
+ cancelConn->conn.whichaddr = 0;
+ cancelConn->conn.try_next_host = false;
+ cancelConn->conn.try_next_addr = false;
+}
+
+/*
+ * PQcancelFinish
+ *
+ * Closes and frees the cancel connection.
+ */
+void
+PQcancelFinish(PGcancelConn * cancelConn)
+{
+ PQfinish((PGconn *) cancelConn);
+}
/*
* PQrequestCancel: old, not thread-safe function for requesting query cancel
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index f3d92204964..95899b9f55b 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -78,7 +78,9 @@ typedef enum
CONNECTION_CONSUME, /* Consuming any extra messages. */
CONNECTION_GSS_STARTUP, /* Negotiating GSSAPI. */
CONNECTION_CHECK_TARGET, /* Checking target server properties. */
- CONNECTION_CHECK_STANDBY /* Checking if server is in standby mode. */
+ CONNECTION_CHECK_STANDBY, /* Checking if server is in standby mode. */
+ CONNECTION_STARTING /* Waiting for connection attempt to be
+ * started. */
} ConnStatusType;
typedef enum
@@ -165,6 +167,11 @@ typedef enum
*/
typedef struct pg_conn PGconn;
+/* PGcancelConn encapsulates a cancel connection to the backend.
+ * The contents of this struct are not supposed to be known to applications.
+ */
+typedef struct pg_cancel_conn PGcancelConn;
+
/* PGresult encapsulates the result of a query (or more precisely, of a single
* SQL command --- a query string given to PQsendQuery can contain multiple
* commands and thus return multiple PGresult objects).
@@ -321,16 +328,28 @@ extern PostgresPollingStatusType PQresetPoll(PGconn *conn);
/* Synchronous (blocking) */
extern void PQreset(PGconn *conn);
+/* issue a cancel request */
+extern PGcancelConn * PQcancelSend(PGconn *conn);
+/* non-blocking version of PQcancelSend */
+extern PGcancelConn * PQcancelConn(PGconn *conn);
+extern PostgresPollingStatusType PQcancelPoll(PGcancelConn * cancelConn);
+extern ConnStatusType PQcancelStatus(const PGcancelConn * cancelConn);
+extern int PQcancelSocket(const PGcancelConn * cancelConn);
+extern char *PQcancelErrorMessage(const PGcancelConn * cancelConn);
+extern void PQcancelReset(PGcancelConn * cancelConn);
+extern void PQcancelFinish(PGcancelConn * cancelConn);
+
+
/* request a cancel structure */
extern PGcancel *PQgetCancel(PGconn *conn);
/* free a cancel structure */
extern void PQfreeCancel(PGcancel *cancel);
-/* issue a cancel request */
+/* a less secure version of PQcancelSend, but one which is signal-safe */
extern int PQcancel(PGcancel *cancel, char *errbuf, int errbufsize);
-/* backwards compatible version of PQcancel; not thread-safe */
+/* deprecated version of PQcancel; not thread-safe */
extern int PQrequestCancel(PGconn *conn);
/* Accessor functions for PGconn objects */
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 760ee3f6912..eaca46b6aa0 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -399,6 +399,10 @@ struct pg_conn
char *target_session_attrs; /* desired session properties */
char *require_auth; /* name of the expected auth method */
+ bool cancelRequest; /* true if this connection is used to send a
+ * cancel request, instead of being a normal
+ * connection that's used for queries */
+
/* Optional file to write trace info to */
FILE *Pfdebug;
int traceFlags;
@@ -606,6 +610,11 @@ struct pg_conn
PQExpBufferData workBuffer; /* expansible string */
};
+struct pg_cancel_conn
+{
+ PGconn conn;
+};
+
/* PGcancel stores all data necessary to cancel a connection. A copy of this
* data is required to safely cancel a connection running on a different
* thread.
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index f48da7d963e..6764ab513b2 100644
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -86,6 +86,264 @@ pg_fatal_impl(int line, const char *fmt,...)
exit(1);
}
+/*
+ * Check that the query on the given connection got cancelled.
+ *
+ * This is a function wrapped in a macro to make the reported line number
+ * in an error match the line number of the invocation.
+ */
+#define confirm_query_cancelled(conn) confirm_query_cancelled_impl(__LINE__, conn)
+static void
+confirm_query_cancelled_impl(int line, PGconn *conn)
+{
+ PGresult *res = NULL;
+
+ res = PQgetResult(conn);
+ if (res == NULL)
+ pg_fatal_impl(line, "PQgetResult returned null: %s",
+ PQerrorMessage(conn));
+ if (PQresultStatus(res) != PGRES_FATAL_ERROR)
+ pg_fatal_impl(line, "query did not fail when it was expected");
+ if (strcmp(PQresultErrorField(res, PG_DIAG_SQLSTATE), "57014") != 0)
+ pg_fatal_impl(line, "query failed with a different error than cancellation: %s",
+ PQerrorMessage(conn));
+ PQclear(res);
+ while (PQisBusy(conn))
+ {
+ PQconsumeInput(conn);
+ }
+}
+
+#define send_cancellable_query(conn, monitorConn) send_cancellable_query_impl(__LINE__, conn, monitorConn)
+static void
+send_cancellable_query_impl(int line, PGconn *conn, PGconn *monitorConn)
+{
+ const char *env_wait;
+ const Oid paramTypes[1] = {INT4OID};
+
+ env_wait = getenv("PG_TEST_TIMEOUT_DEFAULT");
+ if (env_wait == NULL)
+ env_wait = "180";
+
+ if (PQsendQueryParams(conn, "SELECT pg_sleep($1)", 1, paramTypes, &env_wait, NULL, NULL, 0) != 1)
+ pg_fatal_impl(line, "failed to send query: %s", PQerrorMessage(conn));
+
+ /*
+ * Wait until the query is actually running. Otherwise sending a
+ * cancellation request might not cancel the query due to race conditions.
+ */
+ while (true)
+ {
+ char *value = NULL;
+ PGresult *res = PQexec(
+ monitorConn,
+ "SELECT count(*) FROM pg_stat_activity WHERE "
+ "query = 'SELECT pg_sleep($1)' "
+ "AND state = 'active'");
+
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ {
+ pg_fatal("Connection to database failed: %s", PQerrorMessage(monitorConn));
+ }
+ if (PQntuples(res) != 1)
+ {
+ pg_fatal("unexpected number of rows received: %d", PQntuples(res));
+ }
+ if (PQnfields(res) != 1)
+ {
+ pg_fatal("unexpected number of columns received: %d", PQnfields(res));
+ }
+ value = PQgetvalue(res, 0, 0);
+ if (*value != '0')
+ {
+ PQclear(res);
+ break;
+ }
+ PQclear(res);
+
+ /*
+ * wait 10ms before polling again
+ */
+ pg_usleep(10000);
+ }
+}
+
+static void
+test_cancel(PGconn *conn, const char *conninfo)
+{
+ PGcancel *cancel = NULL;
+ PGcancelConn *cancelConn = NULL;
+ PGconn *monitorConn = NULL;
+ char errorbuf[256];
+
+ fprintf(stderr, "test cancellations... ");
+
+ if (PQsetnonblocking(conn, 1) != 0)
+ pg_fatal("failed to set nonblocking mode: %s", PQerrorMessage(conn));
+
+ /*
+ * Make a connection to the database to monitor the query on the main
+ * connection.
+ */
+ monitorConn = PQconnectdb(conninfo);
+ if (PQstatus(conn) != CONNECTION_OK)
+ {
+ pg_fatal("Connection to database failed: %s",
+ PQerrorMessage(conn));
+ }
+
+ /* test PQcancel */
+ send_cancellable_query(conn, monitorConn);
+ cancel = PQgetCancel(conn);
+ if (!PQcancel(cancel, errorbuf, sizeof(errorbuf)))
+ {
+ pg_fatal("failed to run PQcancel: %s", errorbuf);
+ };
+ confirm_query_cancelled(conn);
+
+ /* PGcancel object can be reused for the next query */
+ send_cancellable_query(conn, monitorConn);
+ if (!PQcancel(cancel, errorbuf, sizeof(errorbuf)))
+ {
+ pg_fatal("failed to run PQcancel: %s", errorbuf);
+ };
+ confirm_query_cancelled(conn);
+
+ PQfreeCancel(cancel);
+
+ /* test PQrequestCancel */
+ send_cancellable_query(conn, monitorConn);
+ if (!PQrequestCancel(conn))
+ pg_fatal("failed to run PQrequestCancel: %s", PQerrorMessage(conn));
+ confirm_query_cancelled(conn);
+
+ /* test PQcancelSend */
+ send_cancellable_query(conn, monitorConn);
+ cancelConn = PQcancelSend(conn);
+ if (PQcancelStatus(cancelConn) == CONNECTION_BAD)
+ pg_fatal("failed to run PQcancelSend: %s", PQcancelErrorMessage(cancelConn));
+ confirm_query_cancelled(conn);
+ PQcancelFinish(cancelConn);
+
+ /* test PQcancelConn and then polling with PQcancelPoll */
+ send_cancellable_query(conn, monitorConn);
+ cancelConn = PQcancelConn(conn);
+ if (PQcancelStatus(cancelConn) == CONNECTION_BAD)
+ pg_fatal("bad cancel connection: %s", PQcancelErrorMessage(cancelConn));
+ while (true)
+ {
+ struct timeval tv;
+ fd_set input_mask;
+ fd_set output_mask;
+ PostgresPollingStatusType pollres = PQcancelPoll(cancelConn);
+ int sock = PQcancelSocket(cancelConn);
+
+ if (pollres == PGRES_POLLING_OK)
+ {
+ break;
+ }
+
+ FD_ZERO(&input_mask);
+ FD_ZERO(&output_mask);
+ switch (pollres)
+ {
+ case PGRES_POLLING_READING:
+ pg_debug("polling for reads\n");
+ FD_SET(sock, &input_mask);
+ break;
+ case PGRES_POLLING_WRITING:
+ pg_debug("polling for writes\n");
+ FD_SET(sock, &output_mask);
+ break;
+ default:
+ pg_fatal("bad cancel connection: %s", PQcancelErrorMessage(cancelConn));
+ }
+
+ if (sock < 0)
+ pg_fatal("sock did not exist: %s", PQcancelErrorMessage(cancelConn));
+
+ tv.tv_sec = 3;
+ tv.tv_usec = 0;
+
+ while (true)
+ {
+ if (select(sock + 1, &input_mask, &output_mask, NULL, &tv) < 0)
+ {
+ if (errno == EINTR)
+ continue;
+ pg_fatal("select() failed: %m");
+ }
+ break;
+ }
+ }
+ if (PQcancelStatus(cancelConn) != CONNECTION_OK)
+ pg_fatal("unexpected cancel connection status: %s", PQcancelErrorMessage(cancelConn));
+ confirm_query_cancelled(conn);
+
+ /*
+ * test PQcancelReset works on the cancel connection and it can be reused
+ * after
+ */
+ PQcancelReset(cancelConn);
+
+ send_cancellable_query(conn, monitorConn);
+ if (PQcancelStatus(cancelConn) == CONNECTION_BAD)
+ pg_fatal("bad cancel connection: %s", PQcancelErrorMessage(cancelConn));
+ while (true)
+ {
+ struct timeval tv;
+ fd_set input_mask;
+ fd_set output_mask;
+ PostgresPollingStatusType pollres = PQcancelPoll(cancelConn);
+ int sock = PQcancelSocket(cancelConn);
+
+ if (pollres == PGRES_POLLING_OK)
+ {
+ break;
+ }
+
+ FD_ZERO(&input_mask);
+ FD_ZERO(&output_mask);
+ switch (pollres)
+ {
+ case PGRES_POLLING_READING:
+ pg_debug("polling for reads\n");
+ FD_SET(sock, &input_mask);
+ break;
+ case PGRES_POLLING_WRITING:
+ pg_debug("polling for writes\n");
+ FD_SET(sock, &output_mask);
+ break;
+ default:
+ pg_fatal("bad cancel connection: %s", PQcancelErrorMessage(cancelConn));
+ }
+
+ if (sock < 0)
+ pg_fatal("sock did not exist: %s", PQcancelErrorMessage(cancelConn));
+
+ tv.tv_sec = 3;
+ tv.tv_usec = 0;
+
+ while (true)
+ {
+ if (select(sock + 1, &input_mask, &output_mask, NULL, &tv) < 0)
+ {
+ if (errno == EINTR)
+ continue;
+ pg_fatal("select() failed: %m");
+ }
+ break;
+ }
+ }
+ if (PQcancelStatus(cancelConn) != CONNECTION_OK)
+ pg_fatal("unexpected cancel connection status: %s", PQcancelErrorMessage(cancelConn));
+ confirm_query_cancelled(conn);
+
+ PQcancelFinish(cancelConn);
+
+ fprintf(stderr, "ok\n");
+}
+
static void
test_disallowed_in_pipeline(PGconn *conn)
{
@@ -985,7 +1243,7 @@ test_prepared(PGconn *conn)
static void
notice_processor(void *arg, const char *message)
{
- int *n_notices = (int *) arg;
+ int *n_notices = (int *) arg;
(*n_notices)++;
fprintf(stderr, "NOTICE %d: %s", *n_notices, message);
@@ -1681,6 +1939,7 @@ usage(const char *progname)
static void
print_test_list(void)
{
+ printf("cancel\n");
printf("disallowed_in_pipeline\n");
printf("multi_pipelines\n");
printf("nosync\n");
@@ -1782,7 +2041,9 @@ main(int argc, char **argv)
PQTRACE_SUPPRESS_TIMESTAMPS | PQTRACE_REGRESS_MODE);
}
- if (strcmp(testname, "disallowed_in_pipeline") == 0)
+ if (strcmp(testname, "cancel") == 0)
+ test_cancel(conn, conninfo);
+ else if (strcmp(testname, "disallowed_in_pipeline") == 0)
test_disallowed_in_pipeline(conn);
else if (strcmp(testname, "multi_pipelines") == 0)
test_multi_pipelines(conn);
--
2.34.1
[application/octet-stream] v17-0001-libpq-Run-pgindent-after-a9e9a9f32b3.patch (22.4K, ../CAGECzQQUw3UFP8ivMYg0tt3-vDscSZMRuwpABjkUO2m9=JUSTA@mail.gmail.com/6-v17-0001-libpq-Run-pgindent-after-a9e9a9f32b3.patch)
download | inline diff:
From 062c395cc25e272b86b88b0095c72312cd1f0fc4 Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Wed, 30 Nov 2022 10:07:19 +0100
Subject: [PATCH v17 1/5] libpq: Run pgindent after a9e9a9f32b3
It seems that pgindent was not run after the error handling refactor in
commit a9e9a9f32b35edf129c88e8b929ef223f8511f59. This fixes that and
also addresses a few other things pgindent wanted to change in libpq.
---
src/interfaces/libpq/fe-exec.c | 16 +++---
src/interfaces/libpq/fe-lobj.c | 42 ++++++++--------
src/interfaces/libpq/fe-misc.c | 10 ++--
src/interfaces/libpq/fe-protocol3.c | 2 +-
src/interfaces/libpq/fe-secure-common.c | 6 +--
src/interfaces/libpq/fe-secure-gssapi.c | 12 ++---
src/interfaces/libpq/fe-secure-openssl.c | 64 ++++++++++++------------
src/interfaces/libpq/fe-secure.c | 8 +--
src/interfaces/libpq/libpq-int.h | 4 +-
9 files changed, 82 insertions(+), 82 deletions(-)
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index a16bbf32ef5..14d706efd57 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -1448,7 +1448,7 @@ PQsendQueryInternal(PGconn *conn, const char *query, bool newQuery)
if (conn->pipelineStatus != PQ_PIPELINE_OFF)
{
libpq_append_conn_error(conn, "%s not allowed in pipeline mode",
- "PQsendQuery");
+ "PQsendQuery");
return 0;
}
@@ -1516,7 +1516,7 @@ PQsendQueryParams(PGconn *conn,
if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT)
{
libpq_append_conn_error(conn, "number of parameters must be between 0 and %d",
- PQ_QUERY_PARAM_MAX_LIMIT);
+ PQ_QUERY_PARAM_MAX_LIMIT);
return 0;
}
@@ -1562,7 +1562,7 @@ PQsendPrepare(PGconn *conn,
if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT)
{
libpq_append_conn_error(conn, "number of parameters must be between 0 and %d",
- PQ_QUERY_PARAM_MAX_LIMIT);
+ PQ_QUERY_PARAM_MAX_LIMIT);
return 0;
}
@@ -1656,7 +1656,7 @@ PQsendQueryPrepared(PGconn *conn,
if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT)
{
libpq_append_conn_error(conn, "number of parameters must be between 0 and %d",
- PQ_QUERY_PARAM_MAX_LIMIT);
+ PQ_QUERY_PARAM_MAX_LIMIT);
return 0;
}
@@ -2103,10 +2103,9 @@ PQgetResult(PGconn *conn)
/*
* We're about to return the NULL that terminates the round of
- * results from the current query; prepare to send the results
- * of the next query, if any, when we're called next. If there's
- * no next element in the command queue, this gets us in IDLE
- * state.
+ * results from the current query; prepare to send the results of
+ * the next query, if any, when we're called next. If there's no
+ * next element in the command queue, this gets us in IDLE state.
*/
pqPipelineProcessQueue(conn);
res = NULL; /* query is complete */
@@ -3051,6 +3050,7 @@ pqPipelineProcessQueue(PGconn *conn)
return;
case PGASYNC_IDLE:
+
/*
* If we're in IDLE mode and there's some command in the queue,
* get us into PIPELINE_IDLE mode and process normally. Otherwise
diff --git a/src/interfaces/libpq/fe-lobj.c b/src/interfaces/libpq/fe-lobj.c
index 4cb6a468597..206266fd043 100644
--- a/src/interfaces/libpq/fe-lobj.c
+++ b/src/interfaces/libpq/fe-lobj.c
@@ -142,7 +142,7 @@ lo_truncate(PGconn *conn, int fd, size_t len)
if (conn->lobjfuncs->fn_lo_truncate == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_truncate");
+ "lo_truncate");
return -1;
}
@@ -205,7 +205,7 @@ lo_truncate64(PGconn *conn, int fd, pg_int64 len)
if (conn->lobjfuncs->fn_lo_truncate64 == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_truncate64");
+ "lo_truncate64");
return -1;
}
@@ -395,7 +395,7 @@ lo_lseek64(PGconn *conn, int fd, pg_int64 offset, int whence)
if (conn->lobjfuncs->fn_lo_lseek64 == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_lseek64");
+ "lo_lseek64");
return -1;
}
@@ -485,7 +485,7 @@ lo_create(PGconn *conn, Oid lobjId)
if (conn->lobjfuncs->fn_lo_create == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_create");
+ "lo_create");
return InvalidOid;
}
@@ -558,7 +558,7 @@ lo_tell64(PGconn *conn, int fd)
if (conn->lobjfuncs->fn_lo_tell64 == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_tell64");
+ "lo_tell64");
return -1;
}
@@ -667,7 +667,7 @@ lo_import_internal(PGconn *conn, const char *filename, Oid oid)
if (fd < 0)
{ /* error */
libpq_append_conn_error(conn, "could not open file \"%s\": %s",
- filename, strerror_r(errno, sebuf, sizeof(sebuf)));
+ filename, strerror_r(errno, sebuf, sizeof(sebuf)));
return InvalidOid;
}
@@ -723,8 +723,8 @@ lo_import_internal(PGconn *conn, const char *filename, Oid oid)
/* deliberately overwrite any error from lo_close */
pqClearConnErrorState(conn);
libpq_append_conn_error(conn, "could not read from file \"%s\": %s",
- filename,
- strerror_r(save_errno, sebuf, sizeof(sebuf)));
+ filename,
+ strerror_r(save_errno, sebuf, sizeof(sebuf)));
return InvalidOid;
}
@@ -778,8 +778,8 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename)
/* deliberately overwrite any error from lo_close */
pqClearConnErrorState(conn);
libpq_append_conn_error(conn, "could not open file \"%s\": %s",
- filename,
- strerror_r(save_errno, sebuf, sizeof(sebuf)));
+ filename,
+ strerror_r(save_errno, sebuf, sizeof(sebuf)));
return -1;
}
@@ -799,8 +799,8 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename)
/* deliberately overwrite any error from lo_close */
pqClearConnErrorState(conn);
libpq_append_conn_error(conn, "could not write to file \"%s\": %s",
- filename,
- strerror_r(save_errno, sebuf, sizeof(sebuf)));
+ filename,
+ strerror_r(save_errno, sebuf, sizeof(sebuf)));
return -1;
}
}
@@ -822,7 +822,7 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename)
if (close(fd) != 0 && result >= 0)
{
libpq_append_conn_error(conn, "could not write to file \"%s\": %s",
- filename, strerror_r(errno, sebuf, sizeof(sebuf)));
+ filename, strerror_r(errno, sebuf, sizeof(sebuf)));
result = -1;
}
@@ -954,56 +954,56 @@ lo_initialize(PGconn *conn)
if (lobjfuncs->fn_lo_open == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_open");
+ "lo_open");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_close == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_close");
+ "lo_close");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_creat == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_creat");
+ "lo_creat");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_unlink == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_unlink");
+ "lo_unlink");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_lseek == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_lseek");
+ "lo_lseek");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_tell == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lo_tell");
+ "lo_tell");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_read == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "loread");
+ "loread");
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_write == 0)
{
libpq_append_conn_error(conn, "cannot determine OID of function %s",
- "lowrite");
+ "lowrite");
free(lobjfuncs);
return -1;
}
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 3653a1a8a62..660cdec93c9 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -749,8 +749,8 @@ retry4:
*/
definitelyEOF:
libpq_append_conn_error(conn, "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.");
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.");
/* Come here if lower-level code already set a suitable errorMessage */
definitelyFailed:
@@ -1067,7 +1067,7 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
char sebuf[PG_STRERROR_R_BUFLEN];
libpq_append_conn_error(conn, "%s() failed: %s", "select",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
}
return result;
@@ -1280,7 +1280,7 @@ libpq_ngettext(const char *msgid, const char *msgid_plural, unsigned long n)
* newline.
*/
void
-libpq_append_error(PQExpBuffer errorMessage, const char *fmt, ...)
+libpq_append_error(PQExpBuffer errorMessage, const char *fmt,...)
{
int save_errno = errno;
bool done;
@@ -1309,7 +1309,7 @@ libpq_append_error(PQExpBuffer errorMessage, const char *fmt, ...)
* format should not end with a newline.
*/
void
-libpq_append_conn_error(PGconn *conn, const char *fmt, ...)
+libpq_append_conn_error(PGconn *conn, const char *fmt,...)
{
int save_errno = errno;
bool done;
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 8ab6a884165..b79d74f7489 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -466,7 +466,7 @@ static void
handleSyncLoss(PGconn *conn, char id, int msgLength)
{
libpq_append_conn_error(conn, "lost synchronization with server: got message type \"%c\", length %d",
- id, msgLength);
+ id, msgLength);
/* build an error result holding the error message */
pqSaveErrorResult(conn);
conn->asyncStatus = PGASYNC_READY; /* drop out of PQgetResult wait loop */
diff --git a/src/interfaces/libpq/fe-secure-common.c b/src/interfaces/libpq/fe-secure-common.c
index de115b37649..3ecc7bf6159 100644
--- a/src/interfaces/libpq/fe-secure-common.c
+++ b/src/interfaces/libpq/fe-secure-common.c
@@ -226,7 +226,7 @@ pq_verify_peer_name_matches_certificate_ip(PGconn *conn,
* wrong given the subject matter.
*/
libpq_append_conn_error(conn, "certificate contains IP address with invalid length %zu",
- iplen);
+ iplen);
return -1;
}
@@ -235,7 +235,7 @@ pq_verify_peer_name_matches_certificate_ip(PGconn *conn,
if (!addrstr)
{
libpq_append_conn_error(conn, "could not convert certificate's IP address to string: %s",
- strerror_r(errno, sebuf, sizeof(sebuf)));
+ strerror_r(errno, sebuf, sizeof(sebuf)));
return -1;
}
@@ -292,7 +292,7 @@ pq_verify_peer_name_matches_certificate(PGconn *conn)
else if (names_examined == 1)
{
libpq_append_conn_error(conn, "server certificate for \"%s\" does not match host name \"%s\"",
- first_name, host);
+ first_name, host);
}
else
{
diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c
index 038e847b7e9..0af4de941af 100644
--- a/src/interfaces/libpq/fe-secure-gssapi.c
+++ b/src/interfaces/libpq/fe-secure-gssapi.c
@@ -213,8 +213,8 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32))
{
libpq_append_conn_error(conn, "client tried to send oversize GSSAPI packet (%zu > %zu)",
- (size_t) output.length,
- PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32));
+ (size_t) output.length,
+ PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32));
errno = EIO; /* for lack of a better idea */
goto cleanup;
}
@@ -349,8 +349,8 @@ pg_GSS_read(PGconn *conn, void *ptr, size_t len)
if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32))
{
libpq_append_conn_error(conn, "oversize GSSAPI packet sent by the server (%zu > %zu)",
- (size_t) input.length,
- PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32));
+ (size_t) input.length,
+ PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32));
errno = EIO; /* for lack of a better idea */
return -1;
}
@@ -590,8 +590,8 @@ pqsecure_open_gss(PGconn *conn)
if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32))
{
libpq_append_conn_error(conn, "oversize GSSAPI packet sent by the server (%zu > %zu)",
- (size_t) input.length,
- PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32));
+ (size_t) input.length,
+ PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32));
return PGRES_POLLING_FAILED;
}
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 4d1e4009ef1..61f8a5c9c6c 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -213,12 +213,12 @@ rloop:
if (result_errno == EPIPE ||
result_errno == ECONNRESET)
libpq_append_conn_error(conn, "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.");
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.");
else
libpq_append_conn_error(conn, "SSL SYSCALL error: %s",
- SOCK_STRERROR(result_errno,
- sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(result_errno,
+ sebuf, sizeof(sebuf)));
}
else
{
@@ -313,12 +313,12 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
result_errno = SOCK_ERRNO;
if (result_errno == EPIPE || result_errno == ECONNRESET)
libpq_append_conn_error(conn, "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.");
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.");
else
libpq_append_conn_error(conn, "SSL SYSCALL error: %s",
- SOCK_STRERROR(result_errno,
- sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(result_errno,
+ sebuf, sizeof(sebuf)));
}
else
{
@@ -415,7 +415,7 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
if (algo_type == NULL)
{
libpq_append_conn_error(conn, "could not find digest for NID %s",
- OBJ_nid2sn(algo_nid));
+ OBJ_nid2sn(algo_nid));
return NULL;
}
break;
@@ -1000,7 +1000,7 @@ initialize_SSL(PGconn *conn)
if (ssl_min_ver == -1)
{
libpq_append_conn_error(conn, "invalid value \"%s\" for minimum SSL protocol version",
- conn->ssl_min_protocol_version);
+ conn->ssl_min_protocol_version);
SSL_CTX_free(SSL_context);
return -1;
}
@@ -1026,7 +1026,7 @@ initialize_SSL(PGconn *conn)
if (ssl_max_ver == -1)
{
libpq_append_conn_error(conn, "invalid value \"%s\" for maximum SSL protocol version",
- conn->ssl_max_protocol_version);
+ conn->ssl_max_protocol_version);
SSL_CTX_free(SSL_context);
return -1;
}
@@ -1070,7 +1070,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "could not read root certificate file \"%s\": %s",
- fnbuf, err);
+ fnbuf, err);
SSLerrfree(err);
SSL_CTX_free(SSL_context);
return -1;
@@ -1122,10 +1122,10 @@ initialize_SSL(PGconn *conn)
*/
if (fnbuf[0] == '\0')
libpq_append_conn_error(conn, "could not get home directory to locate root certificate file\n"
- "Either provide the file or change sslmode to disable server certificate verification.");
+ "Either provide the file or change sslmode to disable server certificate verification.");
else
libpq_append_conn_error(conn, "root certificate file \"%s\" does not exist\n"
- "Either provide the file or change sslmode to disable server certificate verification.", fnbuf);
+ "Either provide the file or change sslmode to disable server certificate verification.", fnbuf);
SSL_CTX_free(SSL_context);
return -1;
}
@@ -1160,7 +1160,7 @@ initialize_SSL(PGconn *conn)
if (errno != ENOENT && errno != ENOTDIR)
{
libpq_append_conn_error(conn, "could not open certificate file \"%s\": %s",
- fnbuf, strerror_r(errno, sebuf, sizeof(sebuf)));
+ fnbuf, strerror_r(errno, sebuf, sizeof(sebuf)));
SSL_CTX_free(SSL_context);
return -1;
}
@@ -1178,7 +1178,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "could not read certificate file \"%s\": %s",
- fnbuf, err);
+ fnbuf, err);
SSLerrfree(err);
SSL_CTX_free(SSL_context);
return -1;
@@ -1277,7 +1277,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "could not load SSL engine \"%s\": %s",
- engine_str, err);
+ engine_str, err);
SSLerrfree(err);
free(engine_str);
return -1;
@@ -1288,7 +1288,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "could not initialize SSL engine \"%s\": %s",
- engine_str, err);
+ engine_str, err);
SSLerrfree(err);
ENGINE_free(conn->engine);
conn->engine = NULL;
@@ -1303,7 +1303,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "could not read private SSL key \"%s\" from engine \"%s\": %s",
- engine_colon, engine_str, err);
+ engine_colon, engine_str, err);
SSLerrfree(err);
ENGINE_finish(conn->engine);
ENGINE_free(conn->engine);
@@ -1316,7 +1316,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "could not load private SSL key \"%s\" from engine \"%s\": %s",
- engine_colon, engine_str, err);
+ engine_colon, engine_str, err);
SSLerrfree(err);
ENGINE_finish(conn->engine);
ENGINE_free(conn->engine);
@@ -1353,10 +1353,10 @@ initialize_SSL(PGconn *conn)
{
if (errno == ENOENT)
libpq_append_conn_error(conn, "certificate present, but not private key file \"%s\"",
- fnbuf);
+ fnbuf);
else
libpq_append_conn_error(conn, "could not stat private key file \"%s\": %m",
- fnbuf);
+ fnbuf);
return -1;
}
@@ -1364,7 +1364,7 @@ initialize_SSL(PGconn *conn)
if (!S_ISREG(buf.st_mode))
{
libpq_append_conn_error(conn, "private key file \"%s\" is not a regular file",
- fnbuf);
+ fnbuf);
return -1;
}
@@ -1421,7 +1421,7 @@ initialize_SSL(PGconn *conn)
if (SSL_use_PrivateKey_file(conn->ssl, fnbuf, SSL_FILETYPE_ASN1) != 1)
{
libpq_append_conn_error(conn, "could not load private key file \"%s\": %s",
- fnbuf, err);
+ fnbuf, err);
SSLerrfree(err);
return -1;
}
@@ -1437,7 +1437,7 @@ initialize_SSL(PGconn *conn)
char *err = SSLerrmessage(ERR_get_error());
libpq_append_conn_error(conn, "certificate does not match private key file \"%s\": %s",
- fnbuf, err);
+ fnbuf, err);
SSLerrfree(err);
return -1;
}
@@ -1490,7 +1490,7 @@ open_client_SSL(PGconn *conn)
if (r == -1)
libpq_append_conn_error(conn, "SSL SYSCALL error: %s",
- SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
else
libpq_append_conn_error(conn, "SSL SYSCALL error: EOF detected");
pgtls_close(conn);
@@ -1532,12 +1532,12 @@ open_client_SSL(PGconn *conn)
case SSL_R_VERSION_TOO_LOW:
#endif
libpq_append_conn_error(conn, "This may indicate that the server does not support any SSL protocol version between %s and %s.",
- conn->ssl_min_protocol_version ?
- conn->ssl_min_protocol_version :
- MIN_OPENSSL_TLS_VERSION,
- conn->ssl_max_protocol_version ?
- conn->ssl_max_protocol_version :
- MAX_OPENSSL_TLS_VERSION);
+ conn->ssl_min_protocol_version ?
+ conn->ssl_min_protocol_version :
+ MIN_OPENSSL_TLS_VERSION,
+ conn->ssl_max_protocol_version ?
+ conn->ssl_max_protocol_version :
+ MAX_OPENSSL_TLS_VERSION);
break;
default:
break;
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 66e401bf3d9..8069e381424 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -255,14 +255,14 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
case EPIPE:
case ECONNRESET:
libpq_append_conn_error(conn, "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.");
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.");
break;
default:
libpq_append_conn_error(conn, "could not receive data from server: %s",
- SOCK_STRERROR(result_errno,
- sebuf, sizeof(sebuf)));
+ SOCK_STRERROR(result_errno,
+ sebuf, sizeof(sebuf)));
break;
}
}
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 88b9838d766..1ff57044508 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -901,8 +901,8 @@ extern char *libpq_ngettext(const char *msgid, const char *msgid_plural, unsigne
*/
#undef _
-extern void libpq_append_error(PQExpBuffer errorMessage, const char *fmt, ...) pg_attribute_printf(2, 3);
-extern void libpq_append_conn_error(PGconn *conn, const char *fmt, ...) pg_attribute_printf(2, 3);
+extern void libpq_append_error(PQExpBuffer errorMessage, const char *fmt,...) pg_attribute_printf(2, 3);
+extern void libpq_append_conn_error(PGconn *conn, const char *fmt,...) pg_attribute_printf(2, 3);
/*
* These macros are needed to let error-handling code be portable between
base-commit: c1f1c1f87fd685981c45da528649c700b6ba0655
--
2.34.1
view thread (12+ messages)
reply
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [EXTERNAL] Re: Add non-blocking version of PQcancel
In-Reply-To: <CAGECzQQUw3UFP8ivMYg0tt3-vDscSZMRuwpABjkUO2m9=JUSTA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox