]> granicus.if.org Git - postgresql/commitdiff
Here is the patch with memory leak checker. This checker allow detect
authorBruce Momjian <bruce@momjian.us>
Tue, 11 Jul 2000 14:30:37 +0000 (14:30 +0000)
committerBruce Momjian <bruce@momjian.us>
Tue, 11 Jul 2000 14:30:37 +0000 (14:30 +0000)
in-chunk leaks, overwrite-next-chunk leaks and overwrite block-freeptr leaks.

A in-chunk leak --- if something overwrite space after wanted (via palloc()
size, but it is still inside chunk. For example

        x = palloc(12);         /* create 16b chunk */
        memset(x, '#', 13);

this leak is in the current source total invisible, because chunk is 16b and
leak is in the "align space".

 For this feature I add data_size to StandardChunk, and all memory which go
from AllocSetAlloc() is marked as 0x7F.

 The MemoryContextCheck() is compiled '#ifdef USE_ASSERT_CHECKING'.

I add this checking to 'tcop/postgres.c' and is active after each backend
query, but it is probably not sufficient, because some MemoryContext exist
only during memory processing --- will good if someone who known where
it is needful (Tom:-) add it for others contexts;
 A problem in the current source is that we have still some malloc()
allocation that is not needful and this allocation is total invisible for
all context routines. For example Dllist in backend (pretty dirty it is in
catcache where values in Dllist are palloc-ed, but list is malloc-ed).
--- and BTW. this Dllist design stand in the way for query cache :-)

 Tom, if you agree I start replace some mallocs.

 BTW. --- Tom, have you idea for across transaction presistent allocation for
          SQL functions? (like regex - now it is via malloc)

 I almost forget. I add one if() to AllocSetAlloc(), for 'size' that are
greater than ALLOC_BIGCHUNK_LIMIT is not needful check AllocSetFreeIndex(),
because 'fidx' is always 'ALLOCSET_NUM_FREELISTS - 1'. It a little brisk up
allocation for very large chunks. Right?

                                                Karel

src/backend/tcop/postgres.c
src/backend/utils/mmgr/aset.c
src/backend/utils/mmgr/mcxt.c
src/include/config.h.in
src/include/nodes/memnodes.h
src/include/utils/memutils.h

index 575e18351ad8cf4d044cb9d94f774d3c867986ee..3c672d8f4b289a723389eaf93cf2e58a9327ac84 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.167 2000/07/08 03:04:15 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.168 2000/07/11 14:30:27 momjian Exp $
  *
  * NOTES
  *       this is the "main" module of the postgres backend and
@@ -1415,7 +1415,7 @@ PostgresMain(int argc, char *argv[], int real_argc, char *real_argv[])
        if (!IsUnderPostmaster)
        {
                puts("\nPOSTGRES backend interactive interface ");
-               puts("$Revision: 1.167 $ $Date: 2000/07/08 03:04:15 $\n");
+               puts("$Revision: 1.168 $ $Date: 2000/07/11 14:30:27 $\n");
        }
 
        /*
@@ -1653,6 +1653,13 @@ PostgresMain(int argc, char *argv[], int real_argc, char *real_argv[])
                        if (IsUnderPostmaster)
                                NullCommand(Remote);
                }
+
+#ifdef MEMORY_CONTEXT_CHECKING
+               /*
+                * Check all memory after each backend loop
+                */
+               MemoryContextCheck(TopMemoryContext);   
+#endif
        }                                                       /* infinite for-loop */
 
        proc_exit(0);                           /* shouldn't get here... */
index e0244ade7836e84bea53e7cfea5e1f071f39fdcf..bf212ff7bb5f5da4d46684d75d5d7b42a2e08ff0 100644 (file)
@@ -11,7 +11,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/utils/mmgr/aset.c,v 1.28 2000/06/28 03:32:50 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/utils/mmgr/aset.c,v 1.29 2000/07/11 14:30:28 momjian Exp $
  *
  * NOTE:
  *     This is a new (Feb. 05, 1999) implementation of the allocation set
@@ -38,6 +38,9 @@
 
 #include "utils/memutils.h"
 
+/* Define this to detail debug alloc information 
+ */
+/*#define HAVE_ALLOCINFO       1*/
 
 /*
  * AllocSetContext is defined in nodes/memnodes.h.
@@ -79,9 +82,12 @@ typedef struct AllocBlockData
 typedef struct AllocChunkData
 {
        /* aset is the owning aset if allocated, or the freelist link if free */
-       void       *aset;
+       void    *aset;
        /* size is always the size of the usable space in the chunk */
-       Size            size;
+       Size    size;
+#ifdef MEMORY_CONTEXT_CHECKING
+       Size    data_size;      
+#endif                 
 } AllocChunkData;
 
 /*
@@ -161,6 +167,11 @@ static void *AllocSetRealloc(MemoryContext context, void *pointer, Size size);
 static void AllocSetInit(MemoryContext context);
 static void AllocSetReset(MemoryContext context);
 static void AllocSetDelete(MemoryContext context);
+
+#ifdef MEMORY_CONTEXT_CHECKING
+static void AllocSetCheck(MemoryContext context);
+#endif
+
 static void AllocSetStats(MemoryContext context);
 
 /*
@@ -173,10 +184,29 @@ static MemoryContextMethods AllocSetMethods = {
        AllocSetInit,
        AllocSetReset,
        AllocSetDelete,
+#ifdef MEMORY_CONTEXT_CHECKING
+       AllocSetCheck,
+#endif
        AllocSetStats
 };
 
 
+/* ----------
+ * Debug macros
+ * ----------
+ */
+#ifdef HAVE_ALLOCINFO
+#define AllocFreeInfo(_cxt, _chunk) \
+                       fprintf(stderr, "AllocFree: %s: %p, %d\n", \
+                               (_cxt)->header.name, (_chunk), (_chunk)->size)
+#define AllocAllocInfo(_cxt, _chunk) \
+                       fprintf(stderr, "AllocAlloc: %s: %p, %d\n", \
+                               (_cxt)->header.name, (_chunk), (_chunk)->size)
+#else
+#define AllocFreeInfo(_cxt, _chunk)
+#define AllocAllocInfo(_cxt, _chunk)
+#endif 
+
 /* ----------
  * AllocSetFreeIndex -
  *
@@ -263,6 +293,11 @@ AllocSetContextCreate(MemoryContext parent,
                context->blocks = block;
                /* Mark block as not to be released at reset time */
                context->keeper = block;
+
+#ifdef MEMORY_CONTEXT_CHECKING
+               /* mark memory for memory leak searching */
+               memset(block->freeptr, 0x7F, blksize - ALLOC_BLOCKHDRSZ);
+#endif
        }
 
        return (MemoryContext) context;
@@ -386,38 +421,53 @@ AllocSetAlloc(MemoryContext context, Size size)
        AllocBlock      block;
        AllocChunk      chunk;
        AllocChunk      priorfree = NULL;
-       int                     fidx;
+       int             fidx;
        Size            chunk_size;
        Size            blksize;
 
        AssertArg(AllocSetIsValid(set));
 
        /*
-        * Lookup in the corresponding free list if there is a free chunk we
-        * could reuse
+        * Small size can be in free list 
         */
-       fidx = AllocSetFreeIndex(size);
-       for (chunk = set->freelist[fidx]; chunk; chunk = (AllocChunk) chunk->aset)
+       if (size < ALLOC_BIGCHUNK_LIMIT)
        {
-               if (chunk->size >= size)
-                       break;
-               priorfree = chunk;
-       }
+               /*
+                * Lookup in the corresponding free list if there is a free chunk we
+                * could reuse
+                */
+               fidx = AllocSetFreeIndex(size);
+               for (chunk = set->freelist[fidx]; chunk; chunk = (AllocChunk) chunk->aset)
+               {
+                       if (chunk->size >= size)
+                               break;
+                       priorfree = chunk;
+               }
 
-       /*
-        * If one is found, remove it from the free list, make it again a
-        * member of the alloc set and return its data address.
-        */
-       if (chunk != NULL)
-       {
-               if (priorfree == NULL)
-                       set->freelist[fidx] = (AllocChunk) chunk->aset;
-               else
-                       priorfree->aset = chunk->aset;
+               /*
+                * If one is found, remove it from the free list, make it again a
+                * member of the alloc set and return its data address.
+                */
+               if (chunk != NULL)
+               {
+                       if (priorfree == NULL)
+                               set->freelist[fidx] = (AllocChunk) chunk->aset;
+                       else
+                               priorfree->aset = chunk->aset;
+       
+                       chunk->aset = (void *) set;
 
-               chunk->aset = (void *) set;
-               return AllocChunkGetPointer(chunk);
-       }
+#ifdef MEMORY_CONTEXT_CHECKING
+                       chunk->data_size = size;
+#endif
+                       AllocAllocInfo(set, chunk);
+                       return AllocChunkGetPointer(chunk);
+               }
+       } 
+       else
+               /* Big chunk
+                */
+               fidx = ALLOCSET_NUM_FREELISTS - 1;
 
        /*
         * Choose the actual chunk size to allocate.
@@ -474,7 +524,13 @@ AllocSetAlloc(MemoryContext context, Size size)
                        block->next = NULL;
                        set->blocks = block;
                }
-
+               
+#ifdef MEMORY_CONTEXT_CHECKING
+               chunk->data_size = size;
+               /* mark memory for memory leak searching */
+               memset(AllocChunkGetPointer(chunk), 0x7F, chunk->size);         
+#endif
+               AllocAllocInfo(set, chunk);
                return AllocChunkGetPointer(chunk);
        }
 
@@ -524,9 +580,15 @@ AllocSetAlloc(MemoryContext context, Size size)
 
                if (block == NULL)
                        elog(ERROR, "Memory exhausted in AllocSetAlloc()");
+                       
                block->aset = set;
                block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ;
                block->endptr = ((char *) block) + blksize;
+
+#ifdef MEMORY_CONTEXT_CHECKING
+               /* mark memory for memory leak searching */
+               memset(block->freeptr, 0x7F, blksize - ALLOC_BLOCKHDRSZ);
+#endif
                block->next = set->blocks;
 
                set->blocks = block;
@@ -538,9 +600,14 @@ AllocSetAlloc(MemoryContext context, Size size)
        chunk = (AllocChunk) (block->freeptr);
        chunk->aset = (void *) set;
        chunk->size = chunk_size;
+       
+#ifdef MEMORY_CONTEXT_CHECKING
+       chunk->data_size = size;
+#endif 
        block->freeptr += (chunk_size + ALLOC_CHUNKHDRSZ);
        Assert(block->freeptr <= block->endptr);
 
+       AllocAllocInfo(set, chunk);
        return AllocChunkGetPointer(chunk);
 }
 
@@ -554,14 +621,17 @@ AllocSetFree(MemoryContext context, void *pointer)
        AllocSet        set = (AllocSet) context;
        AllocChunk      chunk = AllocPointerGetChunk(pointer);
 
-#ifdef CLOBBER_FREED_MEMORY
-       /* Wipe freed memory for debugging purposes */
+#if defined(CLOBBER_FREED_MEMORY) || defined(MEMORY_CONTEXT_CHECKING)
+       /* Wipe freed memory for debugging purposes or for memory leak 
+        * searching (in freelist[] must be mark memory
+        */
        memset(pointer, 0x7F, chunk->size);
 #endif
 
+       AllocFreeInfo(set, chunk);
+
        if (chunk->size >= ALLOC_BIGCHUNK_LIMIT)
        {
-
                /*
                 * Big chunks are certain to have been allocated as single-chunk
                 * blocks.      Find the containing block and return it to malloc().
@@ -598,6 +668,10 @@ AllocSetFree(MemoryContext context, void *pointer)
                int                     fidx = AllocSetFreeIndex(chunk->size);
 
                chunk->aset = (void *) set->freelist[fidx];
+
+#ifdef MEMORY_CONTEXT_CHECKING
+               chunk->data_size = 0;
+#endif         
                set->freelist[fidx] = chunk;
        }
 }
@@ -637,6 +711,10 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
                AllocBlock      prevblock = NULL;
                Size            blksize;
 
+#ifdef MEMORY_CONTEXT_CHECKING         
+               Size            data_size = size;
+#endif
+
                while (block != NULL)
                {
                        if (chunk == (AllocChunk) (((char *) block) + ALLOC_BLOCKHDRSZ))
@@ -665,6 +743,13 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
                else
                        prevblock->next = block;
                chunk->size = size;
+               
+#ifdef MEMORY_CONTEXT_CHECKING
+               /* mark memory for memory leak searching */
+               memset(((char *) chunk) + (ALLOC_CHUNKHDRSZ + data_size), 
+                               0x7F, size - data_size);
+               chunk->data_size = data_size;
+#endif
                return AllocChunkGetPointer(chunk);
        }
        else
@@ -720,3 +805,126 @@ AllocSetStats(MemoryContext context)
                        set->header.name, totalspace, nblocks, freespace, nchunks,
                        totalspace - freespace);
 }
+
+
+/* 
+ * AllocSetCheck
+ *             Walk on chunks and check consistence of memory. 
+ */
+#ifdef MEMORY_CONTEXT_CHECKING
+
+static void 
+AllocSetCheck(MemoryContext context)
+{
+       AllocSet        set = (AllocSet) context;       
+       AllocBlock      block = NULL;
+       AllocChunk      chunk = NULL;
+       char            *name = set->header.name;
+
+       for (block = set->blocks; block != NULL; block = block->next)
+       {       
+               char    *bpoz = ((char *) block) + ALLOC_BLOCKHDRSZ;
+       /*      long    blk_size = block->endptr - ((char *) block);*/
+               long    blk_free = block->endptr - block->freeptr; 
+               long    blk_used = block->freeptr - bpoz;
+               long    blk_data = 0;
+               long    nchunks  = 0;
+
+               /*
+                * Empty block - empty can be keeper-block only
+                */
+               if (!blk_used)
+               {
+                       if (set->keeper == block)
+                               continue;
+                       else    
+                               elog(ERROR, "AllocSetCheck(): %s: empty block %p", 
+                                               name, block);
+               }       
+               
+               /*
+                * Chunk walker
+                */     
+               do {
+                       Size    chsize,
+                               dsize;
+                       char    *chdata_end,
+                               *chend;
+                               
+                       chunk = (AllocChunk) bpoz;
+                       
+                       chsize = chunk->size;           /* align chunk size */
+                       dsize = chunk->data_size;       /* real data */
+                       
+                       chdata_end = ((char *) chunk) + (ALLOC_CHUNKHDRSZ + dsize);
+                       chend = ((char *) chunk) + (ALLOC_CHUNKHDRSZ + chsize);
+                       
+                       if (!dsize && chsize < dsize)
+                               elog(ERROR, "AllocSetCheck(): %s: internal error for chunk %p in block %p",
+                                               name, chunk, block);                    
+                       /*
+                        * Check chunk size
+                        */
+                       if (chsize < (1 << ALLOC_MINBITS))
+                               elog(ERROR, "AllocSetCheck(): %s: bad size '%d' for chunk %p in block %p",
+                                               name, chsize, chunk, block);
+                                               
+                       /* single-chunk block */
+                       if (chsize >= ALLOC_BIGCHUNK_LIMIT &&
+                           chsize + ALLOC_CHUNKHDRSZ != blk_used)
+                               elog(ERROR, "AllocSetCheck(): %s: bad singel-chunk %p in block %p",
+                                               name, chunk, block);
+                                               
+                       /*
+                        * Check in-chunk leak
+                        */             
+                       if (dsize < chsize && *chdata_end != 0x7F)
+                       {
+                               fprintf(stderr, "\n--- Leak %p ---\n", chdata_end);
+                               fprintf(stderr, "Chunk dump size: %ld (chunk-header %ld + chunk-size: %d), data must be: %d\n--- dump begin ---\n", 
+                                       chsize + ALLOC_CHUNKHDRSZ, 
+                                       ALLOC_CHUNKHDRSZ, chsize, dsize);
+                                       
+                               fwrite((void *) chunk, chsize+ALLOC_CHUNKHDRSZ, sizeof(char), stderr);
+                               fputs("\n--- dump end ---\n", stderr);
+                               
+                               elog(ERROR, "AllocSetCheck(): %s: found in-chunk memory leak (block %p; chunk %p; leak at %p",
+                                               name, block, chunk, chdata_end);
+                       }
+                       
+                       /*
+                        * Check block-freeptr leak 
+                        */
+                       if (chend == block->freeptr && blk_free && 
+                                                       *chdata_end != 0x7F) {
+                               
+                               fprintf(stderr, "\n--- Leak %p ---\n", chdata_end);
+                               fprintf(stderr, "Dump size: %ld (chunk-header %ld + chunk-size: %d + block-freespace: %ld), data must be: %d\n--- dump begin ---\n", 
+                                       chsize + ALLOC_CHUNKHDRSZ + blk_free, 
+                                       ALLOC_CHUNKHDRSZ, chsize, blk_free, dsize);
+                                       
+                               fwrite((void *) chunk, chsize+ALLOC_CHUNKHDRSZ+blk_free, sizeof(char), stderr);
+                               fputs("\n--- dump end ---\n", stderr);
+                               
+                               elog(ERROR, "AllocSetCheck(): %s: found block-freeptr memory leak (block %p; chunk %p; leak at %p",
+                                               name, block, chunk, chdata_end);
+                       }                       
+                       
+                       blk_data += chsize;
+                       nchunks++;
+                       
+                       if (chend < block->freeptr)
+                               bpoz += ALLOC_CHUNKHDRSZ + chsize;
+                       else
+                               break;
+
+               } while(block->freeptr > bpoz); /* chunk walker */              
+       
+               
+               if ((blk_data + (nchunks * ALLOC_CHUNKHDRSZ)) != blk_used)
+
+                       elog(ERROR, "AllocSetCheck(): %s: found non-consistent memory block %p",
+                                       name, block);
+       }
+}
+#endif
index 71877c4e62a5edb080dfd35341dfbdbb0fce83d2..874f609671d08aa20e9f9d8e5cdcdb024c7e0067 100644 (file)
@@ -14,7 +14,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/utils/mmgr/mcxt.c,v 1.22 2000/06/28 03:32:50 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/utils/mmgr/mcxt.c,v 1.23 2000/07/11 14:30:28 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -238,6 +238,27 @@ MemoryContextStats(MemoryContext context)
        }
 }
 
+
+/*
+ * MemoryContextCheck
+ *             Check all chunks in the named context.
+ *
+ * This is just a debugging utility, so it's not fancy.  
+ */
+#ifdef MEMORY_CONTEXT_CHECKING
+void
+MemoryContextCheck(MemoryContext context)
+{
+       MemoryContext   child;
+
+       (*context->methods->check) (context);
+       for (child = context->firstchild; child != NULL; child = child->nextchild)
+       {
+               MemoryContextCheck(child);
+       }
+}
+#endif
+
 /*
  * MemoryContextContains
  *             Detect whether an allocated chunk of memory belongs to a given
index 25e8be291ff899942010ce6cbee18a03d4d703e5..583c20dc5455cd19b9a69c3685cda9533890eee1 100644 (file)
@@ -8,7 +8,7 @@
  * or in config.h afterwards.  Of course, if you edit config.h, then your
  * changes will be overwritten the next time you run configure.
  *
- * $Id: config.h.in,v 1.124 2000/07/09 13:14:13 petere Exp $
+ * $Id: config.h.in,v 1.125 2000/07/11 14:30:30 momjian Exp $
  */
 
 #ifndef CONFIG_H
 #define CLOBBER_FREED_MEMORY
 #endif
 
+/* Define this to check memory leaks
+ */
+#ifdef USE_ASSERT_CHECKING 
+#define MEMORY_CONTEXT_CHECKING
+#endif
+
 /* Define this to force all parse and plan trees to be passed through
  * copyObject(), to facilitate catching errors and omissions in copyObject().
  * XXX For 7.1 development, define this automatically if --enable-cassert.
index abc38e5c60d26ac22c52f70ff37e7139ed37aa0f..4296795e5d1a12d023034acf544b7a8b5a89c499 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: memnodes.h,v 1.17 2000/06/28 03:33:15 tgl Exp $
+ * $Id: memnodes.h,v 1.18 2000/07/11 14:30:34 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -42,6 +42,9 @@ typedef struct MemoryContextMethods
        void            (*init) (MemoryContext context);
        void            (*reset) (MemoryContext context);
        void            (*delete) (MemoryContext context);
+#ifdef MEMORY_CONTEXT_CHECKING 
+       void            (*check) (MemoryContext context);
+#endif
        void            (*stats) (MemoryContext context);
 } MemoryContextMethods;
 
index cbabdbf275efaf2866a23aa00ced6a076f89938f..be322cbccc3dc6c7c2105ce84c2fbe31daef3468 100644 (file)
@@ -10,7 +10,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: memutils.h,v 1.36 2000/06/28 03:33:33 tgl Exp $
+ * $Id: memutils.h,v 1.37 2000/07/11 14:30:37 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -48,6 +48,9 @@ typedef struct StandardChunkHeader
 {
        MemoryContext context;                  /* owning context */
        Size size;                                              /* size of data space allocated in chunk */
+#ifdef MEMORY_CONTEXT_CHECKING
+       Size data_size;                         /* real data size  (without align) */
+#endif
 } StandardChunkHeader;
 
 #define STANDARDCHUNKHEADERSIZE  MAXALIGN(sizeof(StandardChunkHeader))
@@ -78,6 +81,7 @@ extern void MemoryContextResetChildren(MemoryContext context);
 extern void MemoryContextDeleteChildren(MemoryContext context);
 extern void MemoryContextResetAndDeleteChildren(MemoryContext context);
 extern void MemoryContextStats(MemoryContext context);
+extern void MemoryContextCheck(MemoryContext context);
 extern bool MemoryContextContains(MemoryContext context, void *pointer);
 
 /*