Skip to content

Leak because AllocSetDeleteFreeList isn't done right? #285

@MBkkt

Description

@MBkkt

Hello,

I think this patch implemented wrong way:
https://github.com/pganalyze/libpg_query/blob/17-latest/patches/06_alloc_set_delete_free_list.patch

Example is simple just allocate something with ALLOCSET_SMALL_SIZES.
And fix is simple:

diff --git a/src/postgres/src_backend_utils_mmgr_aset.c b/src/postgres/src_backend_utils_mmgr_aset.c
index 9795aa7..30c3420 100644
--- a/src/postgres/src_backend_utils_mmgr_aset.c
+++ b/src/postgres/src_backend_utils_mmgr_aset.c
@@ -1679,10 +1679,9 @@ AllocSetCheck(MemoryContext context)
 void
 AllocSetDeleteFreeList(MemoryContext context)
 {
-       AllocSet set = (AllocSet) context;
-       if (set->freeListIndex >= 0)
+       for (int index = 0; index != 2; ++index)
        {
-               AllocSetFreeList *freelist = &context_freelists[set->freeListIndex];
+               AllocSetFreeList *freelist = &context_freelists[index];
 
                while (freelist->first_free != NULL)
                {

But in general issue in my opinion is quite worse:
what if user for some needs created context in one thread and destroy in some other thread there no one called pg_query_init.

In such case there will be leak because context isn't created in such thread but free list won't be empty.

So in my opinion free list should be way to avoid putting context in this freelist.
I'm using such patch, it's useful for contexts which exist long time, in our case they're sql view objects with parsed state

diff --git a/src/postgres/include/utils/memutils.h b/src/postgres/include/utils/memutils.h
index 2fcf6cc..b4f26f1 100644
--- a/src/postgres/include/utils/memutils.h
+++ b/src/postgres/include/utils/memutils.h
@@ -113,7 +113,8 @@ extern MemoryContext AllocSetContextCreateInternal(MemoryContext parent,
 												   const char *name,
 												   Size minContextSize,
 												   Size initBlockSize,
-												   Size maxBlockSize);
+												   Size maxBlockSize,
+												   bool enableFreeListIndex);
 extern void AllocSetDeleteFreeList(MemoryContext context);
 
 /*
@@ -125,10 +126,10 @@ extern void AllocSetDeleteFreeList(MemoryContext context);
 #define AllocSetContextCreate(parent, name, ...) \
 	(StaticAssertExpr(__builtin_constant_p(name), \
 					  "memory context names must be constant strings"), \
-	 AllocSetContextCreateInternal(parent, name, __VA_ARGS__))
+	 AllocSetContextCreateInternal(parent, name, __VA_ARGS__, true))
 #else
-#define AllocSetContextCreate \
-	AllocSetContextCreateInternal
+#define AllocSetContextCreate(...) \
+	AllocSetContextCreateInternal(__VA_ARGS__, true)
 #endif
 
 /* slab.c */
diff --git a/src/postgres/src_backend_utils_mmgr_aset.c b/src/postgres/src_backend_utils_mmgr_aset.c
index 9795aa7..691c1ca 100644
--- a/src/postgres/src_backend_utils_mmgr_aset.c
+++ b/src/postgres/src_backend_utils_mmgr_aset.c
@@ -362,7 +362,8 @@ AllocSetContextCreateInternal(MemoryContext parent,
 							  const char *name,
 							  Size minContextSize,
 							  Size initBlockSize,
-							  Size maxBlockSize)
+							  Size maxBlockSize,
+							  bool enableFreeListIndex)
 {
 	int			freeListIndex;
 	Size		firstBlockSize;
@@ -428,6 +429,7 @@ AllocSetContextCreateInternal(MemoryContext parent,
 
 			/* Update its maxBlockSize; everything else should be OK */
 			set->maxBlockSize = maxBlockSize;
+			set->freeListIndex = enableFreeListIndex ? freeListIndex : -1;
 
 			/* Reinitialize its header, installing correct name and parent */
 			MemoryContextCreate((MemoryContext) set,
@@ -494,7 +496,7 @@ AllocSetContextCreateInternal(MemoryContext parent,
 	set->initBlockSize = initBlockSize;
 	set->maxBlockSize = maxBlockSize;
 	set->nextBlockSize = initBlockSize;
-	set->freeListIndex = freeListIndex;
+	set->freeListIndex = enableFreeListIndex ? freeListIndex : -1;
 
 	/*
 	 * Compute the allocation chunk size limit for this context.  It can't be

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions