From 2e372869aa38a9d6e4552c192da4454b17e01e38 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 18 Feb 2022 11:43:04 -0500 Subject: [PATCH] Don't let libpq PGEVT_CONNRESET callbacks break a PGconn. As currently implemented, failure of a PGEVT_CONNRESET callback forces the PGconn into the CONNECTION_BAD state (without closing the socket, which is inconsistent with other failure paths), and prevents later callbacks from being called. This seems highly questionable, and indeed is questioned by comments in the source. Instead, let's just ignore the result value of PGEVT_CONNRESET calls. Like the preceding commit, this converts event callbacks into "pure observers" that cannot affect libpq's processing logic. Discussion: https://postgr.es/m/3185105.1644960083@sss.pgh.pa.us --- doc/src/sgml/libpq.sgml | 11 +++++------ src/interfaces/libpq/fe-connect.c | 28 ++++++---------------------- 2 files changed, 11 insertions(+), 28 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 40c39feb7d..64e17401cd 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -7183,12 +7183,11 @@ typedef struct The connection reset event is fired on completion of or PQresetPoll. In - both cases, the event is only fired if the reset was successful. If - the event procedure fails, the entire connection reset will fail; the - PGconn is put into - CONNECTION_BAD status and - PQresetPoll will return - PGRES_POLLING_FAILED. + both cases, the event is only fired if the reset was successful. + The return value of the event procedure is ignored + in PostgreSQL v15 and later. + With earlier versions, however, it's important to return success + (nonzero) or the connection will be aborted. typedef struct diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 30d6b7b377..9c9416e8ff 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -4276,8 +4276,7 @@ PQreset(PGconn *conn) if (connectDBStart(conn) && connectDBComplete(conn)) { /* - * Notify event procs of successful reset. We treat an event proc - * failure as disabling the connection ... good idea? + * Notify event procs of successful reset. */ int i; @@ -4286,15 +4285,8 @@ PQreset(PGconn *conn) PGEventConnReset evt; evt.conn = conn; - if (!conn->events[i].proc(PGEVT_CONNRESET, &evt, - conn->events[i].passThrough)) - { - conn->status = CONNECTION_BAD; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("PGEventProc \"%s\" failed during PGEVT_CONNRESET event\n"), - conn->events[i].name); - break; - } + (void) conn->events[i].proc(PGEVT_CONNRESET, &evt, + conn->events[i].passThrough); } } } @@ -4336,8 +4328,7 @@ PQresetPoll(PGconn *conn) if (status == PGRES_POLLING_OK) { /* - * Notify event procs of successful reset. We treat an event proc - * failure as disabling the connection ... good idea? + * Notify event procs of successful reset. */ int i; @@ -4346,15 +4337,8 @@ PQresetPoll(PGconn *conn) PGEventConnReset evt; evt.conn = conn; - if (!conn->events[i].proc(PGEVT_CONNRESET, &evt, - conn->events[i].passThrough)) - { - conn->status = CONNECTION_BAD; - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("PGEventProc \"%s\" failed during PGEVT_CONNRESET event\n"), - conn->events[i].name); - return PGRES_POLLING_FAILED; - } + (void) conn->events[i].proc(PGEVT_CONNRESET, &evt, + conn->events[i].passThrough); } }