Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: postgresql-cfbot/postgresql
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: cf/5772~1
Choose a base ref
...
head repository: postgresql-cfbot/postgresql
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: cf/5772
Choose a head ref
  • 5 commits
  • 11 files changed
  • 2 contributors

Commits on Jul 24, 2025

  1. Create infrastructure to reliably prevent leakage of PGresults. 

    Commit 232d8ca fixed a case where postgres_fdw could lose track
of a PGresult object, resulting in a process-lifespan memory leak.
But I have little faith that there aren't other potential PGresult
leakages, now or in future, in the backend modules that use libpq.
Therefore, this patch proposes infrastructure that makes all
PGresults returned from libpq act as though they are palloc'd
in the CurrentMemoryContext (with the option to relocate them to
another context later).  This should greatly reduce the risk of
careless leaks, and it also permits removal of a bunch of code
that attempted to prevent such leaks via PG_TRY blocks.

This patch adds infrastructure that wraps each PGresult in a
"libpqsrv_PGresult" that provides a memory context reset callback
to PQclear the PGresult.  Code using this abstraction is inherently
memory-safe to the same extent as we are accustomed to in most backend
code.  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 in all the backend code that
uses libpq.

Perhaps in future we could create similar infrastructure for
PGconn objects, but there seems less need for that.

This patch just creates the infrastructure and makes relevant code
use it, including reverting 232d8ca in favor of this mechanism.
A good deal of follow-on simplification is possible now that we don't
have to be so cautious about freeing PGresults, but I'll put that in
a separate patch.

Author: Tom Lane <[email protected]>
Reviewed-by: Matheus Alcantara <[email protected]>
Discussion: https://postgr.es/m/[email protected]
    @tglsfdc
    tglsfdc authored and Commitfest Bot committed Jul 24, 2025
    Configuration menu
    Copy the full SHA
    7cbb251 View commit details
    Browse the repository at this point in the history

  2. Reap the benefits of not having to avoid leaking PGresults. 

    Remove a bunch of PG_TRY constructs, de-volatilize related
variables, remove some PQclear calls in error paths.
Aside from making the code simpler and shorter, this should
provide some marginal performance gains.

For ease of review, I did not re-indent code within the removed
PG_TRY constructs.  That'll be done in a separate patch.

Author: Tom Lane <[email protected]>
Reviewed-by: Matheus Alcantara <[email protected]>
Discussion: https://postgr.es/m/[email protected]
    @tglsfdc
    tglsfdc authored and Commitfest Bot committed Jul 24, 2025
    Configuration menu
    Copy the full SHA
    25fdc59 View commit details
    Browse the repository at this point in the history

  3. Run pgindent on the changes of the previous patch. 

    This step can be checked mechanically.

Author: Tom Lane <[email protected]>
Reviewed-by: Matheus Alcantara <[email protected]>
Discussion: https://postgr.es/m/[email protected]
    @tglsfdc
    tglsfdc authored and Commitfest Bot committed Jul 24, 2025
    Configuration menu
    Copy the full SHA
    f426631 View commit details
    Browse the repository at this point in the history

  4. 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.

Author: Tom Lane <[email protected]>
Reviewed-by: Matheus Alcantara <[email protected]>
Discussion: https://postgr.es/m/[email protected]
    @tglsfdc
    tglsfdc authored and Commitfest Bot committed Jul 24, 2025
    Configuration menu
    Copy the full SHA
    3019609 View commit details
    Browse the repository at this point in the history

  5. [CF 5772] v8 - Fixing memory leaks in postgres_fdw 

    This branch was automatically generated by a robot using patches from an
email thread registered at:

https://commitfest.postgresql.org/patch/5772

The branch will be overwritten each time a new patch version is posted to
the thread, and also periodically to check for bitrot caused by changes
on the master branch.

Patch(es): https://www.postgresql.org/message-id/[email protected]
Author(s): Tom Lane
    Commitfest Bot committed Jul 24, 2025
    Configuration menu
    Copy the full SHA
    a3fefd4 View commit details
    Browse the repository at this point in the history
Loading