dblink, postgres_fdw: Handle interrupts during connection establishment

Until now dblink and postgres_fdw did not process interrupts during connection
establishment. Besides preventing query cancellations etc, this can lead to
undetected deadlocks, as global barriers are not processed.

These aforementioned undetected deadlocks are the reason for the spate of CI
test failures in the FreeBSD 'test_running' step.

Fix the bug by using the helper from libpq-be-fe-helpers.h, introduced in a
prior commit. Besides fixing the bug, this also removes duplicated code
around reserving file descriptors.

As the change is relatively large and there are no field reports of the
problem, don't backpatch for now.

Reviewed-by: Thomas Munro <[email protected]>
Discussion: https://postgr.es/m/20220925232237[email protected]
Backpatch:

diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 8dd122042b47c876ee96b196041dd292b3312a9b..8982d623d3b208df6357f059fb8fff5edb6c3bcf 100644 (file)
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -48,6 +48,7 @@
 #include "funcapi.h"
 #include "lib/stringinfo.h"
 #include "libpq-fe.h"
+#include "libpq/libpq-be-fe-helpers.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "parser/scansup.h"
@@ -199,37 +200,14 @@ dblink_get_conn(char *conname_or_str,
            connstr = conname_or_str;
        dblink_connstr_check(connstr);
 
-       /*
-        * We must obey fd.c's limit on non-virtual file descriptors.  Assume
-        * that a PGconn represents one long-lived FD.  (Doing this here also
-        * ensures that VFDs are closed if needed to make room.)
-        */
-       if (!AcquireExternalFD())
-       {
-#ifndef WIN32                  /* can't write #if within ereport() macro */
-           ereport(ERROR,
-                   (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
-                    errmsg("could not establish connection"),
-                    errdetail("There are too many open files on the local server."),
-                    errhint("Raise the server's max_files_per_process and/or \"ulimit -n\" limits.")));
-#else
-           ereport(ERROR,
-                   (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
-                    errmsg("could not establish connection"),
-                    errdetail("There are too many open files on the local server."),
-                    errhint("Raise the server's max_files_per_process setting.")));
-#endif
-       }
-
        /* OK to make connection */
-       conn = PQconnectdb(connstr);
+       conn = libpqsrv_connect(connstr, PG_WAIT_EXTENSION);
 
        if (PQstatus(conn) == CONNECTION_BAD)
        {
            char       *msg = pchomp(PQerrorMessage(conn));
 
-           PQfinish(conn);
-           ReleaseExternalFD();
+           libpqsrv_disconnect(conn);
            ereport(ERROR,
                    (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
                     errmsg("could not establish connection"),
@@ -312,36 +290,13 @@ dblink_connect(PG_FUNCTION_ARGS)
    /* check password in connection string if not superuser */
    dblink_connstr_check(connstr);
 
-   /*
-    * We must obey fd.c's limit on non-virtual file descriptors.  Assume that
-    * a PGconn represents one long-lived FD.  (Doing this here also ensures
-    * that VFDs are closed if needed to make room.)
-    */
-   if (!AcquireExternalFD())
-   {
-#ifndef WIN32                  /* can't write #if within ereport() macro */
-       ereport(ERROR,
-               (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
-                errmsg("could not establish connection"),
-                errdetail("There are too many open files on the local server."),
-                errhint("Raise the server's max_files_per_process and/or \"ulimit -n\" limits.")));
-#else
-       ereport(ERROR,
-               (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
-                errmsg("could not establish connection"),
-                errdetail("There are too many open files on the local server."),
-                errhint("Raise the server's max_files_per_process setting.")));
-#endif
-   }
-
    /* OK to make connection */
-   conn = PQconnectdb(connstr);
+   conn = libpqsrv_connect(connstr, PG_WAIT_EXTENSION);
 
    if (PQstatus(conn) == CONNECTION_BAD)
    {
        msg = pchomp(PQerrorMessage(conn));
-       PQfinish(conn);
-       ReleaseExternalFD();
+       libpqsrv_disconnect(conn);
        if (rconn)
            pfree(rconn);
 
@@ -366,10 +321,7 @@ dblink_connect(PG_FUNCTION_ARGS)
    else
    {
        if (pconn->conn)
-       {
-           PQfinish(pconn->conn);
-           ReleaseExternalFD();
-       }
+           libpqsrv_disconnect(conn);
        pconn->conn = conn;
    }
 
@@ -402,8 +354,7 @@ dblink_disconnect(PG_FUNCTION_ARGS)
    if (!conn)
        dblink_conn_not_avail(conname);
 
-   PQfinish(conn);
-   ReleaseExternalFD();
+   libpqsrv_disconnect(conn);
    if (rconn)
    {
        deleteConnection(conname);
@@ -838,10 +789,7 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async)
    {
        /* if needed, close the connection to the database */
        if (freeconn)
-       {
-           PQfinish(conn);
-           ReleaseExternalFD();
-       }
+           libpqsrv_disconnect(conn);
    }
    PG_END_TRY();
 
@@ -1516,10 +1464,7 @@ dblink_exec(PG_FUNCTION_ARGS)
    {
        /* if needed, close the connection to the database */
        if (freeconn)
-       {
-           PQfinish(conn);
-           ReleaseExternalFD();
-       }
+           libpqsrv_disconnect(conn);
    }
    PG_END_TRY();
 
@@ -2606,8 +2551,7 @@ createNewConnection(const char *name, remoteConn *rconn)
 
    if (found)
    {
-       PQfinish(rconn->conn);
-       ReleaseExternalFD();
+       libpqsrv_disconnect(rconn->conn);
        pfree(rconn);
 
        ereport(ERROR,
@@ -2647,8 +2591,7 @@ dblink_security_check(PGconn *conn, remoteConn *rconn)
    {
        if (!PQconnectionUsedPassword(conn))
        {
-           PQfinish(conn);
-           ReleaseExternalFD();
+           libpqsrv_disconnect(conn);
            if (rconn)
                pfree(rconn);
 

diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index ed75ce3f79c7a0a473412200c9133b185b59ae36..7760380f00df399a22df3ae9b0e0904779b2264d 100644 (file)
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -17,6 +17,7 @@
 #include "catalog/pg_user_mapping.h"
 #include "commands/defrem.h"
 #include "funcapi.h"
+#include "libpq/libpq-be-fe-helpers.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "pgstat.h"
@@ -446,35 +447,10 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
        /* verify the set of connection parameters */
        check_conn_params(keywords, values, user);
 
-       /*
-        * We must obey fd.c's limit on non-virtual file descriptors.  Assume
-        * that a PGconn represents one long-lived FD.  (Doing this here also
-        * ensures that VFDs are closed if needed to make room.)
-        */
-       if (!AcquireExternalFD())
-       {
-#ifndef WIN32                  /* can't write #if within ereport() macro */
-           ereport(ERROR,
-                   (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
-                    errmsg("could not connect to server \"%s\"",
-                           server->servername),
-                    errdetail("There are too many open files on the local server."),
-                    errhint("Raise the server's max_files_per_process and/or \"ulimit -n\" limits.")));
-#else
-           ereport(ERROR,
-                   (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
-                    errmsg("could not connect to server \"%s\"",
-                           server->servername),
-                    errdetail("There are too many open files on the local server."),
-                    errhint("Raise the server's max_files_per_process setting.")));
-#endif
-       }
-
        /* OK to make connection */
-       conn = PQconnectdbParams(keywords, values, false);
-
-       if (!conn)
-           ReleaseExternalFD();    /* because the PG_CATCH block won't */
+       conn = libpqsrv_connect_params(keywords, values,
+                                       /* expand_dbname = */ false,
+                                      PG_WAIT_EXTENSION);
 
        if (!conn || PQstatus(conn) != CONNECTION_OK)
            ereport(ERROR,
@@ -507,12 +483,7 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
    }
    PG_CATCH();
    {
-       /* Release PGconn data structure if we managed to create one */
-       if (conn)
-       {
-           PQfinish(conn);
-           ReleaseExternalFD();
-       }
+       libpqsrv_disconnect(conn);
        PG_RE_THROW();
    }
    PG_END_TRY();
@@ -528,9 +499,8 @@ disconnect_pg_server(ConnCacheEntry *entry)
 {
    if (entry->conn != NULL)
    {
-       PQfinish(entry->conn);
+       libpqsrv_disconnect(entry->conn);
        entry->conn = NULL;
-       ReleaseExternalFD();
    }
 }