Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit1afe31f

Browse files
committed
Preserve CurrentMemoryContext across Start/CommitTransactionCommand.
Up to now, committing a transaction has caused CurrentMemoryContext toget set to TopMemoryContext. Most callers did not pay any particularheed to this, which is problematic because TopMemoryContext is along-lived context that never gets reset. If the caller assumes itcan 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 inMessageContext. That outer-loop code assumes that whatever itallocates will be cleaned up when we're done processing the currentclient message --- but if we service a notify interrupt, then whatevergets allocated before the next switch to MessageContext will bepermanently leaked in TopMemoryContext. sinval catchup interruptshave a similar problem, and I strongly suspect that some places inlogical replication do too.To fix this in a generic way, let's redefine the behavior as"CommitTransactionCommand restores the memory context that was currentat entry to StartTransactionCommand". This clearly fixes the issuefor the notify and sinval cases, and it seems to match the mentalmodel that's in use in the logical replication code, to the extentthat anybody thought about it there at all.For consistency, likewise make subtransaction exit restore the contextthat was current at subtransaction start (rather than always selectingthe CurTransactionContext of the parent transaction level). This casehas less risk of resulting in a permanent leak than the outer-levelbehavior has, but it would not meet the principle of least surprisefor some CommitTransactionCommand calls to restore the previouscontext while others don't.While we're here, also change xact.c so that we resetTopTransactionContext at transaction exit and then re-use it in latertransactions, rather than dropping and recreating it in each cycle.This probably doesn't save a lot given the context recycling mechanismin aset.c, but it should save a little bit. Per suggestion from DavidRowley. (Parenthetically, the text in src/backend/utils/mmgr/READMEimplies that this is how I'd planned to implement it as far back ascommit1aebc36 --- but the code actually added in that commit justdrops and recreates it each time.)This commit also cleans up a few places outside xact.c that wereneedlessly making TopMemoryContext current, and thus risking moreleaks of the same kind. I don't think any of them representsignificant leak risks today, but let's deal with them while theissue is top-of-mind.Per bug #18512 from wizardbrony. Commit to HEAD only, as this changeseems to have some risk of breaking things for some callers. We'llapply a narrower fix for the known-broken cases in the back branches.Discussion:https://postgr.es/m/3478884.1718656625@sss.pgh.pa.us
1 parent6e16b1e commit1afe31f

File tree

5 files changed

+80
-40
lines changed

5 files changed

+80
-40
lines changed

‎src/backend/access/transam/xact.c

Lines changed: 72 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ typedef struct TransactionStateData
200200
intgucNestLevel;/* GUC context nesting depth */
201201
MemoryContextcurTransactionContext;/* my xact-lifetime context */
202202
ResourceOwnercurTransactionOwner;/* my query resources */
203+
MemoryContextpriorContext;/* CurrentMemoryContext before xact started */
203204
TransactionId*childXids;/* subcommitted child XIDs, in XID order */
204205
intnChildXids;/* # of subcommitted child XIDs */
205206
intmaxChildXids;/* allocated size of childXids[] */
@@ -1174,6 +1175,11 @@ AtStart_Memory(void)
11741175
{
11751176
TransactionStates=CurrentTransactionState;
11761177

1178+
/*
1179+
* Remember the memory context that was active prior to transaction start.
1180+
*/
1181+
s->priorContext=CurrentMemoryContext;
1182+
11771183
/*
11781184
* If this is the first time through, create a private context for
11791185
* AbortTransaction to work in. By reserving some space now, we can
@@ -1190,17 +1196,15 @@ AtStart_Memory(void)
11901196
32*1024);
11911197

11921198
/*
1193-
* We shouldn't have a transaction context already.
1194-
*/
1195-
Assert(TopTransactionContext==NULL);
1196-
1197-
/*
1198-
* Create a toplevel context for the transaction.
1199+
* Likewise, if this is the first time through, create a top-level context
1200+
* for transaction-local data. This context will be reset at transaction
1201+
* end, and then re-used in later transactions.
11991202
*/
1200-
TopTransactionContext=
1201-
AllocSetContextCreate(TopMemoryContext,
1202-
"TopTransactionContext",
1203-
ALLOCSET_DEFAULT_SIZES);
1203+
if (TopTransactionContext==NULL)
1204+
TopTransactionContext=
1205+
AllocSetContextCreate(TopMemoryContext,
1206+
"TopTransactionContext",
1207+
ALLOCSET_DEFAULT_SIZES);
12041208

12051209
/*
12061210
* In a top-level transaction, CurTransactionContext is the same as
@@ -1251,6 +1255,11 @@ AtSubStart_Memory(void)
12511255

12521256
Assert(CurTransactionContext!=NULL);
12531257

1258+
/*
1259+
* Remember the context that was active prior to subtransaction start.
1260+
*/
1261+
s->priorContext=CurrentMemoryContext;
1262+
12541263
/*
12551264
* Create a CurTransactionContext, which will be used to hold data that
12561265
* survives subtransaction commit but disappears on subtransaction abort.
@@ -1576,20 +1585,30 @@ AtCCI_LocalCache(void)
15761585
staticvoid
15771586
AtCommit_Memory(void)
15781587
{
1588+
TransactionStates=CurrentTransactionState;
1589+
15791590
/*
1580-
* Now that we're "out" of a transaction, have the system allocate things
1581-
* in the top memory context instead of per-transaction contexts.
1591+
* Return to the memory context that was current before we started the
1592+
* transaction. (In principle, this could not be any of the contexts we
1593+
* are about to delete. If it somehow is, assertions in mcxt.c will
1594+
* complain.)
15821595
*/
1583-
MemoryContextSwitchTo(TopMemoryContext);
1596+
MemoryContextSwitchTo(s->priorContext);
15841597

15851598
/*
1586-
* Release all transaction-local memory.
1599+
* Release all transaction-local memory. TopTransactionContext survives
1600+
* but becomes empty; any sub-contexts go away.
15871601
*/
15881602
Assert(TopTransactionContext!=NULL);
1589-
MemoryContextDelete(TopTransactionContext);
1590-
TopTransactionContext=NULL;
1603+
MemoryContextReset(TopTransactionContext);
1604+
1605+
/*
1606+
* Clear these pointers as a pro-forma matter. (Notionally, while
1607+
* TopTransactionContext still exists, it's currently not associated with
1608+
* this TransactionState struct.)
1609+
*/
15911610
CurTransactionContext=NULL;
1592-
CurrentTransactionState->curTransactionContext=NULL;
1611+
s->curTransactionContext=NULL;
15931612
}
15941613

15951614
/* ----------------------------------------------------------------
@@ -1942,13 +1961,18 @@ AtSubAbort_childXids(void)
19421961
staticvoid
19431962
AtCleanup_Memory(void)
19441963
{
1945-
Assert(CurrentTransactionState->parent==NULL);
1964+
TransactionStates=CurrentTransactionState;
1965+
1966+
/* Should be at top level */
1967+
Assert(s->parent==NULL);
19461968

19471969
/*
1948-
* Now that we're "out" of a transaction, have the system allocate things
1949-
* in the top memory context instead of per-transaction contexts.
1970+
* Return to the memory context that was current before we started the
1971+
* transaction. (In principle, this could not be any of the contexts we
1972+
* are about to delete. If it somehow is, assertions in mcxt.c will
1973+
* complain.)
19501974
*/
1951-
MemoryContextSwitchTo(TopMemoryContext);
1975+
MemoryContextSwitchTo(s->priorContext);
19521976

19531977
/*
19541978
* Clear the special abort context for next time.
@@ -1957,13 +1981,20 @@ AtCleanup_Memory(void)
19571981
MemoryContextReset(TransactionAbortContext);
19581982

19591983
/*
1960-
* Release all transaction-local memory.
1984+
* Release all transaction-local memory, the same as in AtCommit_Memory,
1985+
* except we must cope with the possibility that we didn't get as far as
1986+
* creating TopTransactionContext.
19611987
*/
19621988
if (TopTransactionContext!=NULL)
1963-
MemoryContextDelete(TopTransactionContext);
1964-
TopTransactionContext=NULL;
1989+
MemoryContextReset(TopTransactionContext);
1990+
1991+
/*
1992+
* Clear these pointers as a pro-forma matter. (Notionally, while
1993+
* TopTransactionContext still exists, it's currently not associated with
1994+
* this TransactionState struct.)
1995+
*/
19651996
CurTransactionContext=NULL;
1966-
CurrentTransactionState->curTransactionContext=NULL;
1997+
s->curTransactionContext=NULL;
19671998
}
19681999

19692000

@@ -1982,8 +2013,15 @@ AtSubCleanup_Memory(void)
19822013

19832014
Assert(s->parent!=NULL);
19842015

1985-
/* Make sure we're not in an about-to-be-deleted context */
1986-
MemoryContextSwitchTo(s->parent->curTransactionContext);
2016+
/*
2017+
* Return to the memory context that was current before we started the
2018+
* subtransaction. (In principle, this could not be any of the contexts
2019+
* we are about to delete. If it somehow is, assertions in mcxt.c will
2020+
* complain.)
2021+
*/
2022+
MemoryContextSwitchTo(s->priorContext);
2023+
2024+
/* Update CurTransactionContext (might not be same as priorContext) */
19872025
CurTransactionContext=s->parent->curTransactionContext;
19882026

19892027
/*
@@ -4904,8 +4942,13 @@ AbortOutOfAnyTransaction(void)
49044942
/* Should be out of all subxacts now */
49054943
Assert(s->parent==NULL);
49064944

4907-
/* If we didn't actually have anything to do, revert to TopMemoryContext */
4908-
AtCleanup_Memory();
4945+
/*
4946+
* Revert to TopMemoryContext, to ensure we exit in a well-defined state
4947+
* whether there were any transactions to close or not. (Callers that
4948+
* don't intend to exit soon should switch to some other context to avoid
4949+
* long-term memory leaks.)
4950+
*/
4951+
MemoryContextSwitchTo(TopMemoryContext);
49094952
}
49104953

49114954
/*

‎src/backend/replication/walsender.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -440,12 +440,10 @@ IdentifySystem(void)
440440

441441
/* syscache access needs a transaction env. */
442442
StartTransactionCommand();
443-
/* make dbname live outside TX context */
444-
MemoryContextSwitchTo(cur);
445443
dbname=get_database_name(MyDatabaseId);
444+
/* copy dbname out of TX context */
445+
dbname=MemoryContextStrdup(cur,dbname);
446446
CommitTransactionCommand();
447-
/* CommitTransactionCommand switches to TopMemoryContext */
448-
MemoryContextSwitchTo(cur);
449447
}
450448

451449
dest=CreateDestReceiver(DestRemoteSimple);

‎src/backend/tcop/backend_startup.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,8 @@ BackendInitialize(ClientSocket *client_sock, CAC_state cac)
196196
* Save remote_host and remote_port in port structure (after this, they
197197
* will appear in log_line_prefix data for log messages).
198198
*/
199-
oldcontext=MemoryContextSwitchTo(TopMemoryContext);
200-
port->remote_host=pstrdup(remote_host);
201-
port->remote_port=pstrdup(remote_port);
199+
port->remote_host=MemoryContextStrdup(TopMemoryContext,remote_host);
200+
port->remote_port=MemoryContextStrdup(TopMemoryContext,remote_port);
202201

203202
/* And now we can issue the Log_connections message, if wanted */
204203
if (Log_connections)
@@ -230,9 +229,8 @@ BackendInitialize(ClientSocket *client_sock, CAC_state cac)
230229
strspn(remote_host,"0123456789.")<strlen(remote_host)&&
231230
strspn(remote_host,"0123456789ABCDEFabcdef:")<strlen(remote_host))
232231
{
233-
port->remote_hostname=pstrdup(remote_host);
232+
port->remote_hostname=MemoryContextStrdup(TopMemoryContext,remote_host);
234233
}
235-
MemoryContextSwitchTo(oldcontext);
236234

237235
/*
238236
* Ready to begin client interaction. We will give up and _exit(1) after

‎src/backend/tcop/postgres.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4418,7 +4418,7 @@ PostgresMain(const char *dbname, const char *username)
44184418
* Now return to normal top-level context and clear ErrorContext for
44194419
* next time.
44204420
*/
4421-
MemoryContextSwitchTo(TopMemoryContext);
4421+
MemoryContextSwitchTo(MessageContext);
44224422
FlushErrorState();
44234423

44244424
/*

‎src/backend/utils/mmgr/mcxt.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1148,7 +1148,8 @@ MemoryContextAllocationFailure(MemoryContext context, Size size, int flags)
11481148
{
11491149
if ((flags&MCXT_ALLOC_NO_OOM)==0)
11501150
{
1151-
MemoryContextStats(TopMemoryContext);
1151+
if (TopMemoryContext)
1152+
MemoryContextStats(TopMemoryContext);
11521153
ereport(ERROR,
11531154
(errcode(ERRCODE_OUT_OF_MEMORY),
11541155
errmsg("out of memory"),

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp