pgsql: Preserve CurrentMemoryContext across Start/CommitTransactionComm

Поиск
Список
Период
Сортировка
От Tom Lane
Тема pgsql: Preserve CurrentMemoryContext across Start/CommitTransactionComm
Дата
Msg-id E1sOJNF-003oA6-1g@gemulon.postgresql.org
обсуждение исходный текст
Список pgsql-committers
Preserve CurrentMemoryContext across Start/CommitTransactionCommand.

Up to now, committing a transaction has caused CurrentMemoryContext to
get set to TopMemoryContext.  Most callers did not pay any particular
heed to this, which is problematic because TopMemoryContext is a
long-lived context that never gets reset.  If the caller assumes it
can leak memory because it's running in a limited-lifespan context,
that behavior translates into a session-lifespan memory leak.

The first-reported instance of this involved ProcessIncomingNotify,
which is called from the main processing loop that normally runs in
MessageContext.  That outer-loop code assumes that whatever it
allocates will be cleaned up when we're done processing the current
client message --- but if we service a notify interrupt, then whatever
gets allocated before the next switch to MessageContext will be
permanently leaked in TopMemoryContext.  sinval catchup interrupts
have a similar problem, and I strongly suspect that some places in
logical replication do too.

To fix this in a generic way, let's redefine the behavior as
"CommitTransactionCommand restores the memory context that was current
at entry to StartTransactionCommand".  This clearly fixes the issue
for the notify and sinval cases, and it seems to match the mental
model that's in use in the logical replication code, to the extent
that anybody thought about it there at all.

For consistency, likewise make subtransaction exit restore the context
that was current at subtransaction start (rather than always selecting
the CurTransactionContext of the parent transaction level).  This case
has less risk of resulting in a permanent leak than the outer-level
behavior has, but it would not meet the principle of least surprise
for some CommitTransactionCommand calls to restore the previous
context while others don't.

While we're here, also change xact.c so that we reset
TopTransactionContext at transaction exit and then re-use it in later
transactions, rather than dropping and recreating it in each cycle.
This probably doesn't save a lot given the context recycling mechanism
in aset.c, but it should save a little bit.  Per suggestion from David
Rowley.  (Parenthetically, the text in src/backend/utils/mmgr/README
implies that this is how I'd planned to implement it as far back as
commit 1aebc3618 --- but the code actually added in that commit just
drops and recreates it each time.)

This commit also cleans up a few places outside xact.c that were
needlessly making TopMemoryContext current, and thus risking more
leaks of the same kind.  I don't think any of them represent
significant leak risks today, but let's deal with them while the
issue is top-of-mind.

Per bug #18512 from wizardbrony.  Commit to HEAD only, as this change
seems to have some risk of breaking things for some callers.  We'll
apply a narrower fix for the known-broken cases in the back branches.

Discussion: https://postgr.es/m/3478884.1718656625@sss.pgh.pa.us

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/1afe31f03cd268a0bbb7a340d56b8eef6419bcb0

Modified Files
--------------
src/backend/access/transam/xact.c   | 101 +++++++++++++++++++++++++-----------
src/backend/replication/walsender.c |   6 +--
src/backend/tcop/backend_startup.c  |   8 ++-
src/backend/tcop/postgres.c         |   2 +-
src/backend/utils/mmgr/mcxt.c       |   3 +-
5 files changed, 80 insertions(+), 40 deletions(-)


В списке pgsql-committers по дате отправления:

Предыдущее
От: Nathan Bossart
Дата:
Сообщение: pgsql: Add --no-sync to pg_upgrade's uses of pg_dump and pg_dumpall.
Следующее
От: Tom Lane
Дата:
Сообщение: pgsql: Preserve CurrentMemoryContext across notify and sinval interrupt