Re: Avoid memory leaks during base backups

On Wed, Sep 28, 2022 at 5:30 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> I'm attaching the v2 patch designed as described above. Please review it.
>
> I've added an entry in CF - https://commitfest.postgresql.org/40/3915/

This looks odd to me. In the case of a regular backend, the
sigsetjmp() handler in src/backend/tcop/postgres.c is responsible for
cleaning up memory. It calls AbortCurrentTransaction() which will call
CleanupTransaction() which will call AtCleanup_Memory() which will
block away TopTransactionContext. I think we ought to do something
analogous here, and we almost do already. Some walsender commands are
going to be SQL commands and some aren't. For those that aren't, the
same block calls WalSndErrorCleanup() which does similar kinds of
cleanup, including in some situations calling WalSndResourceCleanup()
which cleans up the resource owner in cases where we have a resource
owner without a transaction. I feel like we ought to be trying to tie
the cleanup into WalSndErrorCleanup() or WalSndResourceCleanup() based
on having the memory context that we ought to be blowing away stored
in a global variable, rather than using a try/catch block.

Like, maybe there's a function EstablishWalSenderMemoryContext() that
commands can call before allocating memory that shouldn't survive an
error. And it's deleted after each command if it exists, or if an
error occurs then WalSndErrorCleanup() deletes it.

--
Robert Haas
EDB: http://www.enterprisedb.com