]> granicus.if.org Git - postgresql/commitdiff
Add some marginal tweaks to eliminate memory leakages associated with
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 16 Sep 2004 20:17:49 +0000 (20:17 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 16 Sep 2004 20:17:49 +0000 (20:17 +0000)
subtransactions.  Trivial subxacts (such as a plpgsql exception block
containing no database access) now demonstrably leak zero bytes.

src/backend/access/transam/xact.c
src/backend/executor/spi.c
src/backend/utils/mmgr/aset.c
src/backend/utils/mmgr/mcxt.c
src/include/nodes/memnodes.h
src/include/utils/memutils.h

index fe99ba15f36f371aec6768d8edc8204800077d4e..17db7dd78d52f2126b669b1c9e88ade46b35ae23 100644 (file)
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.189 2004/09/16 16:58:26 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.190 2004/09/16 20:17:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -861,9 +861,6 @@ AtCommit_Memory(void)
 
 /*
  * AtSubCommit_Memory
- *
- * We do not throw away the child's CurTransactionContext, since the data
- * it contains will be needed at upper commit.
  */
 static void
 AtSubCommit_Memory(void)
@@ -875,6 +872,18 @@ AtSubCommit_Memory(void)
        /* Return to parent transaction level's memory context. */
        CurTransactionContext = s->parent->curTransactionContext;
        MemoryContextSwitchTo(CurTransactionContext);
+
+       /*
+        * Ordinarily we cannot throw away the child's CurTransactionContext,
+        * since the data it contains will be needed at upper commit.  However,
+        * if there isn't actually anything in it, we can throw it away.  This
+        * avoids a small memory leak in the common case of "trivial" subxacts.
+        */
+       if (MemoryContextIsEmpty(s->curTransactionContext))
+       {
+               MemoryContextDelete(s->curTransactionContext);
+               s->curTransactionContext = NULL;
+       }
 }
 
 /*
@@ -890,13 +899,27 @@ AtSubCommit_childXids(void)
 
        Assert(s->parent != NULL);
 
-       old_cxt = MemoryContextSwitchTo(s->parent->curTransactionContext);
+       /*
+        * We keep the child-XID lists in TopTransactionContext; this avoids
+        * setting up child-transaction contexts for what might be just a few
+        * bytes of grandchild XIDs.
+        */
+       old_cxt = MemoryContextSwitchTo(TopTransactionContext);
 
        s->parent->childXids = lappend_xid(s->parent->childXids,
                                                                           s->transactionId);
 
-       s->parent->childXids = list_concat(s->parent->childXids, s->childXids);
-       s->childXids = NIL;                     /* ensure list not doubly referenced */
+       if (s->childXids != NIL)
+       {
+               s->parent->childXids = list_concat(s->parent->childXids,
+                                                                                  s->childXids);
+               /*
+                * list_concat doesn't free the list header for the second list;
+                * do so here to avoid memory leakage (kluge)
+                */
+               pfree(s->childXids);
+               s->childXids = NIL;
+       }
 
        MemoryContextSwitchTo(old_cxt);
 }
@@ -1092,6 +1115,23 @@ AtSubAbort_Memory(void)
        MemoryContextSwitchTo(TopTransactionContext);
 }
 
+/*
+ * AtSubAbort_childXids
+ */
+static void
+AtSubAbort_childXids(void)
+{
+       TransactionState s = CurrentTransactionState;
+
+       /*
+        * We keep the child-XID lists in TopTransactionContext (see
+        * AtSubCommit_childXids).  This means we'd better free the list
+        * explicitly at abort to avoid leakage.
+        */
+       list_free(s->childXids);
+       s->childXids = NIL;
+}
+
 /*
  * RecordSubTransactionAbort
  */
@@ -3317,7 +3357,10 @@ AbortSubTransaction(void)
 
                /* Advertise the fact that we aborted in pg_clog. */
                if (TransactionIdIsValid(s->transactionId))
+               {
                        RecordSubTransactionAbort();
+                       AtSubAbort_childXids();
+               }
 
                /* Post-abort cleanup */
                CallSubXactCallbacks(SUBXACT_EVENT_ABORT_SUB, s->subTransactionId,
index ab26a91c1aa53a78b51ace630ea1a26004a0015a..3845b94eb92bf06daaee17ef792eb7b555a8de22 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.128 2004/09/16 16:58:29 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.129 2004/09/16 20:17:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -104,6 +104,8 @@ SPI_connect(void)
        _SPI_current = &(_SPI_stack[_SPI_connected]);
        _SPI_current->processed = 0;
        _SPI_current->tuptable = NULL;
+       _SPI_current->procCxt = NULL; /* in case we fail to create 'em */
+       _SPI_current->execCxt = NULL;
        _SPI_current->connectSubid = GetCurrentSubTransactionId();
 
        /*
@@ -144,7 +146,9 @@ SPI_finish(void)
 
        /* Release memory used in procedure call */
        MemoryContextDelete(_SPI_current->execCxt);
+       _SPI_current->execCxt = NULL;
        MemoryContextDelete(_SPI_current->procCxt);
+       _SPI_current->procCxt = NULL;
 
        /*
         * Reset result variables, especially SPI_tuptable which is probably
@@ -214,11 +218,24 @@ AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid)
 
                found = true;
 
+               /*
+                * Release procedure memory explicitly (see note in SPI_connect)
+                */
+               if (connection->execCxt)
+               {
+                       MemoryContextDelete(connection->execCxt);
+                       connection->execCxt = NULL;
+               }
+               if (connection->procCxt)
+               {
+                       MemoryContextDelete(connection->procCxt);
+                       connection->procCxt = NULL;
+               }
+
                /*
                 * Pop the stack entry and reset global variables.      Unlike
                 * SPI_finish(), we don't risk switching to memory contexts that
-                * might be already gone, or deleting memory contexts that have
-                * been or will be thrown away anyway.
+                * might be already gone.
                 */
                _SPI_connected--;
                _SPI_curid = _SPI_connected;
index 79da5fe01753c9057d735dd947e9efe95d22aea5..b25d870059aa2e99b583396b296bc63c51bbb2c0 100644 (file)
@@ -11,7 +11,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/mmgr/aset.c,v 1.57 2004/08/29 05:06:51 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/mmgr/aset.c,v 1.58 2004/09/16 20:17:33 tgl Exp $
  *
  * NOTE:
  *     This is a new (Feb. 05, 1999) implementation of the allocation set
@@ -205,6 +205,7 @@ static void AllocSetInit(MemoryContext context);
 static void AllocSetReset(MemoryContext context);
 static void AllocSetDelete(MemoryContext context);
 static Size AllocSetGetChunkSpace(MemoryContext context, void *pointer);
+static bool AllocSetIsEmpty(MemoryContext context);
 static void AllocSetStats(MemoryContext context);
 
 #ifdef MEMORY_CONTEXT_CHECKING
@@ -222,6 +223,7 @@ static MemoryContextMethods AllocSetMethods = {
        AllocSetReset,
        AllocSetDelete,
        AllocSetGetChunkSpace,
+       AllocSetIsEmpty,
        AllocSetStats
 #ifdef MEMORY_CONTEXT_CHECKING
        ,AllocSetCheck
@@ -991,6 +993,26 @@ AllocSetGetChunkSpace(MemoryContext context, void *pointer)
        return chunk->size + ALLOC_CHUNKHDRSZ;
 }
 
+/*
+ * AllocSetIsEmpty
+ *             Is an allocset empty of any allocated space?
+ */
+static bool
+AllocSetIsEmpty(MemoryContext context)
+{
+       AllocSet        set = (AllocSet) context;
+
+       /*
+        * For now, we say "empty" only if the context never contained any
+        * space at all.  We could examine the freelists to determine if all
+        * space has been freed, but it's not really worth the trouble for
+        * present uses of this functionality.
+        */
+       if (set->blocks == NULL)
+               return true;
+       return false;
+}
+
 /*
  * AllocSetStats
  *             Displays stats about memory consumption of an allocset.
index f8e0af4f8b03220b7a0419e3c6df52cf9a5f229f..581c7f396d8c782dcbc1d52926813a5c8b4ab02f 100644 (file)
@@ -14,7 +14,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/mmgr/mcxt.c,v 1.50 2004/08/29 05:06:51 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/mmgr/mcxt.c,v 1.51 2004/09/16 20:17:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -291,6 +291,25 @@ GetMemoryChunkContext(void *pointer)
        return header->context;
 }
 
+/*
+ * MemoryContextIsEmpty
+ *             Is a memory context empty of any allocated space?
+ */
+bool
+MemoryContextIsEmpty(MemoryContext context)
+{
+       AssertArg(MemoryContextIsValid(context));
+
+       /*
+        * For now, we consider a memory context nonempty if it has any children;
+        * perhaps this should be changed later.
+        */
+       if (context->firstchild != NULL)
+               return false;
+       /* Otherwise use the type-specific inquiry */
+       return (*context->methods->is_empty) (context);
+}
+
 /*
  * MemoryContextStats
  *             Print statistics about the named context and all its descendants.
@@ -662,7 +681,6 @@ void
 pgport_pfree(void *pointer)
 {
        pfree(pointer);
-       return;
 }
 
-#endif
+#endif /* WIN32 */
index 1fc13af47f7685121f590665691b50a75f8f85a3..e57ee2aad27bab332573feea5aedb5d6b1bf4042 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/memnodes.h,v 1.28 2004/08/29 04:13:07 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/memnodes.h,v 1.29 2004/09/16 20:17:42 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -43,6 +43,7 @@ typedef struct MemoryContextMethods
        void            (*reset) (MemoryContext context);
        void            (*delete) (MemoryContext context);
        Size            (*get_chunk_space) (MemoryContext context, void *pointer);
+       bool            (*is_empty) (MemoryContext context);
        void            (*stats) (MemoryContext context);
 #ifdef MEMORY_CONTEXT_CHECKING
        void            (*check) (MemoryContext context);
index 8cf7d8bc4697a6b39527d2c05f14efdb57e718ec..671cbbf1ef246cb5c7b4f0702538867505c70fe4 100644 (file)
@@ -10,7 +10,7 @@
  * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/utils/memutils.h,v 1.57 2004/08/29 04:13:11 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/utils/memutils.h,v 1.58 2004/09/16 20:17:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -91,6 +91,7 @@ extern void MemoryContextDeleteChildren(MemoryContext context);
 extern void MemoryContextResetAndDeleteChildren(MemoryContext context);
 extern Size GetMemoryChunkSpace(void *pointer);
 extern MemoryContext GetMemoryChunkContext(void *pointer);
+extern bool MemoryContextIsEmpty(MemoryContext context);
 extern void MemoryContextStats(MemoryContext context);
 
 #ifdef MEMORY_CONTEXT_CHECKING