Re: Fixing memory leaks in postgres_fdw - Mailing list pgsql-hackers

I wrote:
> Here is an attempt at making a bulletproof fix by having all backend
> users of libpq go through a wrapper layer that provides the memory
> context callback.  Perhaps this is more code churn than we want to
> accept; I'm not sure.  I thought about avoiding most of the niggling
> code changes by adding
> #define PGresult BEPGresult
> #define PQclear BEPQclear
> #define PQresultStatus BEPQresultStatus
> and so forth at the bottom of the new header file, but I'm afraid
> that would create a lot of confusion.

I tried that, and it leads to such a less-messy patch that I think
we should probably do it this way, confusion or no.  One argument
that can be made in favor is that we don't really want random
notational differences between frontend and backend code that's
doing the same thing.

Also, I'd been struggling with the assumption that we should
palloc the wrapper object before calling PQgetResult; there
doesn't seem to be any nice way to make that transparent to
callers.  I realized that we can make it simple as long as
we're willing to assume that allocating with MCXT_ALLOC_NO_OOM
can't throw an error.  But we assume that in other usages too.

Hence, v3 attached.  The 0002 patch is still the same as before.

            regards, tom lane

From be4b888d0d2936cb80e63092d14a3133fab590da Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 25 May 2025 19:49:33 -0400
Subject: [PATCH v3 1/2] Fix memory leakage in postgres_fdw's DirectModify code
 path.

postgres_fdw tries to use PG_TRY blocks to ensure that it will
eventually free the PGresult created by the remote modify command.
However, it's fundamentally impossible for this scheme to work
reliably when there's RETURNING data, because the query could fail
in between invocations of postgres_fdw's DirectModify methods.
There is at least one instance of exactly this situation in the
regression tests, and the ensuing session-lifespan leak is visible
under Valgrind.

We can improve matters by using a memory context reset callback
attached to the ExecutorState context.  That ensures that the
PGresult will be freed when the ExecutorState context is torn
down, even if control never reaches postgresEndDirectModify.
Also, by switching to this approach, we can eliminate some
PG_TRY overhead since it's no longer necessary to be so
cautious about errors.

This patch adds infrastructure that wraps each PGresult in a
"libpqsrv_PGresult" that provides the reset callback.  Code using
this abstraction is inherently memory-safe (so long as it attaches
the reset callbacks to an appropriate memory context).  Furthermore,
we add some macros that automatically redirect calls of the libpq
functions concerned with PGresults to use this infrastructure,
so that almost no source-code changes are needed to wheel this
infrastructure into place.

So far I created the infrastructure and made relevant code
use it, but I have not done very much towards making the
caller simplifications that should now be possible.  I did
remove a few PQclear calls in error-exit paths that are now
no longer necessary.  But we should be able to remove a lot
of uses of PG_TRY and "volatile" variable markings, as well
as simplify error handling, now that it's not necessary to be
so paranoid about freeing PGresults before throwing an error.
I did fix libpqsrv_get_result_last() that way, just as proof
of concept.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/2976982.1748049023@sss.pgh.pa.us
---
 contrib/postgres_fdw/postgres_fdw.c           |  19 +-
 contrib/postgres_fdw/postgres_fdw.h           |   2 +-
 .../libpqwalreceiver/libpqwalreceiver.c       |  31 +--
 src/backend/utils/mmgr/mcxt.c                 |  39 ++-
 src/include/libpq/libpq-be-fe-helpers.h       |  59 ++--
 src/include/libpq/libpq-be-fe.h               | 259 ++++++++++++++++++
 src/include/utils/palloc.h                    |   2 +
 src/tools/pgindent/typedefs.list              |   1 +
 8 files changed, 327 insertions(+), 85 deletions(-)
 create mode 100644 src/include/libpq/libpq-be-fe.h

diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 331f3fc088d..3cc27c5a1e2 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -3779,9 +3779,6 @@ create_cursor(ForeignScanState *node)

     /*
      * Get the result, and check for success.
-     *
-     * We don't use a PG_TRY block here, so be careful not to throw error
-     * without releasing the PGresult.
      */
     res = pgfdw_get_result(conn);
     if (PQresultStatus(res) != PGRES_COMMAND_OK)
@@ -4178,9 +4175,6 @@ execute_foreign_modify(EState *estate,

     /*
      * Get the result, and check for success.
-     *
-     * We don't use a PG_TRY block here, so be careful not to throw error
-     * without releasing the PGresult.
      */
     res = pgfdw_get_result(fmstate->conn);
     if (PQresultStatus(res) !=
@@ -4248,9 +4242,6 @@ prepare_foreign_modify(PgFdwModifyState *fmstate)

     /*
      * Get the result, and check for success.
-     *
-     * We don't use a PG_TRY block here, so be careful not to throw error
-     * without releasing the PGresult.
      */
     res = pgfdw_get_result(fmstate->conn);
     if (PQresultStatus(res) != PGRES_COMMAND_OK)
@@ -4590,9 +4581,6 @@ execute_dml_stmt(ForeignScanState *node)

     /*
      * Get the result, and check for success.
-     *
-     * We don't use a PG_TRY block here, so be careful not to throw error
-     * without releasing the PGresult.
      */
     dmstate->result = pgfdw_get_result(dmstate->conn);
     if (PQresultStatus(dmstate->result) !=
@@ -4600,6 +4588,13 @@ execute_dml_stmt(ForeignScanState *node)
         pgfdw_report_error(ERROR, dmstate->result, dmstate->conn, true,
                            dmstate->query);

+    /*
+     * The result potentially needs to survive across multiple executor row
+     * cycles, so move it to the context where the dmstate is.
+     */
+    dmstate->result = libpqsrv_PGresultSetParent(dmstate->result,
+                                                 GetMemoryChunkContext(dmstate));
+
     /* Get the number of rows affected. */
     if (dmstate->has_returning)
         dmstate->num_tuples = PQntuples(dmstate->result);
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 81358f3bde7..9cb4ee84139 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -15,7 +15,7 @@

 #include "foreign/foreign.h"
 #include "lib/stringinfo.h"
-#include "libpq-fe.h"
+#include "libpq/libpq-be-fe.h"
 #include "nodes/execnodes.h"
 #include "nodes/pathnodes.h"
 #include "utils/relcache.h"
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 7b4ddf7a8f5..d615f340ac7 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -418,31 +418,22 @@ libpqrcv_identify_system(WalReceiverConn *conn, TimeLineID *primary_tli)
                         "IDENTIFY_SYSTEM",
                         WAIT_EVENT_LIBPQWALRECEIVER_RECEIVE);
     if (PQresultStatus(res) != PGRES_TUPLES_OK)
-    {
-        PQclear(res);
         ereport(ERROR,
                 (errcode(ERRCODE_PROTOCOL_VIOLATION),
                  errmsg("could not receive database system identifier and timeline ID from "
                         "the primary server: %s",
                         pchomp(PQerrorMessage(conn->streamConn)))));
-    }

     /*
      * IDENTIFY_SYSTEM returns 3 columns in 9.3 and earlier, and 4 columns in
      * 9.4 and onwards.
      */
     if (PQnfields(res) < 3 || PQntuples(res) != 1)
-    {
-        int            ntuples = PQntuples(res);
-        int            nfields = PQnfields(res);
-
-        PQclear(res);
         ereport(ERROR,
                 (errcode(ERRCODE_PROTOCOL_VIOLATION),
                  errmsg("invalid response from primary server"),
                  errdetail("Could not identify system: got %d rows and %d fields, expected %d rows and %d or more
fields.",
-                           ntuples, nfields, 1, 3)));
-    }
+                           PQntuples(res), PQnfields(res), 1, 3)));
     primary_sysid = pstrdup(PQgetvalue(res, 0, 0));
     *primary_tli = pg_strtoint32(PQgetvalue(res, 0, 1));
     PQclear(res);
@@ -604,13 +595,10 @@ libpqrcv_startstreaming(WalReceiverConn *conn,
         return false;
     }
     else if (PQresultStatus(res) != PGRES_COPY_BOTH)
-    {
-        PQclear(res);
         ereport(ERROR,
                 (errcode(ERRCODE_PROTOCOL_VIOLATION),
                  errmsg("could not start WAL streaming: %s",
                         pchomp(PQerrorMessage(conn->streamConn)))));
-    }
     PQclear(res);
     return true;
 }
@@ -718,26 +706,17 @@ libpqrcv_readtimelinehistoryfile(WalReceiverConn *conn,
                         cmd,
                         WAIT_EVENT_LIBPQWALRECEIVER_RECEIVE);
     if (PQresultStatus(res) != PGRES_TUPLES_OK)
-    {
-        PQclear(res);
         ereport(ERROR,
                 (errcode(ERRCODE_PROTOCOL_VIOLATION),
                  errmsg("could not receive timeline history file from "
                         "the primary server: %s",
                         pchomp(PQerrorMessage(conn->streamConn)))));
-    }
     if (PQnfields(res) != 2 || PQntuples(res) != 1)
-    {
-        int            ntuples = PQntuples(res);
-        int            nfields = PQnfields(res);
-
-        PQclear(res);
         ereport(ERROR,
                 (errcode(ERRCODE_PROTOCOL_VIOLATION),
                  errmsg("invalid response from primary server"),
                  errdetail("Expected 1 tuple with 2 fields, got %d tuples with %d fields.",
-                           ntuples, nfields)));
-    }
+                           PQntuples(res), PQnfields(res))));
     *filename = pstrdup(PQgetvalue(res, 0, 0));

     *len = PQgetlength(res, 0, 1);
@@ -841,13 +820,10 @@ libpqrcv_receive(WalReceiverConn *conn, char **buffer,
             return -1;
         }
         else
-        {
-            PQclear(res);
             ereport(ERROR,
                     (errcode(ERRCODE_PROTOCOL_VIOLATION),
                      errmsg("could not receive data from WAL stream: %s",
                             pchomp(PQerrorMessage(conn->streamConn)))));
-        }
     }
     if (rawlen < -1)
         ereport(ERROR,
@@ -971,13 +947,10 @@ libpqrcv_create_slot(WalReceiverConn *conn, const char *slotname,
     pfree(cmd.data);

     if (PQresultStatus(res) != PGRES_TUPLES_OK)
-    {
-        PQclear(res);
         ereport(ERROR,
                 (errcode(ERRCODE_PROTOCOL_VIOLATION),
                  errmsg("could not create replication slot \"%s\": %s",
                         slotname, pchomp(PQerrorMessage(conn->streamConn)))));
-    }

     if (lsn)
         *lsn = DatumGetLSN(DirectFunctionCall1Coll(pg_lsn_in, InvalidOid,
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 15fa4d0a55e..ce01dce9861 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -560,9 +560,7 @@ MemoryContextDeleteChildren(MemoryContext context)
  * the specified context, since that means it will automatically be freed
  * when no longer needed.
  *
- * There is no API for deregistering a callback once registered.  If you
- * want it to not do anything anymore, adjust the state pointed to by its
- * "arg" to indicate that.
+ * Note that callers can assume this cannot fail.
  */
 void
 MemoryContextRegisterResetCallback(MemoryContext context,
@@ -577,6 +575,41 @@ MemoryContextRegisterResetCallback(MemoryContext context,
     context->isReset = false;
 }

+/*
+ * MemoryContextUnregisterResetCallback
+ *        Undo the effects of MemoryContextRegisterResetCallback.
+ *
+ * This can be used if a callback's effects are no longer required
+ * at some point before the context has been reset/deleted.  It is the
+ * caller's responsibility to pfree the callback struct (if needed).
+ *
+ * An assertion failure occurs if the callback was not registered.
+ * We could alternatively define that case as a no-op, but that seems too
+ * likely to mask programming errors such as passing the wrong context.
+ */
+void
+MemoryContextUnregisterResetCallback(MemoryContext context,
+                                     MemoryContextCallback *cb)
+{
+    MemoryContextCallback *prev,
+               *cur;
+
+    Assert(MemoryContextIsValid(context));
+
+    for (prev = NULL, cur = context->reset_cbs; cur != NULL;
+         prev = cur, cur = cur->next)
+    {
+        if (cur != cb)
+            continue;
+        if (prev)
+            prev->next = cur->next;
+        else
+            context->reset_cbs = cur->next;
+        return;
+    }
+    Assert(false);
+}
+
 /*
  * MemoryContextCallResetCallbacks
  *        Internal function to call all registered callbacks for context.
diff --git a/src/include/libpq/libpq-be-fe-helpers.h b/src/include/libpq/libpq-be-fe-helpers.h
index 16205b824fa..6f4e2bd7ef9 100644
--- a/src/include/libpq/libpq-be-fe-helpers.h
+++ b/src/include/libpq/libpq-be-fe-helpers.h
@@ -30,17 +30,7 @@
 #ifndef LIBPQ_BE_FE_HELPERS_H
 #define LIBPQ_BE_FE_HELPERS_H

-/*
- * Despite the name, BUILDING_DLL is set only when building code directly part
- * of the backend. Which also is where libpq isn't allowed to be
- * used. Obviously this doesn't protect against libpq-fe.h getting included
- * otherwise, but perhaps still protects against a few mistakes...
- */
-#ifdef BUILDING_DLL
-#error "libpq may not be used code directly built into the backend"
-#endif
-
-#include "libpq-fe.h"
+#include "libpq/libpq-be-fe.h"
 #include "miscadmin.h"
 #include "storage/fd.h"
 #include "storage/latch.h"
@@ -289,41 +279,30 @@ libpqsrv_exec_params(PGconn *conn,
 static inline PGresult *
 libpqsrv_get_result_last(PGconn *conn, uint32 wait_event_info)
 {
-    PGresult   *volatile lastResult = NULL;
+    PGresult   *lastResult = NULL;

-    /* In what follows, do not leak any PGresults on an error. */
-    PG_TRY();
+    for (;;)
     {
-        for (;;)
-        {
-            /* Wait for, and collect, the next PGresult. */
-            PGresult   *result;
+        /* Wait for, and collect, the next PGresult. */
+        PGresult   *result;

-            result = libpqsrv_get_result(conn, wait_event_info);
-            if (result == NULL)
-                break;            /* query is complete, or failure */
+        result = libpqsrv_get_result(conn, wait_event_info);
+        if (result == NULL)
+            break;                /* query is complete, or failure */

-            /*
-             * Emulate PQexec()'s behavior of returning the last result when
-             * there are many.
-             */
-            PQclear(lastResult);
-            lastResult = result;
-
-            if (PQresultStatus(lastResult) == PGRES_COPY_IN ||
-                PQresultStatus(lastResult) == PGRES_COPY_OUT ||
-                PQresultStatus(lastResult) == PGRES_COPY_BOTH ||
-                PQstatus(conn) == CONNECTION_BAD)
-                break;
-        }
-    }
-    PG_CATCH();
-    {
+        /*
+         * Emulate PQexec()'s behavior of returning the last result when there
+         * are many.
+         */
         PQclear(lastResult);
-        PG_RE_THROW();
-    }
-    PG_END_TRY();
+        lastResult = result;

+        if (PQresultStatus(lastResult) == PGRES_COPY_IN ||
+            PQresultStatus(lastResult) == PGRES_COPY_OUT ||
+            PQresultStatus(lastResult) == PGRES_COPY_BOTH ||
+            PQstatus(conn) == CONNECTION_BAD)
+            break;
+    }
     return lastResult;
 }

diff --git a/src/include/libpq/libpq-be-fe.h b/src/include/libpq/libpq-be-fe.h
new file mode 100644
index 00000000000..e3f796b0230
--- /dev/null
+++ b/src/include/libpq/libpq-be-fe.h
@@ -0,0 +1,259 @@
+/*-------------------------------------------------------------------------
+ *
+ * libpq-be-fe.h
+ *      Wrapper functions for using libpq in extensions
+ *
+ * Code built directly into the backend is not allowed to link to libpq
+ * directly. Extension code is allowed to use libpq however. One of the
+ * main risks in doing so is leaking the malloc-allocated structures
+ * returned by libpq, causing a process-lifespan memory leak.
+ *
+ * This file provides wrapper objects to help in building memory-safe code.
+ * A PGresult object wrapped this way acts much as if it were palloc'd:
+ * it will go away when the specified context is reset or deleted.
+ * We might later extend the concept to other objects such as PGconns.
+ *
+ * See also the libpq-be-fe-helpers.h file, which provides additional
+ * facilities built on top of this one.
+ *
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/libpq/libpq-be-fe.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef LIBPQ_BE_FE_H
+#define LIBPQ_BE_FE_H
+
+/*
+ * Despite the name, BUILDING_DLL is set only when building code directly part
+ * of the backend. Which also is where libpq isn't allowed to be
+ * used. Obviously this doesn't protect against libpq-fe.h getting included
+ * otherwise, but perhaps still protects against a few mistakes...
+ */
+#ifdef BUILDING_DLL
+#error "libpq may not be used in code directly built into the backend"
+#endif
+
+#include "libpq-fe.h"
+
+/*
+ * Memory-context-safe wrapper object for a PGresult.
+ */
+typedef struct libpqsrv_PGresult
+{
+    PGresult   *res;            /* the wrapped PGresult */
+    MemoryContext ctx;            /* the MemoryContext it's attached to */
+    MemoryContextCallback cb;    /* the callback that implements freeing */
+} libpqsrv_PGresult;
+
+
+/*
+ * Wrap the given PGresult in a libpqsrv_PGresult object, so that it will
+ * go away automatically if the current memory context is reset or deleted.
+ *
+ * To avoid potential memory leaks, backend code must always apply this
+ * immediately to the output of any PGresult-yielding libpq function.
+ */
+static inline libpqsrv_PGresult *
+libpqsrv_PQwrap(PGresult *res)
+{
+    libpqsrv_PGresult *bres;
+    MemoryContext ctx = CurrentMemoryContext;
+
+    /* We pass through a NULL result as-is, since there's nothing to free */
+    if (res == NULL)
+        return NULL;
+    /* Attempt to allocate the wrapper ... this had better not throw error */
+    bres = (libpqsrv_PGresult *)
+        MemoryContextAllocExtended(ctx,
+                                   sizeof(libpqsrv_PGresult),
+                                   MCXT_ALLOC_NO_OOM);
+    /* If we failed to allocate a wrapper, free the PGresult before failing */
+    if (bres == NULL)
+    {
+        PQclear(res);
+        ereport(ERROR,
+                (errcode(ERRCODE_OUT_OF_MEMORY),
+                 errmsg("out of memory")));
+    }
+    /* Okay, set up the wrapper */
+    bres->res = res;
+    bres->ctx = ctx;
+    bres->cb.func = (MemoryContextCallbackFunction) PQclear;
+    bres->cb.arg = res;
+    MemoryContextRegisterResetCallback(ctx, &bres->cb);
+    return bres;
+}
+
+/*
+ * Free a wrapped PGresult, after detaching it from the memory context.
+ * Like PQclear(), allow the argument to be NULL.
+ */
+static inline void
+libpqsrv_PQclear(libpqsrv_PGresult *bres)
+{
+    if (bres)
+    {
+        MemoryContextUnregisterResetCallback(bres->ctx, &bres->cb);
+        PQclear(bres->res);
+        pfree(bres);
+    }
+}
+
+/*
+ * Move a wrapped PGresult to have a different parent context.
+ */
+static inline libpqsrv_PGresult *
+libpqsrv_PGresultSetParent(libpqsrv_PGresult *bres, MemoryContext ctx)
+{
+    libpqsrv_PGresult *newres;
+
+    /* We pass through a NULL result as-is */
+    if (bres == NULL)
+        return NULL;
+    /* Make a new wrapper in the target context, raising error on OOM */
+    newres = (libpqsrv_PGresult *)
+        MemoryContextAlloc(ctx, sizeof(libpqsrv_PGresult));
+    /* Okay, set up the new wrapper */
+    newres->res = bres->res;
+    newres->ctx = ctx;
+    newres->cb.func = (MemoryContextCallbackFunction) PQclear;
+    newres->cb.arg = bres->res;
+    MemoryContextRegisterResetCallback(ctx, &newres->cb);
+    /* Disarm and delete the old wrapper */
+    MemoryContextUnregisterResetCallback(bres->ctx, &bres->cb);
+    pfree(bres);
+    return newres;
+}
+
+/*
+ * Convenience wrapper for PQgetResult.
+ *
+ * We could supply wrappers for other PGresult-returning functions too,
+ * but at present there's no need.
+ */
+static inline libpqsrv_PGresult *
+libpqsrv_PQgetResult(PGconn *conn)
+{
+    return libpqsrv_PQwrap(PQgetResult(conn));
+}
+
+/*
+ * Accessor functions for libpqsrv_PGresult.  While it's not necessary to use
+ * these, they emulate the behavior of the underlying libpq functions when
+ * passed a NULL pointer.  This is particularly important for PQresultStatus,
+ * which is often the first check on a result.
+ */
+
+static inline ExecStatusType
+libpqsrv_PQresultStatus(const libpqsrv_PGresult *res)
+{
+    if (!res)
+        return PGRES_FATAL_ERROR;
+    return PQresultStatus(res->res);
+}
+
+static inline const char *
+libpqsrv_PQresultErrorMessage(const libpqsrv_PGresult *res)
+{
+    if (!res)
+        return "";
+    return PQresultErrorMessage(res->res);
+}
+
+static inline char *
+libpqsrv_PQresultErrorField(const libpqsrv_PGresult *res, int fieldcode)
+{
+    if (!res)
+        return NULL;
+    return PQresultErrorField(res->res, fieldcode);
+}
+
+static inline char *
+libpqsrv_PQcmdStatus(const libpqsrv_PGresult *res)
+{
+    if (!res)
+        return NULL;
+    return PQcmdStatus(res->res);
+}
+
+static inline int
+libpqsrv_PQntuples(const libpqsrv_PGresult *res)
+{
+    if (!res)
+        return 0;
+    return PQntuples(res->res);
+}
+
+static inline int
+libpqsrv_PQnfields(const libpqsrv_PGresult *res)
+{
+    if (!res)
+        return 0;
+    return PQnfields(res->res);
+}
+
+static inline char *
+libpqsrv_PQgetvalue(const libpqsrv_PGresult *res, int tup_num, int field_num)
+{
+    if (!res)
+        return NULL;
+    return PQgetvalue(res->res, tup_num, field_num);
+}
+
+static inline int
+libpqsrv_PQgetlength(const libpqsrv_PGresult *res, int tup_num, int field_num)
+{
+    if (!res)
+        return 0;
+    return PQgetlength(res->res, tup_num, field_num);
+}
+
+static inline int
+libpqsrv_PQgetisnull(const libpqsrv_PGresult *res, int tup_num, int field_num)
+{
+    if (!res)
+        return 1;                /* pretend it is null */
+    return PQgetisnull(res->res, tup_num, field_num);
+}
+
+static inline char *
+libpqsrv_PQfname(const libpqsrv_PGresult *res, int field_num)
+{
+    if (!res)
+        return NULL;
+    return PQfname(res->res, field_num);
+}
+
+static inline const char *
+libpqsrv_PQcmdTuples(const libpqsrv_PGresult *res)
+{
+    if (!res)
+        return "";
+    return PQcmdTuples(res->res);
+}
+
+/*
+ * Redefine these libpq entry point names concerned with PGresults so that
+ * they will operate on libpqsrv_PGresults instead.  This avoids needing to
+ * convert a lot of pre-existing code, and reduces the notational differences
+ * between frontend and backend libpq-using code.
+ */
+#define PGresult libpqsrv_PGresult
+#define PQclear libpqsrv_PQclear
+#define PQgetResult libpqsrv_PQgetResult
+#define PQresultStatus libpqsrv_PQresultStatus
+#define PQresultErrorMessage libpqsrv_PQresultErrorMessage
+#define PQresultErrorField libpqsrv_PQresultErrorField
+#define PQcmdStatus libpqsrv_PQcmdStatus
+#define PQntuples libpqsrv_PQntuples
+#define PQnfields libpqsrv_PQnfields
+#define PQgetvalue libpqsrv_PQgetvalue
+#define PQgetlength libpqsrv_PQgetlength
+#define PQgetisnull libpqsrv_PQgetisnull
+#define PQfname libpqsrv_PQfname
+#define PQcmdTuples libpqsrv_PQcmdTuples
+
+#endif                            /* LIBPQ_BE_FE_H */
diff --git a/src/include/utils/palloc.h b/src/include/utils/palloc.h
index e1b42267b22..039b9cba61a 100644
--- a/src/include/utils/palloc.h
+++ b/src/include/utils/palloc.h
@@ -133,6 +133,8 @@ MemoryContextSwitchTo(MemoryContext context)
 /* Registration of memory context reset/delete callbacks */
 extern void MemoryContextRegisterResetCallback(MemoryContext context,
                                                MemoryContextCallback *cb);
+extern void MemoryContextUnregisterResetCallback(MemoryContext context,
+                                                 MemoryContextCallback *cb);

 /*
  * These are like standard strdup() except the copied string is
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index a8346cda633..0b1a8f71af6 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3714,6 +3714,7 @@ lclTocEntry
 leafSegmentInfo
 leaf_item
 libpq_source
+libpqsrv_PGresult
 line_t
 lineno_t
 list_sort_comparator
--
2.43.5

From 3d568261eedcecedeabdce585002c4ae2e4a480c Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 25 May 2025 19:49:42 -0400
Subject: [PATCH v3 2/2] Silence leakage complaint about postgres_fdw's
 InitPgFdwOptions.

Valgrind complains that the PQconninfoOption array returned by libpq
is leaked.  We apparently believed that we could suppress that warning
by storing that array's address in a static variable.  However, modern
C compilers are bright enough to optimize the static variable away.

We could escalate that arms race by making the variable global.
But on the whole it seems better to revise the code so that it
can free libpq's result properly.  The only thing that costs
us is copying the parameter-name keywords; which seems like a
pretty negligible cost in a function that runs at most once per
process.
---
 contrib/postgres_fdw/option.c | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index c2f936640bc..d6fa89bad93 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -21,6 +21,7 @@
 #include "libpq/libpq-be.h"
 #include "postgres_fdw.h"
 #include "utils/guc.h"
+#include "utils/memutils.h"
 #include "utils/varlena.h"

 /*
@@ -39,12 +40,6 @@ typedef struct PgFdwOption
  */
 static PgFdwOption *postgres_fdw_options;

-/*
- * Valid options for libpq.
- * Allocated and filled in InitPgFdwOptions.
- */
-static PQconninfoOption *libpq_options;
-
 /*
  * GUC parameters
  */
@@ -239,6 +234,7 @@ static void
 InitPgFdwOptions(void)
 {
     int            num_libpq_opts;
+    PQconninfoOption *libpq_options;
     PQconninfoOption *lopt;
     PgFdwOption *popt;

@@ -307,8 +303,8 @@ InitPgFdwOptions(void)
      * Get list of valid libpq options.
      *
      * To avoid unnecessary work, we get the list once and use it throughout
-     * the lifetime of this backend process.  We don't need to care about
-     * memory context issues, because PQconndefaults allocates with malloc.
+     * the lifetime of this backend process.  Hence, we'll allocate it in
+     * TopMemoryContext.
      */
     libpq_options = PQconndefaults();
     if (!libpq_options)            /* assume reason for failure is OOM */
@@ -325,19 +321,11 @@ InitPgFdwOptions(void)
     /*
      * Construct an array which consists of all valid options for
      * postgres_fdw, by appending FDW-specific options to libpq options.
-     *
-     * We use plain malloc here to allocate postgres_fdw_options because it
-     * lives as long as the backend process does.  Besides, keeping
-     * libpq_options in memory allows us to avoid copying every keyword
-     * string.
      */
     postgres_fdw_options = (PgFdwOption *)
-        malloc(sizeof(PgFdwOption) * num_libpq_opts +
-               sizeof(non_libpq_options));
-    if (postgres_fdw_options == NULL)
-        ereport(ERROR,
-                (errcode(ERRCODE_FDW_OUT_OF_MEMORY),
-                 errmsg("out of memory")));
+        MemoryContextAlloc(TopMemoryContext,
+                           sizeof(PgFdwOption) * num_libpq_opts +
+                           sizeof(non_libpq_options));

     popt = postgres_fdw_options;
     for (lopt = libpq_options; lopt->keyword; lopt++)
@@ -355,8 +343,8 @@ InitPgFdwOptions(void)
         if (strncmp(lopt->keyword, "oauth_", strlen("oauth_")) == 0)
             continue;

-        /* We don't have to copy keyword string, as described above. */
-        popt->keyword = lopt->keyword;
+        popt->keyword = MemoryContextStrdup(TopMemoryContext,
+                                            lopt->keyword);

         /*
          * "user" and any secret options are allowed only on user mappings.
@@ -371,6 +359,9 @@ InitPgFdwOptions(void)
         popt++;
     }

+    /* Done with libpq's output structure. */
+    PQconninfoFree(libpq_options);
+
     /* Append FDW-specific options and dummy terminator. */
     memcpy(popt, non_libpq_options, sizeof(non_libpq_options));
 }
--
2.43.5