Re: BUG #19438: segfault with temp_file_limit inside cursor

On Sat, 28 Mar 2026 at 06:41, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> In MEMORY_CONTEXT_CHECKING builds, we can cheaply detect double frees
> by using the existing behavior that requested_size is set to
> InvalidAllocSize during AllocSetFree.  Another plausible idea is to
> change a freed chunk's MemoryContextMethodID to something invalid,
> which'd permit detection of double frees even in
> non-MEMORY_CONTEXT_CHECKING builds.
>
> I made draft patches showing how to do it both ways.  (Both patches
> pass check-world and are able to detect the bug in v17.)  The
> methodid-change way seems like the better alternative to me,
> but it is more invasive and does add a cycle or two when freeing or
> reusing a chunk.

I do think it's quite nice that we can detect the double free in
production builds by switching the MemoryContextMethodID to an unused
one. However, I did spend quite a bit of time making all that code as
fast as possible. For example, storing the freelist index in the chunk
header rather than the size, just to save the (pretty cheap)
AllocSetFreeIndex() call during pfree to get the freelist index from
the chunk size. That sort of thing was done because I could measure a
speedup from doing it.

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%.

select run,pg_allocate_memory_test(8,512,1024::bigint*1024*1024,'aset')
as seconds from generate_Series(1,3) run;

master
 run | seconds
-----+----------
   1 | 0.823345
   2 | 0.834834
   3 | 0.835506

patched
 run | seconds
-----+----------
   1 | 0.887794
   2 | 0.884866
   3 |  0.88592

I would rather see us using the requested_size method in
MEMORY_CONTEXT_CHECKING enabled builds.

Thanks for working on the patches.

David