Skip to content

Commit 257cccb

Browse files
tglsfdctglsfdc
committed
Add some marginal tweaks to eliminate memory leakages associated with
subtransactions. Trivial subxacts (such as a plpgsql exception block containing no database access) now demonstrably leak zero bytes.
1 parent 86fff99 commit 257cccb

File tree

6 files changed

+118
-16
lines changed

6 files changed

+118
-16
lines changed

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

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.189 2004/09/16 16:58:26 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.190 2004/09/16 20:17:16 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -861,9 +861,6 @@ AtCommit_Memory(void)
861861

862862
/*
863863
* AtSubCommit_Memory
864-
*
865-
* We do not throw away the child's CurTransactionContext, since the data
866-
* it contains will be needed at upper commit.
867864
*/
868865
static void
869866
AtSubCommit_Memory(void)
@@ -875,6 +872,18 @@ AtSubCommit_Memory(void)
875872
/* Return to parent transaction level's memory context. */
876873
CurTransactionContext = s->parent->curTransactionContext;
877874
MemoryContextSwitchTo(CurTransactionContext);
875+
876+
/*
877+
* Ordinarily we cannot throw away the child's CurTransactionContext,
878+
* since the data it contains will be needed at upper commit. However,
879+
* if there isn't actually anything in it, we can throw it away. This
880+
* avoids a small memory leak in the common case of "trivial" subxacts.
881+
*/
882+
if (MemoryContextIsEmpty(s->curTransactionContext))
883+
{
884+
MemoryContextDelete(s->curTransactionContext);
885+
s->curTransactionContext = NULL;
886+
}
878887
}
879888

880889
/*
@@ -890,13 +899,27 @@ AtSubCommit_childXids(void)
890899

891900
Assert(s->parent != NULL);
892901

893-
old_cxt = MemoryContextSwitchTo(s->parent->curTransactionContext);
902+
/*
903+
* We keep the child-XID lists in TopTransactionContext; this avoids
904+
* setting up child-transaction contexts for what might be just a few
905+
* bytes of grandchild XIDs.
906+
*/
907+
old_cxt = MemoryContextSwitchTo(TopTransactionContext);
894908

895909
s->parent->childXids = lappend_xid(s->parent->childXids,
896910
s->transactionId);
897911

898-
s->parent->childXids = list_concat(s->parent->childXids, s->childXids);
899-
s->childXids = NIL; /* ensure list not doubly referenced */
912+
if (s->childXids != NIL)
913+
{
914+
s->parent->childXids = list_concat(s->parent->childXids,
915+
s->childXids);
916+
/*
917+
* list_concat doesn't free the list header for the second list;
918+
* do so here to avoid memory leakage (kluge)
919+
*/
920+
pfree(s->childXids);
921+
s->childXids = NIL;
922+
}
900923

901924
MemoryContextSwitchTo(old_cxt);
902925
}
@@ -1092,6 +1115,23 @@ AtSubAbort_Memory(void)
10921115
MemoryContextSwitchTo(TopTransactionContext);
10931116
}
10941117

1118+
/*
1119+
* AtSubAbort_childXids
1120+
*/
1121+
static void
1122+
AtSubAbort_childXids(void)
1123+
{
1124+
TransactionState s = CurrentTransactionState;
1125+
1126+
/*
1127+
* We keep the child-XID lists in TopTransactionContext (see
1128+
* AtSubCommit_childXids). This means we'd better free the list
1129+
* explicitly at abort to avoid leakage.
1130+
*/
1131+
list_free(s->childXids);
1132+
s->childXids = NIL;
1133+
}
1134+
10951135
/*
10961136
* RecordSubTransactionAbort
10971137
*/
@@ -3317,7 +3357,10 @@ AbortSubTransaction(void)
33173357

33183358
/* Advertise the fact that we aborted in pg_clog. */
33193359
if (TransactionIdIsValid(s->transactionId))
3360+
{
33203361
RecordSubTransactionAbort();
3362+
AtSubAbort_childXids();
3363+
}
33213364

33223365
/* Post-abort cleanup */
33233366
CallSubXactCallbacks(SUBXACT_EVENT_ABORT_SUB, s->subTransactionId,

‎src/backend/executor/spi.c

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.128 2004/09/16 16:58:29 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.129 2004/09/16 20:17:20 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -104,6 +104,8 @@ SPI_connect(void)
104104
_SPI_current = &(_SPI_stack[_SPI_connected]);
105105
_SPI_current->processed = 0;
106106
_SPI_current->tuptable = NULL;
107+
_SPI_current->procCxt = NULL; /* in case we fail to create 'em */
108+
_SPI_current->execCxt = NULL;
107109
_SPI_current->connectSubid = GetCurrentSubTransactionId();
108110

109111
/*
@@ -144,7 +146,9 @@ SPI_finish(void)
144146

145147
/* Release memory used in procedure call */
146148
MemoryContextDelete(_SPI_current->execCxt);
149+
_SPI_current->execCxt = NULL;
147150
MemoryContextDelete(_SPI_current->procCxt);
151+
_SPI_current->procCxt = NULL;
148152

149153
/*
150154
* Reset result variables, especially SPI_tuptable which is probably
@@ -214,11 +218,24 @@ AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid)
214218

215219
found = true;
216220

221+
/*
222+
* Release procedure memory explicitly (see note in SPI_connect)
223+
*/
224+
if (connection->execCxt)
225+
{
226+
MemoryContextDelete(connection->execCxt);
227+
connection->execCxt = NULL;
228+
}
229+
if (connection->procCxt)
230+
{
231+
MemoryContextDelete(connection->procCxt);
232+
connection->procCxt = NULL;
233+
}
234+
217235
/*
218236
* Pop the stack entry and reset global variables. Unlike
219237
* SPI_finish(), we don't risk switching to memory contexts that
220-
* might be already gone, or deleting memory contexts that have
221-
* been or will be thrown away anyway.
238+
* might be already gone.
222239
*/
223240
_SPI_connected--;
224241
_SPI_curid = _SPI_connected;

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* Portions Copyright (c) 1994, Regents of the University of California
1212
*
1313
* IDENTIFICATION
14-
* $PostgreSQL: pgsql/src/backend/utils/mmgr/aset.c,v 1.57 2004/08/29 05:06:51 momjian Exp $
14+
* $PostgreSQL: pgsql/src/backend/utils/mmgr/aset.c,v 1.58 2004/09/16 20:17:33 tgl Exp $
1515
*
1616
* NOTE:
1717
* This is a new (Feb. 05, 1999) implementation of the allocation set
@@ -205,6 +205,7 @@ static void AllocSetInit(MemoryContext context);
205205
static void AllocSetReset(MemoryContext context);
206206
static void AllocSetDelete(MemoryContext context);
207207
static Size AllocSetGetChunkSpace(MemoryContext context, void *pointer);
208+
static bool AllocSetIsEmpty(MemoryContext context);
208209
static void AllocSetStats(MemoryContext context);
209210

210211
#ifdef MEMORY_CONTEXT_CHECKING
@@ -222,6 +223,7 @@ static MemoryContextMethods AllocSetMethods = {
222223
AllocSetReset,
223224
AllocSetDelete,
224225
AllocSetGetChunkSpace,
226+
AllocSetIsEmpty,
225227
AllocSetStats
226228
#ifdef MEMORY_CONTEXT_CHECKING
227229
,AllocSetCheck
@@ -991,6 +993,26 @@ AllocSetGetChunkSpace(MemoryContext context, void *pointer)
991993
return chunk->size + ALLOC_CHUNKHDRSZ;
992994
}
993995

996+
/*
997+
* AllocSetIsEmpty
998+
* Is an allocset empty of any allocated space?
999+
*/
1000+
static bool
1001+
AllocSetIsEmpty(MemoryContext context)
1002+
{
1003+
AllocSet set = (AllocSet) context;
1004+
1005+
/*
1006+
* For now, we say "empty" only if the context never contained any
1007+
* space at all. We could examine the freelists to determine if all
1008+
* space has been freed, but it's not really worth the trouble for
1009+
* present uses of this functionality.
1010+
*/
1011+
if (set->blocks == NULL)
1012+
return true;
1013+
return false;
1014+
}
1015+
9941016
/*
9951017
* AllocSetStats
9961018
* Displays stats about memory consumption of an allocset.

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

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
*
1515
*
1616
* IDENTIFICATION
17-
* $PostgreSQL: pgsql/src/backend/utils/mmgr/mcxt.c,v 1.50 2004/08/29 05:06:51 momjian Exp $
17+
* $PostgreSQL: pgsql/src/backend/utils/mmgr/mcxt.c,v 1.51 2004/09/16 20:17:33 tgl Exp $
1818
*
1919
*-------------------------------------------------------------------------
2020
*/
@@ -291,6 +291,25 @@ GetMemoryChunkContext(void *pointer)
291291
return header->context;
292292
}
293293

294+
/*
295+
* MemoryContextIsEmpty
296+
* Is a memory context empty of any allocated space?
297+
*/
298+
bool
299+
MemoryContextIsEmpty(MemoryContext context)
300+
{
301+
AssertArg(MemoryContextIsValid(context));
302+
303+
/*
304+
* For now, we consider a memory context nonempty if it has any children;
305+
* perhaps this should be changed later.
306+
*/
307+
if (context->firstchild != NULL)
308+
return false;
309+
/* Otherwise use the type-specific inquiry */
310+
return (*context->methods->is_empty) (context);
311+
}
312+
294313
/*
295314
* MemoryContextStats
296315
* Print statistics about the named context and all its descendants.
@@ -662,7 +681,6 @@ void
662681
pgport_pfree(void *pointer)
663682
{
664683
pfree(pointer);
665-
return;
666684
}
667685

668-
#endif
686+
#endif /* WIN32 */

‎src/include/nodes/memnodes.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/nodes/memnodes.h,v 1.28 2004/08/29 04:13:07 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/nodes/memnodes.h,v 1.29 2004/09/16 20:17:42 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -43,6 +43,7 @@ typedef struct MemoryContextMethods
4343
void (*reset) (MemoryContext context);
4444
void (*delete) (MemoryContext context);
4545
Size (*get_chunk_space) (MemoryContext context, void *pointer);
46+
bool (*is_empty) (MemoryContext context);
4647
void (*stats) (MemoryContext context);
4748
#ifdef MEMORY_CONTEXT_CHECKING
4849
void (*check) (MemoryContext context);

‎src/include/utils/memutils.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
1111
* Portions Copyright (c) 1994, Regents of the University of California
1212
*
13-
* $PostgreSQL: pgsql/src/include/utils/memutils.h,v 1.57 2004/08/29 04:13:11 momjian Exp $
13+
* $PostgreSQL: pgsql/src/include/utils/memutils.h,v 1.58 2004/09/16 20:17:49 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -91,6 +91,7 @@ extern void MemoryContextDeleteChildren(MemoryContext context);
9191
extern void MemoryContextResetAndDeleteChildren(MemoryContext context);
9292
extern Size GetMemoryChunkSpace(void *pointer);
9393
extern MemoryContext GetMemoryChunkContext(void *pointer);
94+
extern bool MemoryContextIsEmpty(MemoryContext context);
9495
extern void MemoryContextStats(MemoryContext context);
9596

9697
#ifdef MEMORY_CONTEXT_CHECKING

0 commit comments

Comments
 (0)