-
Notifications
You must be signed in to change notification settings - Fork 212
Open
Description
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
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels