public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tom Lane <[email protected]>
To: David Rowley <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: BUG #19438: segfault with temp_file_limit inside cursor
Date: Sun, 29 Mar 2026 11:32:54 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAApHDvox3Ro8mZJxignuyB-dGXJ9=wQNEkOFni9025GP=rOKkg@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<[email protected]>
<CAApHDvox3Ro8mZJxignuyB-dGXJ9=wQNEkOFni9025GP=rOKkg@mail.gmail.com>
David Rowley <[email protected]> writes:
> For the switching MemoryContextMethodID patch, I applied the memory
> context benchmarking patch I used when writing that code to test out
> the overhead in a tight palloc/pfree loop (attached). I can see an
> overhead of a little over 6.5%.
Hm. I got an overhead of about 2% on an Apple M4, which might be
argued to be acceptable, but 12% on an aging x86_64 platform.
Realistically, given that we failed to notice this omission at
all for more than three years, it's hard to argue that testing
for it in non-debug builds is worth any overhead.
Here's a fleshed-out version of the requested_size method.
I noted that AllocSetRealloc needs a defense too, and then
extended the patch to generation.c and slab.c. bump.c
doesn't have an issue, and I don't think alignedalloc.c
needs its own defense either: it can rely on the underlying
context type.
regards, tom lane
Attachments:
[text/x-diff] v2-detect-double-free-with-requested_size.patch (3.6K, 2-v2-detect-double-free-with-requested_size.patch)
download | inline diff:
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index 161c2e2d3df..bea9e47ee4f 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -1175,6 +1175,10 @@ AllocSetFree(void *pointer)
link = GetFreeListLink(chunk);
#ifdef MEMORY_CONTEXT_CHECKING
+ /* Test for previously-freed chunk */
+ if (unlikely(chunk->requested_size == InvalidAllocSize))
+ elog(WARNING, "detected double pfree in %s %p",
+ set->header.name, chunk);
/* Test for someone scribbling on unused space in chunk */
if (chunk->requested_size < GetChunkSizeFromFreeListIdx(fidx))
if (!sentinel_ok(pointer, chunk->requested_size))
@@ -1373,6 +1377,10 @@ AllocSetRealloc(void *pointer, Size size, int flags)
oldchksize = GetChunkSizeFromFreeListIdx(fidx);
#ifdef MEMORY_CONTEXT_CHECKING
+ /* Test for previously-freed chunk */
+ if (unlikely(chunk->requested_size == InvalidAllocSize))
+ elog(WARNING, "detected realloc of freed chunk in %s %p",
+ set->header.name, chunk);
/* Test for someone scribbling on unused space in chunk */
if (chunk->requested_size < oldchksize)
if (!sentinel_ok(pointer, chunk->requested_size))
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index 9077ed299b4..fe9d087c85e 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -762,6 +762,10 @@ GenerationFree(void *pointer)
}
#ifdef MEMORY_CONTEXT_CHECKING
+ /* Test for previously-freed chunk */
+ if (unlikely(chunk->requested_size == InvalidAllocSize))
+ elog(WARNING, "detected double pfree in %s %p",
+ ((MemoryContext) block->context)->name, chunk);
/* Test for someone scribbling on unused space in chunk */
Assert(chunk->requested_size < chunksize);
if (!sentinel_ok(pointer, chunk->requested_size))
@@ -867,6 +871,10 @@ GenerationRealloc(void *pointer, Size size, int flags)
set = block->context;
#ifdef MEMORY_CONTEXT_CHECKING
+ /* Test for previously-freed chunk */
+ if (unlikely(chunk->requested_size == InvalidAllocSize))
+ elog(WARNING, "detected realloc of freed chunk in %s %p",
+ ((MemoryContext) set)->name, chunk);
/* Test for someone scribbling on unused space in chunk */
Assert(chunk->requested_size < oldsize);
if (!sentinel_ok(pointer, chunk->requested_size))
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index bd00bab18fe..0d1e6285c29 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -539,6 +539,7 @@ SlabAllocSetupNewChunk(MemoryContext context, SlabBlock *block,
MemoryChunkSetHdrMask(chunk, block, MAXALIGN(slab->chunkSize), MCTX_SLAB_ID);
#ifdef MEMORY_CONTEXT_CHECKING
+ chunk->requested_size = size;
/* slab mark to catch clobber of "unused" space */
Assert(slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ));
set_sentinel(MemoryChunkGetPointer(chunk), size);
@@ -748,11 +749,17 @@ SlabFree(void *pointer)
slab = block->slab;
#ifdef MEMORY_CONTEXT_CHECKING
+ /* Test for previously-freed chunk */
+ if (unlikely(chunk->requested_size == InvalidAllocSize))
+ elog(WARNING, "detected double pfree in %s %p",
+ slab->header.name, chunk);
/* Test for someone scribbling on unused space in chunk */
Assert(slab->chunkSize < (slab->fullChunkSize - Slab_CHUNKHDRSZ));
if (!sentinel_ok(pointer, slab->chunkSize))
elog(WARNING, "detected write past chunk end in %s %p",
slab->header.name, chunk);
+ /* Reset requested_size to InvalidAllocSize in free chunks */
+ chunk->requested_size = InvalidAllocSize;
#endif
/* push this chunk onto the head of the block's free list */
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19438: segfault with temp_file_limit inside cursor
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox