Don't let libpq PGEVT_CONNRESET callbacks break a PGconn.
authorTom Lane <[email protected]>
Fri, 18 Feb 2022 16:43:04 +0000 (11:43 -0500)
committerTom Lane <[email protected]>
Fri, 18 Feb 2022 16:43:04 +0000 (11:43 -0500)
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
src/interfaces/libpq/fe-connect.c

index 40c39feb7d0d5abf1587023bebdec2c22e125a4c..64e17401cdf1235ac14fd4c246d74ae6e4c6c896 100644 (file)
@@ -7183,12 +7183,11 @@ typedef struct
       <para>
        The connection reset event is fired on completion of
        <xref linkend="libpq-PQreset"/> or <function>PQresetPoll</function>.  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
-       <structname>PGconn</structname> is put into
-       <literal>CONNECTION_BAD</literal> status and
-       <function>PQresetPoll</function> will return
-       <literal>PGRES_POLLING_FAILED</literal>.
+       both cases, the event is only fired if the reset was successful.
+       The return value of the event procedure is ignored
+       in <productname>PostgreSQL</productname> v15 and later.
+       With earlier versions, however, it's important to return success
+       (nonzero) or the connection will be aborted.
 
 <synopsis>
 typedef struct
index 30d6b7b3778e665b7312c9aade4546e7f103d6ae..9c9416e8fffa1b709101a9a53319adb061af8a93 100644 (file)
@@ -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);
            }
        }